Bug 23092 - Transfers should record 'requested' as well as 'sent' and 'arrived' dates
Summary: Transfers should record 'requested' as well as 'sent' and 'arrived' dates
Status: Pushed to master
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: 24297
Blocks: 25755 26057
  Show dependency treegraph
 
Reported: 2019-06-11 09:46 UTC by Martin Renvoize
Modified: 2020-07-31 20:14 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 23092: Add 'daterequested' to branchtransfers table (2.16 KB, patch)
2020-01-17 12:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23092: Add 'daterequested' to branchtransfers table (2.16 KB, patch)
2020-01-17 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23092: Add 'daterequested' to branchtransfers table (2.22 KB, patch)
2020-03-12 12:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23092: Add 'daterequested' to branchtransfers table (3.48 KB, patch)
2020-03-20 12:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23092: Add 'daterequested' to branchtransfers table (3.48 KB, patch)
2020-06-16 12:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23092: Add unit tests (1.92 KB, patch)
2020-06-16 12:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23092: Add 'daterequested' to branchtransfers table (3.48 KB, patch)
2020-06-16 13:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23092: Add unit tests (2.30 KB, patch)
2020-06-16 13:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23092: Add 'daterequested' to branchtransfers table (3.55 KB, patch)
2020-07-06 21:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23092: Add unit tests (2.36 KB, patch)
2020-07-06 21:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23092: Add 'daterequested' to branchtransfers table (3.60 KB, patch)
2020-07-23 13:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23092: Add unit tests (2.42 KB, patch)
2020-07-23 13:41 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-06-11 09:46:10 UTC
The transfers system is very much tied to the Holds/Reserves features at the moment, but slowly some other subsystems are starting to use it too (Stock Rotation, Rotating Collections).

For full audit purposes we should record the 'requested' date along with the 'sent' and 'arrived' dates.

Currently stock rotation misuses 'sent' to mean 'requested', so the item may have not actually been picked off the shelf, captured at checkin, and put into physical transit to the next library.
Comment 1 Martin Renvoize 2020-01-17 12:06:28 UTC
Created attachment 97512 [details] [review]
Bug 23092: Add 'daterequested' to branchtransfers table
Comment 2 Martin Renvoize 2020-01-17 12:08:09 UTC
This patch simply adds the field and sets it's to default to current timestamp on row creation.

As such it does not yet affect any functionality so the test plan is simply to check the field was created in the database.

Updating transfers to use this functionality will be done in dependant bugs.
Comment 3 Martin Renvoize 2020-01-17 16:03:23 UTC
Created attachment 97542 [details] [review]
Bug 23092: Add 'daterequested' to branchtransfers table
Comment 4 Nick Clemens 2020-03-12 12:58:19 UTC
Created attachment 100631 [details] [review]
Bug 23092: Add 'daterequested' to branchtransfers table

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Martin Renvoize 2020-03-20 12:31:56 UTC
Created attachment 101093 [details] [review]
Bug 23092: Add 'daterequested' to branchtransfers table

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Jonathan Druart 2020-04-27 10:35:45 UTC
Please add a test, Martin.
Comment 7 Martin Renvoize 2020-06-15 15:19:37 UTC
Please clarify what sort of test you are requesting Jonathan.. I'm confused.
Comment 8 Jonathan Druart 2020-06-16 08:40:25 UTC
We want to test the behaviour of this field. As it has no code we should highlight in the test what we are expecting from the DBMS.
Here it seems that we want it to be set to now when, and only when, a transfer is created. Does that make sense?
Comment 9 Martin Renvoize 2020-06-16 12:51:27 UTC
Created attachment 105912 [details] [review]
Bug 23092: Add 'daterequested' to branchtransfers table

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Martin Renvoize 2020-06-16 12:51:30 UTC
Created attachment 105913 [details] [review]
Bug 23092: Add unit tests
Comment 11 Martin Renvoize 2020-06-16 13:21:11 UTC
Created attachment 105916 [details] [review]
Bug 23092: Add 'daterequested' to branchtransfers table

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Martin Renvoize 2020-06-16 13:21:14 UTC
Created attachment 105917 [details] [review]
Bug 23092: Add unit tests
Comment 13 Katrin Fischer 2020-07-06 21:14:06 UTC
Created attachment 106609 [details] [review]
Bug 23092: Add 'daterequested' to branchtransfers table

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2020-07-06 21:14:10 UTC
Created attachment 106610 [details] [review]
Bug 23092: Add unit tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2020-07-06 21:14:50 UTC
Is there a situation currently where date requested and date sent will be different?
Comment 16 Martin Renvoize 2020-07-08 09:33:26 UTC
(In reply to Katrin Fischer from comment #15)
> Is there a situation currently where date requested and date sent will be
> different?

Stock rotation use a cronjob to trigger transfers.. I think there's at least one other case too.. I've got a branch ongoing where I update the logic codebase wide (via a bunch of bugs linked to this one) to utilise this.
Comment 17 Jonathan Druart 2020-07-20 14:06:44 UTC
Should not it be named "requested_on" instead?
Comment 18 Martin Renvoize 2020-07-21 14:37:37 UTC
(In reply to Jonathan Druart from comment #17)
> Should not it be named "requested_on" instead?

So.. yes I agree, but at the same time I was trying to maintain consistency with the existing fields in the table.  Happy to go either way really.

As a side point.. I'm going to mark this one as BLOCKED for now as I'd really like to get the rest of the tree of dependants back into shape rather than just introduce this without the corresponding use cases.
Comment 19 Martin Renvoize 2020-07-23 13:41:14 UTC
Created attachment 107238 [details] [review]
Bug 23092: Add 'daterequested' to branchtransfers table

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Martin Renvoize 2020-07-23 13:41:17 UTC
Created attachment 107239 [details] [review]
Bug 23092: Add unit tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 21 Martin Renvoize 2020-07-23 13:46:29 UTC
(In reply to Jonathan Druart from comment #17)
> Should not it be named "requested_on" instead?

I'd like to maintain table consistency and handle updating all the filed to be `_on` in another bug.
Comment 22 Jonathan Druart 2020-07-24 12:14:00 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 23 Lucas Gass 2020-07-31 20:14:00 UTC
not backporting enhancement to 20.05.x series