Copy locations table should have a 'deleted' flag

[Michele Morgan]

Evergreen 2.3.7

When a copy location is no longer in use, it is preferable for a library to remove it from their list of locations so users don't see the defunct location and potentially choose it in error. Users with permission are able to delete copy locations in the copy locations editor. When a copy location is deleted successfully, the row is removed from asset.copy_location.

This presents several problems:

If items are coded with the location, the deletion fails. Even if the items have been deleted, it is not possible to delete the location.

The asset.copy_location.id field is referenced by action.circulation.copy_location and action.aged_circulation.copy_location. If the row in the copy_location table is deleted, circulation by location statistcs are lost.

If there were a deleted flag in the copy location table that was set to TRUE when the location was deleted, these relationships would be preserved.

The deleted flag in the copy location table should cause the location not to display in the library's list of locations. Checks should be made against undeleted items, before the deleted flag can be set to TRUE. Ideally, checks should also be made against saved copy templates.

[[e-mail] (tji)]

This is related to #979302.

Shelving locations used by only deleted items should be allowed to be deleted, or marked deleted. This is causing lots of confusion and trouble. The deleted items are invisible on the client. Staff can't do anything with them.

[Lindsay Stratton (lstratton)]

Pioneer agrees that this is a problem, and agrees that: there should be a deleted flag in the copy locations table, and that deleted locations should not display in the library's list of locations.

This crops up regularly as libraries re-arrange and update their collections.

[Jason Boyer (jboyer)]

I've got a squashed branch at http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/jboyer/lp1210541_del_acpl that addresses this. I'd appreciate some testing (especially the db upgrade bits) and we might finally be able to wipe out some old locations.

Note: Even though there's an error message defined for trying to delete a location that's not empty, it's never displayed. The copy location editor is one of those interfaces that fails silently into the JS Console. I haven't taken the time to change that in addition to this.

[Blake GH (bmagic)]

Im setting this up in a sandbox for bug squashing day. I had a problem seeding the database. It errored here:

040.schema.asset.sql:

CREATE OR REPLACE RULE protect_copy_location_delete AS ON DELETE TO asset.copy_location
DO INSTEAD (
UPDATE asset.copy_location SET deleted = TRUE WHERE OLD.id = asset.copy_location.id;
DELETE FROM asset.copy_location_order WHERE location = OLD.id;
DELETE FROM asset.copy_location_group_map WHERE location = OLD.id;
);

The database has not yet created asset.copy_location_order and asset.copy_location_group_map and therefore it throws an error. I edited this file and put that stanza on line 73

directly after the CREATE TABLE asset.copy_location_group_map stanza. That fixed the seeding issue.

[Jason Boyer (jboyer)]

Ugh, thanks for the catch. I'll update my branch. :(

[Jason Boyer (jboyer)]

I force-pushed an update to correct the rule placement.

[Michele Morgan (mmorgan)]

This is looking very promising!

There are a few issues I came across in testing. Not sure all of these should be considered showstoppers, I have listed them here in order of most to least importance (IMO) :

1. Copy templates containing deleted locations can be applied to items. The item can then be saved with the applied deleted copy location. One approach would be to prevent the saving of an item with a deleted copy location with an appropriate failure message.

2. If an undeleted item is in a deleted copy location, the item is opac visible. In theory, this should never happen, but perhaps deleted copy locations should NOT show in the opac by default.

3. If a copy location has been deleted, a new copy location with the same name cannot be added.

5. Deleted copy locations appear in Admin - Local Administration - Copy Location Groups.

6. Deleted copy locations appear in Admin - Local Administration - Copy Location Order

4. A Copy location that is referenced in a location group can be deleted.

7. A Copy location that is referenced in a circulation policy can be deleted.

[Jason Boyer (jboyer)]

Good news, I found time to really look this thing over and added a bunch of changes that would have been really irritating to track down later. The branch above has been updated against master as of 12/3. I may not have time to do much testing before the 7th, so if anyone else has time, try your best to break it!

A few responses re: Michele's notes:

1: Good point, added a check for that.

2: Once the bugs have been shaken out the only way for this to happen is playing around in psql (or pgadmin, etc.). I'd rather that reporting be accurate even though this isn't necessarily the kind of thing that people would often look for. If there are strong feelings either way one more line could be added to the delete protection rule.

3, 5, 6: Should be taken care of. I made the mistake of modeling my deleted location handling too closely on bibs, which you can pull up by id, rather than call numbers, which you can't.

4,7: Anywhere that had "ON DELETE CASCADE" I tried to address in the delete rule, not many things outside of action.circulation depend on them sticking around. I intentionally didn't remove circ rules referencing a deleted location though, but if it's desired those rules could have their active flag turned off. (I didn't want there to be any mysterious changes to circ policies unless you actively make changes to them.)

[Jason Boyer (jboyer)]

Finally, I have had time to look into this a bit. I was missing multiple direct pcrud searches for acpl's. One more forced update to that branch and now locations should no longer show up in any interface. Feeling much better about this one.

[Michele Morgan (mmorgan)]

This is looking great!

I tested Jason’s fixes to the issues above and everything looks good!

I have added my signoff:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mmorgan/LP1210541_Copy_Location_Deleted_Flag_signoff

One tiny issue remains in that deleted copy locations still show in the Copy Locations Order interface. I will open a separate bug for that.

Thanks for the great work, Jason!

[Jason Boyer (jboyer)]

I missed a one-line change in a call to pcrud.search in the copy locations order editor, that's been added and updated and it's ready to test. It should be ready to go now.

[Michele Morgan (mmorgan)]

Retested this and everything now looks good! Force pushed my new signoff to the branch above.

[Kathy Lussier (klussier)]

Added a release notes entry at http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/LP1210541_Copy_Location_Deleted_Flag_signoff

[Ben Shum (bshum)]

Pushed along to master for 2.8 beta.