Bug 37216 - Fix dbrev for EmailFieldSelection
Summary: Fix dbrev for EmailFieldSelection
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Emily Lamancusa (emlam)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords: rel_24_05_candidate, RM_priority
Depends on: 12802
Blocks:
  Show dependency treegraph
 
Reported: 2024-06-27 20:24 UTC by Emily Lamancusa (emlam)
Modified: 2024-09-10 18:27 UTC (History)
6 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:
24.11.00,24.05.04
Circulation function:


Attachments
Bug 37216: Fix SQL for EmailFieldSelection (4.01 KB, patch)
2024-06-27 20:38 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 37216: Fix SQL for EmailFieldSelection (4.06 KB, patch)
2024-06-28 10:03 UTC, David Nind
Details | Diff | Splinter Review
Bug 37216: Fix SQL for EmailFieldSelection (4.12 KB, patch)
2024-06-28 15:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37216: (QA follow-up) Add update to set existing options (1.30 KB, patch)
2024-06-28 15:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37216: Fix SQL for EmailFieldSelection (4.12 KB, patch)
2024-06-28 15:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37216: (QA follow-up) Add update to set existing options (1.30 KB, patch)
2024-06-28 15:04 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37216: (follow-up) Clear invalid value (1.23 KB, patch)
2024-06-28 21:04 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Emily Lamancusa (emlam) 2024-06-27 20:24:44 UTC
The values for the "value" column and the "options" column are swapped in the SQL that added the EmailFieldSelection syspref (for both upgrades and new installations). I'm not sure if this causes any problems except in the situation where the user sets EmailFieldPrimary to MULTIPLE but doesn't set EmailFieldSelection (which doesn't make sense to do), but we should fix it anyway.
Comment 1 Emily Lamancusa (emlam) 2024-06-27 20:38:49 UTC
Created attachment 168218 [details] [review]
Bug 37216: Fix SQL for EmailFieldSelection

Test plan:
Part A: New installation
1. Start a fresh test instance
2. Set EmailFieldPrimary to "selected addresses", and do not touch
    EmailFieldSelection
3. Edit a patron to ensure the following fields are set:
   - Primary email
   - Secondary email
   - Alternate email
   - Enable email notices for item checkout
4. Attempt to check an item out to that patron
--> Koha explodes!
5. Apply patch
6. reset_all
7. Repeat steps 2-4
--> Checkout succeeds!
8. Ensure test plan for bug 12802 still passes

Part B: Upgraded installation
1. Start a fresh test instance at version 23.11
2. Switch to main
3. Install database update
4. Set EmailFieldPrimary to "selected addresses", and do not touch
    EmailFieldSelection
5. Edit a patron to ensure the following fields are set:
   - Primary email
   - Secondary email
   - Alternate email
   - Enable email notices for item checkout
6. Attempt to check an item out to that patron
--> Koha explodes!
7. Go back to 23.11 and reset_all
8. Switch to main and apply patch
9. Repeat steps 4-6
--> Checkout succeeds!
10. Ensure test plan for bug 12802 still passes
Comment 2 David Nind 2024-06-28 10:03:53 UTC
Created attachment 168230 [details] [review]
Bug 37216: Fix SQL for EmailFieldSelection

Test plan:
Part A: New installation
1. Start a fresh test instance
2. Set EmailFieldPrimary to "selected addresses", and do not touch
    EmailFieldSelection
3. Edit a patron to ensure the following fields are set:
   - Primary email
   - Secondary email
   - Alternate email
   - Enable email notices for item checkout
4. Attempt to check an item out to that patron
--> Koha explodes!
5. Apply patch
6. reset_all
7. Repeat steps 2-4
--> Checkout succeeds!
8. Ensure test plan for bug 12802 still passes

Part B: Upgraded installation
1. Start a fresh test instance at version 23.11
2. Switch to main
3. Install database update
4. Set EmailFieldPrimary to "selected addresses", and do not touch
    EmailFieldSelection
5. Edit a patron to ensure the following fields are set:
   - Primary email
   - Secondary email
   - Alternate email
   - Enable email notices for item checkout
6. Attempt to check an item out to that patron
--> Koha explodes!
7. Go back to 23.11 and reset_all
8. Switch to main and apply patch
9. Repeat steps 4-6
--> Checkout succeeds!
10. Ensure test plan for bug 12802 still passes

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2024-06-28 10:11:56 UTC
Testing notes (using KTD):

1. For Part B step 3, I also ran a yarn build and restart_all.
2. For Part B step 9, I repeated steps 3-6 (I think you need to run the database updates?), I also ran a yarn build and restart_all.
3. For step 10, see the consolidated test plan below for bug 12802 - Send notices using several email addresses.

12802 - Test plan
~~~~~~~~~~~~~~~~~

This enhancement (added in 24.05) allows libraries to choose which patron email address(es) to use when sending notices.

To use, select the email address(es) for sending notices using the EmailFieldPrimary system preference. If the "selected addresses" option is chosen, then use EmailFieldSelection to choose which email address(es) to use - you can select one or more addresses (alternate, primary, secondary, or all email addresses).

1. Go to Koha Administration > System preferences. Search for EmailFieldPrimary. Confirm that you can only select one option at a time.
2. Search for a patron. Put different email addresses in each of the possible email fields (primary email, secondary email, alternate email).
   TIP: If you only have one Gmail address, put the dot in a different position and check the to field when you receive the email. For example, my.email@gmail.com, m.yemail@gmail.com, and mye.mail@gmail.com.
3. Repeat steps 4-6 for each of these options to confirm that the correct email address(es) are used for the email message:
   3.1 EmailFieldPrimary = alternate email
   3.2 EmailFieldPrimary = card number (change the patron's card number to a valid email address)
   3.3 EmailFieldPrimary = primary email
   3.4 EmailFieldPrimary = secondary email
   3.5 EmailFieldPrimary = selected addresses
       with EmailFieldSelection = all, then individually - alternate email, primary email, secondary email
   3.6 EmailFieldPrimary = first valid email address
       For testing, remove the primary email address - email address used should be the secondary email
       (This uses the EmailFieldPrecedence system preference, and uses the order specified and sends to the first valid email address - default order checked is email|emailpro|B_email, where email =  the primary email address, emailpro = the secondary email address, B_email = the alternate email address)
4. Send the welcome message for the patron: go to your patron > More > Send a welcome email
5. Check that the correct address(es) are shown in the database for the option you are testing:
   5.1 Access the database: sudo koha-mysql kohadev
   5.2 Show the messages in the queue: select * from message_queue;
   5.3 Confirm that correct address(es) for the option selected are shown in the to_address field (multiple address will be separated by a comma)
6. Set up KTD so that it can actually send email [1] (without email configured you will not be able to confirm the delivery to multiple addresses). 
7. Run misc/cronjobs/process_message_queue.pl to trigger sending the queued notices from steps 3-6.
8. Check that the addresses in the to_address all actually receive the notice for each of the messages generated for steps 3 to 5.
   TIP: The notices section for the patron now shows the to addresses in the delivery column.

[1] To test sending emails using a Google account:
- Set up an App password for your Google Account
- Configure a new SMTP Server under Administration > SMTP servers
  using the following settings where 'User name' = your Google email
  address and 'Password' = your APP password, not your Google account
  password):
   Host: smtp.gmail.com
   Port: 587
   Timeout: 120
   SSL: STARTTLS
   User name: GOOGLEACCOUNTUSER
   Password: GOOGLEAPPPASSWORD
- Set this server as default
Comment 4 Katrin Fischer 2024-06-28 13:15:33 UTC
Emily, do you think we can do a database update to at least fix the options list? And maybe we could check the value for a false value and correct it for that case?
Comment 5 Martin Renvoize (ashimema) 2024-06-28 15:03:42 UTC
Created attachment 168253 [details] [review]
Bug 37216: Fix SQL for EmailFieldSelection

Test plan:
Part A: New installation
1. Start a fresh test instance
2. Set EmailFieldPrimary to "selected addresses", and do not touch
    EmailFieldSelection
3. Edit a patron to ensure the following fields are set:
   - Primary email
   - Secondary email
   - Alternate email
   - Enable email notices for item checkout
4. Attempt to check an item out to that patron
--> Koha explodes!
5. Apply patch
6. reset_all
7. Repeat steps 2-4
--> Checkout succeeds!
8. Ensure test plan for bug 12802 still passes

Part B: Upgraded installation
1. Start a fresh test instance at version 23.11
2. Switch to main
3. Install database update
4. Set EmailFieldPrimary to "selected addresses", and do not touch
    EmailFieldSelection
5. Edit a patron to ensure the following fields are set:
   - Primary email
   - Secondary email
   - Alternate email
   - Enable email notices for item checkout
6. Attempt to check an item out to that patron
--> Koha explodes!
7. Go back to 23.11 and reset_all
8. Switch to main and apply patch
9. Repeat steps 4-6
--> Checkout succeeds!
10. Ensure test plan for bug 12802 still passes

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2024-06-28 15:03:44 UTC
Created attachment 168254 [details] [review]
Bug 37216: (QA follow-up) Add update to set existing options

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2024-06-28 15:04:24 UTC
Created attachment 168255 [details] [review]
Bug 37216: Fix SQL for EmailFieldSelection

Test plan:
Part A: New installation
1. Start a fresh test instance
2. Set EmailFieldPrimary to "selected addresses", and do not touch
    EmailFieldSelection
3. Edit a patron to ensure the following fields are set:
   - Primary email
   - Secondary email
   - Alternate email
   - Enable email notices for item checkout
4. Attempt to check an item out to that patron
--> Koha explodes!
5. Apply patch
6. reset_all
7. Repeat steps 2-4
--> Checkout succeeds!
8. Ensure test plan for bug 12802 still passes

Part B: Upgraded installation
1. Start a fresh test instance at version 23.11
2. Switch to main
3. Install database update
4. Set EmailFieldPrimary to "selected addresses", and do not touch
    EmailFieldSelection
5. Edit a patron to ensure the following fields are set:
   - Primary email
   - Secondary email
   - Alternate email
   - Enable email notices for item checkout
6. Attempt to check an item out to that patron
--> Koha explodes!
7. Go back to 23.11 and reset_all
8. Switch to main and apply patch
9. Repeat steps 4-6
--> Checkout succeeds!
10. Ensure test plan for bug 12802 still passes

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize (ashimema) 2024-06-28 15:04:27 UTC
Created attachment 168256 [details] [review]
Bug 37216: (QA follow-up) Add update to set existing options

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Emily Lamancusa (emlam) 2024-06-28 20:53:28 UTC
Thanks, all, for the thorough testing and follow-up!

I think it's safe to clear the value field if it still contains 'email|emailpro|B_email' too, since that isn't a valid value for that field to have. (If the user actually selects all three, the value will be comma-delimited, not pipe-delimited--that's what crashes the notice generation in the first place.) Adding one more follow-up for that...
Comment 10 Emily Lamancusa (emlam) 2024-06-28 21:04:37 UTC
Created attachment 168298 [details] [review]
Bug 37216: (follow-up) Clear invalid value
Comment 11 Katrin Fischer 2024-07-01 16:25:50 UTC
Thanks for the fix!
Comment 12 Katrin Fischer 2024-07-01 16:59:02 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 13 Tomás Cohen Arazi (tcohen) 2024-08-28 19:00:38 UTC Comment hidden (obsolete)
Comment 14 Lucas Gass (lukeg) 2024-08-28 19:28:16 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 15 Fridolin Somers 2024-09-10 08:12:04 UTC
Depends on Bug 12802 not in 23.11.x
Comment 16 Caroline Cyr La Rose 2024-09-10 18:27:41 UTC
This looks like a bug fix, nothing to change/edit in the manual. Please reopen with more details if needed.