Skip to content

Conversation

@seabeeberry
Copy link
Contributor

@seabeeberry seabeeberry commented Aug 1, 2025

Resolves #5225

Implemented the Itemized Request Report, modelled after the Itemized Distribution Report. When you click on Reports, the option for Requests - Itemized apears at the bottom (this can be placed elsewhere if wanted). This shows the list of requested items and how many are on hand at the moment. This can be filtered by time range.

I implented tests as well to verify intended behavior. I checked for Linting with bundle exec erb_lint --lint-all and ran all tests (bundle exec rake).

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Reports -> Requests - Itemized

You should see a list of the requested items, their units, the number of requested items and the number of items on hand. Items which are below the set onhand_minimum parameter should be displayed as red in the table.

Screenshot

Screenshot 2025-08-03 at 16 05 44

@cielf cielf self-requested a review August 2, 2025 17:10
Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @seabeeberry -- thank you for taking this on! It looks pretty good functionally, but I have one note:

Because the total requested included requests that have already been fulfilled, making the total on hand red if it's less than the total requested will highlight items that they aren't actually short on. Using the same criteria as for distribution for making the number red should work, though.

When we've got that addressed, I'll pass it on for technical comments.

@seabeeberry
Copy link
Contributor Author

Got it! Will make the necessary changes :)

…th the same criteria for distributions. Modelled files more closely after itemized distributions
@seabeeberry seabeeberry requested a review from cielf August 3, 2025 14:10
@cielf
Copy link
Collaborator

cielf commented Aug 4, 2025

Hey @seabeeberry -- It's looking good

There's one more thing I'd like to tweak:

I know we said "model it on the distributions-itemized" report -- it turns out there is an outstanding issue on that report, and I'm wondering if we could avoid it for this one, instead of having to make another issue to address it.

If an item is inactive, the total on hand currently shows "Unknown". (See Adult Briefs (Large/X-Large)). We made a change so that inactive items always have 0 items a few months ago, so we no longer need to show "Unknown" -- we can just show the actual level.

Could you make that adjustment?

@seabeeberry
Copy link
Contributor Author

I changed the fallback in the view from "Unknown" to 0, hopefully that is the intended fix here. I had added a fallback to the fetch method in my first commit, so that the number on hand is always set to 0 in case it's nil, but that crashed one of the automated tests.

@cielf
Copy link
Collaborator

cielf commented Aug 5, 2025

@seabeeberry

Hmmm -- This is a case where requests are going to be different than distributions -- because if there has been a distribution, there has to have been inventory at some point -- and that is simply not the case with requests. So we might have items that have never had inventory in the list.

Your solution will give the desired result in that case, but I'd like @dorner's take on whether it's the best way to handle it -- and it's time to get his overall technical advice anyway.

@cielf cielf requested a review from dorner August 5, 2025 20:01
@seabeeberry
Copy link
Contributor Author

I implemented the rest of the suggestions. Let me know if it needs any changes still!

@seabeeberry seabeeberry requested a review from dorner August 15, 2025 01:14
@seabeeberry seabeeberry requested a review from dorner August 18, 2025 13:13
@dorner
Copy link
Collaborator

dorner commented Aug 22, 2025

All good on my end. @cielf - not sure if there was something you still wanted to check?

Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. On to merging!

@cielf cielf merged commit 2b4d3e5 into rubyforgood:main Aug 24, 2025
12 checks passed
@github-actions
Copy link
Contributor

@seabeeberry: Your PR Resolves #5225: Implemented "Itemized Request Report" is part of today's Human Essentials production release: 2025.08.24.
Thank you very much for your contribution!

@DasTapan
Copy link
Contributor

hi @seabeeberry , I have a query regarding this pr, can we connect ?

@seabeeberry
Copy link
Contributor Author

@DasTapan sure! should I write you an email or do you have a discord?

@DasTapan
Copy link
Contributor

@seabeeberry , I have discord, we can connect. Shall I create a server?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Itemized Request Report

4 participants