Merged
Conversation
Contributor
WalkthroughThe member-email-editor component now reads global config, derives a tenorConfig from 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts (1)
325-408: Extract shared GIF-test setup into a helper.Both Tenor GIF tests repeat route stubbing, navigation, modal open, and editor prep. Pulling that into a helper will reduce maintenance overhead and make future editor-menu tests easier to add.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts` around lines 325 - 408, Extract the repeated Tenor/GIF test setup into a helper (e.g., setupWelcomeEmailGifTest) that performs the page.route stub for 'https://tenor.googleapis.com/**', calls mockApi with provided config (use configWithWelcomeEmailEditorEnabled or configWithWelcomeEmailEditorAndTenorEnabled), navigates to '/#/memberemails', waits for networkidle, opens the free welcome email preview (getByTestId('free-welcome-email-preview')), waits for the welcome-email-modal, focuses the editor (locator('[data-kg="editor"] div[contenteditable="true"]').first()), clears content and types '/gif'; then update the two tests ('welcome email editor does not show GIF selector when Tenor is not configured' and 'welcome email editor shows GIF selector when Tenor is configured') to call this helper with the appropriate config and only assert visibility of the slash menu and 'GIF' entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts`:
- Around line 325-365: The test "welcome email editor does not show GIF selector
when Tenor is not configured" currently stubs Tenor responses but doesn't assert
Tenor was never contacted; update the existing
page.route('https://tenor.googleapis.com/**', ...) handler to record a flag
(e.g. tenorCalled = false; set true inside the route callback) and after typing
'/gif' and asserting the GIF menu is not visible, add an assertion that the flag
is still false (or fail if the Tenor route was invoked) to ensure no Tenor
request was made; reference the route setup and the test block around the
editor/suffix checks to locate where to add the flag and final assertion.
---
Nitpick comments:
In
`@apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts`:
- Around line 325-408: Extract the repeated Tenor/GIF test setup into a helper
(e.g., setupWelcomeEmailGifTest) that performs the page.route stub for
'https://tenor.googleapis.com/**', calls mockApi with provided config (use
configWithWelcomeEmailEditorEnabled or
configWithWelcomeEmailEditorAndTenorEnabled), navigates to '/#/memberemails',
waits for networkidle, opens the free welcome email preview
(getByTestId('free-welcome-email-preview')), waits for the welcome-email-modal,
focuses the editor (locator('[data-kg="editor"]
div[contenteditable="true"]').first()), clears content and types '/gif'; then
update the two tests ('welcome email editor does not show GIF selector when
Tenor is not configured' and 'welcome email editor shows GIF selector when Tenor
is configured') to call this helper with the appropriate config and only assert
visibility of the slash menu and 'GIF' entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a4800b7-35ff-474d-80f3-f0ce1227267f
📥 Commits
Reviewing files that changed from the base of the PR and between 6c4680f and 7a1f26ab5d635bfba3a684bcf7e2a956820ab5b4.
📒 Files selected for processing (2)
apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsxapps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts
apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts
Show resolved
Hide resolved
EvanHahn
added
the
ok to merge for me
9larsons
approved these changes
Mar 4, 2026
EvanHahn
force-pushed
the
ny1123-welcome-email-editor-gifs
branch
from
6dfad3c to
f0e663f
Compare
EvanHahn
force-pushed
the
ny1123-welcome-email-editor-gifs
branch
from
f0e663f to
543b660
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts (1)
381-465: Consider extracting shared GIF-test setup into a helper.Both GIF tests repeat the same modal/editor bootstrapping sequence; pulling that into a small helper would reduce maintenance cost and keep future test updates in sync.
♻️ Optional refactor sketch
+const openWelcomeEmailEditorAndFocus = async (page: Page) => { + await page.goto('/#/memberemails'); + await page.waitForLoadState('networkidle'); + const section = page.getByTestId('memberemails'); + await expect(section).toBeVisible({timeout: 10000}); + await section.getByTestId('free-welcome-email-preview').click(); + const modal = page.getByTestId('welcome-email-modal'); + await expect(modal).toBeVisible(); + const editor = modal.locator('[data-kg="editor"] div[contenteditable="true"]').first(); + await editor.click({timeout: 5000}); + await expect(editor).toBeFocused(); + await editor.press('ControlOrMeta+a'); + await editor.press('Backspace'); + return {modal, editor}; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts` around lines 381 - 465, Extract the repeated modal/editor setup in the two GIF tests into a reusable helper function (e.g., openWelcomeEmailEditor) that performs the mockApi call setup for browseConfig and browseAutomatedEmails, navigates to '/#/memberemails', waits for networkidle, clicks the 'free-welcome-email-preview' in the 'memberemails' section, waits for 'welcome-email-modal' visibility, and returns the editor locator (modal.locator('[data-kg="editor"] div[contenteditable="true"]').first()) and the slashMenu locator (page.locator('[data-kg-slash-menu]')); replace the duplicated code in both tests to call this helper and then perform the editor interactions and assertions for GIF visibility, keeping existing mocks for the Tenor route outside or passed in as parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts`:
- Around line 381-465: Extract the repeated modal/editor setup in the two GIF
tests into a reusable helper function (e.g., openWelcomeEmailEditor) that
performs the mockApi call setup for browseConfig and browseAutomatedEmails,
navigates to '/#/memberemails', waits for networkidle, clicks the
'free-welcome-email-preview' in the 'memberemails' section, waits for
'welcome-email-modal' visibility, and returns the editor locator
(modal.locator('[data-kg="editor"] div[contenteditable="true"]').first()) and
the slashMenu locator (page.locator('[data-kg-slash-menu]')); replace the
duplicated code in both tests to call this helper and then perform the editor
interactions and assertions for GIF visibility, keeping existing mocks for the
Tenor route outside or passed in as parameters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6043223c-035b-46a5-ad54-0d0c69e241e8
📥 Commits
Reviewing files that changed from the base of the PR and between 7a1f26ab5d635bfba3a684bcf7e2a956820ab5b4 and f0e663f6b4ad3beca755ade20115d4c9b88a5f5e.
📒 Files selected for processing (2)
apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsxapps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts (1)
382-393: Extract duplicated Tenor route mocking into a helper.The same
page.route('https://tenor.googleapis.com/**', ...)setup appears twice; consolidating it will reduce maintenance overhead.♻️ Proposed refactor
+const mockTenorApi = async (page: Page) => { + await page.route('https://tenor.googleapis.com/**', async (route) => { + await route.fulfill({ + status: 200, + body: JSON.stringify({ + next: null, + results: [] + }), + headers: { + 'content-type': 'application/json' + } + }); + }); +}; + test.describe('Member emails settings', async () => { @@ test('welcome email editor does not show GIF selector when Tenor is not configured', async ({page}) => { - await page.route('https://tenor.googleapis.com/**', async (route) => { - await route.fulfill({ - status: 200, - body: JSON.stringify({ - next: null, - results: [] - }), - headers: { - 'content-type': 'application/json' - } - }); - }); + await mockTenorApi(page); @@ test('welcome email editor shows GIF selector when Tenor is configured', async ({page}) => { - await page.route('https://tenor.googleapis.com/**', async (route) => { - await route.fulfill({ - status: 200, - body: JSON.stringify({ - next: null, - results: [] - }), - headers: { - 'content-type': 'application/json' - } - }); - }); + await mockTenorApi(page);Also applies to: 424-435
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts` around lines 382 - 393, Duplicate page.route('https://tenor.googleapis.com/**', ...) mocking should be extracted into a reusable helper; create a helper function (e.g., mockTenorRoute or setupTenorMock) that accepts the Playwright page and calls page.route with the existing fulfill payload (status 200, body JSON { next:null, results:[] }, content-type application/json), then replace both inline route mocks in member-welcome-emails.test.ts (the two page.route blocks) with calls to that helper to keep behavior identical and reduce duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts`:
- Around line 382-393: Duplicate page.route('https://tenor.googleapis.com/**',
...) mocking should be extracted into a reusable helper; create a helper
function (e.g., mockTenorRoute or setupTenorMock) that accepts the Playwright
page and calls page.route with the existing fulfill payload (status 200, body
JSON { next:null, results:[] }, content-type application/json), then replace
both inline route mocks in member-welcome-emails.test.ts (the two page.route
blocks) with calls to that helper to keep behavior identical and reduce
duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 445e7631-f989-48b9-b5d8-386d9b636a1f
📥 Commits
Reviewing files that changed from the base of the PR and between f0e663f6b4ad3beca755ade20115d4c9b88a5f5e and 543b660.
📒 Files selected for processing (2)
apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsxapps/admin-x-settings/test/acceptance/membership/member-welcome-emails.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsx
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.
closes https://linear.app/ghost/issue/NY-1123
Screencast.mp4