Web Staff Client - Problems saving notification preferences

Bug #1642035 reported by Terran McCanna on 2016-11-15
This bug affects 2 people
Affects Status Importance Assigned to Milestone

Bug Description

In the 2.11 web client:

In the patron editor (both when creating a new patron and when editing a patron account), the list of SMS Carriers displays, but cannot be selected (just shows blank field after attempting to select a carrier from the list). Saving the form does not save the SMS checkbox or the phone checkbox either.

(The email checkbox seems to be fine.)

Andrea Neiman (aneiman) wrote :

confirmed in 2.12

Changed in evergreen:
status: New → Confirmed
Galen Charlton (gmc) wrote :

Couple notes for whoever takes this on:

- the input for the default SMS number is not bound to a model
- the list of SMS carriers is implemented as a uib-dropdown; this really ought to be a select

tji@sitka.bclibraries.ca (tji) wrote :

A related issue. Weird behaviour with Hold Notices checkboxes. It's inconsistent.

For example, when all 3 checkboxes (phone, email and sms) are selected when the record is loaded. Deselect sms and phone, click save. SMS is shown as selected, the other two are not. Quit then reload the record. SMS is still the only box selected.

When Phone and Email are selected when a record is loaded, deselect Phone then save. Only Email is shown as selected correctly. Deselect Email, select Phone, then Save. Phone is shown as selected, as expected. Deselect Phone, select email and sms, then save. None of the boxes is selected.

Cesar V (cesardv) on 2017-05-25
Changed in evergreen:
assignee: nobody → Cesar V (cesardv)
Jason Boyer (jboyer) wrote :

I have the hold notifications and opac.default_sms_notify values sorted here:

But opac.default_sms_carrier is going to require more Angular knowledge than I have currently.

tags: added: pullrequest
Jason Boyer (jboyer) wrote :

If you grabbed that branch before 2017-05-31 4:45pm reload it, I changed my mind about how to approach it and force-pushed an updated version. Everything in the user notification section should work with the exception of the carrier.

Jason Boyer (jboyer) wrote :

I found an example of a dropdown that I was able to learn from and adapt and now all of the hold notification settings should operate as usual (there are 2 commits at the tip of the above mentioned branch).

Cesar V (cesardv) wrote :

I've tested this fix, and it works fine.

Jason, opac.default_sms_carrier seems to be working for me... Are you still having issues with it?

However, I think there might a bug with the underlying ng controller though, since if you say add/modify the Default SMS/Text field or something on the Patron Edit view, Save, and then hit "Search Patron" on the top right, get another patron, the same SMS number will be there, as if it's part of the newly fetched patron's record, but it's not, it's old inputted data from the previous. Going to test further and report if needed.

Cesar V (cesardv) wrote :

On top of Jason's changes, I've added a small refactor in the controller's (regctl.js) compress_hold_notify() function.
No need to manually create a ":"-separated string of values, when array.join(':') will do that for you, and DRYs up the logic making it easier to read.

See: user/cesardv/LP1642035_Patron_Hold_Notify_Prefs


Changed in evergreen:
assignee: Cesar V (cesardv) → nobody
Jason Boyer (jboyer) wrote :

Cesar, the sms carrier option should be working as well; that's what I was referring to in my rather vague comment #6. :) Good catch on the stale settings when loading a new patron, hopefully that's not too hard to track down.

Cesar V (cesardv) wrote :

Jason, got it, thanks.

Yes, personally, I'm not a fan of the mixed-together-way that this patron/search view works... the "patron summary" area on the left keeps patron data visible even when the user has quit the patron to search for another patron record. I think that if the user clicks on the "Patron Search" this at least implicitly means they are done with the previous patron and their record should be "closed and forgotten," so to speak.

I feel there's also a privacy/security concern with that too... i.e patron PII data left visible on screen for longer than necessary just rubs me the wrong way... But I'm fairly new to Evergreen so take my 2 cents with a grain of salt... :)

Kathy Lussier (klussier) on 2017-06-21
Changed in evergreen:
assignee: nobody → Kathy Lussier (klussier)
Kathy Lussier (klussier) wrote :

Works for me. Merged to master and release 2.12.

Thank you Jason and Cesar!

Changed in evergreen:
assignee: Kathy Lussier (klussier) → nobody
milestone: 3.next → 3.0-alpha
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers