Bug 17509 - Notify patrons to return items requested on hold by another person
Summary: Notify patrons to return items requested on hold by another person
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Radek Šiman (R-Bit Technology, s.r.o.)
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 13846 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-10-27 10:29 UTC by Radek Šiman (R-Bit Technology, s.r.o.)
Modified: 2018-04-12 15:49 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17509: Added NotifyToReturnItemWhenHoldIsPlaced syspref (3.39 KB, patch)
2016-11-03 22:19 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17509: Added system preference to restrict recipients based on library (3.98 KB, patch)
2016-11-03 22:19 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17509: Improved system preferences for item return notification (4.43 KB, patch)
2016-11-03 22:20 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17509: renamed atomic file to suit conventions (1.34 KB, patch)
2016-11-03 22:20 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17509: Removed old name of atomic update file (1.35 KB, patch)
2016-11-03 22:20 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17509: Added notification letter HOLDPLACED_CONTACT (31.85 KB, patch)
2016-11-03 22:20 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17509: notify patrons to return an item whenever a hold is placed on it (4.84 KB, patch)
2016-11-03 22:20 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 17509: Fixed invalid patterns (1.43 KB, patch)
2016-11-03 22:21 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Added NotifyToReturnItemWhenHoldIsPlaced syspref (3.46 KB, patch)
2016-11-04 09:20 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Added system preference to restrict recipients based on library (4.05 KB, patch)
2016-11-04 09:20 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Improved system preferences for item return notification (4.50 KB, patch)
2016-11-04 09:20 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: renamed atomic file to suit conventions (1.41 KB, patch)
2016-11-04 09:21 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Removed old name of atomic update file (1.41 KB, patch)
2016-11-04 09:21 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Added notification letter HOLDPLACED_CONTACT (31.93 KB, patch)
2016-11-04 09:21 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: notify patrons to return an item whenever a hold is placed on it (4.88 KB, patch)
2016-11-04 09:21 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Fixed invalid patterns (1.33 KB, patch)
2016-11-04 09:21 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17509: Added tests (5.72 KB, patch)
2017-05-08 20:53 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Added NotifyToReturnItemWhenHoldIsPlaced syspref (3.42 KB, patch)
2017-05-09 06:26 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Added system preference to restrict recipients based on library (4.02 KB, patch)
2017-05-09 06:27 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Improved system preferences for item return notification (4.47 KB, patch)
2017-05-09 06:27 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: renamed atomic file to suit conventions (1.41 KB, patch)
2017-05-09 06:27 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Removed old name of atomic update file (1.41 KB, patch)
2017-05-09 06:27 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Added notification letter HOLDPLACED_CONTACT (32.04 KB, patch)
2017-05-09 06:27 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: notify patrons to return an item whenever a hold is placed on it (4.88 KB, patch)
2017-05-09 06:27 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Fixed invalid patterns (1.33 KB, patch)
2017-05-09 06:27 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17509: Added tests (5.80 KB, patch)
2017-05-09 06:27 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17509: (followup) Logic moved from C4 to Koha namespace (8.28 KB, patch)
2017-10-11 23:08 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-27 10:29:58 UTC
Create a new notification allowing to ask patrons to return a book which has just been put on hold by somebody else.
Comment 1 Radek Šiman (R-Bit Technology, s.r.o.) 2016-11-03 22:19:38 UTC
Created attachment 57143 [details] [review]
Bug 17509: Added NotifyToReturnItemWhenHoldIsPlaced syspref

This system preference will allow to ask patrons to return items
whenever a new hold is placed on them

Test plan:
1) Open System preferences / Circulation / Holds Policy
2) Change value from Enable to Don't enable and vice versa
3) Check NotifyToReturnItemWhenHoldIsPlaced is changed in database
Comment 2 Radek Šiman (R-Bit Technology, s.r.o.) 2016-11-03 22:19:57 UTC
Created attachment 57144 [details] [review]
Bug 17509: Added system preference to restrict recipients based on library

Test plan:
1) apply patch
2) go to System preferences / Circulation / Holds Policy
3) change which patrons will be contacted
4) compare your change against database value
NotifyToReturnItemFromLibrary
Comment 3 Radek Šiman (R-Bit Technology, s.r.o.) 2016-11-03 22:20:08 UTC
Created attachment 57145 [details] [review]
Bug 17509: Improved system preferences for item return notification

Test plan:
1) apply patch
2) go to System preferences / Circulation / Holds Policy
3) change NotifyToReturn* variables as you wish
4) verify your changes in database
Comment 4 Radek Šiman (R-Bit Technology, s.r.o.) 2016-11-03 22:20:20 UTC
Created attachment 57146 [details] [review]
Bug 17509: renamed atomic file to suit conventions
Comment 5 Radek Šiman (R-Bit Technology, s.r.o.) 2016-11-03 22:20:31 UTC
Created attachment 57147 [details] [review]
Bug 17509: Removed old name of atomic update file
Comment 6 Radek Šiman (R-Bit Technology, s.r.o.) 2016-11-03 22:20:46 UTC
Created attachment 57148 [details] [review]
Bug 17509: Added notification letter HOLDPLACED_CONTACT
Comment 7 Radek Šiman (R-Bit Technology, s.r.o.) 2016-11-03 22:20:59 UTC
Created attachment 57149 [details] [review]
Bug 17509: notify patrons to return an item whenever a hold is placed on it

Test plan:
1) apply patch
2) run database update
3) enable sending an email to the patrons to return an item whenever a
   hold request is placed on it
4) place a hold
5) patrons having checked-out the item on hold are notified by email
   (see HOLDPLACED_CONTACT letter)
Comment 8 Radek Šiman (R-Bit Technology, s.r.o.) 2016-11-03 22:21:13 UTC
Created attachment 57150 [details] [review]
Bug 17509: Fixed invalid patterns
Comment 9 Josef Moravec 2016-11-04 09:20:38 UTC
Created attachment 57157 [details] [review]
[SIGNED-OFF] Bug 17509: Added NotifyToReturnItemWhenHoldIsPlaced syspref

This system preference will allow to ask patrons to return items
whenever a new hold is placed on them

Test plan:
1) Open System preferences / Circulation / Holds Policy
2) Change value from Enable to Don't enable and vice versa
3) Check NotifyToReturnItemWhenHoldIsPlaced is changed in database

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Josef Moravec 2016-11-04 09:20:53 UTC
Created attachment 57159 [details] [review]
[SIGNED-OFF] Bug 17509: Added system preference to restrict recipients based on library

Test plan:
1) apply patch
2) go to System preferences / Circulation / Holds Policy
3) change which patrons will be contacted
4) compare your change against database value
NotifyToReturnItemFromLibrary

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 11 Josef Moravec 2016-11-04 09:20:57 UTC
Created attachment 57160 [details] [review]
[SIGNED-OFF] Bug 17509: Improved system preferences for item return notification

Test plan:
1) apply patch
2) go to System preferences / Circulation / Holds Policy
3) change NotifyToReturn* variables as you wish
4) verify your changes in database

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 12 Josef Moravec 2016-11-04 09:21:02 UTC
Created attachment 57161 [details] [review]
[SIGNED-OFF] Bug 17509: renamed atomic file to suit conventions

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Josef Moravec 2016-11-04 09:21:06 UTC
Created attachment 57162 [details] [review]
[SIGNED-OFF] Bug 17509: Removed old name of atomic update file

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 14 Josef Moravec 2016-11-04 09:21:11 UTC
Created attachment 57163 [details] [review]
[SIGNED-OFF] Bug 17509: Added notification letter HOLDPLACED_CONTACT

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 15 Josef Moravec 2016-11-04 09:21:15 UTC
Created attachment 57164 [details] [review]
[SIGNED-OFF] Bug 17509: notify patrons to return an item whenever a hold is placed on it

Test plan:
1) apply patch
2) run database update
3) enable sending an email to the patrons to return an item whenever a
   hold request is placed on it
4) place a hold
5) patrons having checked-out the item on hold are notified by email
   (see HOLDPLACED_CONTACT letter)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 16 Josef Moravec 2016-11-04 09:21:19 UTC
Created attachment 57165 [details] [review]
[SIGNED-OFF] Bug 17509: Fixed invalid patterns

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 17 Jonathan Druart 2016-12-14 11:21:02 UTC
2 quick comments:

1/ Maybe it would be better to have only 1 syspref, or at least squash them into the same syspref block (in the .pref file)
2/ It would be great to move GetBorrowersToSatisfyHold to the Koha namespace and return objects instead.

I don't think they are blocker, but recommended.

I am failing it because you do not provide tests for the changes you made in subroutines.
Comment 18 Radek Šiman (R-Bit Technology, s.r.o.) 2016-12-14 14:55:30 UTC
Jonathan, could you please tell me more regarding your syspref suggestion? I thought it's been already squashed into one single section having 2 sysprefs. Have you meant to do it another way? This is current layout:
http://prntscr.com/djd0f0
Comment 19 Jonathan Druart 2016-12-14 16:37:22 UTC
(In reply to Radek Šiman (R-Bit Technology, s.r.o.) from comment #18)
> Jonathan, could you please tell me more regarding your syspref suggestion? I
> thought it's been already squashed into one single section having 2
> sysprefs. Have you meant to do it another way? This is current layout:
> http://prntscr.com/djd0f0

Indeed, I wrongly wrote the diff. You can forget this point.
Comment 20 Katrin Fischer 2016-12-27 09:57:01 UTC
*** Bug 13846 has been marked as a duplicate of this bug. ***
Comment 21 pierre.genty02 2017-03-23 09:23:11 UTC
Hi, 
We are really interested in this new feature. Is someone still working on it ?
Comment 22 Radek Šiman (R-Bit Technology, s.r.o.) 2017-03-23 09:31:01 UTC
Hi, yes, it's still in development, but I'm currently working on another important issue within payments, so this one has to wait a bit. We need this kind of notification too.
Comment 23 Radek Šiman (R-Bit Technology, s.r.o.) 2017-05-08 20:53:29 UTC
Created attachment 63258 [details] [review]
Bug 17509: Added tests

prove t/db_dependent/Reserves/Notification.t
Comment 24 Josef Moravec 2017-05-09 06:26:45 UTC
Created attachment 63269 [details] [review]
[SIGNED-OFF] Bug 17509: Added NotifyToReturnItemWhenHoldIsPlaced syspref

This system preference will allow to ask patrons to return items
whenever a new hold is placed on them

Test plan:
1) Open System preferences / Circulation / Holds Policy
2) Change value from Enable to Don't enable and vice versa
3) Check NotifyToReturnItemWhenHoldIsPlaced is changed in database

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 25 Josef Moravec 2017-05-09 06:27:04 UTC
Created attachment 63270 [details] [review]
[SIGNED-OFF] Bug 17509: Added system preference to restrict recipients based on library

Test plan:
1) apply patch
2) go to System preferences / Circulation / Holds Policy
3) change which patrons will be contacted
4) compare your change against database value
NotifyToReturnItemFromLibrary

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 26 Josef Moravec 2017-05-09 06:27:14 UTC
Created attachment 63271 [details] [review]
[SIGNED-OFF] Bug 17509: Improved system preferences for item return notification

Test plan:
1) apply patch
2) go to System preferences / Circulation / Holds Policy
3) change NotifyToReturn* variables as you wish
4) verify your changes in database

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 27 Josef Moravec 2017-05-09 06:27:20 UTC
Created attachment 63272 [details] [review]
[SIGNED-OFF] Bug 17509: renamed atomic file to suit conventions

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 28 Josef Moravec 2017-05-09 06:27:25 UTC
Created attachment 63273 [details] [review]
[SIGNED-OFF] Bug 17509: Removed old name of atomic update file

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 29 Josef Moravec 2017-05-09 06:27:30 UTC
Created attachment 63274 [details] [review]
[SIGNED-OFF] Bug 17509: Added notification letter HOLDPLACED_CONTACT

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 30 Josef Moravec 2017-05-09 06:27:35 UTC
Created attachment 63275 [details] [review]
[SIGNED-OFF] Bug 17509: notify patrons to return an item whenever a hold is placed on it

Test plan:
1) apply patch
2) run database update
3) enable sending an email to the patrons to return an item whenever a
   hold request is placed on it
4) place a hold
5) patrons having checked-out the item on hold are notified by email
   (see HOLDPLACED_CONTACT letter)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 31 Josef Moravec 2017-05-09 06:27:40 UTC
Created attachment 63276 [details] [review]
[SIGNED-OFF] Bug 17509: Fixed invalid patterns

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 32 Josef Moravec 2017-05-09 06:27:45 UTC
Created attachment 63277 [details] [review]
[SIGNED-OFF] Bug 17509: Added tests

prove t/db_dependent/Reserves/Notification.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Test pass
Comment 33 Josef Moravec 2017-05-09 06:28:58 UTC
I tested the last patch and rebased the whole patchset on master.
Comment 34 David Cook 2017-06-13 01:50:35 UTC
In other systems, I think they refer to this as "recalling" an item. We might want to use similar terminology for the sake of users.
Comment 35 Katrin Fischer 2017-06-13 05:52:01 UTC
I think a recall goes usually further than this, possibly shortening the due date and removing the option to renew etc. - I think this is just a friendly email so not quite a recall yet.
Comment 36 Radek Šiman (R-Bit Technology, s.r.o.) 2017-06-13 06:19:58 UTC
This feature is meant exactly as Katrin said: it kindly asks the user to return the item ASAP only.
Comment 37 Liz Rea 2017-07-26 23:29:45 UTC
Let's start here with recalls, and build the other stuff in.
Comment 38 Radek Šiman (R-Bit Technology, s.r.o.) 2017-07-27 07:22:01 UTC
I'd like to see this patch to be published as is. The other stuff should be developed in a separately filed bug.
Comment 39 Liz Rea 2017-07-27 21:06:18 UTC
That's what I mean, let's get this in and build the rest of recalls off of it somewhere else.
Comment 40 Nick Clemens 2017-07-31 15:23:41 UTC
I like this one a lot, but a few issues:
1 - GetMember has been removed should use Koha::Patrons
2 - The sub should not go into C4:Reserves but should be a class method in Koha::Holds
you can get patrons with something like (adding conditions per the pref) 
Koha::Patrons->search({'item.biblionumber'=>$hold->biblionumber,issue_id=>{'!=' => undef} },{ join => {'issues' => 'item'} })
Comment 41 Radek Šiman (R-Bit Technology, s.r.o.) 2017-10-11 23:08:05 UTC
Created attachment 67982 [details] [review]
Bug 17509: (followup) Logic moved from C4 to Koha namespace

This patch simplifies logic by using Koha objects. Automated test has
been moved from t/db_dependent/Reserves to t/db_dependent/Holds.

Test plan:
1) apply patch
2) run database update
3) enable sending an email to the patrons to return an item whenever a
   hold request is placed on it (NotifyToReturnItemFromLibrary)
4) place a hold
5) patrons having checked-out the item on hold are notified by email
   (see HOLDPLACED_CONTACT letter)
Comment 42 Alex Buckley 2017-10-21 06:54:19 UTC
Hi 

Can you please rebase against master, a conflict is thrown when applying these patches on a clean, up to date branch.

Specifically the patch causing the issue is '[SIGNED-OFF] Bug 17509: Added notification letter HOLDPLACED_CONTACT' and the file with the conflict is in installer/data/mysql/uk-UA/mandatory/sample_notices.sql

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17509: Added NotifyToReturnItemWhenHoldIsPlaced syspref
Applying: Bug 17509: Added system preference to restrict recipients based on library
Applying: Bug 17509: Improved system preferences for item return notification
Applying: Bug 17509: renamed atomic file to suit conventions
Applying: Bug 17509: Removed old name of atomic update file
Applying: Bug 17509: Added notification letter HOLDPLACED_CONTACT
Using index info to reconstruct a base tree...
M	installer/data/mysql/uk-UA/mandatory/sample_notices.sql
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/uk-UA/mandatory/sample_notices.sql
CONFLICT (content): Merge conflict in installer/data/mysql/uk-UA/mandatory/sample_notices.sql
Failed to merge in the changes.
Patch failed at 0001 Bug 17509: Added notification letter HOLDPLACED_CONTACT
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/SIGNED-OFF-Bug-17509-Added-notification-letter-HOL-iFsWW_.patch