I wanted to translate the AUTO_RENEWALS notice, but it's not in the sample_notices.sql. It is only in the updatedatabase.pl Martin confirmed on IRC that it should be in both.
Created attachment 93937 [details] [review] Bug 23787: Add AUTO_RENEWALS in sample_notices.sql This patch adds the AUTO_RENEWALS notice SQL insert query that was only available in the updatedatabase script into all the language-specific sample_notices.sql files in the installer (de-DE, en, es-ES, fr-CA, fr-FR, nb-NO, pl-PL, ru-RU, uk-UA). To test, make a fresh install in a language listed above, do NOT run updatedatabase, make sure the AUTO_RENEWALS is in the available notices.
I simply copied the INSERT query into all the files. I hope that's okay.
Hello Caroline, I believe you should add a script in atomicupdate that inserts this notice in letter table. AUTO_RENEWALS in updatedatabase.pl is inserted in version $DBversion = '16.12.00.035', so if someone has a version after that, they won't have that notice in the database. There you should make an "insert ignore" just in case.
2 things: When you run xt/sample_noticed.t you can see that it-IT is missing. I am adding it in a follow up. But another thing: The notice states twice... "The following item...", but there is no item information in the sample text. Should we add something? Also I'd like to make the subject a bit more patron friendly, maybe "Automatic renewals on your account" or similar? And "notification" would be nicer as "Notification". I can help with the updatedatabase if necessary!
Created attachment 94968 [details] [review] Bug 23787: Add AUTO_RENEWALS to it-IT installer
I'm editing the wording of this notice in updatedatabse over in Bug 24378. The English version has some grammatical errors and other issues. We should make sure those changes get reflected in both places.
Created attachment 100418 [details] [review] Bug 23787: Add AUTO_RENEWALS in sample_notices.sql This patch adds the AUTO_RENEWALS notice SQL insert query that was only available in the updatedatabase script into all the language-specific sample_notices.sql files in the installer (de-DE, en, es-ES, fr-CA, fr-FR, it-IT, nb-NO, pl-PL, ru-RU, uk-UA). To test, make a fresh install in a language listed above, do NOT run updatedatabase, make sure the AUTO_RENEWALS is in the available notices.
Updated Caroline's work to reflect wording changes I made over in bug 24378, folded it-IT and atomicupdate all into one patch. Fingers crossed I didn't mess it up!
Patch doesn't apply: Applying: Bug 23787: Add AUTO_RENEWALS in sample_notices.sql Using index info to reconstruct a base tree... A installer/data/mysql/en/mandatory/sample_notices.sql Falling back to patching base and 3-way merge... CONFLICT (modify/delete): installer/data/mysql/en/mandatory/sample_notices.sql deleted in HEAD and modified in Bug 23787: Add AUTO_RENEWALS in sample_notices.sql. Version Bug 23787: Add AUTO_RENEWALS in sample_notices.sql of installer/data/mysql/en/mandatory/sample_notices.sql left in tree. error: Failed to merge in the changes. Patch failed at 0001 Bug 23787: Add AUTO_RENEWALS in sample_notices.sql
Learned an important lesson about grabbing the latest version of master before writing a patch. Will try to get back to this today. Thanks!
Created attachment 100486 [details] [review] Bug 23787: add AUTO_RENEWALS to sample_notices To test: - create a new install - confirm presence of auto renewal notice
Adapted to new yaml format. It's my understanding that means one need not copy the notice into all the separate languages, so I only put it in the one yaml file. Please let me know if I'm mistaken in that. Thaanks!
Created attachment 100526 [details] [review] Bug 23787: Add AUTO_RENEWALS to sample_notices To test: - create a new install - confirm presence of auto renewal notice Signed-off-by: David Nind <david@davidnind.com>
You should insert the template notice for existing installs only if it does not exist yet.
(In reply to Jonathan Druart from comment #14) > You should insert the template notice for existing installs only if it does > not exist yet. I would recommend against adding the notice to existing installations. If you are using auto renewals they will be activated immediately. It might come as an unwanted surprise to libraries that have deleted it on purpose as they don't want to use it. (it seems we added it before, but might still want to handle this differently here?)
Created attachment 102731 [details] [review] Bug 23787: add AUTO_RENEWALS to sample_notices Removed the atomicupdate so we're not adding this to existing installs. To test: - create a new install - confirm presence of auto renewal notice
Created attachment 102829 [details] [review] Bug 23787: Add AUTO_RENEWALS to sample_notices To test: - create a new English (en) install - confirm presence of auto renewal notice Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 102830 [details] [review] Bug 23787: Add sample AUTO_RENEWALS notice to translatd installers This adds the default auto renewal notice to the translated installers. Translating will be handled by maintainers of the installers, but this way we don't end up with installations having a different set of notices than others. To test: - Verify the sample_notices.sql files load correctly or - Run the web installer for each of those languages and verify sample_notices loads correctly.
Created attachment 102831 [details] [review] Bug 23787: Add AUTO_RENEALS sample notice to es-ES installer Keeping this one separate as there is another bug to move es-ES to use the new .yml based tanslation process. This way this patch could be dropped easily if the other bug makes it in first. Test plan of second patch applies.
Hi Andrew, Signed off on your patch and made follow-ups for the translated installers. Could you maybe sign-off on those?
Created attachment 102868 [details] [review] Bug 23787: Add sample AUTO_RENEWALS notice to translatd installers This adds the default auto renewal notice to the translated installers. Translating will be handled by maintainers of the installers, but this way we don't end up with installations having a different set of notices than others. To test: - Verify the sample_notices.sql files load correctly or - Run the web installer for each of those languages and verify sample_notices loads correctly. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 102869 [details] [review] Bug 23787: Add AUTO_RENEALS sample notice to es-ES installer Keeping this one separate as there is another bug to move es-ES to use the new .yml based tanslation process. This way this patch could be dropped easily if the other bug makes it in first. Test plan of second patch applies. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed off. Thanks, Katrin! Learned some stuff about making different languages go in Koha :)
Please double check, using 'en', I see \' when editing the letter: It\'s too late to renew this item.
Created attachment 103763 [details] [review] Bug 23787: Add AUTO_RENEWALS to sample_notices Removed "\" from "It\'s too late..." -- apparently that doesn't need to get escaped in YAML To test: - create a new English (en) install - confirm presence of auto renewal notice Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Don't we want the date to be formatted as a due date?
I wanted to fix this, but I can't get the AUTO_RENEWAL notice to generate :( I was not sure if it was enough to add: ... | $KohaDates as_due_date => 1 %] or if it would also need: [%- USE KohaDates -%] I our sample notices library we have both versions.
It works for me The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %] You don't need the USE KohaDates (it's included by default). However, I am wondering: it is not possible to have the time part after a renewal, 23:59 is always set. That sounds like a bug, right? - due date is yesterday at 12:34, renew it, it become another day at 23:59 (with or without auto renewal).
I am not sure it's a bug - it would probably require that you define your circ rules in hours instead of days - but not sure if we support "hourly renewal" at all yet. What did you set up to get the notice? I had: - Set up the notice - Checked out several items with different due dates today and in the past - Put a hold on one (to prevent auto_renewal) - Ran the job... no notice, but renewals took place for some of my items as expected Patron had an email address. Are any of the messaging settings involved?
Did you pass --send-notices to the script?
(In reply to Jonathan Druart from comment #30) > Did you pass --send-notices to the script? Nope! I'll try again.
Created attachment 104385 [details] [review] Bug 23787: (follow-up) Format due date as due date
Created attachment 104675 [details] [review] Bug 23787: Add sample AUTO_RENEWALS notice to translatd installers This adds the default auto renewal notice to the translated installers. Translating will be handled by maintainers of the installers, but this way we don't end up with installations having a different set of notices than others. To test: - Verify the sample_notices.sql files load correctly or - Run the web installer for each of those languages and verify sample_notices loads correctly. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 104676 [details] [review] Bug 23787: Add AUTO_RENEWALS to sample_notices Removed "\" from "It\'s too late..." -- apparently that doesn't need to get escaped in YAML To test: - create a new English (en) install - confirm presence of auto renewal notice Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 104677 [details] [review] Bug 23787: (follow-up) Format due date as due date Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
not backported to 19.11.x ...because yaml.