[Bug 42809] New: Notification falls to print back when two messaging preferences are set but only one is valid.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 Bug ID: 42809 Summary: Notification falls to print back when two messaging preferences are set but only one is valid. Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: baptiste.wojtkowski@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: martin.renvoize@openfifth.co.uk Target Milestone: --- See https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40960#c18 This [Bug 40960] causes problem for patrons with their messaging preferences set to receive both sms and mail and one of the transport types fails. For example when the patron does not have a mail registered but both message types are checked on their account they will receive both sms and printed notices. When using standard/pre-filled messaging preferences based on patron category this can create a large number of unwanted print notices. When a notice is sent with sms OR mail you don't want to create a print notice. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |40960 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40960 [Bug 40960] Only generate a notice for patrons about holds filled if they have set messaging preferences -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |baptiste.wojtkowski@biblibr |ity.org |e.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #1 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Not sure if this is a regression -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 Anneli Österman <anneli.osterman@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |anneli.osterman@koha-suomi. | |fi --- Comment #2 from Anneli Österman <anneli.osterman@koha-suomi.fi> --- We use a JavaScript to check that the patron has email address or sms number when the corresponding messaging preference is selected. If they do not have email address/sms number and the preference is selected, the script removes the selection (and notifies the staff about the removal). This naturally works only when the patron infromation is modified or created. Maybe this kind of function could be added to Koha? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #3 from David Cook <dcook@prosentient.com.au> --- (In reply to Baptiste Wojtkowski (bwoj) from comment #1)
Not sure if this is a regression
It's certainly a regression. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #3)
(In reply to Baptiste Wojtkowski (bwoj) from comment #1)
Not sure if this is a regression
It's certainly a regression.
It's because you're only incrementing $notification_sent when you generate a print notice, so if you generate an email or a SMS, it doesn't increment, so then it activates when it shouldn't. This should be fixed with a one line change. Basically just undoing the minus here: &$send_notification( $mtt, $letter_code, $messagingprefs->{wants_digest} ); - $notification_sent++; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |dcook@prosentient.com.au |y.org | --- Comment #5 from David Cook <dcook@prosentient.com.au> --- @bwoj if you want to write that patch I can test and QA it -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #4)
This should be fixed with a one line change.
Basically just undoing the minus here: &$send_notification( $mtt, $letter_code, $messagingprefs->{wants_digest} ); - $notification_sent++;
Actually... no that's not right. That would be a sequencing problem. It would only work if a valid notification was generated before the the failed notification. So it would be vulnerable to following the order of message_transport_types, which is email then itiva then phone then print then sms. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- So yeah... you'd be better off reverting the change and then adding a 2nd condition where it's 'if (!$notitication_sent && $notification_required)' or something like that, and you can increment that $notification_required within the loop. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CONFIRMED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|CONFIRMED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- Created attachment 207015 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207015&action=edit Bug 42809: Fix messaging preference 'print' fallback This change fixes the conditions around the messaging preference 'print' fallback, so that the print fallback is only used if the user has (1) messaging preferences defined for this notice, and (2) the user does not have the metadata needed to fulfill *any* of the messaging prferences (ie 0 preferred notices could be generated) To reproduce: - Set SMSSendDriver syspref to Email - Using KTD, search for patron with cardnumber 42 - Go to the "Details" tab and "Edit" the "Patron messaging preferences" - For "Hold filled" tick both SMS and Email and click Save - Place a hold for the user - e.g. Search for biblionumber:"29" - Go to Holds - Put 42 in the search box - Hit "Search" button - Hit "Place hold" when the hold screen comes up - Check in the item for biblionumber 29 - e.g. On the top bar go to the "Check in" tab and enter 39999000001310, and press the arrow button - Click "Confirm hold(Y)" - In the patron record, go to "Notices" and note that thre's a print notice generated. This is to be expected since we don't have an email stored yet. - Now cancel the hold, add an email address (doesn't have to be real), and repeat the hold placed and hold filled process above - Note when there is an email address but no SMS number, we have both an "email" and a "print" notice generated. This is the bug! - Edit the user and remove the email address but add a SMS number - Now cancel the hold, and repeat the hold placed and hold filled process above - In the patron record, go to "Notices" and note that we have both a "print" and a "sms" notice generated. This is also the bug! - I'm not quite sure how sure how to check the itiva and phone notifications, but the above should be sufficient to illustrate the bug -- To test: - Apply the patch - restart_all - As per "To reproduce", cancel any holds for user with cardnumber 42 - With a SMS number but no email, repeat the process for placing and filling a hold outlined above - Check the user's notices and note that only a SMS notice is generated - Cancel the hold - Remove the SMS number and add an email address - Repeat the process for placing and filling a hold outlined above - Check the user's notices and note that only an email notice is generated - Cancel the hold - Remove the email address so there is no email address nor a SMS number - Repeat the process for placing and filling a hold outlined above - Check the user's notices and note that only a print notice is generated - Cancel the hold - Remove the messaging preferences for "SMS" and "Email" - Repeat the process for placing and filling a hold outlined above - Check the user's notices and note that no notice is generated - Cancel the hold -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|dcook@prosentient.com.au |testopia@bugs.koha-communit | |y.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- Technically, the best solution would be to create a Koha::Patron method like Koha::Patron->available_messaging_transports(), and unit test that, but... I just wanted to fix the bug ASAP. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #10 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- On main on ktd while reproducing bug I could not manage to get a print notice when setting the hold, am I missing something ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #11 from David Nind <david@davidnind.com> --- (In reply to Baptiste Wojtkowski (bwoj) from comment #10)
On main on ktd while reproducing bug I could not manage to get a print notice when setting the hold, am I missing something ?
I was able to reproduce the issue before the patch: 1. SMSSendDriver system preference set to Email 2. Messaging preferences for the patron: Hold filled = SMS and Email selected 3. No SMS number or email address: ==> Print notice generated 4. No SMS number but has an email address: ==> Print and sms notices generated 5. An SMS number but no email address: ==> sms notice generated -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #207015|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 207158 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207158&action=edit Bug 42809: Fix messaging preference 'print' fallback This change fixes the conditions around the messaging preference 'print' fallback, so that the print fallback is only used if the user has (1) messaging preferences defined for this notice, and (2) the user does not have the metadata needed to fulfill *any* of the messaging prferences (ie 0 preferred notices could be generated) To reproduce: - Set SMSSendDriver syspref to Email - Using KTD, search for patron with cardnumber 42 - Go to the "Details" tab and "Edit" the "Patron messaging preferences" - For "Hold filled" tick both SMS and Email and click Save - Place a hold for the user - e.g. Search for biblionumber:"29" - Go to Holds - Put 42 in the search box - Hit "Search" button - Hit "Place hold" when the hold screen comes up - Check in the item for biblionumber 29 - e.g. On the top bar go to the "Check in" tab and enter 39999000001310, and press the arrow button - Click "Confirm hold(Y)" - In the patron record, go to "Notices" and note that thre's a print notice generated. This is to be expected since we don't have an email stored yet. - Now cancel the hold, add an email address (doesn't have to be real), and repeat the hold placed and hold filled process above - Note when there is an email address but no SMS number, we have both an "email" and a "print" notice generated. This is the bug! - Edit the user and remove the email address but add a SMS number - Now cancel the hold, and repeat the hold placed and hold filled process above - In the patron record, go to "Notices" and note that we have both a "print" and a "sms" notice generated. This is also the bug! - I'm not quite sure how sure how to check the itiva and phone notifications, but the above should be sufficient to illustrate the bug -- To test: - Apply the patch - restart_all - As per "To reproduce", cancel any holds for user with cardnumber 42 - With a SMS number but no email, repeat the process for placing and filling a hold outlined above - Check the user's notices and note that only a SMS notice is generated - Cancel the hold - Remove the SMS number and add an email address - Repeat the process for placing and filling a hold outlined above - Check the user's notices and note that only an email notice is generated - Cancel the hold - Remove the email address so there is no email address nor a SMS number - Repeat the process for placing and filling a hold outlined above - Check the user's notices and note that only a print notice is generated - Cancel the hold - Remove the messaging preferences for "SMS" and "Email" - Repeat the process for placing and filling a hold outlined above - Check the user's notices and note that no notice is generated - Cancel the hold Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes the generation release notes| |of patron notices. If a | |patron had an email address | |but no SMS number, both an | |"email" and a "print" | |notice were generated. Now, | |only an email address is | |printed. | | | |Example scenario: | |1. SMSSendDriver system | |preference set to Email | |2. Messaging preferences | |for a patron: Hold filled = | |SMS and Email selected | |3. No SMS number or email | |address: | | ==> Print notice | |generated (as expected) | |4. No SMS number but has an | |email address: | | ==> Before the fix: | |email and print notices | |generated (this was the | |issue this bug fixes - only | |the email notice should | |have been generated) | | ==> After the fix: email | |notice generated (now works | |as expected) | |5. An SMS number but no | |email address: | | ==> sms notice generated | |(as expected) Summary|Notification falls to print |Notification falls to print |back when two messaging |back when two messaging |preferences are set but |preferences are set but |only one is valid. |only one is valid -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 --- Comment #13 from David Nind <david@davidnind.com> --- (In reply to David Nind from comment #11)
4. No SMS number but has an email address: ==> Print and sms notices generated
Mucked this up, should read:
==> Print and email notices generated
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42809 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |bibliothek@th-wildau.de |y.org | CC| |bibliothek@th-wildau.de -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org