From 27e771b3a8917449ccb19db07a65e46b914429fd Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Mon, 6 May 2024 22:04:13 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20Admin=20search=20sometim?= =?UTF-8?q?es=20stalling=20on=20first=20query=20(#20143)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes https://linear.app/tryghost/issue/MOM-103 - the `yield waitForProperty(...)` call that was supposed to return once the content refresh occurred never reached a valid state so the first search query (or any later query) where a content refresh occurred would never resolve causing search to look like it had stalled - switched to waiting for the last running task to resolve instead which does the same as the previous code intended - exported the `getPosts` request handler function so in mirage config so we can re-use it with different timing on a per-case basis --- ghost/admin/app/services/search.js | 4 +- ghost/admin/mirage/config/posts.js | 43 +++++++++++---------- ghost/admin/tests/acceptance/search-test.js | 23 +++++++++++ 3 files changed, 48 insertions(+), 22 deletions(-) diff --git a/ghost/admin/app/services/search.js b/ghost/admin/app/services/search.js index 657d60da70..ccebac2911 100644 --- a/ghost/admin/app/services/search.js +++ b/ghost/admin/app/services/search.js @@ -3,7 +3,7 @@ import Service from '@ember/service'; import {isBlank, isEmpty} from '@ember/utils'; import {pluralize} from 'ember-inflector'; import {inject as service} from '@ember/service'; -import {task, timeout, waitForProperty} from 'ember-concurrency'; +import {task, timeout} from 'ember-concurrency'; export default class SearchService extends Service { @service ajax; @@ -59,7 +59,7 @@ export default class SearchService extends Service { // wait for any on-going refresh to finish if (this.refreshContentTask.isRunning) { - yield waitForProperty(this, 'refreshContentTask.isIdle'); + yield this.refreshContentTask.lastRunning; } const searchResult = this._searchContent(term); diff --git a/ghost/admin/mirage/config/posts.js b/ghost/admin/mirage/config/posts.js index 4d4643a12d..3771dd191b 100644 --- a/ghost/admin/mirage/config/posts.js +++ b/ghost/admin/mirage/config/posts.js @@ -23,6 +23,28 @@ function extractTags(postAttrs, tags) { }); } +// TODO: handle authors filter +export function getPosts({posts}, {queryParams}) { + let {filter, page, limit} = queryParams; + + page = +page || 1; + limit = +limit || 15; + + let statusFilter = extractFilterParam('status', filter); + + let collection = posts.all().filter((post) => { + let matchesStatus = true; + + if (!isEmpty(statusFilter)) { + matchesStatus = statusFilter.includes(post.status); + } + + return matchesStatus; + }); + + return paginateModelCollection('posts', collection, page, limit); +} + export default function mockPosts(server) { server.post('/posts', function ({posts, users, tags}) { let attrs = this.normalizedRequestAttrs(); @@ -38,26 +60,7 @@ export default function mockPosts(server) { }); // TODO: handle authors filter - server.get('/posts/', function ({posts}, {queryParams}) { - let {filter, page, limit} = queryParams; - - page = +page || 1; - limit = +limit || 15; - - let statusFilter = extractFilterParam('status', filter); - - let collection = posts.all().filter((post) => { - let matchesStatus = true; - - if (!isEmpty(statusFilter)) { - matchesStatus = statusFilter.includes(post.status); - } - - return matchesStatus; - }); - - return paginateModelCollection('posts', collection, page, limit); - }); + server.get('/posts/', getPosts); server.get('/posts/:id/', function ({posts}, {params}) { let {id} = params; diff --git a/ghost/admin/tests/acceptance/search-test.js b/ghost/admin/tests/acceptance/search-test.js index e275b8a1c8..339748371e 100644 --- a/ghost/admin/tests/acceptance/search-test.js +++ b/ghost/admin/tests/acceptance/search-test.js @@ -3,6 +3,7 @@ import {authenticateSession} from 'ember-simple-auth/test-support'; import {click, currentURL, find, findAll, triggerKeyEvent, visit} from '@ember/test-helpers'; import {describe, it} from 'mocha'; import {expect} from 'chai'; +import {getPosts} from '../../mirage/config/posts'; import {setupApplicationTest} from 'ember-mocha'; import {setupMirage} from 'ember-cli-mirage/test-support'; import {typeInSearch} from 'ember-power-select/test-support/helpers'; @@ -142,4 +143,26 @@ describe('Acceptance: Search', function () { await typeInSearch('x'); expect(find('.ember-power-select-option--no-matches-message'), 'no results message').to.contain.text('No results found'); }); + + // https://linear.app/tryghost/issue/MOM-103/search-stalls-on-query-when-refresh-occurs + it('handles refresh on first search being slow', async function () { + this.server.get('/posts/', getPosts, {timing: 200}); + + await visit('/dashboard'); + await click('[data-test-button="search"]'); + await typeInSearch('first'); // search is not case sensitive + + // all groups are present + const groupNames = findAll('.ember-power-select-group-name'); + expect(groupNames, 'group names').to.have.length(4); + expect(groupNames.map(el => el.textContent.trim())).to.deep.equal(['Posts', 'Pages', 'Staff', 'Tags']); + + // correct results are found + const options = findAll('.ember-power-select-option'); + expect(options, 'number of search results').to.have.length(4); + expect(options.map(el => el.textContent.trim())).to.deep.equal(['First post', 'First page', 'First user', 'First tag']); + + // first item is selected + expect(options[0]).to.have.attribute('aria-current', 'true'); + }); });