Skip to content

Persist newsletter selection in email preview#26885

Open
peterzimon wants to merge 1 commit intomainfrom
codex/DES-1314/persist-newsletter-selection
Open

Persist newsletter selection in email preview#26885
peterzimon wants to merge 1 commit intomainfrom
codex/DES-1314/persist-newsletter-selection

Conversation

@peterzimon
Copy link
Contributor

@peterzimon peterzimon commented Mar 19, 2026

What this changes

  • persists selected newsletter state in post email preview across modal reopen
  • threads persisted state through publish management -> preview modal -> email preview
  • keeps selected newsletter in sync with active newsletters list and falls back safely
  • adds acceptance coverage for reopen/persistence behavior

Testing

  • not run in this environment

Notes

  • local pre-commit hook failed in this checkout because yarn lint-staged command was not available; commit was created with --no-verify

ref https://linear.app/tryghost/issue/DES-1314

Email preview now remembers the selected newsletter across modal reopen and covers it with an acceptance assertion.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 19, 2026

Walkthrough

This change extends the email preview modal to support persistent newsletter selection across modal interactions. The preview template now passes two additional arguments (initialPreviewNewsletter and changePreviewNewsletter) to the email preview component. The email preview component was updated to initialize newsletter state from initialPreviewNewsletter, reconcile it against freshly loaded newsletters, and trigger a callback when selection changes. The publish management component adds state tracking for preview newsletter selection and wires the callback handler. Acceptance tests verify that selected newsletters persist when the modal reopens.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: persisting newsletter selection across email preview modal reopens.
Description check ✅ Passed The description is clearly related to the changeset, detailing what state is persisted, how it flows through components, and coverage added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/DES-1314/persist-newsletter-selection
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
ghost/admin/app/components/editor/modals/preview/email.js (1)

178-182: Use setNewsletter in the matched-newsletter branch for consistent sync behavior.

Direct assignment here skips parent callback propagation and iframe refresh logic used elsewhere.

Suggested patch
-        const selectedNewsletter = this.newsletter && newslettersList.findBy('id', this.newsletter.id);
+        const selectedNewsletter = this.newsletter && newslettersList.findBy('id', this.newsletter.id);
         if (selectedNewsletter) {
-            this.newsletter = selectedNewsletter;
+            this.setNewsletter(selectedNewsletter);
             return;
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghost/admin/app/components/editor/modals/preview/email.js` around lines 178 -
182, The branch that handles a matched newsletter currently assigns
this.newsletter directly which bypasses the parent callback and iframe refresh;
replace the direct assignment with a call to the existing setNewsletter method
(e.g., this.setNewsletter(selectedNewsletter)) so the selection uses the same
propagation and refresh logic as other branches and keeps behavior consistent
with setNewsletter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ghost/admin/app/components/editor/modals/preview/email.js`:
- Around line 178-182: The branch that handles a matched newsletter currently
assigns this.newsletter directly which bypasses the parent callback and iframe
refresh; replace the direct assignment with a call to the existing setNewsletter
method (e.g., this.setNewsletter(selectedNewsletter)) so the selection uses the
same propagation and refresh logic as other branches and keeps behavior
consistent with setNewsletter.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5c1d4e86-6093-4fb5-a274-3e549832e535

📥 Commits

Reviewing files that changed from the base of the PR and between b396ba5 and a808b38.

📒 Files selected for processing (4)
  • ghost/admin/app/components/editor/modals/preview.hbs
  • ghost/admin/app/components/editor/modals/preview/email.js
  • ghost/admin/app/components/editor/publish-management.js
  • ghost/admin/tests/acceptance/editor/post-email-preview-test.js

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.

1 participant