Moved remaining membership settings acceptance tests into Admin - #29386
Conversation
ref https://linear.app/ghost/issue/PLA-243 - finishes the split membership test migration before the legacy harness teardown - keeps browser-level editor and API contracts in Admin while relying on focused unit and E2E coverage for lower- and higher-level behavior
WalkthroughReplaces removed Admin-X Playwright acceptance tests with Admin Vitest browser acceptance suites. The new tests cover member welcome email previews, editor behavior, sender customization, activation state, membership navigation visibility, and recommendations listing, validation, CRUD, deletion confirmation, and incoming recommendations. Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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 @tryghost/admin:test:acceptance |
✅ Succeeded | 4m 3s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-16 14:15:48 UTC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/admin/src/settings/growth/recommendations.acceptance.test.tsx (2)
91-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant locator reassignment.
Locators returned by
settingsScreen.section(...)evaluate dynamically on each action. Reassigningmodalto the exact same locator after transitioning to the next step is redundant.♻️ Proposed refactor
- let modal = settingsScreen.section("add-recommendation-modal"); + const modal = settingsScreen.section("add-recommendation-modal"); await modal.getByLabelText("URL").fill("not a real url"); await modal.getByRole("button", {name: "Next"}).click(); await expect.element(modal.getByText("Enter a valid URL")).toBeVisible(); await modal.getByLabelText("URL").fill("https://example.com/a-cool-website"); await modal.getByRole("button", {name: "Next"}).click(); - modal = settingsScreen.section("add-recommendation-modal");🤖 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 `@apps/admin/src/settings/growth/recommendations.acceptance.test.tsx` around lines 91 - 98, Remove the redundant reassignment of modal from settingsScreen.section("add-recommendation-modal") after clicking Next; continue using the existing modal locator for subsequent actions, since it resolves the current modal state dynamically.
86-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReturn a mocked recommendation entity from the create endpoint.
Returning an empty array from the
POSTcreation endpoint deviates from the actual API contract. While the current test passes because the frontend might not interact with the response body directly, it is safer to return a mocked representation of the created entity. This aligns with standard behavior and prevents future test breakages if the component begins to utilize the response (e.g., to read the new ID or updated timestamps).♻️ Proposed refactor
- const addApi = fakeAdminEndpoint("POST", "/recommendations/", {recommendations: []}); + const addApi = fakeAdminEndpoint("POST", "/recommendations/", { + recommendations: [{ + id: "new-recommendation-id", + title: "This is a title", + url: "https://example.com/a-cool-website", + description: "This is a description", + excerpt: null, + featured_image: null, + favicon: null, + one_click_subscribe: true, + }] + });🤖 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 `@apps/admin/src/settings/growth/recommendations.acceptance.test.tsx` at line 86, Update the mocked POST endpoint configured by addApi to return a representative created recommendation entity instead of an empty recommendations array. Reuse the test’s existing recommendation fixture or factory where available, ensuring the response includes the fields the real create API returns, such as the entity ID and timestamps.
🤖 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 `@apps/admin/src/settings/growth/recommendations.acceptance.test.tsx`:
- Around line 91-98: Remove the redundant reassignment of modal from
settingsScreen.section("add-recommendation-modal") after clicking Next; continue
using the existing modal locator for subsequent actions, since it resolves the
current modal state dynamically.
- Line 86: Update the mocked POST endpoint configured by addApi to return a
representative created recommendation entity instead of an empty recommendations
array. Reuse the test’s existing recommendation fixture or factory where
available, ensuring the response includes the fields the real create API
returns, such as the entity ID and timestamps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6802149-3575-4b06-a9b2-8d1541c2a9bc
📒 Files selected for processing (6)
apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.tsapps/admin-x-settings/test/acceptance/membership/membership-settings.test.tsapps/admin-x-settings/test/acceptance/membership/recommendations.test.tsapps/admin/src/settings/growth/recommendations.acceptance.test.tsxapps/admin/src/settings/membership/member-welcome-emails.acceptance.test.tsxapps/admin/src/settings/membership/membership-settings.acceptance.test.tsx
💤 Files with no reviewable changes (3)
- apps/admin-x-settings/test/acceptance/membership/membership-settings.test.ts
- apps/admin-x-settings/test/acceptance/membership/recommendations.test.ts
- apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts
1 participant
What changed
Coverage decisions
The replacement suite keeps browser-level behavior and API contracts in acceptance coverage:
Coverage was consolidated rather than copied mechanically:
Validation
apps/admin: 3 migrated files, 34/34 acceptance testsapps/admin-x-settings: 3 related unit files, 11/11 testsFollow-ups discovered
TableRowstructure (<td>beneath<div>) in recommendations and newsletters. This is outside the port-only scope and should be fixed separately.bson-objectid, reloaded the browser mid-run, and produced duplicate-React hook failures in unrelated suites. Focused suites remain green; the plan already identifies cold-cache optimization as a local papercut, and CI will provide the stable full-lane signal.ref https://linear.app/ghost/issue/PLA-243