|
Description
Martin Renvoize (ashimema)
2019-06-11 09:46:10 UTC
Created attachment 97512 [details] [review] Bug 23092: Add 'daterequested' to branchtransfers table 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. Created attachment 97542 [details] [review] Bug 23092: Add 'daterequested' to branchtransfers table Created attachment 100631 [details] [review] Bug 23092: Add 'daterequested' to branchtransfers table Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 101093 [details] [review] Bug 23092: Add 'daterequested' to branchtransfers table Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Please add a test, Martin. Please clarify what sort of test you are requesting Jonathan.. I'm confused. 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? Created attachment 105912 [details] [review] Bug 23092: Add 'daterequested' to branchtransfers table Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 105913 [details] [review] Bug 23092: Add unit tests Created attachment 105916 [details] [review] Bug 23092: Add 'daterequested' to branchtransfers table Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 105917 [details] [review] Bug 23092: Add unit tests 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> Created attachment 106610 [details] [review] Bug 23092: Add unit tests Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Is there a situation currently where date requested and date sent will be different? (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. Should not it be named "requested_on" instead? (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. 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> 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> (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. Pushed to master for 20.11, thanks to everybody involved! not backporting enhancement to 20.05.x series 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. |