In order to be used correctly on DBIC the table needs a primary key. Otherwise calls to ->delete would fail.
Created attachment 58422 [details] [review] Bug 17813: Add a primary key to borrower_attributes This patch adds 'borrower_attributes' a field (if) which will act as a primary key. This is needed for DBIC to be used to handle rows, and also will help when faced with the implementation of the REST api for this resource. To test: - Run all patron modification / attributes and verify nothing breaks - Sign off :-D
Created attachment 58423 [details] [review] Bug 17813: DBIC update
Created attachment 58493 [details] [review] Bug 17813: Add a primary key to borrower_attributes This patch adds 'borrower_attributes' a field (if) which will act as a primary key. This is needed for DBIC to be used to handle rows, and also will help when faced with the implementation of the REST api for this resource. To test: - Run all patron modification / attributes and verify nothing breaks - Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 58494 [details] [review] Bug 17813: DBIC update Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
I think we need to provide a version using column_exists, from bug 17234.
Created attachment 58879 [details] [review] Bug 17813: (QA followup) Properly check DB structure before altering it This patch makes the atomic update use the new functions introduced by bug 17234 for checking the structure before attempting to call ALTER TABLE. It checks for the column existence, and also if it is a primary key. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 59070 [details] [review] Bug 17813: Add a primary key to borrower_attributes This patch adds 'borrower_attributes' a field (if) which will act as a primary key. This is needed for DBIC to be used to handle rows, and also will help when faced with the implementation of the REST api for this resource. To test: - Run all patron modification / attributes and verify nothing breaks - Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 59071 [details] [review] Bug 17813: DBIC update Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 59072 [details] [review] Bug 17813: (QA followup) Properly check DB structure before altering it This patch makes the atomic update use the new functions introduced by bug 17234 for checking the structure before attempting to call ALTER TABLE. It checks for the column existence, and also if it is a primary key. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 17.05, thanks Tomas!
I have changed this to 'enh' as I think while it's needed to move forward it's not fixing a bug at the moment (false testable behaviour). Not including this in 16.11.2.
Just wondering - why don't we replace the old for the new variable in the notices with SQL? I think we have done that once when we added the new syntax for <<items.fine>> already.
(In reply to Katrin Fischer from comment #12) > Just wondering - why don't we replace the old for the new variable in the > notices with SQL? I think we have done that once when we added the new > syntax for <<items.fine>> already. There is no old column, only a new one.