Not possible to create copy status without manual code changes

[Chris Sharp]

Evergreen offers system adminstrative users the ability to create new copy statuses via Admin -> Server Administration -> Copy Statuses, but the status's attributes are incomplete until the status is added, depending on the circumstances, to the following two files (assuming the XUL client here - there may be others in the web client):

Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm (in the "check_checkin_copy_status" subroutine):

If not added to the list of copy statuses considered "OK", staff will get a "COPY_BAD_STATUS" alert at checkin.

Open-ILS/xul/staff_client/chrome/content/main/constants.js (in the "my_constants" variable assignment):

If not added to the list of "magical_statuses", the status is assignable by catalogers in the Item Editor, for example. There are two other attributes there too ("block_mark_item_damaged", and "block_mark_item_action").

There may be other places in the code that need to be considered as well. As long as we're allowing end users to create custom statuses, each of these attributes need to also be configurable from the Copy Statuses UI and not within scripts.

[Michele Morgan (mmorgan)]

Marking this confirmed. Attributes like checkin_ok and magical_status should be columns in config.copy_status so the attributes can be customized, similar to was done for "is_available" in bug 1464709.

[Terran McCanna (tmccanna)]

This was originally reported in the XUL client - is someone able to confirm that it is still true in the web client?

[Kyle Huckins (khuckins)]

On current master, I'm not seeing any error/alert/toast about COPY_BAD_STATUS when checking in an item with a custom copy status, and the item status went to Reshelving, however I second making checkin_ok and magical_status a column in config.copy_status, as having those hardcoded is less maintainable overall.

[Kyle Huckins (khuckins)]

It appears that a few other locations reference hardcoded perl constant for each status as well:

1) OpenILS/Application/Circ/Transit.pm -> __abort_transit
If the copy status is "In Transit" and the transit's copy status is Available, Checked Out, In Process, On Holds Shelf, In Transit, Cataloging, On Reservation Shelf, or Reshelving, it is changed it Canceled Transit. Otherwise the copy status reverts to what it was before the transit. An additional field for this seems like it would be excessive unless we were to have an "additional_rules" string field, or possibly a map table, "copy status properties/ccsp" for example, though that could also be excessive.

2) OpenILS/SIP/Item.pm -> sip_circulation_status
Returns an arbitrary number for the following statuses: On Order(02), Available(03), Checked Out(04), In Process(06), On Holds Shelf(08), Reshelving(09), In Transit(10), Lost(12), Lost and Paid(12), and Missing(13). If the copy status is not any of those, it returns 01. This doesn't seem like it would need changing, as in this case 1 refers to "Other."

3) OpenILS/SIP/Item.PM -> available
This one is checking if an item is available, but it's hardcoded to the Available and Reshelving statuses based on IDs, rather than checking if the is_available boolean is true or false - seems like a potential oversight.

[Kyle Huckins (khuckins)]

WIP branch with a refactor of the mentioned subroutine in Circulation.PM and new columns based on the magical statuses for constants.js, as well as checkin_ok and a copy_status_event so that events don't need to be hardcoded: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1616170-custom-copy-status-maintainability

TODO:
- Ensure SIP/Item.PM available subroutine is factoring in is_available
- Make use of the cancel_transit_on_abort field in __abort_transit and apply cancel_transit_on_abort values in data.seed-values

[Kyle Huckins (khuckins)]

I've refactored the code in the above branch to separate out the magical statuses into a separate commit, as I'm not noticing any code outside the XUL client that references it. I've also covered the SIP and Transit bits in separate commits within the branch. I haven't tested the SIP code yet, but I'm not running into any issues with the other changes.

[Kyle Huckins (khuckins)]

I've amended my previous commit for magical statuses, removing the hardcoded magical_status checks, now instead checking to determine if the status itself has content in its magical_status field. After some testing, this should be good to go, though I've had trouble testing the SIP changes, as the current SIP docs seem to indicate that SIP2 doesn't currently support checking out an item(not certain on that).

[Jennifer Pringle (jpringle-u)]

When this is tested, one thing to check is whether or not items in the newly created statuses show up on the holds pull list.

[Terran McCanna (tmccanna)]

The good news:

I tested this on current master (3.7 beta-ish) and I am able to create new statuses and have them appear in the Status dropdown when editing an item.

I verified that it does pay attention to whether or not the status is OPAC-visible and whether or not the status is holdable.

I verified that if I place a hold (either title level or item level) and scan that item in, it recognizes the hold and routes it to the holds shelf.

And the bad:

However, the status does NOT appear to be recognized by the process that populates the Holds Pull List. (Thanks to jpringle for mentioning that in comment #8.)