Bug 32917 - Change patron.firstname and patron.surname in password change sample notice
Summary: Change patron.firstname and patron.surname in password change sample notice
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on: 25936
Blocks:
  Show dependency treegraph
 
Reported: 2023-02-08 20:47 UTC by Caroline Cyr La Rose
Modified: 2024-07-04 20:37 UTC (History)
4 users (show)

See Also:
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:


Attachments
Bug 32917: Fix sample notice for CHANGE_PASSWORD (2.72 KB, patch)
2023-04-16 12:25 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32917: Fix sample notice for CHANGE_PASSWORD (2.77 KB, patch)
2023-04-22 23:42 UTC, David Nind
Details | Diff | Splinter Review
Bug 32917: Fix sample notice for CHANGE_PASSWORD (2.85 KB, patch)
2023-04-25 14:16 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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