Skip to content

Conversation

@jmgasper
Copy link
Collaborator

@jmgasper jmgasper commented Nov 3, 2025

@jmgasper jmgasper requested a review from kkartunov as a code owner November 3, 2025 21:23
}
// for other cases, take the `actualEndDate` as phase is already closed
return new Date(phase.scheduledEndDate || phase.actualEndDate);
return new Date(phase.actualEndDate || phase.scheduledEndDate);
Copy link

Choose a reason for hiding this comment

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

[❗❗ correctness]
The change in logic from phase.scheduledEndDate || phase.actualEndDate to phase.actualEndDate || phase.scheduledEndDate could potentially alter the behavior of the function. Ensure that this change is intentional and that actualEndDate should indeed take precedence over scheduledEndDate in all scenarios.

}
// For all other cases, take the `actualStartDate` as phase is already started
return new Date(phase.scheduledStartDate || phase.actualStartDate);
return new Date(phase.actualStartDate || phase.scheduledStartDate);
Copy link

Choose a reason for hiding this comment

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

[❗❗ correctness]
Similar to the change in phaseEndDate, the logic here has been altered from phase.scheduledStartDate || phase.actualStartDate to phase.actualStartDate || phase.scheduledStartDate. Verify that this change is intentional and that actualStartDate should take precedence over scheduledStartDate in all cases.

@jmgasper jmgasper merged commit 7b42471 into master Nov 3, 2025
9 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.

3 participants