Browse hold shelf displays deleted copies

[Steve Callender]

Browse hold shelf is showing copies that have been deleted while in a Hold Shelf status. The query used to build the hold shelf list didn't check if the item was deleted or not. I've created a patch to address that.

It can be found at,

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=fb2e6756b1f8894128bbdd1b4b6b65739ec140d5

I've patched against Master but the problem appears in 2.0+ up to current.

Steve

[Thomas Berezansky (tsbere)]

I would argue that the problem isn't that they show up, but that they were deleted while in that status. And I believe that has been fixed with other code that would at least alert about it and require an override by default.

[Michael Peters (mrpeters)]

Nice catch, Steve. Cherry picked to my 2.2.2 server and tested. I don't want to blame the patch, but I started getting errors querying the database in the Browse Hold Shelf after applying it. Most of the shelf loads, but about 5 rows stay in "Retrieving..." status after the network errors, but, for what it's worth the deleted copy on the shelf didn't display.

I rolled back to a virgin Holds.pm and didn't have any issues loading the full hold shelf, but, of course my deleted copy was there.

I think Thomas brings up a good point, though. If an item is on the hold shelf it shouldn't be deleted without notifying the patron.

[Michael Peters (mrpeters)]

Confirmed the original bug, however.

[Steve Callender (stevecallender)]

Thanks for testing Mike. I have it installed on a local 2.2.2 and I cannot duplicate the error you are seeing. Very odd. The change itself should just add a simple extra parameter in the query. It sounds like it might not be needed though anyways.

Although I do come across folks who like to purge their data manually in the database, and a simple change like this would protect the hold shelf list from items that were deleted outside of the normal methods. I understand that it's not something the system really should anticipate or protect against (manual data manipulation) but to some degree there would be a benefit to a change like this.

Steve

[Mike Rylander (mrylander)]

Merged for great justice. Thanks, Steve!

[Ben Shum (bshum)]

Assigning milestones for 2.2 and 2.3 since it looks like it was backported to rel_2_3 and rel_2_2.