Conversation
mdworken
left a comment
mdworken
left a comment
There was a problem hiding this comment.
Looks awesome! It might be good to add tests re: active vs inactive line items if there aren't any yet.
|
|
||
| def selected_range | ||
| now = DateTime.now | ||
| now = Time.zone.now |
There was a problem hiding this comment.
I'm surprised Rubocop didn't complain about this sooner.
There was a problem hiding this comment.
IKR?
Actually -- you know I bet it did, but that it's in the rubocop ratcheting ignore file that Brock made for us a while back
app/views/dashboard/index.html.erb
Outdated
| </div> | ||
| <div class="box-body text-center float-center"> | ||
| <h3 class="text-center"><%= @recent_donations.by_source(:diaper_drive).count %> Diaper Drives <%= display_interval %> for a total of <%= number_with_delimiter(@recent_donations.by_source(:diaper_drive).sum { |d| d.line_items.total }) %> items collected</h3> | ||
| <h3 class="text-center"><%= @recent_donations.by_source(:diaper_drive).map(&:diaper_drive_participant).uniq.size %> Diaper Drives <%= display_interval %> for a total of <%= total_received_from_diaper_drives %> items collected</h3> |
There was a problem hiding this comment.
Do you want to call .uniq on this? If they had 20 diaper drives and 15 of those were at one location won't it show as 6 diaper drives in the period not 20?
There was a problem hiding this comment.
I think this is one of the areas that we need to revisit with Diaper Drives. The way we have it, it could be entered in a way that treats Diaper Drive Participants as either:
- the diaper drive itself, assuming diaper drives are separate efforts (so all donations coming in from diaper drive X get associated with Diaper Drive Participant X)
- an organization that contributes to a unnamed diaper drive (Diaper Drive Participants X, Y and Z all contribute to a general "diaper drive")
it's possible that I just don't have a correct understanding of how it works, too :) I think the UI should provide a little more guidance about the expectation of what the things mean
Oh -- I do agree that the language is incorrect, though -- it says "diaper drives" but really it's tracking "number of diaper drive participants" -- I'll fix that now so at least the language is consistent.
I don't know how to track how many individual diaper drives there are (assuming they can be separate things and are not just an ongoing effort).
| organization { Organization.try(:first) || create(:organization) } | ||
| issued_at { nil } | ||
| amount_spent_in_cents { 1000 } | ||
| amount_spent_in_cents { 10_00 } |
…es Audit factory, begins expanding and improving Dashboard test coverage. Adds time zone specificity to datetime helpers in dashboard helper. Renames a couple of the date range names
…re it was counting the number of diaper drive donations as the number of participants, and the total was being limited to the recent total only.
…uage changes. Moves Date range filter down closer to the area where it actually affects. Cleans up the remaining specs that were lingering. There are a few examples marked pending because I'm not sure how to handle them. Will create an issue.
…s to need tightening. As did the changes to the Donation factory.
armahillo
force-pushed
the
972-incorrect-diaper-drive-numbers
branch
from
23ecb75 to
d18ec3b
Compare
3 participants
This accomplishes 2 things:
Closes #972 - it addresses the bug that @pdxdiaperbank reported
Ref #1024 - it expands test coverage substantially for the Dashboard.
We didn't really have any good test coverage for when the user wanted to filter by the different parameters. there were a few tests, but nothing exhaustive. This adds some pretty robust tests for each of the conditions. It also renames a couple of the options to make it a bit more clear what the users can expect.
This PR also fixes a couple minor bugs in factories (line items were creating duplicate itemizables when they were built in factories -- whoops!).
I need to finish cleaning up the file still, but it's nearly done.