[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Katrin Fischer changed: What|Removed |Added See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=25554 --- Comment #14 from Katrin Fischer --- Hos does this relate to bug 25554? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Lari Taskula changed: What|Removed |Added Blocks|25089 | --- Comment #13 from Lari Taskula --- (In reply to Joonas Kylmälä from comment #12) > I agree with Jonathan that this change would make the code > confusing and also I think it would make it error prone because the coder > could think the scope is more restricted than what it in reality will be. I > wonder if there would be any other way to make the rule setting more > intuitive. Thanks for the feedback Jonathan and Joonas. It seems we need more opinions and some support before this can move forward. I'll leave this to "In Discussion" status. Since this is not a critical change, the dependency chain that it was built for, and is currently part of, can use the original set_rules() behavior. So I'm removing Bug 25089 from the blocker list. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25089 [Bug 25089] Add checkout_type to circulation rules -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Joonas Kylmälä changed: What|Removed |Added Status|Signed Off |In Discussion --- Comment #12 from Joonas Kylmälä --- Hi, I understood now that it was in Signed off status because it was waiting QA comment. I agree with Jonathan that this change would make the code confusing and also I think it would make it error prone because the coder could think the scope is more restricted than what it in reality will be. I wonder if there would be any other way to make the rule setting more intuitive. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Joonas Kylmälä changed: What|Removed |Added CC||joonas.kylm...@helsinki.fi --- Comment #11 from Joonas Kylmälä --- Lari, can you clarify the bug status? It is now Signed Off but the patches were resubmitted without signed-off-by line. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Lari Taskula changed: What|Removed |Added Attachment #105788|0 |1 is obsolete|| --- Comment #10 from Lari Taskula --- Created attachment 107651 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107651=edit Bug 25112: Make set_rules() handle rule scopes Koha::CirculationRules->set_rules is currently too complicated to use. Right now, in order to specify multiple rules at once, they must all be rules that accept the same set of scopes. Otherwise we can get this type of errors: 1/9 set_rule cannot set 'holds_per_record' for a 'checkout_type'! at t/db_dependent/Circulation/GetHardDueDate.t line 215. Validating scopes at set_rule() is good, but set_rules() should examine each rule and pass the correct scope to set_rule() instead of passing all scopes to every rule like it is doing now. To test: 1. prove t/db_dependent/Koha/CirculationRules.t 2. Observe success Sponsored-by: The National Library of Finland -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Lari Taskula changed: What|Removed |Added Attachment #105787|0 |1 is obsolete|| --- Comment #9 from Lari Taskula --- Created attachment 107650 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107650=edit Bug 25112: Add a failing test for CirculationRules.t To reproduce the problem, do: 1. prove t/db_dependent/Koha/CirculationRules.t 2. Observe a failing test set_rule cannot set 'holdallowed' for a 'categorycode'! at t/db_dependent/Koha/CirculationRules.t line 304. Sponsored-by: The National Library of Finland -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 --- Comment #8 from Jonathan Druart --- Yes it shorten the code, but it makes it confusing in my opinion. Waiting for someone from QA to get their opinion. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112
--- Comment #7 from Lari Taskula ---
(In reply to Jonathan Druart from comment #4)
> I think that's expected.
> That may make the caller codes confusing. Why do you need that?
This may be a matter of preference but I would expect "set_rules" method to
take all rules at once regardless of the developer having to study which set of
rules use the same set of scopes and then determine how many calls are
necessary. It should simply process all given rules with the scopes that they
are able to utilize. It reduces code repetition and makes adding new/modifying
scopes of existing rules easier because you don't have to split any of the
set_rules methods in parts.
A good example is in smart-rules.pl where we are repeating the calls multiple
times due to current restriction of set_rules. It looks bad and is quite
confusing to make changes to, especially when adding a new scope. I know this
page will eventually be removed but for the sake of future usability I propose
this change.
For visualization, here is the worst case with the current scopes and the
result after my proposed change:
Koha::CirculationRules->set_rules({
branchcode => 'CPL',
rules => {
refund => 1
}
})
Koha::CirculationRules->set_rules({
branchcode => 'CPL',
categorycode => 'PT',
rules => {
patron_maxissueqty => 1
}
})
Koha::CirculationRules->set_rules({
branchcode => 'CPL',
itemtype => 'BK',
rules => {
holdallowed => 1
}
})
Koha::CirculationRules->set_rules({
branchcode => 'CPL',
categorycode => 'PT'
itemtype => 'BK',
rules => {
article_requests => 1
}
})
vs proposed change:
Koha::CirculationRules->set_rules({
branchcode => 'CPL',
categorycode => 'PT'
itemtype => 'BK',
rules => {
refund => 1
patron_maxissueqty => 1
holdallowed => 1,
article_requests => 1
}
})
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Joonas Kylmälä changed: What|Removed |Added Attachment #102763|0 |1 is obsolete|| Attachment #102764|0 |1 is obsolete|| --- Comment #5 from Joonas Kylmälä --- Created attachment 105787 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105787=edit Bug 25112: Add a failing test for CirculationRules.t To reproduce the problem, do: 1. prove t/db_dependent/Koha/CirculationRules.t 2. Observe a failing test set_rule cannot set 'holdallowed' for a 'categorycode'! at t/db_dependent/Koha/CirculationRules.t line 304. Sponsored-by: The National Library of Finland Signed-off-by: Joonas Kylmälä -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 --- Comment #6 from Joonas Kylmälä --- Created attachment 105788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105788=edit Bug 25112: Make set_rules() handle rule scopes Koha::CirculationRules->set_rules is currently too complicated to use. Right now, in order to specify multiple rules at once, they must all be rules that accept the same set of scopes. Otherwise we can get this type of errors: 1/9 set_rule cannot set 'holds_per_record' for a 'checkout_type'! at t/db_dependent/Circulation/GetHardDueDate.t line 215. Validating scopes at set_rule() is good, but set_rules() should examine each rule and pass the correct scope to set_rule() instead of passing all scopes to every rule like it is doing now. To test: 1. prove t/db_dependent/Koha/CirculationRules.t 2. Observe success Sponsored-by: The National Library of Finland Signed-off-by: Joonas Kylmälä -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Joonas Kylmälä changed: What|Removed |Added Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Jonathan Druart changed: What|Removed |Added CC||jonathan.dru...@bugs.koha-c ||ommunity.org --- Comment #4 from Jonathan Druart --- I think that's expected. That may make the caller codes confusing. Why do you need that? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Lari Taskula changed: What|Removed |Added Blocks||25089 --- Comment #3 from Lari Taskula --- (In reply to Lari Taskula from comment #1) > Created attachment 102763 [details] [review] > Bug 25112: Add a failing test for CirculationRules.t > > To reproduce the problem, do: > 1. prove t/db_dependent/Koha/CirculationRules.t > 2. Observe a failing test > > set_rule cannot set 'holdallowed' for a 'categorycode'! at > t/db_dependent/Koha/CirculationRules.t line 304. > > Sponsored-by: The National Library of Finland Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25089 [Bug 25089] Add checkout_type to circulation rules -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Lari Taskula changed: What|Removed |Added Status|ASSIGNED|Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 --- Comment #2 from Lari Taskula --- Created attachment 102764 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102764=edit Bug 25112: Make set_rules() handle rule scopes Koha::CirculationRules->set_rules is currently too complicated to use. Right now, in order to specify multiple rules at once, they must all be rules that accept the same set of scopes. Otherwise we can get this type of errors: 1/9 set_rule cannot set 'holds_per_record' for a 'checkout_type'! at t/db_dependent/Circulation/GetHardDueDate.t line 215. Validating scopes at set_rule() is good, but set_rules() should examine each rule and pass the correct scope to set_rule() instead of passing all scopes to every rule like it is doing now. To test: 1. prove t/db_dependent/Koha/CirculationRules.t 2. Observe success Sponsored-by: The National Library of Finland -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Lari Taskula changed: What|Removed |Added CC||lari.task...@hypernova.fi -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 --- Comment #1 from Lari Taskula --- Created attachment 102763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102763=edit Bug 25112: Add a failing test for CirculationRules.t To reproduce the problem, do: 1. prove t/db_dependent/Koha/CirculationRules.t 2. Observe a failing test set_rule cannot set 'holdallowed' for a 'categorycode'! at t/db_dependent/Koha/CirculationRules.t line 304. Sponsored-by: The National Library of Finland -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Lari Taskula changed: What|Removed |Added Depends on|25089 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25089 [Bug 25089] Add checkout_type to circulation rules -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25112] Koha::CirculationRules->set_rules should handle scopes per rule
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25112 Lari Taskula changed: What|Removed |Added Change sponsored?|--- |Sponsored Depends on||25089 Status|NEW |ASSIGNED Assignee|koha-b...@lists.koha-commun |lari.task...@hypernova.fi |ity.org | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25089 [Bug 25089] Add checkout_type to circulation rules -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
