Bug 15705

Summary: Notify the user on auto renewing
Product: Koha Reporter: Jonathan Druart <jonathan.druart+koha>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart+koha>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: cbrannon, claire_gravely, gmcharlt, h.meissner.82, janet.mcgowan, katrin.fischer, kyle.m.hall, kyle, martin.renvoize, nick, szapoun
Version: master   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 14757, 15582    
Bug Blocks: 18532, 21392, 19014    
Attachments: Bug 15705: Notify the user on auto renewing
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: Notify the user on auto renewing
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: Notify the user on auto renewing
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: Add is_tt param to the new letter
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: Notify the user on auto renewing
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: Add is_tt param to the new letter
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: Notify the user on auto renewing
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: Add is_tt param to the new letter
Bug 15705: Add specific warning messages for auto_too_much_oweing
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: Increasing size of auto_renew_error to 32
Bug 15705: DO NOT PUSH DBIC Changes
Bug 15705: Add specific warning messages for auto_too_much_oweing
Bug 15705: Add specific warning messages for auto_too_much_oweing
Bug 15705: Notify the user on auto renewing
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: Add is_tt param to the new letter
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: Increasing size of auto_renew_error to 32
Bug 15705: DO NOT PUSH DBIC Changes
Bug 15705: Add specific warning messages for auto_too_much_oweing
Bug 15705: Notify the user on auto renewing
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: Add is_tt param to the new letter
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: Increasing size of auto_renew_error to 32
Bug 15705: DO NOT PUSH DBIC Changes
Bug 15705: Add specific warning messages for auto_too_much_oweing
Bug 15705: Notify the user on auto renewing
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705: Add is_tt param to the new letter
Bug 15705: [DO NOT PUSH] DBIC Schema
Bug 15705: Increasing size of auto_renew_error to 32
Bug 15705: DO NOT PUSH DBIC Changes
Bug 15705: Add specific warning messages for auto_too_much_oweing
Bug 15705: Reset the last error when an auto renew successes
Bug 15705: Reset the last error when an auto renew successes
Bug 15705: DBIC Schema changes
Bug 15705: Add --send-notices to the cronjob script
Bug 15705: Notify the user on auto renewing
Bug 15705: Add specific warning messages for auto_too_much_oweing
Bug 15705: Reset the last error when an auto renew successes
Bug 15705: DBIC Schema changes
Bug 15705: Add --send-notices to the cronjob script
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice
Bug 15705 (QA Followup) Remove unused variable, fix typo and add due date to notice

Description Jonathan Druart 2016-01-29 16:55:56 UTC
Customisable notices must be triggered to tell patrons if the items were renewed and when failed to renew.
Comment 1 Jonathan Druart 2016-01-29 17:17:26 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-01-29 17:17:30 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-01-29 17:17:33 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-01-29 17:18:42 UTC
Sponsored-by: University of the Arts London
Comment 5 Jonathan Druart 2016-04-18 15:05:13 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-04-18 15:05:16 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-04-18 15:05:21 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-04-18 15:06:41 UTC
Patches have been rebased.
Comment 9 Jonathan Druart 2016-04-18 15:14:44 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2016-04-18 15:14:47 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2016-04-18 15:26:51 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-04-18 15:26:55 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2016-04-18 15:27:00 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2016-04-25 21:54:39 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2016-04-25 21:54:43 UTC
Created attachment 50681 [details] [review]
Bug 15705: Notify the user on auto renewing

When an issue is auto renewed, a notice will be sent to the patron.
The notice will tell if the renewed has been successfully done.

Note that this patch is not ready to be pushed yet, as it has some
defects:
- 1 notice per issue
- no way to disable the notice generation
- no way to specify patron categories to enable/disable the
  notifications

Test plan:
Use the automatic_renewals.pl script to auto renew issues.
If the auto renew has failed or succeeded, a notice will be generated in the
message_queue table.
If the error is "too_soon" or is the same as the previous error, the
notice won't be generated (we do not want to spam the patron).
Comment 16 Jonathan Druart 2016-04-25 21:54:47 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2016-04-25 21:54:51 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2016-04-26 16:51:20 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2016-04-26 16:51:41 UTC Comment hidden (obsolete)
Comment 20 Sofia Zapounidou 2016-06-03 09:32:54 UTC
We tested this patch twice on the ptfs-europe sandbox 2 and we received the following error:
The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 15705:
<h1>Something went wrong !</h1>Applying: Bug 15705: Notify the user on auto renewing
Applying: Bug 15705: Add is_tt param to the new letter
fatal: sha1 information is lacking or useless (installer/data/mysql/atomicupdate/bug_XXXXX.sql).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.

Patch failed at 0001 Bug 15705: Add is_tt param to the new letter

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.
Bug 15705 - Notify the user on auto renewing

50681 - Bug 15705: Notify the user on auto renewing
50683 - Bug 15705: Add is_tt param to the new letter
50732 - Bug 15705: [DO NOT PUSH] DBIC Schema
50733 - Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the AUTO_RENEWALS notice

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-15705-Add-istt-param-to-the-new-letter-rBphBw.patch .
Comment 21 Jonathan Druart 2016-06-03 11:26:35 UTC
(In reply to Sofia Zapounidou from comment #20)
> We tested this patch twice on the ptfs-europe sandbox 2 and we received the
> following error:
> The sandbox you've requested is not ready.

This patch depends on 3 other bug reports, did you apply them before?
Comment 22 Sofia Zapounidou 2016-06-06 08:11:42 UTC
No I did not. I did not understand this from the test plan.
Comment 23 Jonathan Druart 2016-10-24 12:18:43 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2016-10-24 12:18:49 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2016-10-24 12:18:54 UTC
Created attachment 56758 [details] [review]
Bug 15705: Add is_tt param to the new letter
Comment 26 Jonathan Druart 2016-10-24 12:18:58 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2016-10-24 12:19:04 UTC Comment hidden (obsolete)
Comment 28 Jonathan Druart 2016-10-24 15:19:59 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2016-10-24 15:20:05 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2016-10-30 18:49:02 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2016-10-30 20:17:10 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2016-11-24 10:59:16 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2016-11-24 10:59:20 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2016-11-24 10:59:23 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2016-11-24 10:59:27 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2016-11-24 10:59:30 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2016-11-24 10:59:34 UTC Comment hidden (obsolete)
Comment 38 Jonathan Druart 2016-11-24 10:59:38 UTC Comment hidden (obsolete)
Comment 39 Jonathan Druart 2017-03-14 16:42:05 UTC Comment hidden (obsolete)
Comment 40 Jonathan Druart 2017-03-14 16:42:11 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2017-03-14 16:42:17 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2017-03-14 16:42:22 UTC Comment hidden (obsolete)
Comment 43 Jonathan Druart 2017-03-14 16:42:27 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2017-03-14 16:42:32 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2017-03-14 16:42:38 UTC Comment hidden (obsolete)
Comment 46 Martin Renvoize 2017-03-23 08:19:11 UTC Comment hidden (obsolete)
Comment 47 Martin Renvoize 2017-03-23 08:19:15 UTC Comment hidden (obsolete)
Comment 48 Martin Renvoize 2017-03-23 08:19:18 UTC Comment hidden (obsolete)
Comment 49 Martin Renvoize 2017-03-23 08:19:22 UTC Comment hidden (obsolete)
Comment 50 Martin Renvoize 2017-03-23 08:19:26 UTC Comment hidden (obsolete)
Comment 51 Martin Renvoize 2017-03-23 08:19:29 UTC Comment hidden (obsolete)
Comment 52 Martin Renvoize 2017-03-23 08:19:33 UTC Comment hidden (obsolete)
Comment 53 Martin Renvoize 2017-03-23 08:20:47 UTC
Independently tested by Jonathan Field and Janet McGowan at PTFS Europe, I'm juts acting as the bugzilla gopher here, uploading the signed patches.
Comment 54 Jonathan Druart 2017-03-24 18:35:31 UTC Comment hidden (obsolete)
Comment 55 Jonathan Druart 2017-03-30 16:59:28 UTC Comment hidden (obsolete)
Comment 56 Jonathan Druart 2017-03-30 17:00:16 UTC
Last minute bug found, resetting the status to request another signoff on the last patch.
Comment 57 Martin Renvoize 2017-04-12 08:52:25 UTC Comment hidden (obsolete)
Comment 58 Martin Renvoize 2017-04-12 08:52:59 UTC
Signed off on final patch :)
Comment 59 Jonathan Druart 2017-05-02 13:33:30 UTC
Nick, the conflicts are on the DBIC schema file. Skip these 2 patches and regenerate the schema please.
Comment 60 Nick Clemens 2017-05-02 15:02:11 UTC
I really like this enhancement and want to see it get in, failing for reasons noted by Jonathan

- 1 notice per issue
**I think this could be later enhancement - is nice but not critical

- no way to disable the notice generation
- no way to specify patron categories to enable/disable the
  notifications
**This could be covered by adding the option to patron messaging preferences, combination of preference 'RenewalSendNotice' and checkout notice options are similar so not sure what the most clear path is, could be script parameters too?

Renewal success/failure messages in opac/staff client should be split onto a new bug
Comment 61 Jonathan Druart 2017-05-03 16:31:09 UTC Comment hidden (obsolete)
Comment 62 Jonathan Druart 2017-05-03 16:31:27 UTC Comment hidden (obsolete)
Comment 63 Jonathan Druart 2017-05-03 16:37:25 UTC
Nick, I added a --send-notices flag to the script.

I opened bug 18532, which may be sponsored soon, to tuned this option more granularly.

Hope it makes sense to you to have this enhancement in as it.
Comment 64 Nick Clemens 2017-05-04 17:30:39 UTC
Created attachment 63138 [details] [review]
Bug 15705: Notify the user on auto renewing

When an issue is auto renewed, a notice will be sent to the patron.
The notice will tell if the renewed has been successfully done.

Note that this patch is not ready to be pushed yet, as it has some
defects:
- 1 notice per issue
- no way to disable the notice generation
- no way to specify patron categories to enable/disable the
  notifications

Test plan:
Use the automatic_renewals.pl script to auto renew issues.
If the auto renew has failed or succeeded, a notice will be generated in the
message_queue table.
If the error is "too_soon" or is the same as the previous error, the
notice won't be generated (we do not want to spam the patron).

Signed-off-by: Janet McGowan <janet.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 65 Nick Clemens 2017-05-04 17:30:44 UTC
Created attachment 63139 [details] [review]
Bug 15705: Add specific warning messages for auto_too_much_oweing

Signed-off-by: Janet McGowan <janet.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 66 Nick Clemens 2017-05-04 17:30:49 UTC
Created attachment 63140 [details] [review]
Bug 15705: Reset the last error when an auto renew successes

The auto_renew_error has to be reset when an auto renew successes,
otherwise the patron is not going to receive the correct notice.

Test plan;
- Checkin an item and mark it as auto renewal (specify a due date in the past to allow auto renewals)
- Set OPACFineNoRenewalsBlockAutoRenew to 'Block' and 'OPACFineNoRenewals' to '1'
- Execute the script
=> Auto renewed, column auto_renew_error is null
- Add a fine of '2' to the patron
- Execute the script
=> Not auto renewed, column auto_renew_error is 'auto_too_much_oweing'
=> On the interface youo see the correct message "Automatic renewal failed, patron has unpaid fines"
- Pay the fine
- Execute the script

Without this patch the auto_renew_error is not reset and the patron is going to
receive a letter telling him he own too much money to the library
With this patch the patron will receive a letter to inform him the renew has been done!

Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 67 Nick Clemens 2017-05-04 17:30:53 UTC
Created attachment 63141 [details] [review]
Bug 15705: DBIC Schema changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 68 Nick Clemens 2017-05-04 17:30:58 UTC
Created attachment 63142 [details] [review]
Bug 15705: Add --send-notices to the cronjob script

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 69 Nick Clemens 2017-05-04 17:31:03 UTC
Created attachment 63143 [details] [review]
Bug 15705: DB Changes - Add [old_]issues.auto_renew_error and the  AUTO_RENEWALS notice

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 70 Nick Clemens 2017-05-04 17:31:07 UTC
Created attachment 63144 [details] [review]
Bug 15705 (QA Followup) Remove unused variable, fix typo and add due date to notice

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 71 Kyle M Hall 2017-05-12 14:48:51 UTC
Pushed to master for 17.05, thanks Jonathan, Nick!
Comment 72 Katrin Fischer 2017-05-14 10:28:00 UTC
This won't get ported back to 16.11.x as it is an enhancement.