🐛 Fixed post settings author search to load users progressively and query the users API for not-yet-loaded authors#28907
Open
PaulAdamDavis wants to merge 1 commit into
Open
🐛 Fixed post settings author search to load users progressively and query the users API for not-yet-loaded authors#28907PaulAdamDavis wants to merge 1 commit into
PaulAdamDavis wants to merge 1 commit into
Conversation
Contributor
WalkthroughThis PR adds a shared NQL escaping helper, updates author selection to load authors in the background while enabling server-side search, and adjusts the Mirage 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost-admin:test |
✅ Succeeded | 2m 54s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 1m 50s | View ↗ |
nx run-many -t lint -p ghost-admin |
✅ Succeeded | 14s | View ↗ |
nx run ghost:build:assets |
✅ Succeeded | 2s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 3s | View ↗ |
nx run ghost:build:tsc |
✅ Succeeded | 5s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-25 16:28:01 UTC
PaulAdamDavis
force-pushed
the
fix-author-selection
branch
from
f2b4ed5 to
b214c7f
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/admin/tests/integration/components/gh-psm-authors-input-test.js (1)
105-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an escaped-apostrophe search test case.
Please add one integration case for terms like
O'Connorto lock theescapeNqlString→ Miragename:~'...'unescape contract and prevent regressions in quote handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/admin/tests/integration/components/gh-psm-authors-input-test.js` around lines 105 - 118, The existing integration coverage in gh-psm-authors-input-test.js only verifies slug/email searching; add another case in the same test module using the authors input flow to search for a name containing an apostrophe, such as O'Connor. Use the existing helpers like render, clickTrigger, typeInSearch, and settled to verify the search term survives the escapeNqlString to Mirage name:~'...' unescape path and that the matching author appears in the results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ghost/admin/tests/integration/components/gh-psm-authors-input-test.js`:
- Around line 105-118: The existing integration coverage in
gh-psm-authors-input-test.js only verifies slug/email searching; add another
case in the same test module using the authors input flow to search for a name
containing an apostrophe, such as O'Connor. Use the existing helpers like
render, clickTrigger, typeInSearch, and settled to verify the search term
survives the escapeNqlString to Mirage name:~'...' unescape path and that the
matching author appears in the results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a2e554d-1900-431b-80ce-39e92f9c167f
📒 Files selected for processing (6)
ghost/admin/app/components/gh-psm-authors-input.hbsghost/admin/app/components/gh-psm-authors-input.jsghost/admin/app/components/gh-resource-select.jsghost/admin/app/helpers/escape-nql-string.jsghost/admin/mirage/config/users.jsghost/admin/tests/integration/components/gh-psm-authors-input-test.js
…uery the users API for not-yet-loaded authors no ref Selecting authors in the post settings menu on a site with a large number of authors can be a very slow process, as users need to wait for them to all load before the dropdown list is populated. This change introduces an API query to find users that match what the user has typed into the input, so they can find a specific author before they have all loaded in. All users continue to load in the background as they currently do, so users can still browse all available authors without searching. For sites with <= 100 users, all users load instantly in a single query. For sites with > 100 users, users are loaded in batches of 100, and instantly added to the dropdown. When searching for a user that has not yet been loaded, it queries the API for users that match the typed query
PaulAdamDavis
force-pushed
the
fix-author-selection
branch
from
b214c7f to
f1fe211
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ghost/admin/app/components/gh-psm-authors-input.js (1)
90-100: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip remote query for empty search terms.
For empty terms, remote querying adds avoidable API load while background paging is already populating authors. Short-circuit to local results for
''/whitespace terms.Proposed fix
_fetchRemoteAuthors(term) { + if (!(term || '').trim()) { + return Promise.resolve([]); + } // match name, slug, or email. The OR group is parenthesised so it's // combined as a unit with the endpoint's default status filter. const nqlTerm = escapeNqlString(term); return this.store.query('user', {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/admin/app/components/gh-psm-authors-input.js` around lines 90 - 100, Skip the remote author lookup for empty or whitespace-only terms in gh-psm-authors-input’s _fetchRemoteAuthors method. Before calling this.store.query, trim the input and short-circuit when the term is blank so the component relies on already-loaded local/background-paged authors instead of issuing an unnecessary API request. Keep the existing remote query path for non-empty terms and preserve the current AUTHORS_INCLUDE, AUTHORS_ORDER, and PAGE_SIZE behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/admin/app/components/gh-psm-authors-input.js`:
- Around line 82-88: The searchAuthorsTask currently rejects if
_fetchRemoteAuthors(term) fails, which discards already-found local matches.
Update the task in gh-psm-authors-input.js to catch remote fetch errors around
_fetchRemoteAuthors and fall back to returning the localAuthors results (or the
merged local-only list) from _localAuthorMatches so search remains usable during
transient API failures. Keep the restartable searchAuthorsTask behavior intact
and ensure _mergeAuthors is still used when remote results are available.
---
Nitpick comments:
In `@ghost/admin/app/components/gh-psm-authors-input.js`:
- Around line 90-100: Skip the remote author lookup for empty or whitespace-only
terms in gh-psm-authors-input’s _fetchRemoteAuthors method. Before calling
this.store.query, trim the input and short-circuit when the term is blank so the
component relies on already-loaded local/background-paged authors instead of
issuing an unnecessary API request. Keep the existing remote query path for
non-empty terms and preserve the current AUTHORS_INCLUDE, AUTHORS_ORDER, and
PAGE_SIZE behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a41ff01-36ef-48ac-aac3-731ba15158c0
📒 Files selected for processing (6)
ghost/admin/app/components/gh-psm-authors-input.hbsghost/admin/app/components/gh-psm-authors-input.jsghost/admin/app/components/gh-resource-select.jsghost/admin/app/helpers/escape-nql-string.jsghost/admin/mirage/config/users.jsghost/admin/tests/integration/components/gh-psm-authors-input-test.js
🚧 Files skipped from review as they are similar to previous changes (4)
- ghost/admin/app/helpers/escape-nql-string.js
- ghost/admin/tests/integration/components/gh-psm-authors-input-test.js
- ghost/admin/mirage/config/users.js
- ghost/admin/app/components/gh-resource-select.js
Comment on lines
+82
to
+88
| searchAuthorsTask: task(function* (term) { | ||
| yield timeout(SEARCH_DEBOUNCE_MS); | ||
|
|
||
| const localAuthors = this._localAuthorMatches(term); | ||
| const authors = yield this._fetchRemoteAuthors(term); | ||
| return this._mergeAuthors(localAuthors, authors.toArray()); | ||
| }).restartable(), |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle remote search failures without dropping local results.
If _fetchRemoteAuthors(term) fails, this task rejects and returns no matches, even though local matches are already available. Catch and fall back to local results so search remains usable during transient API errors.
Proposed fix
searchAuthorsTask: task(function* (term) {
yield timeout(SEARCH_DEBOUNCE_MS);
const localAuthors = this._localAuthorMatches(term);
- const authors = yield this._fetchRemoteAuthors(term);
- return this._mergeAuthors(localAuthors, authors.toArray());
+ try {
+ const authors = yield this._fetchRemoteAuthors(term);
+ return this._mergeAuthors(localAuthors, authors.toArray());
+ } catch (error) {
+ return localAuthors;
+ }
}).restartable(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| searchAuthorsTask: task(function* (term) { | |
| yield timeout(SEARCH_DEBOUNCE_MS); | |
| const localAuthors = this._localAuthorMatches(term); | |
| const authors = yield this._fetchRemoteAuthors(term); | |
| return this._mergeAuthors(localAuthors, authors.toArray()); | |
| }).restartable(), | |
| searchAuthorsTask: task(function* (term) { | |
| yield timeout(SEARCH_DEBOUNCE_MS); | |
| const localAuthors = this._localAuthorMatches(term); | |
| try { | |
| const authors = yield this._fetchRemoteAuthors(term); | |
| return this._mergeAuthors(localAuthors, authors.toArray()); | |
| } catch (error) { | |
| return localAuthors; | |
| } | |
| }).restartable(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ghost/admin/app/components/gh-psm-authors-input.js` around lines 82 - 88, The
searchAuthorsTask currently rejects if _fetchRemoteAuthors(term) fails, which
discards already-found local matches. Update the task in gh-psm-authors-input.js
to catch remote fetch errors around _fetchRemoteAuthors and fall back to
returning the localAuthors results (or the merged local-only list) from
_localAuthorMatches so search remains usable during transient API failures. Keep
the restartable searchAuthorsTask behavior intact and ensure _mergeAuthors is
still used when remote results are available.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
no ref
Selecting authors in the post settings menu on a site with a large number of authors can be a very slow process, as users need to wait for them to all load before the dropdown list is populated.
This change introduces an API query to find users that match what the user has typed into the input, so they can find a specific author before they have all loaded in. All users continue to load in the background as they currently do, so users can still browse all available authors without searching.
For sites with <= 100 users, all users load instantly in a single query. For sites with > 100 users, users are loaded in batches of 100, and instantly added to the dropdown. When searching for a user that has not yet been loaded, it queries the API for users that match the typed query