Removed bool type from schema

refs https://github.com/TryGhost/Toolbox/issues/441

- we tend to have a mix of `bool` and `boolean` in the schema and
  migrations, which has become a real nit for me at this point
- we don't do any special handling between `bool` and `boolean`, it's
  just something we pass to Knex
- `bool` is an alias for `boolean` but `boolean` is actually documented - https://knexjs.org/guide/schema-builder.html#boolean
- this commit switches Ghost to only using `boolean` in the schema and
  migrations, and removes `bool` from the allowlist in tests to prevent
  us from adding it again in the future
- this should make absolutely no difference to the DB because both
  resulted in the same column
This commit is contained in:
Daniel Lockyer 2022-10-12 21:53:43 +07:00 committed by Daniel Lockyer
parent 3917a058a4
commit 143ae857c9
13 changed files with 30 additions and 44 deletions

View File

@ -3,7 +3,7 @@ const {addTable} = require('../../utils');
module.exports = addTable('members_subscribe_events', {
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
member_id: {type: 'string', maxlength: 24, nullable: false, unique: false, references: 'members.id', cascadeDelete: true},
subscribed: {type: 'bool', nullable: false, defaultTo: true},
subscribed: {type: 'boolean', nullable: false, defaultTo: true},
created_at: {type: 'dateTime', nullable: false},
source: {type: 'string', maxlength: 50, nullable: true}
});

View File

@ -1,7 +1,7 @@
const {createAddColumnMigration} = require('../../utils');
module.exports = createAddColumnMigration('posts_meta', 'email_only', {
type: 'bool',
type: 'boolean',
nullable: false,
defaultTo: false
});

View File

@ -4,7 +4,7 @@ module.exports = addTable('stripe_prices', {
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
stripe_price_id: {type: 'string', maxlength: 255, nullable: false, unique: true},
stripe_product_id: {type: 'string', maxlength: 255, nullable: false, unique: false, references: 'stripe_products.stripe_product_id', cascadeDelete: true},
active: {type: 'bool', nullable: false},
active: {type: 'boolean', nullable: false},
nickname: {type: 'string', maxlength: 50, nullable: true},
currency: {type: 'string', maxLength: 3, nullable: false},
amount: {type: 'integer', nullable: false},

View File

@ -12,7 +12,7 @@ module.exports = addTable('newsletters', {
sender_name: {type: 'string', maxlength: 191, nullable: false},
sender_email: {type: 'string', maxlength: 191, nullable: false, validations: {isEmail: true}},
sender_reply_to: {type: 'string', maxlength: 191, nullable: false, validations: {isEmail: true}},
default: {type: 'bool', nullable: false, defaultTo: false},
default: {type: 'boolean', nullable: false, defaultTo: false},
status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'active'},
recipient_filter: {
type: 'text',
@ -20,6 +20,6 @@ module.exports = addTable('newsletters', {
nullable: false,
defaultTo: ''
},
subscribe_on_signup: {type: 'bool', nullable: false, defaultTo: false},
subscribe_on_signup: {type: 'boolean', nullable: false, defaultTo: false},
sort_order: {type: 'integer', nullable: false, unsigned: true, defaultTo: 0}
});

View File

@ -15,15 +15,15 @@ module.exports = recreateTable('newsletters', {
nullable: false,
defaultTo: 'members'
},
subscribe_on_signup: {type: 'bool', nullable: false, defaultTo: true},
subscribe_on_signup: {type: 'boolean', nullable: false, defaultTo: true},
sort_order: {type: 'integer', nullable: false, unsigned: true, defaultTo: 0},
header_image: {type: 'string', maxlength: 2000, nullable: true},
show_header_icon: {type: 'bool', nullable: false, defaultTo: true},
show_header_title: {type: 'bool', nullable: false, defaultTo: true},
show_header_icon: {type: 'boolean', nullable: false, defaultTo: true},
show_header_title: {type: 'boolean', nullable: false, defaultTo: true},
title_font_category: {type: 'string', maxlength: 191, nullable: false, defaultTo: 'sans_serif', validations: {isIn: [['serif', 'sans_serif']]}},
title_alignment: {type: 'string', maxlength: 191, nullable: false, defaultTo: 'center', validations: {isIn: [['center', 'left']]}},
show_feature_image: {type: 'bool', nullable: false, defaultTo: true},
show_feature_image: {type: 'boolean', nullable: false, defaultTo: true},
body_font_category: {type: 'string', maxlength: 191, nullable: false, defaultTo: 'sans_serif', validations: {isIn: [['serif', 'sans_serif']]}},
footer_content: {type: 'text', maxlength: 1000000000, nullable: true},
show_badge: {type: 'bool', nullable: false, defaultTo: true}
show_badge: {type: 'boolean', nullable: false, defaultTo: true}
});

View File

@ -1,7 +1,7 @@
const {createAddColumnMigration} = require('../../utils');
module.exports = createAddColumnMigration('emails', 'track_clicks', {
type: 'bool',
nullable: false,
type: 'boolean',
nullable: false,
defaultTo: false
});

View File

@ -1,7 +1,7 @@
const {createAddColumnMigration} = require('../../utils');
module.exports = createAddColumnMigration('newsletters', 'feedback_enabled', {
type: 'bool',
type: 'boolean',
nullable: false,
defaultTo: false
});

View File

@ -13,7 +13,7 @@ module.exports = {
uuid: {type: 'string', maxlength: 36, nullable: false, unique: true, validations: {isUUID: true}},
name: {type: 'string', maxlength: 191, nullable: false, unique: true},
description: {type: 'string', maxlength: 2000, nullable: true},
feedback_enabled: {type: 'bool', nullable: false, defaultTo: false},
feedback_enabled: {type: 'boolean', nullable: false, defaultTo: false},
slug: {type: 'string', maxlength: 191, nullable: false, unique: true},
sender_name: {type: 'string', maxlength: 191, nullable: true},
sender_email: {type: 'string', maxlength: 191, nullable: true},
@ -25,18 +25,18 @@ module.exports = {
nullable: false,
defaultTo: 'members'
},
subscribe_on_signup: {type: 'bool', nullable: false, defaultTo: true},
subscribe_on_signup: {type: 'boolean', nullable: false, defaultTo: true},
sort_order: {type: 'integer', nullable: false, unsigned: true, defaultTo: 0},
header_image: {type: 'string', maxlength: 2000, nullable: true},
show_header_icon: {type: 'bool', nullable: false, defaultTo: true},
show_header_title: {type: 'bool', nullable: false, defaultTo: true},
show_header_icon: {type: 'boolean', nullable: false, defaultTo: true},
show_header_title: {type: 'boolean', nullable: false, defaultTo: true},
title_font_category: {type: 'string', maxlength: 191, nullable: false, defaultTo: 'sans_serif', validations: {isIn: [['serif', 'sans_serif']]}},
title_alignment: {type: 'string', maxlength: 191, nullable: false, defaultTo: 'center', validations: {isIn: [['center', 'left']]}},
show_feature_image: {type: 'bool', nullable: false, defaultTo: true},
show_feature_image: {type: 'boolean', nullable: false, defaultTo: true},
body_font_category: {type: 'string', maxlength: 191, nullable: false, defaultTo: 'sans_serif', validations: {isIn: [['serif', 'sans_serif']]}},
footer_content: {type: 'text', maxlength: 1000000000, nullable: true},
show_badge: {type: 'bool', nullable: false, defaultTo: true},
show_header_name: {type: 'bool', nullable: false, defaultTo: true},
show_badge: {type: 'boolean', nullable: false, defaultTo: true},
show_header_name: {type: 'boolean', nullable: false, defaultTo: true},
created_at: {type: 'dateTime', nullable: false},
updated_at: {type: 'dateTime', nullable: true}
},
@ -51,7 +51,7 @@ module.exports = {
comment_id: {type: 'string', maxlength: 50, nullable: true},
plaintext: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true},
feature_image: {type: 'string', maxlength: 2000, nullable: true},
featured: {type: 'bool', nullable: false, defaultTo: false},
featured: {type: 'boolean', nullable: false, defaultTo: false},
type: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'post', validations: {isIn: [['post', 'page']]}},
status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'draft', validations: {isIn: [['published', 'draft', 'scheduled', 'sent']]}},
// NOTE: unused at the moment and reserved for future features
@ -103,7 +103,7 @@ module.exports = {
frontmatter: {type: 'text', maxlength: 65535, nullable: true},
feature_image_alt: {type: 'string', maxlength: 191, nullable: true, validations: {isLength: {max: 125}}},
feature_image_caption: {type: 'text', maxlength: 65535, nullable: true},
email_only: {type: 'bool', nullable: false, defaultTo: false}
email_only: {type: 'boolean', nullable: false, defaultTo: false}
},
// NOTE: this is the staff table
users: {
@ -607,7 +607,7 @@ module.exports = {
subscription_id: {type: 'string', maxlength: 255, nullable: false, unique: true},
stripe_price_id: {type: 'string', maxlength: 255, nullable: false, unique: false, index: true, defaultTo: ''},
status: {type: 'string', maxlength: 50, nullable: false},
cancel_at_period_end: {type: 'bool', nullable: false, defaultTo: false},
cancel_at_period_end: {type: 'boolean', nullable: false, defaultTo: false},
cancellation_reason: {type: 'string', maxlength: 500, nullable: true},
current_period_end: {type: 'dateTime', nullable: false},
start_date: {type: 'dateTime', nullable: false},
@ -653,7 +653,7 @@ module.exports = {
members_subscribe_events: {
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
member_id: {type: 'string', maxlength: 24, nullable: false, unique: false, references: 'members.id', cascadeDelete: true},
subscribed: {type: 'bool', nullable: false, defaultTo: true},
subscribed: {type: 'boolean', nullable: false, defaultTo: true},
created_at: {type: 'dateTime', nullable: false},
source: {
type: 'string', maxlength: 50, nullable: true, validations: {
@ -673,7 +673,7 @@ module.exports = {
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
stripe_price_id: {type: 'string', maxlength: 255, nullable: false, unique: true},
stripe_product_id: {type: 'string', maxlength: 255, nullable: false, unique: false, references: 'stripe_products.stripe_product_id'},
active: {type: 'bool', nullable: false},
active: {type: 'boolean', nullable: false},
nickname: {type: 'string', maxlength: 50, nullable: true},
currency: {type: 'string', maxLength: 3, nullable: false},
amount: {type: 'integer', nullable: false},
@ -723,8 +723,8 @@ module.exports = {
reply_to: {type: 'string', maxlength: 2000, nullable: true},
html: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true},
plaintext: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true},
track_opens: {type: 'bool', nullable: false, defaultTo: false},
track_clicks: {type: 'bool', nullable: false, defaultTo: false},
track_opens: {type: 'boolean', nullable: false, defaultTo: false},
track_clicks: {type: 'boolean', nullable: false, defaultTo: false},
submitted_at: {type: 'dateTime', nullable: false},
newsletter_id: {type: 'string', maxlength: 24, nullable: true, references: 'newsletters.id'},
created_at: {type: 'dateTime', nullable: false},

View File

@ -63,7 +63,7 @@ function validateSchema(tableName, model, options) {
// validate boolean columns
if (Object.prototype.hasOwnProperty.call(schema[tableName][columnKey], 'type')
&& (schema[tableName][columnKey].type === 'bool' || schema[tableName][columnKey].type === 'boolean')) {
&& schema[tableName][columnKey].type === 'boolean') {
if (!(validator.isBoolean(strVal) || validator.isEmpty(strVal))) {
message = tpl(messages.valueMustBeBoolean, {
tableName: tableName,

View File

@ -78,7 +78,7 @@ module.exports = function (Bookshelf) {
_.each(attrs, function each(value, key) {
const tableDef = schema.tables[self.tableName];
const columnDef = tableDef ? tableDef[key] : null;
if (columnDef && (columnDef.type === 'bool' || columnDef.type === 'boolean')) {
if (columnDef?.type === 'boolean') {
attrs[key] = value ? true : false;
}
});

View File

@ -35,7 +35,7 @@ const validateRouteSettings = require('../../../../../core/server/services/route
*/
describe('DB version integrity', function () {
// Only these variables should need updating
const currentSchemaHash = 'ea7c1c293556a58c4f928111dc3be3c6';
const currentSchemaHash = '7f8c1a992b307dff960165b0cfa1d2db';
const currentFixturesHash = '8cf221f0ed930ac1fe8030a58e60d64b';
const currentSettingsHash = '2978a5684a2d5fcf089f61f5d368a0c0';
const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';

View File

@ -7,10 +7,6 @@ const VALID_KEYS = {
bigInteger: [
'nullable'
],
bool: [
'nullable',
'defaultTo'
],
boolean: [
'nullable',
'defaultTo'

View File

@ -64,16 +64,6 @@ describe('Validate Schema', function () {
);
});
it('transforms 0 and 1 (bool)', function () {
const post = models.Post.forge(testUtils.DataGenerator.forKnex.createPost({slug: 'test', featured: 0}));
post.get('featured').should.eql(0);
return validateSchema('posts', post, {method: 'insert'})
.then(function () {
post.get('featured').should.eql(false);
});
});
it('transforms 0 and 1 (boolean)', async function () {
const user = models.User.forge(testUtils.DataGenerator.forKnex.createUser({email: 'test@example.com', comment_notifications: 0}));
user.get('comment_notifications').should.eql(0);