Migrated staff invite browser test to root e2e framework#26977
Conversation
The old browser test in `ghost/core/test/e2e-browser/portal/invites.spec.js` had two identical tests (the "2FA" variant never enabled 2FA). This replaces both with a single e2e test that uses Mailpit to extract the invite URL instead of reaching into the database via models.
WalkthroughThis change refactors and modernizes the end-to-end testing infrastructure for staff invite functionality. It introduces new page object helpers ( 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
e2e/tests/admin/settings/staff-invites.test.ts (1)
23-26: Consider using the page object'sgotomethod for consistency.Line 25 calls
signupPage.goto(inviteUrl)on the raw Playwright page, whileinviteSignupis the page object. UsinginviteSignup.goto(inviteUrl)would be more consistent with page object patterns, asBasePage.goto()accepts an optional URL parameter that overridespageUrl.Suggested fix
await withIsolatedPage(browser, {baseURL}, async ({page: signupPage}) => { const inviteSignup = new InviteSignupPage(signupPage); - await signupPage.goto(inviteUrl); + await inviteSignup.goto(inviteUrl); await inviteSignup.acceptInvite('Test Invite User', testEmail, 'test123456');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/tests/admin/settings/staff-invites.test.ts` around lines 23 - 26, The test uses the raw Playwright page object (signupPage.goto(inviteUrl)) instead of the InviteSignupPage page-object; change the call to use the page-object's navigation method (inviteSignup.goto(inviteUrl)) so BasePage.goto() can apply its optional URL handling and keep the pattern consistent with InviteSignupPage and its acceptInvite(...) usage.e2e/helpers/pages/admin/invite-signup-page.ts (1)
20-26: Consider whether filling the email field is necessary.When accepting an invite, Ghost typically pre-populates the email field from the invite token (it's the email the invite was sent to). The
fill()call on line 23 may overwrite a pre-populated value with the same value, which is harmless but potentially redundant.If the email field is ever made read-only in the future, this could cause the test to fail. Consider checking if filling is actually required, or adding a comment explaining why it's explicitly filled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/helpers/pages/admin/invite-signup-page.ts` around lines 20 - 26, The acceptInvite method is filling the email field even though the invite flow usually pre-populates it; to avoid redundant writes or future failures if the field becomes read-only, update acceptInvite to only fill the email when necessary (e.g., if emailField.inputValue() is empty or emailField.isEditable() returns true) or remove the emailField.fill(email) call and add a short comment on why the email is omitted; reference the acceptInvite function and the nameField, emailField, passwordField, and createAccountButton elements when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/tests/admin/settings/staff-invites.test.ts`:
- Around line 19-21: Add a guard after calling emailClient.search to ensure
messages[0] exists before passing it to emailClient.getMessageDetailed;
specifically, check the messages array returned by emailClient.search (used with
testEmail) and if empty/undefined throw or assert with a clear error like
"Invite email not found for testEmail" so extractInviteLink and
emailClient.getMessageDetailed are only called when a message is present.
---
Nitpick comments:
In `@e2e/helpers/pages/admin/invite-signup-page.ts`:
- Around line 20-26: The acceptInvite method is filling the email field even
though the invite flow usually pre-populates it; to avoid redundant writes or
future failures if the field becomes read-only, update acceptInvite to only fill
the email when necessary (e.g., if emailField.inputValue() is empty or
emailField.isEditable() returns true) or remove the emailField.fill(email) call
and add a short comment on why the email is omitted; reference the acceptInvite
function and the nameField, emailField, passwordField, and createAccountButton
elements when making the change.
In `@e2e/tests/admin/settings/staff-invites.test.ts`:
- Around line 23-26: The test uses the raw Playwright page object
(signupPage.goto(inviteUrl)) instead of the InviteSignupPage page-object; change
the call to use the page-object's navigation method
(inviteSignup.goto(inviteUrl)) so BasePage.goto() can apply its optional URL
handling and keep the pattern consistent with InviteSignupPage and its
acceptInvite(...) usage.
🪄 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: 1caaf755-cb58-4492-b7ed-4165149baf92
📒 Files selected for processing (6)
e2e/helpers/pages/admin/index.tse2e/helpers/pages/admin/invite-signup-page.tse2e/helpers/pages/admin/settings/sections/staff-section.tse2e/helpers/services/email/utils.tse2e/tests/admin/settings/staff-invites.test.tsghost/core/test/e2e-browser/portal/invites.spec.js
💤 Files with no reviewable changes (1)
- ghost/core/test/e2e-browser/portal/invites.spec.js
| const messages = await emailClient.search({subject: 'has invited you to join', to: testEmail}); | ||
| const latestMessage = await emailClient.getMessageDetailed(messages[0]); | ||
| const inviteUrl = extractInviteLink(latestMessage); |
There was a problem hiding this comment.
Add guard for missing email message.
If the invite email isn't found (e.g., timing issue or Mailpit delay), messages[0] will be undefined, leading to a confusing error from getMessageDetailed(). Adding an explicit check would provide a clearer failure message.
Suggested fix
const messages = await emailClient.search({subject: 'has invited you to join', to: testEmail});
+ expect(messages.length).toBeGreaterThan(0);
const latestMessage = await emailClient.getMessageDetailed(messages[0]);
const inviteUrl = extractInviteLink(latestMessage);Or with a more descriptive assertion:
const messages = await emailClient.search({subject: 'has invited you to join', to: testEmail});
+ expect(messages, `Expected invite email to be sent to ${testEmail}`).toHaveLength(1);
const latestMessage = await emailClient.getMessageDetailed(messages[0]);📝 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.
| const messages = await emailClient.search({subject: 'has invited you to join', to: testEmail}); | |
| const latestMessage = await emailClient.getMessageDetailed(messages[0]); | |
| const inviteUrl = extractInviteLink(latestMessage); | |
| const messages = await emailClient.search({subject: 'has invited you to join', to: testEmail}); | |
| expect(messages, `Expected invite email to be sent to ${testEmail}`).toHaveLength(1); | |
| const latestMessage = await emailClient.getMessageDetailed(messages[0]); | |
| const inviteUrl = extractInviteLink(latestMessage); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/tests/admin/settings/staff-invites.test.ts` around lines 19 - 21, Add a
guard after calling emailClient.search to ensure messages[0] exists before
passing it to emailClient.getMessageDetailed; specifically, check the messages
array returned by emailClient.search (used with testEmail) and if
empty/undefined throw or assert with a clear error like "Invite email not found
for testEmail" so extractInviteLink and emailClient.getMessageDetailed are only
called when a message is present.
1 participant
Summary
ghost/core/test/e2e-browser/portal/invites.spec.jstoe2e/tests/admin/settings/staff-invites.test.tsmodels.Invite.findOne()Changes
e2e/tests/admin/settings/staff-invites.test.ts— invite acceptance teste2e/helpers/pages/admin/invite-signup-page.ts— page object for/ghost/#/signup/:tokene2e/helpers/pages/admin/settings/sections/staff-section.ts— addedinviteUser()methode2e/helpers/services/email/utils.ts— addedextractInviteLink()ghost/core/test/e2e-browser/portal/invites.spec.jsTest plan
yarn lintpassesyarn test:typespassesyarn test tests/admin/settings/staff-invites.test.tspasses