Bug 7088

Summary: Cannot renew items on hold even with override
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: CirculationAssignee: Andrew Isherwood <bugzilla>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: david, fiona.borthwick, gmcharlt, katrin.fischer, leonardobareno, marjorie.barry-vila, martin.renvoize, mathsabypro, nick, oleonard, sally.healey, tomascohen, wizzyrea
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23551
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement enables items that are on hold to be renewed with a due date specified by the user. It is enabled by the new "AllowRenewalOnHoldOverride" system preference. It can appear in two locations: 1. In the "Checkouts" table on the Patron Details screen. It is now possible to select on loan items that would otherwise fulfil a hold request to be renewed. When such an item is selected, an additional date selection box is displayed to allow the user to specify the due date for all on hold items that are to be renewed. 2. In the Circulation > Renew alert screen. When a barcode of an on loan item that would ordinarily fulfil a hold request is entered, the usual alert is displayed indicating that the item is on hold, it is still possible to override this, and renew, however it is now also possible to specify a due date.
Version(s) released in:
19.05.00
Bug Depends on:    
Bug Blocks: 25999, 23551    
Attachments: Bug 7088: Add AllowRenewalOnHoldOverride syspref
Bug 7088: Allow renew on hold items with due date
Bug 7088: DO NOT PUSH - Compiled CSS
Bug 7088: (follow-up) Fix renewal limit bug
Bug 7088: Add AllowRenewalOnHoldOverride syspref
Bug 7088: Allow renew on hold items with due date
Bug 7088: (follow-up) Fix renewal limit bug
Bug 7088: Add AllowRenewalOnHoldOverride syspref
Bug 7088: Allow renew on hold items with due date
Bug 7088: (follow-up) Fix renewal limit bug
Bug 7088: (follow-up) Remove script "type" attribute
Bug 7088: (follow-up) Set syspref default to off
Bug 7088: (follow-up) Default hold checkbox display
Bug 7088: Add AllowRenewalOnHoldOverride syspref
Bug 7088: Allow renew on hold items with due date
Bug 7088: (follow-up) Fix renewal limit bug
Bug 7088: (follow-up) Remove script "type" attribute
Bug 7088: (follow-up) Set syspref default to off
Bug 7088: (follow-up) Default hold checkbox display
Bug 7088: (follow-up) Add pref to checkbox
Bug 7088: (follow-up) AllowRenewalLimitOverride fix
Bug 7088: (follow-up) Modify checkbox enabling
Bug 7088: Add AllowRenewalOnHoldOverride syspref
Bug 7088: Allow renew on hold items with due date
Bug 7088: (follow-up) Fix renewal limit bug
Bug 7088: (follow-up) Remove script "type" attribute
Bug 7088: (follow-up) Set syspref default to off
Bug 7088: (follow-up) Default hold checkbox display
Bug 7088: (follow-up) Add pref to checkbox
Bug 7088: (follow-up) AllowRenewalLimitOverride fix
Bug 7088: (follow-up) Modify checkbox enabling
Bug 7088: Add AllowRenewalOnHoldOverride syspref
Bug 7088: Allow renew on hold items with due date
Bug 7088: (follow-up) Fix renewal limit bug
Bug 7088: (follow-up) Remove script "type" attribute
Bug 7088: (follow-up) Set syspref default to off
Bug 7088: (follow-up) Default hold checkbox display
Bug 7088: (follow-up) Add pref to checkbox
Bug 7088: (follow-up) AllowRenewalLimitOverride fix
Bug 7088: (follow-up) Modify checkbox enabling
Bug 7088: Add AllowRenewalOnHoldOverride syspref
Bug 7088: Allow renew on hold items with due date
Bug 7088: (follow-up) Fix renewal limit bug
Bug 7088: (follow-up) Remove script "type" attribute
Bug 7088: (follow-up) Set syspref default to off
Bug 7088: (follow-up) Default hold checkbox display
Bug 7088: (follow-up) Add pref to checkbox
Bug 7088: (follow-up) AllowRenewalLimitOverride fix
Bug 7088: (follow-up) Modify checkbox enabling

Description Nicole C. Engard 2011-10-25 14:23:17 UTC
I have AllowRenewalLimitOverride  set to allow and the renew checkbox is always there even if i don't click the override checkbox at the bottom and if the book is on hold it never ever gives me the checkbox to renew.
Comment 1 Nicole C. Engard 2011-10-25 14:27:20 UTC
with the preference set to 'don't allow' it still allows me to renew items that have 0 renewals left - but not items on hold.
Comment 2 Nicole C. Engard 2011-10-25 14:30:33 UTC
I was reading things a big wrong - i thought the 0 (zero) was saying no renewals allowed - but it was saying renewed 0 times 

The bug is that I can't renew an item that's on hold even if I'm allowing renewal overrides.
Comment 3 Owen Leonard 2011-10-25 15:51:40 UTC
AllowRenewalLimitOverride should only affect the renewals which are prevented because of the renewal limit, not because of the hold status. Shouldn't it?
Comment 4 Nicole C. Engard 2011-10-26 11:54:21 UTC
If I'm going to let my staff override renewals then they should be able to override renewals - regardless of why the renewal is blocked. I'm pretty sure it used to work that way.

Nicole
Comment 5 Owen Leonard 2011-10-26 12:31:35 UTC
(In reply to comment #4)
> I'm pretty sure it used to work that way.

I rolled back git again and again looking for a time when it worked that way and couldn't find it. The "RenewalLimit" part of "AllowRenewalLimitOverride" suggests to me that the preference is specifically targeted towards the renewal *limit*, not all renewal overrides. I think we need a new preference (and staff permission) to accomplish this.
Comment 6 Nicole C. Engard 2011-10-26 12:37:33 UTC
Okey dokey. Enhancement request then.
Comment 7 Nicole C. Engard 2012-10-16 20:34:51 UTC
Reopening as an enhancement. Most people seem to think that an override should be an override and that means even items on hold.
Comment 8 Mathieu Saby 2012-10-17 17:00:47 UTC
Hi
We discovered this problem some days ago, but I have just find this bug in BZ.
As a librarian I completely agree with Nicole Engard : an override should be an override...

M. Saby
Rennes 2 University
Comment 9 Nicole C. Engard 2013-02-14 01:49:13 UTC
I got asked about this again today, this is still a desired enhancement.
Comment 10 Sally 2016-08-18 09:34:55 UTC
This would be a nice feature to add to Koha.

As it's an enhancement, I think it's worth asking:  could we also consider a system preference to have a different circ period if the item is required by another patron?

For instance, standard circ period is 21 days.  
Item is on hold - renewal only gives a new circ period of 7 days.
Comment 11 Andrew Isherwood 2018-10-01 07:43:03 UTC Comment hidden (obsolete)
Comment 12 Andrew Isherwood 2018-10-01 07:43:07 UTC Comment hidden (obsolete)
Comment 13 Andrew Isherwood 2018-10-01 07:43:12 UTC Comment hidden (obsolete)
Comment 14 Andrew Isherwood 2018-10-11 11:45:53 UTC
Test plan:

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it is NOT possible to select this item
- Enable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it IS possible to select this item
- Select the item
- TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
- TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
- TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
- TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
- TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified
Comment 15 Andrew Isherwood 2018-11-13 14:00:16 UTC Comment hidden (obsolete)
Comment 16 Andrew Isherwood 2018-12-10 09:59:51 UTC Comment hidden (obsolete)
Comment 17 Andrew Isherwood 2018-12-10 09:59:55 UTC Comment hidden (obsolete)
Comment 18 Andrew Isherwood 2018-12-10 09:59:59 UTC Comment hidden (obsolete)
Comment 19 Andrew Isherwood 2018-12-10 10:00:56 UTC Comment hidden (obsolete)
Comment 20 PTFS Europe Sandboxes 2018-12-12 10:23:18 UTC
Created attachment 83092 [details] [review]
Bug 7088: Add AllowRenewalOnHoldOverride syspref

Enables the renewal of on hold items with specified due dates to be turned on/off

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>
Comment 21 PTFS Europe Sandboxes 2018-12-12 10:23:24 UTC
Created attachment 83093 [details] [review]
Bug 7088: Allow renew on hold items with due date

This patch adds the ability for items that are on hold to be renewed with a due date specfied by the user. It is enabled by the new "AllowRenewalOnHoldOverride" syspref. It is manifested in two locations:

1. In the "Checkouts" table on the Patron Details screen. It is now possible to select on loan items that would otherwise fulfil a hold request to be renewed. When such an item is selected, an additional date selection box is displayed to allow the user to specify the due date for all on hold items that are to be renewed.

2. In the Circulation > Renew alert screen. When a barcode of an on loan item that would ordinarily fulfil a hold request is entered, the usual alert is displayed indicating that the item is on hold, it is still possible to override this, and renew, however it is now also possible to specify a due date.

Test plan:

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it is NOT possible to select this item
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it IS possible to select this item
- Select the item
- TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
- TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
- TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
- TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
- TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>
Comment 22 PTFS Europe Sandboxes 2018-12-12 10:23:30 UTC
Created attachment 83094 [details] [review]
Bug 7088: (follow-up) Fix renewal limit bug

There was a bug in that altering the state of the "Override renewal
limit" checkbox was altering the appearance of the per-item "On hold"
checkbox. This is incorrect, the "Override renewal limit" checkbox is
unconnected with this bug.

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>
Comment 23 Andrew Isherwood 2018-12-12 10:24:31 UTC
Test plan:

- Apply bug
- Rebuild CSS
- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it is NOT possible to select this item
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it IS possible to select this item
- Select the item
- TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
- TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
- TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
- TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
- TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified
Comment 24 Katrin Fischer 2019-01-27 13:31:54 UTC
Hi Andrew,

some first QA notes here:

1) The QA script fails, easy to fix:
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt
   OK	  filters
   FAIL	  forbidden patterns
		forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 206)

2) We don't want to change the current behaviour for updating installations, so the feature should be set to of in the database update. I think since all the other override features also default to "no", I'd also make it no for new installations.

3) I don't understand the new entry form for the hold due date. My feeling is that we should use the one existing already for entering the new due date for any override. Having 2 forms seems confusing.

4) I'd also have expected the renewal to be locked initially and the checkbox to unlock it. Reasoning: Renewing an item with a hold should always be an exception. The way it's now a "renew all" checked on top will also check the "on hold" ones which might be easily missed. We could relabel the checkbox and use it for both: Allow override of renewal restrictions?

Last 2 are up for discussion of course. Feel free to seek an additional opinion.
Comment 25 Andrew Isherwood 2019-01-30 11:04:08 UTC
Created attachment 84509 [details] [review]
Bug 7088: (follow-up) Remove script "type" attribute

As per item 1 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24
Comment 26 Andrew Isherwood 2019-01-30 11:06:11 UTC
Created attachment 84510 [details] [review]
Bug 7088: (follow-up) Set syspref default to off

As per item 2 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24
Comment 27 Andrew Isherwood 2019-02-07 15:44:42 UTC
Created attachment 84850 [details] [review]
Bug 7088: (follow-up) Default hold checkbox display

As per item 4 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Now, the checkbox for renewing on hold items behaves the same way as the
checkbox for renewing items that have reached their renew limit. It is
necessary to check the re-labelled "Override renewal restrictions"
checkbox before it is possible to select on-hold items for renewal.
Comment 28 Andrew Isherwood 2019-02-07 15:55:26 UTC
(In reply to Katrin Fischer from comment #24)

> 4) I'd also have expected the renewal to be locked initially and the
> checkbox to unlock it. Reasoning: Renewing an item with a hold should always
> be an exception. The way it's now a "renew all" checked on top will also
> check the "on hold" ones which might be easily missed. We could relabel the
> checkbox and use it for both: Allow override of renewal restrictions?

Hi Katrin

I've just attached a patch that addresses this. The on hold items now behave in the same way as items that have reached their renewal limit, they are not selectable until the checkbox is checked. Speaking of which, I've re-labelled the checkbox from "Override renewal limit" to "Override renewal restrictions" as per you suggestion. So now, "Select all" and "Renew all" will not act upon the on hold items unless the override is checked.

I'm not familiar with the process when a UI label is changed, is there any maintenance work that needs to happen so translations don't break?

>3) I don't understand the new entry form for the hold due date. My feeling is that we should use the one existing already for entering the new due date for any override. Having 2 forms seems confusing.

We discussed this internally at great length when speccing out the development. It was felt by both our customers and us that having two date selection boxes made most sense since it's conceivable that the user may wish to have different due dates for on hold items and non-on hold items. The fact that the "On hold due date" date input doesn't appear until an on hold item is checked for renewal (if you're not seeing this behaviour, rebuild your CSS) reinforces it's purpose.

I'm going to set this back to Signed off as I believe it's ready for another QA run :) Thanks for your feedback!
Comment 29 Katrin Fischer 2019-02-20 07:03:57 UTC
Hi Andrew,

I think we need to work on the visibility of the checkbox and maybe the logic of triggering things with it now (sorry, I think that makes things a bit more complicated:

                 <fieldset class="action">
                     [% IF ( CAN_user_circulate_override_renewals ) %]
                         [% IF CAN_user_circulate_override_renewals && Koha.Preference( 'AllowRenewalLimitOverride' ) %]
-                            <label for="override_limit">Override renewal limit:</label>
+                            <label for="override_limit">Override renewal restrictions:</label>

You will want to add your new pref there with an OR condition there I think Another option might be to have && and a separate label if only hold override is allowed?

I am still not sure about the second date box - I'd like to get some opinions on it. Adding Owen and Liz for now, but looking for any librarians opinion really.
Comment 30 Andrew Isherwood 2019-02-20 09:16:03 UTC
Thanks for the feedback regarding the pref Katrin, good spot, I'd completely missed that, I'll create a patch to sort it out shortly.

As far as the second date selector for on hold due dates is concerned, it may be useful to know that this development was specced and has been accepted by three of our customers, all of whom comprise librarians, they are all happy with a second date selector that only appears when an on hold item is selected for renewal. I'll try and get them to comment on this bug if that would be helpful.
Comment 31 Liz Rea 2019-02-20 23:01:51 UTC
Just a note for testers, if you are testing as I was, with an item that has multiple items that are not all checked out, you will need  AllowRenewalIfOtherItemsAvailable turned on to see the checkbox.

Liz
Comment 32 Andrew Isherwood 2019-02-21 12:23:54 UTC
Created attachment 85434 [details] [review]
Bug 7088: Add AllowRenewalOnHoldOverride syspref

Enables the renewal of on hold items with specified due dates to be turned on/off

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>
Comment 33 Andrew Isherwood 2019-02-21 12:23:58 UTC
Created attachment 85435 [details] [review]
Bug 7088: Allow renew on hold items with due date

This patch adds the ability for items that are on hold to be renewed with a due date specfied by the user. It is enabled by the new "AllowRenewalOnHoldOverride" syspref. It is manifested in two locations:

1. In the "Checkouts" table on the Patron Details screen. It is now possible to select on loan items that would otherwise fulfil a hold request to be renewed. When such an item is selected, an additional date selection box is displayed to allow the user to specify the due date for all on hold items that are to be renewed.

2. In the Circulation > Renew alert screen. When a barcode of an on loan item that would ordinarily fulfil a hold request is entered, the usual alert is displayed indicating that the item is on hold, it is still possible to override this, and renew, however it is now also possible to specify a due date.

Test plan:

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it is NOT possible to select this item
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it IS possible to select this item
- Select the item
- TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
- TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
- TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
- TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
- TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>
Comment 34 Andrew Isherwood 2019-02-21 12:24:02 UTC
Created attachment 85436 [details] [review]
Bug 7088: (follow-up) Fix renewal limit bug

There was a bug in that altering the state of the "Override renewal
limit" checkbox was altering the appearance of the per-item "On hold"
checkbox. This is incorrect, the "Override renewal limit" checkbox is
unconnected with this bug.

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>
Comment 35 Andrew Isherwood 2019-02-21 12:24:06 UTC
Created attachment 85437 [details] [review]
Bug 7088: (follow-up) Remove script "type" attribute

As per item 1 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24
Comment 36 Andrew Isherwood 2019-02-21 12:24:10 UTC
Created attachment 85438 [details] [review]
Bug 7088: (follow-up) Set syspref default to off

As per item 2 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24
Comment 37 Andrew Isherwood 2019-02-21 12:24:14 UTC
Created attachment 85439 [details] [review]
Bug 7088: (follow-up) Default hold checkbox display

As per item 4 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Now, the checkbox for renewing on hold items behaves the same way as the
checkbox for renewing items that have reached their renew limit. It is
necessary to check the re-labelled "Override renewal restrictions"
checkbox before it is possible to select on-hold items for renewal.
Comment 38 Andrew Isherwood 2019-02-21 12:24:18 UTC
Created attachment 85440 [details] [review]
Bug 7088: (follow-up) Add pref to checkbox

As per commit #29 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c29
Comment 39 Liz Rea 2019-03-01 16:50:07 UTC
I personally don't like the 2nd date picker and think it's confusing, at least how it is labelled. There is something that I find ambiguous about "On hold due date" but I'm having a hard time putting what it is into words.

Maybe a hover text on the label that explains more what the box is for?
Comment 40 Owen Leonard 2019-03-07 16:43:27 UTC
What about changing the label to "Due date for on hold items" ?

I think having the second date picker is something which looks to me like clutter but I assume that those who want to use the feature will appreciate it.

I do have one issue: The new date picker should be hidden initially, correct? For me it isn't.
Comment 41 Owen Leonard 2019-03-07 16:50:30 UTC
(In reply to Owen Leonard from comment #40)
> I do have one issue: The new date picker should be hidden initially,
> correct? For me it isn't.

Sorry, my mistake -- I didn't rebuild CSS. I think this can be put back to "Signed off" unless someone wants to suggest a label change.
Comment 42 Katrin Fischer 2019-04-10 13:40:19 UTC
Hi Andrew,

sorry, we are almost there. Most things worked really well (like checkbox visibility), but found one case that didn't work as expected:

HoldOverride = Allow
LimitOverride = Not allowed

Item is on hold, 2 of 2 renewals remaining.
Staff user has override permission.

But: Checking the checkbox doesn't make the item renewable.
Comment 43 Andrew Isherwood 2019-04-17 13:37:11 UTC
(In reply to Katrin Fischer from comment #42)
> Hi Andrew,
> 
> sorry, we are almost there. Most things worked really well (like checkbox
> visibility), but found one case that didn't work as expected:
> 
> HoldOverride = Allow
> LimitOverride = Not allowed
> 
> Item is on hold, 2 of 2 renewals remaining.
> Staff user has override permission.
> 
> But: Checking the checkbox doesn't make the item renewable.

Hi Katrin,

The test plan specifies that LimitOverride needs to be enabled for the functionality to work. If it's disabled the click listener on the override checkbox isn't created, which explains why it doesn't do anything. This isn't something that has been modified in my patches, it's pre-existing Koha functionality. Are you expecting something different?
Comment 44 Katrin Fischer 2019-04-17 13:45:12 UTC
(In reply to Andrew Isherwood from comment #43)
> (In reply to Katrin Fischer from comment #42)
> > Hi Andrew,
> > 
> > sorry, we are almost there. Most things worked really well (like checkbox
> > visibility), but found one case that didn't work as expected:
> > 
> > HoldOverride = Allow
> > LimitOverride = Not allowed
> > 
> > Item is on hold, 2 of 2 renewals remaining.
> > Staff user has override permission.
> > 
> > But: Checking the checkbox doesn't make the item renewable.
> 
> Hi Katrin,
> 
> The test plan specifies that LimitOverride needs to be enabled for the
> functionality to work. If it's disabled the click listener on the override
> checkbox isn't created, which explains why it doesn't do anything. This
> isn't something that has been modified in my patches, it's pre-existing Koha
> functionality. Are you expecting something different?

Hm, maybe the checkbox should now shop up in this case then? Right now the checkbox just appears to have no effect - and if I understand you correctly that's the expected behaviour as both need to be allowed to work?
Comment 45 Andrew Isherwood 2019-04-23 09:33:38 UTC
(In reply to Katrin Fischer from comment #44)
> Hm, maybe the checkbox should now shop up in this case then? Right now the
> checkbox just appears to have no effect - and if I understand you correctly
> that's the expected behaviour as both need to be allowed to work?

Actually, looking again, I think this is a misunderstanding on my part. The "Override renewal restrictions" checkbox is displayed if the user has AllowRenewalLimitOverride OR AllowRenewalOnHoldOverride. But the click listener is only created if the user has AllowRenewalLimitOverride, which I think is incorrect. I think it needs adapting so it is created if the user has AllowRenewalLimitOverride OR AllowRenewalOnHoldOverride.

I'll create a patch to do that now.
Comment 46 Andrew Isherwood 2019-04-23 09:37:14 UTC
Sorry, my comment above is incorrect. The click event listener is currently only created if the AllowRenewalLimitOverride syspref is turned on, I think it needs adapting so it is created if the AllowRenewalLimitOverride OR AllowRenewalOnHoldOverride sysprefs are turned on. The checkbox will then be displayed if the user has circulate_override_renewals AND either of those sysprefs are enabled.
Comment 47 Andrew Isherwood 2019-04-23 09:47:41 UTC
Created attachment 88446 [details] [review]
Bug 7088: (follow-up) AllowRenewalLimitOverride fix

This patch causes the click event listener on the #override_limit to be
created if EITHER the AllowRenewalOnHoldOverride OR
AllowRenewalLimitOverride sysprefs are enabled. Previously, it would
only be created if the AllowRenewalLimitOverride syspref was enabled,
which would cause the checkbox to be displayed but do nothing.
Comment 48 Andrew Isherwood 2019-04-23 10:02:45 UTC
Hi Katrin

Now I've added that patch, the checkbox will be displayed and be functional if the user has "circulate_override_renewals" and EITHER "AllowRenewalLimitOverride" OR "AllowRenewalOnHoldOverride" are enabled.

Consequently, the test plan is now slightly incorrect, revised version below:

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it is NOT possible to select this item
- ** Carry out all following steps, first with the "AllowRenewalLimitOverride" syspref enabled, then with the "AllowRenewalOnHoldOverride" syspref enabled **

- Return to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- Check the "Override renewal restrictions" checkbox
- TEST: Observe that it IS possible to select this item
- Select the item
- TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
- TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
- TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
- TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
- TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified
Comment 49 Katrin Fischer 2019-04-24 20:40:38 UTC
Hi Andrew,

I am very sorry, not sure the last patch is quite right. I think what happens now is that either will make 'all' checkboxes appear, if the item is on hold or not. I think we need to control the checkboxes separately, if that makes sense?

Test case:
Have at least 2 items checked out, 1 is on hold.

AllowRenewalOnHoldOverride
AllowRenewalLimitOverride
 
- Restriction override checkbox below table appears - OK
- Checking the box, enables the on hold item for renewal - OK
- Checking the box also enabled all other items for renewal - not OK
Comment 50 Katrin Fischer 2019-04-26 09:17:47 UTC
(In reply to Katrin Fischer from comment #49)
> Hi Andrew,
> 
> I am very sorry, not sure the last patch is quite right. I think what
> happens now is that either will make 'all' checkboxes appear, if the item is
> on hold or not. I think we need to control the checkboxes separately, if
> that makes sense?
> 
> Test case:
> Have at least 2 items checked out, 1 is on hold.
> 
> AllowRenewalOnHoldOverride Allow
> AllowRenewalLimitOverride Don't allow
>  
> - Restriction override checkbox below table appears - OK
> - Checking the box, enables the on hold item for renewal - OK
> - Checking the box also enabled all other items for renewal - not OK
Comment 51 Andrew Isherwood 2019-04-26 10:10:19 UTC
Created attachment 88868 [details] [review]
Bug 7088: (follow-up) Modify checkbox enabling

As described in comment #50, we need to only enable renewal on items
that are disabled in line with which sysprefs are enabled. So we now
only enable renewal of on hold items if the AllowRenewalOnHoldOverride
syspref is on and we only enable renewal of all other limited items
when the AllowRenewalLimitOverride syspref is on.

NOTE: The test plan below has changed substantially from previous
iterations

Test plan (you might want to make a coffee first...):

- Apply patch
- Rebuild CSS

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- The patron also needs to have an item on loan that cannot be renewed for some other reason, say renewal limit has been reached
=> TEST: Observe that it is NOT possible to select either item for renewal

- Enable the "AllowRenewalLimitOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it IS possible to select the the item that has reached the renewal limit, but not the item that is on hold

- Disable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it is NOT possible to select the the item that has reached the renewal limit, but it IS possible to select the item on hold

- Enable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it IS possible to select the the item that has reached the renewal limit, AND it IS possible to select the item on hold

- Select the item that is on hold
=> TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
=> TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
=> TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
=> TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
=> TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
=> TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
=> TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
=> TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified
Comment 52 Katrin Fischer 2019-04-27 14:08:35 UTC
I am very very sorry :(

The checkbox behaviour looks better now, but the 'hold date' input is now always visible and not just appearing when needed.
Comment 53 Andrew Isherwood 2019-04-29 07:31:00 UTC
Hi Katrin - It sounds like you didn't rebuild the CSS. Can you check you did, the hold date input box was behaving correctly for me after I performed the last modifications.
Comment 54 Katrin Fischer 2019-04-29 07:35:10 UTC
(In reply to Andrew Isherwood from comment #53)
> Hi Katrin - It sounds like you didn't rebuild the CSS. Can you check you
> did, the hold date input box was behaving correctly for me after I performed
> the last modifications.

I thought I did, but can#t make sure before tonight. Can you please retest to make sure?
Comment 55 Andrew Isherwood 2019-04-29 09:42:03 UTC
Hi Katrin - I've re-tested and all seems fine to me. I recorded a video just to make sure you agree: https://nimb.ws/oMZJ7F
Comment 56 Liz Rea 2019-04-29 15:49:00 UTC
Created attachment 89026 [details] [review]
Bug 7088: Add AllowRenewalOnHoldOverride syspref

Enables the renewal of on hold items with specified due dates to be turned on/off

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 57 Liz Rea 2019-04-29 15:49:09 UTC
Created attachment 89027 [details] [review]
Bug 7088: Allow renew on hold items with due date

This patch adds the ability for items that are on hold to be renewed with a due date specfied by the user. It is enabled by the new "AllowRenewalOnHoldOverride" syspref. It is manifested in two locations:

1. In the "Checkouts" table on the Patron Details screen. It is now possible to select on loan items that would otherwise fulfil a hold request to be renewed. When such an item is selected, an additional date selection box is displayed to allow the user to specify the due date for all on hold items that are to be renewed.

2. In the Circulation > Renew alert screen. When a barcode of an on loan item that would ordinarily fulfil a hold request is entered, the usual alert is displayed indicating that the item is on hold, it is still possible to override this, and renew, however it is now also possible to specify a due date.

Test plan:

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it is NOT possible to select this item
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it IS possible to select this item
- Select the item
- TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
- TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
- TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
- TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
- TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 58 Liz Rea 2019-04-29 15:49:13 UTC
Created attachment 89028 [details] [review]
Bug 7088: (follow-up) Fix renewal limit bug

There was a bug in that altering the state of the "Override renewal
limit" checkbox was altering the appearance of the per-item "On hold"
checkbox. This is incorrect, the "Override renewal limit" checkbox is
unconnected with this bug.

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 59 Liz Rea 2019-04-29 15:49:16 UTC
Created attachment 89029 [details] [review]
Bug 7088: (follow-up) Remove script "type" attribute

As per item 1 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 60 Liz Rea 2019-04-29 15:49:20 UTC
Created attachment 89030 [details] [review]
Bug 7088: (follow-up) Set syspref default to off

As per item 2 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 61 Liz Rea 2019-04-29 15:49:25 UTC
Created attachment 89031 [details] [review]
Bug 7088: (follow-up) Default hold checkbox display

As per item 4 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Now, the checkbox for renewing on hold items behaves the same way as the
checkbox for renewing items that have reached their renew limit. It is
necessary to check the re-labelled "Override renewal restrictions"
checkbox before it is possible to select on-hold items for renewal.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 62 Liz Rea 2019-04-29 15:49:29 UTC
Created attachment 89032 [details] [review]
Bug 7088: (follow-up) Add pref to checkbox

As per commit #29 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c29

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 63 Liz Rea 2019-04-29 15:49:33 UTC
Created attachment 89033 [details] [review]
Bug 7088: (follow-up) AllowRenewalLimitOverride fix

This patch causes the click event listener on the #override_limit to be
created if EITHER the AllowRenewalOnHoldOverride OR
AllowRenewalLimitOverride sysprefs are enabled. Previously, it would
only be created if the AllowRenewalLimitOverride syspref was enabled,
which would cause the checkbox to be displayed but do nothing.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 64 Liz Rea 2019-04-29 15:49:37 UTC
Created attachment 89034 [details] [review]
Bug 7088: (follow-up) Modify checkbox enabling

As described in comment #50, we need to only enable renewal on items
that are disabled in line with which sysprefs are enabled. So we now
only enable renewal of on hold items if the AllowRenewalOnHoldOverride
syspref is on and we only enable renewal of all other limited items
when the AllowRenewalLimitOverride syspref is on.

NOTE: The test plan below has changed substantially from previous
iterations

Test plan (you might want to make a coffee first...):

- Apply patch
- Rebuild CSS

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- The patron also needs to have an item on loan that cannot be renewed for some other reason, say renewal limit has been reached
=> TEST: Observe that it is NOT possible to select either item for renewal

- Enable the "AllowRenewalLimitOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it IS possible to select the the item that has reached the renewal limit, but not the item that is on hold

- Disable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it is NOT possible to select the the item that has reached the renewal limit, but it IS possible to select the item on hold

- Enable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it IS possible to select the the item that has reached the renewal limit, AND it IS possible to select the item on hold

- Select the item that is on hold
=> TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
=> TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
=> TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
=> TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
=> TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
=> TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
=> TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
=> TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 65 Katrin Fischer 2019-04-29 19:29:44 UTC
Created attachment 89044 [details] [review]
Bug 7088: Add AllowRenewalOnHoldOverride syspref

Enables the renewal of on hold items with specified due dates to be turned on/off

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 66 Katrin Fischer 2019-04-29 19:29:50 UTC
Created attachment 89045 [details] [review]
Bug 7088: Allow renew on hold items with due date

This patch adds the ability for items that are on hold to be renewed with a due date specfied by the user. It is enabled by the new "AllowRenewalOnHoldOverride" syspref. It is manifested in two locations:

1. In the "Checkouts" table on the Patron Details screen. It is now possible to select on loan items that would otherwise fulfil a hold request to be renewed. When such an item is selected, an additional date selection box is displayed to allow the user to specify the due date for all on hold items that are to be renewed.

2. In the Circulation > Renew alert screen. When a barcode of an on loan item that would ordinarily fulfil a hold request is entered, the usual alert is displayed indicating that the item is on hold, it is still possible to override this, and renew, however it is now also possible to specify a due date.

Test plan:

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it is NOT possible to select this item
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it IS possible to select this item
- Select the item
- TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
- TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
- TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
- TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
- TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 67 Katrin Fischer 2019-04-29 19:29:55 UTC
Created attachment 89046 [details] [review]
Bug 7088: (follow-up) Fix renewal limit bug

There was a bug in that altering the state of the "Override renewal
limit" checkbox was altering the appearance of the per-item "On hold"
checkbox. This is incorrect, the "Override renewal limit" checkbox is
unconnected with this bug.

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 68 Katrin Fischer 2019-04-29 19:29:59 UTC
Created attachment 89047 [details] [review]
Bug 7088: (follow-up) Remove script "type" attribute

As per item 1 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 69 Katrin Fischer 2019-04-29 19:30:04 UTC
Created attachment 89048 [details] [review]
Bug 7088: (follow-up) Set syspref default to off

As per item 2 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 70 Katrin Fischer 2019-04-29 19:30:09 UTC
Created attachment 89049 [details] [review]
Bug 7088: (follow-up) Default hold checkbox display

As per item 4 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Now, the checkbox for renewing on hold items behaves the same way as the
checkbox for renewing items that have reached their renew limit. It is
necessary to check the re-labelled "Override renewal restrictions"
checkbox before it is possible to select on-hold items for renewal.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 71 Katrin Fischer 2019-04-29 19:30:14 UTC
Created attachment 89050 [details] [review]
Bug 7088: (follow-up) Add pref to checkbox

As per commit #29 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c29

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 72 Katrin Fischer 2019-04-29 19:30:19 UTC
Created attachment 89051 [details] [review]
Bug 7088: (follow-up) AllowRenewalLimitOverride fix

This patch causes the click event listener on the #override_limit to be
created if EITHER the AllowRenewalOnHoldOverride OR
AllowRenewalLimitOverride sysprefs are enabled. Previously, it would
only be created if the AllowRenewalLimitOverride syspref was enabled,
which would cause the checkbox to be displayed but do nothing.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 73 Katrin Fischer 2019-04-29 19:30:24 UTC
Created attachment 89052 [details] [review]
Bug 7088: (follow-up) Modify checkbox enabling

As described in comment #50, we need to only enable renewal on items
that are disabled in line with which sysprefs are enabled. So we now
only enable renewal of on hold items if the AllowRenewalOnHoldOverride
syspref is on and we only enable renewal of all other limited items
when the AllowRenewalLimitOverride syspref is on.

NOTE: The test plan below has changed substantially from previous
iterations

Test plan (you might want to make a coffee first...):

- Apply patch
- Rebuild CSS

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- The patron also needs to have an item on loan that cannot be renewed for some other reason, say renewal limit has been reached
=> TEST: Observe that it is NOT possible to select either item for renewal

- Enable the "AllowRenewalLimitOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it IS possible to select the the item that has reached the renewal limit, but not the item that is on hold

- Disable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it is NOT possible to select the the item that has reached the renewal limit, but it IS possible to select the item on hold

- Enable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it IS possible to select the the item that has reached the renewal limit, AND it IS possible to select the item on hold

- Select the item that is on hold
=> TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
=> TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
=> TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
=> TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
=> TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
=> TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
=> TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
=> TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 74 Katrin Fischer 2019-04-29 19:42:47 UTC
Hi Andrew, it might be my messed up rebase (changes from 21757 caused a conflict), but now the hold due date never shows up.

Test case:
- Hold renewals allowed
- Renewal limit overrides are not

2 items on my account, both are 'on hold'

Check the checkbox - checkboxes unlock
But: the date doesn't show up

I am sorry, I'll give it another go tomorrow!
Comment 75 Katrin Fischer 2019-04-30 05:44:24 UTC
I guess I was too sleepy last night - the date field shows up once you actually click on a hold to renew. I think it might be debatable to show it with clicking the checkbox, but really no reason at all to fail this :)
Comment 76 Katrin Fischer 2019-04-30 05:45:32 UTC
Created attachment 89086 [details] [review]
Bug 7088: Add AllowRenewalOnHoldOverride syspref

Enables the renewal of on hold items with specified due dates to be turned on/off

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 77 Katrin Fischer 2019-04-30 05:45:37 UTC
Created attachment 89087 [details] [review]
Bug 7088: Allow renew on hold items with due date

This patch adds the ability for items that are on hold to be renewed with a due date specfied by the user. It is enabled by the new "AllowRenewalOnHoldOverride" syspref. It is manifested in two locations:

1. In the "Checkouts" table on the Patron Details screen. It is now possible to select on loan items that would otherwise fulfil a hold request to be renewed. When such an item is selected, an additional date selection box is displayed to allow the user to specify the due date for all on hold items that are to be renewed.

2. In the Circulation > Renew alert screen. When a barcode of an on loan item that would ordinarily fulfil a hold request is entered, the usual alert is displayed indicating that the item is on hold, it is still possible to override this, and renew, however it is now also possible to specify a due date.

Test plan:

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it is NOT possible to select this item
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- TEST: Observe that it IS possible to select this item
- Select the item
- TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
- TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
- TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
- TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
- TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
- TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 78 Katrin Fischer 2019-04-30 05:45:42 UTC
Created attachment 89088 [details] [review]
Bug 7088: (follow-up) Fix renewal limit bug

There was a bug in that altering the state of the "Override renewal
limit" checkbox was altering the appearance of the per-item "On hold"
checkbox. This is incorrect, the "Override renewal limit" checkbox is
unconnected with this bug.

Sponsored-by: Cheshire Libraries Shared Services
Sponsored-by: Halton Borough Council
Sponsored-by: Sefton Council

Signed-off-by: Andrew Farthing <Andrew.Farthing@sefton.gov.uk>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 79 Katrin Fischer 2019-04-30 05:45:46 UTC
Created attachment 89089 [details] [review]
Bug 7088: (follow-up) Remove script "type" attribute

As per item 1 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 80 Katrin Fischer 2019-04-30 05:45:51 UTC
Created attachment 89090 [details] [review]
Bug 7088: (follow-up) Set syspref default to off

As per item 2 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 81 Katrin Fischer 2019-04-30 05:45:56 UTC
Created attachment 89091 [details] [review]
Bug 7088: (follow-up) Default hold checkbox display

As per item 4 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c24

Now, the checkbox for renewing on hold items behaves the same way as the
checkbox for renewing items that have reached their renew limit. It is
necessary to check the re-labelled "Override renewal restrictions"
checkbox before it is possible to select on-hold items for renewal.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 82 Katrin Fischer 2019-04-30 05:46:00 UTC
Created attachment 89092 [details] [review]
Bug 7088: (follow-up) Add pref to checkbox

As per commit #29 here:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7088#c29

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 83 Katrin Fischer 2019-04-30 05:46:05 UTC
Created attachment 89093 [details] [review]
Bug 7088: (follow-up) AllowRenewalLimitOverride fix

This patch causes the click event listener on the #override_limit to be
created if EITHER the AllowRenewalOnHoldOverride OR
AllowRenewalLimitOverride sysprefs are enabled. Previously, it would
only be created if the AllowRenewalLimitOverride syspref was enabled,
which would cause the checkbox to be displayed but do nothing.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 84 Katrin Fischer 2019-04-30 05:46:10 UTC
Created attachment 89094 [details] [review]
Bug 7088: (follow-up) Modify checkbox enabling

As described in comment #50, we need to only enable renewal on items
that are disabled in line with which sysprefs are enabled. So we now
only enable renewal of on hold items if the AllowRenewalOnHoldOverride
syspref is on and we only enable renewal of all other limited items
when the AllowRenewalLimitOverride syspref is on.

NOTE: The test plan below has changed substantially from previous
iterations

Test plan (you might want to make a coffee first...):

- Apply patch
- Rebuild CSS

- Go to the Patron Details page for a patron who has an item on loan that would fulfil an outstanding loan request.
- The patron also needs to have an item on loan that cannot be renewed for some other reason, say renewal limit has been reached
=> TEST: Observe that it is NOT possible to select either item for renewal

- Enable the "AllowRenewalLimitOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it IS possible to select the the item that has reached the renewal limit, but not the item that is on hold

- Disable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it is NOT possible to select the the item that has reached the renewal limit, but it IS possible to select the item on hold

- Enable the "AllowRenewalLimitOverride" syspref
- Enable the "AllowRenewalOnHoldOverride" syspref
- Return to the Patron Details page
- Check the "Override renewal restrictions" checkbox
=> TEST: Observe that it IS possible to select the the item that has reached the renewal limit, AND it IS possible to select the item on hold

- Select the item that is on hold
=> TEST: Observe that an additional "On hold due date" input box is displayed
- De-select the item
=> TEST: Observe that an additional "On hold due date" input box is hidden
- Select the item
- In the "On hold due date" input box, select a due date for the item
- Click "Renew or check in selected items"
=> TEST: Observe that the item is renewed as usual
- In the "Renewal due date" input box, select a due date
- Remove the contents of the "On hold due date" input box
- Click "Renew or check in selected items"
=> TEST: Observe that the item is renewed by falling back to the "Renewal due date" value if a value is not specified in the "On hold due date" input box
- Remove the contents of the "Renewal due date" input box
- Click "Renew or check in selected items"
=> TEST: Observe that the standard loan period is used for the renewal period if no due date is specified in either box
- In the "On hold due date" input box, select a due date for the item
- In the "Renewal due date" input box, select a different due date
- Click "Renew all"
=> TEST: Observe that all non on hold items are renewed using the value in "Renewal due date" and on hold items are renewed using the value in "On hold due date"
- From the main staff client from page, choose "Circulation", then choose "Renew"
- Enter the barcode of an item that you know to be on hold and submit
=> TEST: In the alert box that appears, observe that a date picker is
displayed
- Choose a due date for this item, then click "Override and renew"
=> TEST: In the "Item renewed" box, observe that the item has been
renewed to the date specified

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 85 Nick Clemens 2019-05-09 18:21:55 UTC
Awesome work all!

Pushed to master for 19.05
Comment 86 Martin Renvoize 2019-05-16 14:14:02 UTC
Enhancement will not be backported to 18.11.x series.