Created attachment 39058 [details] [review] Bug 14187 - branchtransfer needs a primary key (id) for DBIx and common sense.
Created attachment 39059 [details] [review] Bug 14187 - branchtransfer needs a primary key (id) for DBIx and common sense.
Created attachment 39060 [details] [review] Bug 14187 - branchtransfer needs a primary key (id) for DBIx and common sense.
Perhaps the key is needed, also a test plan. And rewrite database update as atomicupdate.
What do you mean with atomicupdate? All existing unit tests pass and this doesn't include any new testable behaviour.
The primary key is needed if you want to do anything with DBIx.
Olli, there is a new way to write database updates - atomicupdate - see here: http://wiki.koha-community.org/wiki/Database_updates I think what Bernardo meant was not unit tests but a better description of your change in the commit message and hints on how to test this.
Created attachment 40075 [details] [review] Bug 14187 - branchtransfer needs a primary key (id) for DBIx and common sense.
My intention is to add atomicupdate, and ensure this still applies nicely on master in order to submit it. Alex
Created attachment 58121 [details] [review] Bug 14187 - branchtransfer needs a primary key (id) for DBIx and common sense. * C4/Circulation.pm (GetTransfers, GetTransfersFromTo): Also return branchtransfer_id in return columns. * installer/data/mysql/atomicupdate/14187.perl: New file. * installer/data/mysql/kohastructure.sql: Modify branchtransfers structure. * t/db_dependent/Circulation/transfers.t: Update tests to expect branchtransfer_id.
Created attachment 58123 [details] [review] Bug 14187 - branchtransfer needs a primary key (id) for DBIx and common sense. * C4/Circulation.pm (GetTransfers, GetTransfersFromTo): Also return branchtransfer_id in return columns. * installer/data/mysql/atomicupdate/14187.perl: New file. * installer/data/mysql/kohastructure.sql: Modify branchtransfers structure. * t/db_dependent/Circulation/transfers.t: Update tests to expect branchtransfer_id.
Created attachment 58124 [details] [review] DO NOT PUSH: Updated Branchtransfer schema. * Koha/Schema/Result/Branchtransfer.pm: Newly generated.
Hello, I've now been able to: - [X] Re-checked code paths to see if all required changes were made - [X] Updated test files - [X] Added atomic update script - [X] Tested database update & new database creation - [X] Separated out schema changes - [X] Ran QA scripts - [X] Upload new patches. This should now be ready for sign-off/QA. Alex
Test Plan - Ensure the patch applies cleanly - Run unit tests, and ensure no regression (especially t/db_dependent/Circulation/transfers.t) - Set in motion a branch transfer (e.g. check in an item at a branch that is not it's home branch when floating stock is switched off) - Receive a branch transfer (check in an item previously in branch transfer) Alex
Created attachment 58646 [details] [review] [SIGNED-OFF] Bug 14187 - branchtransfer needs a primary key (id) for DBIx and common sense. * C4/Circulation.pm (GetTransfers, GetTransfersFromTo): Also return branchtransfer_id in return columns. * installer/data/mysql/atomicupdate/14187.perl: New file. * installer/data/mysql/kohastructure.sql: Modify branchtransfers structure. * t/db_dependent/Circulation/transfers.t: Update tests to expect branchtransfer_id. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Test plan successful on all steps.
Created attachment 58730 [details] [review] Bug 14187 - branchtransfer needs a primary key (id) for DBIx and common sense. * C4/Circulation.pm (GetTransfers, GetTransfersFromTo): Also return branchtransfer_id in return columns. * installer/data/mysql/atomicupdate/14187.perl: New file. * installer/data/mysql/kohastructure.sql: Modify branchtransfers structure. * t/db_dependent/Circulation/transfers.t: Update tests to expect branchtransfer_id. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Test plan successful on all steps. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Amended patch: Remove Schema changes from this patch
Created attachment 58880 [details] [review] Bug 14187: (QA followup) Properly check DB structure before altering it Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 58881 [details] [review] Bug 14187: branchtransfer needs a primary key (id) for DBIx and common sense. * C4/Circulation.pm (GetTransfers, GetTransfersFromTo): Also return branchtransfer_id in return columns. * installer/data/mysql/atomicupdate/14187.perl: New file. * installer/data/mysql/kohastructure.sql: Modify branchtransfers structure. * t/db_dependent/Circulation/transfers.t: Update tests to expect branchtransfer_id. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Test plan successful on all steps. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Amended patch: Remove Schema changes from this patch Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 58882 [details] [review] Bug 14187: (QA followup) Properly check DB structure before altering it Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 17.11, thanks Olli, Tomas!
This won't get ported back to 16.11.x as it is an enhancement.