[Bug 9180] New: Default rules are not always used for overdues
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Bug ID: 9180 Summary: Default rules are not always used for overdues Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: sophie.meynieux@biblibre.com Default overdue rules are defined for a lybrary with several branches Overdue rules are also defined for one specific branch A user is late to return a book from a branch without overdue rules. But default rules should apply. No notification is sent. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #1 from Sophie MEYNIEUX <sophie.meynieux@biblibre.com> --- Created attachment 13799 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13799&action=edit Bug 9180 : Use default overdue rules when specific rules are not defined for a branch Test plan : - define default overdue rules - define overdue rules for one branch - find a user with late documents in another branch Without the patch, if you run misc/cronjobs/overdue_notices.pl -n -v you can see that only branch with specific rules is processed. The user does not get any notice. With the patch all branches are processes, those without specific rules using default rules. A notice is generated for the user. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #2 from Sophie MEYNIEUX <sophie.meynieux@biblibre.com> --- Also tested that without default rules, only branch with specific rules is processed. And without any rules at all, nothing is processed. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=6030 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Melia Meggs <melia@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |melia@bywatersolutions.com --- Comment #3 from Melia Meggs <melia@bywatersolutions.com> --- I can't seem to reproduce this bug on master. Here's what I tried: - I set up default circulation rules. - I set up circ rules for Centerville Branch. - I found a patron from Midway Branch with overdue items. (Midway Branch has no specific circ rules so should be using the default circ rules.) - I ran misc/cronjobs/overdue_notices.pl -n -v. I found that an overdue notice was correctly generated a notice for my Midway Branch patron, as well as patrons from other branches. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Melia, thank you for all your testing work! Reading through this bug, I think Sophie meant the overdue notice triggers in tools - could you take a look at those? I think to make the overdues job use those you have to use -t as an option when running it after setting the rules up. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion CC| |chris@bigballofwax.co.nz --- Comment #5 from Chris Cormack <chris@bigballofwax.co.nz> --- I also can not recreate, with rules set up in overdue notices and running with -n -v and -n -v -t I get the same output before and after the patch. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |sophie.meynieux@biblibre.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #6 from Sophie MEYNIEUX <sophie.meynieux@biblibre.com> --- Another of our library client complains about overdue notices not generated. They've got overduerules for all kind of patrons and specific rules only for one branch U1BUCG : Here is the output logs I've got : "Found 1 branch with first message enabled: 'U1BUCG' branch U1AGCCPF passed on parameter No active overduerules for branch 'U1AGCCPF' No active overduerules for DEFAULT either!" In database, default overdue rules are stored with '' as a branchcode. And the function that get all overduerules does not select thoses (C4::Overdues::GetBranchcodesWithOverdueRules): "SELECT DISTINCT branchcode FROM overduerules WHERE delay1 IS NOT NULL AND branchcode <> '' ORDER BY branchcode" I don't know how it can work without the patch... Maybe your default rules got another branchcode than '' ? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- We never use the default rules but always set up specific rules for each branch, because there has been problems with the default for a longer while. I am not sure if the problem is still on master or why Chris couldn't recreate - but I know there have been other bugs for this around. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=3393 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- See bug 3393 and all the duplicates linked with it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #9 from Sophie MEYNIEUX <sophie.meynieux@biblibre.com> --- bug 3393 says the patch does not apply because "it looks like" another patch has fixed the pb. But I can't find any patch that corrects it in C4/Overdues.pm history. My patch does the same as bug 3393 and should be applied in order default rules are taken into account -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Koha Team AMU <koha.aixmarseille@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |koha.aixmarseille@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #10 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- This patch got status Signed off by koha.aixmarseille@gmail.com. Please add a signoff line and explain what/how you tested in view of the multiple comments referring to impossibilities to test it. Thanks for your patience. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |jonathan.druart@biblibre.co | |m --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to M. de Rooy from comment #10)
This patch got status Signed off by koha.aixmarseille@gmail.com. Please add a signoff line and explain what/how you tested in view of the multiple comments referring to impossibilities to test it. Thanks for your patience.
Switch back to Needs Signoff, I don't understand the "Failed QA" status -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13799|0 |1 is obsolete| | --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 22574 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22574&action=edit Bug 9180: All branches should be returned if a default rule exists The C4::Overdues::GetBranchcodesWithOverdueRules routine has a bug. If a default rule *and* a specific rule exist, only the branchcode for the specific rule is returned. Test plan: prove t/db_dependent/Overdues.t and verify the unit tests are consistent. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Jonathan, sorry for commenting without testing - but I am not sure I understand your comment correctly. I think if a specific rule exists, we only want to use this rule, so why would returning only the specific rule be a bug? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #13)
Hi Jonathan, sorry for commenting without testing - but I am not sure I understand your comment correctly. I think if a specific rule exists, we only want to use this rule, so why would returning only the specific rule be a bug?
Katrin, The routine does not return the rules but the branchcodes. So if a default rule exists in the DB, all branchcodes should be returned. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, why not only the branchcodes for those where no specific rule exists? I should really have tested first, sorry for confusing. Overdues is quite a sensitive area for us. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #15)
Hm, why not only the branchcodes for those where no specific rule exists? I should really have tested first, sorry for confusing. Overdues is quite a sensitive area for us.
The routine returns all branchcodes with overdue rules defined. If you have only specific rules, all branches with at least a rule defined will be returned. But if you have at least 1 default rule, ALL branchcodes should be returned (the default rule affect all branches). If the routine does not return all branchcodes, the issue described by Sophie occurs. It will only affect instance with at least a default rule defined. This is marked as major but I think it is a critical one. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22574|0 |1 is obsolete| | --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 25947 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25947&action=edit Bug 9180: All branches should be returned if a default rule exists The C4::Overdues::GetBranchcodesWithOverdueRules routine has a bug. If a default rule *and* a specific rule exist, only the branchcode for the specific rule is returned. Test plan: prove t/db_dependent/Overdues.t and verify the unit tests are consistent. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Assignee|sophie.meynieux@biblibre.co |kyle@bywatersolutions.com |m | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|kyle@bywatersolutions.com |sophie.meynieux@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Sophie, I have tested this twice, but I think there is still a bug/regression in this: Test user from library A has checked out - various books from library A and B in A - books from libary B in B ./misc/cronjobs/overdue_notices.pl -t -v Tests: Overdue notice triggers 1) only defined for default/all libraries - with and without patch: 2 notices are generated. Both generated notices contain a complete list of overdue items from both branches. 2) defined for default/all libraries and library C - without patch: no notice is generated - with patch: 2 notices are generated. 3) defined for default/all libraries and library A - without patch: only a notice for library A is generated - with patch: 2 notices are generated. 4) only defined for library A - with and without patch: only 1 notice is generated for library A 5) defined for library A and B, no default rule - without patch: 2 messages are generated - with patch: only 1 message is generated For this bug: 2) seems ok with the patch, but we seem to introduce a regression on 5). In general: Should the item lists be identical for the patches? I think it would only make sense if each overdue notice contained a complementary list of items. But should they be divided by checkout library/homebranch? Should it be controlled by one of the existing preferences (CircControl or HomeOrHoldingBranch)? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #19 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 27427 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27427&action=edit Bug 9180: Use selectcol instead of selectrow The selectrow_* method only returns the *first* row of data! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #18) Oops, sorry Katrin, noob error. Fixed in a followup. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 --- Comment #21 from Jonathan Druart <jonathan.druart@biblibre.com> --- This one is major, please test. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|sophie.meynieux@biblibre.co |jonathan.druart@biblibre.co |m |m --- Comment #22 from Jonathan Druart <jonathan.druart@biblibre.com> --- I cannot QA this patch, still major... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Doing some first tests on this, noticing a few things: 1) The unit tests (yay!) are in a new file Overdues.pl - I think they need to be integrated into Overdues.t or the file should be renamed to be a .t file. 2) While t/db_dependent/Overdues.t passes with the tests, Ovedues.pl does not: bumblebee:~/kohaclone (19-9180-defaultoverduerules) $ perl t/db_dependent/Overdues.pl 1..4 not ok 1 - If a default rule exists, all branches should be returned # Failed test 'If a default rule exists, all branches should be returned' # at t/db_dependent/Overdues.pl line 26. # Structures begin differing at: # $got->[0] = 'TPL' # $expected->[0] = 'FRL' not ok 2 - If a default rule exists and a specific rule exists, all branches should be returned # Failed test 'If a default rule exists and a specific rule exists, all branches should be returned' # at t/db_dependent/Overdues.pl line 36. # Structures begin differing at: # $got->[0] = 'RPL' # $expected->[0] = 'FRL' ok 3 - If only a specific rule exist, only 1 branch should be returned ok 4 - If only 2 specific rules exist, 2 branches should be returned # Looks like you failed 2 tests of 4. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25947|0 |1 is obsolete| | Attachment #27427|0 |1 is obsolete| | --- Comment #24 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 29548 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29548&action=edit Bug 9180: All branches should be returned if a default rule exists The C4::Overdues::GetBranchcodesWithOverdueRules routine has a bug. If a default rule *and* a specific rule exist, only the branchcode for the specific rule is returned. Test plan: prove t/db_dependent/Overdues.t and verify the unit tests are consistent. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|m.de.rooy@rijksmuseum.nl | --- Comment #25 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #23)
Doing some first tests on this, noticing a few things:
1) The unit tests (yay!) are in a new file Overdues.pl - I think they need to be integrated into Overdues.t or the file should be renamed to be a .t file.
I really don't understand what I did... I squashed the patches, rename the unit tests file and added a "sort" on comparing the array (this fixes the test failures). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |m.de.rooy@rijksmuseum.nl --- Comment #26 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Taking a look -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Small patch --- Comment #27 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Changing complexity: Changing a routine in a module (Overdues) is generally not considered trivial. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29548|0 |1 is obsolete| | --- Comment #28 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 30392 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30392&action=edit Bug 9180: All branches should be returned if a default rule exists The C4::Overdues::GetBranchcodesWithOverdueRules routine has a bug. If a default rule *and* a specific rule exist, only the branchcode for the specific rule is returned. Test plan: prove t/db_dependent/Overdues.t and verify the unit tests are consistent. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9180 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolyn.somers@biblibre.co | |m --- Comment #30 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Can this be backported to 3.16.x ? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org