Bug 23092

Summary: Add 'daterequested' to the transfers table
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: caroline.cyr-la-rose, gmcharlt, jonathan.druart, kyle.m.hall, lucas, sally.healey
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Bug Depends on: 24297    
Bug Blocks: 29594, 30581, 25755, 29792    
Attachments: Bug 23092: Add 'daterequested' to branchtransfers table
Bug 23092: Add 'daterequested' to branchtransfers table
Bug 23092: Add 'daterequested' to branchtransfers table
Bug 23092: Add 'daterequested' to branchtransfers table
Bug 23092: Add 'daterequested' to branchtransfers table
Bug 23092: Add unit tests
Bug 23092: Add 'daterequested' to branchtransfers table
Bug 23092: Add unit tests
Bug 23092: Add 'daterequested' to branchtransfers table
Bug 23092: Add unit tests
Bug 23092: Add 'daterequested' to branchtransfers table
Bug 23092: Add unit tests

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
Comment 24 Caroline Cyr La Rose 2020-08-20 21:11:32 UTC
With the release notes in mind, I changed the component from "Circulation" to "Architecture, internals and plumbing" as this only affects the database, and it's not actually a circulation functionality. I also changed the title a bit to reflect this.

I hope it's ok. Feel free to change it back if you think this is wrong.