Automattic
Untangle: Add Subscribers to Jetpack Cloud (v2)
Related to https://github.com/Automattic/dotcom-forge/issues/5445
Because of a bad merge, this is the follow-up of https://github.com/Automattic/wp-calypso/pull/87615
Proposed Changes
Add Subscribers page to Jetpack Cloud.
TO-DO
- [x] Import Endpoint responds
Invalid(Related to fbhepr%2Skers%2Sjcpbz%2Sjc%2Qpbagrag%2Serfg%2Qncv%2Qcyhtvaf%2Sraqcbvagf%2Sfhofpevoref.cuc%3Se%3Qpnq3o86s%23360-og ) - [ ] Check Migrate from other WordPress sites.
Testing Instructions
- Run yarn start-jetpack-cloud
- Go to http://jetpack.cloud.localhost:3000/
- Go to /subscribers
- Check if everything is working (except TO-DOs items)
- Test it on Jetpack Self hosted & Atomic sites.
Jetpack Cloud live (direct link)
|
|
https://calypso.live?image=registry.a8c.com/calypso/app:build-99288&env=jetpack |
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This PR modifies the release build for the following Calypso Apps:
For info about this notification, see here: PCYsg-OT6-p2
- editing-toolkit
- odyssey-stats
- wpcom-block-editor
To test WordPress.com changes, run install-plugin.sh $pluginSlug add/jetpack-cloud-subscribers-v2 on your sandbox.
Sharing testing notes:
Atomic sites
The subscribers screen mainly works
🐛 There is a bug when there aren't any subscribers because the Launchpad component is not working on Atomic and Jetpack sites. This issue exists in production too. It was introduced in https://github.com/Automattic/wp-calypso/pull/83996
| EXPECTED | RESULT |
|---|---|
The import modal works
The request is POST /subscribers/import.
However, the email that the notice says I should receive after the import wasn't delivered.
The migration tool doesn't work
The request POST /migrate returns 405 "Not authorized" because the method is not allowed.
Jetpack sites
The same results as for Atomic, except that the screen shown when there aren't subscribers is different:
I'll continue investigating to fix these issues. cc: @ebinnion @lsl @taipeicoder
I have added some changes to this PR:
- [x] Fixed the migration feature using
calypso/lib/wpfor the request so the user gets authenticated.- Tested on Calypso green and blue with Atomic, Simple, and Jetpack sites.
- Verified that an email is sent when the migration is completed.
- [x] Updated the icon. Asked Jetpack designers in p1708701853576159-slack-C0D96691V
TODOs:
- [x] Investigate why the Launchpad component, shown when there aren't subscribers, doesn't work on some Atomic sites.
| Atomic 1 | Atomic 2 |
|---|---|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:
App Entrypoints (~322 bytes added 📈 [gzipped])
name parsed_size gzip_size
entry-stepper +2417 B (+0.1%) +165 B (+0.0%)
entry-main +2417 B (+0.1%) +151 B (+0.0%)
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.
Sections (~732 bytes added 📈 [gzipped])
name parsed_size gzip_size
jetpack-cloud +1101 B (+0.3%) +221 B (+0.2%)
subscribers +213 B (+0.0%) +97 B (+0.0%)
people +62 B (+0.0%) +24 B (+0.0%)
write-flow +30 B (+0.0%) +15 B (+0.0%)
videopress-flow +30 B (+0.0%) +12 B (+0.0%)
update-design-flow +30 B (+0.0%) +5 B (+0.0%)
start-writing-flow +30 B (+0.2%) +22 B (+0.4%)
sites-dashboard +30 B (+0.0%) +18 B (+0.0%)
settings-writing +30 B (+0.0%) +7 B (+0.0%)
settings-security +30 B (+0.0%) +7 B (+0.0%)
settings-reading +30 B (+0.0%) +2 B (+0.0%)
settings-podcast +30 B (+0.0%) +9 B (+0.0%)
settings-performance +30 B (+0.0%) +7 B (+0.0%)
settings-newsletter +30 B (+0.0%) +7 B (+0.0%)
settings-discussion +30 B (+0.0%) +7 B (+0.0%)
settings +30 B (+0.0%) +7 B (+0.0%)
sensei-flow +30 B (+0.0%) +17 B (+0.0%)
newsletter-post-setup-flow +30 B (+0.0%) +15 B (+0.1%)
newsletter-flow +30 B (+0.2%) +13 B (+0.3%)
marketing +30 B (+0.0%) +7 B (+0.0%)
link-in-bio-tld-flow +30 B (+0.0%) +17 B (+0.0%)
link-in-bio-post-setup-flow +30 B (+0.0%) +15 B (+0.1%)
link-in-bio-flow +30 B (+0.2%) +15 B (+0.3%)
import-hosted-site-flow +30 B (+0.0%) +18 B (+0.0%)
import-flow +30 B (+0.0%) +18 B (+0.0%)
hundred-year-plan +30 B (+0.1%) +12 B (+0.1%)
hosting +30 B (+0.0%) +6 B (+0.0%)
home +30 B (+0.0%) +24 B (+0.0%)
free-flow +30 B (+0.1%) +12 B (+0.2%)
earn +30 B (+0.0%) +20 B (+0.0%)
design-first-flow +30 B (+0.2%) +23 B (+0.4%)
copy-site-flow +30 B (+0.0%) -1 B (-0.0%)
checkout +30 B (+0.0%) +6 B (+0.0%)
build-flow +30 B (+0.0%) +14 B (+0.0%)
assembler-first-flow +30 B (+0.1%) +25 B (+0.2%)
ai-assembler-flow +30 B (+0.1%) +26 B (+0.2%)
scan +26 B (+0.0%) +14 B (+0.0%)
jetpack-search +26 B (+0.0%) +14 B (+0.0%)
jetpack-cloud-plugin-management +26 B (+0.0%) +16 B (+0.0%)
jetpack-cloud-partner-portal +26 B (+0.0%) +14 B (+0.0%)
jetpack-cloud-overview +26 B (+0.0%) +18 B (+0.0%)
jetpack-cloud-agency-sites-v2 +26 B (+0.0%) +14 B (+0.0%)
backup +26 B (+0.0%) +14 B (+0.0%)
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.
Async-loaded Components (~365 bytes added 📈 [gzipped])
name parsed_size gzip_size
async-load-calypso-jetpack-cloud-sections-sidebar-navigation-manage-selected-... +1101 B (+1.0%) +225 B (+0.6%)
async-load-calypso-my-sites-stats-pages-subscribers +181 B (+0.1%) +77 B (+0.2%)
async-load-masterbar-launchpad-navigator +30 B (+0.0%) +21 B (+0.1%)
async-load-design +30 B (+0.0%) +18 B (+0.0%)
async-load-automattic-help-center +30 B (+0.0%) +24 B (+0.0%)
React components that are loaded lazily, when a certain part of UI is displayed for the first time.
Legend
What is parsed and gzip size?
Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.
Generated by performance advisor bot at iscalypsofastyet.com.
@Automattic/apex @daledupreez do you know what's up with the launchpad component above https://github.com/Automattic/wp-calypso/pull/87684#issuecomment-1961660982?
Hi!
I tested it and found some details. Let me know if I should create issues for them:
JN sites Launchpad behaves differently than the Atomic ones. The correct design is the one from Atomic, including the actions. We shouldn't open Support Pages here, users will end up in a rabbit hole.
The heading font size isn't right, here are all the design specs (spacings and typography): S4xGQTEkLhYdUvcIdn2Cfr-fi-67_16943.
Turn visitors into subscribers
In JN it opens support pages and on Atomic, a modal.
Add the subscribe block to your site
JN opens support pages, Atomic doesn't work. It should open the site editor, with the Subscribe block: /site-editor/jetpackgoku.wpcomstaging.com?canvas=edit&help-center=subscribe-block
Share your site
JN opens a support page and Atomic the Share modal.
@simison and/or @miksansegundo, could you check whether the problematic back end Atomic site is running a custom/dev version of wpcomsh? That's the most likely cause of the issue, as the Launchpad REST API validates that the checklist slug exists locally:
https://github.com/Automattic/jetpack/blob/253e44209c31006822c2c2d396ad2ebbb4eb165b/projects/packages/jetpack-mu-wpcom/src/features/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-launchpad.php#L42
Side note: jetpack-mu-wpcom is loaded as a Composer dependency of wpcomsh on Atomic sites.
could you check whether the problematic back end Atomic site is running a custom/dev version of wpcomsh?
@daledupreez, that was it. That site is a testing site that was not using the latest version of wpcomsh. Thanks for spotting it.
There are still issues only on Calypso green because the requests made from Launchpad are not authenticated. The error is a 401 rest_forbidden.
Launchpad requests work when we use wpcom.req instead of wpcomRequest because the user needs to be authenticated via Authorization: Bearer rather than via Authorization: X-WPCOOKIE.
| BEFORE | AFTER |
|---|---|
The commit https://github.com/Automattic/wp-calypso/pull/87684/commits/1c9f6f8da8d2ffa46c1c49445b803afe5654b700 is an example, that fixes the request to fetch a Launchpad checklist. However, it's required also to disable the linter with /* eslint-disable no-restricted-imports */ because the import for calypso/lib/wp is restricted there.
We'll have to update all the requests related to the Launchpad for Subscribers to ensure the user is authenticated.
@daledupreez @simison, what are your thoughts on refactoring Launchpad partially to use wpcom.req?
The commit https://github.com/Automattic/wp-calypso/pull/87684/commits/c12495d62f4eece013a368472bbcab4289b4848c is another example of how we can fix the request authentication issue.
It fixes the issue with the emails not being sent when a subscriber is imported via the following modal:
Users see this notice:
And the email is received:
JN sites Launchpad behaves differently than the Atomic ones. The correct design is the one from Atomic, including the actions. We shouldn't open Support Pages here, users will end up in a rabbit hole.
Noting that currently, in production, we show the Subscribers Launchpad only to Atomic sites. For Jetpack sites, we show the empty-list-view component.
Production
| Atomic | Jetpack |
|---|---|
For Jetpack sites, we show the empty-list-view component.
Can we do the same for Atomic sites on jetpack cloud? Leaving the launchpad behavior in dotcom default sites only?
For Jetpack sites, we show the empty-list-view component.
Can we do the same for Atomic sites on jetpack cloud? Leaving the launchpad behavior in dotcom default sites only?
Is there any functionality loss? If it is only guidance, seems fine to me.
JN opens support pages, Atomic doesn't work. It should open the site editor, with the Subscribe block: /site-editor/jetpackgoku.wpcomstaging.com?canvas=edit&help-center=subscribe-block
The task "Add the subscribe block to your site" opens the editor with the param help-center=subscribe-block to show the block's help center. See https://github.com/Automattic/wp-calypso/pull/84609
The task link is a Calypso path to open the editor in wordpress.com, which doesn't exist within the Jetpack Manage domain (cloud.jetpack.com). See launchpad-task-definitions.php#L575
I intend to find a way to open the link in wordpress.com dynamically by checking the browser location.host because I'm unsure if it is a good idea to use an absolute URL in the task definition. cc: @daledupreez
Is there any functionality loss? If it is only guidance, seems fine to me.
Yes, there is functionality loss because the items in empty-list-view are only links to support docs while the other component has actions.
The support docs are:
https://wordpress.com/support/wordpress-editor/blocks/subscribe-block/ https://wordpress.com/support/import-subscribers-to-a-newsletter/ https://wordpress.com/support/category/grow-your-audience/
Is there any functionality loss? If it is only guidance, seems fine to me.
Let's not please remove the empty state launchpad for Atomic sites. It's working on Calypso side just fine:
What's the problem exactly for it not working with Jetpack Cloud?
The same launchpad is also used for empty subscriber stats (also for Atomic sites):
For self-hosted Jetpack sites fine to keep as-is today without launchpad.
I intend to find a way to open the link in wordpress.com dynamically by checking the browser location.host because I'm unsure if it is a good idea to use an absolute URL in the task definition.
Yep, linking to site editor in .com is fine (assuming it'll end up in wp-admin instead of iframed eventually). You can also use selectors to pull the site editor URL, example:
https://github.com/Automattic/wp-calypso/blob/409a670741634cd9298cc31f46bc49413e93ca23/client/my-sites/site-settings/settings-newsletter/SubscribeModalSetting.tsx#L27-L30
We'll be replacing the task to a new flow soonish anyway as the current isn't great, but not fast enough to support this work.
Just did a quick test to see if Launchpad would work with Jetpack sites and found that the API returns a 404 rest_no_route.
https://github.com/Automattic/wp-calypso/assets/1881481/100b1f5d-7b02-445e-965d-4ed0301061a3
Yep, linking to site editor in .com is fine (assuming it'll end up in wp-admin instead of iframed eventually).
Initially, I solved that in https://github.com/Automattic/wp-calypso/pull/87684/commits/a53a5dc5a09bf83a0946bd0abb62c1bc78ddaa2f but then reverted it because that URL is being updated in Jetpack via https://github.com/Automattic/jetpack/pull/36014
For self-hosted Jetpack sites fine to keep as-is today without launchpad.
👍
What's the problem exactly for it not working with Jetpack Cloud?
At the moment, all issues seem solved. The three tasks are working:
https://github.com/Automattic/wp-calypso/assets/1881481/9df265fe-d452-4f50-b9d3-f17cbd5a015a
Heads up, these cards links look to be broken at the bottom of the page:
Stats link specifically should probably link a bit different places depending on is it WP.com or Jetpack site, and state of calypso-untangling for WP.com?
Importing is throwing an error for both Jetpack and WP.com sites:
The importing itself did work but error is a problem.
Also seeing weird things happen with site switcher:
https://github.com/Automattic/wp-calypso/assets/87168/ef17901e-7554-4862-aade-00c9d3373fdc
Also seeing weird things happen with site switcher:
I noticed this also happens on Production and with all menus 🤔
Also seeing weird things happen with site switcher:
It seems to be the expected behavior as we set the base path to /landing for Jetpack Cloud and site selector switches to that path and you switch the site.
