-
-
Notifications
You must be signed in to change notification settings - Fork 516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
4513 incontinence supplies #4794
base: main
Are you sure you want to change the base?
4513 incontinence supplies #4794
Conversation
…te zero values test
… and distribution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jadekstewart3 -- the adults served isn't coming up as described in the issue. Could you take a look, please?
.merge(Item.adult_incontinence) | ||
.where.not(items: {kit_id: nil}) | ||
.sum('line_items.quantity / COALESCE(items.distribution_quantity, 50)') / 12 | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per the issue, "
The number of adults assisted with kit items is the number of kits that contain adult incontinence supplies distributed divided by the "quantity per individual" on the kit item. if there is no quantity per individual provided, assume that quantity is 1."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick question, also I apologize for missing that bit! Is it a safe assumption that the distribution_quantity is equal to the "quantity per individual"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Sorry for the terminology ambiguity -- they are the same.
…ibed in issue, and update tests
e64abc5
to
607bec6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jadekstewart3 - I distributed 100,000 of a kit that contained 50 - 100 of a few different AI products. The supplies distributed looks plausible, but the adults assisted per month and adult incontinence supplies per adult per month didn't budge from what they were before the distribution. That doesn't seem right.
Can you track that down?
@cielf Okay, I think I have tracked down the issue! Let me know if you run into any snags and Ill address it asap, I know we are in crunch time for this report! |
.merge(Item.adult_incontinence) | ||
.where.not(items: {kit_id: nil}) | ||
.sum('line_items.quantity / COALESCE(items.distribution_quantity, 1)') | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this is giving you kits that are identified as adult_incontinence, rather than kits that have adult incontinence items in them? Though you used the same pattern as for the disposables... Huh.
Hey @jadekstewart3 There's definitely something wrong with the diapers as well. There was a change re the kits recently, so I'm wondering if that's what's going on. Stay tuned (though, for expectations, I can't work on this tomorrow) |
Hey @jadekstewart3 -- One thing I notice is that this is based off a version of main from April. There have been an awful lot of changes since then. Could you rebase please? |
… 4513_incontinence_supplies
…l_people_served_with_loose_supplies_per_month
…increasing in value
I should be able to look at this again tomorrow |
@jadekstewart3 Taking a look now, it's still awry (I don't know what the adults assisted per month should be without calculating it, but I'm sure it's not 1) First level of pokingat it -- the number assisted with loose looks plausible. (I added 6000 loose, and this number went up by 10. So that tradks.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current state is that the number of supplies from kits needs adjusting to match the item scope adult_incontinence. I think we're going to have to go with a sql call to get the total people served with kits, and have left some thoughts to pursue on that. Good Luck!
…fiddle with total_people_served_with_supplies_from_kits_per_month to no avail, yet.
…er_month using sql
Hi @dorner. I still need to do more manual testing on this, but it's time to bring you in. Status: as of Friday late afternoon, Jade thinks this is working in real life, but is having trouble with some of the testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't quite figured out quite what the right SQL statement for the number of adults helped is, but wanted to get some steps toward it out there before @dorner takes a look.
…incontinence_items_from_kits still noodling total_kits_with_adult_incontinence_items_distributed
…istributed kits being returned. Taking another look at report calculations to ensure accuracy
…n of kits with extra objects
@jadekstewart3 The number of adults assisted from kits is not coming through correctly. I think the best approach may be to break that down into steps -- determine which kits have A.I. in them, then get the number of adults assisted by those kits -- because the SQL we have is very complicated indeed, and hard to think about. |
Noting things as i go along on this pass. |
@jadekstewart3 Hrm. I added a kit with 25 Adult Brief ML, 1 Wipes(Adult), and added a distribtuion of 120 of those in 2024. Recalculated the report, and saw no change in the Adults Assisted per month. I expected the Adults Assisted Per month to go up by 10 (default 1 kit per individual). |
Checklist:
X I have performed a self-review of my own code,
X I have commented my code, particularly in hard-to-understand areas,
X I have added tests that prove my fix is effective or that my feature works,
X New and existing unit tests pass locally with my changes ("bundle exec rake"),
X I acknowledge that I will not force push my branch once reviews have started.
-->
Resolves #4513
Description
Made all necessary changes to the incontinence annual survey including:
Note: when creating a kit using the current factory, it creates an extra item (with the correct base category that includes adult but was not previously included in the 'Adult incontinence supplies' list. I've added that item to the test because it is technically correct. Let me know if you would like to see a different course of action, and I will be happy to fix it :)
Type of change
How Has This Been Tested?
I have created a kit with items and distribution to the test file to ensure that the calculations are indeed including kits and kit items