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(*): correctly allow date ranges where start date is in future #2005

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

sleidig
Copy link
Member

@sleidig sleidig commented Sep 18, 2023

allow date ranges where start date is in future and no end date is set

⚠️ mark commit for @codo-mentoring

@github-actions
Copy link

Deployed to https://pr-2005.aam-digital.net/

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Collaborator

@TheSlimvReal TheSlimvReal left a comment

Choose a reason for hiding this comment

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

I am not sure that I can reproduce the bug. When I use the latest master, I can also enter or select a date range in the future.

@sleidig
Copy link
Member Author

sleidig commented Sep 20, 2023

hmm, interesting. In the demo I can't reproduce either @TheSlimvReal . But here and in a client system it is happening: https://qm.aam-digital.com/matching (maybe due to synced mode or because the "end" field is not part of the form?)

@TheSlimvReal
Copy link
Collaborator

@sleidig I think you can reproduce it in the demo. The actual fix to the problem has not yet been deployed (#1988) With the "All" filter option it is also possible to only have a start or end date.

@sleidig
Copy link
Member Author

sleidig commented Sep 20, 2023

@TheSlimvReal , indeed this bug only occurs if the "end" field is not part of the confirmation popup of the matching view.

@TheSlimvReal
Copy link
Collaborator

It seems like moment has a different behaviour depending on whether undefined or null is passed as end date.

@sleidig
Copy link
Member Author

sleidig commented Sep 21, 2023

It seems like moment has a different behaviour depending on whether undefined or null is passed as end date.

ah, that makes sense, I guess.
I'd suggest to go ahead with this PR though, as the fix makes the behavior more explicit and reliable instead of using a not very well known default of moment.js

@sleidig sleidig merged commit d7776fd into master Sep 21, 2023
6 checks passed
@sleidig sleidig deleted the end-date-validation branch September 21, 2023 15:02
@aam-digital-ci
Copy link
Collaborator

🎉 This PR is included in version 3.24.0-master.9 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@aam-digital-ci aam-digital-ci added the released on @master managed by CI (semantic-release) label Sep 21, 2023
@aam-digital-ci
Copy link
Collaborator

🎉 This PR is included in version 3.24.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@aam-digital-ci aam-digital-ci added the released managed by CI (semantic-release) label Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @master managed by CI (semantic-release) released managed by CI (semantic-release)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants