Description
Nicole C. Engard
2011-10-25 14:23:17 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. 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. AllowRenewalLimitOverride should only affect the renewals which are prevented because of the renewal limit, not because of the hold status. Shouldn't it? 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 (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. Okey dokey. Enhancement request then. Reopening as an enhancement. Most people seem to think that an override should be an override and that means even items on hold. 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 I got asked about this again today, this is still a desired enhancement. 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. Created attachment 79689 [details] [review] Bug 7088: Add AllowRenewalOnHoldOverride syspref Enables the renewal of on hold items with specified due dates to be turned on/off Created attachment 79690 [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 Created attachment 79691 [details] [review] Bug 7088: DO NOT PUSH - Compiled CSS This is never going to merge, but I'm including it as I'm not sure whether we should or not. 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 Created attachment 82274 [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. Created attachment 83004 [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 Created attachment 83005 [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 Created attachment 83006 [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 Comment on attachment 79691 [details] [review] Bug 7088: DO NOT PUSH - Compiled CSS Obsoleting this, I can't see any way in which it's helpful. 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> 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> 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> 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 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. 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 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 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. (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! 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. 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. 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 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> 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> 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> 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 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 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. 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 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? 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. (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. 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. (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? (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? (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. 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. 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. 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 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 (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 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 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. 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. (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? 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 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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> 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! 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 :) 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> 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> 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> 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> 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> 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> 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> 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> 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> Awesome work all! Pushed to master for 19.05 Enhancement will not be backported to 18.11.x series. |