Skip to content

🐛 Fixed staff invite error for existing users#28294

Merged
aileen merged 2 commits into
TryGhost:mainfrom
aileen:aileen/onc-1778-staff-invite-error
Jun 2, 2026
Merged

🐛 Fixed staff invite error for existing users#28294
aileen merged 2 commits into
TryGhost:mainfrom
aileen:aileen/onc-1778-staff-invite-error

Conversation

@aileen
Copy link
Copy Markdown
Member

@aileen aileen commented Jun 1, 2026

closes https://linear.app/ghost/issue/ONC-1778

Summary

  • Maps the invite API duplicate-user validation error back to the email field in the staff invite modal
  • Keeps pagination behavior intact while avoiding the generic failure toast for existing staff users
  • Adds an acceptance test for an existing user outside the loaded first users page

Testing

  • NODE_PATH=/Users/aileen/code/tryghost/Ghost/node_modules/.pnpm/node_modules pnpm --filter @tryghost/admin-x-settings test:acceptance test/acceptance/general/users/invite.test.ts --project=chromium
  • NODE_PATH=/Users/aileen/code/tryghost/Ghost/node_modules/.pnpm/node_modules pnpm --filter @tryghost/admin-x-settings lint:js

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 878cf6ef-a9db-4752-a49e-61edad9507b8

📥 Commits

Reviewing files that changed from the base of the PR and between 81e2cea and 1ce6687.

📒 Files selected for processing (4)
  • apps/admin-x-settings/src/components/settings/general/invite-user-modal.tsx
  • apps/admin-x-settings/test/acceptance/general/users/invite.test.ts
  • ghost/core/core/server/api/endpoints/utils/validators/input/invites.js
  • ghost/core/test/e2e-api/admin/invites.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/admin-x-settings/src/components/settings/general/invite-user-modal.tsx

Walkthrough

This PR refines the invite-user modal to provide clearer error feedback when users attempt to invite existing users. It imports ValidationError, defines two error message constants, and adds special-case handling in the error catch block: if a ValidationError contains the "User is already registered." message, the component clears save state and sets the email field error to "A user with that email address already exists." instead of showing a generic API failure toast. A new acceptance test verifies this behavior by mocking a 422 validation response for an existing user email and confirming field-level validation text appears without a visible error toast.

Suggested reviewers

  • sam-lord
  • mike182uk
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: fixing a staff invite error for existing users, which aligns with the core objective of mapping the duplicate-user validation error.
Description check ✅ Passed The description is directly related to the changeset, outlining how the invite API validation error is mapped to the email field, pagination behavior is preserved, and includes a test case reference.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@aileen aileen force-pushed the aileen/onc-1778-staff-invite-error branch from a0afecf to b7677be Compare June 2, 2026 03:07
closes [ONC-1778](https://linear.app/ghost/issue/ONC-1778/bad-error-message-when-inviting-staff-that-already-exist)

Staff invite validation could miss existing users outside the first paginated users response, then show a generic failure toast even though the API returned a known duplicate-user validation error. Mapping that server validation back to the email field keeps the paginated staff list while preserving a useful form error.
@aileen aileen force-pushed the aileen/onc-1778-staff-invite-error branch from b7677be to 81e2cea Compare June 2, 2026 03:08
@aileen aileen requested a review from sagzy June 2, 2026 03:09
@aileen aileen changed the title Fixed staff invite error for existing users 🐛 Fixed staff invite error for existing users Jun 2, 2026
Comment thread apps/admin-x-settings/src/components/settings/general/invite-user-modal.tsx Outdated
ref [ONC-1778](https://linear.app/ghost/issue/ONC-1778/bad-error-message-when-inviting-staff-that-already-exist)

The staff invite modal should not depend on a translated validation message or context string to identify duplicate staff users. The API now returns a stable USER_ALREADY_REGISTERED code for that validation, and the Admin UI maps that code to the existing inline email error.

type RoleType = 'administrator' | 'editor' | 'author' | 'contributor' | 'super editor';

const USER_ALREADY_REGISTERED_CODE = 'USER_ALREADY_REGISTERED';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unnecessary constant / abstraction, could inline it

if (validationError?.code === 'USER_ALREADY_REGISTERED') { ... } 

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree to disagree here. I would class this as a classical magic string which I'd probably tidy up going through the code later on 😄 I guess that's more about personal preference in this case 😅

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh definitely taste-based!

Personally, I go for:

  • Abstract in a constant if used in multiple places (benefit: if updated, need to update a single place)
  • Keep it inline if used in a single place (still have the same benefit of updating a single place, but don't create the overhead of an abstraction, i.e. having to jump to another piece of code when reading)

@aileen aileen merged commit 9656f21 into TryGhost:main Jun 2, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants