*** Bug 19736 has been marked as a duplicate of this bug. ***

Please please please make it so patrons can Digest the auto renew emails.  When you get 30 or more emails in one day it is obnoxious.  I get MANY patron complaints about this. 

Is there any development on this?  We could maybe help out with that financially.  

Catherine Ingram
Cedar Park Public Library, TX

(In reply to Catherine E. Ingram from comment #2)
> Please please please make it so patrons can Digest the auto renew emails. 
> When you get 30 or more emails in one day it is obnoxious.  I get MANY
> patron complaints about this. 
> 
> Is there any development on this?  We could maybe help out with that
> financially.  
> 
> Catherine Ingram
> Cedar Park Public Library, TX

I get clients complaining about their patrons complaining about this as well.

I haven't done any development on this, but it's been more of a question of time than money, as I've been busy with other projects.

But there might be someone else able to do it for money.

UAL (along with I think 4 other Koha users) is part funding the turning of AUTO_RENEWALS into a digest email. My understanding of this is that Bywater Solutions hope to have the work completed for inclusion in 19.11 and we are right now in the process of sending the money through. This will be a great development as we have had to use the workaround of using RENEWAL (renewal success notification) & ODUE1 (renewal failure notification) in place of AUTO_RENEWALS.

*** Bug 23922 has been marked as a duplicate of this bug. ***

Created attachment 95030 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Created attachment 95031 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure renewals are allowed in circ rules
4. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row

Comment on attachment 95031 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

Review of attachment 95031 [details] [review]:
-----------------------------------------------------------------

::: misc/cronjobs/automatic_renewals.pl
@@ +2,4 @@
>  
>  # This file is part of Koha.
>  #
> +# Copyright (C) 2014 Hochschule f�r Gesundheit (hsg), Germany

This looks like a typo, can you fix please?

Comment on attachment 95031 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

Review of attachment 95031 [details] [review]:
-----------------------------------------------------------------

::: misc/cronjobs/automatic_renewals.pl
@@ +2,4 @@
>  
>  # This file is part of Koha.
>  #
> +# Copyright (C) 2014 Hochschule f�r Gesundheit (hsg), Germany

It looks like maybe the encoding of the file was changed? it Should be reading "für".

Created attachment 95337 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Created attachment 95338 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure renewals are allowed in circ rules
4. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row

(In reply to Katrin Fischer from comment #9)
> Comment on attachment 95031 [details] [review] [review]
> Bug 18532: set auto_renewals as patron messaging preference
> 
> Review of attachment 95031 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: misc/cronjobs/automatic_renewals.pl
> @@ +2,4 @@
> >  
> >  # This file is part of Koha.
> >  #
> > +# Copyright (C) 2014 Hochschule f�r Gesundheit (hsg), Germany
> 
> It looks like maybe the encoding of the file was changed? it Should be
> reading "für".

I didn't realize, but seems my editor didn't recognize that character encoding. I saved it now as UTF-8

Created attachment 95456 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure renewals are allowed in circ rules
4. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row

Signed-off-by: tgoat <tgoatley@gmail.com>

Created attachment 96049 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure renewals are allowed in circ rules
4. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Created attachment 96050 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure renewals are allowed in circ rules
4. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

We have recently started receiving complaints about the individual emails for auto-renewals.  It would be great to see this development happen.

(In reply to David Murguia from comment #16)
> We have recently started receiving complaints about the individual emails
> for auto-renewals.  It would be great to see this development happen.

It would help if you tested it

Created attachment 96487 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure renewals are allowed in circ rules
4. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

Created attachment 96494 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

Created attachment 96495 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure renewals are allowed in circ rules
4. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

We were playing around with it this morning and wondered about roll out.  The way it looked in the sandbox was that it would default to all existing patrons getting NO notices (with both email and digest un-checked). Did anyone else encounter this?  
~Catherine

(In reply to Catherine E. Ingram from comment #21)
> We were playing around with it this morning and wondered about roll out. 
> The way it looked in the sandbox was that it would default to all existing
> patrons getting NO notices (with both email and digest un-checked). Did
> anyone else encounter this?  
> ~Catherine

Setting to FQA to get Augustin to take a look here.

Did anyone have a chance to look at this?  Did you have questions for me?

---------------------------------
Catherine E. Ingram, MLS
Assistant Library Director
Cedar Park Public Library
Office: 512-401-5640
Catherine.Ingram@cedarparktexas.gov
550 Discovery Boulevard
Cedar Park, TX  78613
www.cedarparktexas.gov/library

Sorry, I was almost abducted by another (evil) bug.

I'll remove those lines from atomicupdate ASAP, so this can move forward.

Created attachment 98078 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

Created attachment 98079 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure automatic renewals are allowed in circ rules
4. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row
5. change autorenewals digest settings and save
6. go to opac with that same patron to "your messaging" option
CHECK => patron's messaging preferences has a "Auto renewals" row, and displays changes made in staff interface.
7. go to patron categories in administration of staff interface and choose a category.
CHECK => in "Default messaging preferences for this patron category" has a "Auto renewals" row
8. set "Auto renewals" messaging preference for a patron and check email and digest.
9. check out an item for that patron, and set it as automatic renewal and set due date as your current date and time
10. perl misc/cronjobs/automatic_renewals.pl --send-notices -v
SUCCESS => item was renewed, and in message_queue table there now should be message of type AUTO_RENEWALS_DGST for the patron
11. Sign off

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

(In reply to Catherine E. Ingram from comment #21)
> We were playing around with it this morning and wondered about roll out. 
> The way it looked in the sandbox was that it would default to all existing
> patrons getting NO notices (with both email and digest un-checked). Did
> anyone else encounter this?  
> ~Catherine

Hi Catherine, you were right.. I believed that if I didn't add that preference on patrons that already had preferences setted, Koha would break, but it didn't happen at the end.

Thanks for spotting that out.

Can you please try it out again?

Thanks.

(In reply to Agustín Moyano from comment #27)
> (In reply to Catherine E. Ingram from comment #21)
> > We were playing around with it this morning and wondered about roll out. 
> > The way it looked in the sandbox was that it would default to all existing
> > patrons getting NO notices (with both email and digest un-checked). Did
> > anyone else encounter this?  
> > ~Catherine
> 
> Hi Catherine, you were right.. I believed that if I didn't add that
> preference on patrons that already had preferences setted, Koha would break,
> but it didn't happen at the end.
> 
> Thanks for spotting that out.
> 
> Can you please try it out again?
> 
> Thanks.

Hi Agustín,

We have tested the latest patch you have submitted.

After applying the patch, we notice that all patrons will have Auto Renewals UNCHECKED for both Email and Digest.  This means NO Auto Renewal communication after we apply the patch.

This is a problem, because patrons will suddenly stop receiving all Auto Renewal communication unless they re-configure their preferences individually.

Can we request that the patch updates ALL patrons to have both Email and Digest CHECKED for the Auto Renewal function?

Thank you!
Catherine

(In reply to Catherine E. Ingram from comment #28)
> (In reply to Agustín Moyano from comment #27)
> > (In reply to Catherine E. Ingram from comment #21)
> > > We were playing around with it this morning and wondered about roll out. 
> > > The way it looked in the sandbox was that it would default to all existing
> > > patrons getting NO notices (with both email and digest un-checked). Did
> > > anyone else encounter this?  
> > > ~Catherine
> > 
> > Hi Catherine, you were right.. I believed that if I didn't add that
> > preference on patrons that already had preferences setted, Koha would break,
> > but it didn't happen at the end.
> > 
> > Thanks for spotting that out.
> > 
> > Can you please try it out again?
> > 
> > Thanks.
> 
> Hi Agustín,
> 
> We have tested the latest patch you have submitted.
> 
> After applying the patch, we notice that all patrons will have Auto Renewals
> UNCHECKED for both Email and Digest.  This means NO Auto Renewal
> communication after we apply the patch.
> 
> This is a problem, because patrons will suddenly stop receiving all Auto
> Renewal communication unless they re-configure their preferences
> individually.
> 
> Can we request that the patch updates ALL patrons to have both Email and
> Digest CHECKED for the Auto Renewal function?
> 
> Thank you!
> Catherine

Hi Catherine, I'm afraid that setting email and digest checked for all patrons would break the rule of keeping behavior as it is when upgrading..

Current behavior would match by making email checked and digest unchecked, but if I set that in every patron's preference that would mean that to get digest messages every patron should set that up individually.

So, what I propose is not to set any preference in patron, but set email checked and digest unchecked for every patron category. 

That way, I believe behavior whould be the same after update, and if a library wants digest messages for there patrons, they only have to set that once in patrons categories.

What do you think?

Regards
Agustín.

OK... But having NEITHER checked also breaks the rule of keeping behavior as it is when upgrading. That has been my whole point. 

To your point the email box SHOULD be default checked. (so they keep getting the auto renew emails)
The digest box should NOT be checked.

If both are not checked they will go from getting way too much email to getting NO emails about automatic renewals.

Catherine

I agree with Catherine that it would be preferable to have the patch update ALL patrons to have both Email and Digest CHECKED for the Auto Renewal function.  If our patrons stop receiving any renewal notices it will cause a big customer service issue for us. 

During our preparation for Go Live, we had some default messaging preferences put in place.  New patrons, whether created via self-registration or by staff, all get specific messaging options checked which they can later change if they want.  Perhaps adding Email and Digest Auto Renewal default messaging preferences would make sure that existing patrons don't lose anything.

Does this sound like it would work?
Barbara

Created attachment 99193 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

Created attachment 99194 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure automatic renewals are allowed in circ rules, have a positive number of allowed renewals and a positive number for renewal period
4. go to patron categories in administration of staff interface and choose a category.
CHECK => in "Default messaging preferences for this patron category" has a "Auto renewals" row and has email and digest options checked
5. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row with email and digest options checked
6. some settings and save
7. go to opac with that same patron to "your messaging" option
CHECK => patron's messaging preferences has a "Auto renewals" row, and displays changes made in staff interface.
8. uncheck email and digest from "Auto renewals" row and save
9. check out an item for that patron, and set it as automatic renewal and set due date as your current yesterday
10. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v
SUCCESS => item was renewed, and in message_queue table there is no new message for the patron
11. update patrons messaging preferences and set email option of "Auto renewals" row as checked
12. repeat steps 9 and 10
SUCCESS => item was renewed, but in message_queue table there is a new message of type AUTO_RENEWALS
13. update patrons messaging preferences and set email and digest options of "Auto renewals" row as checked
14. repeat steps 9 and 10
CHECK => item was not renewed
15. run step 10 again, but add -d flag, like this:
    perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v -d
SUCCESS => item was renewed, and in message_queue table there is a new message of type AUTO_RENEWALS_DGST
16. Sign off

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

(In reply to Barbara Johnson from comment #31)
> I agree with Catherine that it would be preferable to have the patch update
> ALL patrons to have both Email and Digest CHECKED for the Auto Renewal
> function.  If our patrons stop receiving any renewal notices it will cause a
> big customer service issue for us. 
> 
> During our preparation for Go Live, we had some default messaging
> preferences put in place.  New patrons, whether created via
> self-registration or by staff, all get specific messaging options checked
> which they can later change if they want.  Perhaps adding Email and Digest
> Auto Renewal default messaging preferences would make sure that existing
> patrons don't lose anything.
> 
> Does this sound like it would work?
> Barbara

Hi Barbara and Catherine,

Both of you were right.. Category messaging preferences didn't work as I expected.. I thought that it would be a default value when a patron didn't set his/hers own preference, but it was more like a template when you create a new patron.

So, in the update script I set email and digest options to checked.

Could you please test this again?

Thanks.

Hi Agustín,

Thanks for the new test plan.  I'm new at testing bugs.  I'm not sure what I'm supposed to do for step 10 (perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v) or 15 (add -d flag)

Thanks.

(In reply to Barbara Johnson from comment #35)
> Hi Agustín,
> 
> Thanks for the new test plan.  I'm new at testing bugs.  I'm not sure what
> I'm supposed to do for step 10 (perl misc/cronjobs/automatic_renewals.pl -c
> --send-notices -v) or 15 (add -d flag)
> 
> Thanks.

Hi Barbara, that step executes what a cronjob would do.

You should copy that line and paste it in a console, un the root of Koha project.

To get a console:
* if you are using koha-testing-docker in your linux console you should:
  1. docker exec -ti koha_koha_1 bash
  2. cd koha
  3. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v

* If you use vagrant, in your linux console, in the root of your vagrant project execute:
  1. vagrant ssh
  2. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v

* If you've got a testing server you should:
  1. ssh youruser@kohatestingserver
  2. cd /koha/project/path
  3. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v

I'm assuming in all cases that you are using any distribution of Linux to develop.

If you're using windows I don't know.

Hope it helps!

(In reply to Agustín Moyano from comment #36)
> (In reply to Barbara Johnson from comment #35)
> > Hi Agustín,
> > 
> > Thanks for the new test plan.  I'm new at testing bugs.  I'm not sure what
> > I'm supposed to do for step 10 (perl misc/cronjobs/automatic_renewals.pl -c
> > --send-notices -v) or 15 (add -d flag)
> > 
> > Thanks.
> 
> Hi Barbara, that step executes what a cronjob would do.
> 
> You should copy that line and paste it in a console, un the root of Koha
> project.
> 
> To get a console:
> * if you are using koha-testing-docker in your linux console you should:
>   1. docker exec -ti koha_koha_1 bash
>   2. cd koha
>   3. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v
> 
> * If you use vagrant, in your linux console, in the root of your vagrant
> project execute:
>   1. vagrant ssh
>   2. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v
> 
> * If you've got a testing server you should:
>   1. ssh youruser@kohatestingserver
>   2. cd /koha/project/path
>   3. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v
> 
> I'm assuming in all cases that you are using any distribution of Linux to
> develop.
> 
> If you're using windows I don't know.
> 
> Hope it helps!

Sorry, just crossed my mind.. if you're not a developer, then you should ask the help of a technician

This causes a behaviour change
 1 - This sets all patrons to want auto_renewal notices in digest form
 2 - The script requires '--digest' if the borrower wants digest or nothing is sent
We should default to all patrons want auto_renewals but not digest
This could be combined too with bug 23787 where we add the 'AUTO_RENEWAL' notice - if combined we could update borrowers to receive the notice if it already exists, and insert it but do not set borrowers to receive the notice if it does not exist

When running with the default notice the content is wrong, I think you want:
Dear [% borrower.firstname %] [% borrower.surname %],

        [% IF error %]
        There were [% error %] items that were not correctly renewed.
        [% END %]
        [% IF success %]
        There were [% success %] items that were correctly renewed.
        [% END %]


Additionally - the current code does not seem to allow for the inclusion of item specific information in the notices, I would say this is a blocker.

Copyright line in auto-renewals is malformed in these patches

We talked this one over this morning, we think the idea of setting  preferences by default is problematic, we would propose a workaround:
1 - Add a new system preference "SendAutoRenewalNotices"
2 - This syspref would replace the 'send-notices' switch on the cron, and would be marked with deprecation
3 - It would allow libraries to review their messaging settings for auto-renewal
4 - It should default to 'off' with a warning in the DB upgrade and release notes and in the cron if set but the cron could enable the syspref if the option is passed - to highlight the chnage
5 - the --digest option in the cron should be removed as the control should rely on patron settings

Created attachment 104880 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

Created attachment 104881 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure automatic renewals are allowed in circ rules, have a positive number of allowed renewals and a positive number for renewal period
4. go to patron categories in administration of staff interface and choose a category.
CHECK => in "Default messaging preferences for this patron category" has a "Auto renewals" row and has email and digest options checked
5. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row with email and digest options checked
6. some settings and save
7. go to opac with that same patron to "your messaging" option
CHECK => patron's messaging preferences has a "Auto renewals" row, and displays changes made in staff interface.
8. uncheck email and digest from "Auto renewals" row and save
9. check out an item for that patron, and set it as automatic renewal and set due date as your current yesterday
10. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v
SUCCESS => item was renewed, and in message_queue table there is no new message for the patron
11. update patrons messaging preferences and set email option of "Auto renewals" row as checked
12. repeat steps 9 and 10
SUCCESS => item was renewed, but in message_queue table there is a new message of type AUTO_RENEWALS
13. update patrons messaging preferences and set email and digest options of "Auto renewals" row as checked
14. repeat steps 9 and 10
CHECK => item was not renewed
15. run step 10 again, but add -d flag, like this:
    perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v -d
SUCCESS => item was renewed, and in message_queue table there is a new message of type AUTO_RENEWALS_DGST
16. Sign off

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

Created attachment 104882 [details] [review]
Bug 18532: (follow-up) Remove digest switch and add preference

Created attachment 104883 [details] [review]
Bug 18532: (follow-up) UNTESTED add individual issues to digest notice

This is how I think this works, I did not test this code

Created attachment 105500 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

Created attachment 105501 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

(Deprecated test plan. Please check the last patch)
To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure automatic renewals are allowed in circ rules, have a positive number of allowed renewals and a positive number for renewal period
4. go to patron categories in administration of staff interface and choose a category.
CHECK => in "Default messaging preferences for this patron category" has a "Auto renewals" row and has email and digest options checked
5. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row with email and digest options checked
6. some settings and save
7. go to opac with that same patron to "your messaging" option
CHECK => patron's messaging preferences has a "Auto renewals" row, and displays changes made in staff interface.
8. uncheck email and digest from "Auto renewals" row and save
9. check out an item for that patron, and set it as automatic renewal and set due date as your current yesterday
10. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v
SUCCESS => item was renewed, and in message_queue table there is no new message for the patron
11. update patrons messaging preferences and set email option of "Auto renewals" row as checked
12. repeat steps 9 and 10
SUCCESS => item was renewed, but in message_queue table there is a new message of type AUTO_RENEWALS
13. update patrons messaging preferences and set email and digest options of "Auto renewals" row as checked
14. repeat steps 9 and 10
CHECK => item was not renewed
15. run step 10 again, but add -d flag, like this:
    perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v -d
SUCCESS => item was renewed, and in message_queue table there is a new message of type AUTO_RENEWALS_DGST
16. Sign off

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>

Created attachment 105502 [details] [review]
Bug 18532: (follow-up) Remove digest switch and add preference

Created attachment 105503 [details] [review]
Bug 18532: add individual issues to digest notice and hide auto_renewals messaging preference when not needed

This patch enhances auto_renewals message, removes auto_renewals messaging preference when AutoRenewalNotices is not set to ‘preferences’ and uses that preference to send notices in automatic_renewals.pl script.

To test:

1. Apply patches
2. updatedatabase
3. make sure automatic renewals are allowed in circ rules, have a positive number of allowed renewals and a positive number for renewal period
4. Check AutoRenewalNotices preference
SUCCESS => AutoRenewalNotices has the value ‘cron’ (means that It keeps the usual behaviour)
5. Checkout two items for a patron, and set them as automatic renewal and set due date as your current yesterday
6. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql
7. Repeat step 5
8. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, and there is one message per item in message_queue table in mysql
9. Change AutoRenewalNotices to ‘never’
10. Repeat step 5
11. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql, even with the -s switch
12. Check any patron’s category, and any detail page in staff or OPAC interface, and in any of them you should find Auto Renewals messaging preference
13. Change AutoRenewalNotices to ‘preferences’
14. Repeat step 12, but this time all of them shows the Auto Renewals messaging preference.
15. Repeat step 5 with a patron that has no messaging preference setted
16. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql, because patron didn’t choose to receive messages
17. Grab a category and modify auto renewals messaging preferences, and save
18. Create a new patron from that category.
SUCCESS => created patron has the same messaging preference for auto renewals
19. Grab that patron and change auto renewals messaging preference to email but not digest
20. Repeat step 5 for that last patron.
21. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => Items were renewed, and there is a message for each item in message_queue table in mysql.
22. Change auto renewals messaging preference from the same patron and set to email and digest.
23. Repeat step 5.
24. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => items where renewed, and now there is only one message in message_queue table with the details of both renewed items.
25. Check that any changes to a patron’s auto renewals messaging preference in staff is reflected in OPAC, and the other way arround too.
26. Sign off

Thak you Nick so much for your code, it was illuminating! :D

Hi Augustin,

1) For the letter content:
installer/data/mysql/atomicupdate/bug-18532.perl

Please make some changes:
notification on auto renewing 
= Notification on auto renewals

was renewed until [% checkout.date_due %] 
Should be formatted as due date (see the other auto renewal notice now)
[% checkout.date_due | $KohaDates as_due_date => 1 %]
The sample files have a filter, but not the due date one, so there is an inconsistency there.

items that were not correctly renewed.
items that where correctly renewed.
Personally I'd get rid of "correctly" here - they are renewed, there is no incorrect renew :)

2) System preference:
The sentence doesn't quite work for the first and last choices, maybe one of the native speakers could help? Can you explain about the deprecation? It might appear as if the cronjob was deprecated, but I think what is meant is a specific option. I think being more clear here would be helpful.
+         - "Send automatic renewal notices "
+         - pref: AutoRenewalNotices
+           choices:
+               never: never
+               preferences: according to patron messaging preferences
+               cron: (Deprecated) according to cron switch

3) Please take a look at the terminology list on the wiki - we are aiming to be more consistent:
branch = library
borrower = patron
issue = checkout
etc.

4) Translated installers
You've only updated the Polish sample notices file, but you need to update every translated installer and the new default .yml file with the new notice.

(In reply to Katrin Fischer from comment #49)
> 2) System preference:
> The sentence doesn't quite work for the first and last choices, maybe one of
> the native speakers could help? Can you explain about the deprecation? It
> might appear as if the cronjob was deprecated, but I think what is meant is
> a specific option. I think being more clear here would be helpful.
> +         - "Send automatic renewal notices "
> +         - pref: AutoRenewalNotices
> +           choices:
> +               never: never
> +               preferences: according to patron messaging preferences
> +               cron: (Deprecated) according to cron switch

We would like to deprecate using the cron switch in favor of the preference. Unfortunately, we don't have a code path forward to set the patron preferences to a default that won't cause changes. The idea would be that libraries using the cron switch should set the patron messaging preferences to match their case, and then update the preference.  

The 'never' option should do what it says and disable auto-renewal notices.

Nick, can you please help with a rephrase of the pref too? I think it needs some more information, like which of the cron options we refer to.

(In reply to Katrin Fischer from comment #51)
> Nick, can you please help with a rephrase of the pref too? I think it needs
> some more information, like which of the cron options we refer to.

Hi Katrin, would it be clear if we set the values like this?
- "Send automatic renewal notices "
         - pref: AutoRenewalNotices
           choices:
               never: never send notices
               preferences: according to patron messaging preferences
               cron: (Deprecated) according to --send-notices cron switch

(In reply to Agustín Moyano from comment #52)
> (In reply to Katrin Fischer from comment #51)
> > Nick, can you please help with a rephrase of the pref too? I think it needs
> > some more information, like which of the cron options we refer to.
> 
> Hi Katrin, would it be clear if we set the values like this?
> - "Send automatic renewal notices "
>          - pref: AutoRenewalNotices
>            choices:
>                never: never send notices
>                preferences: according to patron messaging preferences
>                cron: (Deprecated) according to --send-notices cron switch

Makes sense to me.

Hm, the first option doesn't work as a sentence:

"Send automatic renewal notices " never send notices.

Maybe: 

Send automatic renewal notices: never 

?

Created attachment 114316 [details] [review]
Bug 18532: (follow-up) Fix notice message, add notice to all installer languages and fix preference message

My library may not adopt auto-renewals until there is a digest notice available, so I'm crossing my fingers that this makes it into 21.05.  Does it just need of testing after the last follow-up? (I haven't tested a lot, please forgive me for not being able to tell from here.)

(In reply to Sarah Cornell from comment #56)
> My library may not adopt auto-renewals until there is a digest notice
> available, so I'm crossing my fingers that this makes it into 21.05.  Does
> it just need of testing after the last follow-up? (I haven't tested a lot,
> please forgive me for not being able to tell from here.)

It's been signed off. If I recall correctly, it needs to go through Quality Assurance before it can be finalized.

But I'm here to make noise on this too - we have implemented Auto-renewals, but in a weird way so that we can get around this bug. We would still very much like to see this make it's way into Koha.

We sponsored this bug, along with three other libraries, and are eagerly awaiting this functionality.  What can we do to get this moving in the QA process?

(In reply to Barbara Johnson from comment #58)
> We sponsored this bug, along with three other libraries, and are eagerly
> awaiting this functionality.  What can we do to get this moving in the QA
> process?

I think making noise is a good way. I'm swamped at the moment, but I've been keen to see this functionality in Koha for a while, so I'll add it to my list. 

Hopefully someone else gets to it before me though. (Keep making noise :D.)

A good way to make noise is actually to test the patch and confirm they work as you expect. Even if already signed off, another stamp is always good to motivate QA :)

Sorry guys, the notice templates no longer apply :(

Martin, what does it mean that the notice templates no longer apply?  Is that an easy fix or are we back to square one?

Created attachment 115798 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Created attachment 115799 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

(Deprecated test plan. Please check the last patch)
To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure automatic renewals are allowed in circ rules, have a positive number of allowed renewals and a positive number for renewal period
4. go to patron categories in administration of staff interface and choose a category.
CHECK => in "Default messaging preferences for this patron category" has a "Auto renewals" row and has email and digest options checked
5. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row with email and digest options checked
6. some settings and save
7. go to opac with that same patron to "your messaging" option
CHECK => patron's messaging preferences has a "Auto renewals" row, and displays changes made in staff interface.
8. uncheck email and digest from "Auto renewals" row and save
9. check out an item for that patron, and set it as automatic renewal and set due date as your current yesterday
10. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v
SUCCESS => item was renewed, and in message_queue table there is no new message for the patron
11. update patrons messaging preferences and set email option of "Auto renewals" row as checked
12. repeat steps 9 and 10
SUCCESS => item was renewed, but in message_queue table there is a new message of type AUTO_RENEWALS
13. update patrons messaging preferences and set email and digest options of "Auto renewals" row as checked
14. repeat steps 9 and 10
CHECK => item was not renewed
15. run step 10 again, but add -d flag, like this:
    perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v -d
SUCCESS => item was renewed, and in message_queue table there is a new message of type AUTO_RENEWALS_DGST
16. Sign off

Signed-off-by: tgoat <tgoatley@gmail.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Created attachment 115800 [details] [review]
Bug 18532: (follow-up) Remove digest switch and add preference

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Created attachment 115801 [details] [review]
Bug 18532: add individual issues to digest notice and hide auto_renewals messaging preference when not needed

This patch enhances auto_renewals message, removes auto_renewals messaging preference when AutoRenewalNotices is not set to ‘preferences’ and uses that preference to send notices in automatic_renewals.pl script.

To test:

1. Apply patches
2. updatedatabase
3. make sure automatic renewals are allowed in circ rules, have a positive number of allowed renewals and a positive number for renewal period
4. Check AutoRenewalNotices preference
SUCCESS => AutoRenewalNotices has the value ‘cron’ (means that It keeps the usual behaviour)
5. Checkout two items for a patron, and set them as automatic renewal and set due date as your current yesterday
6. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql
7. Repeat step 5
8. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, and there is one message per item in message_queue table in mysql
9. Change AutoRenewalNotices to ‘never’
10. Repeat step 5
11. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql, even with the -s switch
12. Check any patron’s category, and any detail page in staff or OPAC interface, and in any of them you should find Auto Renewals messaging preference
13. Change AutoRenewalNotices to ‘preferences’
14. Repeat step 12, but this time all of them shows the Auto Renewals messaging preference.
15. Repeat step 5 with a patron that has no messaging preference setted
16. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql, because patron didn’t choose to receive messages
17. Grab a category and modify auto renewals messaging preferences, and save
18. Create a new patron from that category.
SUCCESS => created patron has the same messaging preference for auto renewals
19. Grab that patron and change auto renewals messaging preference to email but not digest
20. Repeat step 5 for that last patron.
21. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => Items were renewed, and there is a message for each item in message_queue table in mysql.
22. Change auto renewals messaging preference from the same patron and set to email and digest.
23. Repeat step 5.
24. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => items where renewed, and now there is only one message in message_queue table with the details of both renewed items.
25. Check that any changes to a patron’s auto renewals messaging preference in staff is reflected in OPAC, and the other way arround too.
26. Sign off

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Created attachment 115802 [details] [review]
Bug 18532: (follow-up) Fix notice message, add notice to all installer languages and fix preference message

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

(In reply to Barbara Johnson from comment #62)
> Martin, what does it mean that the notice templates no longer apply?  Is
> that an easy fix or are we back to square one?

Tomas understood :). It meant that the .SQL files had moved on in Koha and so the patches here no longer applied without some intervention.. 

Moving on to QAing now.

Created attachment 115829 [details] [review]
Bug 18532: Add new letter AUTO_RENEWALS_DGST, new Auto_Renewals in message_attributes, and configured message_transports

Signed-off-by: tgoat <tgoatley@gmail.com>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 115830 [details] [review]
Bug 18532: set auto_renewals as patron messaging preference

This patch adds the "Auto renewals" messaging preference on intranet and OPAC, and adds digest feature to misc/cronjobs/automatic_renewals.pl script.

(Deprecated test plan. Please check the last patch)
To test:
1. apply patches
2. perl installer/data/mysql/updatedatabase.pl
3. make sure automatic renewals are allowed in circ rules, have a positive number of allowed renewals and a positive number for renewal period
4. go to patron categories in administration of staff interface and choose a category.
CHECK => in "Default messaging preferences for this patron category" has a "Auto renewals" row and has email and digest options checked
5. grab a patron and go to details page
CHECK => patron's messaging preferences has a "Auto renewals" row with email and digest options checked
6. some settings and save
7. go to opac with that same patron to "your messaging" option
CHECK => patron's messaging preferences has a "Auto renewals" row, and displays changes made in staff interface.
8. uncheck email and digest from "Auto renewals" row and save
9. check out an item for that patron, and set it as automatic renewal and set due date as your current yesterday
10. perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v
SUCCESS => item was renewed, and in message_queue table there is no new message for the patron
11. update patrons messaging preferences and set email option of "Auto renewals" row as checked
12. repeat steps 9 and 10
SUCCESS => item was renewed, but in message_queue table there is a new message of type AUTO_RENEWALS
13. update patrons messaging preferences and set email and digest options of "Auto renewals" row as checked
14. repeat steps 9 and 10
CHECK => item was not renewed
15. run step 10 again, but add -d flag, like this:
    perl misc/cronjobs/automatic_renewals.pl -c --send-notices -v -d
SUCCESS => item was renewed, and in message_queue table there is a new message of type AUTO_RENEWALS_DGST
16. Sign off

Signed-off-by: tgoat <tgoatley@gmail.com>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Marti Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 115831 [details] [review]
Bug 18532: (follow-up) Remove digest switch and add preference

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 115832 [details] [review]
Bug 18532: Add individual issues to digest notice and hide auto_renewals messaging preference when not needed

This patch enhances auto_renewals message, removes auto_renewals messaging preference when AutoRenewalNotices is not set to ‘preferences’ and uses that preference to send notices in automatic_renewals.pl script.

To test:

1. Apply patches
2. updatedatabase
3. make sure automatic renewals are allowed in circ rules, have a positive number of allowed renewals and a positive number for renewal period
4. Check AutoRenewalNotices preference
SUCCESS => AutoRenewalNotices has the value ‘cron’ (means that It keeps the usual behaviour)
5. Checkout two items for a patron, and set them as automatic renewal and set due date as your current yesterday
6. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql
7. Repeat step 5
8. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, and there is one message per item in message_queue table in mysql
9. Change AutoRenewalNotices to ‘never’
10. Repeat step 5
11. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql, even with the -s switch
12. Check any patron’s category, and any detail page in staff or OPAC interface, and in any of them you should find Auto Renewals messaging preference
13. Change AutoRenewalNotices to ‘preferences’
14. Repeat step 12, but this time all of them shows the Auto Renewals messaging preference.
15. Repeat step 5 with a patron that has no messaging preference setted
16. perl misc/cronjobs/automatic_renewals.pl -c -s -v
SUCCESS => items were renewed, but there is no message in message_queue table in mysql, because patron didn’t choose to receive messages
17. Grab a category and modify auto renewals messaging preferences, and save
18. Create a new patron from that category.
SUCCESS => created patron has the same messaging preference for auto renewals
19. Grab that patron and change auto renewals messaging preference to email but not digest
20. Repeat step 5 for that last patron.
21. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => Items were renewed, and there is a message for each item in message_queue table in mysql.
22. Change auto renewals messaging preference from the same patron and set to email and digest.
23. Repeat step 5.
24. perl misc/cronjobs/automatic_renewals.pl -c -v
SUCCESS => items where renewed, and now there is only one message in message_queue table with the details of both renewed items.
25. Check that any changes to a patron’s auto renewals messaging preference in staff is reflected in OPAC, and the other way arround too.
26. Sign off

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 115833 [details] [review]
Bug 18532: (follow-up) Fix notice message, add notice to all installer languages and fix preference message

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Well done everyone, that was a hard slog but the final result is really nicely done.

No regressions found, QA scripts are happy and the feature works as expected.

Passing QA

Yay!  Thanks so much to Tomas and Martin for the final push.

Please fill the release notes.

Just wanted to check on this bug.  It passed QA on 1/26/21 but it’s now been set back to Failed QA.  Can someone provide an update on the problem?

(In reply to Barbara Johnson from comment #77)
> Just wanted to check on this bug.  It passed QA on 1/26/21 but it’s now been
> set back to Failed QA.  Can someone provide an update on the problem?

It is just missing a good description on the 'Text to go in the release notes' field here in Bugzilla!

Thanks for that info Tomas.

(In reply to Barbara Johnson from comment #79)
> Thanks for that info Tomas.

As a non-native speaker I welcome any idiomatic fixes. Feel free to jump in!

I cannot test, the "auto renewal" option does not appear in the "Patron messaging preferences" form. Please double-check.

Created attachment 118198 [details] [review]
Bug 18532: (follow-up) Fix notice message, add notice to all installer languages and fix preference message

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

(In reply to Jonathan Druart from comment #81)
> I cannot test, the "auto renewal" option does not appear in the "Patron
> messaging preferences" form. Please double-check.

Have you set AutoRenewalNotices accordingly ('preferences')?

Created attachment 118199 [details] [review]
Bug 18532: (QA follow-up) Atomicupdate syntax

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Gotcha, trying to see why it doesn't show, everything is in place.

Created attachment 118205 [details] [review]
Bug 18532: (follow-up) Fix rebasing issue

When the ILL notices were added, a bad rebase got rid of the notice
description in both messaging-preference-form.inc and opac-messaging.tt

This patch restores that line:

1. Set AutoRenewalNotices to 'according to patron...'
2. Edit a patron category
=> FAIL: There's an 'Unknown' label for 'Auto renewal'
3. Look at a patron's messaging preferences
=> FAIL: There's an 'Unknown' label for 'Auto renewal'
4. Look at a patron's messaging preferences in OPAC
=> FAIL: There's an 'Unknown' label for 'Auto renewal'
5. Apply this patch
6. Repeat 2, 3, 4
=> SUCCESS: 'Auto renewal' is displayed as expected

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

@RM: this is now ready. Both the atomic update and the templates issues, were a bad rebase after ILL patches got into master.

Created attachment 118318 [details] [review]
Bug 18532: (follow-up) Fix atomic update

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Looks like there is one more "7 must be 9" occurrence there:

 39     $dbh->do( q{
 40         insert into borrower_message_transport_preferences (borrower_message_preference_id, message_transport_type)                                                                                           
 41         select  p.borrower_message_preference_id, 'email'
 42         from    borrower_message_preferences p
 43         left join
 44                 borrower_message_transport_preferences t
 45         on      p.borrower_message_preference_id = t.borrower_message_preference_id
 46         where   p.message_attribute_id = 7
 47                 and t.borrower_message_preference_id is null;
 48     });

I can fix but I am wondering what's the point of this query. Can you explain please?

"was not renewed with error: [% checkout.auto_renew_error %]"

This is not good, we are going to display the raw error code in the template :-/

(In reply to Jonathan Druart from comment #90)
> "was not renewed with error: [% checkout.auto_renew_error %]"
> 
> This is not good, we are going to display the raw error code in the template
> :-/

And we don't want to deal with auto_too_soon for instance.
I think we should pick what we have in the existing notice template.

"There were 1 items that where renewed."

where => were

(In reply to Jonathan Druart from comment #92)
> "There were 1 items that where renewed."
> 
> where => were

And maybe we should deal with the plural? :-/ Or remove completely the numbers (and so success/error handling code in the cronjob)?

Created attachment 118329 [details] [review]
Bug 18532: (follow-up) Fix atomic update

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Created attachment 118330 [details] [review]
Bug 18532: (follow-up) fix messages and bug in automatic_renewals script

Created attachment 118331 [details] [review]
Bug 18532: (follow-up) fix messages and bug in automatic_renewals script

I am willing to push as it, but few more comments:

1. Please fix the plural handling on a follow-up bug report (to be done for 21.05, only for 'en' is fine)

2. Maybe we should actually not use the digest template if there is only 1 items (but yes, it's the same behaviour as the other digest notices)

3. This script would have benefit *a lot* from having the code in a module, covered by tests. Any additional enhancements to this file must take into account this move.

You could say "N item(s) successfully renewed" so it works for one or more and doesn't have to deal with verb tenses.

Pushed to master for 21.05, thanks to everybody involved!

Created attachment 118356 [details] [review]
Bug 18532: Add missing notice template for it-IT

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Follow-up pushed to master

Enhancement not pushed to 20.11.x