webstaff UX: default hold notification preferences for patrons confusingly presented

[Galen Charlton]

Here's a situation; using bullet points to describe it for (hopefully) clarity:

* If a patron has a value set for the opac.hold_notify user setting (including the empty string), that value governs which notification checkboxes are selected on the OPAC place hold form.
* If they do not have a value set for that user setting, the OPAC place hold form will default to checking the phone and email notification boxes.
* In the XUL patron editor (when editing an existing patron), if the patron has a value set for that user setting, that governs which notification methods are checked on the registration form.
* In the XUL patron editor (when editing an existing patron), if the patron does NOT have a value set, the registration form defaults to checking the email and phone boxes, matching what the OPAC would do for the place hold form.
* The XUL patron editor did not always save values for opac.hold_notify when registering a new patron.
* Migrated patron records are not guaranteed to have opac.hold_notify set.

And here's the kicker:

* The web staff client's patron registration form (when editing an existing patron) will /always/ reflect the value, or not, of the user setting; it will NOT check the phone and email boxes if the user setting is not present.

This has led to usability confusion for at least one library, where the state of the checkboxes on the patron registration form in XUL reflected end result behavior while in the web staff client, the checkboxes just represent the state of the user setting.

Evergreen 3.0+

[Jason Stephenson (jstephenson)]

I've noticed also that patrons edited or created in the web staff client get entries in opac.hold_notify of an empty string if these are not set. I do not recall seeing that with the XUL client.

[a. bellenir (abellenir)]

users with empty strings for opac.hold_notify in actor.usr_setting seem to be problematic. when attempting to place a hold for one such user, i got javascript errors in the browser:

SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at JSON2js (JSON_v1.js:9)
    at eframe.js:249
    at u (vendor.bundle.js:6)
    at vendor.bundle.js:6
    at h.$digest (vendor.bundle.js:6)
    at vendor.bundle.js:6
    at r (vendor.bundle.js:6)
    at vendor.bundle.js:6 "Possibly unhandled rejection: {}"

i'll consider that a separate issue for now.

as for this bug, with the patron settings editor, how's this?

* specify a default hold notify of ':phone:email' (maybe someday make this a configurable setting)
* use the default user preference if the actual user preference is falsey (null or empty string)
* always prefix the saved preference with a ':' so that a user that specifies no contact as their preference will not fallback to default (do not save an empty string.)
* skip saving a value for the preference if begins at and remains the default

here's a branch: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commitdiff;h=21e0315bcbecaeaae862a28a1ef9893f6169321f

[[e-mail] (tji)]

* specify a default hold notify of ':phone:email' (maybe someday make this a configurable setting)

The default value could be set up via User Setting Type. We set them up with phone:email to tackle the issue caused by BiblioCommons (affecting only a few libraries), which does not set up phone_notify or email_notify in hold records if opac_hold_notify is not present.

Now on the web client, changing the default value seems having no effect on Patron registration screen.

I like to have the old behaviour on XUL back. Without the presence of opac_hold_notify, display both phone and email selected on patron edit/registration and in My Account. (BTW, these two checkboxes are still selected on Place Hold screen.)

Now this new behaviour on the web client is causing confusion (not knowing patron's choice is going with default of both email and phone or not being notified at all), and trouble when editing patron account (an entry for opac_hold_notify with "" is created).

Tina Ji
BC Libraries Coop

[Rogan Hamby (rogan-hamby)]

Patched tested and signed off on here: user/rogan/lp1774268signoff

[John Amundson (jamundson)]

I have tested this patch on a 3.2 system, and I'm seeing a couple issues with it.

* specify a default hold notify of ':phone:email' (maybe someday make this a configurable setting)
In my testing, registering a patron without changing notify preferences did not create a user setting for the patron. This was how the XUL client behaved. LP bug #1361258 sought to fix this by creating a default setting of "phone:email" for a patron created in the web client.

Adding the default setting was an improvement in a couple different fronts, including the OPAC, (see Full Testing below). There is also a community mobile app or two out there that only recognizes preferences if they exist in user settings.

Changes made in the OPAC to preferences are also not correctly reflected. If preferences are removed here, the setting is updated to an empty string, which the web client Edit screen now sees as phone/email notify but the Place Hold screen sees as "Do not notify".

Here are my thoughts after testing:
The current web client behavior works well for accounts created in the web client. The only issue comes from accounts created in the XUL client, but as the XUL client is deprecated in 3.2, perhaps a temporary solution of running a script to add user settings for opac.hold_notify of "phone:email" where they do not exist would be better.

Or, perhaps there would be a way to simply allow the web client Edit screen to translate a user with no hold notify settings as checkmarks for phone/email.

Here's my full testing:

Creating patron account in web client with default settings left:
- no user setting is added in database
- phone, email checked by default when placing a hold in web/xul/opac

Creating patron account in xul client with default settings left:
- no user setting is added in database
- phone, email checked by default when placing a hold in web/xul/opac

Updating default notify patron record in OPAC to remove all notify preferences:
- preferences screen shows email/phone not checked even though still set to default. Simply saving the screen does NOT update settings. Settings must be checked, saved, unchecked, and saved again to update...
- user setting updated to ""
- phone, email not checked by default when placing a hold in web/xul/opac

Updating patron record in web client to remove all notify preferences:
- user setting updated to ":"
- phone, email not checked by default when placing a hold in web/xul/opac

Updating patron record in xul client to remove all notify preferences:
- user setting updated to ":"
- phone, email not checked by default when placing a hold in web/xul/opac

EXTRA PROBLEM:
- when a patron record with an empty string, "", is opened in the web client, phone/email show checked by default on Edit screen even though this is reflected as a "don't notify" on the Place Hold screen
- saving the edit screen without making any changes keeps the empty string setting.

[[e-mail] (tji)]

I tested on 3.1.7.

I can confirm the above issues:

1. deselecting all checkboxes from Patron Edit and My Account adds different values in the usr_setting entry: ":" from Patron > Edit; "" from My Account

2. entry with "" in usr_setting is not correctly reflected as none checkbox being selected on Patron > Edit

If ":" is preferred for not_to_notify choice, the old value of "" needs to be taken care of.

Additional related findings:

If opac_hold_notify usr_setting_type has no default value, no entry is created for newly created patrons, though both email and phone checkboxes appear selected on Patron Registration/Edit/Place Hold/My Account.

If "phone:email" is set as the registration default in the usr_setting_type, you will see the same behaviour, but there is an entry with value of ":phone:email" created on patron registration.

This is XUL behaviour, too. We use the default value to force creating the usr_setting entry.

Tina Ji
BC Libraries Coop

[Chris Sharp (chrissharp123)]

Removed signedoff tag since two testers have found issues.

[Michele Morgan (mmorgan)]

Setting importance to High as we are seeing that increasing numbers of patrons are not being notified of holds due to the change in behavior in the web client.

[Suzanne Paterno (paterno)]

On patron edit screen set the email and phone notification to true when patron doesn't have any preferences. This mimics the behavior in the xul client.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/spaterno/lp1774268_holds_notifications_default

[Terran McCanna (tmccanna)]

Regarding the most recent patch - it is perfectly valid for a patron to not want to get notifications. I haven't had a chance to test this, but from the code it looks like that preference would be ignored.

[Michele Morgan (mmorgan)]

If a patron's notification preferences have been explicitly set to not receive notifications, either by staff or by the patron, there will be an entry in actor.usr_setting for that patron with the value "" for the opac.hold_notify preference.

This preference will be respected, none of the notification boxes will show as checked in the patron edit screen.

We have had this patch on our production system for over a month and it's performing as expected.

[Jason Etheridge (phasefx)]

For me, this patch did treat the empty string "" as a go ahead to check email and phone in patron registration, in today's master.

[Michele Morgan (mmorgan)]

I'm removing the needsrepatch tag and leaving the pullrequest on for Suzanne's branch in comment 10. Suzanne's branch lacks a signoff, so I've added mine:

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

We have been running this patch in production for over six months and it has been working well for us.

It would be great to get another signoff and get this fix in during Feedback Fest.

[Michele Morgan (mmorgan)]

Adding back the needsrepatch tag as this is now not working for the case when the user has selected no notifications. Will have an updated branch shortly.

[Jeff Davis (jdavis-sitka)]

In my testing of Suzanne's branch, if the user setting is "" (empty string), the hold notify checkboxes will be checked in the patron editor, which is not what we want.

Here is a branch, based on Suzanne's, with an additional commit to rectify the problem:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1774268-hold-notify-handle-empty-string

[Michele Morgan (mmorgan)]

Thanks for the patch Jeff! I tested it and it works great. I specifically tested the following cases for the effect of the user setting opac.hold_notify on the patron registration screen:

New patron - phone and email boxes are checked
User setting indicates one or more preferences - appropriate boxes are checked
User setting does not exist - phone and email boxes are checked
User setting value = "" - no boxes are checked

Here's my signoff to Jeff's updated branch:

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

[Jason Boyer (jboyer)]

Looking into this today things have been complicated by bug 1570072 which allows staff or patrons to update existing holds when these values change. for <3.5 it can be as simple as changing the condition in Suzanne's branch from "if (!notify)" to "if (!notify && !(notify === ''))" to combine her and Jeff's approaches, but for 3.5+ it needs to do a little more than just nope out. I'll see if I can throw some tuits at it.

[Jason Boyer (jboyer)]

On second thought, this works fine as-is; there's no need to over-complicate it. (My alternative thought of just setting the value when missing probably isn't ideal either...) so I've pushed this and condensed the two options to be a little bit tidier to rel_3_3 -> master.

Thanks everyone.

[Jason Boyer (jboyer)]

(All of that is to say that I Have Thoughts about ways to improve default hold notification settings, including actually making it a default setting vs a convention that's C&P'd wherever needed, but doing that stuff later doesn't mean this can't be fixed today.)