From 14927ee24b932b9c5f1dbc4d0338b6f018f25a22 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Mon, 13 Nov 2023 12:00:20 +0100 Subject: [PATCH] Added quotes to NQL filters with ids (#18958) refs https://github.com/TryGhost/Product/issues/4120 Updated some places where we don't add quotes around ids in NQL filters, which can be an issue when the id is a number --- apps/comments-ui/src/utils/api.ts | 2 +- .../components/members/filters/audience-feedback.js | 12 ++++++------ ghost/admin/app/components/posts/analytics.js | 12 ++++++------ ghost/admin/app/components/posts/old-analytics.js | 6 +++--- .../posts/post-activity-feed/footer-links.js | 4 ++-- ghost/admin/app/helpers/history-event-filter.js | 2 +- ghost/admin/app/helpers/members-event-filter.js | 4 ++-- ghost/admin/app/routes/mentions.js | 2 +- ghost/core/core/server/models/comment.js | 4 ++-- ghost/core/core/server/models/post.js | 4 ++-- .../test/e2e-api/members-comments/comments.test.js | 4 ++-- .../services/email-service/batch-sending.test.js | 8 ++++---- ghost/email-service/lib/BatchSendingService.js | 4 ++-- ghost/email-service/lib/EmailSegmenter.js | 2 +- ghost/email-service/test/email-segmenter.test.js | 6 +++--- ghost/link-tracking/lib/LinkClickTrackingService.js | 2 +- .../members-api/lib/repositories/MemberRepository.js | 4 ++-- ghost/post-revisions/src/PostRevisions.ts | 2 +- 18 files changed, 42 insertions(+), 42 deletions(-) diff --git a/apps/comments-ui/src/utils/api.ts b/apps/comments-ui/src/utils/api.ts index 38a0d9a718..2b49d81993 100644 --- a/apps/comments-ui/src/utils/api.ts +++ b/apps/comments-ui/src/utils/api.ts @@ -124,7 +124,7 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}: {site browse({page, postId}: {page: number, postId: string}) { firstCommentsLoadedAt = firstCommentsLoadedAt ?? new Date().toISOString(); - const filter = encodeURIComponent(`post_id:${postId}+created_at:<=${firstCommentsLoadedAt}`); + const filter = encodeURIComponent(`post_id:'${postId}'+created_at:<=${firstCommentsLoadedAt}`); const order = encodeURIComponent('created_at DESC, id DESC'); const url = endpointFor({type: 'members', resource: 'comments', params: `?limit=5&order=${order}&filter=${filter}&page=${page}`}); diff --git a/ghost/admin/app/components/members/filters/audience-feedback.js b/ghost/admin/app/components/members/filters/audience-feedback.js index 3551f13563..f149eccb21 100644 --- a/ghost/admin/app/components/members/filters/audience-feedback.js +++ b/ghost/admin/app/components/members/filters/audience-feedback.js @@ -4,15 +4,15 @@ const FEEDBACK_RELATION_OPTIONS = [ ]; export const AUDIENCE_FEEDBACK_FILTER = { - label: 'Responded with feedback', - name: 'newsletter_feedback', - valueType: 'string', - resource: 'email', + label: 'Responded with feedback', + name: 'newsletter_feedback', + valueType: 'string', + resource: 'email', relationOptions: FEEDBACK_RELATION_OPTIONS, - feature: 'audienceFeedback', + feature: 'audienceFeedback', buildNqlFilter: (filter) => { // Added brackets to make sure we can parse as a single AND filter - return `(feedback.post_id:${filter.value}+feedback.score:${filter.relation})`; + return `(feedback.post_id:'${filter.value}'+feedback.score:${filter.relation})`; }, parseNqlFilter: (filter) => { if (!filter.$and) { diff --git a/ghost/admin/app/components/posts/analytics.js b/ghost/admin/app/components/posts/analytics.js index 4ea7345422..3ec21fff26 100644 --- a/ghost/admin/app/components/posts/analytics.js +++ b/ghost/admin/app/components/posts/analytics.js @@ -96,8 +96,8 @@ export default class Analytics extends Component { const values = [this.post.count.positive_feedback, this.post.count.negative_feedback]; const labels = ['More like this', 'Less like this']; const links = [ - {filterParam: '(feedback.post_id:' + this.post.id + '+feedback.score:1)'}, - {filterParam: '(feedback.post_id:' + this.post.id + '+feedback.score:0)'} + {filterParam: '(feedback.post_id:\'' + this.post.id + '\'+feedback.score:1)'}, + {filterParam: '(feedback.post_id:\'' + this.post.id + '\'+feedback.score:0)'} ]; const colors = ['#F080B2', '#8452f633']; return {values, labels, links, colors}; @@ -234,7 +234,7 @@ export default class Analytics extends Component { return link; }); - const filter = `post_id:${this.post.id}+to:'${currentLink}'`; + const filter = `post_id:'${this.post.id}'+to:'${currentLink}'`; let bulkUpdateUrl = this.ghostPaths.url.api(`links/bulk`) + `?filter=${encodeURIComponent(filter)}`; yield this.ajax.put(bulkUpdateUrl, { data: { @@ -246,7 +246,7 @@ export default class Analytics extends Component { }); // Refresh links data - const linksFilter = `post_id:${this.post.id}`; + const linksFilter = `post_id:'${this.post.id}'`; let statsUrl = this.ghostPaths.url.api(`links/`) + `?filter=${encodeURIComponent(linksFilter)}`; let result = yield this.ajax.request(statsUrl); this.updateLinkData(result.links); @@ -271,7 +271,7 @@ export default class Analytics extends Component { @task *_fetchLinks() { - const filter = `post_id:${this.post.id}`; + const filter = `post_id:'${this.post.id}'`; let statsUrl = this.ghostPaths.url.api(`links/`) + `?filter=${encodeURIComponent(filter)}`; let result = yield this.ajax.request(statsUrl); this.updateLinkData(result.links); @@ -286,7 +286,7 @@ export default class Analytics extends Component { @task *_fetchMentions() { - const filter = `resource_id:${this.post.id}+resource_type:post`; + const filter = `resource_id:'${this.post.id}'+resource_type:post`; this.mentions = yield this.store.query('mention', {limit: 5, order: 'created_at desc', filter}); } diff --git a/ghost/admin/app/components/posts/old-analytics.js b/ghost/admin/app/components/posts/old-analytics.js index 5213bd364e..dbc66cf564 100644 --- a/ghost/admin/app/components/posts/old-analytics.js +++ b/ghost/admin/app/components/posts/old-analytics.js @@ -204,7 +204,7 @@ export default class Analytics extends Component { return link; }); - const filter = `post_id:${this.post.id}+to:'${currentLink}'`; + const filter = `post_id:'${this.post.id}'+to:'${currentLink}'`; let bulkUpdateUrl = this.ghostPaths.url.api(`links/bulk`) + `?filter=${encodeURIComponent(filter)}`; yield this.ajax.put(bulkUpdateUrl, { data: { @@ -216,7 +216,7 @@ export default class Analytics extends Component { }); // Refresh links data - const linksFilter = `post_id:${this.post.id}`; + const linksFilter = `post_id:'${this.post.id}'`; let statsUrl = this.ghostPaths.url.api(`links/`) + `?filter=${encodeURIComponent(linksFilter)}`; let result = yield this.ajax.request(statsUrl); this.updateLinkData(result.links); @@ -241,7 +241,7 @@ export default class Analytics extends Component { @task *_fetchLinks() { - const filter = `post_id:${this.post.id}`; + const filter = `post_id:'${this.post.id}'`; let statsUrl = this.ghostPaths.url.api(`links/`) + `?filter=${encodeURIComponent(filter)}`; let result = yield this.ajax.request(statsUrl); this.updateLinkData(result.links); diff --git a/ghost/admin/app/components/posts/post-activity-feed/footer-links.js b/ghost/admin/app/components/posts/post-activity-feed/footer-links.js index 076b63ed41..c0ed565b40 100644 --- a/ghost/admin/app/components/posts/post-activity-feed/footer-links.js +++ b/ghost/admin/app/components/posts/post-activity-feed/footer-links.js @@ -3,8 +3,8 @@ import Component from '@glimmer/component'; export default class FooterLinks extends Component { get feedbackLinks() { const post = this.args.post; - const positiveLink = {filterParam: '(feedback.post_id:' + post.id + '+feedback.score:1)', label: 'More like this'}; - const negativeLink = {filterParam: '(feedback.post_id:' + post.id + '+feedback.score:0)', label: 'Less like this'}; + const positiveLink = {filterParam: '(feedback.post_id:\'' + post.id + '\'+feedback.score:1)', label: 'More like this'}; + const negativeLink = {filterParam: '(feedback.post_id:\'' + post.id + '\'+feedback.score:0)', label: 'Less like this'}; const data = [ {link: positiveLink, hidden: !post.count.positive_feedback}, diff --git a/ghost/admin/app/helpers/history-event-filter.js b/ghost/admin/app/helpers/history-event-filter.js index 5be85ad665..2b09acdaac 100644 --- a/ghost/admin/app/helpers/history-event-filter.js +++ b/ghost/admin/app/helpers/history-event-filter.js @@ -33,7 +33,7 @@ export default class HistoryEventFilter extends Helper { } if (user) { - filterParts.push(`actor_id:${user}`); + filterParts.push(`actor_id:'${user}'`); } return filterParts.join('+'); diff --git a/ghost/admin/app/helpers/members-event-filter.js b/ghost/admin/app/helpers/members-event-filter.js index cd66d0c514..15dc531204 100644 --- a/ghost/admin/app/helpers/members-event-filter.js +++ b/ghost/admin/app/helpers/members-event-filter.js @@ -45,11 +45,11 @@ export default class MembersEventFilter extends Helper { } if (member) { - filterParts.push(`data.member_id:${member}`); + filterParts.push(`data.member_id:'${member}'`); } if (post) { - filterParts.push(`data.post_id:${post}`); + filterParts.push(`data.post_id:'${post}'`); } return filterParts.join('+'); diff --git a/ghost/admin/app/routes/mentions.js b/ghost/admin/app/routes/mentions.js index bb078c56ef..cff4f3d8ef 100644 --- a/ghost/admin/app/routes/mentions.js +++ b/ghost/admin/app/routes/mentions.js @@ -39,7 +39,7 @@ export default class MentionsRoute extends AuthenticatedRoute { let extension = undefined; if (params.post_id) { - paginationSettings.filter = `resource_id:${params.post_id}+resource_type:post`; + paginationSettings.filter = `resource_id:'${params.post_id}'+resource_type:post`; } else { // Only return mentions with the same source once paginationSettings.unique = true; diff --git a/ghost/core/core/server/models/comment.js b/ghost/core/core/server/models/comment.js index de64d1db9c..d5555fa15a 100644 --- a/ghost/core/core/server/models/comment.js +++ b/ghost/core/core/server/models/comment.js @@ -78,7 +78,7 @@ const Comment = ghostBookshelf.Model.extend({ // Enforce _blank and safe URLs transformTags: { a: sanitizeHtml.simpleTransform('a', { - target: '_blank', + target: '_blank', rel: 'ugc noopener noreferrer nofollow' }) } @@ -106,7 +106,7 @@ const Comment = ghostBookshelf.Model.extend({ if (options.parentId === null) { return 'parent_id:null'; } - return 'parent_id:' + options.parentId; + return 'parent_id:\'' + options.parentId + '\''; } return null; diff --git a/ghost/core/core/server/models/post.js b/ghost/core/core/server/models/post.js index 7992906ff4..094a091284 100644 --- a/ghost/core/core/server/models/post.js +++ b/ghost/core/core/server/models/post.js @@ -904,7 +904,7 @@ Post = ghostBookshelf.Model.extend({ ops.push(function updateRevisions() { return ghostBookshelf.model('MobiledocRevision') .findAll(Object.assign({ - filter: `post_id:${model.id}`, + filter: `post_id:'${model.id}'`, columns: ['id'] }, _.pick(options, 'transacting'))) .then((revisions) => { @@ -958,7 +958,7 @@ Post = ghostBookshelf.Model.extend({ ops.push(async function updateRevisions() { const revisionModels = await ghostBookshelf.model('PostRevision') .findAll(Object.assign({ - filter: `post_id:${model.id}`, + filter: `post_id:'${model.id}'`, columns: ['id', 'lexical', 'created_at', 'author_id', 'title', 'reason', 'post_status', 'created_at_ts', 'feature_image'] }, _.pick(options, 'transacting'))); diff --git a/ghost/core/test/e2e-api/members-comments/comments.test.js b/ghost/core/test/e2e-api/members-comments/comments.test.js index 4444f6f90e..8ce1f1fe6f 100644 --- a/ghost/core/test/e2e-api/members-comments/comments.test.js +++ b/ghost/core/test/e2e-api/members-comments/comments.test.js @@ -616,7 +616,7 @@ describe('Comments API', function () { .expectEmptyBody(); // Check report - const reports = await models.CommentReport.findAll({filter: 'comment_id:' + commentId}); + const reports = await models.CommentReport.findAll({filter: 'comment_id:\'' + commentId + '\''}); reports.models.length.should.eql(1); const report = reports.models[0]; @@ -641,7 +641,7 @@ describe('Comments API', function () { .expectEmptyBody(); // Check report should be the same (no extra created) - const reports = await models.CommentReport.findAll({filter: 'comment_id:' + commentId}); + const reports = await models.CommentReport.findAll({filter: 'comment_id:\'' + commentId + '\''}); reports.models.length.should.eql(1); const report = reports.models[0]; diff --git a/ghost/core/test/integration/services/email-service/batch-sending.test.js b/ghost/core/test/integration/services/email-service/batch-sending.test.js index cee09088e7..107433319a 100644 --- a/ghost/core/test/integration/services/email-service/batch-sending.test.js +++ b/ghost/core/test/integration/services/email-service/batch-sending.test.js @@ -567,7 +567,7 @@ describe('Batch sending tests', function () { // Test if all links are replaced and contain the member id const cheerio = require('cheerio'); const $ = cheerio.load(html); - const links = await linkRedirectRepository.getAll({filter: 'post_id:' + emailModel.get('post_id')}); + const links = await linkRedirectRepository.getAll({filter: 'post_id:\'' + emailModel.get('post_id') + '\''}); for (const el of $('a').toArray()) { const href = $(el).attr('href'); @@ -612,7 +612,7 @@ describe('Batch sending tests', function () { const {emailModel, html} = await sendEmail(agent); assert.match(html, /\m=/); - const links = await linkRedirectRepository.getAll({filter: 'post_id:' + emailModel.get('post_id')}); + const links = await linkRedirectRepository.getAll({filter: 'post_id:\'' + emailModel.get('post_id') + '\''}); for (const link of links) { // Check ref not added to all replaced links @@ -627,7 +627,7 @@ describe('Batch sending tests', function () { const {emailModel, html} = await sendEmail(agent); assert.match(html, /\m=/); - const links = await linkRedirectRepository.getAll({filter: 'post_id:' + emailModel.get('post_id')}); + const links = await linkRedirectRepository.getAll({filter: 'post_id:\'' + emailModel.get('post_id') + '\''}); for (const link of links) { // Check ref not added to all replaced links @@ -640,7 +640,7 @@ describe('Batch sending tests', function () { const {emailModel, html} = await sendEmail(agent); assert.doesNotMatch(html, /\m=/); - const links = await linkRedirectRepository.getAll({filter: 'post_id:' + emailModel.get('post_id')}); + const links = await linkRedirectRepository.getAll({filter: 'post_id:\'' + emailModel.get('post_id') + '\''}); assert.equal(links.length, 0); }); }); diff --git a/ghost/email-service/lib/BatchSendingService.js b/ghost/email-service/lib/BatchSendingService.js index 4250913342..1db162e86d 100644 --- a/ghost/email-service/lib/BatchSendingService.js +++ b/ghost/email-service/lib/BatchSendingService.js @@ -216,7 +216,7 @@ class BatchSendingService { async getBatches(email) { logging.info(`Getting batches for email ${email.id}`); - return await this.#models.EmailBatch.findAll({filter: 'email_id:' + email.id}); + return await this.#models.EmailBatch.findAll({filter: 'email_id:\'' + email.id + '\''}); } /** @@ -501,7 +501,7 @@ class BatchSendingService { * @returns {Promise} */ async getBatchMembers(batchId) { - let models = await this.#models.EmailRecipient.findAll({filter: `batch_id:${batchId}`, withRelated: ['member', 'member.stripeSubscriptions', 'member.products']}); + let models = await this.#models.EmailRecipient.findAll({filter: `batch_id:'${batchId}'`, withRelated: ['member', 'member.stripeSubscriptions', 'member.products']}); const BATCH_SIZE = this.#sendingService.getMaximumRecipients(); if (models.length > BATCH_SIZE) { diff --git a/ghost/email-service/lib/EmailSegmenter.js b/ghost/email-service/lib/EmailSegmenter.js index ccdf61f26a..938d4adc19 100644 --- a/ghost/email-service/lib/EmailSegmenter.js +++ b/ghost/email-service/lib/EmailSegmenter.js @@ -26,7 +26,7 @@ class EmailSegmenter { } getMemberFilterForSegment(newsletter, emailRecipientFilter, segment) { - const filter = [`newsletters.id:${newsletter.id}`, 'email_disabled:0']; + const filter = [`newsletters.id:'${newsletter.id}'`, 'email_disabled:0']; switch (emailRecipientFilter) { case 'all': diff --git a/ghost/email-service/test/email-segmenter.test.js b/ghost/email-service/test/email-segmenter.test.js index 49606ad7aa..5df9610458 100644 --- a/ghost/email-service/test/email-segmenter.test.js +++ b/ghost/email-service/test/email-segmenter.test.js @@ -37,7 +37,7 @@ describe('Email segmenter', function () { ); listStub.calledOnce.should.be.true(); listStub.calledWith({ - filter: 'newsletters.id:newsletter-123+email_disabled:0' + filter: 'newsletters.id:\'newsletter-123\'+email_disabled:0' }).should.be.true(); response.should.eql(12); }); @@ -94,7 +94,7 @@ describe('Email segmenter', function () { listStub.calledOnce.should.be.true(); listStub.calledWith({ - filter: 'newsletters.id:newsletter-123+email_disabled:0+(labels:test)+status:-free' + filter: 'newsletters.id:\'newsletter-123\'+email_disabled:0+(labels:test)+status:-free' }).should.be.true(); response.should.eql(12); }); @@ -118,7 +118,7 @@ describe('Email segmenter', function () { listStub.calledOnce.should.be.true(); listStub.calledWith({ - filter: 'newsletters.id:newsletter-123+email_disabled:0+(labels:test)+(status:free)' + filter: 'newsletters.id:\'newsletter-123\'+email_disabled:0+(labels:test)+(status:free)' }).should.be.true(); response.should.eql(12); }); diff --git a/ghost/link-tracking/lib/LinkClickTrackingService.js b/ghost/link-tracking/lib/LinkClickTrackingService.js index bdd776a4a0..0c0e746e04 100644 --- a/ghost/link-tracking/lib/LinkClickTrackingService.js +++ b/ghost/link-tracking/lib/LinkClickTrackingService.js @@ -154,7 +154,7 @@ class LinkClickTrackingService { // manages transformation of current url to relative for comparision const transformedOldUrl = this.#urlUtils.absoluteToTransformReady(redirectUrl.href); - const filterQuery = `post_id:${postId}+to:'${transformedOldUrl}'`; + const filterQuery = `post_id:'${postId}'+to:'${transformedOldUrl}'`; const updatedFilterOptions = { ...filterOptions, diff --git a/ghost/members-api/lib/repositories/MemberRepository.js b/ghost/members-api/lib/repositories/MemberRepository.js index e79f1a3604..d1175f6ced 100644 --- a/ghost/members-api/lib/repositories/MemberRepository.js +++ b/ghost/members-api/lib/repositories/MemberRepository.js @@ -764,9 +764,9 @@ module.exports = class MemberRepository { if (data.action === 'unsubscribe') { const hasNewsletterSelected = (Object.prototype.hasOwnProperty.call(data, 'newsletter') && data.newsletter !== null); if (hasNewsletterSelected) { - const membersArr = memberIds.join(','); + const membersArr = memberIds.map(i => `'${i}'`).join(','); const unsubscribeRows = await this._MemberNewsletter.getFilteredCollectionQuery({ - filter: `newsletter_id:${data.newsletter}+member_id:[${membersArr}]` + filter: `newsletter_id:'${data.newsletter}'+member_id:[${membersArr}]` }); const toUnsubscribe = unsubscribeRows.map(row => row.id); diff --git a/ghost/post-revisions/src/PostRevisions.ts b/ghost/post-revisions/src/PostRevisions.ts index e6536f47c7..49d1994894 100644 --- a/ghost/post-revisions/src/PostRevisions.ts +++ b/ghost/post-revisions/src/PostRevisions.ts @@ -127,7 +127,7 @@ export class PostRevisions { // eslint-disable-next-line @typescript-eslint/no-explicit-any async removeAuthorFromRevisions(authorId: string, options: any): Promise { const revisions = await this.model.findAll({ - filter: `author_id:${authorId}`, + filter: `author_id:'${authorId}'`, columns: ['id'], transacting: options.transacting });