-
-
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
Fix issue #3324 to add donation source details #4045
Fix issue #3324 to add donation source details #4045
Conversation
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.
Looks good to me! Going to hold off on merging till Sunday just because we've already validated the contents of main
for a deploy tomorrow.
@elasticspoon question - when you say "your linter", do you mean the regular rubocop
gem included in the Gemfile? I'm a little confused as to why that gem would be making changes when the linter was already passing on main
previously.
1128dd5
to
e17e0fa
Compare
It's an html linter that doing it. Not sure it prettier, or eslint, or whichever lsp I have. I hope its not an issue? I usually let it do its thing because it does a good job catching unclosed html tags. |
Could you just run that linter against the entire codebase and put that up as a separate PR? And maybe we can look into adding it to our linting process in general. I'd rather keep this PR as just the change requested. |
e17e0fa
to
4013a67
Compare
This commit adds RSpec model, request and service specs for expected behavior of donation sources: Source column shows type of donation Details columns shows name for all but misc donations. Those show a truncated comment. Service specs test csv creation. Model specs test the newly added donation.details method.
Changes donation/index to have Source and Details columns for donations. Adds a details methods to a donation to provide appropriate details.
e7978d6
to
77be372
Compare
This hasn't been merged yet. Adding a note on the Sunday meeting agenda to actually merge it at the end. |
@elasticspoon: Your PR |
This PR changes the donations page and the CSV export on said page to do the following:
Resolves #3324
Note: know that it might look like there were a lot of changes in the views, but that was just my linter doing its thing.
Screenshots
Before:
After:
Type of change
How Has This Been Tested?
Added request tests in
spec/requests/donations_requests_spec.rb
Added model tests in
spec/models/donation_spec.rb
Added service tests in
spec/services/exports/export_donations_csv_service_spec.rb
Small Question:
I found the column names to be a bit confusing. What exactly is "In Kind Value"? Does it refer to the total value of the items that got donated? Would it make sense to reorder the columns to reflect that?
Before:
After: