Request Item → Item Request #5196
Conversation
| @inventory = View::Inventory.new(@request.organization_id) | ||
| @default_storage_location = @request.partner.default_storage_location_id || @request.organization.default_storage_location | ||
| @location = StorageLocation.find_by(id: @default_storage_location) | ||
|
|
||
| @custom_units = Flipper.enabled?(:enable_packs) && @request.item_requests.any? { |item| item.request_unit } |
There was a problem hiding this comment.
These variables were previously being built directly in the view.
|
|
||
| private | ||
|
|
||
| # TODO: remove the need to de-duplicate items in the request |
There was a problem hiding this comment.
Going forward (for a while now), duplication isn't allowed when the request is created in the first place, so we don't need to merge things for the email anymore.
| "Value/item", | ||
| "In-Kind Value Received", | ||
| "Packages"]] | ||
| inventory = View::Inventory.new(@distribution.organization_id) |
There was a problem hiding this comment.
Interestingly, this appears to already have not been used. This time rubocop noticed and had me remove it.
app/views/requests/show.html.erb
Outdated
| <td><%= item.quantity %></td> | ||
| <% if custom_units %> | ||
| <td><%= item.unit&.pluralize(item.quantity) %></td> | ||
| <td><%= item_request.item&.name || item_request.name %></td> |
There was a problem hiding this comment.
Tries to look up the "current" item name, and then falls back to the cached item_request.name. We do this elsewhere also.
| let(:mail) { RequestsConfirmationMailer.confirmation_email(request) } | ||
|
|
||
| let(:request_w_varied_quantities) { create(:request, :with_varied_quantities, organization: organization) } | ||
| let(:request_w_varied_quantities) { create(:request, :with_varied_quantities, :with_item_requests, organization: organization) } |
There was a problem hiding this comment.
A lot of these factories now require :with_item_requests role; in a further PR I'll make that the default and then remove all of the instances of the param.
| ["Item 1", "", 50, "$1.00", "$50.00", "1"], | ||
| ["Item 2", 30, 100, "$2.00", "$200.00", nil], | ||
| ["Item 3", 50, "", "$3.00", nil, nil], | ||
| ["Item 2", "30", 100, "$2.00", "$200.00", nil], |
There was a problem hiding this comment.
These are surprisingly stored as strings; in the old one they ended up number-ified somewhere, but they are turned into strings in the PDF anyway so strings are fine.
dorner
left a comment
dorner
left a comment
There was a problem hiding this comment.
Minor suggestion, otherwise looks good. 🎉
app/views/requests_confirmation_mailer/confirmation_email.html.erb
Outdated
Show resolved
Hide resolved
| 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.
I think you forgot to use the new method here? :)
There was a problem hiding this comment.
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
First pass at removing
request_itemeverywhere.