Bug 28487

Summary:	Overdue_notices does not fall back to default language
Product:	Koha	Reporter:	Andreas Jonsson <andreas.jonsson>
Component:	Notices	Assignee:	Jonathan Druart <jonathan.druart>
Status:	Pushed to oldoldoldstable ---	QA Contact:	Testopia <testopia>
Severity:	major
Priority:	P5 - low	CC:	david, fridolin.somers, jonathan.druart, kyle, nick, victor, wainuiwitikapark
Version:	Main
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27660 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28555 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28712
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
Circulation function:
Bug Depends on:	26420
Bug Blocks:	28514
Attachments:	Bug 28487: Remove getletter Bug 28487: Fallback to default template in overdue_notices Bug 28487: Fallback to default template in overdue_notices Bug 28487: Fallback to default template in overdue_notices

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 (ashimema) 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 (ashimema) 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 (kidclamp) 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 (khall) 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 Wainui Witika-Park 2021-06-24 02:06:31 UTC

Should this be backported to 19.11.x? For 19.11.19?

Comment 18 Wainui Witika-Park 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 Wainui Witika-Park 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