Skip to content

Request Item → Item Request - #5196

Merged
awwaiid merged 11 commits into
mainfrom
item-request
Feb 28, 2026
Merged

awwaiid merged 11 commits into
mainfrom
item-request

Conversation

@awwaiid

@awwaiid awwaiid commented May 11, 2025 •

Copy link
Copy Markdown
Collaborator

First pass at removing request_item everywhere.

  • Main reads on the Bank's Request controller/view
  • Update distribution PDF to read from item_request
  • Update confirmation email to read from item_request
  • Fix specs to make the correct mock data

Comment thread app/controllers/requests_controller.rb
Comment thread app/mailers/requests_confirmation_mailer.rb
Comment thread app/pdfs/distribution_pdf.rb
Comment thread app/views/requests/show.html.erb Outdated
Comment thread spec/mailers/requests_confirmation_mailer_spec.rb
Comment thread spec/pdfs/distribution_pdf_spec.rb
@awwaiid
awwaiid marked this pull request as ready for review May 11, 2025 15:48
@awwaiid
awwaiid requested a review from dorner May 11, 2025 15:48

@cielf cielf left a comment

Copy link
Copy Markdown
Collaborator

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 dorner left a comment

Copy link
Copy Markdown
Collaborator

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. 🎉

Comment thread app/views/requests_confirmation_mailer/confirmation_email.html.erb Outdated

@dorner dorner left a comment

Copy link
Copy Markdown
Collaborator

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)}"

Copy link
Copy Markdown
Collaborator

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? :)

Copy link
Copy Markdown
Collaborator Author

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

Copy link
Copy Markdown
Collaborator

@awwaiid @dorner does this one still need more work?

@janeewheatley
janeewheatley requested a review from dorner January 13, 2026 19:50
@dorner

dorner commented Jan 23, 2026

Copy link
Copy Markdown
Collaborator

@janeewheatley @awwaiid yep, my most recent comments have not been addressed.

@awwaiid

awwaiid commented Jan 25, 2026

Copy link
Copy Markdown
Collaborator Author

@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?

@dorner dorner left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't honestly remember much about this but I'm good with the very last interaction 😂 This is good for a manual test.

@awwaiid
awwaiid merged commit c0edf94 into main Feb 28, 2026
22 checks passed
@awwaiid
awwaiid deleted the item-request branch February 28, 2026 20:49
@github-actions

github-actions Bot commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

@awwaiid: Your PR Request Item → Item Request is part of today's Human Essentials production release: 2026.03.01.
Thank you very much for your contribution!

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.

4 participants