Created attachment 170840 [details] [review] Bug 37757: Unit tests Created attachment 170841 [details] [review] Bug 37757: Update EmailFieldPrimary value Created attachment 170842 [details] [review] Bug 37757: Make notice_email_address() handle EmailFieldPrimary values more robustly This patchset makes: * The EmailFieldPrimary value 'OFF' be replaced by an empty string * Koha::Patron->notice_email_address() handle the empty case more gracefully To test: 1. On main, manually set the EmailFieldPrimary syspref to empty $ ktd --shell k$ koha-mysql kohadev > UPDATE systempreferences SET value='' WHERE variable='EmailFieldPrimary'; 2. Enable `AutoEmailNewUser` 3. Create a new user => FAIL: Koha explodes like: ``` The method Koha::Patron-> is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Object.pm line 992 Koha::Object::AUTOLOAD('Koha::Patron=HASH(0xaaaae37ac550)') called at /kohadevbox/koha/Koha/Patron.pm line 1662 Koha::Patron::notice_email_address('Koha::Patron=HASH(0xaaaae37ac550)') called at /kohadevbox/koha/members/memberentry.pl line 448 ... 4. Apply this patches 5. Run: k$ updatedatabase => SUCCESS: Update succeeds 6. Restart all and retry creating a patron => SUCCESS: It worked! 7. Sign off :-D Created attachment 170843 [details] [review] Bug 37757: (follow-up) Warn on bad values and survive Created attachment 171160 [details] [review] Bug 37757: Unit tests Signed-off-by: David Nind <david@davidnind.com> Created attachment 171161 [details] [review] Bug 37757: Update EmailFieldPrimary value Signed-off-by: David Nind <david@davidnind.com> Created attachment 171162 [details] [review] Bug 37757: Make notice_email_address() handle EmailFieldPrimary values more robustly This patchset makes: * The EmailFieldPrimary value 'OFF' be replaced by an empty string * Koha::Patron->notice_email_address() handle the empty case more gracefully To test: 1. On main, manually set the EmailFieldPrimary syspref to empty $ ktd --shell k$ koha-mysql kohadev > UPDATE systempreferences SET value='' WHERE variable='EmailFieldPrimary'; 2. Enable `AutoEmailNewUser` 3. Create a new user => FAIL: Koha explodes like: ``` The method Koha::Patron-> is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Object.pm line 992 Koha::Object::AUTOLOAD('Koha::Patron=HASH(0xaaaae37ac550)') called at /kohadevbox/koha/Koha/Patron.pm line 1662 Koha::Patron::notice_email_address('Koha::Patron=HASH(0xaaaae37ac550)') called at /kohadevbox/koha/members/memberentry.pl line 448 ... 4. Apply this patches 5. Run: k$ updatedatabase => SUCCESS: Update succeeds 6. Restart all and retry creating a patron => SUCCESS: It worked! 7. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Created attachment 171163 [details] [review] Bug 37757: (follow-up) Warn on bad values and survive Signed-off-by: David Nind <david@davidnind.com> Created attachment 171447 [details] [review] Bug 37757: Unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 171448 [details] [review] Bug 37757: Update EmailFieldPrimary value Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 171449 [details] [review] Bug 37757: Make notice_email_address() handle EmailFieldPrimary values more robustly This patchset makes: * The EmailFieldPrimary value 'OFF' be replaced by an empty string * Koha::Patron->notice_email_address() handle the empty case more gracefully To test: 1. On main, manually set the EmailFieldPrimary syspref to empty $ ktd --shell k$ koha-mysql kohadev > UPDATE systempreferences SET value='' WHERE variable='EmailFieldPrimary'; 2. Enable `AutoEmailNewUser` 3. Create a new user => FAIL: Koha explodes like: ``` The method Koha::Patron-> is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Object.pm line 992 Koha::Object::AUTOLOAD('Koha::Patron=HASH(0xaaaae37ac550)') called at /kohadevbox/koha/Koha/Patron.pm line 1662 Koha::Patron::notice_email_address('Koha::Patron=HASH(0xaaaae37ac550)') called at /kohadevbox/koha/members/memberentry.pl line 448 ... 4. Apply this patches 5. Run: k$ updatedatabase => SUCCESS: Update succeeds 6. Restart all and retry creating a patron => SUCCESS: It worked! 7. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 171450 [details] [review] Bug 37757: (follow-up) Warn on bad values and survive Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Good spot, all working as described and unit tested. Passing QA Pushed for 24.11! Well done everyone, thank you! It looks like this change broke tests, please fix! # Failed test 'SendQueuedMessages fails when no to_address, patron notice email and RedirectGuaranteeEmail is not set' 17:11:40 koha_1 | # at t/db_dependent/Letters.t line 1158. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: FAIL: No 'to_address', email address or guarantors email address for borrowernumber (1606) at /kohadevbox/koha/C4/Letters.pm line 1394. 17:11:40 koha_1 | # expected to find warning: (?^u:No 'to_address', email address or guarantors email address for borrowernumber) 17:11:40 koha_1 | 17:11:40 koha_1 | # Failed test 'SendQueuedMessages is using the mocked send_or_die routine' 17:11:40 koha_1 | # at t/db_dependent/Letters.t line 1169. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:11:40 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:11:40 koha_1 | 17:11:40 koha_1 | # Failed test 'SendQueuedMessages is using the mocked send_or_die routine' 17:11:40 koha_1 | # at t/db_dependent/Letters.t line 1201. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:11:40 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:11:40 koha_1 | 17:11:40 koha_1 | # Failed test 'SendQueuedMessages is using the mocked send_or_die routine' 17:11:40 koha_1 | # at t/db_dependent/Letters.t line 1233. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:11:40 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:13:26 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:13:26 koha_1 | # Looks like you failed 4 tests of 19. 17:13:26 koha_1 | 17:13:26 koha_1 | # Failed test 'Test guarantor handling in SendQueuedMessages' 17:13:26 koha_1 | # at t/db_dependent/Letters.t line 1262. 17:13:26 koha_1 | 17:13:26 koha_1 | # Failed test 'SendQueuedMessages with limit 1' 17:13:26 koha_1 | # at t/db_dependent/Letters.t line 1402. 17:13:26 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:13:26 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:13:26 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:13:26 koha_1 | 17:13:26 koha_1 | # Failed test 'SendQueuedMessages with limit 2' 17:13:26 koha_1 | # at t/db_dependent/Letters.t line 1409. 17:13:26 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:13:26 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:13:26 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:13:26 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:13:26 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:13:26 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:13:26 koha_1 | 17:13:26 koha_1 | # Failed test 'SendQueuedMessages with limit 3' 17:13:26 koha_1 | # at t/db_dependent/Letters.t line 1416. 17:13:26 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:13:26 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:13:26 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:13:26 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:13:26 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:13:26 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:13:26 koha_1 | 17:13:26 koha_1 | # Failed test 'SendQueuedMessages with limit 2 and domain limits' 17:13:26 koha_1 | # at t/db_dependent/Letters.t line 1447. 17:13:26 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:13:26 koha_1 | # found warning: Sending messages: domain wrong.net reached limit of 6/1m at /kohadevbox/koha/Koha/Notice/Util.pm line 97. 17:13:26 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:13:26 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:13:26 koha_1 | # found warning: Sending messages: domain fake1.domain reached limit of 1/1m at /kohadevbox/koha/Koha/Notice/Util.pm line 97. 17:13:26 koha_1 | # found warning: Fake send_or_die at t/db_dependent/Letters.t line 45. 17:13:26 koha_1 | # expected to find warning: (?^u:wrong.net reached limit) 17:13:26 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:13:26 koha_1 | # expected to find warning: (?^u:fake1.domain reached limit) 17:13:26 koha_1 | # expected to find warning: (?^u:Fake send_or_die) 17:13:26 koha_1 | # Looks like you failed 4 tests of 18. 17:13:26 koha_1 | 17:13:26 koha_1 | # Failed test 'Test limit parameter for SendQueuedMessages' 17:13:26 koha_1 | # at t/db_dependent/Letters.t line 1452. 17:13:26 koha_1 | # Looks like you failed 2 tests of 103. 17:13:26 koha_1 | [15:11:39] t/db_dependent/Letters.t .................................................... 17:13:26 koha_1 | Dubious, test returned 2 (wstat 512, 0x200) 17:13:26 koha_1 | Failed 2/103 subtests This one too: 17:05:38 koha_1 | # Failed test 'WELCOME notice Prepared, Enqueued and Send' 17:05:38 koha_1 | # at t/db_dependent/Auth_with_shibboleth.t line 233. 17:05:38 koha_1 | # found warning: Invalid value for EmailFieldPrimary (OFF) at /kohadevbox/koha/Koha/Patron.pm line 1645. 17:05:38 koha_1 | # found warning: GetPreparedLetter called at t/db_dependent/Auth_with_shibboleth.t line 62. 17:05:38 koha_1 | # found warning: EnqueueLetter called at t/db_dependent/Auth_with_shibboleth.t line 66. 17:05:38 koha_1 | # found warning: SendQueuedMessages called with message_id: 42 at t/db_dependent/Auth_with_shibboleth.t line 72. 17:05:38 koha_1 | # expected to find warning: GetPreparedLetter called 17:05:38 koha_1 | # expected to find warning: EnqueueLetter called 17:05:38 koha_1 | # expected to find warning: SendQueuedMessages called with message_id: 42 17:05:38 koha_1 | # Looks like you failed 1 test of 52. 17:05:38 koha_1 | 17:05:38 koha_1 | # Failed test 'checkpw_shib tests' 17:05:38 koha_1 | # at t/db_dependent/Auth_with_shibboleth.t line 291. Created attachment 171496 [details] [review] Bug 37757: (follow-up) Adjust existing tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Follow-up pushed to master. I think we might need to revisit koha-misc4dev's default configuration. Backported to 24.05.x for upcoming 24.05.06 |
Found this in the wild on `22.11`, from the AutoEmailPrimaryAddress era. Which had the syspref set to an empty string. And was able to reproduce on `main`. To reproduce: 1. Have EmailFieldPrimary set to an empty string or NULL $ ktd --shell k$ koha-mysql kohadev > UPDATE systempreferences SET value='' WHERE variable='EmailFieldPrimary'; 2. Enable `AutoEmailNewUser` 3. Create a new user => FAIL: Koha explodes like: ``` The method Koha::Patron-> is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Object.pm line 992 Koha::Object::AUTOLOAD('Koha::Patron=HASH(0xaaaae37ac550)') called at /kohadevbox/koha/Koha/Patron.pm line 1662 Koha::Patron::notice_email_address('Koha::Patron=HASH(0xaaaae37ac550)') called at /kohadevbox/koha/members/memberentry.pl line 448 ... ```