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

Use correct end date for test run depending on org set timezone. #870

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

apognu
Copy link
Contributor

@apognu apognu commented Feb 24, 2025

Creating a test run had two issues related to the end date selection:

  • The end date would always be at the start of the selected day, where instinctively, you would think that selecting an end date of "Feb 27th" would still allow the test run to continue on the 27th.
  • The frontend sends us a time relative to the browser's locale, which may or may not be the same as the organization's set timezone. The "end of day" calculation must therefore be performed in the org's timezone, but from the selected date (the user's intent).

This PR sets the end date and time for a test run as the end of the selected day in the organization's timezone.

@apognu apognu added bug Something isn't working go Pull requests that update Go code labels Feb 24, 2025
@apognu apognu self-assigned this Feb 24, 2025
@apognu apognu force-pushed the fix/test-run-end-date branch from 78d0e77 to 6f25551 Compare February 24, 2025 08:23
@apognu apognu marked this pull request as ready for review February 24, 2025 08:38
Copy link
Contributor

@Pascal-Delange Pascal-Delange left a comment

Choose a reason for hiding this comment

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

👍
(or could we have done the "enrich and adapt" logic directly in the usecase? it's not a big deal, but to avoid leaking this logic into the controller. It's really up to you though)

@apognu
Copy link
Contributor Author

apognu commented Feb 25, 2025

👍 (or could we have done the "enrich and adapt" logic directly in the usecase? it's not a big deal, but to avoid leaking this logic into the controller. It's really up to you though)

This is a good question, though. This bit of logic is very specific to getting the input from a client which timezone we do not control, and that only gives the option for "accurate-to-the-day" timestamps, which is why I added it to the specific DTO that comes from the browser.

Adding it to the usecase, in my opinion, would couple the business logic to how the browser clients work, which would be a bad thing in my opinion (for example if we need, at some point, to allow creating test runs ending at specific times).

@apognu
Copy link
Contributor Author

apognu commented Feb 25, 2025

Poke @Pascal-Delange for opinion.

@apognu apognu force-pushed the fix/test-run-end-date branch from 6f25551 to eb2ca4a Compare February 25, 2025 12:52
@Pascal-Delange
Copy link
Contributor

@apognu thanks, makes sense !

@apognu apognu force-pushed the fix/test-run-end-date branch from eb2ca4a to 21e0902 Compare February 26, 2025 11:25
@apognu apognu merged commit d160afb into main Feb 26, 2025
3 checks passed
@apognu apognu deleted the fix/test-run-end-date branch February 26, 2025 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants