Bug 32917

Summary: Change patron.firstname and patron.surname in password change sample notice
Product: Koha Reporter: Caroline Cyr La Rose <caroline.cyr-la-rose>
Component: NoticesAssignee: Katrin Fischer <katrin.fischer>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: david, dcook, jonathan.druart, lucas
Version: unspecified   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06
Circulation function:
Bug Depends on: 25936    
Bug Blocks:    
Attachments: Bug 32917: Fix sample notice for CHANGE_PASSWORD
Bug 32917: Fix sample notice for CHANGE_PASSWORD
Bug 32917: Fix sample notice for CHANGE_PASSWORD

Description Caroline Cyr La Rose 2023-02-08 20:47:12 UTC
Unless I am mistaken, I think the password change sample notice/notice should contain [% borrower.firstname %] and [% borrower.surname %] rather than [% patron.firstname %] and [% patron.surname %]

from installer/data/mysql/en/mandatory/sample_notices.yml

        - module: members
          code: PASSWORD_CHANGE
          branchcode: ""
          name: "Notification of password change"
          is_html: 1
          title: "Library account password change notification for [% patron.firstname %] [% patron.surname %]"
          message_transport_type: email
          lang: default
          content:
            - "Dear [% patron.firstname %] [% patron.surname %],"
            - ""
            - "We want to notify you that your password has been changed. If you did not change it yourself (or requested that change), please contact library staff."
            - ""
            - "Your library."
Comment 1 Caroline Cyr La Rose 2023-02-08 20:54:10 UTC
The translation files usually change the TT variables by %s in the original string

For example 

Dear [% borrower.firstname %] [% borrower.surname %]

becomes

Dear %s %s


But for this letter, the variable are written in long form

Library account password change notification for [% patron.firstname %] [% patron.surname %]
Comment 2 Katrin Fischer 2023-04-16 12:25:25 UTC
Created attachment 149729 [details] [review]
Bug 32917: Fix sample notice for CHANGE_PASSWORD

We've identified 3 issues:
* Name of patron is not showing up in subject line
* Name of patron is now showing in notice text
* Notice is set to be HTML, but has no line breaks and uses no HTML tags

In the database update, these issues don't occur:

$dbh->do( q{
    INSERT IGNORE INTO letter (module, code, name, title, content, message_transport_type) VALUES ('members', 'PASSWORD_CHANGE', 'Notification of password change', 'Library account password change notification',
    "Dear [% borrower.firstname %] [% borrower.surname %],\r\n\r\nWe want to notify you that your password has been changed. If you did not change it yourself (or requested that change), please contact library staff.\r\n\r\nYour library.", 'email');
});

So this patch will make them match up:

* Remove HTML flag
* Update patron. to borrower. in message text
* Remove patron name from subject (it won't work even with the correct variables)

To test:
* Add an email address to your favourite sample user
* Make sure NotifyPasswordChange is set to "Notify"
* Update the sample user's password using the "change password" feature
* Verify a notice is shown in the notices tabs
  Note: you won't see the line break issue there because of bug 30287
* Apply patch
* Recreate your database/run web installer so sample notices are loaded
* Repeat the test, it should generate a nice lookig notice now
Comment 3 David Nind 2023-04-22 23:42:31 UTC
Created attachment 150111 [details] [review]
Bug 32917: Fix sample notice for CHANGE_PASSWORD

We've identified 3 issues:
* Name of patron is not showing up in subject line
* Name of patron is now showing in notice text
* Notice is set to be HTML, but has no line breaks and uses no HTML tags

In the database update, these issues don't occur:

$dbh->do( q{
    INSERT IGNORE INTO letter (module, code, name, title, content, message_transport_type) VALUES ('members', 'PASSWORD_CHANGE', 'Notification of password change', 'Library account password change notification',
    "Dear [% borrower.firstname %] [% borrower.surname %],\r\n\r\nWe want to notify you that your password has been changed. If you did not change it yourself (or requested that change), please contact library staff.\r\n\r\nYour library.", 'email');
});

So this patch will make them match up:

* Remove HTML flag
* Update patron. to borrower. in message text
* Remove patron name from subject (it won't work even with the correct variables)

To test:
* Add an email address to your favourite sample user
* Make sure NotifyPasswordChange is set to "Notify"
* Update the sample user's password using the "change password" feature
* Verify a notice is shown in the notices tabs
  Note: you won't see the line break issue there because of bug 30287
* Apply patch
* Recreate your database/run web installer so sample notices are loaded
* Repeat the test, it should generate a nice lookig notice now

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2023-04-22 23:46:35 UTC
Testing notes (using KTD):

1. After applying the patch, a reset_all loads the updated notice:
  - remember to add email address again to patron used to test

2. I also tested by actually sending the email[1] - this showed that the formatting was fixed (line breaks now show where expected, so it is now nicely formatted).

[1] To test sending emails using a Google account, edit /etc/koha/sites/kohadev/koha-conf.xml file and add this configuration near the end (where <user_name> = your Google email address; <password> = your APP password, not your Google account password):

 <smtp_server>
    <host>smtp.gmail.com</host>
    <port>587</port>
    <timeout>5</timeout>
    <ssl_mode>STARTTLS</ssl_mode>
    <user_name>GOOGLEACCOUNTUSER</user_name>
    <password>GOOGLEAPPPASSWORD</password>
    <debug>1</debug>
 </smtp_server>
Comment 5 Jonathan Druart 2023-04-25 14:16:37 UTC
Created attachment 150207 [details] [review]
Bug 32917: Fix sample notice for CHANGE_PASSWORD

We've identified 3 issues:
* Name of patron is not showing up in subject line
* Name of patron is now showing in notice text
* Notice is set to be HTML, but has no line breaks and uses no HTML tags

In the database update, these issues don't occur:

$dbh->do( q{
    INSERT IGNORE INTO letter (module, code, name, title, content, message_transport_type) VALUES ('members', 'PASSWORD_CHANGE', 'Notification of password change', 'Library account password change notification',
    "Dear [% borrower.firstname %] [% borrower.surname %],\r\n\r\nWe want to notify you that your password has been changed. If you did not change it yourself (or requested that change), please contact library staff.\r\n\r\nYour library.", 'email');
});

So this patch will make them match up:

* Remove HTML flag
* Update patron. to borrower. in message text
* Remove patron name from subject (it won't work even with the correct variables)

To test:
* Add an email address to your favourite sample user
* Make sure NotifyPasswordChange is set to "Notify"
* Update the sample user's password using the "change password" feature
* Verify a notice is shown in the notices tabs
  Note: you won't see the line break issue there because of bug 30287
* Apply patch
* Recreate your database/run web installer so sample notices are loaded
* Repeat the test, it should generate a nice lookig notice now

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Tomás Cohen Arazi (tcohen) 2023-05-09 14:47:53 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 7 Matt Blenkinsop 2023-05-15 12:21:16 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 8 Lucas Gass (lukeg) 2023-06-15 18:59:09 UTC
Missing dependencies for 22.05.x, no backport