Org unit settings for "Selfcheck override events list" cannot be entered in the client's Library Settings Editor

[Michele Morgan]

Evergreen 2.6.2

When using the self check interface at https://<hostname>/eg/circ/selfcheck/main certain events requiring overrides can be set to override automatically. The org unit setting that controls this is "Selfcheck override events list" (database code: circ.selfcheck.auto_override_checkout_events)

When configuring this setting in client's Library Settings Editor, the values are not entered into the database in the proper format. This causes a javascript error and failed checkout with no message in the self check interface.

Entries in the actor.org_unit_setting table need to look like these. For a single event:

id org_unit name value

432 18 circ.selfcheck.auto_override_checkout_events ["COPY_ALERT_MESSAGE"]

For multiple events:

id org_unit name value

432 18 circ.selfcheck.auto_override_checkout_events ["COPY_ALERT_MESSAGE","COPY_NEEDED_FOR_HOLD"]

When entered or edited through the client, they are stored in the database like this:

id org_unit name value

433 37 circ.selfcheck.auto_override_checkout_events "[\"COPY_ALERT_MESSAGE\"]"

In the Library Settings Editor, in addition to storing the data in the proper format in the database, an example of what to enter should be displayed to the user.

[Michele Morgan (mmorgan)]

Meant to add a link to a relevant IRC log:

http://irc.evergreen-ils.org/evergreen/2014-09-19#i_125077

[Bob Wicksall (bwicksall)]

We have the same problem on 2.7.3.

I don't think the original bug description is correct:

"When configuring this setting in client's Library Settings Editor, the values are not entered into the database in the proper format."

Actually that format is fine. The problem is in selfcheck.js. JSON2js needs to be applied so the variable is loaded correctly.

This:

    var overrideEvents = this.orgSettings[SET_AUTO_OVERRIDE_EVENTS];

Needs to be changed to this:

    var overrideEvents = JSON2js(this.orgSettings[SET_AUTO_OVERRIDE_EVENTS]);

[Galen Charlton (gmc)]

Well, what the self-check interface is expecting is that the override event list as stored in the database would be something like this:

["EVENT_1","EVENT_2","EVENT_3"]

In other words, a JSON array. What the library settings editor actually produces is in fact:

"[\"EVENT_1\",\"EVENT_2\",\"EVENT_3\"]"

That isn't a JSON array -- it's a JSON string, that if run through JSON2js, *then* becomes a JSON array.

The stock OU type definition for circ.selfcheck.auto_override_checkout_events (and circ.selfcheck.block_checkout_on_copy_status) has its datatype set to "array", not "text".

This is a long-winded way of saying that I think that the original bug description is OK.

Now, Bob's suggested change does work if one has entered the setting via the library settings editor rather than directly via the database -- but it will break things if the setting had been entered as a JSON array in the DB.

So there are a few possibilities:

- we enhance the library settings editor so that it can let one edit array settings sensibly - e.g., by providing a textarea where each non-blank line is interpreted as an entry in the array, or by providing a multi-select and enumerating the appropriate options. (This would be easier to do well, by the way, if we moved the list of circ events into the database).
- we make the library settings editor refuse to update array settings, which will at least prevent breakage, but forces people to edit certain settings directly in the database.
- we write a specialize interface for managing circ.selfcheck.auto_override_checkout_events (and circ.selfcheck.block_checkout_on_copy_status), similar to what exists for ui.hide_copy_editor_fields.
- we run with Bob's approach, but that also means changing the type of circ.selfcheck.auto_override_checkout_events (and circ.selfcheck.block_checkout_on_copy_status) to text and providing a database update to munge previously-entered values of those settings accordingly

[Bob Wicksall (bwicksall)]

Thanks for the clarification Galen. That's what I get for jumping to conclusions.

I'm not sure what the best solution is but I'd like to avoid direct database edits whenever possible. Some of us don't host our own Evergreen installs so we don't have database write access. ;-) I like to minimize support calls for simple things.

[Mike Rylander (mrylander)]

For arbitrary arrays, I think teaching the UI to manage them is simplest. To that end, I offer the following (untested) patch as food for thought:

diff --git a/Open-ILS/xul/staff_client/server/admin/org_unit_settings.js b/Open-ILS/xul/staff_client/server/admin/org_unit_settings.js
index 92bf2ea..2a4c661 100644
--- a/Open-ILS/xul/staff_client/server/admin/org_unit_settings.js
+++ b/Open-ILS/xul/staff_client/server/admin/org_unit_settings.js
@@ -436,6 +436,8 @@ function osLaunchEditor(name) {
             case 'bool':
                 widget = osEditBoolSelect;
                 break;
+ case 'array':
+ osSettings[name].value = osSettings[name].value.join(', ');
             default:
                 widget = osEditTextBox;
         }
@@ -475,6 +477,9 @@ function osEditSetting(deleteMe) {
                     var val = osEditBoolSelect.getValue();
                     obj[name] = (val == 'true') ? true : false;
                     break;
+ case 'array':
+ obj[name] = osEditTextBox.getValue().split(/,\s*/);
+ break;
                 default:
                     obj[name] = osEditTextBox.getValue();
                     if(obj[name] == null) return;

[Bob Wicksall (bwicksall)]

I tried that patch against my 2.7 test server. If the setting has not been set and I try to edit I get:

    TypeError:osSettings[name].value is null

If the setting HAS been set and I try to edit I get:

   osSettings[name].value.join is not a function

I do like the strategy though.

[Michele Morgan (mmorgan)]

Marking Confirmed based on Comments.