|
Description
Marc Véron
2017-04-19 06:59:44 UTC
Created attachment 62353 [details] [review] Bug 18453: Syspref ExportCircHistory is broken The checkouts table on a patron's checkout and detail screen always show a column 'Export' and the 'Export checkouts using format' fields/buttons. They should be hidden if syspref 'ExportCircHistory' is set to 'Don't show'. This patch brings some variables in circulation.tt and in moremember.tt in sync. They are used by the checkouts table (checkouts-table.inc). To test: - Apply patch - Verify that visibility of export fields in checkouts table depends on syspref 'ExportCircHistory' (for both moremember.pl and circulation.pl) - Verify that other functionality behaves as expected on both screens. Created attachment 62362 [details] [review] [SIGNED-OFF] Bug 18453: Syspref ExportCircHistory is broken The checkouts table on a patron's checkout and detail screen always show a column 'Export' and the 'Export checkouts using format' fields/buttons. They should be hidden if syspref 'ExportCircHistory' is set to 'Don't show'. This patch brings some variables in circulation.tt and in moremember.tt in sync. They are used by the checkouts table (checkouts-table.inc). To test: - Apply patch - Verify that visibility of export fields in checkouts table depends on syspref 'ExportCircHistory' (for both moremember.pl and circulation.pl) - Verify that other functionality behaves as expected on both screens. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Comment on attachment 62362 [details] [review] [SIGNED-OFF] Bug 18453: Syspref ExportCircHistory is broken Review of attachment 62362 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ +35,4 @@ > var theme = "[% theme %]"; > var borrowernumber = "[% borrowernumber %]"; > var branchcode = "[% branch %]"; > +var exports_enabled = [% Koha.Preference('ExportCircHistory') %]; If the pref is empty, the generated JS will be var exports_enabled = ; And will raise a syntax error. ::: koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ +27,4 @@ > <script type="text/JavaScript"> > //<![CDATA[ > /* Set some variable needed in circulation.js */ > +var MSG_DT_LOADING_RECORDS = _("Loading... you may continue scanning."); You cannot scan from moremember. @@ +36,3 @@ > var AllowRenewalLimitOverride = [% (CAN_user_circulate_override_renewals && Koha.Preference('AllowRenewalLimitOverride') )? 1: 0 %]; > +var AllowCirculate = [% (CAN_user_circulate_circulate_remaining_permissions)? 1 : 0 %]; > +var script = "circulation"; Did you find where this script variable is used? @@ +43,4 @@ > > var MSG_EXPORT_SELECT_CHECKOUTS = _("You must select checkout(s) to export"); > > +columns_settings = [% ColumnsSettings.GetColumns( 'circ', 'circulation', 'issues-table', 'json' ) %] Hum? Why that? We are on moremember, we need to keep the config for moremember. Created attachment 62393 [details] [review] [ALTERNATIVE-PATCH] Bug 18453: Hide the export column when needed If exports_enabled is equal to "0" it will be evaluated to true. Marc, this single line patch seems to solve the issue. Please confirm. Created attachment 62394 [details] [review] Bug 18453: Remove exports_enabled in circulation_batch_checkouts.tt This is a c/p from circulation.tt, this variable is not needed in this template Created attachment 62399 [details]
Table broken on moremember.pl
Hi Jonathan
First I tried a similar one-liner as well (remove the quotes around the value in following line (both circulation.tt and moremember.tt)
var exports_enabled = [% Koha.Preference('ExportCircHistory') %];
That makes the value a number and the following works:
"bVisible": exports_enabled ? true : false,...
But I still had a nasty behaviour on moremember.pl (the check-out column was not visible, if set to visible with button 'Column visibility' it shows up, but the table is broken, see screenshot.
It does not happen on the checkout page.
The same happens with your two patches applied. So there is more to be investigated.
Anyway, I obsolete my first patch.
OK, found. The table problem is an issue with a colum settings in Administration (issues-table > checkin set to hidden by default), for both circulation and patrons. I will file a new bug for that. Setting to Needs Signoff for alternative patches. Created attachment 62401 [details] [review] Bug 18453: Hide the export column when needed If exports_enabled is equal to "0" it will be evaluated to true. Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 62402 [details] [review] Bug 18453: Remove exports_enabled in circulation_batch_checkouts.tt This is a c/p from circulation.tt, this variable is not needed in this template Signed-off-by: Marc Véron <veron@veron.ch> Comment on attachment 62399 [details]
Table broken on moremember.pl
Will move this issue to a new bug
Created attachment 62694 [details] [review] Bug 18453: Hide the export column when needed If exports_enabled is equal to "0" it will be evaluated to true. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 62695 [details] [review] Bug 18453: Remove exports_enabled in circulation_batch_checkouts.tt This is a c/p from circulation.tt, this variable is not needed in this template Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 17.05, thanks Jonathan! Dependency is not in 16.11.x. |