Bug 30555 - Add more sample notice for SMS messages
Summary: Add more sample notice for SMS messages
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Shi Yao Wang
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 35029
  Show dependency treegraph
 
Reported: 2022-04-15 15:42 UTC by Shi Yao Wang
Modified: 2023-10-11 13:25 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00


Attachments
Bug 30555 : Add more sample notice for sms messages (4.85 KB, patch)
2022-04-15 15:45 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 30555: Add more sample notice for sms messages (7.66 KB, patch)
2022-08-23 17:33 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 30555: Add more sample notice for sms messages (8.11 KB, patch)
2022-09-02 21:34 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 30555: Add more sample notice for sms messages (8.17 KB, patch)
2023-03-16 08:36 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Shi Yao Wang 2022-04-15 15:42:09 UTC

    
Comment 1 Shi Yao Wang 2022-04-15 15:45:08 UTC Comment hidden (obsolete)
Comment 2 Shi Yao Wang 2022-05-05 15:42:27 UTC
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
Comment 3 David Nind 2022-05-05 20:58:50 UTC
Have added you as the assignee - it's easy to forget to do.
Comment 4 David Nind 2022-05-09 05:09:38 UTC
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
Comment 5 Shi Yao Wang 2022-08-23 17:33:23 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2022-09-02 17:53:37 UTC
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.
Comment 7 Shi Yao Wang 2022-09-02 21:34:09 UTC
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!
Comment 8 Laura Escamilla 2023-03-06 12:52:17 UTC
(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>
Comment 9 Martin Renvoize 2023-03-16 08:36:04 UTC
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>
Comment 10 Martin Renvoize 2023-03-16 08:37:49 UTC
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 :)
Comment 11 Tomás Cohen Arazi 2023-03-20 12:43:09 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 12 Martin Renvoize 2023-03-20 16:59:55 UTC
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
Comment 13 Katrin Fischer 2023-10-11 13:25:54 UTC
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