[Bug 43380] New: Koha will generate spurious print notices if itiva is enabled and a patron has only selected phone notices
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43380 Bug ID: 43380 Summary: Koha will generate spurious print notices if itiva is enabled and a patron has only selected phone notices Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: martin.renvoize@openfifth.co.uk Target Milestone: --- If a patron only has itiva notices enabled ( and possibly sometimes with other transports enabled, depending on sorting ), Koha will always generate a print notice for them. Take this excerpt from Reserves.pm: 2086 while ( my ( $mtt, $letter_code ) = each %{ $messagingprefs->{transports} } ) { 2087 if ( 2088 ( $mtt eq 'email' and not $to_address ) # No email address 2089 or ( $mtt eq 'sms' and not $patron->smsalertnumber ) # No SMS number 2090 or ( $mtt eq 'itiva' 2091 and C4::Context->preference('TalkingTechItivaPhoneNotification') 2092 ) # Notice is handled by TalkingTech_itiva_outbound.pl 2093 or ( $mtt eq 'phone' and not $patron->phone ) # No phone number to call 2094 ) 2095 { 2096 unless ($notification_sent) { 2097 $notification_sent++; 2098 &$send_notification( 'print', 'HOLD' ); 2099 } 2100 next; 2101 } 2102 2103 &$send_notification( $mtt, $letter_code, $messagingprefs->{wants_digest} ); 2104 } 2105 } Lines 2090/2091 will always be true if itiva is enabled. If the patron has only itiva enabled ( or itiva happens to be the first transport handled in the loop ), the value check at line 2096 will always be false, and thus the call to send_notification for the print version will be triggered at line 2098. It seems that we should replace lines 2090/2091 with something like: or ( ( $mtt eq 'itiva' and !C4::Context->preference('TalkingTechItivaPhoneNotification') ) or ( $mtt eq 'itiva' and not $patron->phone ) ) so if they are to be sent an itiva notice but itiva is not enabled, or itiva *is* enabled but they don't have a voice phone number set, *then* send the 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=43380 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=43380 --- Comment #1 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 204173 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204173&action=edit Bug 43380: Koha will generate spurious print notices if itiva is enabled and a patron has only selected phone notices If a patron only has itiva notices enabled ( and possibly sometimes with other transports enabled, depending on sorting ), Koha will always generate a print notice for them. The itiva check is always true if TalkingTechItivaPhoneNotification is enabled, so itiva falls into the same branch as a transport that can't be delivered and a print notice is sent. The itiva notice itself is generated by TalkingTech_itiva_outbound.pl, so there is nothing to queue here and no reason to fall back to print. This patch skips itiva when TalkingTech_itiva_outbound.pl will handle the notice, and falls back to print when it won't, that is when TalkingTechItivaPhoneNotification is turned off or the patron has no phone number for itiva to call. Before this patch that second case queued an itiva notice that nothing would ever send. Test Plan: 1) Apply this patch 2) Restart all the things! 3) Set TalkingTechItivaPhoneNotification to "Enable" 4) Find a patron with a phone number set 5) Set their "Hold filled" messaging preference to phone only 6) Place a hold for them and check the item in to fill the hold 7) Note there is no print notice in their notices tab 8) Clear the patron's phone number, repeat steps 5 and 6 9) Note the print notice is sent this time 10) Set TalkingTechItivaPhoneNotification to "Disable", restore the phone number, repeat steps 5 and 6 11) Note the print notice is sent this time as well! -- 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=43380 Avery Campbell (Butte County Library (CA)) <acampbell@buttecounty.ca.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |acampbell@buttecounty.ca.go | |v -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43380 John Vinke <jvinke@austinseminary.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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=43380 John Vinke <jvinke@austinseminary.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #204173|0 |1 is obsolete| | -- 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=43380 --- Comment #2 from John Vinke <jvinke@austinseminary.edu> --- Created attachment 208406 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208406&action=edit Bug 43380: Koha will generate spurious print notices if itiva is enabled and a patron has only selected phone notices If a patron only has itiva notices enabled ( and possibly sometimes with other transports enabled, depending on sorting ), Koha will always generate a print notice for them. The itiva check is always true if TalkingTechItivaPhoneNotification is enabled, so itiva falls into the same branch as a transport that can't be delivered and a print notice is sent. The itiva notice itself is generated by TalkingTech_itiva_outbound.pl, so there is nothing to queue here and no reason to fall back to print. This patch skips itiva when TalkingTech_itiva_outbound.pl will handle the notice, and falls back to print when it won't, that is when TalkingTechItivaPhoneNotification is turned off or the patron has no phone number for itiva to call. Before this patch that second case queued an itiva notice that nothing would ever send. Test Plan: 1) Apply this patch 2) Restart all the things! 3) Set TalkingTechItivaPhoneNotification to "Enable" 4) Find a patron with a phone number set 5) Set their "Hold filled" messaging preference to phone only 6) Place a hold for them and check the item in to fill the hold 7) Note there is no print notice in their notices tab 8) Clear the patron's phone number, repeat steps 5 and 6 9) Note the print notice is sent this time 10) Set TalkingTechItivaPhoneNotification to "Disable", restore the phone number, repeat steps 5 and 6 11) Note the print notice is sent this time as well! Signed-off-by: John Vinke <jvinke@austinseminary.edu> -- 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=43380 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- 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=43380 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #208406|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=43380 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208667 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208667&action=edit Bug 43380: Koha will generate spurious print notices if itiva is enabled and a patron has only selected phone notices If a patron only has itiva notices enabled ( and possibly sometimes with other transports enabled, depending on sorting ), Koha will always generate a print notice for them. The itiva check is always true if TalkingTechItivaPhoneNotification is enabled, so itiva falls into the same branch as a transport that can't be delivered and a print notice is sent. The itiva notice itself is generated by TalkingTech_itiva_outbound.pl, so there is nothing to queue here and no reason to fall back to print. This patch skips itiva when TalkingTech_itiva_outbound.pl will handle the notice, and falls back to print when it won't, that is when TalkingTechItivaPhoneNotification is turned off or the patron has no phone number for itiva to call. Before this patch that second case queued an itiva notice that nothing would ever send. Test Plan: 1) Apply this patch 2) Restart all the things! 3) Set TalkingTechItivaPhoneNotification to "Enable" 4) Find a patron with a phone number set 5) Set their "Hold filled" messaging preference to phone only 6) Place a hold for them and check the item in to fill the hold 7) Note there is no print notice in their notices tab 8) Clear the patron's phone number, repeat steps 5 and 6 9) Note the print notice is sent this time 10) Set TalkingTechItivaPhoneNotification to "Disable", restore the phone number, repeat steps 5 and 6 11) Note the print notice is sent this time as well! Signed-off-by: John Vinke <jvinke@austinseminary.edu> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43380 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208668 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208668&action=edit Bug 43380: (QA follow-up) Add tests for itiva handling in _koha_notify_reserve Cover the three cases: itiva handled by TalkingTech_itiva_outbound.pl (no print, nothing queued), no phone number (print fallback), and TalkingTechItivaPhoneNotification disabled (print fallback). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43380 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- QA review (Martin Renvoize) Reviewed the patch for regressions, code quality, Koha standards, security and performance. No blockers found. The fix is correct: itiva is skipped when TalkingTech_itiva_outbound.pl will generate the notice (pref enabled and patron has a phone), and falls back to a print notice otherwise. This also removes the dependence on hash ordering in the transports loop, and the case where an itiva message was queued that nothing would ever send. QA follow-up: added tests to t/db_dependent/Reserves.t covering itiva handled by the cron (no print, nothing queued), no phone number (print fallback) and TalkingTechItivaPhoneNotification disabled (print fallback). The new tests fail without the fix and pass with it. Reserves.t passes in full and koha-qa has been run. Out of scope, to be filed as a separate bug depending on this one: Koha::Patron::queue_notice has the same itiva check without the phone test, and the itiva cron emits rows with a blank phone number. Note: the previous attachment was obsoleted because the patches were re-attached with sign-off lines added. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org