[Bug 42646] New: ModSuggestion: SMS status-change notices always contain email template content
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42646 Bug ID: 42646 Summary: ModSuggestion: SMS status-change notices always contain email template content Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org In C4/Suggestions.pm, ModSuggestion() selects the transport type (email or sms) at ~line 253 and stores it in $transport, but then calls GetPreparedLetter() at ~line 256 without passing message_transport_type. GetPreparedLetter() defaults message_transport_type to 'email' when the parameter is absent (C4/Letters.pm ~line 585), so it always fetches the email template variant of the notice â even when $transport was resolved to 'sms'. The result is that any patron who legitimately receives an ACCEPTED/AVAILABLE/REJECTED/ORDERED/CHECKED notice via SMS receives the email-formatted body text rather than the SMS-specific template content. Related to: Bug 39984, Bug 42645 Test plan: 1. Configure SMSSendDriver and a patron with smsalertnumber but no email. 2. Enable FallbackToSMSIfNoEmail. 3. Create SMS-specific templates for the ACCEPTED notice code (Notices > ACCEPTED > SMS tab). 4. Submit a suggestion as that patron and accept it. 5. Check the message_queue table: the content column will contain the email template body, not the SMS template body. 6. Apply patch (add message_transport_type => $transport to the GetPreparedLetter() call in ModSuggestion). 7. Repeat: the content column now contains the SMS template body. -- 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=42646 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- 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=42646 Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hdunne-howrie@westminster.g | |ov.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42646 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Comma delimited| |OpenFifth list of Sponsors| |<https://openfifth.co.uk/> Depends on| |42645 Sponsorship status|--- |Sponsored Patch complexity|--- |Trivial patch Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42645 [Bug 42645] ModSuggestion: FallbackToSMSIfNoEmail ignores EmailFieldPrimary/EmailFieldPrecedence -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42646 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199300 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199300&action=edit Bug 42646: Pass message_transport_type to GetPreparedLetter in ModSuggestion GetPreparedLetter() was called without a message_transport_type parameter, causing it to default to 'email' and always fetch the email template variant — even when the resolved transport was 'sms'. The SMS-specific template tab was therefore never consulted for suggestion status-change notices. Sponsored-by: OpenFifth <https://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=42646 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199301 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199301&action=edit Bug 42646: Add/update tests for SMS template selection in ModSuggestion - Add CHECKED/sms and ORDERED/sms letter fixtures so SMS transport tests exercise the correct code path. - Assert that the queued message content matches the SMS template variant, not the email variant. - Reset FallbackToSMSIfNoEmail at the start of the GetUnprocessedSuggestions subtest to prevent state from the transport tests leaking in. Sponsored-by: OpenFifth <https://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=42646 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Happy to sign off, as everything works as per the test plan. However, if FallbackToSMSIfNoEmail is set to "Enabled" then the tests fail (they pass if set to "Disable"): prove t/db_dependent/Suggestions.t t/db_dependent/Suggestions.t .. 1/56 # Failed test 'ModSuggestion status warning is correct' # at t/db_dependent/Suggestions.t line 240. # found warning: No suggestions STALLED letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 598. # expected to find warning: No suggestions STALLED letter transported by email t/db_dependent/Suggestions.t .. 54/56 # Looks like you failed 1 test of 56. t/db_dependent/Suggestions.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/56 subtests Test Summary Report ------------------- t/db_dependent/Suggestions.t (Wstat: 256 (exited 1) Tests: 56 Failed: 1) Failed test: 23 Non-zero exit status: 1 Files=1, Tests=56, 5 wallclock secs ( 0.03 usr 0.00 sys + 3.55 cusr 1.14 csys = 4.72 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42646 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199384&action=edit Bug 42646: (QA follow-up) Mock FallbackToSMSIfNoEmail at top of Suggestions.t Without this mock, tests fail when FallbackToSMSIfNoEmail is set to Enabled in the database. The patron used in early tests has an smsalertnumber but no email, so the transport resolves to 'sms' and the expected warning 'transported by email' does not match. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42646 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Thanks for testing again David. That follow-up should make the test more resilient and pass regardless of the settings you have enabled. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42646 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |lisette@bywatersolutions.co |y.org |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42646 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=42646 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199300|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=42646 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199301|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=42646 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199384|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=42646 --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 199386 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199386&action=edit Bug 42646: Pass message_transport_type to GetPreparedLetter in ModSuggestion GetPreparedLetter() was called without a message_transport_type parameter, causing it to default to 'email' and always fetch the email template variant — even when the resolved transport was 'sms'. The SMS-specific template tab was therefore never consulted for suggestion status-change notices. Sponsored-by: OpenFifth <https://openfifth.co.uk/> 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=42646 --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 199387 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199387&action=edit Bug 42646: Add/update tests for SMS template selection in ModSuggestion - Add CHECKED/sms and ORDERED/sms letter fixtures so SMS transport tests exercise the correct code path. - Assert that the queued message content matches the SMS template variant, not the email variant. - Reset FallbackToSMSIfNoEmail at the start of the GetUnprocessedSuggestions subtest to prevent state from the transport tests leaking in. Sponsored-by: OpenFifth <https://openfifth.co.uk/> 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=42646 --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 199388 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199388&action=edit Bug 42646: (QA follow-up) Mock FallbackToSMSIfNoEmail at top of Suggestions.t Without this mock, tests fail when FallbackToSMSIfNoEmail is set to Enabled in the database. The patron used in early tests has an smsalertnumber but no email, so the transport resolves to 'sms' and the expected warning 'transported by email' does not match. 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=42646 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #9 from Lisette Scheer <lisette@bywatersolutions.com> --- Worked great, but tests are failing: # found warning: No suggestions STALLED letter transported by sms at /kohadevbox/koha/C4/Letters.pm line 598. # expected to find warning: No suggestions STALLED letter transported by email -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org