Taken from bug 14224: (Marcel de Rooy from comment #122 ) > <<borrowers.firstname>> <<borrowers.surname>> has added a note to the item > <<biblio.item>> - <<biblio.author>> (<<biblio.biblionumber>>). > > What is biblio.item supposed to print here? > I got this mail: > M De Rooy has added a note to the item - Schwartz, Randal L (23). > > And must say that it might not be that clear.. > Do we need biblio.title and an item number ? Have tested and can confirm that this bug exists.
Created attachment 64937 [details] [review] Bug 18915: Fix checkout note email message This patch changes patron_note to checkout_note in the appropriate places, and ensures the email gets the title of the item. To test: 1) Apply patch and update database 2) View the message_queue table in mysql 3) Check out an item if haven't already 4) Go to OPAC and set a checkout note for an item 5) View message_queue table and confirm it the title is included in the email and all instances of 'patron note' have been replaced with 'checkout note' 6) Disable javascript in browser 7) repeat steps 4 and 5 and confirm all works as expected Sponsored-by: Catalyst IT
What happens if a library has modified the note? - Modifications for title and content would get lost with the following statement in bug_18915_-_modifying_patron_note_notice.sql UPDATE letter SET code = 'CHECKOUT_NOTE', name = 'Checkout note on item set by patron', title = 'Checkout note', content = '<<borrowers.firstname>> <<borrowers.surname>> has added a note to the item <<biblio.title>> - <<biblio.author>> (<<biblio.biblionumber>>).' WHERE code = 'PATRON_NOTE';
(In reply to Marc Véron from comment #2) > What happens if a library has modified the note? - Modifications for title > and content would get lost with the following statement in > bug_18915_-_modifying_patron_note_notice.sql > > UPDATE letter SET code = 'CHECKOUT_NOTE', name = 'Checkout note on item set > by patron', title = 'Checkout note', content = '<<borrowers.firstname>> > <<borrowers.surname>> has added a note to the item <<biblio.title>> - > <<biblio.author>> (<<biblio.biblionumber>>).' WHERE code = 'PATRON_NOTE'; I see what you mean, but this patch is fixing the content of the note, so it does need to be changed. Unless you can think of another way to fix the note?
cc Marcel because of the initial comment Maybe a warning in update notes that contents of note have been reset? However, IMO librarians would not be happy about.
I think in another case, when <<items.fine>> was introduced, Dobrica managed to just replace the broken bit. Could this be an option here? Just replacing biblio.item with biblio.title? The old database update could give an example.
(In reply to Aleisha Amohia from comment #3) > (In reply to Marc Véron from comment #2) > > What happens if a library has modified the note? - Modifications for title > > and content would get lost with the following statement in > > bug_18915_-_modifying_patron_note_notice.sql > > > > UPDATE letter SET code = 'CHECKOUT_NOTE', name = 'Checkout note on item set > > by patron', title = 'Checkout note', content = '<<borrowers.firstname>> > > <<borrowers.surname>> has added a note to the item <<biblio.title>> - > > <<biblio.author>> (<<biblio.biblionumber>>).' WHERE code = 'PATRON_NOTE'; > > I see what you mean, but this patch is fixing the content of the note, so it > does need to be changed. Unless you can think of another way to fix the note? Aleisha, Could you fix the note by just a SQL replace for the wrong field? UPDATE table SET field = REPLACE(field ,from_str, to_str) etc etc So look for biblio.item only and replace that by biblio.title. If the library changed the note and fixed it already, there will be no problem. Please adjust this entry too: installer/data/mysql/updatedatabase.pl: VALUES ('circulation', 'PATRON_NOTE', '', 'Patron note on item', '0', 'Patron issue note', '<<borrowers.firstname>> <<borrowers.surname>> has added a note to the item <<biblio.item>> - <<biblio.author>> (<<biblio.biblionumber>>).','email'); And do not forget to add PATRON_NOTE (or CHECKOUT_NOTE) to installer/data/mysql/en/mandatory/sample_notices.sql It does not occur yet.. Thanks !
Created attachment 66518 [details] [review] Bug 18915: Fix checkout note email message This patch replaces the <<biblio.item>> in the email with <<biblio.title>> and uses CHECKOUT_NOTE instead of PATRON_NOTE. This patch also adds the notice to installer/data/mysql/en/mandatory/sample_notices.sql, and updates the PATRON_NOTE entry in installer/data/mysql/updatedatabase.pl To test: 1) Apply patch and update database 2) View the message_queue table in mysql 3) Check out an item if haven't already 4) Go to OPAC and set a checkout note for an item 5) View message_queue table and confirm it the title is included in the email and all instances of 'patron note' have been replaced with 'checkout note' 6) Disable javascript in browser 7) repeat steps 4 and 5 and confirm all works as expected Sponsored-by: Catalyst IT
Comment on attachment 66518 [details] [review] Bug 18915: Fix checkout note email message Review of attachment 66518 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/updatedatabase.pl @@ +14308,4 @@ > > $dbh->do(q| > INSERT IGNORE INTO letter (`module`, `code`, `branchcode`, `name`, `is_html`, `title`, `content`, `message_transport_type`) > + VALUES ('circulation', 'CHECKOUT_NOTE', '', 'Checkout note on item set by patron', '0', 'Checkout note', '<<borrowers.firstname>> <<borrowers.surname>> has added a note to the item <<biblio.title>> - <<biblio.author>> (<<biblio.biblionumber>>).','email'); I don't think updatedatabase should be modified like this. This is already done. You add a new DB update which tweaks PATRON_NOTE into CHECKOUT_NOTE, if this is correct logic. ::: opac/svc/patron_notes @@ +82,4 @@ > my $biblionumber = $issue->item->biblionumber; > my $letter = C4::Letters::GetPreparedLetter ( > module => 'circulation', > + letter_code => 'CHECKOUT_NOTE', Why even change the letter code, given that you aren't changing the svc name?
The test plan works. However, is it normal that if I add <<issues.note>> in the notice it comes back as blank in the message_queue table? I checked and my notes are in the issues table.
*** Bug 19772 has been marked as a duplicate of this bug. ***
Created attachment 69839 [details] [review] Bug 18915: Giving CHECKOUT_NOTE access to issues table This patch allows the notice to reference the issues table so that the checkout note itself can be added to the note if wanted, such as in Comment 9. Sponsored-by: Catalyst IT
(In reply to M. Tompsett from comment #8) > ::: installer/data/mysql/updatedatabase.pl > @@ +14308,4 @@ > > > > $dbh->do(q| > > INSERT IGNORE INTO letter (`module`, `code`, `branchcode`, `name`, `is_html`, `title`, `content`, `message_transport_type`) > > + VALUES ('circulation', 'CHECKOUT_NOTE', '', 'Checkout note on item set by patron', '0', 'Checkout note', '<<borrowers.firstname>> <<borrowers.surname>> has added a note to the item <<biblio.title>> - <<biblio.author>> (<<biblio.biblionumber>>).','email'); > > I don't think updatedatabase should be modified like this. This is already > done. You add a new DB update which tweaks PATRON_NOTE into CHECKOUT_NOTE, > if this is correct logic. I thought this change was necessary for fresh installs. If the atomic update is enough then I'm happy to take this out. > > ::: opac/svc/patron_notes > @@ +82,4 @@ > > my $biblionumber = $issue->item->biblionumber; > > my $letter = C4::Letters::GetPreparedLetter ( > > module => 'circulation', > > + letter_code => 'CHECKOUT_NOTE', > > Why even change the letter code, given that you aren't changing the svc name? Mostly so that the name of the note is consistent with the name of the syspref (AllowCheckoutNotes), therefore, the name of the feature. The filename should also be changed as checkout note is more appropriate than patron note anyway - I think this was just a missed case from the original bug.
Created attachment 69840 [details] [review] Bug 18915: Renaming svc/patron_notes to svc/checkout_notes For consistency.
QA: Please make sure bug 19772 is fixed.
(In reply to Jonathan Druart from comment #14) > QA: Please make sure bug 19772 is fixed. Yes this has been fixed. The problem was I'd missed a case when changing PATRON_NOTE to CHECKOUT_NOTE in svc/checkout_notes.
Patch tested with a sandbox, by Marjorie Vila <marjorie.barry-vila@collecto.ca>
Created attachment 70376 [details] [review] Bug 18915: Fix checkout note email message This patch replaces the <<biblio.item>> in the email with <<biblio.title>> and uses CHECKOUT_NOTE instead of PATRON_NOTE. This patch also adds the notice to installer/data/mysql/en/mandatory/sample_notices.sql, and updates the PATRON_NOTE entry in installer/data/mysql/updatedatabase.pl To test: 1) Apply patch and update database 2) View the message_queue table in mysql 3) Check out an item if haven't already 4) Go to OPAC and set a checkout note for an item 5) View message_queue table and confirm it the title is included in the email and all instances of 'patron note' have been replaced with 'checkout note' 6) Disable javascript in browser 7) repeat steps 4 and 5 and confirm all works as expected Sponsored-by: Catalyst IT Signed-off-by: Marjorie Vila <marjorie.barry-vila@collecto.ca>
Created attachment 70377 [details] [review] Bug 18915: Giving CHECKOUT_NOTE access to issues table This patch allows the notice to reference the issues table so that the checkout note itself can be added to the note if wanted, such as in Comment 9. Sponsored-by: Catalyst IT Signed-off-by: Marjorie Barry-Vila <marjorie.barry-vila@collecto.ca> https://bugs.koha-community.org/show_bug.cgi?id=19801 Signed-off-by: Marjorie Vila <marjorie.barry-vila@collecto.ca>
Created attachment 70378 [details] [review] Bug 18915: Renaming svc/patron_notes to svc/checkout_notes For consistency. Signed-off-by: Marjorie Barry-Vila <marjorie.barry-vila@collecto.ca> https://bugs.koha-community.org/show_bug.cgi?id=19801 Signed-off-by: Marjorie Vila <marjorie.barry-vila@collecto.ca>
Created attachment 70460 [details] [review] Bug 18915: Fix checkout note email message This patch replaces the <<biblio.item>> in the email with <<biblio.title>> and uses CHECKOUT_NOTE instead of PATRON_NOTE. This patch also adds the notice to installer/data/mysql/en/mandatory/sample_notices.sql, and updates the PATRON_NOTE entry in installer/data/mysql/updatedatabase.pl To test: 1) Apply patch and update database 2) View the message_queue table in mysql 3) Check out an item if haven't already 4) Go to OPAC and set a checkout note for an item 5) View message_queue table and confirm it the title is included in the email and all instances of 'patron note' have been replaced with 'checkout note' 6) Disable javascript in browser 7) repeat steps 4 and 5 and confirm all works as expected Sponsored-by: Catalyst IT Signed-off-by: Marjorie Vila <marjorie.barry-vila@collecto.ca> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 70461 [details] [review] Bug 18915: Giving CHECKOUT_NOTE access to issues table This patch allows the notice to reference the issues table so that the checkout note itself can be added to the note if wanted, such as in Comment 9. Sponsored-by: Catalyst IT Signed-off-by: Marjorie Barry-Vila <marjorie.barry-vila@collecto.ca> https://bugs.koha-community.org/show_bug.cgi?id=19801 Signed-off-by: Marjorie Vila <marjorie.barry-vila@collecto.ca> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 70462 [details] [review] Bug 18915: Renaming svc/patron_notes to svc/checkout_notes For consistency. Signed-off-by: Marjorie Barry-Vila <marjorie.barry-vila@collecto.ca> https://bugs.koha-community.org/show_bug.cgi?id=19801 Signed-off-by: Marjorie Vila <marjorie.barry-vila@collecto.ca> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 70463 [details] [review] Bug 18915: (QA follow-up) Add note for installs after 17.05 Since the 17.05 upgrade adds the note and the sql file did not yet include the note, we should add it when it is not there (for example new 17.11s). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested by running the dbrev while there is a letter and while not.
(In reply to Aleisha Amohia from comment #15) > (In reply to Jonathan Druart from comment #14) > > QA: Please make sure bug 19772 is fixed. > > Yes this has been fixed. The problem was I'd missed a case when changing > PATRON_NOTE to CHECKOUT_NOTE in svc/checkout_notes. No it is not fixed, if the notice template does not exist I do receive a success message "Your note about ITEM_TITLE has been saved and sent to the library."
Pushed to master for 18.05, thanks to everybody involved!
Created attachment 70492 [details] [review] Bug 18915: Add CHECKOUT_NOTE notice template for other languages Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Awesome work all! Pushed to Stable for 17.11.02
Pushed to 17.05.x for v17.05.09