KPAC Hold Notifications - SMS

Bug #1454871 reported by Terran McCanna on 2015-05-13
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Medium
Unassigned
2.8
Medium
Unassigned
2.9
Medium
Unassigned

Bug Description

This is a followup to LP#1282783 which was related to the KPAC not setting the patron's default hold notification preferences when a new hold is placed. The previous fix solved the problem if the library was only using email and phone notifications but did not take SMS text messages into account. I've reworked it to accommodate all three notification options and will post the code for review shortly.

Problem observed in Evergreen 2.7, 2.8

Changed in evergreen:
assignee: nobody → Terran McCanna (tmccanna)
Revision history for this message
Terran McCanna (tmccanna) wrote :
tags: added: pullrequest
Changed in evergreen:
assignee: Terran McCanna (tmccanna) → nobody
Changed in evergreen:
assignee: nobody → Heidi Florenzen (hflorenzen)
Revision history for this message
Heidi Florenzen (hflorenzen) wrote :

Unable to test if default settings for SMS notices are applied when placing a hold in the KPAC. Fix creates an Internal Server Error when attempting to place a hold from the KPAC. Holds were successful on the same server using the TPAC and the staff client.

Changed in evergreen:
assignee: Heidi Florenzen (hflorenzen) → nobody
Revision history for this message
Terran McCanna (tmccanna) wrote :

Drat, I have it working in production, but I missed something when I tried to apply it to master. I will troubleshoot it.

Changed in evergreen:
assignee: nobody → Terran McCanna (tmccanna)
tags: removed: pullrequest
Revision history for this message
Terran McCanna (tmccanna) wrote :

I removed the pullrequest tag since I need to work on it more.

Revision history for this message
Terran McCanna (tmccanna) wrote :

My apologies, I had an extra END tag in my earlier patch. I've redone it and it is working on my own sandbox:

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

Thanks in advance to whoever has a few moments to test it!

Terran

tags: added: pullrequest
Changed in evergreen:
assignee: Terran McCanna (tmccanna) → nobody
Revision history for this message
Ben Shum (bshum) wrote :

Rescuing this from getting too lost in the wild and assigning milestone targets for maintenance fixes.

Changed in evergreen:
status: New → Triaged
importance: Undecided → Medium
Changed in evergreen:
assignee: nobody → Josh Stompro (u-launchpad-stompro-org)
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I tested this against master and it works just like it should.

1. After install I enabled the SMS Notification YAOUS.
2. Set all notification options for a test patron.
3. Placed a hold in the kpac
4. Confirmed that the hold had all the notification options set, especially the Text Notification option.

Signoff Branch is at
working/user/stompro/lp1454871_kpac_hold_notifications_v2_signoff
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1454871_kpac_hold_notifications_v2_signoff

tags: added: signedoff
Changed in evergreen:
assignee: Josh Stompro (u-launchpad-stompro-org) → nobody
Galen Charlton (gmc) on 2016-03-04
Changed in evergreen:
milestone: none → 2.10-rc
status: Triaged → Fix Committed
status: Fix Committed → Confirmed
Revision history for this message
Galen Charlton (gmc) wrote :

Pushed all the way to rel_2_8. Thanks, Terran!

One note: could you ensure that your text editor is set to use spaces rather than tabs? Only C code (and Makefiles) currently use literal tabs.

Changed in evergreen:
status: Confirmed → Fix Committed
Revision history for this message
Terran McCanna (tmccanna) wrote :

Will do, thanks Galen.

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