Bug 29393

Summary: Ability to send emails from patron details page
Product: Koha Reporter: Alex Arnaud <alex.arnaud>
Component: NoticesAssignee: Alex Arnaud <alex.arnaud>
Status: Pushed to main --- QA Contact: Martin Renvoize <martin.renvoize>
Severity: new feature    
Priority: P5 - low CC: andrewfh, anneli.osterman, bwsdonna, dcook, fridolin.somers, lucas, martin.renvoize, michaela.sieber, rachael.laritz
Version: MainKeywords: release-notes-needed
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32436
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00
Bug Depends on: 12802    
Bug Blocks: 36697, 36717    
Attachments: Bug 29393 - Ability to send emails from patron details page
Bug 29393 - Ability to send emails from patron details page
Bug 29393 - Use patron language in letters
Bug 29393 - add a dedicated letters module for add message
Bug 29393 - Add send_messages_to_borrowers permission
Bug 29393 - Don't show notice templates for each language
Bug 29393 - Ability to send emails from patron details page
Bug 29393 - Use patron language in letters
Bug 29393 - add a dedicated letters module for add message
Bug 29393 - Add send_messages_to_borrowers permission
Bug 29393 - Don't show notice templates for each language
Bug 29393 - Sort notice templates by name
Bug 29393: (follow-up) Explicite use of methods from C4
Bug 29393: Ability to send emails from patron details page
Bug 29393: Use patron language in letters
Bug 29393: Add a dedicated letters module for add message
Bug 29393: Add send_messages_to_borrowers permission
Bug 29393: Don't show notice templates for each language
Bug 29393: Sort notice templates by name
Bug 29393: (follow-up) Explicite use of methods from C4
Bug 29393: Prevent SQL errors when SQL strict mode is set
Bug 29393: Ability to send emails from patron details page
Bug 29393: Use patron language in letters
Bug 29393: Add a dedicated letters module for add message
Bug 29393: Add send_messages_to_borrowers permission
Bug 29393: Don't show notice templates for each language
Bug 29393: Sort notice templates by name
Bug 29393: (follow-up) Explicite use of methods from C4
Bug 29393: Prevent SQL errors when SQL strict mode is set
Bug 29393: Prevent SQL errors when SQL strict mode is set
Bug 29393: Prevent SQL errors when SQL strict mode is set
Bug 29393: Prevent SQL errors when SQL strict mode is set
Bug 29393: Ability to send emails from patron details page
Bug 29393: Use patron language in letters
Bug 29393: Add a dedicated letters module for add message
Bug 29393: Add send_messages_to_borrowers permission
Bug 29393: Don't show notice templates for each language
Bug 29393: Sort notice templates by name
Bug 29393: (follow-up) Explicite use of methods from C4
Bug 29393: Prevent SQL errors when SQL strict mode is set
Bug 29393: Ability to send emails from patron details page
Bug 29393: Use patron language in letters
Bug 29393: Add a dedicated letters module for add message
Bug 29393: Add send_messages_to_borrowers permission
Bug 29393: Don't show notice templates for each language
Bug 29393: Sort notice templates by name
Bug 29393: (follow-up) Explicite use of methods from C4
Bug 29393: Prevent SQL errors when SQL strict mode is set
Bug 29393: (QA follow-up) Make database update idempotent
Bug 29393: (QA follow-up) Tidy add_message.pl
Bug 29393: (QA follow-up) Tidy and make executable the new test
Bug 29393: (QA follow-up) Improve modal styling
Bug 29393: (QA follow-up) Add key to Permissions test

Description Alex Arnaud 2021-11-02 15:04:18 UTC

    
Comment 1 Alex Arnaud 2021-11-03 10:19:50 UTC Comment hidden (obsolete)
Comment 2 Alex Arnaud 2021-11-03 10:22:36 UTC
Created attachment 127238 [details] [review]
Bug 29393 - Ability to send emails from patron details page

Test plan
  - apply Bug 12802
  - go to a patron's details page
  - click on "Add message" in tools bar
  - choose "Email" in "Add a message for"
  - Select a notice (module members) or manualy fill the message
  - Save
  - Check the message is saved in message_queue table
Comment 3 Alex Arnaud 2021-11-03 10:27:46 UTC
This adds the ability to send email via patron messages (/cgi-bin/koha/circ/add_message.pl).
This feature depends on bug 12802.

Recipients are defined by AutoEmailPrimaryAddress.
Comment 4 Owen Leonard 2021-11-03 11:33:24 UTC
I worry that too many of the available notices depend on logic or circumstances which aren't taken into account for here.

- ACCTDETAILS Account Details Template - DEFAULT -- This one is useful, although it's not good that our sample notice includes the encrypted password.

- MEMBERSHIP_EXPIRY Account expiration -- I can see this being useful as well.

- OPAC_REG_VERIFY Opac Self-Registration Verification Email -- This would be useful if it could be done, but can it? If you require email verification for self-registrations the patron record isn't created until verified. Meaning you couldn't find the patron record to re-send the verification.

- PASSWORD_RESET Online password reset -- Useful, but doesn't work. There is no link in the message.

- DISCHARGE Discharge -- This notice is usually generated as a PDF, not sent via email. Does an email work for the discharge process?

These notices don't make sense because there's not enough context for the message:

PROBLEM_REPORT OPAC Problem Report
SHARE_ACCEPT Notification about an accepted share
SHARE_INVITE Invitation for sharing a list
Comment 5 Donna 2021-11-03 12:45:24 UTC
I feel this would benefit from having a permission attached to it.  Many libraries will not want all staff to have this ability and will want an easy way to disable it, without blocking staff from being able to use the Add message functionality.
Comment 6 Alex Arnaud 2021-11-03 12:46:40 UTC
(In reply to Owen Leonard from comment #4)
> I worry that too many of the available notices depend on logic or
> circumstances which aren't taken into account for here.
> 

Hello Owen,

I would say that it is the responsibility of the user/librarian to use the 
appropriate letter. Is there any security/system/GDPR risk according to you ?

The goal here is to allow librarian to have some predefined letters to be used in the "Add message" feature. I choose "members" as letters category for this.
Would a new module be better ?

And thank you for testing.
Comment 7 Alex Arnaud 2021-11-08 14:16:44 UTC
Created attachment 127452 [details] [review]
Bug 29393 - Use patron language in letters
Comment 8 Alex Arnaud 2021-11-10 10:34:50 UTC
Created attachment 127506 [details] [review]
Bug 29393 - add a dedicated letters module for add message
Comment 9 Alex Arnaud 2021-11-10 13:45:49 UTC
Created attachment 127511 [details] [review]
Bug 29393 - Add send_messages_to_borrowers permission
Comment 10 Alex Arnaud 2021-11-19 09:00:29 UTC
Created attachment 127847 [details] [review]
Bug 29393 - Don't show notice templates for each language
Comment 11 Alex Arnaud 2021-12-03 14:00:44 UTC
Created attachment 128215 [details] [review]
Bug 29393 - Ability to send emails from patron details page

Test plan
  - apply Bug 12802
  - go to a patron's details page
  - click on "Add message" in tools bar
  - choose "Email" in "Add a message for"
  - Select a notice (module members) or manualy fill the message
  - Save
  - Check the message is saved in message_queue table

Sponsored-by: Aix-Marseille University
Comment 12 Alex Arnaud 2021-12-03 14:00:59 UTC
Created attachment 128216 [details] [review]
Bug 29393 - Use patron language in letters

Sponsored-by: Aix-Marseille University
Comment 13 Alex Arnaud 2021-12-03 14:01:09 UTC
Created attachment 128217 [details] [review]
Bug 29393 - add a dedicated letters module for add message

Sponsored-by: Aix-Marseille University
Comment 14 Alex Arnaud 2021-12-03 14:01:20 UTC
Created attachment 128218 [details] [review]
Bug 29393 - Add send_messages_to_borrowers permission

Sponsored-by: Aix-Marseille University
Comment 15 Alex Arnaud 2021-12-03 14:01:30 UTC
Created attachment 128219 [details] [review]
Bug 29393 - Don't show notice templates for each language

Sponsored-by: Aix-Marseille University
Comment 16 Alex Arnaud 2021-12-08 13:04:07 UTC
Created attachment 128363 [details] [review]
Bug 29393 - Sort notice templates by name

Sponsored-by: Aix-Marseille University
Comment 17 Rachael 2022-07-04 13:38:09 UTC
patch doesn't apply
Comment 18 Fridolin Somers 2023-03-14 20:42:15 UTC
Created attachment 148187 [details] [review]
Bug 29393: (follow-up) Explicite use of methods from C4
Comment 19 Fridolin Somers 2023-03-14 20:43:25 UTC
Created attachment 148188 [details] [review]
Bug 29393: Ability to send emails from patron details page

Test plan
  - apply Bug 12802
  - go to a patron's details page
  - click on "Add message" in tools bar
  - choose "Email" in "Add a message for"
  - Select a notice (module members) or manualy fill the message
  - Save
  - Check the message is saved in message_queue table
Comment 20 Fridolin Somers 2023-03-14 20:43:28 UTC
Created attachment 148189 [details] [review]
Bug 29393: Use patron language in letters
Comment 21 Fridolin Somers 2023-03-14 20:43:31 UTC
Created attachment 148190 [details] [review]
Bug 29393: Add a dedicated letters module for add message
Comment 22 Fridolin Somers 2023-03-14 20:43:34 UTC
Created attachment 148191 [details] [review]
Bug 29393: Add send_messages_to_borrowers permission
Comment 23 Fridolin Somers 2023-03-14 20:43:37 UTC
Created attachment 148192 [details] [review]
Bug 29393: Don't show notice templates for each language
Comment 24 Fridolin Somers 2023-03-14 20:43:40 UTC
Created attachment 148193 [details] [review]
Bug 29393: Sort notice templates by name
Comment 25 Fridolin Somers 2023-03-14 20:43:43 UTC
Created attachment 148194 [details] [review]
Bug 29393: (follow-up) Explicite use of methods from C4
Comment 26 Fridolin Somers 2023-03-14 20:45:22 UTC
rebased on current master
+ a small follow-up
Comment 27 Julian Maurice 2023-04-18 14:31:13 UTC
Created attachment 149812 [details] [review]
Bug 29393: Prevent SQL errors when SQL strict mode is set
Comment 28 Lucas Gass 2023-05-25 16:54:30 UTC
Merge conflicts in circ/add_message.pl.
Comment 29 Martin Renvoize 2023-12-20 12:37:33 UTC
Created attachment 160107 [details] [review]
Bug 29393: Ability to send emails from patron details page

Test plan
  - apply Bug 12802
  - go to a patron's details page
  - click on "Add message" in tools bar
  - choose "Email" in "Add a message for"
  - Select a notice (module members) or manualy fill the message
  - Save
  - Check the message is saved in message_queue table
Comment 30 Martin Renvoize 2023-12-20 12:37:37 UTC
Created attachment 160108 [details] [review]
Bug 29393: Use patron language in letters
Comment 31 Martin Renvoize 2023-12-20 12:37:40 UTC
Created attachment 160109 [details] [review]
Bug 29393: Add a dedicated letters module for add message
Comment 32 Martin Renvoize 2023-12-20 12:37:43 UTC
Created attachment 160110 [details] [review]
Bug 29393: Add send_messages_to_borrowers permission
Comment 33 Martin Renvoize 2023-12-20 12:37:47 UTC
Created attachment 160111 [details] [review]
Bug 29393: Don't show notice templates for each language
Comment 34 Martin Renvoize 2023-12-20 12:37:50 UTC
Created attachment 160112 [details] [review]
Bug 29393: Sort notice templates by name
Comment 35 Martin Renvoize 2023-12-20 12:37:53 UTC
Created attachment 160113 [details] [review]
Bug 29393: (follow-up) Explicite use of methods from C4
Comment 36 Martin Renvoize 2023-12-20 12:37:56 UTC
Created attachment 160114 [details] [review]
Bug 29393: Prevent SQL errors when SQL strict mode is set
Comment 37 Martin Renvoize 2023-12-20 12:39:42 UTC
Just a rebase for now..

Reviewing the code a bit next.
Comment 38 Martin Renvoize 2023-12-20 12:56:50 UTC
I'm interested in the idea's here.. but I'd love to see a specification or set or original requirements attached.

I fear we're ending up with a series of overlapping features.. we need someone to take a step back and look at the big picture.

We now have 'OPAC Problem reports', 'Catalog concerns (misnamed due to original requirements.. should really be 'request tracking'), Notices, 'OPAC Note' and checkout notes.
Comment 39 Martin Renvoize 2023-12-20 12:57:55 UTC
I feel like we could improve the notice template functionality, displaying a preview (maybe even editable) of the notice prior to sending.
Comment 40 Anneli Österman 2024-01-02 11:06:10 UTC
Some notes:

- If you add a new notice to Patrons (MEMBERS) category, they do not show up in Add message email drop down. Test plan does not apply in this case.
- If you add a new notice to "Add message" category, they show up in Add message email drop down. There is no mention of this category in the test plan.
- When you choose a notice to send, you cannot see it's content. You can see it only after it is saved to message queue.
Comment 41 Matthias Le Gac 2024-01-08 15:07:54 UTC
Created attachment 160633 [details] [review]
Bug 29393: Prevent SQL errors when SQL strict mode is set

Signed-off-by: matthias <matthias.le-gac@inlibro.net>
Comment 42 Matthias Le Gac 2024-01-08 15:11:11 UTC
Created attachment 160638 [details] [review]
Bug 29393: Prevent SQL errors when SQL strict mode is set

Signed-off-by: matthias <matthias.le-gac@inlibro.net>
Comment 43 Phan Tung Bui 2024-01-08 15:11:31 UTC
Created attachment 160639 [details] [review]
Bug 29393: Prevent SQL errors when SQL strict mode is set

Signed-off-by: Phan Tung Bui <phan-tung.bui@inlibro.com>
Comment 44 Martin Renvoize 2024-04-25 08:16:20 UTC
Created attachment 165498 [details] [review]
Bug 29393: Ability to send emails from patron details page

Test plan
  - apply Bug 12802
  - go to a patron's details page
  - click on "Add message" in tools bar
  - choose "Email" in "Add a message for"
  - Select a notice (module members) or manualy fill the message
  - Save
  - Check the message is saved in message_queue table
Comment 45 Martin Renvoize 2024-04-25 08:16:23 UTC
Created attachment 165499 [details] [review]
Bug 29393: Use patron language in letters
Comment 46 Martin Renvoize 2024-04-25 08:16:26 UTC
Created attachment 165500 [details] [review]
Bug 29393: Add a dedicated letters module for add message
Comment 47 Martin Renvoize 2024-04-25 08:16:29 UTC
Created attachment 165501 [details] [review]
Bug 29393: Add send_messages_to_borrowers permission
Comment 48 Martin Renvoize 2024-04-25 08:16:32 UTC
Created attachment 165502 [details] [review]
Bug 29393: Don't show notice templates for each language
Comment 49 Martin Renvoize 2024-04-25 08:16:35 UTC
Created attachment 165503 [details] [review]
Bug 29393: Sort notice templates by name
Comment 50 Martin Renvoize 2024-04-25 08:16:38 UTC
Created attachment 165504 [details] [review]
Bug 29393: (follow-up) Explicite use of methods from C4
Comment 51 Martin Renvoize 2024-04-25 08:16:41 UTC
Created attachment 165505 [details] [review]
Bug 29393: Prevent SQL errors when SQL strict mode is set

Signed-off-by: Phan Tung Bui <phan-tung.bui@inlibro.com>
Comment 52 Martin Renvoize 2024-04-25 08:18:36 UTC
Another rebase done.. still haven't had a chance to properly review and I still wonder if we have a bunch of conflicting coding going on.
Comment 53 Martin Renvoize 2024-04-25 09:40:18 UTC
I'm going to experiment a bit on bug 32436. I'm looking at introducing API routes to allow fetching the list of available notices in a particular 'module' and also to fetch the result of a GetPreparedLetter.  I think that would allow us to have a considerably nicer interface implementation here.

I'd also be interested in some refinements to enhance the modal to appear more like the 'Display ticket' or 'Add catalog concern' modals from that area as I think they're more appealing than the current proposal here.

That all said, I don't want to block this one, so it would be nice to find someone to QA it... I'll ask around.
Comment 54 Martin Renvoize 2024-04-25 11:01:01 UTC
Created attachment 165511 [details] [review]
Bug 29393: Ability to send emails from patron details page

Test plan
  - apply Bug 12802
  - go to a patron's details page
  - click on "Add message" in tools bar
  - choose "Email" in "Add a message for"
  - Select a notice (module members) or manualy fill the message
  - Save
  - Check the message is saved in message_queue table

Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2024-04-25 11:01:04 UTC
Created attachment 165512 [details] [review]
Bug 29393: Use patron language in letters

Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 56 Martin Renvoize 2024-04-25 11:01:07 UTC
Created attachment 165513 [details] [review]
Bug 29393: Add a dedicated letters module for add message

Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 57 Martin Renvoize 2024-04-25 11:01:11 UTC
Created attachment 165514 [details] [review]
Bug 29393: Add send_messages_to_borrowers permission

Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 58 Martin Renvoize 2024-04-25 11:01:14 UTC
Created attachment 165515 [details] [review]
Bug 29393: Don't show notice templates for each language

Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Martin Renvoize 2024-04-25 11:01:17 UTC
Created attachment 165516 [details] [review]
Bug 29393: Sort notice templates by name

Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 60 Martin Renvoize 2024-04-25 11:01:21 UTC
Created attachment 165517 [details] [review]
Bug 29393: (follow-up) Explicite use of methods from C4

Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 61 Martin Renvoize 2024-04-25 11:01:24 UTC
Created attachment 165518 [details] [review]
Bug 29393: Prevent SQL errors when SQL strict mode is set

Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Phan Tung Bui <phan-tung.bui@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 62 Martin Renvoize 2024-04-25 11:01:27 UTC
Created attachment 165519 [details] [review]
Bug 29393: (QA follow-up) Make database update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 63 Martin Renvoize 2024-04-25 11:01:31 UTC
Created attachment 165520 [details] [review]
Bug 29393: (QA follow-up) Tidy add_message.pl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 64 Martin Renvoize 2024-04-25 11:01:35 UTC
Created attachment 165521 [details] [review]
Bug 29393: (QA follow-up) Tidy and make executable the new test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 65 Martin Renvoize 2024-04-25 11:01:38 UTC
Created attachment 165522 [details] [review]
Bug 29393: (QA follow-up) Improve modal styling

The modal still used a lot of older styles, this patch updates the
classes to make it more consistent with other message type modals.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 66 Martin Renvoize 2024-04-25 11:02:52 UTC
So, whilst I think there's lots of improvements we can make here, I think this is good enough as is for a first push.

I'm going to submit bugs for the follow-ups I'd like to see later.

QA scripts are happy and the feature works reasonably well.

Tests passing

Passing QA
Comment 67 Katrin Fischer 2024-04-29 15:28:33 UTC
Please also look out for the small stuff ;)

1) Database update success message should be specific
+        say $out "Update is going well so far";

Fixed inline when moving the database update.

2) Name of new notice module

Updated from "Add message" to read "Patrons (custom message)".

We also discussed merging with "Patrons (custom slips)" making use of the different transport methods. 

NOTE: Please file bug :)
Comment 68 Katrin Fischer 2024-04-29 15:42:58 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 69 Katrin Fischer 2024-04-29 16:59:38 UTC
I need a follow-up here to fix the tests:
t_Koha_Auth_Permissions_t.superlibrarian tests (11)
Comment 70 Martin Renvoize 2024-05-01 06:31:52 UTC
Created attachment 165942 [details] [review]
Bug 29393: (QA follow-up) Add key to Permissions test

We missed adding the new superlibrarian key to the Permissions test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 71 Martin Renvoize 2024-05-01 12:04:57 UTC
Follow-up pushed to make Jenkins happy (it's a public holiday for the RM so I'm going in as RM backup today)
Comment 72 Katrin Fischer 2024-05-02 11:09:56 UTC
(In reply to Martin Renvoize from comment #71)
> Follow-up pushed to make Jenkins happy (it's a public holiday for the RM so
> I'm going in as RM backup today)

Switching back to 'Pushed to main' :)