Site-kit-wp: Only display Reset link in setup flow when relevant

Created on 31 Oct 2019 · 5Comments · Source: google/site-kit-wp

Bug Description

753 added a Reset link displayed in the setup screen. However, this is permanently shown there now, even when you access the page for the first time. Furthermore, there's no confirmation message after resetting, which can especially be confusing as the link to reset remains available afterwards (both problems outlined in https://github.com/google/site-kit-wp/issues/753#issuecomment-548581164). Let's fix this.

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

Only display Reset link on proxy setup screen when there is data to reset. We can do a basic check like:
- If site ID and/or site secret are present in the DB, we can consider the plugin "resettable". This is the first data that typically ever gets set, so that is a good indicator.
Show a confirmation message after successful reset:
- Message: "Site Kit by Google was successfully reset."

Implementation Brief

Define and expose whether Site Kit is in a resettable state or not

Add a new isResettable to localized setup data in:
\Google\Site_Kit\Core\Authentication\Authentication::inline_js_setup_data
- Boolean based on presence of non-empty credentials option for now (could be filterable or more robust in the future)
- Would be nice if Credentials exposed a new method to check if its option is set, but not required here.
  
  _Credentials:has no longer works for this purpose since it does not differentiate between the source of the credentials (option/filter)_
Use this new data to conditionally display the reset button on the setup screen
- Can likely add this to the ResetButton - if Site Kit is not resettable, don't render it

Display Feedback on Reset

Add a new query parameter to the URL the user is sent to after resetting in https://github.com/google/site-kit-wp/blob/102e534484f8b9735621c459eff72586cfde286b/assets/js/components/reset-button.js#L63

getSiteKitAdminURL( 'googlesitekit-splash', { notification: 'reset_success' } )

Conditionally render a success <Notification> in setup-proxy.js based on the presence and value of this query parameter
- For copy and behavior see ACs

Changelog entry

Only display the Reset button on the setup screen if there actually is something to reset, and provide a feedback notice.

P0 Bug

Source

felixarntz

👍1

All 5 comments

@aaemnnosttv

Add a new hasCredentialsOption to localized setup data in

I think we should keep this more generic as hasDataToReset or hasResettableData.

Also note that the IB doesn't yet cover the feedback notice.

felixarntz on 4 Nov 2019

👍1

Thanks @felixarntz - I've updated the IB to include both items you mentioned.

One thing to highlight here that's missing in the AC are the details of the notice (exactly what it should say, etc) which I added to the IB as something which is still needed as well. (cc: @marrrmarrr)