#4770: Don't crash on deleted item for event history #4876
Conversation
| @@ -11,7 +11,7 @@ | |||
| <%= 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_path(entry.item_id) %>: | |||
| <%= link_to items.find { |i| i.id == entry.item_id}&.name || "Item #{entry.item_id} (deleted)", item_path(entry.item_id) %>: | |||
There was a problem hiding this comment.
Don't you also have to apply this same fix in app/views/events/_event_row.html.erb as well?
There was a problem hiding this comment.
Also does it make sense to create a link to a deleted item? Won't it just lead to a 404?
There was a problem hiding this comment.
And looking into the future... maybe we would want to implement soft deletes for items, so that we could grab the items name here. Because it doesn't seem like displaying "Item 1234 (deleted)" will be very helpful for users.
There was a problem hiding this comment.
@coalest, we only allow deletes per se on items that have had no transactions associated with them. Otherwise we allow deactivates if the inventory level on all storage locations for the item is 0.
This is dealing with a particular edge case . If we deleted an item that existed on the snapshot, it was making the event run crash.
It reall is an edge case, but could possibly happen again, as the intent is to do periodic refersher snapshots.
This report is about 90% for us, though I expect some of our most astute users will find it useful.
There was a problem hiding this comment.
This is essentially saying "nothing to see here. Move on."
|
@dorner. I'm ok with the concept. I'm not sure how I manually test it, though. |
|
@cielf To reproduce a similar bug (not snapshot event but a different event):
|
|
Added the fix for the other partial, and also removed the link for deleted items. |
|
|
|
Alas. I have found a new problem. One would expect the level on the item to be 0 (and hence it could be deleted, for the testing of this PR). Hopefully there aren't any instances in the current "wild" but we can run a check on that. Do you want to treat this as a separate issue? |
|
@dorner Just checking in case this has fallen through the cracks. |
Resolves #4770.
Description
Fixes a crash on the history page when an item was deleted. Instead it will display
Item {item id} (deleted).Type of change