-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
Add feature to print unfulfilled requests as a picklists PDF #4598
Add feature to print unfulfilled requests as a picklists PDF #4598
Conversation
da53d82
to
69135be
Compare
Ready for a review @norrismei? |
@awwaiid There are a few changes I need to make and then I'll fill out the PR description and mark this ready for review. |
db42034
to
419a86c
Compare
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 @norrismei! Good job so far!
I am a little concerned about perceived performance (prod is slower than our locals), but I'll let @awwaiid / @dorner chime in on that from a technical pov.
(Edit: I could see adding the number of requests that will be in the picklist into the button. Just as an indication of what they are getting into.)
One other question -- does this include "started" requests?
@norrismei Please do change the button to have the number of unfulfilled requests.. e.g. Print Unfulfilled Picklists (44) |
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.
Great first pass!
c1f35e8
to
84c788b
Compare
5b31977
to
03a7b79
Compare
Hey @norrismei - just noting that the lint is failing -- just a wee thing to change there. |
@norrismei, but do please change to @item_requests -- we are going to be stripping out request_items entirely in the near future. I think I'll hold off on a final functional check until that's in. |
826d4ec
to
61aada6
Compare
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.
This needs a unit test please!
854389f
to
b385ba2
Compare
b385ba2
to
a537810
Compare
@dorner unit tests added. Both for the PDF and the button on the index page |
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 from a functional pov.
@norrismei FYI: We had an urgent fix that required all the senior contributors this week , so we didn't get to look at this again. Hopefully this week will go better. |
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.
One minor comment. Otherwise this looks great!
8243bad
to
9173819
Compare
fe12dfa
to
8439cc2
Compare
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.
Looks great! Thank you :)
@@ -40,6 +40,20 @@ def start | |||
redirect_to new_distribution_path(request_id: request.id) | |||
end | |||
|
|||
def print_unfulfilled | |||
requests = Request.includes(partner: [:profile]).where(status: [0, 1]) |
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.
Note to self -- is there something more explicit than 0
and 1
?
app/views/requests/index.html.erb
Outdated
<% if @requests.any? { |request| request.status == "pending" || request.status == "started" } %> | ||
<%= print_button_to( | ||
print_unfulfilled_requests_path(format: :pdf), | ||
text: "Print Unfulfilled Picklists", |
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.
Maybe add a count so they know what to expect. "Print Unfulfilled Picklists (34 partner requests)"
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.
Ahh -- this needs to pass the filter through for the pdf to also apply.
@norrismei: Your PR |
Resolves #4406
Description
Stakeholder asked for new functionality that allows printing a single file containing all outstanding requests so that pickers may use the documents as picklists, checking off as they go and making note of any differences.
When an organization goes to their Requests, there will now be a
Print Unfulfilled Picklists
button next to theExport Requests
button.Design decisions made:
1 of 1
or1 of 2
because if somehow the pages were scrambled up, we'd want a way of putting them back in order vs. risking not being able to identify multiple non-descript2 of 2
pages.OpenSans
font does not support checkboxes so I chose to go with[ ]
(open and close brackets) to get the job done with minimal effort.Things to note
Type of change
How Has This Been Tested?
I manually tested the following scenarios:
Units (if applicable)
column.To enable, I pointed my browser to http://localhost:3000/flipper and logged in
userid: admin
password: password
Partner Pickup Person
if there is one listed under the Partner settings. I logged in as a Partner to toggle this setting.Screenshots
No packs enabled and no partner pickup person:
With packs enabled and no partner pickup person:
With packs enabled and a partner pickup person listed:
Index page with print button displaying unfulfilled requests count