Bug 8361 - Issuing rule if no rule is defined
Summary: Issuing rule if no rule is defined
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 8362
  Show dependency treegraph
 
Reported: 2012-07-05 13:12 UTC by Paul Poulain
Modified: 2017-05-19 06:48 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
In the previous versions if no circulation rule was defined, Koha always allowed to check out. Now, with this development, Koha refuses check-out if no rule is found.


Attachments
Bug 8361: Do not allow checkouts if no rules are defined (1.99 KB, patch)
2016-11-22 11:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8361: Do not allow checkouts if no rules are defined (2.06 KB, patch)
2016-11-22 22:23 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 8361: Do not allow checkouts if no rules are defined (2.10 KB, patch)
2017-01-13 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 8361 (QA Followup) Add warnings (3.16 KB, patch)
2017-01-13 18:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 8361 (QA Followup) Add warnings (1.33 KB, patch)
2017-01-17 11:49 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 8361 (QA Followup) Add warnings (1.34 KB, patch)
2017-01-17 12:02 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Poulain 2012-07-05 13:12:03 UTC
If we reach a case where no issuing rule is defined, Koha always grant issuing. Koha should refuse any check-out if no rule is found

(validating the issuing is silent, while rejecting the check-out will clearly show that something is missing in your configuration)
Comment 1 Jonathan Druart 2015-03-31 13:49:03 UTC
Is this still an issue?
Why should we refuse checkouts if no rule is defined?
Comment 2 Christopher Brannon 2016-04-06 14:15:12 UTC
(In reply to Jonathan Druart from comment #1)
> Is this still an issue?
> Why should we refuse checkouts if no rule is defined?

Because that is expected behavior.  If I don't put gas in the car, I don't expect it to still drive.  There are basic components that are just required.

I guess we could make Koha ask questions when certain things are missing, like when the item is due.  If that were the only piece missing, that would be fine.  But if there are no circ rules defined, chances are there is a lot more missing in setup.
Comment 3 Jonathan Druart 2016-04-06 14:24:57 UTC
(In reply to Christopher Brannon from comment #2)
> (In reply to Jonathan Druart from comment #1)
> > Is this still an issue?
> > Why should we refuse checkouts if no rule is defined?
> 
> Because that is expected behavior.  If I don't put gas in the car, I don't
> expect it to still drive.  There are basic components that are just required.

Yes exactly, so do not expect Koha to behave correctly if you did not configure it correctly :)
Comment 4 Jonathan Druart 2016-11-22 11:25:23 UTC
Created attachment 57704 [details] [review]
Bug 8361: Do not allow checkouts if no rules are defined

We should require a circulation rule to allow checkouts and reject them
if no rules are defined.

Test plan:
- Delete all issuing rules
- Check an item out
=> Without this patch the checkout is allowed
=> With this patch applied it is rejected
Comment 5 M. Tompsett 2016-11-22 22:23:34 UTC
Created attachment 57717 [details] [review]
[SIGNED-OFF] Bug 8361: Do not allow checkouts if no rules are defined

We should require a circulation rule to allow checkouts and reject them
if no rules are defined.

Test plan:
- Delete all issuing rules
- Check an item out
=> Without this patch the checkout is allowed
=> With this patch applied it is rejected

NOTE: Nicely fails when test is unchanged too.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 6 M. Tompsett 2016-11-22 22:28:39 UTC
Perhaps add an about.tt/about.pl check for issuing rules being defined?
Comment 7 Jonathan Druart 2016-11-23 07:56:36 UTC
(In reply to M. Tompsett from comment #6)
> Perhaps add an about.tt/about.pl check for issuing rules being defined?

I'd even suggest to bump up a warning on the main page.
Comment 8 Kyle M Hall 2016-12-23 13:21:17 UTC
Shouldn't the circulation.tt template be updated with a NO_RULE_DEFINED error case?
Comment 9 Jonathan Druart 2016-12-27 08:48:04 UTC
(In reply to Kyle M Hall from comment #8)
> Shouldn't the circulation.tt template be updated with a NO_RULE_DEFINED
> error case?

The message says "This patron can't check out this item per library circulation policy"
That sounds ok to me.
We could add a check somewhere else (about page).
Comment 10 Nick Clemens 2017-01-13 18:11:20 UTC
Created attachment 58966 [details] [review]
Bug 8361: Do not allow checkouts if no rules are defined

We should require a circulation rule to allow checkouts and reject them
if no rules are defined.

Test plan:
- Delete all issuing rules
- Check an item out
=> Without this patch the checkout is allowed
=> With this patch applied it is rejected

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens 2017-01-13 18:11:26 UTC
Created attachment 58967 [details] [review]
Bug 8361 (QA Followup) Add warnings

- Added message to mainpage.pl if no rules defined
 - Added message to circulation.tt to warn if rule undefined for
patron/itemtype combination

To test:
1 - Remove all rules
2 - Note that there is a warning on mainpage
3 - Add one rule
4 - Checkout to patron an itemtype that is outside of rule
defined above
5 - Note explanation that no rule is deifned
Comment 12 Nick Clemens 2017-01-13 18:12:49 UTC
Jonathan, your patches work, passing QA

I agree with Kyle though, let's give a reason if we know it.

Can you take a look at my followup?

-Nick
Comment 13 Jonathan Druart 2017-01-16 11:19:15 UTC
Nick,
Bug 17855 is going to add a circ rule during the installer process.
So I think it will no longer be necessary to check the existence of a circ rule from the mainpage.
Eventually we could add a check on the about page.
Comment 14 Nick Clemens 2017-01-17 11:49:15 UTC
Created attachment 59086 [details] [review]
Bug 8361 (QA Followup) Add warnings

- Added message to circulation.tt to warn if rule undefined for
patron/itemtype combination

To test:
1 - Remove all circ rules
2 - Add one rule
3 - Checkout to patron an itemtype that is outside of rule
defined above
4 - Note explanation that no rule is defined
Comment 15 Nick Clemens 2017-01-17 12:02:12 UTC
Created attachment 59087 [details] [review]
Bug 8361 (QA Followup) Add warnings

- Added message to circulation.tt to warn if rule undefined for
patron/itemtype combination

To test:
1 - Remove all circ rules
2 - Add one rule
3 - Checkout to patron an itemtype that is outside of rule
defined above
4 - Note explanation that no rule is defined
Comment 16 Kyle M Hall 2017-01-20 13:41:17 UTC
Pushed to master for 17.05, thanks Jonathan, Nick!
Comment 17 Katrin Fischer 2017-01-22 19:39:53 UTC
Contains strings - will check later if this is a candidate for 16.11.03.
Comment 18 Katrin Fischer 2017-02-07 06:54:29 UTC
These patches have been pushed to 16.11.x and will be in 16.11.04.
Comment 19 Julian Maurice 2017-02-09 08:49:18 UTC
Pushed to 3.22.x for 3.22.17
Comment 20 Mason James 2017-02-22 23:00:25 UTC
Pushed to 16.05.x, for 16.05.10 release