Fix children served with kits calculation

[coalest]

Resolves #4915

Description

Fixes the children served with kits in disposable calculation.

Note: The old logic was attempting to count all the kits distributed in a given year with disposable items in them. I refactored it to calculate how many children were served by the disposable items inside the kits in a given year. I think this makes more sense, but it might be unexpected for stakeholders hence this warning.

Regarding the implementation, I saw two possibilities here:
1. Rewrite the query in SQL. Con: We would have to repeat the Item.disposable scope logic.
2. Fix it using only Active Record. Pro: Reuse the item.disposable logic so everything is dry. Con: For the sum logic I am referencing table aliases directly and this might be brittle.

I went with option number 2 to keep things dry (in case the logic of Item.disposable changes in the future), but if option 1 is preferred I can refactor this using SQL.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Modified this report service spec.

[cielf]

@coalest I haven't dug into your code as yet -- but for clarity it should be based on the number of kits that have disposable items in them -- assuming 1 kit per child unless the kit item has a different quantity per individual on it -- rather than the number of disposable items in the kits (as your note seems to suggest).

I don't know if this will make a big difference 'in the wild', but I can imagine use cases where it would undercount the number of children helped if we count the contents rather than the kits.

[coalest]

Ah ok, I didn't realize there was a general 1 kit per child rule. I will refactor this then.

[cielf]

Hey @coalest

Running a fairly basic example with 1 kit per child, and 240 kits of 25 size 1 diapers and 2 baby wipes. Assuming 1 kit per child, this would give us 20 extra children per month.

The number of diapers went from 32876 to 38876, as expected.

But the number of children served monthly went from 54 to 113? Something doesn't seem right there.

(Also, if I change the quantity per individual on the kit item from the default to 3, the number of children served monthly doesn't change)

[coalest]

I'll have to take a look at why the monthly average is 3 times higher than it should be.

Also I thought the quantity per individual for kits didnt matter, because we were assuming one kit per child. So i will have to adjust that

[cielf]

"unless the kit item has a different quantity per individual on it "

[coalest]

Where does the "quantity per individual" value come from for a kit? I don't see something like that on the Kit model, and if it's supposed to come from items in the kit then i don't understand how to calculate it when a kit can have multiple items in it.

[cielf]

There is an item is created when you create a kit, which represents the kit when you are picking things, and is the thing that is in the line items for the distribution. I'm not positive without looking, but I would think there would be a link to that item in the kit model.

That's what I mean when I say "kit item" above.

[coalest]

Ah I see, thanks for the clarification. I get it now.

[coalest]

@cielf I think I have the calculation right this time. Ready for review again.

[cielf]

Might not get to this for a couple of days (needs time for proper testing)

[cielf]

@coalest -- Interim report -- ok it looks to me like the kit part is basically working in that it works with 600 of a single kit (and it works with a quantity of 2 kits per individual as well). I haven't tested numbers that won't come out even or multiple kits yet. But I can confirm that you've got the basic idea right!

The loose part -- not so much -- I'm getting 235 total children for 12001 diapers with blank quantites per individual at the moment - which I expect is somehow due to the integer arithmetic issue, multiplied over several items? I'm ok with doing that as a separate PR -- but fixing that is realistically higher priority (in that it affects more banks) than the kits calculation.

I haven't reviewed the code itself, so I don't know if those same issues will turn up with the kits.

[cielf]

@coalest -- Update : I tried the above case with 7 (the smallest number that doesn't go evenly into 600) kits per individual. I got the monthly children served I more or less expected (though I would think we should ceiling rather than floor that calculation) but the total children served came out as 320 when I expected 321 (and again, that we should ceiling rather than floor that -- for these purposes a fractional child is a child).

To test the kits in isolation, i think I'm going to have to create a new company and just do kits. (sigh). Not sure I'll get to that today, and I'm away tomorrow.

[cielf]

I added another kit -- 75 premies, 2 wipes -- with 3 quantity -- distributed 100 -- and the gap between the expected total children served and the reported widened -- which I kind of expected, to tell the truth. That makes sense if it's a problem regarding division that's repeated for each item -- is it?

[coalest]

@cielf i added a commit to round individual children served to integer ceilings, and rounding average monthly to 2 decimal places.

[coalest]

I think there might be an issue with the calculation when a kit has multiple items in it, we may be getting duplicate counts. I need to look into that further.

[coalest]

Ok, there was an issue if a kit had multiple disposable items inside it, then it would result in duplicate rows, so therefore duplicate children served for those kit items.

My last commit fixes the issue, so now this should be ready for another round of testing.

The old spec file wasn't setting up the kits and kit items correctly, so I didn't catch this earlier.