Wp-calypso: Jetpack Checklist: Plugin autoupdate tour wrongly shows success

[tyxla]

It appears that the Plugin autoupdate guided tour will sometimes display the success message before the user has actually enabled the autoupdate toggle for Jetpack.

Steps to reproduce

1. Pick a freshly connected Jetpack site.
2. Go to https://wpcalypso.wordpress.com/plans/my-plan/:site where :site is the site slug
3. Click on the "Do it" button of the "Automatic Plugjn Updates" task.

What I expected

• To see the first message that nudges me to activate the autoupdate toggle
• Then to see the success message after I've enabled the toggle

What happened instead

I _sometimes_ see a success message, although I haven't yet activated the autoupdate toggle.

Screenshot / Video

Context / Source

This defect was discovered by @jeffgolenski while testing the security checklist: https://cloudup.com/cqK-IkaWi3O

manual-testing

[joanrho]

Hey Luna! 👋 Just tested again and confirmed this is still a 🐞.

[ockham]

Relevant files:

• https://github.com/Automattic/wp-calypso/blob/928d21293635d49a159f425d7febcfa09c8d396b/client/layout/guided-tours/tours/jetpack-plugin-updates-tour/index.js
• https://github.com/Automattic/wp-calypso/blob/928d21293635d49a159f425d7febcfa09c8d396b/client/layout/guided-tours/component.jsx

The latter fires a number of Tracks events that can be used for debugging by entering

localStorage.setItem( 'debug', 'calypso:analytics:tracks' )

into the browser console and reloading prior to testing that flow. This gives

[ockham]

Seems like when doesn't behave as expected. I'd expect the following to not show any Guided Tour at all, but instead, it also shows the 'Success' step:

diff --git a/client/layout/guided-tours/tours/jetpack-plugin-updates-tour/index.js b/client/layout/guided-tours/tours/jetpack-plugin-updates-tour/index.js
index ae4ea0daa8..c69fd38b20 100644
--- a/client/layout/guided-tours/tours/jetpack-plugin-updates-tour/index.js
+++ b/client/layout/guided-tours/tours/jetpack-plugin-updates-tour/index.js
@@ -26,12 +26,7 @@ export const JetpackPluginUpdatesTour = makeTour(
        <Tour { ...meta }>
                <Step
                        name="init"
-                       when={ state => {
-                               const site = getSelectedSite( state );
-                               const res =
-                                       ! PluginsStore.isFetchingSite( site ) && !! PluginsStore.getSitePlugin( site, 'jetpack' );
-                               return res;
-                       } }
+                       when={ () => false }
                        target=".plugin-item-jetpack .form-toggle__switch"
                        arrow="top-left"
                        placement="below"

If I change that return value to true, things _almost_ work: I get the first step (asking me to enable autoupdates for the Jetpack plugin) -- but only after scrolling.

[ockham]

Re-reading https://github.com/Automattic/wp-calypso/blob/master/client/layout/guided-tours/docs/API.md, we might've just added the when prop erroneously to Step rather than Tour :thinking:

For Tour:

when (function, optional): This is a Redux selector function. Use this to define conditions for the tour to start. Can be overridden by adding a tour query argument to the URL like so: ?tour=tourName, in which case the tour will be triggered even if when would evaluate to false. This is useful for sending along a tour via email or chat. On the other hand, the framework will try to not trigger a tour multiple times (see toursSeen in ARCHITECTURE.md). Note: you can reset the tours history by adding ?tour=reset to the URL.

For Step:

when: (function, optional) This is a Redux selector that can prevent a step from showing when it evaluates to false. When using when you should also set the next prop to tell Guided Tours the name of the step it should skip to. If you omit this prop, step will be rendered as expected. Example usage would be to show a certain step only for non-mobile environments: when={ not( isMobile ) }

So it sounds like this behavior is normal, and the fix is to move the when prop to Tour. I'll file a fix.

[simison]

@ockham I take this is now fixed?

[simison]

So looks like a combination of https://github.com/Automattic/wp-calypso/pull/33258 and https://github.com/Automattic/wp-calypso/pull/33359 has solved this. Closing :thumbsup:

Feel free to re-open (or open new issue) if there still are pending issues.