Bug 28768

Summary: OPAC reading history page (opac-readingrecord.pl) wont display news items
Product: Koha Reporter: Lucas Gass <lucas>
Component: OPACAssignee: Lucas Gass <lucas>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, janet.mcgowan, jonathan.druart, kyle, victor
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.05,20.11.12
Bug Depends on:    
Bug Blocks: 24223    
Attachments: Bug 28768: remove unnessesary template variable
Bug 28768: remove unnessesary template variable
Bug 28768: remove unnessesary template variable
Bug 28768: (QA follow-up) Remove unused variable
Bug 28768: Fix borrowernumber from opac-readingrecord.tt

Description Lucas Gass 2021-07-27 14:15:38 UTC
To recreate:

1. Add some news that should display on opac-readingrecord.pl. For example: opacheader, OpacCustomSearch, opaccredits

2. Go to opac-readingrecord.pl, the news doesn't display

Checking the errors logs I see:
[WARN] File not found : default/js/locale_data.js at /kohadevbox/koha/Koha/Template/Plugin/Asset.pm line 84
Comment 1 Andrew Isherwood 2021-07-27 15:00:08 UTC
I've been investigating this one today not knowing it had already been reported!

It seems to be due to the template for this page not receiving the language (e.g. en-GB) and instead receiving "default".The effect of this is that the call to KohaNews.get in masthead.inc fails.

I've not been able to determine why this page in particular is suffering from this issue, I'm hoping someone with more experience of this area can fix it.
Comment 2 Andrew Isherwood 2021-07-27 15:01:36 UTC
I should have said "call to KohaNews.get in any templates/includes that make the call" since this doesn't just affect masthead.inc
Comment 3 Lucas Gass 2021-07-27 18:05:57 UTC
It is this line in opac-readingrecord.pl:

$template->param(%{$borr});

which is setting the language to 'default'. I'm pretty sure its unnecessary to pass this to the template and we can remove the line.
Comment 4 Lucas Gass 2021-07-27 18:11:34 UTC
Created attachment 123230 [details] [review]
Bug 28768: remove unnessesary template variable

To test:
1. Add some news that should display on opac-readingrecord.pl. For example: opacheader, OpacCustomSearch, opaccredits
2. Go to opac-readingrecord.pl, the news doesn't display
3. Apply patch, restart_all
4. Go to opac-readingrecord.pl, the news displays
5. Turn on 'opacreadinghistory' and test to make sure it still works
Comment 5 Andrew Isherwood 2021-07-28 07:52:00 UTC
Created attachment 123241 [details] [review]
Bug 28768: remove unnessesary template variable

To test:
1. Add some news that should display on opac-readingrecord.pl. For example: opacheader, OpacCustomSearch, opaccredits
2. Go to opac-readingrecord.pl, the news doesn't display
3. Apply patch, restart_all
4. Go to opac-readingrecord.pl, the news displays
5. Turn on 'opacreadinghistory' and test to make sure it still works

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 6 Nick Clemens 2021-07-30 12:15:03 UTC
Created attachment 123311 [details] [review]
Bug 28768: remove unnessesary template variable

To test:
1. Add some news that should display on opac-readingrecord.pl. For example: opacheader, OpacCustomSearch, opaccredits
2. Go to opac-readingrecord.pl, the news doesn't display
3. Apply patch, restart_all
4. Go to opac-readingrecord.pl, the news displays
5. Turn on 'opacreadinghistory' and test to make sure it still works

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens 2021-07-30 12:15:06 UTC
Created attachment 123312 [details] [review]
Bug 28768: (QA follow-up) Remove unused variable

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Jonathan Druart 2021-07-30 13:58:19 UTC
Suspecting a regression:

239     var borrowernumber = "[% borrowernumber | html %]";

I don't see borrowernumber passed from the pl.

That's not an insignificant change, the different include must be checked to make sure none is using one patron's attribute.
Comment 9 Jonathan Druart 2021-11-05 08:32:18 UTC
Created attachment 127339 [details] [review]
Bug 28768: Fix borrowernumber from opac-readingrecord.tt
Comment 10 Jonathan Druart 2021-11-05 08:32:38 UTC
(In reply to Jonathan Druart from comment #8)
> That's not an insignificant change, the different include must be checked to
> make sure none is using one patron's attribute.

Done, didn't find anything else.
Comment 11 Jonathan Druart 2021-11-05 11:14:52 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 12 Kyle M Hall 2021-11-11 12:27:26 UTC
Pushed to 21.05.x for 21.05.05
Comment 13 Fridolin Somers 2021-11-12 23:14:36 UTC
Pushed to 20.11.x for 20.11.12
Comment 14 Victor Grousset/tuxayo 2021-11-16 08:15:16 UTC
Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed.