Bug 21754 - If an item is marked as lost, any outstanding transfers upon it should be automatically cancelled
Summary: If an item is marked as lost, any outstanding transfers upon it should be aut...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Buckley
QA Contact: Martin Renvoize
URL:
Keywords:
: 21732 (view as bug list)
Depends on:
Blocks: 20844
  Show dependency treegraph
 
Reported: 2018-11-01 21:26 UTC by Alex Buckley
Modified: 2019-01-03 14:27 UTC (History)
4 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
If an item is marked as lost, then any pending transfers the item had will now be removed.
Version(s) released in:


Attachments
Bug 21754: Automatically clean up outstanding transfers on lost items (5.72 KB, patch)
2018-11-01 22:09 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21754: Automatically clean up outstanding transfers on lost items (5.89 KB, patch)
2018-11-07 10:05 UTC, Andreas Hedström Mace
Details | Diff | Splinter Review
Bug 21754: Automatically clean up outstanding transfers on lost items (5.84 KB, patch)
2018-11-07 10:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21754: Automatically clean up outstanding transfers on lost items (5.83 KB, patch)
2018-12-12 02:24 UTC, Alex Buckley
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Buckley 2018-11-01 21:26:28 UTC
When an item is marked as lost automatically cancel any outstanding transfers on it (this is an alternative to bug 21732 - https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21732- which checks if a syspref is enabled before cancelling outstanding transfers on the lost item).

Also set the items holding branch to the transfers source ('from') branch.
Comment 1 Alex Buckley 2018-11-01 22:09:51 UTC
Created attachment 81854 [details] [review]
Bug 21754: Automatically clean up outstanding transfers on lost items

This is an alternative to bug 21732 as transfers are automatically cancelled on marking an item lost, and the items holding rbanch is set to the transfers source ('from') branch.

When an item is marked as lost, the routine should also clean up any
outstanding transfers.

Also added tests to t/db_dependent/Circulation.t which check:
* If transfer is automatically deleted when item is marked as lost
* If the items holdingbranch automatically changes when item with
transfers on it is marked as lost.

Test plan:
1. Find a item which is in transfer, i.e. find an item with the text in
the 'Status' field of the table in detail.pl that indicates it is in
transfer
2. Set the item to 'Lost' either by clicking on Edit->Edit items from
the detail.pl page
OR
clicking on the Items tab on the left side of the detail.pl page
3. Notice that the transfer is now cancelled for the item and the items
holdingbranch is the transfers source ('from') branch
4. Run t/db_dependent/Circulation.t

Sponsored-By: Brimbank Library, Australia
Comment 2 Martin Renvoize 2018-11-06 08:37:58 UTC
We sought additional feedback on the mailing lists and currently, the feedback (https://lists.katipo.co.nz/public/koha/2018-November/051788.html) is that this version of the bug is preferable to the original bug 21732, as such I'm going to mark that one as a duplicate and close it so we can move forward with this preferred approach.

I'll try to find someone to signoff today so I can proceed with QA.
Comment 3 Martin Renvoize 2018-11-06 08:38:41 UTC
*** Bug 21732 has been marked as a duplicate of this bug. ***
Comment 4 Andreas Hedström Mace 2018-11-07 10:05:05 UTC
Created attachment 82016 [details] [review]
Bug 21754: Automatically clean up outstanding transfers on lost items

This is an alternative to bug 21732 as transfers are automatically cancelled on marking an item lost, and the items holding rbanch is set to the transfers source ('from') branch.

When an item is marked as lost, the routine should also clean up any
outstanding transfers.

Also added tests to t/db_dependent/Circulation.t which check:
* If transfer is automatically deleted when item is marked as lost
* If the items holdingbranch automatically changes when item with
transfers on it is marked as lost.

Test plan:
1. Find a item which is in transfer, i.e. find an item with the text in
the 'Status' field of the table in detail.pl that indicates it is in
transfer
2. Set the item to 'Lost' either by clicking on Edit->Edit items from
the detail.pl page
OR
clicking on the Items tab on the left side of the detail.pl page
3. Notice that the transfer is now cancelled for the item and the items
holdingbranch is the transfers source ('from') branch
4. Run t/db_dependent/Circulation.t

Sponsored-By: Brimbank Library, Australia
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Comment 5 Martin Renvoize 2018-11-07 10:44:45 UTC
Created attachment 82017 [details] [review]
Bug 21754: Automatically clean up outstanding transfers on lost items

This is an alternative to bug 21732 as transfers are automatically cancelled on marking an item lost, and the items holding rbanch is set to the transfers source ('from') branch.

When an item is marked as lost, the routine should also clean up any
outstanding transfers.

Also added tests to t/db_dependent/Circulation.t which check:
* If transfer is automatically deleted when item is marked as lost
* If the items holdingbranch automatically changes when item with
transfers on it is marked as lost.

Test plan:
1. Find a item which is in transfer, i.e. find an item with the text in
the 'Status' field of the table in detail.pl that indicates it is in
transfer
2. Set the item to 'Lost' either by clicking on Edit->Edit items from
the detail.pl page
OR
clicking on the Items tab on the left side of the detail.pl page
3. Notice that the transfer is now cancelled for the item and the items
holdingbranch is the transfers source ('from') branch
4. Run t/db_dependent/Circulation.t

Sponsored-by: Brimbank Library, Australia
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
(fixed the introduction of a whitespace line and removed a double
declare warning from the new tests as part of QA)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2018-11-07 10:45:43 UTC
Great work Alex, thank you for all the perseverance. No regressions found, Passing QA.
Comment 7 Alex Buckley 2018-12-12 02:23:56 UTC
As this is still waiting to be pushed and now conflicts upon being applied I am attaching a rebased patch with the only change being I have changed the number of tests in t/db_dependent/Circulation.pl from 120 to 123.
Comment 8 Alex Buckley 2018-12-12 02:24:51 UTC
Created attachment 83061 [details] [review]
Bug 21754: Automatically clean up outstanding transfers on lost items

This is an alternative to bug 21732 as transfers are automatically cancelled on marking an item lost, and the items holding rbanch is set to the transfers source ('from') branch.

When an item is marked as lost, the routine should also clean up any
outstanding transfers.

Also added tests to t/db_dependent/Circulation.t which check:
* If transfer is automatically deleted when item is marked as lost
* If the items holdingbranch automatically changes when item with
transfers on it is marked as lost.

Test plan:
1. Find a item which is in transfer, i.e. find an item with the text in
the 'Status' field of the table in detail.pl that indicates it is in
transfer
2. Set the item to 'Lost' either by clicking on Edit->Edit items from
the detail.pl page
OR
clicking on the Items tab on the left side of the detail.pl page
3. Notice that the transfer is now cancelled for the item and the items
holdingbranch is the transfers source ('from') branch
4. Run t/db_dependent/Circulation.t

Sponsored-by: Brimbank Library, Australia
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
(fixed the introduction of a whitespace line and removed a double
declare warning from the new tests as part of QA)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Alex Buckley 2018-12-12 02:27:41 UTC
I am not sure if you need to re-test, if you do then confirming the patch applies cleanly and the t/db_dependent/Circulation.t passes after applying the patch would be the only necessary things to test.
Comment 10 Nick Clemens 2018-12-26 20:53:25 UTC
Awesome work all!

Pushed to master for 19.05
Comment 11 Martin Renvoize 2019-01-03 14:27:51 UTC
Enhancement will not be backported to 18.11.x series