Bug 24298 - Record if a transfer was triggered by 'return to homebranch'
Summary: Record if a transfer was triggered by 'return to homebranch'
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 24296
Blocks: 24898 24299
  Show dependency treegraph
 
Reported: 2019-12-23 09:45 UTC by Martin Renvoize
Modified: 2020-11-30 21:49 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00


Attachments
Bug 24298: Add return enums to branchstransfer triggers (2.32 KB, patch)
2020-01-16 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Record transfer triggers for returns (6.02 KB, patch)
2020-01-16 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Add return enums to branchstransfer triggers (2.31 KB, patch)
2020-01-17 11:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Record transfer triggers for returns (6.02 KB, patch)
2020-01-17 11:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Add return enums to branchstransfer triggers (2.30 KB, patch)
2020-01-17 16:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Record transfer triggers for returns (6.02 KB, patch)
2020-01-17 16:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Add return enums to branchstransfer triggers (2.30 KB, patch)
2020-02-03 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Record transfer triggers for returns (6.02 KB, patch)
2020-02-03 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Add tests for ModItemTransfer (3.09 KB, patch)
2020-02-03 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Add return enums to branchstransfer triggers (2.30 KB, patch)
2020-02-28 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Record transfer triggers for returns (6.02 KB, patch)
2020-02-28 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Add tests for ModItemTransfer (3.09 KB, patch)
2020-02-28 15:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: (follow-up) Add missing filter! (1.44 KB, patch)
2020-02-28 15:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24298: Add return enums to branchstransfer triggers (2.38 KB, patch)
2020-02-28 16:30 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 24298: Record transfer triggers for returns (6.32 KB, patch)
2020-02-28 16:30 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 24298: Add tests for ModItemTransfer (3.19 KB, patch)
2020-02-28 16:31 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 24298: (follow-up) Add missing filter! (1.50 KB, patch)
2020-02-28 16:31 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 24298: Add return enums to branchstransfer triggers (2.45 KB, patch)
2020-03-12 12:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24298: Record transfer triggers for returns (6.39 KB, patch)
2020-03-12 12:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24298: Add tests for ModItemTransfer (3.25 KB, patch)
2020-03-12 12:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24298: (follow-up) Add missing filter! (1.55 KB, patch)
2020-03-12 12:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24298: Update test count and use build_sample_item (3.20 KB, patch)
2020-03-12 12:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24298: (RM follow-up) Re-introduce TransferTrigger drop through (941 bytes, patch)
2020-04-27 13:25 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2019-12-23 09:45:09 UTC
Use the new 'trigger' field in branchtransfers to record when a transfer was triggered automatically by the return to home branch settings.
Comment 1 Martin Renvoize 2020-01-16 15:37:09 UTC
Created attachment 97450 [details] [review]
Bug 24298: Add return enums to branchstransfer triggers
Comment 2 Martin Renvoize 2020-01-16 15:37:12 UTC
Created attachment 97451 [details] [review]
Bug 24298: Record transfer triggers for returns

This patch adds handling to catch transfers that may have been triggered
on item return.
Comment 3 Martin Renvoize 2020-01-17 11:44:56 UTC
Created attachment 97506 [details] [review]
Bug 24298: Add return enums to branchstransfer triggers
Comment 4 Martin Renvoize 2020-01-17 11:44:59 UTC
Created attachment 97507 [details] [review]
Bug 24298: Record transfer triggers for returns

This patch adds handling to catch transfers that may have been triggered
on item return.
Comment 5 Martin Renvoize 2020-01-17 11:47:19 UTC
Test Plan:

1) Enable AutoItemReturn and accompanying system preference
2) Return an item to the wrong branch and note the transfer
3) Check the database for the new trigger value
Comment 6 Martin Renvoize 2020-01-17 16:00:54 UTC
Created attachment 97535 [details] [review]
Bug 24298: Add return enums to branchstransfer triggers
Comment 7 Martin Renvoize 2020-01-17 16:00:57 UTC
Created attachment 97536 [details] [review]
Bug 24298: Record transfer triggers for returns

This patch adds handling to catch transfers that may have been triggered
on item return.
Comment 8 Martin Renvoize 2020-02-03 13:22:09 UTC
Created attachment 98292 [details] [review]
Bug 24298: Add return enums to branchstransfer triggers
Comment 9 Martin Renvoize 2020-02-03 13:22:12 UTC
Created attachment 98293 [details] [review]
Bug 24298: Record transfer triggers for returns

This patch adds handling to catch transfers that may have been triggered
on item return.
Comment 10 Martin Renvoize 2020-02-03 13:22:15 UTC
Created attachment 98294 [details] [review]
Bug 24298: Add tests for ModItemTransfer
Comment 11 Bernardo Gonzalez Kriegel 2020-02-28 11:45:57 UTC
Hi,
qa tools complains

FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
   FAIL   filters
                missing_filter at line 522 (                                                    <input type="hidden" name="trigger" value="[% trigger %]" />)

$raw/html?
Comment 12 Martin Renvoize 2020-02-28 15:47:54 UTC
Created attachment 99774 [details] [review]
Bug 24298: Add return enums to branchstransfer triggers
Comment 13 Martin Renvoize 2020-02-28 15:47:57 UTC
Created attachment 99775 [details] [review]
Bug 24298: Record transfer triggers for returns

This patch adds handling to catch transfers that may have been triggered
on item return.
Comment 14 Martin Renvoize 2020-02-28 15:48:00 UTC
Created attachment 99776 [details] [review]
Bug 24298: Add tests for ModItemTransfer
Comment 15 Martin Renvoize 2020-02-28 15:48:03 UTC
Created attachment 99777 [details] [review]
Bug 24298: (follow-up) Add missing filter!
Comment 16 Martin Renvoize 2020-02-28 15:48:33 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #11)
> Hi,
> qa tools complains
> 
> FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
>    FAIL   filters
>                 missing_filter at line 522 (                                
> <input type="hidden" name="trigger" value="[% trigger %]" />)
> 
> $raw/html?

Thanks for taking a look Bernardo.. all fixed up in the followup :)
Comment 17 Bernardo Gonzalez Kriegel 2020-02-28 16:30:54 UTC
Created attachment 99781 [details] [review]
Bug 24298: Add return enums to branchstransfer triggers

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
updatedabase runs Ok
Comment 18 Bernardo Gonzalez Kriegel 2020-02-28 16:30:58 UTC
Created attachment 99782 [details] [review]
Bug 24298: Record transfer triggers for returns

This patch adds handling to catch transfers that may have been triggered
on item return.

Test Plan:

1) Enable AutomaticItemReturn and accompanying system preference
2) Return an item to the wrong branch and note the transfer
3) Check the database for the new trigger value

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Added test plan. Works Ok, trigger saved in database.
Comment 19 Bernardo Gonzalez Kriegel 2020-02-28 16:31:02 UTC
Created attachment 99783 [details] [review]
Bug 24298: Add tests for ModItemTransfer

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
All 8 tests in 'ModItemTransfer tests' pass
Comment 20 Bernardo Gonzalez Kriegel 2020-02-28 16:31:06 UTC
Created attachment 99784 [details] [review]
Bug 24298: (follow-up) Add missing filter!

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 21 Bernardo Gonzalez Kriegel 2020-02-28 16:32:38 UTC
(In reply to Martin Renvoize from comment #5)
> 1) Enable AutoItemReturn and accompanying system preference

It is AutomaticItemReturn :)
Comment 22 Nick Clemens 2020-03-12 12:15:49 UTC
Created attachment 100621 [details] [review]
Bug 24298: Add return enums to branchstransfer triggers

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
updatedabase runs Ok

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Nick Clemens 2020-03-12 12:15:53 UTC
Created attachment 100622 [details] [review]
Bug 24298: Record transfer triggers for returns

This patch adds handling to catch transfers that may have been triggered
on item return.

Test Plan:

1) Enable AutomaticItemReturn and accompanying system preference
2) Return an item to the wrong branch and note the transfer
3) Check the database for the new trigger value

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Added test plan. Works Ok, trigger saved in database.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Nick Clemens 2020-03-12 12:15:56 UTC
Created attachment 100623 [details] [review]
Bug 24298: Add tests for ModItemTransfer

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
All 8 tests in 'ModItemTransfer tests' pass

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 25 Nick Clemens 2020-03-12 12:15:59 UTC
Created attachment 100624 [details] [review]
Bug 24298: (follow-up) Add missing filter!

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Nick Clemens 2020-03-12 12:16:02 UTC
Created attachment 100625 [details] [review]
Bug 24298: Update test count and use build_sample_item

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Nick Clemens 2020-03-12 12:27:07 UTC
One note here: AddIssue closes existing transfers but doesn't update the reason - probably 'Checkout' should be a transfer reason - this can be done on another bug I think
Comment 28 Martin Renvoize 2020-03-18 19:06:47 UTC
(In reply to Nick Clemens from comment #27)
> One note here: AddIssue closes existing transfers but doesn't update the
> reason - probably 'Checkout' should be a transfer reason - this can be done
> on another bug I think

Hmm, yeah, I remember spotting that and pondering how to handle it.

I think I opted at the time to defer that work.. the 'reason' is really all about what triggered the transfer in the first place rather than if/why it may have been cancelled.  It's pretty bad that we wipe out all trace of where the item as meant to be getting transferred to in my opinion.. even if we cancel it we aught to really record where it was meant to go in case we want to restore it later.

I think, as you say, I'll open another bug to work no this however.
Comment 29 Martin Renvoize 2020-03-19 08:56:55 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 30 Martin Renvoize 2020-03-19 09:23:35 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 31 Joy Nelson 2020-04-03 20:17:33 UTC
enhancement not backported to 19.11.x
Comment 32 Emmi Takkinen 2020-04-24 11:27:39 UTC
Shouldn't 'TransferTrigger' be included as empty elseif statement inside error messages loop in returns.pl? Now when I try to return item needing transfer with 'AutomaticItemReturn' disabled I get error: 

'Unknown error code TransferTrigger at /var/Koha/circ/returns.pl line 542.'
Comment 33 Martin Renvoize 2020-04-27 13:16:43 UTC
Taking a look now, thanks
Comment 34 Martin Renvoize 2020-04-27 13:25:15 UTC
Created attachment 103792 [details] [review]
Bug 24298: (RM follow-up) Re-introduce TransferTrigger drop through

A rebase lost the fall through for TransferTrigger as a message. This
patch simply adds back in the dropped elsif statement.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 35 Martin Renvoize 2020-04-27 13:25:38 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 36 Martin Renvoize 2020-04-27 13:26:26 UTC
Good catch.. seems we lost it in a rebase somewhere along the line.. it was in my original local branch :)

Pushing as an RM followup now