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.

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

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

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 %>

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 %>

app/views/admin/reservations/reservation_loans/_reservation_hold.html.erb

@@ -10,10 +10,10 @@
   <% reservation_hold.reservation_loans.each do |reservation_loan| %>
     <tr id="<%= dom_id(reservation_loan) %>">
       <% if reservation_loan.quantity %>
+        <td>*</td>
         <td><%= reservation_hold.item_pool.name %></td>
-        <td><%= reservation_loan.quantity %></td>
       <% else %>
-        <td><%= reservation_loan.reservable_item.id %></td>
+        <td><%= reservation_loan.reservable_item.number %></td>
         <td><%= reservation_loan.reservable_item.name %></td>
       <% end %>
       <td>

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