wp-calypso icon indicating copy to clipboard operation
wp-calypso copied to clipboard

Add launchpad_screen check in post-publish redirects

Open daledupreez opened this issue 1 year ago • 8 comments

Fixes https://github.com/Automattic/wp-calypso/issues/87416

Proposed Changes

  • This PR adds some logic to heavily limit when we redirect to the fullscreen launchpad from the editor. We will now redirect only when all of the following are true, with new restrictions in bold:
    • the site has either the design-first or the start-writing intent (which are created from the /setup/design-first and /setup/start-writing flows, respectively)
    • the URL for the editor includes start-writing=true as a URL parameter
    • the user is editing a post (and not a template, page, navigation, or any other post type, including custom post types)
    • the launchpad_screen option is specifically set to 'full'

Prior to this change, we were redirecting in a number of situations where the redirect was unexpected, including the following:

  • The user was editing the index template for their site via the Site Editor (without starting from the original flow)
  • The user had skipped the launchpad, so the launchpad_screen option was set to 'skipped'

Why are these changes being made?

  • We've seen multiple reports where users continually and unexpectedly get redirected to the fullscreen launchpad. Even after trying to disable the launchpad, however, they are still being redirected. This patch ensures that "normal" approaches to marking the launchpad as disabled or skipped will be honoured, as well as preventing redirects when users are not editing posts specifically from the original onboarding flow.

Testing Instructions

Get the site set up to reproduce the issue

For these steps, use the standard Calypso UI, not this code.

  • Create a new site via https://wordpress.com/setup/start-writing/
  • Work through the flow to publish your first post
  • After publishing, you should be taken to the fullscreen launchpad.
    • You may see a congratulatory modal instead -- if so, pick "Next steps" to get taken to the launchpad
  • On the fullscreen launchpad, select the "Skip for now" option in the top right corner to take you to Customer Home
  • Navigate to the Site Editor via Appearance => Editor
  • Make one or more changes to the template and save them.
  • Reload the site editor.
  • You should be redirected to the launchpad screen and then back to Customer Home.
  • Keep this tab open -- we'll use it again!

Test the launchpad_screen fix

  • Follow the instructions in this comment to apply the changes from this PR to your WordPress.com development sandbox
  • Ensure that the widgets.wp.com host is pointed at your sandbox
  • Ensure that your browser is configured not to cache content - this is easiest if you open up your browsers' development console. (The cache buster for the resources may not have been bumped, so it's simplest to make sure you're loading the files every time.)
  • Reload the site editor.
  • Verify that you are not redirected to the fullscreen launchpad.
  • Keep this tab open.
  • Add ?start-writing=true to the URL, and reload the site editor.
  • Verify that you are not redirected to the fullscreen launchpad.

Test the redirect applies only to posts

  • Use a shell command to reset the launchpad_screen option to full for your site:
> switch_to_blog( 1234567890 ); // replace with your site's blog ID
> update_option( 'launchpad_screen', 'full' );
  • Reload the site editor above, keeping the start-writing URL parameter as true.
  • Verify that you are not redirected to the fullscreen launchpad.

Test the post editor

  • Navigate to /wp-admin/post-new.php?start-writing=true
  • Add some content and then publish the post
  • Verify that you are redirected to the fullscreen launchpad

Pre-merge Checklist

  • [x] Has the general commit checklist been followed? (PCYsg-hS-p2)
  • [x] Have you written new tests for your changes?
  • [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • [x] Have you checked for TypeScript, React or other console errors?
  • [N/A] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • [N/A] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
  • [N/A] For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

daledupreez avatar May 14 '24 18:05 daledupreez

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot avatar May 14 '24 18:05 matticbot

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • blaze-dashboard
  • command-palette-wp-admin
  • help-center
  • notifications
  • odyssey-stats
  • whats-new
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug try/add-launchpad-screen-check-for-post-publish-redirects on your sandbox.

matticbot avatar May 14 '24 18:05 matticbot

Tested and everything worked well up until publishing a new post in the post editor, where I get sent back and forth in a loop between Home and the fullscreen Launchpad:

https://github.com/user-attachments/assets/cead0d5e-b32e-4c0b-8c0b-9c6bce0ce698

sixhours avatar Aug 27 '24 16:08 sixhours

I can't reproduce the broken behavior at all.

https://github.com/user-attachments/assets/f8dcb4ef-ef19-479c-9c5f-498de4f27091

markbiek avatar Aug 27 '24 17:08 markbiek

I can't reproduce the broken behavior at all.

@markbiek I couldn't reproduce the original brokenness either until I published a new page from within the site editor (went to Pages -> add new, I think)

sixhours avatar Aug 27 '24 19:08 sixhours

I couldn't reproduce the original brokenness either until I published a new page from within the site editor (went to Pages -> add new, I think)

I was able to reproduce the issue by adding a new page from the site editor. Thanks!

markbiek avatar Aug 27 '24 19:08 markbiek

I managed to reproduce the issue @sixhours encountered, and I think I tracked it back to the seemingly innocuous line of code on line 40 below: https://github.com/Automattic/wp-calypso/blob/1112c9618ed8a8d17a8dea97e8f55678ab8c458c/client/landing/stepper/declarative-flow/internals/steps-repository/launchpad/index.tsx#L40-L46

What I am seeing is that our first render is setting launchpadKey to the site slug, because we haven't fetched the site yet and don't have a site ID. However, once we receive the site object, launchpadKey is now getting set to site.ID, which means that when we call useLaunchpad(), we're now using the site ID as a query key, and our first call uses a cached value for that site ID, which in this case is stale, and still has the skipped value for the launchpad_screen option.

I think this is a pre-existing bug that my testing instructions uncovered, so I don't think we need to address that problem in this PR. cc @markbiek

daledupreez avatar Aug 28 '24 12:08 daledupreez

Noting that I deployed this change to production in D159963-code.

daledupreez avatar Aug 29 '24 15:08 daledupreez