-
Notifications
You must be signed in to change notification settings - Fork 295
fix(database): Add foreign keys to *_mailbox_id fields in mail_accounts table
#12220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(database): Add foreign keys to *_mailbox_id fields in mail_accounts table
#12220
Conversation
b657442 to
6162415
Compare
6162415 to
949f5cd
Compare
949f5cd to
7ee1b92
Compare
| public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { | ||
| $schema = $schemaClosure(); | ||
|
|
||
| if ($schema->hasTable('mail_accounts')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: What's the best-practice with the hasTable() / hasColumn() / etc. checks? Is it even necessary here? I've done it because it's being done in older migrations as well, but I'm not sure why it's done there 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The checks shouldn't be necessary ideally, but in case of a migration error it should be possible to re-run that migration.
Imagine a migration creates two tables: t1 and t2. Creating t2 failed (something temporary that the admin could resolve, or a patch is applied). The upgrade runs the same migration again, because the migration is not yet written to oc_migrations.
If the migration tries to create t1 no matter if it exists, it will now fail here. If the migration has a check it can skip creating t1 and creates t2.
This is why we add the table/column exists checks :)
…s table Signed-off-by: David Dreschner <[email protected]>
7ee1b92 to
d6e5430
Compare
The
mail_accountstable includes several fields with references to themail_mailboxestable. This relation wasn't declared via foreign keys previously, which means that mailboxes with a special meaning (e.g., Junk) can be deleted without removing them from the corresponding*_mailbox_idfield. Inconsistent entries are being set toNULLbefore changing the database schema.Testing
This fixes #11243.