human-essentials
human-essentials copied to clipboard
Is Org/Bank Dashboard's "Top Manufacturer Donations" list correct?
Summary
What is the intent of the Org/Bank Dashboard's Top Manufacturer Donations list? Is it both labelled and implemented as intended/desired?
Some (incomplete?) observations:
- Other dashboard sections have Recent <Thing>s lists
- Those lists are affected by the filtered date range
- The Top Manufacturer Donations list does not appear to be filtered/affected by the date range filter
- Unless I'm missing something, it does not appear to be sorted by any of
- Number of donations per manufacturer
- Total items donated by manufacturer
- Donation date
For example:
Given
Donations by these 10 manufacturers (#s 1 & 2 from rails db:seed
), all dated & created during the last week:
When
- I create a new donation on a new manufacturer (11) dated a year ago and
- I return to the dashboard and
- I filter to "This year"
Then The new donation appears at the top of the list, bumping off Manufacturer 1 which has both more donations and total items donated
Is that desired, correct, expected, etc? Should the automated test check the list items' order?
Implementation Details
- https://github.com/rubyforgood/human-essentials/blob/da1085c7edeb630e0814106e1d90950fd4adc075/app/views/dashboard/index.html.erb#L261-L262
- https://github.com/rubyforgood/human-essentials/blob/da1085c7edeb630e0814106e1d90950fd4adc075/app/controllers/dashboard_controller.rb#L28
- https://github.com/rubyforgood/human-essentials/blob/da1085c7edeb630e0814106e1d90950fd4adc075/app/models/manufacturer.rb#L23-L32
[Add/restore remaining sections if there is something to be done here]
So it appears the list is "correct" but probably not showing the information that is most useful and in the most useful way for the banks.
We probably want to change that manufacturer method your pointed out:
def self.by_donation_count(count = 10)
# selects manufacturers that have donation qty > 0
# and sorts them by highest volume of donation
select { |m| m.volume.positive? }.sort.reverse.first(count)
end
to be:
def self.by_donation_count(count = 10)
# selects manufacturers that have donation qty > 0
# and sorts them by highest volume of donation
select { |m| m.volume.positive? }.sort.first(count)
end
Remove the reverse
from it. Does this make sense? I'll defer to you since you definitely have much more context on the dashboard.
1/ I don't think it's correct -- as Bob described it, a donation from earlier is being included in this year's donations.
2/ I do think we should order them from largest donation to smallest
Oh -- the spec comments call out that it is deliberately including the before and after date range donations. This is not what I would expect looking at the page, though. If that is the intent, we should change the heading to indicate that it is "all time".
We need to re-verify that this is an issue.
Hmmm... I expect this is a using item creation date rather than donation date kind of issue --similar to https://github.com/rubyforgood/human-essentials/issues/3311
There are definitely some issues here, but I think we might want to write up a new issue for it that is clear and doesn't have this long comment trail which would be confusing to someone not-from-the-core-team. (And yes, I'll take care of it.)
I have made a proto-issue with a clear proposition -- am putting it in the "Needs Stakeholder Input" of the project, and adding it as a "Want Stakeholder input" item for the Feb 1 2023 meeting.
Closing this issue because its confusion.