-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
3732 - Extract Donations (All Sources) from Dashboard #3795
3732 - Extract Donations (All Sources) from Dashboard #3795
Conversation
5f799be
to
e3d712d
Compare
Create new route/controller for Reports::DonationsSummary that presents all information that was previously available on the Dashboard under Reporting & Auditing instead.
Reporting & Auditing was not expanding for most of the navigation items in the list. In addition, the currently selected report was not highlighted. active_class and menu_open? now check if the listed path is in the controller_path instead of doing an exact match on controller_path. In addition, the report nav items have been updated to reference their own path, so all reports aren't listed as active when one is selected.
e3d712d
to
39af23b
Compare
travel_to(test_time) | ||
example.run | ||
travel_back |
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.
For a minute I got confused because I'm used to TimeCop where travel
is different than freeze
. But we're not using that in this project! That said, this could be slightly different:
travel_to(test_time) | |
example.run | |
travel_back | |
travel_to(test_time) do | |
example.run | |
end |
... but also since I had to read the docs to learn how this isn't TimeCop I see that travel_to
automatically adds a travel_back
, so you could do a before
instead of around
.
All that said -- the way it's written works and I'd like to get this merged, so I might merge it.
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.
I just moved the existing spec and simplified it to avoid using a PageObject. I didn't mess with the time travel.
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.
Great, thank you!!
Ah ... I didn't realize that the tests hadn't run on this. I'll fix them up in another branch I'm doing. |
Sorry about that. I'm too used to CI too. We can do a revert PR, and I can fix the specs up later this weekend. I think it's the |
Naw is ok -- I'm going backwards on the nav changes, but otherwise I think it is mostly removing the dashboard specs. I'll let you know if it doesn't come out cleanly but so far seems fine |
@ChaelCodes: Your PR |
Resolves #3732
Description
We want to extract the "Donations (All Sources)" card from the dashboard and move it to a Donations Summary page.
Type of change
How Has This Been Tested?
/reports/donations_summary
Tested manually, and tested auth using request specs, and tested using system specs.
Screenshots