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)
Is this still an issue? Why should we refuse checkouts if no rule is defined?
(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.
(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 :)
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
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>
Perhaps add an about.tt/about.pl check for issuing rules being defined?
(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.
Shouldn't the circulation.tt template be updated with a NO_RULE_DEFINED error case?
(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).
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>
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
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
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.
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
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
Pushed to master for 17.05, thanks Jonathan, Nick!
Contains strings - will check later if this is a candidate for 16.11.03.
These patches have been pushed to 16.11.x and will be in 16.11.04.
Pushed to 3.22.x for 3.22.17
Pushed to 16.05.x, for 16.05.10 release