Bug 9543 - Show patrons messaging subscription on holds notification
Summary: Show patrons messaging subscription on holds notification
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Marc Véron
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-04 20:02 UTC by Nicole C. Engard
Modified: 2017-12-07 22:15 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
The notification that pops up when checking in an item that has a hold on it it tells the librarian if the patron has subscribed to hold notifications (via email, sms, or phone). So now the librarian knows not to call the patron personally or email him or whatever.
Version(s) released in:
Circulation function:

Attachments
Bug 9543 - Show patrons messaging subscription on holds notification (3.48 KB, patch)
2016-01-26 14:53 UTC, Marc Véron 		Details | Diff | Splinter Review
Bug 9543 - Show patrons messaging subscription on holds notification (3.53 KB, patch)
2016-03-24 15:17 UTC, Marc Véron 		Details | Diff | Splinter Review
Bug 9543 - Show patrons messaging subscription on holds notification (3.58 KB, patch)
2016-04-06 00:48 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 9543 - Show patrons messaging subscription on holds notification not already waiting (1.87 KB, patch)
2016-04-06 00:57 UTC, Nick Clemens (kidclamp) 		Details | Diff | Splinter Review
Bug 16390: Accounts.t does not need MPL (1.53 KB, patch)
2016-05-09 20:56 UTC, Chris Kirby 		Details | Diff | Splinter Review
Bug 9543 - Show patrons messaging subscription on holds notification (3.64 KB, patch)
2016-05-16 09:53 UTC, Chris Kirby 		Details | Diff | Splinter Review
Bug 9543 - Show patrons messaging subscription on holds notification not already waiting (1.93 KB, patch)
2016-05-16 09:53 UTC, Chris Kirby 		Details | Diff | Splinter Review
Bug 9543 - (Follow-up) Show patrons messaging subscription on holds notification (3.61 KB, patch)
2016-05-20 09:09 UTC, Marc Véron 		Details | Diff | Splinter Review
Bug 9543 - Show patrons messaging subscription on holds notification (3.71 KB, patch)
2016-05-20 15:31 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 9543 - Show patrons messaging subscription on holds notification not already waiting (2.00 KB, patch)
2016-05-20 15:31 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 9543 - (Follow-up) Show patrons messaging subscription on holds notification (3.68 KB, patch)
2016-05-20 15:31 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 9543: Separate MTT with commas (1.31 KB, patch)
2016-05-20 15:31 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Show Obsolete (8) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2013-02-04 20:02:32 UTC 
It would be nice if on the notification that pops up when checking in an item that has a hold on it it told the librarian if the patron has subscribed to hold notifications (via email, sms, or phone) so that the librarian knew not to call the patron personally or email them or whatever.
Comment 1 Marc Véron 2015-12-20 16:07:06 UTC 
Still valid?
Comment 2 Nicole C. Engard 2016-01-25 21:27:34 UTC 
Yes.
Comment 3 Marc Véron 2016-01-26 14:53:56 UTC Comment hidden (obsolete)
Comment 4 Srdjan Jankovic 2016-03-18 03:33:27 UTC 
I apologise if this is a silly question, but why 'Hold_Filled' message? I'd expect 'Item_Check_in'.
Comment 5 Srdjan Jankovic 2016-03-18 03:39:22 UTC 
Please disregard, previous message, error in reading
Comment 6 Marc Véron 2016-03-24 15:17:18 UTC Comment hidden (obsolete)
Comment 7 Nick Clemens (kidclamp) 2016-04-06 00:48:30 UTC Comment hidden (obsolete)
Comment 8 Nick Clemens (kidclamp) 2016-04-06 00:57:00 UTC Comment hidden (obsolete)
Comment 9 Nick Clemens (kidclamp) 2016-04-06 00:57:37 UTC 
I added a follow up to display the information on the message box before a hold is confirmed, but only if no transfer is required
Comment 10 Marc Véron 2016-04-17 13:25:03 UTC 
Still applies on current master.
1st patch has signoff by Nick Clemens
2nd patch (follow-up by Nick Clemens) needs a signoff.
Comment 11 Chris Kirby 2016-05-06 17:27:26 UTC 
I have applied the patches. However, I am seeing a "Please transfer item to" message when I check in a book where patron home library and checkin library are the same.
Comment 12 Chris Kirby 2016-05-09 19:32:02 UTC 
-Applied patches - Made sure that SMS driver and TalkingTech sysprefs are defined  to enable SMS and Phone preferences for users (SMS driver can be a dummy
  value).
- Placed a hold for a patron
- Set patron's messaging preferences to different choices (including none)
- Check in the item that is on hold (it has not to be checked out for the
  test
- Verified that below the message "Hold found (item is already waiting") an
  information appears about the patron's messaging preferences.

--Message (notification preference) only appears where a transfer is not required.
Comment 13 Chris Kirby 2016-05-09 20:56:23 UTC Comment hidden (obsolete)
Comment 14 Chris Kirby 2016-05-10 00:28:12 UTC 
Hi Marc and Nick,
After signing off on the two patches (and obsoleting yours) I tried to attach my commit, but obviously I screwed up the commit. I'll try to straighten this out, but if you have suggestions let me know.
Comment 15 Nick Clemens (kidclamp) 2016-05-10 00:48:05 UTC 
(In reply to Chris Kirby from comment #14)
Hi Chris,

It looks like maybe you did:
git bz attach 9543 HEAD~2

Commits are counted from head, so the last patches are:
HEAD HEAD~1 (HEAD~2) (HEAD~3)

Two attach the two latest you want to do:
git bz attach 9543 HEAD~1^..

I un-obsoleted the original patches and obsoleted the extraneous one.  I am leaving as signed off for now
Comment 16 Chris Kirby 2016-05-10 02:00:49 UTC 
Thanks, Nick.
Comment 17 Chris Kirby 2016-05-16 09:53:26 UTC Comment hidden (obsolete)
Comment 18 Chris Kirby 2016-05-16 09:53:46 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2016-05-16 11:41:25 UTC 
QA comments:
- Avoid c/p of code (use BLOCK instead)
- Correctly name the variables: key is a message_transport_type, it would make sense to describe it better (mtt?)
- If several MTT are selected, the display is not nice: "Patron is notified: SMS. Email. Phone."
Comment 20 Marc Véron 2016-05-20 09:09:05 UTC Comment hidden (obsolete)
Comment 21 Marc Véron 2016-05-20 09:10:31 UTC 
Not sure if it can be switched back to signed-off or if it needs an other sign-off.
Comment 22 Jonathan Druart 2016-05-20 14:25:22 UTC 
Thanks Marc.
What about
  [%~ UNLESS loop.last %], [% ELSE %].[% END ~%]
To separate the mtt with a comma and end them with a dot?
I can provide a follow-up if you think it's better.
Comment 23 Marc Véron 2016-05-20 14:31:32 UTC 
(In reply to Jonathan Druart from comment #22)
> Thanks Marc.
> What about
>   [%~ UNLESS loop.last %], [% ELSE %].[% END ~%]
> To separate the mtt with a comma and end them with a dot?
> I can provide a follow-up if you think it's better.

Hi Jonathan, that would be nicer indeed, and yes please for the follow up :-)
Comment 24 Jonathan Druart 2016-05-20 15:31:13 UTC 
Created attachment 51660 [details] [review]
Bug 9543 - Show patrons messaging subscription on holds notification

Display an information about a patron's messaging preferences if an item
is checked in where the patron has put a hold.

To test:

- Apply patch
- Make sure that SMS driver and TalkingTech sysprefs are defined to
  enable SMS and Phone preferences for users (SMS driver can be a dummy
  value)
- Place a hold for a patron
- Set patron's messaging preferences to different choices (including none)
- Check in the item that is on hold (it has not to be checked out for the
  test
- Verify that below the message "Hold found (item is already waiting") an
  information appears about the patron's messaging preferences.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Chris Kirby <christopherlawrencekirby@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Jonathan Druart 2016-05-20 15:31:17 UTC 
Created attachment 51661 [details] [review]
Bug 9543 - Show patrons messaging subscription on holds notification not already waiting

Same plan as previous patch, but this one should show the message for a
hold not already waiting but not requiring a transfer (hold-found2 alert
        box)

Signed-off-by: Chris Kirby <christopherlawrencekirby@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Jonathan Druart 2016-05-20 15:31:21 UTC 
Created attachment 51662 [details] [review]
Bug 9543 - (Follow-up) Show patrons messaging subscription on holds notification

As of comment #19
- Code de-duplicated to a BLOCK statement
- Variable renamed
- Wording slightly changed. I like it that way, but feel free to change it
  in a follow-up

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 27 Jonathan Druart 2016-05-20 15:31:24 UTC 
Created attachment 51663 [details] [review]
Bug 9543: Separate MTT with commas

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 28 Brendan Gallagher 2016-06-03 08:49:28 UTC 
Pushed to Master - Should be in the November 2016 Release. Thanks
Comment 29 Frédéric Demians 2016-06-15 05:48:47 UTC 
I'm undecided whether to push this enhancement to 16.05 branch. CONS: As a enhancement, it should be reserved to November release. PRO: It's not that big. It impacts portions of code (returns.tt) that may have to be modified later by bugfixes that would be difficult to apply if master diverges too much from stable. Any advice?
Comment 30 Jonathan Druart 2016-06-18 13:41:47 UTC 
Why do you want to backport it?
Comment 31 Jonathan Druart 2016-06-18 13:45:26 UTC 
(In reply to Jonathan Druart from comment #30)
> Why do you want to backport it?

Ok, misread the PRO.
I'd say don't backport enhancements, in general.