Allowed embedding in new welcome email editor#26649
Conversation
towards https://linear.app/ghost/issue/NY-1101 ref TryGhost/Koenig#1750 You can now paste URLs and use the bookmark card in the welcome email editor.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughAdds a new React hook 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
🤖 Velo CI Failure AnalysisClassification: 🟠 SOFT FAIL
|
towards https://linear.app/ghost/issue/NY-1101 ref TryGhost/Ghost#26649 See TryGhost/Ghost#26649 for details on this change. --------- Co-authored-by: Steve Larson <9larsons@gmail.com>
|
Bumping Koenig packages in #26654 and then we can update this branch. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 226-265: The test is failing because the test IDs
embed-url-loading-container and embed-iframe referenced by the test do not
exist; either add these IDs where the Koenig editor renders embed state/iframe
in the MemberEmailEditor or WelcomeEmailModal (or inside KoenigEditorBase where
embed rendering occurs) or change the test to use the actual selectors/test IDs
currently emitted by those components; update the code in the component that
handles oEmbed loading/iframe rendering (look for MemberEmailEditor,
WelcomeEmailModal, and KoenigEditorBase) to include
data-testid="embed-url-loading-container" for the loading indicator and
data-testid="embed-iframe" on the rendered iframe element, or modify the test
assertions to target the existing DOM hooks instead.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/admin-x-framework/src/hooks.tsapps/admin-x-framework/src/hooks/use-koenig-fetch-embed.tsapps/admin-x-framework/src/index.tsapps/admin-x-framework/test/unit/hooks/use-koenig-fetch-embed.test.tsapps/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
🤖 Velo CI Failure AnalysisClassification: 🟠 SOFT FAIL
|
9larsons
left a comment
There was a problem hiding this comment.
We can likely clean up these tests however that's a project for another day. I'd sooner see us move settings into admin's app.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #26649 +/- ##
==========================================
- Coverage 73.24% 73.24% -0.01%
==========================================
Files 1528 1528
Lines 120335 120335
Branches 14544 14544
==========================================
- Hits 88139 88134 -5
+ Misses 31195 31182 -13
- Partials 1001 1019 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
towards https://linear.app/ghost/issue/NY-1101
ref TryGhost/Koenig#1750
You can now paste URLs and use the bookmark card in the welcome email editor.
Screencast.mp4