Created attachment 133360 [details] [review] Bug 30555 : Add more sample notice for sms messages It would be nice if we had sms sample notice for due, hold, checkin and checkout
Comment on attachment 133360 [details] [review] Bug 30555 : Add more sample notice for sms messages >From a2b2d34ad2b89d0ba3fa559f7fb7482137fbbbdd Mon Sep 17 00:00:00 2001 >From: Shi Yao Wang <shiyao@inlibro.com> >Date: Fri, 15 Apr 2022 11:42:29 -0400 >Subject: [PATCH] Bug 30555 : Add more sample notice for sms messages > >Adds sms sample notice for due, hold, checkin and checkout >--- > .../mysql/en/mandatory/sample_notices.yml | 94 +++++++++++++++++++ > 1 file changed, 94 insertions(+) > >diff --git a/installer/data/mysql/en/mandatory/sample_notices.yml b/installer/data/mysql/en/mandatory/sample_notices.yml >index 0af93cd37b..300b7e7c6b 100644 >--- a/installer/data/mysql/en/mandatory/sample_notices.yml >+++ b/installer/data/mysql/en/mandatory/sample_notices.yml >@@ -396,6 +396,22 @@ tables: > - "----" > - "Thank you." > >+ - module: circulation >+ code: CHECKIN >+ branchcode: "" >+ name: "Item check-in (digest)" >+ is_html: 0 >+ title: "Check-ins" >+ message_transport_type: sms >+ lang: default >+ content: >+ - "Hi <<borrowers.firstname>> <<borrowers.surname>>," >+ - "The following items have been checked in:" >+ - "----" >+ - "[% biblio.title %]" >+ - "----" >+ - "Thank you." >+ > - module: circulation > code: CHECKOUT > branchcode: "" >@@ -411,6 +427,22 @@ tables: > - "----" > - "Thank you for visiting [% branch.branchname %]." > >+ - module: circulation >+ code: CHECKOUT >+ branchcode: "" >+ name: "Item check-out (digest)" >+ is_html: 0 >+ title: "Checkouts" >+ message_transport_type: sms >+ lang: default >+ content: >+ - "Hi <<borrowers.firstname>> <<borrowers.surname>>," >+ - "The following items have been checked out:" >+ - "----" >+ - "[% biblio.title %]" >+ - "----" >+ - "Thank you." >+ > - module: circulation > code: CHECKOUT_NOTE > branchcode: "" >@@ -437,6 +469,19 @@ tables: > - "" > - "<<biblio.title>>, <<biblio.author>> (<<items.barcode>>)" > >+ - module: circulation >+ code: DUE >+ branchcode: "" >+ name: "Item due reminder" >+ is_html: 0 >+ title: "Item due reminder" >+ message_transport_type: sms >+ lang: default >+ content: >+ - "Hi," >+ - "The following item(s) are now due:" >+ - "<<biblio.title>>" >+ > - module: circulation > code: DUEDGST > branchcode: "" >@@ -454,6 +499,18 @@ tables: > - "" > - "Thank you." > >+ - module: circulation >+ code: DUEDGST >+ branchcode: "" >+ name: "Item due reminder (digest)" >+ is_html: 0 >+ title: "Item due reminder" >+ message_transport_type: sms >+ lang: default >+ content: >+ - "Hi" >+ - "You have <<count>> item(s) that are now due" >+ > - module: circulation > code: HOLD_SLIP > branchcode: "" >@@ -621,6 +678,19 @@ tables: > - "" > - "<<biblio.title>>, <<biblio.author>> (<<items.barcode>>)" > >+ - module: circulation >+ code: PREDUE >+ branchcode: "" >+ name: "Advance notice of item due" >+ is_html: 0 >+ title: "Advance notice of item due" >+ message_transport_type: sms >+ lang: default >+ content: >+ - "Hi" >+ - "The following item will be due soon:" >+ - "<<biblio.title>>, <<biblio.author>> (<<items.barcode>>)" >+ > - module: circulation > code: PREDUEDGST > branchcode: "" >@@ -638,6 +708,18 @@ tables: > - "" > - "Thank you." > >+ - module: circulation >+ code: PREDUEDGST >+ branchcode: "" >+ name: "Advance notice of item due (digest)" >+ is_html: 0 >+ title: "Advance notice of item due" >+ message_transport_type: sms >+ lang: default >+ content: >+ - "Hi" >+ - "You have <<count>> item(s) that will be due soon." >+ > - module: circulation > code: RENEWAL > branchcode: "" >@@ -1227,6 +1309,18 @@ tables: > - "<<branches.branchaddress3>>" > - "<<branches.branchcity>> <<branches.branchzip>>" > >+ - module: reserves >+ code: HOLD >+ branchcode: "" >+ name: "Hold available for pickup" >+ is_html: 0 >+ title: "Hold available for pickup at <<branches.branchname>>" >+ message_transport_type: sms >+ lang: default >+ content: >+ - "Hi" >+ - "Your hold for <<biblio.title>> is available for pickup as of <<reserves.waitingdate>>" >+ > - module: reserves > code: HOLD > branchcode: "" >-- >2.25.1
Have added you as the assignee - it's easy to forget to do.
Hi Shi Yao Wang. I think this is a great idea, but it needs a test plan. I've created a draft test plan - I use koha-testing-docker for testing. I think this enhancement will require a database update for existing installations - where they don't have notices already, these should be updated. I'm not sure about this and how it should be done, so please check with some other developers. And apologies in advance if I'm not correct! The format or database updates was recently updated, see https://wiki.koha-community.org/wiki/Database_updates#How_to_write_an_atomicupdate_file Draft test plan: 1. Check that there are no SMS messages for these notices (Tools > Notices & slips): . CHECKIN - Item check-in (digest) . CHECKOUT - Item check-out (digest) . DUE - Item due reminder . DUEDGST - Item due reminder (digest) . PREDUE - Advance notice of item due . PREDUEDGST - Advance notice of item due (digest) . HOLD - Hold available for pickup 2. Apply the patch. 3. Update the database (updatedatabase when using koha-testing-docker) (I think a database update is required for this patch to work correctly for existing installations - maybe check with some other developers on how to do this. It should check that if there are no SMS messages for the notices, then the new default messages should be added. See https://wiki.koha-community.org/wiki/Database_updates#How_to_write_an_atomicupdate_file ) 4. flush_memcached and restart_all (when using koha-testing-docker). 5. Check that there are now SMS notices for the notices listed in step 1. 6. To check a new installation: . reset_all (for koha-testing-docker) . login and check that there are now SMS messages for the notices listed in step 1 7. Sign-off! Additional item: I think the QA team will probably want the commit message formatted correctly. In this case, the title should formatted with no space after the bug number - Bug 30555: Add .... See https://wiki.koha-community.org/wiki/Commit_messages David
Created attachment 139692 [details] [review] Bug 30555: Add more sample notice for sms messages Adds sms sample notice for due, hold, checkin and checkout Test plan: 1. Check that there are no SMS messages for these notices (Tools > Notices & slips): . CHECKIN - Item check-in (digest) . CHECKOUT - Item check-out (digest) . DUE - Item due reminder . DUEDGST - Item due reminder (digest) . PREDUE - Advance notice of item due . PREDUEDGST - Advance notice of item due (digest) . HOLD - Hold available for pickup 2. Apply the patch. 3. Update the database (./installer/data/mysql/updatedatabase.pl) 4. Refresh page 5. Check that there are now SMS notices for the notices listed in step 1. 6. Sign-off!
This is a little nit-picky I know, but I would like to see the new SMS notices match the existing email notice where possible within the character limit. Some of the updates do this but not all of them.
Created attachment 140192 [details] [review] Bug 30555: Add more sample notice for sms messages Adds sms sample notice for due, hold, checkin and checkout Test plan: 1. Check that there are no SMS messages for these notices (Tools > Notices & slips): . CHECKIN - Item check-in (digest) . CHECKOUT - Item check-out (digest) . DUE - Item due reminder . DUEDGST - Item due reminder (digest) . PREDUE - Advance notice of item due . PREDUEDGST - Advance notice of item due (digest) . HOLD - Hold available for pickup 2. Apply the patch. 3. Update the database (./installer/data/mysql/updatedatabase.pl) 4. Refresh page 5. Check that there are now SMS notices for the notices listed in step 1. 6. Sign-off!
(In reply to Shi Yao Wang from comment #7) > Created attachment 140192 [details] [review] [review] > Bug 30555: Add more sample notice for sms messages > > Adds sms sample notice for due, hold, checkin and checkout > > Test plan: > 1. Check that there are no SMS messages for these notices (Tools > Notices & > slips): > . CHECKIN - Item check-in (digest) > . CHECKOUT - Item check-out (digest) > . DUE - Item due reminder > . DUEDGST - Item due reminder (digest) > . PREDUE - Advance notice of item due > . PREDUEDGST - Advance notice of item due (digest) > . HOLD - Hold available for pickup > 2. Apply the patch. > 3. Update the database (./installer/data/mysql/updatedatabase.pl) > 4. Refresh page > 5. Check that there are now SMS notices for the notices listed in step 1. > 6. Sign-off! Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Created attachment 148252 [details] [review] Bug 30555: Add more sample notice for sms messages Adds sms sample notice for due, hold, checkin and checkout Test plan: 1. Check that there are no SMS messages for these notices (Tools > Notices & slips): . CHECKIN - Item check-in (digest) . CHECKOUT - Item check-out (digest) . DUE - Item due reminder . DUEDGST - Item due reminder (digest) . PREDUE - Advance notice of item due . PREDUEDGST - Advance notice of item due (digest) . HOLD - Hold available for pickup 2. Apply the patch. 3. Update the database (./installer/data/mysql/updatedatabase.pl) 4. Refresh page 5. Check that there are now SMS notices for the notices listed in step 1. 6. Sign-off! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
All working as expected and a nice little improvement. Whilst I take Owens comments on board, I believe the choices made regarding subtle differences make sense.. They're there to try to limit character count where a variable of unknown length is used.. The digests might even still need a switch to <<count>> rather than listing all items in my opinion.. but that's really a point for discussion. Passing QA as is for now :)
Pushed to master for 23.05. Nice work everyone, thanks!
As much as I like this change, it's cetainly an enhancement that wouldn't be expected on a running stable branch.. so chosing not to backport this to 22.11.x
Some lesson to learn here for future projects like this: letter.name gets translated, so we need to query the existing one with the code of the letter and use it for the new entries. If we do not do this, we end up with a display issue in the notices and slips editor, see: bug 35029