-
-
Notifications
You must be signed in to change notification settings - Fork 573
Request Item → Item Request #5196
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
base: main
Are you sure you want to change the base?
Conversation
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.
Passes functional check. On to @dorner for technical comments.
dorner
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.
Minor suggestion, otherwise looks good. 🎉
app/views/requests_confirmation_mailer/confirmation_email.html.erb
Outdated
Show resolved
Hide resolved
dorner
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.
one last miss!
| def name_with_unit(quantity_override = nil) | ||
| if Flipper.enabled?(:enable_packs) && request_unit.present? | ||
| "#{item&.name || name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}" | ||
| "#{item_name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}" |
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.
I think you forgot to use the new method here? :)
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 is one of the display helpers:
- ItemRequest#item_name gives the item's name and falls back to the local copy
- ItemRequest#quantity_with_units shows the qty and units but not the name
- ItemRequest#name_with_unit shows the name and the unit but not qty
|
@janeewheatley @awwaiid yep, my most recent comments have not been addressed. |
|
@dorner mmmm I think the one you mentioned shouldn't or can't use the other display helper (and this is a different display helper). Eh? |
First pass at removing
request_itemeverywhere.