Bug 9322 - If multiple transfers exist (from circ/branchtransfers.pl), completing one completes them all
Summary: If multiple transfers exist (from circ/branchtransfers.pl), completing one co...
Status: RESOLVED DUPLICATE of bug 19530
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: ---
Assignee: Kyle M Hall
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 8292
  Show dependency treegraph
 
Reported: 2012-12-27 01:50 UTC by Liz Rea
Modified: 2019-06-17 12:06 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9322 - If multiple transfers exist (from circ/branchtransfers.pl), completing one completes them all (1.55 KB, patch)
2013-02-05 15:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9322 - 'Resolve' previous open transfer when adding a new transfer for a given item (1.78 KB, patch)
2013-02-05 16:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9322 - 'Resolve' previous open transfer when adding a new transfer for a given item (1.87 KB, patch)
2013-02-05 16:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9322 - Delete previous open transfer when adding a new transfer for a given item (1.53 KB, patch)
2013-02-05 16:04 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Liz Rea 2012-12-27 01:50:37 UTC
To replicate: 

* get an item's barcode
* go to circulation -> transfers
* select a branch to transfer to
* put in your barcode
* submit the transfer
* select a different branch to transfer to
* (I don't know why you would do this, but) put your same barcode in again
* submit the transfer
* select a third branch to transfer to
* (again, I don't know why you would do this, but) put your same barcode in again
* submit the transfer

A query against the database will show 3 transfers: 

select * from branchtransfers;

+------------+---------------------+------------+---------------------+----------+---------------+
| itemnumber | datesent            | frombranch | datearrived         | tobranch | comments      |
+------------+---------------------+------------+---------------------+----------+---------------+
|        989 | 2012-12-27 14:31:14 | CPL        |                     | FFL      | NULL          |
|        989 | 2012-12-27 14:31:20 | FFL        |                     | FRL      | NULL          |
|        989 | 2012-12-27 14:34:22 | FRL        |                     | CPL      | NULL          |

You can see it's done something kind of smart, by putting the 2nd transfer to go from the destination of the first transfer. Makes good sense.

What doesn't make such good sense is that when you complete the first transfer, the following happens:

select * from branchtransfers;

+------------+---------------------+------------+---------------------+----------+---------------+
| itemnumber | datesent            | frombranch | datearrived         | tobranch | comments      |
+------------+---------------------+------------+---------------------+----------+---------------+
|        989 | 2012-12-27 14:31:14 | CPL        | 2012-12-27 14:34:36 | FFL      | NULL          |
|        989 | 2012-12-27 14:31:20 | FFL        | 2012-12-27 14:34:36 | FRL      | NULL          |
|        989 | 2012-12-27 14:34:22 | FRL        | 2012-12-27 14:34:36 | CPL      | NULL          |
+------------+---------------------+------------+---------------------+----------+---------------+

All three transfers are received when the first one is. The other two don't trigger. It's like they never existed.

I think this also happens when you have a reserve triggered transit in addition to a circ/branchtransfers.pl transit. This is the cause of the occasional "Item on hold for <patron>, item is already waiting" messages when you would normally be expecting a "Confirm hold for <patron>" message.
Comment 1 Liz Rea 2012-12-27 02:40:04 UTC
This will only happen if you return the item at the first one in the series - any other combination will trigger a "return this item to the branch that is expecting it" - for example

item supposed to go cpl -> ffl, 
item returned at mpl
Message: please return to FFL - this is fine and good. Do note, however, that no wrongtransfer is generated in this scenario. It probably should be, as we might want a record that it had gone to mpl instead of ffl. Currently there is none for branch transfers. (I think wrongtransfer only happens on transits initiated by reserves - but I could be wrong about that).
Comment 2 Liz Rea 2012-12-27 02:40:20 UTC

Another very bad no good scenario - 

say you have some transits placed independently of reserves:

+------------+---------------------+------------+---------------------+----------+---------------+
| itemnumber | datesent            | frombranch | datearrived         | tobranch | comments      |
+------------+---------------------+------------+---------------------+----------+---------------+
|        989 | 2012-12-27 15:25:38 | CPL        | NULL                | RPL      | NULL          |
|        989 | 2012-12-27 15:25:45 | RPL        | NULL                | SPL      | NULL          |
|        989 | 2012-12-27 15:25:51 | SPL        | NULL                | UPL      | NULL          |

Then you get one that goes with a reserve: 
+------------+---------------------+------------+---------------------+----------+---------------+
| itemnumber | datesent            | frombranch | datearrived         | tobranch | comments      |
+------------+---------------------+------------+---------------------+----------+---------------+
|        989 | 2012-12-27 15:25:38 | CPL        | NULL                | RPL      | NULL          |
|        989 | 2012-12-27 15:25:45 | RPL        | NULL                | SPL      | NULL          |
|        989 | 2012-12-27 15:25:51 | SPL        | NULL                | UPL      | NULL          |
|        989 | 2012-12-27 15:32:32 | SPL        | NULL                | CPL      | NULL          |
+------------+---------------------+------------+---------------------+----------+---------------+

Your staff member, seeing dual prompts (because you will see dual prompts here), receives the item - all of the transits are marked arrived (and thus lost), and no matter how many times you check this item in, nothing will happen - the reserve is hung up, the transits are gone, it's in a properly bad way. This will require human intervention and detective work to get out of: 
+------------+---------------------+------------+---------------------+----------+---------------+
| itemnumber | datesent            | frombranch | datearrived         | tobranch | comments      |
+------------+---------------------+------------+---------------------+----------+---------------+
|        989 | 2012-12-27 15:25:38 | CPL        | 2012-12-27 15:33:00 | RPL      | NULL          |
|        989 | 2012-12-27 15:25:45 | RPL        | 2012-12-27 15:33:00 | SPL      | NULL          |
|        989 | 2012-12-27 15:25:51 | SPL        | 2012-12-27 15:33:00 | UPL      | NULL          |
|        989 | 2012-12-27 15:32:32 | SPL        | 2012-12-27 15:33:00 | CPL      | NULL          |
+------------+---------------------+------------+---------------------+----------+---------------+
Comment 3 Liz Rea 2013-02-01 02:39:50 UTC
bug 8292 introduces an automaticitemreturn transfer that will trigger this as well.
Comment 4 Kyle M Hall 2013-02-01 12:24:48 UTC
Would it not make sense for any given item to only ever have a single row in the branch transfers table at any given time? I.e. initiating a transfer from LibraryA to LibraryB, and then a transfer from LibraryA to LibraryC will cause the system to delete the A to B, and replace it with A to C.

I cannot think of a scenario where multiple open branch transfers for a single item are useful.
Comment 5 Kyle M Hall 2013-02-05 15:54:38 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2013-02-05 15:57:19 UTC
This patch deletes the unresolved but 'overwritten' transfers. It may be a good idea to keep them around for historical data purposes, and set the date to '0000-00-00 00:00:00' to mark them as resolved but never completed.
Comment 7 Kyle M Hall 2013-02-05 16:01:16 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2013-02-05 16:02:45 UTC
Created attachment 15061 [details] [review]
Bug 9322 - 'Resolve' previous open transfer when adding a new transfer for a given item

This patch prevents multiple unresolved item transfers for
a given item to exist at the same time. If an item has an
open transfer and is subject to a second open transfer, the
previous open transfer will be given a datearrived time of
'0000-00-00 00:00:00' to indicate the transfer was resolved
but never completed.

Test Plan:
 1) Get an item's barcode
 2) Go to circulation -> transfers
 3) Select a branch to transfer to
 4) Put in your barcode
 5) Submit the transfer
 6) Examine the branchtransfers table, note that
    there is now one open transfer for the item
 7) Select a different branch to transfer to
 8) Put your same barcode in again
 9) Submit the transfer
10) Reexamine the branchtransfers table, note that
    the previous open transfer now has a datearrived
    value of '0000-00-00 00:00:00' and the new transfer
    is the only transfer for this item with a datearrived
    value of NULL
Comment 9 Kyle M Hall 2013-02-05 16:04:36 UTC
Created attachment 15062 [details] [review]
Bug 9322 - Delete previous open transfer when adding a new transfer for a given item

This patch prevents multiple unresolved item transfers for
a given item to exist at the same time.

Test Plan:
 1) Get an item's barcode
 2) Go to circulation -> transfers
 3) Select a branch to transfer to
 4) Put in your barcode
 5) Submit the transfer
 6) Examine the branchtransfers table, note that
    there is now one open transfer for the item
 7) Select a different branch to transfer to
 8) Put your same barcode in again
 9) Submit the transfer
10) Reexamine the branchtransfers table, note that
    the previous open transfer has been deleted and
    the new open transfer has replaced it.
Comment 10 Kyle M Hall 2013-02-05 16:06:42 UTC
There are now two mutually exclusive patches to resolve this issue. One deletes previous open transfers from the database, the other marks them as arrived with an arrival date of '0000-00-00 00:00:00'.

Please pick the one you feel is the better implementation and deprecate the alternative when you sign off on the patch.
Comment 11 MJ Ray (software.coop) 2013-02-28 17:07:07 UTC
Could you ask on koha mailing list/forums which approach librarians would prefer?
Comment 12 Liz Rea 2013-02-28 20:07:04 UTC
I don't personally like either of these approaches - I think only the transfer being completed should be completed - if it's for a reserve, it should only cancel the reserve transfer.

I was thinking maybe stash the reserve number in the comment or create a new field for "what kind of transfer it is (reserve, with a number, or branchtransfer, with branchtransfer.)"
Comment 13 Galen Charlton 2013-02-28 22:48:47 UTC
(In reply to comment #12)
> I don't personally like either of these approaches - I think only the
> transfer being completed should be completed - if it's for a reserve, it
> should only cancel the reserve transfer.
> 
> I was thinking maybe stash the reserve number in the comment or create a new
> field for "what kind of transfer it is (reserve, with a number, or
> branchtransfer, with branchtransfer.)"

I think adding the reserve number to the transfer record would be useful, although I prefer doing it as a real, nullable foreign key rather than sticking it in the comments.
Comment 14 Galen Charlton 2013-03-01 00:02:15 UTC
And while we're in the area ... this would be a good time to assign a numeric PK to branchtransfers.
Comment 15 MJ Ray (software.coop) 2013-03-13 19:12:22 UTC
This is being discussed on koha-devel in Feb/March. One summary was http://koha.1045719.n5.nabble.com/Opinions-needed-on-bug-9322-td5744019.html but I'm not sure whether that means either of these patches could be used - I think "a librarian might legitimately want to do chained transfers" means something else needs to be done.
Comment 16 Nick Clemens 2019-06-17 12:06:58 UTC

*** This bug has been marked as a duplicate of bug 19530 ***