From 45d65663f4af83cb4233a85d6555075298a34863 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Thu, 29 Sep 2022 22:08:45 +0100 Subject: [PATCH] Simplified link tracking related tables naming (#15480) - Removes superfluous "link" from table names - Fixes type definititon of dropTables util - Updates & renames models - Noop existing migrations to avoid unnecessary work --- .../core/server/data/exporter/table-lists.js | 4 +-- .../server/data/migrations/utils/tables.js | 2 +- ...22-09-19-09-04-add-link-redirects-table.js | 19 ++++++------- ...-05-add-members-link-click-events-table.js | 17 +++++------ ...9-27-13-53-remove-click-tracking-tables.js | 28 +++++++++++++++++++ .../2022-09-27-13-55-add-redirects-table.js | 10 +++++++ ...27-13-56-add-members-click-events-table.js | 8 ++++++ ghost/core/core/server/data/schema/schema.js | 6 ++-- ...k-click-event.js => member-click-event.js} | 12 ++++---- ghost/core/core/server/models/post.js | 8 +++--- .../models/{link-redirect.js => redirect.js} | 14 +++++----- .../server/services/link-redirection/index.js | 2 +- .../link-tracking/LinkClickRepository.js | 4 +-- .../server/services/link-tracking/index.js | 4 +-- .../core/core/server/services/members/api.js | 2 +- .../integration/exporter/exporter.test.js | 4 +-- .../unit/server/data/schema/integrity.test.js | 2 +- 17 files changed, 96 insertions(+), 50 deletions(-) create mode 100644 ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-53-remove-click-tracking-tables.js create mode 100644 ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-55-add-redirects-table.js create mode 100644 ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-56-add-members-click-events-table.js rename ghost/core/core/server/models/{member-link-click-event.js => member-click-event.js} (56%) rename ghost/core/core/server/models/{link-redirect.js => redirect.js} (74%) diff --git a/ghost/core/core/server/data/exporter/table-lists.js b/ghost/core/core/server/data/exporter/table-lists.js index b6018d341f..d7a997b51d 100644 --- a/ghost/core/core/server/data/exporter/table-lists.js +++ b/ghost/core/core/server/data/exporter/table-lists.js @@ -39,8 +39,8 @@ const BACKUP_TABLES = [ 'comment_likes', 'comment_reports', 'jobs', - 'link_redirects', - 'members_link_click_events' + 'redirects', + 'members_click_events' ]; // NOTE: exposing only tables which are going to be included in a "default" export file diff --git a/ghost/core/core/server/data/migrations/utils/tables.js b/ghost/core/core/server/data/migrations/utils/tables.js index 1e372795cf..557862c236 100644 --- a/ghost/core/core/server/data/migrations/utils/tables.js +++ b/ghost/core/core/server/data/migrations/utils/tables.js @@ -38,7 +38,7 @@ function addTable(name, tableSpec) { /** * Creates migration which will drop a table * - * @param {[string]} names - names of the tables to drop + * @param {string[]} names - names of the tables to drop */ function dropTables(names) { return createIrreversibleMigration( diff --git a/ghost/core/core/server/data/migrations/versions/5.16/2022-09-19-09-04-add-link-redirects-table.js b/ghost/core/core/server/data/migrations/versions/5.16/2022-09-19-09-04-add-link-redirects-table.js index c560f9df88..e722c2edbd 100644 --- a/ghost/core/core/server/data/migrations/versions/5.16/2022-09-19-09-04-add-link-redirects-table.js +++ b/ghost/core/core/server/data/migrations/versions/5.16/2022-09-19-09-04-add-link-redirects-table.js @@ -1,10 +1,9 @@ -const {addTable} = require('../../utils'); - -module.exports = addTable('link_redirects', { - id: {type: 'string', maxlength: 24, nullable: false, primary: true}, - from: {type: 'string', maxlength: 2000, nullable: false}, - to: {type: 'string', maxlength: 2000, nullable: false}, - post_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'posts.id', setNullDelete: true}, - created_at: {type: 'dateTime', nullable: false}, - updated_at: {type: 'dateTime', nullable: true} -}); +const logging = require('@tryghost/logging'); +module.exports = { + async up() { + logging.warn('Skipping migration - noop'); + }, + async down() { + logging.warn('Skipping migration - noop'); + } +}; diff --git a/ghost/core/core/server/data/migrations/versions/5.16/2022-09-19-09-05-add-members-link-click-events-table.js b/ghost/core/core/server/data/migrations/versions/5.16/2022-09-19-09-05-add-members-link-click-events-table.js index 5635fb558e..e722c2edbd 100644 --- a/ghost/core/core/server/data/migrations/versions/5.16/2022-09-19-09-05-add-members-link-click-events-table.js +++ b/ghost/core/core/server/data/migrations/versions/5.16/2022-09-19-09-05-add-members-link-click-events-table.js @@ -1,8 +1,9 @@ -const {addTable} = require('../../utils'); - -module.exports = addTable('members_link_click_events', { - id: {type: 'string', maxlength: 24, nullable: false, primary: true}, - member_id: {type: 'string', maxlength: 24, nullable: false, references: 'members.id', cascadeDelete: true}, - link_id: {type: 'string', maxlength: 24, nullable: false, references: 'link_redirects.id', cascadeDelete: true}, - created_at: {type: 'dateTime', nullable: false} -}); +const logging = require('@tryghost/logging'); +module.exports = { + async up() { + logging.warn('Skipping migration - noop'); + }, + async down() { + logging.warn('Skipping migration - noop'); + } +}; diff --git a/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-53-remove-click-tracking-tables.js b/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-53-remove-click-tracking-tables.js new file mode 100644 index 0000000000..0d01a1b397 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-53-remove-click-tracking-tables.js @@ -0,0 +1,28 @@ +const {addTable, combineNonTransactionalMigrations} = require('../../utils'); + +function reverseMigration({up, down, config}) { + return { + config, + up: down, + down: up + }; +} + +module.exports = reverseMigration( + combineNonTransactionalMigrations( + addTable('link_redirects', { + id: {type: 'string', maxlength: 24, nullable: false, primary: true}, + from: {type: 'string', maxlength: 2000, nullable: false}, + to: {type: 'string', maxlength: 2000, nullable: false}, + post_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'posts.id', setNullDelete: true}, + created_at: {type: 'dateTime', nullable: false}, + updated_at: {type: 'dateTime', nullable: true} + }), + addTable('members_link_click_events', { + id: {type: 'string', maxlength: 24, nullable: false, primary: true}, + member_id: {type: 'string', maxlength: 24, nullable: false, references: 'members.id', cascadeDelete: true}, + link_id: {type: 'string', maxlength: 24, nullable: false, references: 'link_redirects.id', cascadeDelete: true}, + created_at: {type: 'dateTime', nullable: false} + }) + ) +); diff --git a/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-55-add-redirects-table.js b/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-55-add-redirects-table.js new file mode 100644 index 0000000000..aeede5a13f --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-55-add-redirects-table.js @@ -0,0 +1,10 @@ +const {addTable} = require('../../utils'); + +module.exports = addTable('redirects', { + id: {type: 'string', maxlength: 24, nullable: false, primary: true}, + from: {type: 'string', maxlength: 2000, nullable: false}, + to: {type: 'string', maxlength: 2000, nullable: false}, + post_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'posts.id', setNullDelete: true}, + created_at: {type: 'dateTime', nullable: false}, + updated_at: {type: 'dateTime', nullable: true} +}); diff --git a/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-56-add-members-click-events-table.js b/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-56-add-members-click-events-table.js new file mode 100644 index 0000000000..7f86e8be13 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/5.17/2022-09-27-13-56-add-members-click-events-table.js @@ -0,0 +1,8 @@ +const {addTable} = require('../../utils'); + +module.exports = addTable('members_click_events', { + id: {type: 'string', maxlength: 24, nullable: false, primary: true}, + member_id: {type: 'string', maxlength: 24, nullable: false, references: 'members.id', cascadeDelete: true}, + redirect_id: {type: 'string', maxlength: 24, nullable: false, references: 'redirects.id', cascadeDelete: true}, + created_at: {type: 'dateTime', nullable: false} +}); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index 09a7592086..93d01d9cb7 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -837,7 +837,7 @@ module.exports = { created_at: {type: 'dateTime', nullable: false}, updated_at: {type: 'dateTime', nullable: true} }, - link_redirects: { + redirects: { id: {type: 'string', maxlength: 24, nullable: false, primary: true}, from: {type: 'string', maxlength: 2000, nullable: false}, to: {type: 'string', maxlength: 2000, nullable: false}, @@ -845,10 +845,10 @@ module.exports = { created_at: {type: 'dateTime', nullable: false}, updated_at: {type: 'dateTime', nullable: true} }, - members_link_click_events: { + members_click_events: { id: {type: 'string', maxlength: 24, nullable: false, primary: true}, member_id: {type: 'string', maxlength: 24, nullable: false, references: 'members.id', cascadeDelete: true}, - link_id: {type: 'string', maxlength: 24, nullable: false, references: 'link_redirects.id', cascadeDelete: true}, + redirect_id: {type: 'string', maxlength: 24, nullable: false, references: 'redirects.id', cascadeDelete: true}, created_at: {type: 'dateTime', nullable: false} } }; diff --git a/ghost/core/core/server/models/member-link-click-event.js b/ghost/core/core/server/models/member-click-event.js similarity index 56% rename from ghost/core/core/server/models/member-link-click-event.js rename to ghost/core/core/server/models/member-click-event.js index 29f7b981f2..45627713a3 100644 --- a/ghost/core/core/server/models/member-link-click-event.js +++ b/ghost/core/core/server/models/member-click-event.js @@ -1,11 +1,11 @@ const errors = require('@tryghost/errors'); const ghostBookshelf = require('./base'); -const MemberLinkClickEvent = ghostBookshelf.Model.extend({ - tableName: 'members_link_click_events', +const MemberClickEvent = ghostBookshelf.Model.extend({ + tableName: 'members_click_events', link() { - return this.belongsTo('LinkRedirect', 'link_id'); + return this.belongsTo('Redirect', 'link_id'); }, member() { @@ -13,14 +13,14 @@ const MemberLinkClickEvent = ghostBookshelf.Model.extend({ } }, { async edit() { - throw new errors.IncorrectUsageError({message: 'Cannot edit MemberLinkClickEvent'}); + throw new errors.IncorrectUsageError({message: 'Cannot edit MemberClickEvent'}); }, async destroy() { - throw new errors.IncorrectUsageError({message: 'Cannot destroy MemberLinkClickEvent'}); + throw new errors.IncorrectUsageError({message: 'Cannot destroy MemberClickEvent'}); } }); module.exports = { - MemberLinkClickEvent: ghostBookshelf.model('MemberLinkClickEvent', MemberLinkClickEvent) + MemberClickEvent: ghostBookshelf.model('MemberClickEvent', MemberClickEvent) }; diff --git a/ghost/core/core/server/models/post.js b/ghost/core/core/server/models/post.js index b0e03758b3..16d361bddd 100644 --- a/ghost/core/core/server/models/post.js +++ b/ghost/core/core/server/models/post.js @@ -1348,10 +1348,10 @@ Post = ghostBookshelf.Model.extend({ }, clicks(modelOrCollection) { modelOrCollection.query('columns', 'posts.*', (qb) => { - qb.countDistinct('members_link_click_events.member_id') - .from('members_link_click_events') - .join('link_redirects', 'members_link_click_events.link_id', 'link_redirects.id') - .whereRaw('posts.id = link_redirects.post_id') + qb.countDistinct('members_click_events.member_id') + .from('members_click_events') + .join('redirects', 'members_click_events.redirect_id', 'redirects.id') + .whereRaw('posts.id = redirects.post_id') .as('count__clicks'); }); } diff --git a/ghost/core/core/server/models/link-redirect.js b/ghost/core/core/server/models/redirect.js similarity index 74% rename from ghost/core/core/server/models/link-redirect.js rename to ghost/core/core/server/models/redirect.js index 0f4f17e95f..7a58e58278 100644 --- a/ghost/core/core/server/models/link-redirect.js +++ b/ghost/core/core/server/models/redirect.js @@ -1,8 +1,8 @@ const ghostBookshelf = require('./base'); const urlUtils = require('../../shared/url-utils'); -const LinkRedirect = ghostBookshelf.Model.extend({ - tableName: 'link_redirects', +const Redirect = ghostBookshelf.Model.extend({ + tableName: 'redirects', post() { return this.belongsTo('Post', 'post_id'); @@ -49,10 +49,10 @@ const LinkRedirect = ghostBookshelf.Model.extend({ countRelations() { return { clicks(modelOrCollection) { - modelOrCollection.query('columns', 'link_redirects.*', (qb) => { - qb.countDistinct('members_link_click_events.member_id') - .from('members_link_click_events') - .whereRaw('link_redirects.id = members_link_click_events.link_id') + modelOrCollection.query('columns', 'redirects.*', (qb) => { + qb.countDistinct('members_click_events.member_id') + .from('members_click_events') + .whereRaw('redirects.id = members_click_events.redirect_id') .as('count__clicks'); }); } @@ -61,5 +61,5 @@ const LinkRedirect = ghostBookshelf.Model.extend({ }); module.exports = { - LinkRedirect: ghostBookshelf.model('LinkRedirect', LinkRedirect) + Redirect: ghostBookshelf.model('Redirect', Redirect) }; diff --git a/ghost/core/core/server/services/link-redirection/index.js b/ghost/core/core/server/services/link-redirection/index.js index 4fcfe02868..7ba4962c33 100644 --- a/ghost/core/core/server/services/link-redirection/index.js +++ b/ghost/core/core/server/services/link-redirection/index.js @@ -14,7 +14,7 @@ class LinkRedirectsServiceWrapper { const {LinkRedirectsService} = require('@tryghost/link-redirects'); this.linkRedirectRepository = new LinkRedirectRepository({ - LinkRedirect: models.LinkRedirect, + LinkRedirect: models.Redirect, urlUtils }); diff --git a/ghost/core/core/server/services/link-tracking/LinkClickRepository.js b/ghost/core/core/server/services/link-tracking/LinkClickRepository.js index fad11eed7a..21f88267db 100644 --- a/ghost/core/core/server/services/link-tracking/LinkClickRepository.js +++ b/ghost/core/core/server/services/link-tracking/LinkClickRepository.js @@ -36,7 +36,7 @@ module.exports = class LinkClickRepository { for (const model of collection.models) { const member = await this.#Member.findOne({id: model.get('member_id')}); result.push(new LinkClick({ - link_id: model.get('link_id'), + link_id: model.get('redirect_id'), member_uuid: member.get('uuid') })); } @@ -57,7 +57,7 @@ module.exports = class LinkClickRepository { const model = await this.#MemberLinkClickEventModel.add({ // Only store the parthname (no support for variable query strings) - link_id: linkClick.link_id.toHexString(), + redirect_id: linkClick.link_id.toHexString(), member_id: member.id }, {}); diff --git a/ghost/core/core/server/services/link-tracking/index.js b/ghost/core/core/server/services/link-tracking/index.js index 5328d87f71..7a72c5a87d 100644 --- a/ghost/core/core/server/services/link-tracking/index.js +++ b/ghost/core/core/server/services/link-tracking/index.js @@ -22,12 +22,12 @@ class LinkTrackingServiceWrapper { const {LinkClickTrackingService} = require('@tryghost/link-tracking'); const postLinkRepository = new PostLinkRepository({ - LinkRedirect: models.LinkRedirect, + LinkRedirect: models.Redirect, linkRedirectRepository: linkRedirection.linkRedirectRepository }); this.linkClickRepository = new LinkClickRepository({ - MemberLinkClickEventModel: models.MemberLinkClickEvent, + MemberLinkClickEventModel: models.MemberClickEvent, Member: models.Member, MemberLinkClickEvent: MemberLinkClickEvent, DomainEvents diff --git a/ghost/core/core/server/services/members/api.js b/ghost/core/core/server/services/members/api.js index 37294e2626..944999141b 100644 --- a/ghost/core/core/server/services/members/api.js +++ b/ghost/core/core/server/services/members/api.js @@ -187,7 +187,7 @@ function createApiInstance(config) { MemberAnalyticEvent: models.MemberAnalyticEvent, MemberCreatedEvent: models.MemberCreatedEvent, SubscriptionCreatedEvent: models.SubscriptionCreatedEvent, - MemberLinkClickEvent: models.MemberLinkClickEvent, + MemberLinkClickEvent: models.MemberClickEvent, OfferRedemption: models.OfferRedemption, Offer: models.Offer, StripeProduct: models.StripeProduct, diff --git a/ghost/core/test/integration/exporter/exporter.test.js b/ghost/core/test/integration/exporter/exporter.test.js index 058221fd24..c6f139c452 100644 --- a/ghost/core/test/integration/exporter/exporter.test.js +++ b/ghost/core/test/integration/exporter/exporter.test.js @@ -36,12 +36,12 @@ describe('Exporter', function () { 'invites', 'jobs', 'labels', - 'link_redirects', + 'redirects', 'members', 'members_cancel_events', 'members_email_change_events', 'members_labels', - 'members_link_click_events', + 'members_click_events', 'members_login_events', 'members_newsletters', 'members_paid_subscription_events', diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index d1ab16bcca..fa811d5068 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -35,7 +35,7 @@ const validateRouteSettings = require('../../../../../core/server/services/route */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = '2a59debcacc1e3dc0b15e2f729ca4bdb'; + const currentSchemaHash = '67009be314428b02976e75281de37b14'; const currentFixturesHash = '8cf221f0ed930ac1fe8030a58e60d64b'; const currentSettingsHash = '2978a5684a2d5fcf089f61f5d368a0c0'; const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';