human-essentials icon indicating copy to clipboard operation
human-essentials copied to clipboard

Include inactive items in distribution reports

Open cielf opened this issue 1 year ago • 32 comments

Summary

Include inactive items in reports of items that have been distributed

Why

Accuracy -- any now-inactive items that were distributed were, indeed, distributed!

Details

Currently, inactive items are included in the distribution totals in the distribution index, but not in the distributions reports (under Reports, when you are logged in as [email protected]). These reports include Distribution by County and Itemized distributions

Criteria for completion

  • [ ] Inactive items included in Distribution by County report
  • [ ] Inactive items included in Itemized Distributions report
  • [ ] Test to demonstrate this is so

cielf avatar Jul 14 '24 15:07 cielf

Hi! I'm new to this project, and I would love to work on this issue. Looking forward to contributing!

donaldwilson8 avatar Aug 24 '24 22:08 donaldwilson8

Welcome aboard -- it's yours!

cielf avatar Aug 25 '24 01:08 cielf

I've been looking into this, and here are my findings.

  • Comparing the total item counts from the distribution index (/distributions) to the distributions by county and itemized distribution reports, I only found a difference of 1 item (119,311 vs 119,310).
  • I wrote a test for the distributions by county report service, which created a distribution with inactive items. The service processed this distribution and returned a breakdown that correctly accounted for these inactive items.
  • I looked through the relevant models, controllers, and services, and I didn’t find any occurrence of items being filtered by the active scope.

Could I please have some guidance on this issue? Am I overlooking something, or does this mean that inactive items are being processed as desired within these reports?

Thanks for your help!

donaldwilson8 avatar Aug 27 '24 05:08 donaldwilson8

Hi @donaldwilson8 -- I will double check this, but it might take me a couple of days to get to it. Thanks for your patience!

cielf avatar Aug 27 '24 15:08 cielf

Checking each report -- does it currently include inactive items ? Distributions Summary -- no Distributions by county -- yes Distributions itemized -- yes

It looks like when we initially wrote down the problem, we got it inside out. The distributions summary should include inactive items to be consistent with the rest of the distribution reports.

My apologies for the mixup.

cielf avatar Aug 28 '24 22:08 cielf

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

github-actions[bot] avatar Sep 28 '24 00:09 github-actions[bot]

Automatically unassigned after 7 days of inactivity.

github-actions[bot] avatar Oct 05 '24 00:10 github-actions[bot]

hi, can i please solve this issue?

prashu0705 avatar Oct 18 '24 05:10 prashu0705

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

github-actions[bot] avatar Nov 20 '24 00:11 github-actions[bot]

Automatically unassigned after 7 days of inactivity.

github-actions[bot] avatar Nov 27 '24 00:11 github-actions[bot]

Available again.

cielf avatar Nov 27 '24 13:11 cielf

Hello, I had a look at this issue per your recommendation, could the issue be this : app>controllers>reports_controller.rb def total_distributed_unformatted(range = selected_range) LineItem.active.where(itemizable: current_organization.distributions.during(range)).sum(:quantity) end

It's only selecting the active items ?

Oce-ane avatar Feb 02 '25 11:02 Oce-ane

I'm tentatively reserving this one for @rrg1459 -- as it's the last Good First Issue at the moment (and @Oce-ane is working on other things).

cielf avatar Feb 03 '25 14:02 cielf

But to answer @Oce-ane's query -- LineItem .active does grab only those line items that have active items -- so it's plausible!

cielf avatar Feb 03 '25 15:02 cielf

I'm tentatively reserving this one for @rrg1459 -- as it's the last Good First Issue at the moment (and @Oce-ane is working on other things).

Please let me break it down, it seems to be a bit more complex than it appears.

rrg1459 avatar Feb 03 '25 17:02 rrg1459

Alright - you can put in a draft PR if need be, and let us know if you have any questions.

cielf avatar Feb 03 '25 23:02 cielf

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

github-actions[bot] avatar Mar 06 '25 00:03 github-actions[bot]

Automatically unassigned after 7 days of inactivity.

github-actions[bot] avatar Mar 13 '25 00:03 github-actions[bot]

Hello! Is this available? If so, could you assign this to me?

silverjellyfish avatar Jul 21 '25 02:07 silverjellyfish

You've got it!

cielf avatar Jul 21 '25 16:07 cielf

Hi @cielf , I have a small doubt regarding the total items distributed. I tried the following query:

SELECT 
  SUM(quantity) AS total_quantity
FROM (
  SELECT 
     d.id,
     COALESCE(SUM(li.quantity), 0) AS quantity,
     COALESCE(SUM(COALESCE(i.value_in_cents, 0) * li.quantity), 0) AS value
FROM distributions d
LEFT OUTER JOIN line_items li
     ON li.itemizable_type = 'Distribution'
     AND li.itemizable_id = d.id
LEFT OUTER JOIN items i
     ON i.id = li.item_id
WHERE d.organization_id = 1
GROUP BY d.id
) subquery;

This is almost identical to what the app uses with DistributionTotalsService in distributions_controller#index. Both the app methods and above query return 59,452, which matches the "All Time" total shown at /distributions.

However, I’m trying to understand where the number 119,311 comes from in a previous reply

Comparing the total item counts from the distribution index (/distributions) to the distributions by county and itemized distribution reports, I only found a difference of 1 item (119,311 vs 119,310)

Am I missing something in the query or excluding some data?

(Logged in as [email protected])

DasTapan avatar Aug 14 '25 06:08 DasTapan

If its available again, could you assign it to me ?

DasTapan avatar Aug 14 '25 06:08 DasTapan

@DasTapan -- there might not be any distributions with inactive items in the seed -- so you might have to add distributions and then make an item in them inactive to see the differences

I'm pretty sure that the quantities in the seed data are random, so you may not find the same values as another person.

We generally give people 30 days on an issue before assigning it to someone else, but I'll reach out to @silverjellyfish to see if they are still working on it.

cielf avatar Aug 14 '25 15:08 cielf

@silverjellyfish Are you still working on this?

cielf avatar Aug 14 '25 15:08 cielf

Hi @cielf, thanks for replying. In my seed data, there is an inactive item. Without it (current implementation), the "All Time" total items quantity is 56,253, and with it, it is 59,452. I think this means the issue is fixed, but I could be wrong.

DasTapan avatar Aug 15 '25 07:08 DasTapan

@DasTapan To see the issue, look at the values in distribution summary. Then, change the active status on an item that has distributions in the appropriate timeframe (you will probably be able to see this by "restoring" an inactive item), Check the values in the distribution summary again. They are different. They should not be.

cielf avatar Aug 15 '25 16:08 cielf

Hi @cielf, i saw the difference in summary after toggling active status of an item, I fixed it, shall I raise a PR for you to see ?

DasTapan avatar Aug 17 '25 14:08 DasTapan

Go ahead. I think @silverjellyfish is not working on it.

cielf avatar Aug 17 '25 15:08 cielf

Hi @cielf , please have a look at the PR

DasTapan avatar Sep 01 '25 05:09 DasTapan

Hey @DasTapan -- I've stepped back from the project, but I've asked the right person for a review.

cielf avatar Sep 02 '25 01:09 cielf