Bug 15494 - Block renewals by arbitrary item values
Summary: Block renewals by arbitrary item values
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL: http://devs.bywatersolutions.com/proj...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-06 16:27 UTC by Nicole C. Engard
Modified: 2017-10-14 23:10 UTC (History)
5 users (show)

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


Attachments
Bug 15494 - Unit tests (5.62 KB, patch)
2017-05-24 14:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15494 - Add _items_denied_renewal sub to C4:Circulation::CanBookBeRenewed (2.20 KB, patch)
2017-05-24 14:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15494 - Update display of renewal errors (7.03 KB, patch)
2017-05-24 14:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15494 - Add syspref (3.60 KB, patch)
2017-05-24 14:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15494 - Unit tests (5.66 KB, patch)
2017-10-10 16:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15494 - Add _items_denied_renewal sub to C4:Circulation::CanBookBeRenewed (2.19 KB, patch)
2017-10-10 16:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15494 - Update display of renewal errors (7.08 KB, patch)
2017-10-10 16:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15494 - Add syspref (3.60 KB, patch)
2017-10-10 16:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15494 - Unit tests (5.71 KB, patch)
2017-10-14 23:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15494 - Add _items_denied_renewal sub to C4:Circulation::CanBookBeRenewed (2.24 KB, patch)
2017-10-14 23:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15494 - Update display of renewal errors (7.14 KB, patch)
2017-10-14 23:10 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15494 - Add syspref (3.66 KB, patch)
2017-10-14 23:10 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2016-01-06 16:27:14 UTC
This development would allow a library to prevent the renewal of items based on the values of any field in the items table including but not limited to any itemlost value.

Work to be done:
1) Add new system preference ItemsDeniedRenewal patterned after OpacHiddenItems
2) Modify C4::Circulation::CanBookBeRenewed to check this system preference


Seeking Co-sponsors: http://devs.bywatersolutions.com/projects/make-longoverduelost-items-not-renewable/
Comment 1 Nick Clemens 2017-05-24 14:46:16 UTC
Created attachment 63700 [details] [review]
Bug 15494 - Unit tests

prove t/db_dependent/Circulation.t
Comment 2 Nick Clemens 2017-05-24 14:46:19 UTC
Created attachment 63701 [details] [review]
Bug 15494 - Add _items_denied_renewal sub to C4:Circulation::CanBookBeRenewed
Comment 3 Nick Clemens 2017-05-24 14:46:22 UTC
Created attachment 63702 [details] [review]
Bug 15494 - Update display of renewal errors

To test:
1 - Define a rule to deny renewal
2 - Checkout an item meeting this rule
3 - Attempt renew an item via the OPAC
4 - Attempt renewal via SIP
5 - Attempt renewal via staff client
6 - Verify errors are reasonable
Comment 4 Nick Clemens 2017-05-24 14:46:25 UTC
Created attachment 63703 [details] [review]
Bug 15494 - Add syspref
Comment 5 Katrin Fischer 2017-10-08 09:51:08 UTC
Code looks good, but there is a bigger conflict in the test file I am not sure about, can you please rebase? 

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15494 - Unit tests
Using index info to reconstruct a base tree...
M	t/db_dependent/Circulation.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Circulation.t
CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t
error: Failed to merge in the changes.
Patch failed at 0001 Bug 15494 - Unit tests
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-15494---Unit-tests-pu3VeI.patch
Comment 6 Nick Clemens 2017-10-10 16:35:30 UTC
Created attachment 67886 [details] [review]
Bug 15494 - Unit tests

prove t/db_dependent/Circulation.t
Comment 7 Nick Clemens 2017-10-10 16:35:33 UTC
Created attachment 67887 [details] [review]
Bug 15494 - Add _items_denied_renewal sub to C4:Circulation::CanBookBeRenewed
Comment 8 Nick Clemens 2017-10-10 16:35:36 UTC
Created attachment 67888 [details] [review]
Bug 15494 - Update display of renewal errors

To test:
1 - Define a rule to deny renewal
2 - Checkout an item meeting this rule
3 - Attempt renew an item via the OPAC
4 - Attempt renewal via SIP
5 - Attempt renewal via staff client
6 - Verify errors are reasonable
Comment 9 Nick Clemens 2017-10-10 16:35:39 UTC
Created attachment 67889 [details] [review]
Bug 15494 - Add syspref
Comment 10 Katrin Fischer 2017-10-14 23:09:20 UTC
This seems to work quite well! Also tested renew through checkout, renewal tab and OPAC.

When renewing the item from the list of checkouts and using the renewal limit override functionality, the message displayed is:

Renew failed: item_denied_renewal

This could be improved, maybe with a small follow-up?
Comment 11 Katrin Fischer 2017-10-14 23:09:52 UTC
Created attachment 68146 [details] [review]
Bug 15494 - Unit tests

prove t/db_dependent/Circulation.t

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2017-10-14 23:09:56 UTC
Created attachment 68147 [details] [review]
Bug 15494 - Add _items_denied_renewal sub to C4:Circulation::CanBookBeRenewed

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2017-10-14 23:10:00 UTC
Created attachment 68148 [details] [review]
Bug 15494 - Update display of renewal errors

To test:
1 - Define a rule to deny renewal
2 - Checkout an item meeting this rule
3 - Attempt renew an item via the OPAC
4 - Attempt renewal via SIP
5 - Attempt renewal via staff client
6 - Verify errors are reasonable

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2017-10-14 23:10:04 UTC
Created attachment 68149 [details] [review]
Bug 15494 - Add syspref

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>