[Bug 12086] New: Hold priorities incorrect, when waiting status was reversed
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Bug ID: 12086 Summary: Hold priorities incorrect, when waiting status was reversed Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: katrin.fischer@bsz-bw.de QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Tested on top of patch for 11947! 1) Prepare for headache. 2) Check out an item from any record. 3) Place 3 holds, for patrons a, b, c on the same record. 4) Return the checked out item - it is now waiting for a, b = 1, c = 2 5) Check out the book to b(!), revert waiting status as offered by the GUI You will end up with: a = 1, c = 1 in the holds list, but in the database it's kind of more correct: a = 1, c = 3 -- 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=12086 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |11947 -- 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=12086 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- I can't reproduce this on master. -- 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=12086 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Gah. I am sure it was there - but what is the missing piece? I reproduced the behaviour twice - one of my holds was item level, I remember that much. I will see if I can still reproduce it. -- 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=12086 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Ok, repeating tests, adding some information to the test plan: 1) Test record has 1 single item, checked out to patron X 2) Place 3 holds for patrons A, B and C, all title level hold this time A, B, C, item branches and staff branch are the same. 3) Return item, confirm hold 4) Confirm item is now waiting for patron A Priorities are: A = Waiting, B = 1, C = 2 5) Open patron account of user B, checkout book Koha asks: Item X has been waiting for patron A... Revert waiting status Confirm. 6) Check priorities: Hold list shows: A = 1, C = 1 Database says: A = 1, C = 3 -- 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=12086 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- I was able to reproduce this issue following the new test plan! -- 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=12086 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=12086 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 27262 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27262&action=edit Bug 12086 - Hold priorities incorrect, when waiting status was reversed 1) Test record has 1 single item, checked out to patron X 2) Place 3 holds for patrons A, B and C, all title level hold this time A, B, C, item branches and staff branch are the same. 3) Return item, confirm hold 4) Confirm item is now waiting for patron A Priorities are: A = Waiting, B = 1, C = 2 5) Open patron account of user B, checkout book Koha asks: Item X has been waiting for patron A... Revert waiting status Confirm. 6) Check priorities: Hold list shows: A = 1, C = 1 Database says: A = 1, C = 3 7) Apply this patch 8) Repeat steps 1-6 9) Note the priorities are correct -- 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=12086 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- 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=12086 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27262|0 |1 is obsolete| | --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 27263 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27263&action=edit Bug 12086 - Hold priorities incorrect, when waiting status was reversed 1) Test record has 1 single item, checked out to patron X 2) Place 3 holds for patrons A, B and C, all title level hold this time A, B, C, item branches and staff branch are the same. 3) Return item, confirm hold 4) Confirm item is now waiting for patron A Priorities are: A = Waiting, B = 1, C = 2 5) Open patron account of user B, checkout book Koha asks: Item X has been waiting for patron A... Revert waiting status Confirm. 6) Check priorities: Hold list shows: A = 1, C = 1 Database says: A = 1, C = 3 7) Apply this patch 8) Repeat steps 1-6 9) Note the priorities are correct -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- /me whispers *regression test*? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 jdemuth@roseville.ca.us <jdemuth@roseville.ca.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jdemuth@roseville.ca.us -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27263|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 27406 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27406&action=edit Bug 12086 - Hold priorities incorrect, when waiting status was reversed 1) Test record has 1 single item, checked out to patron X 2) Place 3 holds for patrons A, B and C, all title level hold this time A, B, C, item branches and staff branch are the same. 3) Return item, confirm hold 4) Confirm item is now waiting for patron A Priorities are: A = Waiting, B = 1, C = 2 5) Open patron account of user B, checkout book Koha asks: Item X has been waiting for patron A... Revert waiting status Confirm. 6) Check priorities: Hold list shows: A = 1, C = 1 Database says: A = 1, C = 3 7) Apply this patch 8) Repeat steps 1-6 9) Note the priorities are correct -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27406|0 |1 is obsolete| | --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 27407 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27407&action=edit Bug 12086 - Hold priorities incorrect, when waiting status was reversed 1) Test record has 1 single item, checked out to patron X 2) Place 3 holds for patrons A, B and C, all title level hold this time A, B, C, item branches and staff branch are the same. 3) Return item, confirm hold 4) Confirm item is now waiting for patron A Priorities are: A = Waiting, B = 1, C = 2 5) Open patron account of user B, checkout book Koha asks: Item X has been waiting for patron A... Revert waiting status Confirm. 6) Check priorities: Hold list shows: A = 1, C = 1 Database says: A = 1, C = 3 7) Apply this patch 8) Repeat steps 1-6 9) Note the priorities are correct -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #7)
/me whispers *regression test*?
Done! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27407|0 |1 is obsolete| | --- Comment #11 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 27428 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27428&action=edit [SIGNED-OFF] Bug 12086 - Hold priorities incorrect, when waiting status was reversed 1) Test record has 1 single item, checked out to patron X 2) Place 3 holds for patrons A, B and C, all title level hold this time A, B, C, item branches and staff branch are the same. 3) Return item, confirm hold 4) Confirm item is now waiting for patron A Priorities are: A = Waiting, B = 1, C = 2 5) Open patron account of user B, checkout book Koha asks: Item X has been waiting for patron A... Revert waiting status Confirm. 6) Check priorities: Hold list shows: A = 1, C = 1 Database says: A = 1, C = 3 7) Apply this patch 8) Repeat steps 1-6 9) Note the priorities are correct Signed-off-by: Owen Leonard <oleonard@myacpl.org> Test plan correctly predicts the error and the correction made by the patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The new tests fail :( bumblebee:~/kohaclone (106-12086-holdreversed) $ perl t/db_dependent/Holds_12086.t 1..3 Subroutine C4::Context::userenv redefined at t/db_dependent/Holds_12086.t line 29. # Creating biblio instance for testing. # Creating item instance for testing. DBD::mysql::st execute failed: Duplicate entry '2' for key 'PRIMARY' at /home/katrin/kohaclone/C4/Reserves.pm line 1194. DBD::mysql::st execute failed: Duplicate entry '2' for key 'PRIMARY' at /home/katrin/kohaclone/C4/Reserves.pm line 1194. # Looks like your test exited with 255 before it could output anything. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Joel Sasse <jsasse@plumcreeklibrary.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jsasse@plumcreeklibrary.net -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 28471 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28471&action=edit Bug 12086 [QA Followup] - Truncate old_reseres in unit test -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Kyle M Hall <kyle@bywatersolutions.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=12086 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Kyle, I think we should not use truncate in the unit tests as it appears to do an implicit commit in MySQL... which we don't really want here. Could you please rewrite it? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Kyle M Hall <kyle@bywatersolutions.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=12086 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28471|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 28482 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28482&action=edit Bug 12086 [QA Followup] - Delete reserves, old_reserves before running unit tests -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 27428 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27428 [SIGNED-OFF] Bug 12086 - Hold priorities incorrect, when waiting status was reversed Review of attachment 27428: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=12086&attachment=27428) ----------------------------------------------------------------- Why don't you use t/db_dependent/Holds.t? ::: t/db_dependent/Holds_12086.t @@ +1,2 @@
+#!/usr/bin/perl +
Are you sure we want 1 file per bug? This is a new way to do. Maybe should you sent an email to the list to get other opinions. @@ +15,5 @@
+ +BEGIN { + use FindBin; + use lib $FindBin::Bin; +}
Why do you use FindBin? @@ +40,5 @@
+my ($bibnum, $title, $bibitemnum) = create_helper_biblio(); + +# Helper item for that biblio. +diag("Creating item instance for testing."); +my $item_barcode = '99999999999999999999';
Hum, yes but why not "my_barcode"? :) @@ +48,5 @@
+my @borrowernumbers; +foreach (1..$borrowers_count) { + my $borrowernumber = AddMember( + firstname => 'my firstname', + surname => 'my surname ' . $_,
Please don't use the variable loop $_ to avoid futures errors. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@biblibre.co | |m --- Comment #17 from Jonathan Druart <jonathan.druart@biblibre.com> --- Kyle, could you please answer my questions please? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #17)
Kyle, could you please answer my questions please?
The answer is a blatantly stole a different unit test as the basis for this one. I chose to create a new unit test file because adding it to the existing holds unit test proved to be difficult. This way, the test can start with no assumptions about what has taken place previously, and does not need to care about what takes place in the future. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #19 from Jonathan Druart <jonathan.druart@biblibre.com> --- I think something like t/db_dependent/Reserves/RevertWaitingStatus.t is more clear. What about others questions? :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Kyle, could you take another look at comments 16 and 19? I like Jonathan's suggestion on renaming the file to make the purpose more clear. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 28873 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28873&action=edit Bug 12086 [QA Followup 2] - Implement Jonathan's suggestions -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@biblibre.co |y.org |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Jonathan Druart <jonathan.druart@biblibre.com> 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=12086 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27428|0 |1 is obsolete| | Attachment #28482|0 |1 is obsolete| | Attachment #28873|0 |1 is obsolete| | --- Comment #22 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 29162 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29162&action=edit Bug 12086 - Hold priorities incorrect, when waiting status was reversed 1) Test record has 1 single item, checked out to patron X 2) Place 3 holds for patrons A, B and C, all title level hold this time A, B, C, item branches and staff branch are the same. 3) Return item, confirm hold 4) Confirm item is now waiting for patron A Priorities are: A = Waiting, B = 1, C = 2 5) Open patron account of user B, checkout book Koha asks: Item X has been waiting for patron A... Revert waiting status Confirm. 6) Check priorities: Hold list shows: A = 1, C = 1 Database says: A = 1, C = 3 7) Apply this patch 8) Repeat steps 1-6 9) Note the priorities are correct Signed-off-by: Owen Leonard <oleonard@myacpl.org> Test plan correctly predicts the error and the correction made by the patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Amended patch: I squashed the 3 patches for a better readability. A file was created in the first patch and deleted later. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #23 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 29163 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29163&action=edit Bug 12086: qa-followup: remove warning on launching unit tests Without this patch, the following warning appears: t/db_dependent/Holds/RevertWaitingStatus.t .. Subroutine C4::Context::userenv redefined at t/db_dependent/Holds/RevertWaitingStatus.t line 25. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Kyle! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 --- Comment #25 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to 3.16.x for inclusion in 3.16.1. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12086 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m --- Comment #26 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Pushed to 3.14.x, will be in 3.14.12. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org