Skip to content
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

Children served not calculated correctly if there are kits #4915

Open
2 tasks
cielf opened this issue Jan 5, 2025 · 6 comments · May be fixed by #5027
Open
2 tasks

Children served not calculated correctly if there are kits #4915

cielf opened this issue Jan 5, 2025 · 6 comments · May be fixed by #5027

Comments

@cielf
Copy link
Collaborator

cielf commented Jan 5, 2025

Summary

The children served number is not including children served with kits appropriately

Why fix?

Consistency. Corrextness.

Details

Regarding the Annual Survey
This method:
def children_served_with_kits_containing_disposables
organization
.distributions
.for_year(year)
.joins(line_items: {item: :kit})
.merge(Item.disposable)
.where.not(items: {kit_id: nil})
.distinct
.count("kits.id")
end

counts kits that are disposables (which will be 0), not kits that have disposables

Fix it.

(You probably shouldn't take this one unless you are familiar with both the annual survey and kits. And you might need to do SQL for this. So it's a little advanced)

Criteria for completion

  • behaviour as decribed
  • tests to confirm the behaviour
@coalest coalest self-assigned this Feb 19, 2025
@github-actions github-actions bot removed the Help Wanted Groomed + open to all! label Feb 19, 2025
@coalest coalest linked a pull request Feb 19, 2025 that will close this issue
@coalest
Copy link
Collaborator

coalest commented Feb 19, 2025

While looking into this I noticed another small issue with the calculations.

For the calculation of total_children_served_with_loose_disposables, we are doing division between integers so we are effectively rounding down for each item. So we could have 100 line items each with a quantity of 49, but if the distribution quantity of those items is 50, then we will get a total_children_served_with_loose_disposables value of 0.

We then use this sum to calculate the average monthly children served (converted to a float), so the average_children_monthly would be 0.

Is this expected behavior?

@cielf
Copy link
Collaborator Author

cielf commented Feb 19, 2025

We probably should do a float division.

@coalest
Copy link
Collaborator

coalest commented Feb 25, 2025

@cielf I will make another PR for that. Double checking these calculations in this PR is already complicated enough imo

@cielf
Copy link
Collaborator Author

cielf commented Feb 26, 2025

@coalest Hrm. Yes. I don't care for the numbers that I'm currently getting for non-kitted diapers. All the "quantity per individual" is showing as blank for all, so with 12001 diapers, I would think I would get 240 total children served. I'm getting 235. So the float division is pretty important, actually -- Given the normal use, it's actually higher priority than getting the kits. right. But on with the kits...

@coalest
Copy link
Collaborator

coalest commented Feb 26, 2025

I can throw the floating point change into this PR if you prefer.

@cielf
Copy link
Collaborator Author

cielf commented Feb 26, 2025

I think what we want is to do the division as floating point, and then do a ceiling on that value -- at least for the total children served. A fractional child is a child for these purposes. Display total children helped as an integer.

The average children helped -- displaying that as a float (maybe 2 decimals) would make sense.

And yeah -- it will actually be easier, I think, to reason about this if we address the error in the 'loose' numbers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants