Skip to content

Conversation

@kevinansfield
Copy link
Member

ref https://linear.app/ghost/issue/ONC-1396/

The email size warning service made a browse request to /newsletters/ on the Admin API in order to pass a newsletter id to the email preview endpoint but Contributors don't have permissions to access the newsletters endpoints resulting in a 403 network error and the email size warning silently failing.

  • reviewing the email preview endpoint shows it already fetches the default newsletter when one isn't provided
  • removed all newsletter fetching/passing code from the email size warning service thereby removing a request, fixing the 403 error noise, and enabling the feature to work for Contributors

ref https://linear.app/ghost/issue/ONC-1396/

The email size warning service made a browse request to `/newsletters/` on the Admin API in order to pass a newsletter id to the email preview endpoint but Contributors don't have permissions to access the newsletters endpoints resulting in a 403 network error and the email size warning silently failing.

- reviewing the email preview endpoint shows it already fetches the default newsletter when one isn't provided
- removed all newsletter fetching/passing code from the email size warning service thereby removing a request, fixing the 403 error noise, and enabling the feature to work for Contributors
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

Walkthrough

The pull request removes newsletter-related functionality from the EmailSizeWarningService. Specifically, the _newsletter property and _loadNewsletter() method were deleted from the service class. The _fetchTask flow was updated to no longer call _loadNewsletter() or include the newsletter slug as a query parameter in the API request URL. The overall fetch logic now executes without resolving or referencing a newsletter object.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing email size checking for Contributors by removing unnecessary newsletter fetching that caused permission errors.
Description check ✅ Passed The description thoroughly explains the issue, root cause, and solution, directly relating to the code changes that remove newsletter fetching from the email size warning service.
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

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.

@kevinansfield kevinansfield enabled auto-merge (squash) January 19, 2026 12:57
@kevinansfield kevinansfield merged commit 314b6ee into main Jan 19, 2026
36 checks passed
@kevinansfield kevinansfield deleted the fix-emails-size-warning-for-contributors branch January 19, 2026 13:14
kevinansfield added a commit that referenced this pull request Jan 19, 2026
ref https://linear.app/ghost/issue/ONC-1396/

The email size warning service made a browse request to `/newsletters/` on the Admin API in order to pass a newsletter id to the email preview endpoint but Contributors don't have permissions to access the newsletters endpoints resulting in a 403 network error and the email size warning silently failing.

- reviewing the email preview endpoint shows it already fetches the default newsletter when one isn't provided
- removed all newsletter fetching/passing code from the email size warning service thereby removing a request, fixing the 403 error noise, and enabling the feature to work for Contributors
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.

2 participants