-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
🏗️ Added meta fields to collections table #17769
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
const {combineNonTransactionalMigrations, createAddColumnMigration} = require('../../utils'); | ||
|
||
module.exports = combineNonTransactionalMigrations( | ||
createAddColumnMigration('collections', 'og_image', { | ||
type: 'string', | ||
maxlength: 2000, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'og_title', { | ||
type: 'string', | ||
maxlength: 300, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'og_description', { | ||
type: 'string', | ||
maxlength: 500, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'twitter_image', { | ||
type: 'string', | ||
maxlength: 2000, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'twitter_title', { | ||
type: 'string', | ||
maxlength: 300, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'twitter_description', { | ||
type: 'string', | ||
maxlength: 500, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'meta_title', { | ||
type: 'string', | ||
maxlength: 2000, | ||
nullable: true, | ||
validations: { | ||
isLength: { | ||
max: 300 | ||
} | ||
} | ||
}), | ||
createAddColumnMigration('collections', 'meta_description', { | ||
type: 'string', | ||
maxlength: 2000, | ||
nullable: true, | ||
validations: { | ||
isLength: { | ||
max: 500 | ||
} | ||
} | ||
}), | ||
createAddColumnMigration('collections', 'codeinjection_head', { | ||
type: 'text', | ||
maxlength: 65535, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'codeinjection_foot', { | ||
type: 'text', | ||
maxlength: 65535, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'canonical_url', { | ||
type: 'string', | ||
maxlength: 2000, | ||
nullable: true | ||
}), | ||
createAddColumnMigration('collections', 'accent_color', { | ||
type: 'string', | ||
maxlength: 50, | ||
nullable: true | ||
}) | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1059,6 +1059,18 @@ module.exports = { | |
type: {type: 'string', maxlength: 50, nullable: false}, | ||
filter: {type: 'text', maxlength: 1000000000, nullable: true}, | ||
feature_image: {type: 'string', maxlength: 2000, nullable: true}, | ||
og_image: {type: 'string', maxlength: 2000, nullable: true}, | ||
og_title: {type: 'string', maxlength: 300, nullable: true}, | ||
og_description: {type: 'string', maxlength: 500, nullable: true}, | ||
twitter_image: {type: 'string', maxlength: 2000, nullable: true}, | ||
twitter_title: {type: 'string', maxlength: 300, nullable: true}, | ||
twitter_description: {type: 'string', maxlength: 500, nullable: true}, | ||
meta_title: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 300}}}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reasoning behind having a validation here that doesn't match the column definition? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a "soft limit" which is set in all "meta_title" fields. More context here. My understanding it that we ha put a larger field size of 2000 chars to have a buffer just in case we ever need to increase the length of the field. In reality it's been 6 years and there was no need for increase. If we are concerned about optimal field sizes, might be a good idea to trim all There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 I see Yeah that seems weird we don't do that with any other fields! |
||
meta_description: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 500}}}, | ||
codeinjection_head: {type: 'text', maxlength: 65535, nullable: true}, | ||
codeinjection_foot: {type: 'text', maxlength: 65535, nullable: true}, | ||
canonical_url: {type: 'string', maxlength: 2000, nullable: true}, | ||
accent_color: {type: 'string', maxlength: 50, nullable: true}, | ||
created_at: {type: 'dateTime', nullable: false}, | ||
updated_at: {type: 'dateTime', nullable: true} | ||
}, | ||
|
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.
This has no effect in a migration, it's used by bookshelf models only
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.
hm. true. we do use this syntax in other migrations too. I think it's here to match the syntax in
schema
to dot?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.
Yeah I've seen it in other migrations, AFAIK they aren't there for a reason though, just misunderstanding or copy/paste errors!
IMO it will cause confusion by being there but 🤷