-
-
Notifications
You must be signed in to change notification settings - Fork 571
Resolves #5225: Implemented "Itemized Request Report" #5300
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
Resolves #5225: Implemented "Itemized Request Report" #5300
Conversation
There was a problem hiding this 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.
|
Got it! Will make the necessary changes :) |
…th the same criteria for distributions. Modelled files more closely after itemized distributions
|
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? |
…ad to fix error thrown in automated test
|
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. |
|
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. |
…mRequest, changed tests appropriately
|
I implemented the rest of the suggestions. Let me know if it needs any changes still! |
|
All good on my end. @cielf - not sure if there was something you still wanted to check? |
cielf
left a comment
There was a problem hiding this 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!
|
@seabeeberry: Your PR |
|
hi @seabeeberry , I have a query regarding this pr, can we connect ? |
|
@DasTapan sure! should I write you an email or do you have a discord? |
|
@seabeeberry , I have discord, we can connect. Shall I create a server? |
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-alland ran all tests (bundle exec rake).Type of change
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_minimumparameter should be displayed as red in the table.Screenshot