Add filters to distribution by county and take into account kitted items#5541
Add filters to distribution by county and take into account kitted items#5541cielf wants to merge 9 commits intorubyforgood:mainfrom
Conversation
… filters, restricting item filter to loose items only, added text re kitted items. Wondering about SQL portability from local to prod.
|
Couple of things right off the top: |
dorner
left a comment
dorner
left a comment
There was a problem hiding this comment.
I've never been able to follow the logic with these complex reports. If it works I'll accept that it works. 😁 What did you mean by whether the SQL was "portable"?
| @reporting_categories = Item.reporting_categories_for_select | ||
| @items = current_organization.items.loose.alphabetized.select(:id, :name) | ||
| @selected_reporting_category = filter_params[:by_reporting_category].presence | ||
| @selected_item = filter_params[:by_item_id].presence |
There was a problem hiding this comment.
Can we bundle these into a view object? I'd like us to use more of these rather than relying on a bunch of instance variables.
There was a problem hiding this comment.
We use View::Donations.in donations#index.
There was a problem hiding this comment.
(nods) Ok - I see we have an example with Donations, so I'll see if I can apply that.
| ) | ||
| end | ||
|
|
||
| helper_method \ |
There was a problem hiding this comment.
Might be more readable if you just define the filter_params method and call helper_method :filter_params at the top.
| end | ||
| end | ||
| end | ||
| context "with kits only" do |
One of the things that has not stayed in my head is whether we are using postgres in production , and I know there are some differences between the SQL used by different relational dbs -- the element that might be of concern is the CASE construct . |
|
We're using postgres in production :) |
2 participants
Description
We've had a request to make the distribution by county more useful. This
Still to be done
Documentation update
Type of change
How Has This Been Tested?