[Bug 13540] New: Item's permanent_location is set to CART if an Item is edited when it's location is CART.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Bug ID: 13540 Summary: Item's permanent_location is set to CART if an Item is edited when it's location is CART. Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: olli-antti.kivilahti@jns.fi QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com :-:TEST SETUP:-: 0a. System preference "ReturnToShelvingCart" must be 'Move'. 0b. Find any Item (I69) 0c. Set I69's location to "Book Cart" (BC) or anything but empty. :-:REPLICATE ISSUE:-: 1. Check-in the I69 and the location should turn to 'CART' (returned today). 2. Go to the "Edit Item"-view for I69, don't change anything, simply click "Save changes" 3. Observe that the permanent_location has changed to 'CART'. This is not good since now the location data is forever lost again (Bugg 7817). The "cart_to_shelf.pl --hours 24"-script cannot return the Item to the proper place ;( -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=7817 -- 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=13540 --- Comment #1 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 35038 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35038&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC if an Item is edited when it's location is CART|PROC. This bug is rather nasty as it happens very discreetly when editing Items that are in either locations 'CART' or 'PROC'. For example when changing one of the Item's statuses or barcode, the permanent_location gets overwritten without the user noticing it immediately. The damage becomes apparent only weeks later when there are potentially thousands of Items missing the permanent_location and location. :-:TEST SETUP:-: 0a. System preference "ReturnToShelvingCart" must be 'Move'. 0b. Find any Item (I69) 0c. Set I69's location to "Book Cart" (BC) or anything but empty. :-:REPLICATE ISSUE:-: 1. Check-in the I69 and the location should turn to 'CART' (returned today). 2. Go to the "Edit Item"-view for I69, don't change anything, simply click "Save changes" 3. Observe that the permanent_location has changed to 'CART'. This is not good since now the location data is forever lost again (Bugg 7817). The "cart_to_shelf.pl --hours 24"-script cannot return the Item to the proper place ;( :-:AFTER THIS PATCH:-: Repeat steps 1-2. 3. Observe that the permanent_location is still the old one, instead of getting overwritten by 'CART'. -- 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=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Item's permanent_location |Item's permanent_location |is set to CART if an Item |is set to CART|PROC if an |is edited when it's |Item is edited when it's |location is CART. |location is CART|PROC. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #2 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Unit tests to follow, gotta go home! -- 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=13540 --- Comment #3 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 35047 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35047&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC when... - Unit tests -- 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=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major Patch complexity|--- |Trivial patch Priority|P5 - low |P2 Assignee|koha-bugs@lists.koha-commun |olli-antti.kivilahti@jns.fi |ity.org | --- Comment #4 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Setting importance to Major P2, because this causes real loss of data in a very conspicious manner. Repairing the damage afterwards can be impossible. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical CC| |katrin.fischer@bsz-bw.de --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Olli, we barely use the P* values, I have upped this to critical. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35047|0 |1 is obsolete| | --- Comment #6 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 35055 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35055&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC when... - Unit tests -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35038|0 |1 is obsolete| | --- Comment #7 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 35336 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35336&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC if an Item is edited when it's location is CART|PROC. This bug is rather nasty as it happens very discreetly when editing Items that are in either locations 'CART' or 'PROC'. For example when changing one of the Item's statuses or barcode, the permanent_location gets overwritten without the user noticing it immediately. The damage becomes apparent only weeks later when there are potentially thousands of Items missing the permanent_location and location. :-:TEST SETUP:-: 0a. System preference "ReturnToShelvingCart" must be 'Move'. 0b. Find any Item (I69) 0c. Set I69's location to "Book Cart" (BC) or anything but empty. :-:REPLICATE ISSUE:-: 1. Check-in the I69 and the location should turn to 'CART' (returned today). 2. Go to the "Edit Item"-view for I69, don't change anything, simply click "Save changes" 3. Observe that the permanent_location has changed to 'CART'. This is not good since now the location data is forever lost again (Bugg 7817). The "cart_to_shelf.pl --hours 24"-script cannot return the Item to the proper place ;( :-:AFTER THIS PATCH:-: Repeat steps 1-2. 3. Observe that the permanent_location is still the old one, instead of getting overwritten by 'CART'. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Chris Cormack <chris@bigballofwax.co.nz> 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=13540 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35055|0 |1 is obsolete| | --- Comment #8 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 35337 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35337&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC when... - Unit tests Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@biblibre.co | |m --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- Looks like something is wrong for me: % prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 23/67 # Failed test 'ReturnToShelvingCart: Adding a new Item with location also sets the permanent_location.' # at t/db_dependent/Circulation.t line 648. # Looks like you failed 1 test of 67. Does it come from my DB? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #10 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 35951 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35951&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC when... - Followup 1 To fix the failing test "ReturnToShelvingCart: Adding a new Item with location also sets the permanent_location." When adding an Item, set the permanent_location to location if present. This behaviour is consistent with ModItem where permanent_location is set to location. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35951|0 |1 is obsolete| | --- Comment #11 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 35952 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35952&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC when... - Followup 1 To fix the failing test "ReturnToShelvingCart: Adding a new Item with location also sets the permanent_location." When adding an Item, set the permanent_location to location if present. This behaviour is consistent with ModItem where permanent_location is set to location. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #12 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Thanks Chris for the sign off! Thanks Joubu for kicking my ass! Followup supplied. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35952|0 |1 is obsolete| | --- Comment #13 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 35953 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35953&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC when... - Followup 1 To fix the failing test "ReturnToShelvingCart: Adding a new Item with location also sets the permanent_location." When adding an Item, set the permanent_location to location if present. This behaviour is consistent with ModItem where permanent_location is set to location. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- 1/ FAIL C4/Items.pm FAIL pod *** ERROR: Unknown command 'head' in file C4/Items.pm *** ERROR: Spurious text after =cut in file C4/Items.pm You need an empty line avec the =cut 2/ Please prefer creating a subtest (`git grep subtest t` for examples), instead of a sub, in the test file. 3/ Why do you create a new routine? IMO you should use C4::Items::GetItem($itemnumber)->{permanent_location}. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #15 from Jonathan Druart <jonathan.druart@biblibre.com> --- You need an empty line avec the =cut means: You need to add an empty line avec the '=cut' in the pod. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- s/avec/after (hum, I need some fresh air...) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #17 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- I created a new subroutine, because I only need the location-data. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #18 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- It's a bit like, if one chocolate bar makes you happy, why eat 4? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #19 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Olli-Antti Kivilahti from comment #17)
I created a new subroutine, because I only need the location-data.
It's another subroutine to maintain, I am not sure it's a good idea. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I agree with Jonathan - in this case a new subroutine is probably a bit too specific. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #21 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #20)
I agree with Jonathan - in this case a new subroutine is probably a bit too specific.
So, failing QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Can we get a new patch for this? It's critical :( -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #23 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 40183 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40183&action=edit Bug 13540: Remove the too specific subroutine This patch replaces the too specific GetRealItemLocations subroutine with a call to GetItem -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35336|0 |1 is obsolete| | --- Comment #24 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 40214 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40214&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC if an Item is edited when it's location is CART|PROC. This bug is rather nasty as it happens very discreetly when editing Items that are in either locations 'CART' or 'PROC'. For example when changing one of the Item's statuses or barcode, the permanent_location gets overwritten without the user noticing it immediately. The damage becomes apparent only weeks later when there are potentially thousands of Items missing the permanent_location and location. :-:TEST SETUP:-: 0a. System preference "ReturnToShelvingCart" must be 'Move'. 0b. Find any Item (I69) 0c. Set I69's location to "Book Cart" (BC) or anything but empty. :-:REPLICATE ISSUE:-: 1. Check-in the I69 and the location should turn to 'CART' (returned today). 2. Go to the "Edit Item"-view for I69, don't change anything, simply click "Save changes" 3. Observe that the permanent_location has changed to 'CART'. This is not good since now the location data is forever lost again (Bugg 7817). The "cart_to_shelf.pl --hours 24"-script cannot return the Item to the proper place ;( :-:AFTER THIS PATCH:-: Repeat steps 1-2. 3. Observe that the permanent_location is still the old one, instead of getting overwritten by 'CART'. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> Working in prod for ~6 months. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #25 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Tests are broken... :( -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #26 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Olli-Antti Kivilahti from comment #25)
Tests are broken... :(
Which ones? I tried t/db_dependent/Circulation.t, and they pass. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40214|0 |1 is obsolete| | --- Comment #27 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 40878 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40878&action=edit Bug 13540 - Item's permanent_location is set to CART|PROC when... - Unit tests Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I have rebased the last patch hopefully correctly and fixed problems with the POD in the test file noted by the QA script. But: $ prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 25/69 # Failed test 'ReturnToShelvingCart: Modifying the whole Item doesn't overwrite permanent_location!' # at t/db_dependent/Circulation.t line 684. # Failed test 'ReturnToShelvingCart: Adding a new Item with location also sets the permanent_location.'$ prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 25/69 # Failed test 'ReturnToShelvingCart: Modifying the whole Item doesn't overwrite permanent_location!' # at t/db_dependent/Circulation.t line 684. # Failed test 'ReturnToShelvingCart: Adding a new Item with location also sets the permanent_location.' # at t/db_dependent/Circulation.t line 704. # Looks like you failed 2 tests of 69. t/db_dependent/Circulation.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/69 subtests Test Summary Report ------------------- t/db_dependent/Circulation.t (Wstat: 512 Tests: 69 Failed: 2) Failed tests: 67, 69 Non-zero exit status: 2 Files=1, Tests=69, 2 wallclock secs ( 0.03 usr 0.00 sys + 1.50 cusr 0.08 csys = 1.61 CPU) Result: FAIL # at t/db_dependent/Circulation.t line 704. # Looks like you failed 2 tests of 69. t/db_dependent/Circulation.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/69 subtests Test Summary Report ------------------- t/db_dependent/Circulation.t (Wstat: 512 Tests: 69 Failed: 2) Failed tests: 67, 69 Non-zero exit status: 2 Files=1, Tests=69, 2 wallclock secs ( 0.03 usr 0.00 sys + 1.50 cusr 0.08 csys = 1.61 CPU) Result: FAIL Leaving this in Failed QA for now - but would love to see this fixed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13540 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |DUPLICATE Status|Failed QA |RESOLVED CC| |josef.moravec@gmail.com --- Comment #29 from Josef Moravec <josef.moravec@gmail.com> --- *** This bug has been marked as a duplicate of bug 14811 *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org