🏗️ Added meta fields to collections table #17769
Conversation
closes https://github.com/TryGhost/Arch/issues/19 - Adds "meta" fields similarly to the ones in "tags" table. We need these fields in collections as collections as a resource will be routable page - should be able to have customizable meta fields for the meta HTML headers and some extras like "accent_color"
github-actions
bot
added
the
migration
|
It looks like this PR contains a migration 👀 General requirements
Schema changes
Data changes
|
refs https://github.com/TryGhost/Arch/issues/72 refs https://github.com/TryGhost/Arch/issues/19 - Added support for collection meta fields in collection entities.
| 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.
What's the reasoning behind having a validation here that doesn't match the column definition?
There was a problem hiding this comment.
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 meta_title fields to a length of 300 in Ghost v6 🤔
There was a problem hiding this comment.
🤔 I see
Yeah that seems weird we don't do that with any other fields!
| type: 'string', | ||
| maxlength: 2000, | ||
| nullable: true, | ||
| validations: { |
There was a problem hiding this comment.
This has no effect in a migration, it's used by bookshelf models only
There was a problem hiding this comment.
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.
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 🤷
|
this should be picked up in few weeks time. |
refs https://github.com/TryGhost/Arch/issues/19
Extra opinion to use the same table for meta fields by @ErisDS :
🤖 Generated by Copilot at 7aabfb5
This pull request adds new meta fields to the
collectionstable to enable collection customization. It includes a migration script, a schema update, and a test update to reflect the new columns and their properties.