Calculating disposable diapers from kits #4017
Conversation
…king into account disposables in kits.
…-essentials into 3989_diapers_in_kits
jadekstewart3
changed the title
There was a problem hiding this comment.
Hey @jadekstewart3 Thank you very much for making the change to the variable names! It made it a lot easier for me to reason about the rest of the code.
Alas, it looks like I wasn't clear enough in describing the relationship between kits and children, so there's going to be a chunk of rework there. (See the inline comments)
I didn't look closely at the tests yet -- I'm holding off until the kids/kits stuff is addressed.
I'll try to keep an eye on the notifications here over the next couple of days if you have questions , or want to check that we really are on the same page - it's usually quicker to poke me on slack, though.
… kits that contain disposable items
|
@jadekstewart3 Hmmm.... doesn't seem to be including the kits in. Here's the sequence I tried. From a fresh seed: Thank you for sticking with this through a fairly long haul! |
|
(Pokes into the code a bit) Looks like you might only be using the magic SQL for the children served service, and not for the aquisition_report_service. That would explain it. |
|
@cielf I am trying to implement the magic sql in the acquisition report and something is funky. Can you take a look? when I test the distributed_disposable_diapers_from_kits it seems to return 100, but in the full report it is returning 1500? |
|
@jadekstewart3 there seems to be some flakiness here - the order of tests seems to matter. If you run it with |
|
I think this has to do with it: disposable_item = organization.items.disposable.first
non_disposable_item = organization.items.where.not(id: organization.items.disposable).firstRather than creating data from scratch, you're relying on items that were created by the factory/seed. To be sure that only the data you care about is being created, you probably should clear out all the items and only create the ones you want to see in the report. |
…o below the creation of the kits, but still getting the same out of range value
| AND LOWER(base_items.category) LIKE '%diaper%' | ||
| AND NOT (LOWER(base_items.category) LIKE '%cloth%' OR LOWER(base_items.name) LIKE '%cloth%') | ||
| SQL | ||
|
|
There was a problem hiding this comment.
We need to make sure that the itemizable type on kit_line_items is Kit, don't we? Isn't there a kit # 1 and a distribution # 1 and a purchase # 1 and a donation # 1?
There was a problem hiding this comment.
@cielf Ill look into this today! I think you may be on to something!
|
@dorner I am getting a return value of 1500 for the sql query that is calculating distributed disposable diapers from kits. I've gone over my test set up a few times and created the kits and kit items in a similar fashion to the set up in the children_served_report_service_spec, but I cant quite figure out why we are returning 1500 instead of the expected 100. |
|
Hey @jadekstewart3 -- did you look into adding in a clause to in your sql to make sure that the itemizable type for the kit line items is kit? |
|
@cielf I was messing with it yesterday but still having troubles, but I think I just got it! |
…modify test expectation
|
@cielf I think this is working! It looks to be failing on a test file that doesnt have anything to do with the reports! 😃 |
|
It passes my functional test (adding a mixed diaper and kit-with-diapers distribution) with flying colours! Hurrah! |
|
Hey @dorner -- Just checking if you have any last comments on this -- you've chimed in, but more on a problem solving than critiquing the code pov. Thanks! |
| .distributions | ||
| .for_year(year) | ||
| .joins(line_items: :item) | ||
| .merge(Item.disposable) | ||
| .sum('line_items.quantity') | ||
| # returning 220 |
There was a problem hiding this comment.
I think this comment shouldn't be here?
| result = ActiveRecord::Base.connection.execute(sanitized_sql) | ||
|
|
||
| result.first['sum'].to_i | ||
| # returning 1500 |
| # .merge(Item.disposable) | ||
| # .where.not(items: {kit_id: nil}) | ||
| # .sum("line_items.quantity") | ||
| # end |
There was a problem hiding this comment.
Let's not leave commented-out code in the repo. If we need it, we can always use git history to get it back. 😄
|
I should probably take this out for one last spin before we merge it. [Edit: Actually -- no -- if it's only comment removal, I don't need to.] |
|
Holding off on the merge until the release of April 7 is done. |
|
@jadekstewart3: Your PR |
-->
Resolves #3989
Description
I made modifications to the acquisition report service and the children served report service to take disposable diapers distributed in kits in the annual report. I have added/ modified tests to reflect the changes to the calculations.
Type of change
How Has This Been Tested?
Tested in the
spec/services/reports/children_served_report_service_spec.rbandspec/services/reports/acquisition_report_service_spec.rb