From 558845892a2dd330ae624389521ff9063e07232f Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 14 Mar 2024 11:00:22 -0400 Subject: [PATCH] fixup! Optimised queries made by get helper for posts --- ghost/core/core/frontend/helpers/get.js | 14 ++-- .../test/e2e-frontend/helpers/get.test.js | 78 +++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/ghost/core/core/frontend/helpers/get.js b/ghost/core/core/frontend/helpers/get.js index 238dd2754ac8..5eab4393efaf 100644 --- a/ghost/core/core/frontend/helpers/get.js +++ b/ghost/core/core/frontend/helpers/get.js @@ -140,7 +140,7 @@ async function makeAPICall(resource, controllerName, action, apiOptions) { const parsedFilter = nqlLang.parse(apiOptions.filter); // Support either `id:blah` or `id:blah+other:stuff` if (parsedFilter.$and || parsedFilter.id) { - const queries = parsedFilter.$and || [parsedFilter.id]; + const queries = parsedFilter.$and || [parsedFilter]; for (const query of queries) { if ('id' in query) { @@ -152,7 +152,9 @@ async function makeAPICall(resource, controllerName, action, apiOptions) { // The default limit is 15, the addition order is to cast apiOptions.limit to a number let limit = apiOptions.limit; - limit = apiOptions.limit && apiOptions.limit !== 'all' ? 1 + apiOptions.limit : 16; + if (apiOptions.limit !== 'all') { + limit = apiOptions.limit ? 1 + parseInt(apiOptions.limit, 10) : 16; + } // We replace with id:-null so we don't have to deal with leading/trailing AND operators const filter = apiOptions.filter.replace(/id:-[a-f0-9A-F]{24}/, 'id:-null'); @@ -164,11 +166,13 @@ async function makeAPICall(resource, controllerName, action, apiOptions) { filter }); + const filteredPosts = result?.posts?.filter((post) => { + return post.id !== idToFilter; + }) || []; + return { ...result, - posts: result?.posts?.filter((post) => { - return post.id !== idToFilter; - }).slice(0, limit - 1) || [] + posts: limit === 'all' ? filteredPosts : filteredPosts.slice(0, limit - 1) }; }; } diff --git a/ghost/core/test/e2e-frontend/helpers/get.test.js b/ghost/core/test/e2e-frontend/helpers/get.test.js index b37bbd237f0b..0bfa5cfed708 100644 --- a/ghost/core/test/e2e-frontend/helpers/get.test.js +++ b/ghost/core/test/e2e-frontend/helpers/get.test.js @@ -100,6 +100,83 @@ describe('e2e {{#get}} helper', function () { }); describe('Filter optimisation', function () { + it('Returns the correct posts with limit as a string', async function () { + await get.call({}, 'posts', { + hash: { + limit: '5' + }, + data: {}, + locals, + fn, + inverse + }); + const firstPostUsually = fn.firstCall.args[0].posts[0]; + const firstLimit = fn.firstCall.args[0].meta.pagination.limit; + assert.equal(firstLimit, 5); + await get.call({}, 'posts', { + hash: { + filter: `id:-${firstPostUsually.id}`, + limit: '5' + }, + data: {}, + locals, + fn, + inverse + }); + assert.equal(fn.secondCall.args[0].meta.pagination.limit, firstLimit + 1); + const foundFilteredPost = fn.secondCall.args[0].posts.find(post => post.id === firstPostUsually.id); + assert.equal(foundFilteredPost, undefined); + }); + it('Returns the correct posts with default limit', async function () { + await get.call({}, 'posts', { + hash: {}, + data: {}, + locals, + fn, + inverse + }); + const firstPostUsually = fn.firstCall.args[0].posts[0]; + const defaultLimit = fn.firstCall.args[0].meta.pagination.limit; + await get.call({}, 'posts', { + hash: { + filter: `id:-${firstPostUsually.id}` + }, + data: {}, + locals, + fn, + inverse + }); + assert.equal(fn.secondCall.args[0].meta.pagination.limit, defaultLimit + 1); + const foundFilteredPost = fn.secondCall.args[0].posts.find(post => post.id === firstPostUsually.id); + assert.equal(foundFilteredPost, undefined); + }); + it('Returns the correct posts with a limit all', async function () { + await get.call({}, 'posts', { + hash: { + limit: 'all' + }, + data: {}, + locals, + fn, + inverse + }); + const firstPostUsually = fn.firstCall.args[0].posts[0]; + const initialCount = fn.firstCall.args[0].posts.length; + await get.call({}, 'posts', { + hash: { + filter: `id:-${firstPostUsually.id}`, + limit: 'all' + }, + data: {}, + locals, + fn, + inverse + }); + assert.equal(fn.secondCall.args[0].posts.length, initialCount - 1); + assert.equal(fn.secondCall.args[0].meta.pagination.limit, 'all'); + const foundFilteredPost = fn.secondCall.args[0].posts.find(post => post.id === firstPostUsually.id); + assert.equal(foundFilteredPost, undefined); + }); it('Returns the correct posts with a solo negative filter', async function () { await get.call({}, 'posts', { hash: { @@ -122,6 +199,7 @@ describe('e2e {{#get}} helper', function () { inverse }); assert.equal(fn.secondCall.args[0].posts.length, 5); + assert.equal(fn.secondCall.args[0].meta.pagination.limit, 6); const foundFilteredPost = fn.secondCall.args[0].posts.find(post => post.id === firstPostUsually.id); assert.equal(foundFilteredPost, undefined); });