Skip to content

Newsletter Mode: add WordPress.com page shell and routes - #50973

Closed
davemart-in wants to merge 2 commits into
trunkfrom
add/wpcom-newsletter-mode-shell
Closed

davemart-in wants to merge 2 commits into
trunkfrom
add/wpcom-newsletter-mode-shell

Conversation

@davemart-in

Copy link
Copy Markdown
Contributor

Proposed changes

Add the initial WordPress.com-only Newsletter Mode page shell to jetpack-mu-wpcom.

This is the first replacement slice for #50680. It establishes the page structure and routes without adding any Newsletter Mode feature content yet.

The PR:

  • Adds a top-level Newsletter admin page backed by @wordpress/build.
  • Adds empty onboarding and stats routes.
  • Uses the shared Jetpack AdminPage chrome and standard Jetpack footer.
  • Supports manual per-site enrollment through the wpcom_newsletter_mode_enabled option.
  • Adds the WPCOM_NEWSLETTER_MODE_DISABLED emergency kill switch.
  • Restricts access to users with the publish_posts capability (author and above).
  • Initializes only for non-AJAX wp-admin requests, avoiding an enrollment-option lookup on front-end, REST, cron, and AJAX requests.
  • Relies on WordPress's add_menu_page() capability handling for menu registration while retaining an explicit capability check before loading direct page requests.
  • Adds PHP and JavaScript coverage for the shell, routes, enrollment, kill switch, and access rules.
  • Adds package-level Jest configuration that discovers Newsletter Mode feature and route tests.

No customer-facing Newsletter Mode functionality is included yet.

Related product discussion/links

Does this pull request change what data or activity we track or use?

No.

Testing instructions

Run the automated checks:

pnpm jetpack test php packages/jetpack-mu-wpcom
pnpm jetpack test js packages/jetpack-mu-wpcom
pnpm jetpack phan packages/jetpack-mu-wpcom
pnpm --filter @automattic/jetpack-mu-wpcom run typecheck
pnpm --filter @automattic/jetpack-mu-wpcom run build-production-js

To test the shell manually on a WordPress.com Simple or Atomic test site:

  1. Build and sync jetpack-mu-wpcom using your normal development workflow.
  2. Enroll the site:
wp option update wpcom_newsletter_mode_enabled 1
  1. Sign in as an administrator or author and confirm that a top-level Newsletter menu appears.

  2. Confirm these pages render the expected placeholder content inside the Jetpack page chrome:

    • wp-admin/admin.php?page=newsletter-mode-wp-admin
    • wp-admin/admin.php?page=newsletter-mode-wp-admin&p=%2Fstats
  3. Sign in as a contributor or subscriber and confirm the Newsletter menu is not available.

  4. Remove enrollment and confirm the menu disappears:

wp option delete wpcom_newsletter_mode_enabled
  1. Optionally define WPCOM_NEWSLETTER_MODE_DISABLED as true and confirm it overrides enrollment.
  2. Confirm ordinary front-end, REST, cron, and AJAX requests continue unaffected.

@github-actions

github-actions Bot commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack or WordPress.com Site Helper), and enable the add/wpcom-newsletter-mode-shell branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack add/wpcom-newsletter-mode-shell
bin/jetpack-downloader test jetpack-mu-wpcom-plugin add/wpcom-newsletter-mode-shell

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions

github-actions Bot commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

@github-actions github-actions Bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jul 31, 2026
@davemart-in davemart-in added [Status] In Progress and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jul 31, 2026
@jp-launch-control

Copy link
Copy Markdown

Code Coverage Summary

Coverage changed in 1 file.

File Coverage Δ% Δ Uncovered
projects/packages/jetpack-mu-wpcom/src/class-jetpack-mu-wpcom.php 39/450 (8.67%) -0.02% 1 ❤️‍🩹

4 files are newly checked for coverage.

File Coverage
projects/packages/jetpack-mu-wpcom/src/features/newsletter-mode/newsletter-mode.php 0/2 (0.00%) 💔
projects/packages/jetpack-mu-wpcom/src/features/newsletter-mode/class-newsletter-mode.php 22/36 (61.11%) 💚
projects/packages/jetpack-mu-wpcom/src/features/newsletter-mode/js/page-shell.test.tsx 19/19 (100.00%) 💚
projects/packages/jetpack-mu-wpcom/src/features/newsletter-mode/js/page-shell.tsx 29/29 (100.00%) 💚

Full summary · PHP report · JS report

If appropriate, add one of these labels to override the failing coverage check: Covered by non-unit tests Use to ignore the Code coverage requirement check when E2Es or other non-unit tests cover the code Coverage tests to be added later Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR I don't care about code coverage for this PR Use this label to ignore the check for insufficient code coveage.

@davemart-in davemart-in added Coverage tests to be added later Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Aug 1, 2026
@davemart-in
davemart-in requested review from enejb and simison August 1, 2026 13:34

@enejb enejb left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Took this for a spin on an Atomic test site and read through it in detail. Nice, well-scoped slice — the gating is clean, it follows the ai-launchpad pattern closely, and the PHP tests asserting capability behaviour indirectly through has_action( 'toplevel_page_…' ) are a nice touch, since that exercises add_menu_page's real capability check rather than reimplementing it.

This is the UI that I saw.

Screenshot 2026-08-06 at 3 47 25 PM

@dhasilva and @CGastrell Notice the the negative margin on the AdminPage. Is that expected?
Can we remove this everywhere?

No blockers. Everything below is a suggestion. I ran the package's PHP tests, JS tests and Phan locally against this branch — all green. (Phan flagged two PhanPluginNeverReturnFunction hits in generated build/pages/*/page.php, but one of those is the pre-existing site-setup page, so that's a build-artifact effect in my worktree, not something this PR introduces.)

I've left five inline notes. Two more that don't have a single line to hang off:

6. No PHP coverage for the init() page-request / capability gate. The suite covers enrollment, the kill switch, admin-vs-frontend, loader wiring and register_menu capability behaviour — good coverage overall. What's missing is is_page_request() and the current_user_can branch inside init(), which is exactly the code path in my inline note on class-newsletter-mode.php.

7. Testing instructions could name the carrier plugin. "Build and sync jetpack-mu-wpcom using your normal development workflow" is a bit under-specified for Atomic. The package needs to be synced via wpcomsh there. I initially synced it via mu-wpcom-plugin instead and got a site-wide fatal — Call to undefined function wpcom_site_has_feature() from 100-year-plan/enhanced-ownership.php and wpcom-global-styles/index.php — because that plugin ships the package without the wpcom platform functions its other features depend on. Nothing to do with this PR, but naming wpcomsh explicitly in step 1 would save the next reviewer the same detour.


add_action( 'admin_menu', array( __CLASS__, 'register_menu' ) );

if ( ! self::is_page_request() || ! current_user_can( self::ACCESS_CAPABILITY ) ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The capability gets evaluated twice, at two different points in the request, and the two can disagree.

This check runs at plugins_loaded (via load_wpcom_sites_features()). add_menu_page() then checks the same capability again at admin_menu — and init fires in between (wp-settings.php: plugins_loaded at L622, init at L771).

So if anything grants publish_posts through a user_has_cap / map_meta_cap filter registered on init — common in membership and role-editor plugins — the two disagree: build.php never loads here, but the menu still registers at admin_menu with the __return_empty_string fallback. The user sees "Newsletter", clicks it, and gets a completely blank page with no error.

Loading build.php early isn't the thing to change — the generated registrars hook wp_default_scripts / wp_default_styles, so it genuinely has to be required before WP_Scripts is first instantiated. It's the early capability check that could go:

if ( ! self::is_page_request() ) {
	return;
}

add_menu_page() already gates access, which is what ai-launchpad relies on. Registering assets for a user who can't reach the page is harmless — nothing enqueues them. It also avoids resolving and caching the current user before init as a side effect.

min-block-size: 12rem;
padding-block: 2rem;
padding-inline: 2rem;
background: var(--wpds-color-background-surface-neutral);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Worth double-checking this token name. --wpds-color-background-surface-neutral doesn't appear anywhere else in the monorepo — every other use I could find is the -strong variant, and in plugins/jetpack it always carries a fallback:

background: var(--wpds-color-background-surface-neutral-strong, #fff);

If the token isn't defined in the admin context, the section just renders with no background. A fallback would make that safe either way.

"test": "pnpm run test:node && pnpm run test:jest",
"test:jest": "jest",
"test:node": "node --test --test-reporter spec src/features/write/test/undo-history.test.mjs",
"test-coverage": "c8 --report-dir=\"$COVERAGE_DIR\" --temp-directory=\"$ARTIFACTS_DIR/v8\" pnpm run test",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

test now runs test:node && test:jest, but test-coverage is still the c8 wrapper — and c8 doesn't meaningfully instrument Jest, so the new .tsx files will produce no coverage data.

The other Jest-based packages in the monorepo use Jest's own coverage instead:

"test-coverage": "pnpm run test --coverage"

That's my best guess at why the "Code coverage requirement" check is currently red, though I didn't run the coverage job to confirm. Since the Coverage tests to be added later label is on, this may well be a deliberate defer — feel free to ignore if so.

Comment on lines +5 to +8
testMatch: [
'<rootDir>/src/features/newsletter-mode/**/*.test.[jt]s?(x)',
'<rootDir>/routes/newsletter-mode-*/**/*.test.[jt]s?(x)',
],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since testMatch is an allowlist, only newsletter-mode paths run. Any future *.test.tsx added elsewhere in jetpack-mu-wpcom silently won't execute — no failure, just silence, which is an easy one to lose a day to.

A short comment explaining the deliberate scoping would be enough, or broaden the match and exclude what you don't want.

@@ -0,0 +1,19 @@
/// <reference types="jest" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor: the filename doesn't quite describe what's under test — this renders the two route stages and never exercises NewsletterModePageShell directly. Something like routes.test.tsx would match, or keep the name and add an actual page-shell unit test alongside.

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

This PR has been marked as stale. This happened because:

  • It has been inactive for the past 3 months.
  • It hasn't been labeled `[Pri] BLOCKER`, `[Pri] High`, `[Status] Keep Open`, etc.

If this PR is still useful, please do a [trunk merge or rebase](https://github.com/Automattic/jetpack/blob/trunk/docs/git-workflow.md#keeping-your-branch-up-to-date) and otherwise make sure it's up to date and has clear testing instructions. You may also want to ping possible reviewers in case they've forgotten about it. Please close this PR if you think it's not valid anymore — if you do, please add a brief explanation.

If the PR is not updated (or at least commented on) in another month, it will be automatically closed.

@davemart-in

Copy link
Copy Markdown
Contributor Author

Closing this out for now.

@davemart-in davemart-in closed this Oct 8, 2026
@github-actions github-actions Bot removed [Status] Needs Review This PR is ready for review. [Status] Stale labels Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Coverage tests to be added later Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR Docs [mu wpcom Feature] Newsletter Mode [Package] Jetpack mu wpcom WordPress.com Features [Tests] Includes Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants