From 2495161fba77b70be18fc470b91e031fbedc0a09 Mon Sep 17 00:00:00 2001 From: Jim Benton <3331+jim@users.noreply.github.com> Date: Fri, 3 Jan 2025 10:54:37 -0600 Subject: [PATCH] Look up items by number instead of database ID (#1807) # What it does This replaces how we look up reservable items in the group lending code to use the human-friendly item number instead of the database ID. This fixes #1777. # Why it is important We're going to import items from an external system, and we want to be able to change the number without messing with data integrity. --- .../admin/reservations/check_ins_controller.rb | 10 +++++----- .../reservations/reservation_loans_controller.rb | 13 ++++++------- .../admin/reservations/check_ins/_form.html.erb | 2 +- .../reservations/reservation_loans/_form.html.erb | 2 +- .../reservation_loans/_reservation_hold.html.erb | 4 ++-- test/system/admin/reservations/reservations_test.rb | 8 ++++---- 6 files changed, 19 insertions(+), 20 deletions(-) diff --git a/app/controllers/admin/reservations/check_ins_controller.rb b/app/controllers/admin/reservations/check_ins_controller.rb index c5a6a24bc..53951712b 100644 --- a/app/controllers/admin/reservations/check_ins_controller.rb +++ b/app/controllers/admin/reservations/check_ins_controller.rb @@ -2,10 +2,10 @@ module Admin module Reservations class CheckInsController < BaseController def create - if (reservation_item_id = reservation_loan_lookup_params[:reservable_item_id]) - @reservable_item = ReservableItem.find_by(id: reservation_item_id) + if (reservation_item_number = reservation_loan_lookup_params[:reservable_item_number]) + @reservable_item = ReservableItem.find_by(number: reservation_item_number) if !@reservable_item - render_form_with_error("no item found with this ID") + render_form_with_error("no item found with this number") return end @reservation_loan = @reservation.reservation_loans.find_by(reservable_item_id: @reservable_item.id) @@ -51,7 +51,7 @@ def destroy def render_form_with_error(message) @reservation_loan_lookup_form = ReservationLoanLookupForm.new - @reservation_loan_lookup_form.errors.add(:reservable_item_id, message) + @reservation_loan_lookup_form.errors.add(:reservable_item_number, message) render_form end @@ -60,7 +60,7 @@ def render_form end def reservation_loan_lookup_params - params.require(:reservation_loan_lookup_form).permit(:reservable_item_id, :reservation_loan_id) + params.require(:reservation_loan_lookup_form).permit(:reservable_item_number, :reservation_loan_id) end end end diff --git a/app/controllers/admin/reservations/reservation_loans_controller.rb b/app/controllers/admin/reservations/reservation_loans_controller.rb index dab837395..bae360dc9 100644 --- a/app/controllers/admin/reservations/reservation_loans_controller.rb +++ b/app/controllers/admin/reservations/reservation_loans_controller.rb @@ -19,14 +19,13 @@ def create reservation_hold: @reservation_hold, quantity: @reservation_hold.quantity ) - elsif reservation_loan_params[:reservable_item_id].blank? - render_form_with_error("please enter an item ID") + elsif reservation_loan_params[:reservable_item_number].blank? + render_form_with_error("please enter an item number") return else - # TODO look items up by number and not id - @reservable_item = ReservableItem.find_by(id: reservation_loan_params[:reservable_item_id]) + @reservable_item = ReservableItem.find_by(number: reservation_loan_params[:reservable_item_number]) if !@reservable_item - render_form_with_error("no item found with this ID") + render_form_with_error("no item found with this number") return end @@ -77,7 +76,7 @@ def destroy def render_form_with_error(message) @reservation_loan = ReservationLoan.new - @reservation_loan.errors.add(:reservable_item_id, message) + @reservation_loan.errors.add(:reservable_item_number, message) render_form end @@ -90,7 +89,7 @@ def set_reservation_loan end def reservation_loan_params - params.require(:reservation_loan).permit(:reservable_item_id, :reservation_hold_id) + params.require(:reservation_loan).permit(:reservable_item_number, :reservation_hold_id) end end end diff --git a/app/views/admin/reservations/check_ins/_form.html.erb b/app/views/admin/reservations/check_ins/_form.html.erb index b984004bf..05e4a8714 100644 --- a/app/views/admin/reservations/check_ins/_form.html.erb +++ b/app/views/admin/reservations/check_ins/_form.html.erb @@ -1,6 +1,6 @@ <%= turbo_frame_tag "reservation-loan-form" do %> <%= form_with(model: reservation_loan_lookup_form, url: admin_reservation_check_ins_path(reservation.id), builder: SpectreFormBuilder) do |form| %> - <%= form.text_field :reservable_item_id, label: "Item ID", autocomplete: "off", autofocus: true %> + <%= form.text_field :reservable_item_number, label: "Item Number", autocomplete: "off", autofocus: true %> <%= form.submit "Return Item" %> <% end %> <% end %> diff --git a/app/views/admin/reservations/reservation_loans/_form.html.erb b/app/views/admin/reservations/reservation_loans/_form.html.erb index 8b761bec6..70f643a72 100644 --- a/app/views/admin/reservations/reservation_loans/_form.html.erb +++ b/app/views/admin/reservations/reservation_loans/_form.html.erb @@ -1,6 +1,6 @@ <%= turbo_frame_tag "reservation-loan-form" do %> <%= form_with(model: reservation_loan, url: admin_reservation_loans_path(reservation.id), builder: SpectreFormBuilder) do |form| %> - <%= form.text_field :reservable_item_id, label: "Item ID", autocomplete: "off", autofocus: true %> + <%= form.text_field :reservable_item_number, label: "Item Number", autocomplete: "off", autofocus: true %> <%= form.submit "Add Item" %> <% end %> <% end %> diff --git a/app/views/admin/reservations/reservation_loans/_reservation_hold.html.erb b/app/views/admin/reservations/reservation_loans/_reservation_hold.html.erb index d5823f900..8adcb7d5f 100644 --- a/app/views/admin/reservations/reservation_loans/_reservation_hold.html.erb +++ b/app/views/admin/reservations/reservation_loans/_reservation_hold.html.erb @@ -10,10 +10,10 @@ <% reservation_hold.reservation_loans.each do |reservation_loan| %> <% if reservation_loan.quantity %> + * <%= reservation_hold.item_pool.name %> - <%= reservation_loan.quantity %> <% else %> - <%= reservation_loan.reservable_item.id %> + <%= reservation_loan.reservable_item.number %> <%= reservation_loan.reservable_item.name %> <% end %> diff --git a/test/system/admin/reservations/reservations_test.rb b/test/system/admin/reservations/reservations_test.rb index 1077f3669..213d4b480 100644 --- a/test/system/admin/reservations/reservations_test.rb +++ b/test/system/admin/reservations/reservations_test.rb @@ -259,7 +259,7 @@ def assert_hold_quantity(item_pool, quantity) visit admin_reservation_loans_path(reservation) assert_active_tab "Items" - fill_in "Item ID", with: hammer.id + fill_in "Item Number", with: hammer.number click_on "Add Item" assert_hold_quantity hammer_pool, "1/1" @@ -283,7 +283,7 @@ def assert_hold_quantity(item_pool, quantity) assert_no_difference "PendingReservationItem.count" do assert_difference "PendingReservationItem.count", 1 do assert_active_tab "Items" - fill_in "Item ID", with: hammer.id + fill_in "Item Number", with: hammer.number click_on "Add Item" assert_text "1 item scanned that did not match the reservation" @@ -306,7 +306,7 @@ def assert_hold_quantity(item_pool, quantity) assert_difference -> { reservation.reservation_holds.count } => 1, -> { reservation.pending_reservation_items.count } => 0 do assert_active_tab "Items" - fill_in "Item ID", with: hammer.id + fill_in "Item Number", with: hammer.number click_on "Add Item" assert_text "1 item scanned that did not match the reservation" @@ -337,7 +337,7 @@ def assert_hold_quantity(item_pool, quantity) end # return hammer - fill_in "Item ID", with: hammer.id + fill_in "Item Number", with: hammer.number click_on "Return Item" within_dom_id(hammer_loan) do