Bug 19641 - Move patron templates JavaScript to the footer
Summary: Move patron templates JavaScript to the footer
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Josef Moravec
URL:
Whiteboard:
Keywords:
: 19728 (view as bug list)
Depends on: 19594 19603 19921
Blocks: 17858 21813 19682 19728 19954 19982 20221 20498 20805
  Show dependency treegraph
 
Reported: 2017-11-16 16:59 UTC by Owen Leonard
Modified: 2018-11-12 18:41 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19641: Move patron templates to the footer (154.28 KB, patch)
2017-11-17 15:19 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19641: Move patron templates to the footer (154.34 KB, patch)
2017-11-28 14:10 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 19641: Move patron templates to the footer (151.53 KB, patch)
2018-01-11 16:30 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19641 (Follow-up) Update circulation.tt too (13.37 KB, patch)
2018-01-11 16:30 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19641 (Follow-up) Correct popup templates (1.98 KB, patch)
2018-01-11 17:44 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19641 (Follow-up) Correct popup templates (2.03 KB, patch)
2018-01-18 21:23 UTC, Zoe Bennett
Details | Diff | Splinter Review
Bug 19641 (Follow-up) Correct popup templates (2.08 KB, patch)
2018-01-18 21:50 UTC, Zoe Bennett
Details | Diff | Splinter Review
Bug 19641: Move patron templates to the footer (151.57 KB, patch)
2018-01-18 21:55 UTC, Zoe Bennett
Details | Diff | Splinter Review
Bug 19641 (Follow-up) Update circulation.tt too (13.42 KB, patch)
2018-01-18 21:55 UTC, Zoe Bennett
Details | Diff | Splinter Review
Bug 19641 (Follow-up) Correct popup templates (2.12 KB, patch)
2018-01-18 21:55 UTC, Zoe Bennett
Details | Diff | Splinter Review
Bug 19641: Follow-up - Fix update child menu option (1.85 KB, patch)
2018-01-26 13:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19641: Move patron templates to the footer (151.69 KB, patch)
2018-02-07 16:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19641: (Follow-up) Update circulation.tt too (13.54 KB, patch)
2018-02-07 16:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19641: (Follow-up) Correct popup templates (2.14 KB, patch)
2018-02-07 16:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19641: (Follow-up) Fix update child menu option (1.96 KB, patch)
2018-02-07 16:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19641: (follow-up) Move patron templates to the footer (13.88 KB, patch)
2018-02-15 18:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19641: (follow-up) Update circulation.tt too (1.18 KB, patch)
2018-02-15 18:20 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2017-11-16 16:59:33 UTC
In order to conform to best practices for performance staff client template javascript should be in the footer instead of the header.

This patch will do so for patron module templates.
Comment 1 Owen Leonard 2017-11-17 15:19:27 UTC Comment hidden (obsolete)
Comment 2 Claire Gravely 2017-11-28 14:10:19 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2017-12-21 23:21:29 UTC
One of the dependencies is not in the QA queue yet, moving to BLOCKED for now, please move back once dependencies have been sorted.
Comment 4 Josef Moravec 2018-01-02 19:58:47 UTC
Hi Owen, looks good, but few thinks:

So, if I want to update child to adult, and have more patron categories for adults, than I should see the form for choosing one of them, but I get blank page...
Also the javascript function confirm_updatechild is defined twice: in members-menu.js and update-child.tt.

Few buttons in patron toolbar on checkouts screen does not work - circulation.tt is not part of this patch and so does not include members-menu.js. I think you will fix this bug 19728, true?
Comment 5 Owen Leonard 2018-01-05 15:38:43 UTC
*** Bug 19728 has been marked as a duplicate of this bug. ***
Comment 6 Owen Leonard 2018-01-05 15:42:26 UTC
(In reply to Josef Moravec from comment #4)

> Few buttons in patron toolbar on checkouts screen does not work -
> circulation.tt is not part of this patch and so does not include
> members-menu.js. I think you will fix this bug 19728, true?

I've marked 19728 as a duplicate of this bug and will submit a follow-up that includes circulation.tt and circulation_batch_checkouts.tt
Comment 7 Owen Leonard 2018-01-11 16:30:05 UTC
Created attachment 70428 [details] [review]
Bug 19641: Move patron templates to the footer

This patch modifies the staff client patron module templates so that
JavaScript is included in the footer instead of the header.

This patch touches a lot of files because the changes are all
interdependent, affecting a couple of module-wide include files.

To test, apply the patch and test the JavaScript-driven features of the
modified templates: All button controls, DataTables functionality, tabs,
etc.

Patrons -> Patrons home, patron search results
  -> Manage pending modification requests
  -> Patron detail page
    -> Edit patron
      -> Set guarantor
    -> Fines
       -> Account, Pay fines, Create manual invoice, Create manual
          credit
       -> Print receipts for different kinds of charges
    -> Routing lists
    -> Circulation history
    -> Holds history
    -> Notices
    -> Statistics
    -> Files
    -> Purchase suggestions
    -> Discharges
    -> Housebound
    -> Set permissions
    -> Change password
    -> Print summary, slips, and overdues
    -> Update child to adult patron type

Patron toolbar and patron search bar operations should work correctly on
all pages.

This patch also updates the template for searching the Norwegian
national patron database, but it has NOT been tested.

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 8 Owen Leonard 2018-01-11 16:30:08 UTC
Created attachment 70429 [details] [review]
Bug 19641 (Follow-up) Update circulation.tt too

This patch makes the same JS-to-footer changes to circulation.tt.
Inclusion of this template in this bug is necessary because it includes
members-toolbar.inc.

To test, apply the patch and clear your browser cache if necessary.

- Enter a patron card for checkout
- Test any JavaScript-driven actions on the checkout page, for instance:
    - Select all/clear all controls
    - Datepicker/timepickers
    - Datatable sorting, columns configuration
    - Renewals, check-ins
    - Patron message interactions, print slip, and other toolbar actions
    - Checkout exports
    - Search to hold
    - Tab loading: Restrictions, Holds, Clubs
Comment 9 Owen Leonard 2018-01-11 17:44:05 UTC
Created attachment 70433 [details] [review]
Bug 19641 (Follow-up) Correct popup templates

This patch corrects the footer include for two patron-related popup
windows. popup-bottom.inc should be considered obsolete, and
"'intranet-bottom.inc' popup_window=1" used in its place.

To test you must have more than one adult-type patron category defined.

- Locate and view a child-type patron record.
- From the "More" menu in the toolbar, choose "Update child to adult
  patron."
  - The popup window which appears should look correct and work
    correctly.
- Add a new patron and enter the first and last names of an existing
  patron.
  - When Koha asks to confirm a possible duplicate record, click the
    "View existing record" link. The popup which is triggered should
    look correct.
Comment 10 Zoe Bennett 2018-01-18 21:23:19 UTC
Created attachment 70720 [details] [review]
Bug 19641 (Follow-up) Correct popup templates

This patch corrects the footer include for two patron-related popup
windows. popup-bottom.inc should be considered obsolete, and
"'intranet-bottom.inc' popup_window=1" used in its place.

To test you must have more than one adult-type patron category defined.

- Locate and view a child-type patron record.
- From the "More" menu in the toolbar, choose "Update child to adult
  patron."
  - The popup window which appears should look correct and work
    correctly.
- Add a new patron and enter the first and last names of an existing
  patron.
  - When Koha asks to confirm a possible duplicate record, click the
    "View existing record" link. The popup which is triggered should
    look correct.

Signed-off-by: Zoe Bennett <zoebennett1308@gmail.com>
Comment 11 Zoe Bennett 2018-01-18 21:50:19 UTC
Created attachment 70723 [details] [review]
Bug 19641 (Follow-up) Correct popup templates

This patch corrects the footer include for two patron-related popup
windows. popup-bottom.inc should be considered obsolete, and
"'intranet-bottom.inc' popup_window=1" used in its place.

To test you must have more than one adult-type patron category defined.

- Locate and view a child-type patron record.
- From the "More" menu in the toolbar, choose "Update child to adult
  patron."
  - The popup window which appears should look correct and work
    correctly.
- Add a new patron and enter the first and last names of an existing
  patron.
  - When Koha asks to confirm a possible duplicate record, click the
    "View existing record" link. The popup which is triggered should
    look correct.

Signed-off-by: Zoe Bennett <zoebennett1308@gmail.com>

Signed-off-by: Your Full Name <your_email>
Comment 12 Zoe Bennett 2018-01-18 21:55:04 UTC
Created attachment 70724 [details] [review]
Bug 19641: Move patron templates to the footer

This patch modifies the staff client patron module templates so that
JavaScript is included in the footer instead of the header.

This patch touches a lot of files because the changes are all
interdependent, affecting a couple of module-wide include files.

To test, apply the patch and test the JavaScript-driven features of the
modified templates: All button controls, DataTables functionality, tabs,
etc.

Patrons -> Patrons home, patron search results
  -> Manage pending modification requests
  -> Patron detail page
    -> Edit patron
      -> Set guarantor
    -> Fines
       -> Account, Pay fines, Create manual invoice, Create manual
          credit
       -> Print receipts for different kinds of charges
    -> Routing lists
    -> Circulation history
    -> Holds history
    -> Notices
    -> Statistics
    -> Files
    -> Purchase suggestions
    -> Discharges
    -> Housebound
    -> Set permissions
    -> Change password
    -> Print summary, slips, and overdues
    -> Update child to adult patron type

Patron toolbar and patron search bar operations should work correctly on
all pages.

This patch also updates the template for searching the Norwegian
national patron database, but it has NOT been tested.

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Signed-off-by: Your Full Name <your_email>
Comment 13 Zoe Bennett 2018-01-18 21:55:07 UTC
Created attachment 70725 [details] [review]
Bug 19641 (Follow-up) Update circulation.tt too

This patch makes the same JS-to-footer changes to circulation.tt.
Inclusion of this template in this bug is necessary because it includes
members-toolbar.inc.

To test, apply the patch and clear your browser cache if necessary.

- Enter a patron card for checkout
- Test any JavaScript-driven actions on the checkout page, for instance:
    - Select all/clear all controls
    - Datepicker/timepickers
    - Datatable sorting, columns configuration
    - Renewals, check-ins
    - Patron message interactions, print slip, and other toolbar actions
    - Checkout exports
    - Search to hold
    - Tab loading: Restrictions, Holds, Clubs

Signed-off-by: Your Full Name <your_email>
Comment 14 Zoe Bennett 2018-01-18 21:55:11 UTC
Created attachment 70726 [details] [review]
Bug 19641 (Follow-up) Correct popup templates

This patch corrects the footer include for two patron-related popup
windows. popup-bottom.inc should be considered obsolete, and
"'intranet-bottom.inc' popup_window=1" used in its place.

To test you must have more than one adult-type patron category defined.

- Locate and view a child-type patron record.
- From the "More" menu in the toolbar, choose "Update child to adult
  patron."
  - The popup window which appears should look correct and work
    correctly.
- Add a new patron and enter the first and last names of an existing
  patron.
  - When Koha asks to confirm a possible duplicate record, click the
    "View existing record" link. The popup which is triggered should
    look correct.

Signed-off-by: Zoe Bennett <zoebennett1308@gmail.com>

Signed-off-by: Your Full Name <your_email>

Signed-off-by: Your Full Name <your_email>
Comment 15 Josef Moravec 2018-01-26 08:33:55 UTC
Sorry, one more thing:

When a patron has adult category, the "Update child to adult patron" looks disabled, but you can click on it, you get a dialog asking you if you want to update the child, and if you click OK, there is internal server error.
Comment 16 Owen Leonard 2018-01-26 13:26:35 UTC
Created attachment 70953 [details] [review]
Bug 19641: Follow-up - Fix update child menu option

This follow-up corrects handling of clicks on the "Update child" menu
option when the patron is an adult. Previously the click handler was
conditionally added via template markup. This patch modifies it to use
the data-tooltip attribute as an indicator that the option is disabled.

To test, apply the patch and clear your browser cache if necessary.

- Open an adult patron's record for viewing.
- Choose More -> Update child from the toolbar menu.
- The menu item should be disabled, and should show a tooltip.
- Clicking the menu item should do nothing.

- Open a child patron's record for viewing.
- Choose More -> Update child from the toolbar menu.
- The patron should be updated to an adult category.
Comment 17 Josef Moravec 2018-02-07 16:37:00 UTC
Created attachment 71289 [details] [review]
Bug 19641: Move patron templates to the footer

This patch modifies the staff client patron module templates so that
JavaScript is included in the footer instead of the header.

This patch touches a lot of files because the changes are all
interdependent, affecting a couple of module-wide include files.

To test, apply the patch and test the JavaScript-driven features of the
modified templates: All button controls, DataTables functionality, tabs,
etc.

Patrons -> Patrons home, patron search results
  -> Manage pending modification requests
  -> Patron detail page
    -> Edit patron
      -> Set guarantor
    -> Fines
       -> Account, Pay fines, Create manual invoice, Create manual
          credit
       -> Print receipts for different kinds of charges
    -> Routing lists
    -> Circulation history
    -> Holds history
    -> Notices
    -> Statistics
    -> Files
    -> Purchase suggestions
    -> Discharges
    -> Housebound
    -> Set permissions
    -> Change password
    -> Print summary, slips, and overdues
    -> Update child to adult patron type

Patron toolbar and patron search bar operations should work correctly on
all pages.

This patch also updates the template for searching the Norwegian
national patron database, but it has NOT been tested.

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Signed-off-by: Zoe Bennett <zoebennett1308@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=19682
Comment 18 Josef Moravec 2018-02-07 16:37:05 UTC
Created attachment 71290 [details] [review]
Bug 19641: (Follow-up) Update circulation.tt too

This patch makes the same JS-to-footer changes to circulation.tt.
Inclusion of this template in this bug is necessary because it includes
members-toolbar.inc.

To test, apply the patch and clear your browser cache if necessary.

- Enter a patron card for checkout
- Test any JavaScript-driven actions on the checkout page, for instance:
    - Select all/clear all controls
    - Datepicker/timepickers
    - Datatable sorting, columns configuration
    - Renewals, check-ins
    - Patron message interactions, print slip, and other toolbar actions
    - Checkout exports
    - Search to hold
    - Tab loading: Restrictions, Holds, Clubs

Signed-off-by: Zoe Bennett <zoebennett1308@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=19682
Comment 19 Josef Moravec 2018-02-07 16:37:10 UTC
Created attachment 71291 [details] [review]
Bug 19641: (Follow-up) Correct popup templates

This patch corrects the footer include for two patron-related popup
windows. popup-bottom.inc should be considered obsolete, and
"'intranet-bottom.inc' popup_window=1" used in its place.

To test you must have more than one adult-type patron category defined.

- Locate and view a child-type patron record.
- From the "More" menu in the toolbar, choose "Update child to adult
  patron."
  - The popup window which appears should look correct and work
    correctly.
- Add a new patron and enter the first and last names of an existing
  patron.
  - When Koha asks to confirm a possible duplicate record, click the
    "View existing record" link. The popup which is triggered should
    look correct.

Signed-off-by: Zoe Bennett <zoebennett1308@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=19682
Comment 20 Josef Moravec 2018-02-07 16:37:14 UTC
Created attachment 71292 [details] [review]
Bug 19641: (Follow-up) Fix update child menu option

This follow-up corrects handling of clicks on the "Update child" menu
option when the patron is an adult. Previously the click handler was
conditionally added via template markup. This patch modifies it to use
the data-tooltip attribute as an indicator that the option is disabled.

To test, apply the patch and clear your browser cache if necessary.

- Open an adult patron's record for viewing.
- Choose More -> Update child from the toolbar menu.
- The menu item should be disabled, and should show a tooltip.
- Clicking the menu item should do nothing.

- Open a child patron's record for viewing.
- Choose More -> Update child from the toolbar menu.
- The patron should be updated to an adult category.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=19682
Comment 23 Jonathan Druart 2018-02-15 16:39:13 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 24 Jonathan Druart 2018-02-15 18:11:19 UTC
Created attachment 71676 [details] [review]
Bug 19641: (follow-up) Move patron templates to the footer

Fix bad merge conflict with bug 12904
Comment 25 Jonathan Druart 2018-02-15 18:20:55 UTC
Created attachment 71677 [details] [review]
Bug 19641: (follow-up) Update circulation.tt too

Fix bad merge conflict with bug 18403
Comment 26 Jonathan Druart 2018-02-15 18:22:07 UTC
Last two patches pushed to master.
Comment 27 Jonathan Druart 2018-02-15 18:28:25 UTC
I have found a problem:
circ/circulation.pl, search for a patron, I get: "ReferenceError: $ is not defined"

circulation.tt includes circ-patron-search-results.inc which has JS inside (bug jQuery is not loaded yet)


Please provide a follow-up here as soon as you can.
Comment 28 Jonathan Druart 2018-03-29 17:03:10 UTC
Something else: code has been moved to members-menu.js but this file is not used anywhere.

The regression I found is:
Edit a patron and open the form in the header [+]
=> It does not display
Comment 29 Owen Leonard 2018-03-30 16:36:49 UTC
(In reply to Jonathan Druart from comment #27)
> I have found a problem:
> circ/circulation.pl, search for a patron, I get: "ReferenceError: $ is not
> defined"

Should have been fixed by Bug 20221, "Fix for JavaScript error during checkout patron search."