Bug 23066

Summary: Add foreign key for issues tables to branches table for branchcodes
Product: Koha Reporter: Kyle M Hall (khall) <kyle>
Component: DatabaseAssignee: Kyle M Hall (khall) <kyle>
Status: In Discussion --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: dcook, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 15985    
Bug Blocks:    
Attachments: Bug 23066: Add foreign key for issues tables to branches table for branchcodes - Update DB
Bug 23066: Add foreign key for issues tables to branches table for branchcodes - Update DB
Bug 23066: Add foreign key for issues tables to branches table for branchcodes - Update DB
Bug 23066: Add foreign key for issues tables to branches table for branchcodes - Update DB

Description Kyle M Hall (khall) 2019-06-06 13:41:10 UTC
We should really have foreign key constrains between the branches table and the issues and old_issues tables.
Comment 1 Kyle M Hall (khall) 2019-06-06 13:42:28 UTC
Created attachment 90372 [details] [review]
Bug 23066: Add foreign key for issues tables to branches table for branchcodes - Update DB

We should really have foreign key constrains between the branches table and the issues and old_issues tables.
Comment 2 Kyle M Hall (khall) 2019-06-06 13:43:53 UTC
Created attachment 90373 [details] [review]
Bug 23066: Add foreign key for issues tables to branches table for branchcodes - Update DB

We should really have foreign key constrains between the branches table and the issues and old_issues tables.
Comment 3 Kyle M Hall (khall) 2019-06-06 13:45:01 UTC
Created attachment 90374 [details] [review]
Bug 23066: Add foreign key for issues tables to branches table for branchcodes - Update DB

We should really have foreign key constrains between the branches table and the issues and old_issues tables.
Comment 4 Katrin Fischer 2019-07-11 06:31:16 UTC
Hi Kyle, should this be NSO?
Comment 5 David Cook 2019-08-09 00:54:07 UTC
This makes me so happy. I was just about to report this, so I'm glad to see the issue and a patch. 

I'm in a rush at the moment, but I'll make a note to come back to this to test and sign off. 

(I wish that there were an alternative to "SET NULL" for old_issues, but I don't think there is with our current architecture.)

A library of ours got bitten by this. They moved their items to a new branch, deleted the old branch, but their issues are all for the old non-existent branch. 

I took a little look around and it seems like these might be the only relevant tables missing the foreign key relationship to the branches table. 

Once again, thanks Kyle!
Comment 6 Katrin Fischer 2019-08-13 05:43:43 UTC
Created attachment 92167 [details] [review]
Bug 23066: Add foreign key for issues tables to branches table for branchcodes - Update DB

We should really have foreign key constrains between the branches table and the issues and old_issues tables.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Marcel de Rooy 2019-08-16 06:56:29 UTC
Please send a mail to the list asking for further feedback from the community before adding this constraint. It is obvious at first sight, but why didn't we add it years ago then? Anything to do with keeping the old branchcode in old issue records or so?
Comment 8 David Cook 2019-12-03 09:11:19 UTC
(In reply to Marcel de Rooy from comment #7)
> Please send a mail to the list asking for further feedback from the
> community before adding this constraint. It is obvious at first sight, but
> why didn't we add it years ago then? Anything to do with keeping the old
> branchcode in old issue records or so?

A search of git suggests it might have just been an oversight?

In master kohastructure.sql we see the following for the branchcode column:

`branchcode` varchar(10) default NULL, -- foreign key, linking to the branches table for the location the item was checked out

However, there is no foreign key. 

It appears the comment was added by commit 91b5a09c6ce (Sun Aug 2011) but even then there was no foreign key there.