Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix incorrect setting of "archived" status for events of course #2344

Merged
merged 3 commits into from
Nov 6, 2023

Conversation

Alphajax
Copy link
Contributor

@Alphajax Alphajax commented Nov 6, 2023

🟒 Add deploy label if you want to deploy this Pull Request to staging environment

πŸ§‘β€βš–οΈ Pull Request Naming Convention

  • Title should follow Conventional Commits
  • Do not put issue id in title
  • Do not put WIP in title. Use Draft PR functionality
  • Consider to add area:* label(s)
  • I followed naming convention rules

πŸ€” This is a ...

  • New feature
  • Bug fix
  • Performance optimization
  • Refactoring
  • Test Case
  • Documentation update
  • Other

πŸ”— Related issue link

issue

πŸ’‘ Background and solution

Describe the big picture of your changes here
changed logic of getEventStatus function at CourseScheduleService
check if endDate exists before compare it with current date

β˜‘οΈ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Database migration is added or not needed
  • Documentation is updated/provided or not needed
  • Changes are tested locally

@Alphajax Alphajax changed the title fix: fix incorrect setting of "archived" status for events of course @Alphajax fix: fix incorrect setting of "archived" status for events of course Nov 6, 2023
@@ -360,7 +360,7 @@ export class CourseScheduleService {
private getEventStatus(courseEvent: CourseEvent) {
const startTime = (courseEvent.dateTime as Date).getTime();
const endTime = Number(courseEvent.endTime) ?? startTime + (courseEvent.duration ?? 60) * 1000 * 60;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw I am not sure this is correct. After conversion to number left part of this expression will never be null or undefined, so right part will not work, I think proper version looks like this, could you please fix it?

Suggested change
const endTime = Number(courseEvent.endTime) ?? startTime + (courseEvent.duration ?? 60) * 1000 * 60;
const endTime = Number(courseEvent.endTime) || startTime + (courseEvent.duration ?? 60) * 1000 * 60;

@valerydluski valerydluski merged commit 0303c37 into rolling-scopes:master Nov 6, 2023
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants