Include inactive items in distribution reports
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
Hi! I'm new to this project, and I would love to work on this issue. Looking forward to contributing!
Welcome aboard -- it's yours!
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!
Hi @donaldwilson8 -- I will double check this, but it might take me a couple of days to get to it. Thanks for your patience!
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.
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.
Automatically unassigned after 7 days of inactivity.
hi, can i please solve this issue?
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.
Automatically unassigned after 7 days of inactivity.
Available again.
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 ?
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).
But to answer @Oce-ane's query -- LineItem .active does grab only those line items that have active items -- so it's plausible!
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.
Alright - you can put in a draft PR if need be, and let us know if you have any questions.
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.
Automatically unassigned after 7 days of inactivity.
Hello! Is this available? If so, could you assign this to me?
You've got it!
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])
If its available again, could you assign it to me ?
@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.
@silverjellyfish Are you still working on this?
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 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.
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 ?
Go ahead. I think @silverjellyfish is not working on it.
Hi @cielf , please have a look at the PR
Hey @DasTapan -- I've stepped back from the project, but I've asked the right person for a review.