Bug 28487 - Overdue_notices does not fall back to default language
Summary: Overdue_notices does not fall back to default language
Status: Pushed to oldoldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 26420
Blocks: 28514
  Show dependency treegraph
 
Reported: 2021-05-28 15:18 UTC by Andreas Jonsson
Modified: 2021-07-14 09:40 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Previously overdue notices exclusively used the default language, but bug 26420 changed this to the opposite - to exclusively use the language chosen by the patron. However, if there is no translation for the overdue notice for the language chosen by the patron then no message is sent. This fixes this so that if there is no translation of the overdue notice for the language chosen by the patron, then the default language notice is used.
Version(s) released in:
21.11.00,21.05.01,20.11.07,20.05.13,19.11.20


Attachments
Bug 28487: Remove getletter (18.81 KB, patch)
2021-06-03 11:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28487: Fallback to default template in overdue_notices (2.36 KB, patch)
2021-06-04 10:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28487: Fallback to default template in overdue_notices (2.42 KB, patch)
2021-06-09 08:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28487: Fallback to default template in overdue_notices (2.48 KB, patch)
2021-06-11 14:37 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Jonsson 2021-05-28 15:18:14 UTC
Previously overdue_notices use exclusively the default language but bug 26420 changed this to the opposite, which is to use exclusively the language chosen by the user.

But if no translation have been provided for the chosen language, no message is sent.

I would expect the behavior to be the same as in GetPreparedLetter:

        $letter = getletter( $module, $letter_code, $branchcode, $mtt, $lang );

        unless ( $letter ) {
            $letter = getletter( $module, $letter_code, $branchcode, $mtt, 'default' )
                or warn( "No $module $letter_code letter transported by " . $mtt ),
                    return;
        }
Comment 1 Katrin Fischer 2021-05-29 13:56:07 UTC
I think this sounds like a major bug - a lot of libraries might only define the default notice in their "main" language. That's the expected behaviour and the described change could be bad trouble espeially as overdues are a sensitive area.
Comment 2 Jonathan Druart 2021-06-03 11:02:48 UTC
Created attachment 121580 [details] [review]
Bug 28487: Remove getletter
Comment 3 Jonathan Druart 2021-06-03 11:04:18 UTC
This patch goes way to far but it's the goal we want to reach to prevent future regressions like that.

I will try and see if I can come up with a simpler fix.
Comment 4 Jonathan Druart 2021-06-04 10:21:45 UTC
Created attachment 121627 [details] [review]
Bug 28487: Fallback to default template in overdue_notices

There is no fallback to the "default" language if there is no
language-specific template for the lang of the patron.

I am not really sure why we are not using GetPreparredLetter here (which
defaults), but this needs to be backported into all stable branches and
so as small as possible.
Comment 5 Jonathan Druart 2021-06-04 10:23:08 UTC
Andreas, can you test this patch please?
Comment 6 David Nind 2021-06-05 20:34:25 UTC
Could we get a test plan for this, and I will happily attempt to test 8-).
Comment 7 Jonathan Druart 2021-06-07 09:26:36 UTC
Hi David,

Test plan:
Hi David, 

Hi David, 
- Make sure you have TranslateNotices turned on
- Set a default lang for a given patron
- Have some overdue rules and translate notice templates
- Generate overdues for the patron
- Run the overdue_notices.pl script
=> The translated notice templates is picked
- Remove the translated notice templates for the lang of the patron
- Run the overdue_notices.pl script
=> Without this patch no notice is generated
=> With this patch a notice is generated using the default notice template.
Comment 8 David Nind 2021-06-07 19:36:39 UTC
(In reply to Jonathan Druart from comment #7)

Thanks Jonathan!

For some reason I couldn't get notices to generate for overdue items I created, so will leave for someone else more familiar with this to test.
Comment 9 Martin Renvoize 2021-06-09 08:28:51 UTC
Created attachment 121726 [details] [review]
Bug 28487: Fallback to default template in overdue_notices

There is no fallback to the "default" language if there is no
language-specific template for the lang of the patron.

I am not really sure why we are not using GetPreparredLetter here (which
defaults), but this needs to be backported into all stable branches and
so as small as possible.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2021-06-09 08:29:30 UTC
Code reads well, testing proves it works as expected.. Signing off.
Comment 11 David Nind 2021-06-09 09:14:02 UTC
Thanks Martin!
Comment 12 Nick Clemens 2021-06-11 14:37:09 UTC
Created attachment 121853 [details] [review]
Bug 28487: Fallback to default template in overdue_notices

There is no fallback to the "default" language if there is no
language-specific template for the lang of the patron.

I am not really sure why we are not using GetPreparredLetter here (which
defaults), but this needs to be backported into all stable branches and
so as small as possible.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Jonathan Druart 2021-06-14 15:34:28 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 14 Kyle M Hall 2021-06-18 12:43:33 UTC
Pushed to 21.05.x for 21.05.01
Comment 15 Fridolin Somers 2021-06-21 12:43:36 UTC
Pushed to 20.11.x for 20.11.07
Comment 16 Victor Grousset/tuxayo 2021-06-22 01:04:56 UTC
Backported: Pushed to 20.05.x branch for 20.05.13
Comment 17 wainuiwitikapark 2021-06-24 02:06:31 UTC
Should this be backported to 19.11.x? For 19.11.19?
Comment 18 wainuiwitikapark 2021-07-04 04:35:02 UTC
Should this be backported to 19.11.x? For 19.11.20?
Comment 19 Jonathan Druart 2021-07-06 07:57:56 UTC
(In reply to wainuiwitikapark from comment #18)
> Should this be backported to 19.11.x? For 19.11.20?

Yes, I think so.
Comment 20 wainuiwitikapark 2021-07-07 02:00:11 UTC
Backported to 19.11.x for 19.11.20
Comment 21 Katrin Fischer 2021-07-14 09:40:39 UTC
Just found another instance of the issue while printing fine related slips:
bug 28712