Bug 21296

Summary: Suspend hold ignores system preference on intranet
Product: Koha Reporter: Donna <bwsdonna>
Component: Hold requestsAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: 1joynelson, bwsdonna, gmcharlt, hayleypelham, jonathan.druart, katrin.fischer, kelly, lucas
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.03, 19.05.08, 18.11.15
Attachments: Bug 21296: suspend hold ignores system preference on intranet
Bug 21296: suspend hold ignores system preference on intranet
Bug 21296: suspend hold ignores system preference on intranet
Bug 21296: [18.11.x] Fix wrong resolution conflict

Description Donna 2018-08-30 20:18:49 UTC
Setting the system preference SuspendHoldsIntranet to Don't Allow has no impact.  Staff can still suspend patron holds from the staff client.

To test:
1. Set SuspendHoldsIntranet to Don't Allow
2. Set SuspendHoldsOPAC to Allow
3. Log in to OPAC and place hold
4. Log in to staff client and pull up patron's account
5. View holds and suspend hold.
6. Date picker window opens and allows suspension of hold.
Comment 1 Owen Leonard 2019-11-20 20:17:29 UTC
Created attachment 95624 [details] [review]
Bug 21296: suspend hold ignores system preference on intranet

This patch modifies the DataTable configuration for the table of holds
which displays under the "Holds" tab on the patron's checkout and
details pages. A variable is now defined in the JavaScript with the
value of the "SuspendHoldsIntranet" system preference. This variable is
passed to the DataTable configuration to determine whether the column is
visible or not.

One of the changes made in this patch moves a <script> block so that it
appears before some of the JS includes on the page. This helps keep the
string-defining JS together in one block.

To test, apply the patch and set the "SuspendHoldsIntranet" system
preference to "Allow."

 - Open the checkout page for a patron with one or more holds.
 - Under the "Holds" tab, the table of holds should have a "Suspend"
   column with controls for suspending or resuming holds.
 - Check that the same is true on the patron detail page.
 - Set the "SuspendHoldsIntranet" system preference to "Don't allow."
 - Test the checkout and detail pages again and confirm that the
   "Suspend" column does not appear.
Comment 2 ByWater Sandboxes 2020-01-17 21:37:15 UTC
Created attachment 97579 [details] [review]
Bug 21296: suspend hold ignores system preference on intranet

This patch modifies the DataTable configuration for the table of holds
which displays under the "Holds" tab on the patron's checkout and
details pages. A variable is now defined in the JavaScript with the
value of the "SuspendHoldsIntranet" system preference. This variable is
passed to the DataTable configuration to determine whether the column is
visible or not.

One of the changes made in this patch moves a <script> block so that it
appears before some of the JS includes on the page. This helps keep the
string-defining JS together in one block.

To test, apply the patch and set the "SuspendHoldsIntranet" system
preference to "Allow."

 - Open the checkout page for a patron with one or more holds.
 - Under the "Holds" tab, the table of holds should have a "Suspend"
   column with controls for suspending or resuming holds.
 - Check that the same is true on the patron detail page.
 - Set the "SuspendHoldsIntranet" system preference to "Don't allow."
 - Test the checkout and detail pages again and confirm that the
   "Suspend" column does not appear.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 3 Katrin Fischer 2020-01-19 21:41:32 UTC
Created attachment 97590 [details] [review]
Bug 21296: suspend hold ignores system preference on intranet

This patch modifies the DataTable configuration for the table of holds
which displays under the "Holds" tab on the patron's checkout and
details pages. A variable is now defined in the JavaScript with the
value of the "SuspendHoldsIntranet" system preference. This variable is
passed to the DataTable configuration to determine whether the column is
visible or not.

One of the changes made in this patch moves a <script> block so that it
appears before some of the JS includes on the page. This helps keep the
string-defining JS together in one block.

To test, apply the patch and set the "SuspendHoldsIntranet" system
preference to "Allow."

 - Open the checkout page for a patron with one or more holds.
 - Under the "Holds" tab, the table of holds should have a "Suspend"
   column with controls for suspending or resuming holds.
 - Check that the same is true on the patron detail page.
 - Set the "SuspendHoldsIntranet" system preference to "Don't allow."
 - Test the checkout and detail pages again and confirm that the
   "Suspend" column does not appear.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 4 Martin Renvoize 2020-01-20 14:04:15 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 5 Joy Nelson 2020-01-31 23:46:11 UTC
This applies to 19.11. branch, but qa tests show this error:

Processing files before patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 287.

Processing files after patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 287.


Not backported unless these are not major errors.

-joy
Comment 6 Joy Nelson 2020-02-08 00:14:32 UTC
QA tests all show OK, except for nonnumeric error. 
Pushing to 19.11.x for 19.11.03
Comment 7 Lucas Gass 2020-02-13 21:26:03 UTC
backported to 19.05.x for 19.05.08
Comment 8 Hayley Pelham 2020-03-16 00:10:32 UTC
Backported to 18.11.x for 18.11.15
Comment 9 Hayley Pelham 2020-03-16 00:56:10 UTC
This patch in the 18.11 build has caused two tests to fail:

_db_dependent_selenium_regressions_t.Display circulation table correctly (476)
t_db_dependent_Circulation_t.AddReturn + suspension_chargeperiod (500)

Is anyone able to fix these tests for 18.11? Alternatively I can revert the patches for now.
Comment 10 Jonathan Druart 2020-03-16 15:54:46 UTC
(In reply to Hayley Mapley from comment #9)
> This patch in the 18.11 build has caused two tests to fail:
> 
> _db_dependent_selenium_regressions_t.Display circulation table correctly
> (476)
> t_db_dependent_Circulation_t.AddReturn + suspension_chargeperiod (500)
> 
> Is anyone able to fix these tests for 18.11? Alternatively I can revert the
> patches for now.

Hi Hayley,
I do recreate the selenium failure locally, not Circulation.t
The checkouts table does not display correctly because of a missing columns_settings variable.

The following commit is part of 19.05 but 18.11:
  commit f6599b2c5480d2e096bac71e4e7ac759686b9fbc
  Bug 17353: Add phone number column to checkout search

 
-        columns_settings = [% ColumnsSettings.GetColumns( 'circ', 'circulation', 'issues-table', 'json' ) | $raw %]
+        columns_settings_issues_table = [% ColumnsSettings.GetColumns( 'circ', 'circulation', 'issues-table', 'json' ) | $raw %]
+        columns_settings_borrowers_table = [% ColumnsSettings.GetColumns( 'circ', 'circulation', 'table_borrowers', 'json' ) | $raw %]
Comment 11 Jonathan Druart 2020-03-16 15:57:34 UTC
Created attachment 100765 [details] [review]
Bug 21296: [18.11.x] Fix wrong resolution conflict

On
  commit a230878b6f3d3afa5d7f33c9d816bd6272c5b1a1
  Bug 21296: suspend hold ignores system preference on intranet

See comment 10.
Comment 12 Hayley Pelham 2020-03-16 22:01:54 UTC
Many thanks Jonathan!

I must have removed it in error during the merge.

Cheers,
Hayley