cielf commented 2 months ago

Summary

In the Distribution index, for the "Total in Category" column that appears when we filter by Item Category, the totals at the bottom of the page should show the total of the numbers in that column, and they don't.

Why fix?

It's confusing.

Details

Recreation

Sign in as org_admin@example.com. Choose "Distribution" from the left hand menu.. Then, filter the list by item category (fill in "Fiter by item category", then click "Filter") You will see that the total of the "Total in Category" column does not match the total of the numbers in the column. It should.

Criteria for completion

[ ] totals in the "Total in Category Diapers" column match the total of the column. (page totals and grand totals)
[ ] tests to confirm this behaviour

rlew421 commented 2 months ago

Hi @cielf, I'm new to this project and am looking for my first ticket. Can you assign this to me?

cielf commented 2 months ago

Sure can! (Welcome aboard)

rlew421 commented 2 months ago

Thank you!

rlew421 commented 2 months ago

I was able to diagnose the source of the issue, but I have questions about the best way to change it.

After filtering based on an item category, the totals aren't calculating correctly at the bottom of the page because the method @selected_item = filter_params[:by_item_id].presence on line 54 of distributions_controller.rb is selecting based on item_id instead of item_category_id. When you select a category and check the filter_params in a pry session, item_id is an empty string while item_category_id contains the id we want to filter on. When you try to select a category and then select an item within that category (category - Category Diapers, item - Adult Cloth Diapers Small/Medium) and check filter_params again, you can see that it now contains the item_category_id and the item_id. I think @selected_item needs to select based on item_category_id instead of item_id, but I run into two issues when I do this.

When I choose a category from the category dropdown, the item dropdown auto-populates to the second item on the list regardless of whether that item type actually belongs to the category selected. The item dropdown menu should not auto-populate to anything. Because this item dropdown auto-populates, this means the user needs to clear this dropdown box before filtering by category each time, and it's inconvenient to have to do that. This may necessitate writing another issue, but I think that once a category is selected, the available items in the item dropdown menu should be limited to items that are part of that category. Should I resolve the auto-populate on the item dropdown list in this ticket? Should I proceed to make the fix (set @selected_item based on item_category_id instead of item_id) and then write another issue to limit the items in the dropdown list?
The totals still don't appear to be calculating correctly, but I think it's due to the seed data.

Before fix:

After fix:

It appears that the line query = query.where(item_id: item.to_i) if item in distributions_controller.rb isn't capturing all the distributions it should capture. When I looked at the item_id field for the distribution line_items in pry, many of them have an item_id that doesn't match any of the item_category_id's which can only be 1, 2, or 3. In order for the totals to calculate correctly, I think the line_items in the seed data need to be restricted to having item_id's that match up with the item_category_id's. Could I get another set of eyes to to check if my theory is accurate?

cielf commented 2 months ago

Hey @rlew421 -- Thanks for reaching out! I think you might be tweaking the wrong place.

@selected_item is, specifically, the item that is selected in the drop-down. It gets fed back into the screen as such (see app/views/distributions/index.html, line 41)

So, putting the item_category in there instead of the item is going to have "unexpected results" g, such as what you've described.

I suspect the best place to try to address this would be moving @selected_item_category up to just below @selected_item, then reworking total_items so that it can know about item_categories as well as items.

Bonus:
Here's a rough sketch of the relationships in the db... (Note that line items are not always connected to distributions, which isn't shown in the sketch)