[Bug 27860] New: Bad KohaAdminEmailAddress breaks patron self registration form until notice is removed from message queue
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Bug ID: 27860 Summary: Bad KohaAdminEmailAddress breaks patron self registration form until notice is removed from message queue Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: katrin.fischer@bsz-bw.de QA Contact: testopia@bugs.koha-community.org When the patron self registration form is used with email verification and a bad entry in the KohaAdminEmailAddress system preference an error will be shown, but the message is still created and added to the message_queue table. Now every time a patron tries to register the system tries to send the already generated message with the wrong from address, which fails. This means the error will remain, although the pref has been corrected meanwhile. Settings: - PatronSelfRegistrationVerifyByEmail = Require - KohaAdminEmailAdress like koha@localhost - PatronSelfRegistration activated -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The fix is to retrieve message_id from EnqueueLetter then pass it to SendQueueMessages _get_unsent_messages needs adjustments to get message_id as parameter. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This might also affect the password reset feature as it uses the same mechanism for sending the email without delay. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Bad KohaAdminEmailAddress |Bad KohaAdminEmailAddress |breaks patron self |breaks patron self |registration form until |registration and password |notice is removed from |reset feature |message queue | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Severity|minor |major Assignee|oleonard@myacpl.org |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 117796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117796&action=edit Bug 27860: Add message_id param to SendQueuedMessages We could use C4::Letter::GetMessage, but the query in _get_unsent_messages join on borrowers and return the branchcode. To prevent any regressions it's preferable to not modify SendQueuedMessages. Ideally we obviously need a Koha methods to have better and clean code.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 117797 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117797&action=edit Bug 27860: Fix self-registration if messages with incorrect email are inqueued If the pref KohaAdminEmailAddress was not set correctly (invalid email) and has been used to generate message, the message_queue table can contain some messages that are not sent (pending) and will be processed each time a new self-reg is done. The PatronSelfRegistrationVerifyByEmail feature must send only the notice we just generated, not the whole pending queue. Test plan: 1. Do not apply the patches 2. Set KohaAdminEmailAddress to an invalid email (root@localhost for instance) 3. Turn on PatronSelfRegistrationVerifyByEmail 4. Self-reg a patron 5. Boom (that must be fixed on a separate bug report) 6. Set KohaAdminEmailAddress to a valid email (root@example.org) 7. Self-reg a patron 8. Still Boom! (because it's processing the first invalid email) 9. Apply the patches, restart_all 10. Self-reg a patron => The email is sent! Notice the change in DB, the first email is still there with status "pending" and second email is sent! QA: at step 10. TODO first email must be 'failed' (highlight in tests) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 117798 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117798&action=edit Bug 27860: Fix password recovery Same as the previous patch but for the password recovery feature. Test plan: 1. Do not apply the patches 2. Set KohaAdminEmailAddress to an invalid email (root@localhost for instance) 3. Use the password recovery feature 5. Boom (that must be fixed on a separate bug report) 6. Set KohaAdminEmailAddress to a valid email (root@example.org) 7. Use again the password recovery for a different user 8. Still Boom! (because it's processing the first invalid email) 9. Apply the patches, restart_all 10. Try again the password recovery => The email is sent! Notice the change in DB, the first email is still there with status "pending" and second email is sent! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Should we also perhaps add a warning to the about page for queued messages with bad from addresses? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117796|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117931&action=edit Bug 27860: Add message_id param to SendQueuedMessages We could use C4::Letter::GetMessage, but the query in _get_unsent_messages join on borrowers and return the branchcode. To prevent any regressions it's preferable to not modify SendQueuedMessages. Ideally we obviously need a Koha methods to have better and clean code.. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117797|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117932 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117932&action=edit Bug 27860: Fix self-registration if messages with incorrect email are inqueued If the pref KohaAdminEmailAddress was not set correctly (invalid email) and has been used to generate message, the message_queue table can contain some messages that are not sent (pending) and will be processed each time a new self-reg is done. The PatronSelfRegistrationVerifyByEmail feature must send only the notice we just generated, not the whole pending queue. Test plan: 1. Do not apply the patches 2. Set KohaAdminEmailAddress to an invalid email (root@localhost for instance) 3. Turn on PatronSelfRegistrationVerifyByEmail 4. Self-reg a patron 5. Boom (that must be fixed on a separate bug report) 6. Set KohaAdminEmailAddress to a valid email (root@example.org) 7. Self-reg a patron 8. Still Boom! (because it's processing the first invalid email) 9. Apply the patches, restart_all 10. Self-reg a patron => The email is sent! Notice the change in DB, the first email is still there with status "pending" and second email is sent! QA: at step 10. TODO first email must be 'failed' (highlight in tests) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117798|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117933 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117933&action=edit Bug 27860: Fix password recovery Same as the previous patch but for the password recovery feature. Test plan: 1. Do not apply the patches 2. Set KohaAdminEmailAddress to an invalid email (root@localhost for instance) 3. Use the password recovery feature 5. Boom (that must be fixed on a separate bug report) 6. Set KohaAdminEmailAddress to a valid email (root@example.org) 7. Use again the password recovery for a different user 8. Still Boom! (because it's processing the first invalid email) 9. Apply the patches, restart_all 10. Try again the password recovery => The email is sent! Notice the change in DB, the first email is still there with status "pending" and second email is sent! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Works as expected.. adding a warning on the about page is a 'nice to have'. Signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Martin Renvoize from comment #10)
Works as expected.. adding a warning on the about page is a 'nice to have'.
We have that already, it's enough IMO: commit 8850b7cf68dada380b8d7e5a8f6c83b1da76ebff Bug 22343: Add warning to the about page if KohaAdminEmailAddress is invalid -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #11)
(In reply to Martin Renvoize from comment #10)
Works as expected.. adding a warning on the about page is a 'nice to have'.
We have that already, it's enough IMO: commit 8850b7cf68dada380b8d7e5a8f6c83b1da76ebff Bug 22343: Add warning to the about page if KohaAdminEmailAddress is invalid
It doesn't warn on existing data within the queue.. but actually, I think perhaps that's an excessive for the user really. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117931|0 |1 is obsolete| | Attachment #117932|0 |1 is obsolete| | Attachment #117933|0 |1 is obsolete| | --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 118201 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118201&action=edit Bug 27860: Add message_id param to SendQueuedMessages We could use C4::Letter::GetMessage, but the query in _get_unsent_messages join on borrowers and return the branchcode. To prevent any regressions it's preferable to not modify SendQueuedMessages. Ideally we obviously need a Koha methods to have better and clean code.. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 118202 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118202&action=edit Bug 27860: Fix self-registration if messages with incorrect email are inqueued If the pref KohaAdminEmailAddress was not set correctly (invalid email) and has been used to generate message, the message_queue table can contain some messages that are not sent (pending) and will be processed each time a new self-reg is done. The PatronSelfRegistrationVerifyByEmail feature must send only the notice we just generated, not the whole pending queue. Test plan: 1. Do not apply the patches 2. Set KohaAdminEmailAddress to an invalid email (root@localhost for instance) 3. Turn on PatronSelfRegistrationVerifyByEmail 4. Self-reg a patron 5. Boom (that must be fixed on a separate bug report) 6. Set KohaAdminEmailAddress to a valid email (root@example.org) 7. Self-reg a patron 8. Still Boom! (because it's processing the first invalid email) 9. Apply the patches, restart_all 10. Self-reg a patron => The email is sent! Notice the change in DB, the first email is still there with status "pending" and second email is sent! QA: at step 10. TODO first email must be 'failed' (highlight in tests) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 118203 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118203&action=edit Bug 27860: Fix password recovery Same as the previous patch but for the password recovery feature. Test plan: 1. Do not apply the patches 2. Set KohaAdminEmailAddress to an invalid email (root@localhost for instance) 3. Use the password recovery feature 5. Boom (that must be fixed on a separate bug report) 6. Set KohaAdminEmailAddress to a valid email (root@example.org) 7. Use again the password recovery for a different user 8. Still Boom! (because it's processing the first invalid email) 9. Apply the patches, restart_all 10. Try again the password recovery => The email is sent! Notice the change in DB, the first email is still there with status "pending" and second email is sent! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)|21.05.00 |21.05.00,20.11.04 released in| | Status|Pushed to master |Pushed to stable --- Comment #17 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27860 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #18 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- This didn't apply cleanly to 20.05 and then failed a test when I cleared the conflicts. Please rebase if needed for 20.05. Here's the error: root@kohadevbox:koha(rmain2005)$ prove t/db_dependent/Letters.t t/db_dependent/Letters.t .. 6/74 Fake sendmail at t/db_dependent/Letters.t line 33. Fake sendmail at t/db_dependent/Letters.t line 33. Fake sendmail at t/db_dependent/Letters.t line 33. Fake sendmail at t/db_dependent/Letters.t line 33. Fake sendmail at t/db_dependent/Letters.t line 33. # Failed test 'Exception thrown if invalid email is passed' # at t/db_dependent/Letters.t line 901. # expecting: Koha::Exceptions::BadParameter # found: normal exit # Failed test 'Invalid KohaAdminEmailAddress => status pending' # at t/db_dependent/Letters.t line 905. # got: 'sent' # expected: 'pending' Fake sendmail at t/db_dependent/Letters.t line 33. # Failed test 'Message 1 status is unchanged' # at t/db_dependent/Letters.t line 912. # got: 'sent' # expected: 'pending' # Looks like you planned 5 tests but ran 4. # Looks like you failed 3 tests of 4 run. # Failed test 'Test message_id parameter for SendQueuedMessages' # at t/db_dependent/Letters.t line 914. # Looks like you failed 1 test of 74. t/db_dependent/Letters.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/74 subtests Test Summary Report ------------------- t/db_dependent/Letters.t (Wstat: 256 Tests: 74 Failed: 1) Failed test: 74 Non-zero exit status: 1 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org