-
-
Notifications
You must be signed in to change notification settings - Fork 719
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
Maintain date range values in the report forms #11732
Maintain date range values in the report forms #11732
Conversation
1168a33
to
3195d17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one ! Thanks for your help 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, this is great!
- start_date ||= params[:q].try(:[], :completed_at_gt) | ||
- end_date ||= params[:q].try(:[], :completed_at_lt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hashes have a nice fail-safe method to access values that may be there:
- start_date ||= params[:q].try(:[], :completed_at_gt) | |
- end_date ||= params[:q].try(:[], :completed_at_lt) | |
- start_date ||= params.dig(:q, :completed_at_gt) | |
- end_date ||= params.dig(:q], :completed_at_lt) |
fix typo
3195d17
to
72089d8
Compare
Hi @yasirazgar, I have staged your PR to one of our servers an compared before and after. I found that before staging the Customers report as well as the Enterprise Fee Summary report were not keeping the date selection. All other reports did. Before stagingCustomers report after clicking 'Go': Enterprise Fee Summary report after clicking 'Go': After stagingCustomers report after clicking 'Go': Enterprise Fee Summary report after clicking 'Go': We can see that the date selection is now kept. 💪 I have also checked that
ResultsExcellent! I couldn't find any issue with this pull request! 🥳 Thanks again! |
What? Why?
Date range selection needs to maintained after form submission
What should we test?
Date range should be maintained after form submission
customer.mov
customer.pack.mov
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates