Bug 24825

Summary: Some templates are missing [% SET footerjs = 1 %]
Product: Koha Reporter: Lucas Gass (lukeg) <lucas>
Component: CirculationAssignee: Lucas Gass (lukeg) <lucas>
Status: CLOSED INVALID QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: david, gmcharlt, jonathan.druart, kyle.m.hall
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:
Circulation function:
Bug Depends on:    
Bug Blocks: 17858    
Attachments: Bug 24825: Add [% SET footerjs = 1 %] to circulation-home.tt
Bug 24825: Add [% SET footerjs = 1 %] to circulation-home.tt
Bug 24825: Add [% SET footerjs = 1 %] to on pages that lack it
Bug 24825: Add [% SET footerjs = 1 %] to on pages that lack it

Description Lucas Gass (lukeg) 2020-03-06 23:35:15 UTC
circulation-home.tt does not have [% SET footerjs = 1 %], therefore javascript/jQuery are impossible on this page.

TO RECERATE:
-add this jQuery to IntranetUserJS: $('#logo').hide();
-load the circulation page, it does not hide the logo.
-go to any other page on the staff client, the logo is hidden.
Comment 1 Lucas Gass (lukeg) 2020-03-06 23:42:09 UTC Comment hidden (obsolete)
Comment 2 David Nind 2020-03-07 06:52:37 UTC Comment hidden (obsolete)
Comment 3 David Nind 2020-03-07 06:55:33 UTC
Hi Lucas.

I have signed this off as this fixes the issue on the circulation page, however the same thing occurs on these pages:

- Tools
- Tools > Patron card creator
- Reports
- Reports > Items with no checkouts
- Circulation > Set library
- Circulation > Transfer
- Circulation > Overdues with fines

Not sure whether these should be fixed in this bug, or a new bug(s) created.

David
Comment 4 Lucas Gass (lukeg) 2020-03-09 15:06:02 UTC
Created attachment 100387 [details] [review]
Bug 24825: Add [% SET footerjs = 1 %] to on pages that lack it

This patch adds the adds the JS Footer to all the pages which lack it:
TEST PLAN:
-add this jQuery to IntranetUserJS: $('#logo').hide();
-load the circulation page, it does not hide the logo.
-go to any other page on the staff client, the logo is hidden.
-apply patch
-load the circulation page, the logo is hidden. Check these other pages to make sure the logo is hidden here to:
- Tools
- Tools > Patron card creator
- Reports
- Reports > Items with no checkouts
- Circulation > Set library
- Circulation > Transfer
- Circulation > Overdues with fines
Comment 5 Lucas Gass (lukeg) 2020-03-09 15:07:36 UTC
Thanks for looking David, I have added a new patch that encompasses all the pages you discovered.
Comment 6 David Nind 2020-03-10 04:56:20 UTC
Created attachment 100419 [details] [review]
Bug 24825: Add [% SET footerjs = 1 %] to on pages that lack it

This patch adds the adds the JS Footer to all the pages which lack it:
TEST PLAN:
-add this jQuery to IntranetUserJS: $('#logo').hide();
-load the circulation page, it does not hide the logo.
-go to any other page on the staff client, the logo is hidden.
-apply patch
-load the circulation page, the logo is hidden. Check these other pages to make sure the logo is hidden here to:
- Tools
- Tools > Patron card creator
- Reports
- Reports > Items with no checkouts
- Circulation > Set library
- Circulation > Transfer
- Circulation > Overdues with fines

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 Jonathan Druart 2020-03-10 12:39:39 UTC
Lucas, how did you generate the list of the files you modified?
Comment 8 Jonathan Druart 2020-03-10 12:42:18 UTC
grep -L "SET footerjs" **/*.tt

All of them are not valid. But some are, for instance acqui/basketgroup.tt
Comment 9 Lucas Gass (lukeg) 2020-03-10 14:39:25 UTC
Jonathan,

I got the list of files from David, who discovered pages where jQuery does not work. 

I don't understand what you mean by 'All of them are not valid.' Are there some cases where I should not add [% SET footerjs %]? Then there is no way to use jQuery on that particular page?

Also, do I have to get every single instance where the template does NOT include [% SET footerjs %]?


Thanks
Comment 10 Jonathan Druart 2020-03-10 18:44:02 UTC
Hi Lucas,

The grep command will list all the template (.tt) files where "SET footerjs" does not exist. In some case it's expected (for instance serials/tables/members_results.tt, or virtualshelves/downloadshelf.tt).

I pointed out basketgroup.tt, but actually this file should not set footerjs to 1 (however the logo does not hide(?)).

Actually I think the bug is somewhere else. Those files should not need to set footerjs to have IntranetUserJS executed correctly.
Comment 11 Jonathan Druart 2020-03-10 18:46:50 UTC
ha, my bad.
Lucas, this is not a bug, you must have you code inside $(document).ready

Put the following code inside IntranetUserJS and it will work:

$(document).ready(function(){
    $('#logo').hide();
});

I think this should be marked as invalid.
Comment 12 Lucas Gass (lukeg) 2020-03-10 19:29:22 UTC
Jonathan, the bad is mine. :( 

Marking as invalid.