The cronjob longoverdue.pl does not require that an item marked as lost be returned automatically, but there is a line in ModItem that automatically marks the item's onloan as false if itemlost is set!
Created attachment 29175 [details] [review] Bug 12467 - Lost items marked as not on loan even if they are! The cronjob longoverdue.pl does not require that an item marked as lost be returned automatically, but there is a line in ModItem that automatically marks the item's onloan as false if itemlost is set! Test Plan: 1) Mark an item as lost with longoverdue.pl, without --mark-returned 2) Inspect the db, note that items.onloan is now 0 3) Apply this patch 4) Mark repeat step 1 5) Inspect the db, noe that items.onloan is still 1 6) Test marking an item as lost from staff interface, ensure there are no regressions.
Created attachment 30766 [details] [review] [SIGNED-OFF] Bug 12467 - Lost items marked as not on loan even if they are! The cronjob longoverdue.pl does not require that an item marked as lost be returned automatically, but there is a line in ModItem that automatically marks the item's onloan as false if itemlost is set! Test Plan: 1) Mark an item as lost with longoverdue.pl, without --mark-returned 2) Inspect the db, note that items.onloan is now 0 3) Apply this patch 4) Mark repeat step 1 5) Inspect the db, noe that items.onloan is still 1 6) Test marking an item as lost from staff interface, ensure there are no regressions. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
This does what it claims to do, but I am a bit confused. I wanted to do the opposite and prove that the onloan is removed when I use --mark-returned - but the item is only marked as lost and never returned. Is this another bug?
Created attachment 30897 [details] [review] [PASSED QA] Bug 12467 - Lost items marked as not on loan even if they are! The cronjob longoverdue.pl does not require that an item marked as lost be returned automatically, but there is a line in ModItem that automatically marks the item's onloan as false if itemlost is set! Test Plan: 1) Mark an item as lost with longoverdue.pl, without --mark-returned 2) Inspect the db, note that items.onloan is now 0 3) Apply this patch 4) Mark repeat step 1 5) Inspect the db, noe that items.onloan is still 1 6) Test marking an item as lost from staff interface, ensure there are no regressions. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Patch works according to test plan and fixes a data loss bug. Some notes: - This patch would be nicer with a regression test. - Also checked that returning the item removes lost status and onloan still. - Tried to test with --mark-returned, but couldn't get it to return my item neither with nor without the patch. (see comment on bug report)
That would definitely be a bug! Please file a separate bug report for that! (In reply to Katrin Fischer from comment #3) > This does what it claims to do, but I am a bit confused. I wanted to do the > opposite and prove that the onloan is removed when I use --mark-returned - > but the item is only marked as lost and never returned. Is this another bug?
Can we have some regression tests for this one?
(In reply to Kyle M Hall from comment #5) > That would definitely be a bug! Please file a separate bug report for that! > > (In reply to Katrin Fischer from comment #3) > > This does what it claims to do, but I am a bit confused. I wanted to do the > > opposite and prove that the onloan is removed when I use --mark-returned - > > but the item is only marked as lost and never returned. Is this another bug? I filed bug 12812.
Created attachment 31265 [details] [review] Bug 12467 [QA Followup] - Unit Tests
Created attachment 31266 [details] [review] Bug 12467 [QA Followup] - Unit Tests
Patches pushed to master. Thanks Kyle!