Code review fixes

Author: dorner

app/views/events/_event_row.html.erb

@@ -29,7 +29,12 @@
     </td>
     <td>
       <% event.data.items.each do |entry| %>
-        <%= link_to items.find { |i| i.id == entry.item_id}.name, item_path(entry.item_id) %>:
+        <% item = items.find { |i| i.id == entry.item_id} %>
+        <% if item %>
+          <%= link_to items.find { |i| i.id == entry.item_id}.name, item_path(entry.item_id) %>:
+        <% else %>
+          Item <%= entry.item_id %> (deleted)
+        <% end %>
         <%= entry.from_storage_location == from_loc ? entry.quantity : -entry.quantity %><br>
       <% end %>
     </td>

app/views/events/_snapshot_event_row.html.erb

@@ -11,7 +11,12 @@
       <%= link_to storage_locs.find { |i| i.id == loc.id}.name, storage_location_path(loc.id) %>
     <td>
       <% loc.items.values.each do |entry| %>
-        <%= link_to items.find { |i| i.id == entry.item_id}&.name || "Item #{entry.item_id} (deleted)", item_path(entry.item_id) %>:
+        <% item = items.find { |i| i.id == entry.item_id} %>
+        <% if item %>
+          <%= link_to items.find { |i| i.id == entry.item_id}.name, item_path(entry.item_id) %>:
+        <% else %>
+          Item <%= entry.item_id %> (deleted)
+        <% end %>
         <%= entry.quantity %><br>
       <% end %>
     </td>

spec/requests/events_requests_spec.rb

@@ -47,7 +47,68 @@
         expect(response.body).not_to include("99<br>")
       end
 
-      it "should show deleted items without crashing" do
+      it "should show deleted items on regular event without crashing" do
+        deleted_item = create(:item, organization: organization)
+        travel(-1.day) do
+          SnapshotEvent.create!(
+            eventable: organization,
+            organization_id: organization.id,
+            event_time: Time.zone.now,
+            data: EventTypes::Inventory.new(
+              organization_id: organization.id,
+              storage_locations: {
+                storage_location.id => EventTypes::EventStorageLocation.new(
+                  id: storage_location.id,
+                  items: {
+                    item.id => EventTypes::EventItem.new(item_id: item.id, quantity: 0),
+                    item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 0)
+                  }
+                ),
+                storage_location2.id => EventTypes::EventStorageLocation.new(
+                  id: storage_location2.id,
+                  items: {
+                    item.id => EventTypes::EventItem.new(item_id: item.id, quantity: 0),
+                    item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 0)
+                  }
+                )
+              }
+            )
+          )
+          donation = create(:donation, organization: organization)
+          DonationEvent.create!(
+            eventable: donation,
+            organization_id: organization.id,
+            event_time: Time.zone.now,
+            data: EventTypes::InventoryPayload.new(
+              items: [
+                EventTypes::EventLineItem.new(item_id: item.id,
+                  quantity: 0,
+                  item_value_in_cents: 5,
+                  from_storage_location: nil,
+                  to_storage_location: storage_location.id),
+                EventTypes::EventLineItem.new(item_id: item2.id,
+                  quantity: 0,
+                  item_value_in_cents: 5,
+                  from_storage_location: nil,
+                  to_storage_location: storage_location.id),
+                EventTypes::EventLineItem.new(item_id: deleted_item.id,
+                  quantity: 0,
+                  item_value_in_cents: 5,
+                  from_storage_location: nil,
+                  to_storage_location: storage_location.id)
+                     ]
+            )
+          )
+        end
+        deleted_id = deleted_item.id
+        deleted_item.destroy
+        subject
+        expect(response.body).to include("Item1</a>")
+        expect(response.body).to include("Item2</a>")
+        expect(response.body).to include("Item #{deleted_id} (deleted)")
+      end
+
+      it "should show deleted items on snapshot without crashing" do
         deleted_item = create(:item, organization: organization)
         travel(-1.day) do
           SnapshotEvent.create!(