Site-kit-wp: Improve messaging to connect versus configure Site Kit for site admins who have not originally done the Site Kit configuration

Created on 26 Jun 2020  ยท  13Comments  ยท  Source: google/site-kit-wp

Bug Description

I'm one of multiple admins on a Newspack site using Site Kit. The other admin has originally configured Site Kit and connected it to the Newspack site. When I now go to the Site Kit UI in the backend of WordPress, the messaging seems to indicate that Site Kit has not yet been set up โ€“ when in fact what I need to do is connect with my Google account to the site (this doesn't feel very intuitive and the messaging doesn't indicate what needs to happen).

The disconnects and confusion I experience:
A. The first time I (a site admin who was not the person who configured Site Kit) go to Site Kit, I see messaging that says "Configure Site Kit," and I become confused and worried that the site was never set up properly
B. I am afraid to click the button "Sign in with Google" because I fear I will connect the site with my personal Google Analytics account, not the official one for the publication
C) Even after I went through the flow to connect, I am confused because I can't see Analytics, which is because I hadn't been added to the publication's GA โ€“ this however makes sense.

Steps to reproduce

  1. Go to Site Kit UI screen in the WordPress backend
  2. I land on the screen "Sign in with Google to configure Site Kit" (see below) โ€“ I'm worried Site Kit has never been configured
  3. Click on "Sign in with Google"
  4. I land on the screen "Welcome to Site Kit! Let's get you set up" (see below) โ€“ I'm worried I'm connecting the site to my personal Google Analytics account and not the publication's
  5. Next screen has me "confirm my choices" (see below), which makes me wonder how this data is shared with the publication site
  6. After setup is complete and I arrive back at the WordPress site I see a "Data error in Analytics" and some language around not having access โ€“ which is because I have not yet been added to the publication's Google analytics account. This makes sense to me since I'm aware of that.

Screenshots

Step 1:
1
Step 3:
2
Step 4:
3
Step 5:
Screenshot 2020-06-24 12 15 02

Conclusion

To improve the user experience and reduce confusion, I would recommend to adjust the messaging on some of the UI screens to clarify that Site Kit has been set up and configured, and that the next step is to connect via my Google account, rather than being met with a "configure Site Kit" messaging.


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

Acceptance criteria

  • The core/site JS data store should expose (via core/site/data/connection API route) a hasConnectedAdministrators: Returns the boolean value of a new hasConnectedAdministrators in API response.

    • Should run a user query whether there are any administrator users who have a Site Kit access token.

  • The logic for the setup (splash) screen to determine whether the current user isSecondaryAdmin should be updated (currently just looks at isResettable which is not really accurate):

    • It should be hasConnectedAdministrators.

  • The copy for secondary administrators in the Site Kit setup (splash) screen should be updated:

    • Heading: _Connect your Google account to Site Kit_

    • Text: _Site Kit has already been configured by another admin of this site. To use Site Kit as well, sign in with your Google account which has access to Google services for this site (e.g. Google Analytics). Once you complete the 3 setup steps, you'll see stats from all already activated Google products._

  • For secondary administrators, the "Reset" button should not be displayed.
  • The SetupUsingProxy JS component should use datastores instead of _googlesitekitLegacyData, except for errorMessage and canSetup.
    These two cases should be annotated with TODO comments to migrate in the future.

Auxiliary datastore changes

  • The core/site store should have new selectors based on info from _googlesitekitBaseData:

    • getProxySetupURL

    • getProxyPermissionsURL

  • The three proxy fields usingProxy, proxySetupURL, and proxyPermissionsURL should always be present in _googlesitekitBaseData. If not using the proxy, they should be false / '' / '' respectively.

Implementation Brief

  • Add a new HasConnectedAdmins Setting class for handling the value used for hasConnectedAdmins

    • Manages a new option: googlesitekit_has_connected_admins, a boolean value

    • When HasConnectedAdmins->get() is called, if the option does not exist, it should query the users to populate it and set the value for next time

      (similar to how a transient might be used)

    • In it's register method, it should register a listener for user meta deletion



      • If the deleted meta's meta_key is a meta key for OAuth_Client::OPTION_ACCESS_TOKEN, call HasConnectedAdmins->delete()


        This will cause the value to be refreshed when get is called next



    • See below for a starting point for this class

    • In the method which queries users:



      • See Migration_1_3_0::get_authenticated_users for an example of a similar query


      • Query users with role administrator


      • The query should be limited to 1 user


      • If a user is found, return 1 otherwise 0



  • Instantiate HasConnectedAdmins as a property in Authentication, similar to Profile and FirstAdmin

    • Register it in Authentication::register()

  • Update the REST Route definition in Authentication for core/site/data/connection to include a hasConnectedAdmins key in the response

    • The value for the new key can be sourced from $this->has_connected_admins->get()


ConnectedAdmins starting point

<?php

use Google\Site_Kit\Core\Authentication\Clients\OAuth_Client;
use Google\Site_Kit\Core\Storage\Options_Interface;
use Google\Site_Kit\Core\Storage\Setting;
use Google\Site_Kit\Core\Storage\User_Options_Interface;

class HasConnectedAdmins extends Setting {
    const OPTION = 'googlesitekit_has_connected_admins';

    protected $user_options;

    /**
    * ConnectedAdmins constructor.
    *
    * @param Options_Interface $options
    * @param User_Options_Interface $user_options
    */
    public function __construct( Options_Interface $options, User_Options_Interface $user_options ) {
        parent::__construct( $options );
        $this->user_options = $user_options;
    }

    public function register() {
        parent::register();

        $access_token_meta_key = $this->user_options->get_meta_key( OAuth_Client::OPTION_ACCESS_TOKEN );

        add_action(
            'deleted_user_meta',
            function ( $meta_ids, $object_id, $meta_key ) use ( $access_token_meta_key ) {
                if ( $meta_key === $access_token_meta_key ) {
                    $this->delete();
                }
            },
            10,
            3
        );
    }

    public function get() {
        // If the option doesn't exist, query the fresh value, set it and return it.
        if ( ! $this->has() ) {
            return $this->query_connected_admins();
        }

        return (bool) parent::get();
    }

    private function query_connected_admins() {
        $has_connected_admins = /* do query ...*/ true;
        $this->set( $has_connected_admins );
        return $has_connected_admins;
    }
}

core/site

  • Add a new hasConnectedAdmins registry selector which returns the value from getConnection
  • Should we add a selector for proxySetupURL to core/site since this is also in _googlesitekitBaseData and used in SetupUsingProxy?

assets/js/components/setup/setup-proxy.js

  • Update SetupUsingProxy to be wrapped with withSelect when exported, passing the following selected data as props:

    • hasConnectedAdmins: select( core/site ).hasConnectedAdmins()

    • isConnected: select( core/site ).isConnected()

    • isResettable: select( core/site ).isResettable()

    • siteURL: select( core/site ).referenceSiteURL()

    • proxySetupURL: select( core/site ).proxySetupURL() (see question above)

  • Annotate use of legacy globals for errorMessage and canSetup with TODO comments for future refactoring
    _All other use of legacy globals should be refactored with datastore data via withSelect and props above_
  • Update conditional rendering of ResetButton to also render if hasConnectedAdmins

QA Brief

  • Install, activate and configure Google Site Kit plugin
  • Create a new admin user and switch to that admin
  • Go to the Site Kit dashboard
  • You should see the following:

    • Heading: _Connect your Google account to Site Kit_

    • Text: _Site Kit has already been configured by another admin of this site. To use Site Kit as well, sign in with your Google account which has access to Google services for this site (e.g. Google Analytics). Once you complete the 3 setup steps, you'll see stats from all activated Google products._

Changelog entry

  • Clarify messaging on initial setup screen for secondary users who need to connect to Site Kit.
P0 Enhancement

All 13 comments

Noting the language of "Let's verify your ownership of xyz.com" might be confusing to a second admin who doesn't "own" the domain at all. What we mean here right now is is "verify your Admin (or "write"?) access to xyz.com".

Lots of good points here.

  • Definitely makes sense to rephrase the initial screen (step 1 here) to clarify what's happening is actually just connecting the second admin's account to Site Kit, which has already been set up.
  • We don't control what's displayed on the OAuth screens (step 4 in the original description), so that copy will remain as-is.
  • Regarding site verification (step 3) -- @adamsilverstein AFAIK we will still place a verification file or token for every admin, regardless of whether they're the second one, so they can get access to SC stats. Can you confirm if that's the case?

Wording change for the initial screen that kicks off the entire process for second admins:
Heading: _Connect your Google account to Site Kit_
Text: _Site Kit has already been configured by another admin of this site. To use Site Kit as well, sign in with your Google account which has access to Google services for this site (e.g. Google Analytics). Once you complete the 3 setup steps, you'll see stats from all already activated Google products._

  • Replace the "reset" functionality with "disconnect" functionality (only affects that specific admin).
  • Don't display "disconnect Site Kit" link for any user who sees this screen for the first time.
    cc @felixarntz we discussed the "reset button" point ^ in detail last week -- let me know if you have any additional comments on this one.

@marrrmarrr Yes, we'll need to verify ownership for all users that use Site Kit, with a verification token as necessary.

I've provided ACs based on the wording you outlined. One small thing is we should probably say "another user" rather than "another admin" - while the latter is right now more accurate, we should make the wording account for that non-admins would be able to use the plugin too (just without administrative permissions). While that is not the case right now, most parts of it are already built as if it was possible so we're more future-proof. Also, technically in WordPress you just need certain Site Kit permissions, developers could modify that so that you don't need to be an admin etc.

Secondary users should never actually see the Reset button if someone else is already using the plugin. Having thought about this further, there is no use-case for allowing "Disconnect" here, since if they were connected they would never end up here anyway.

@felixarntz regarding the logic for hasConnectedAdministrators it seems like this should be covered by First_Admin / googlesitekit_first_admin, although this seems to be set improperly by Authentication::inline_js_setup_data, rather than in something like OAuth_Client::authorize_user which I think would make more sense.

Should we update this to store the user ID of the first admin Site Kit is actually set up with and base the logic on this instead of the user query?

@aaemnnosttv

@felixarntz regarding the logic for hasConnectedAdministrators it seems like this should be covered by First_Admin / googlesitekit_first_admin, although this seems to be set improperly by Authentication::inline_js_setup_data, rather than in something like OAuth_Client::authorize_user which I think would make more sense.

I think that's different, the goal of hasConnectedAdministrators is to check whether there are currently any administrators with an access token, while googlesitekit_first_admin is just a single user ID that does (intentionally) not get unset if you disconnect. So it might still be set although there is no connected user.

Your suggestion to move the logic makes sense, that is however already part of #1743, since it's not really needed here when we don't rely on googlesitekit_first_admin.

We may also want to rename the isSecondaryAdmin variable for clarity; this is really about not resetting while someone else with admin privileges is connected.

@aaemnnosttv IB mostly sounds good to me, I think there's one detail we should change: In your current approach the option is more like a boolean to check whether there are any such administrators (it just checks for one). We should probably call the option googlesitekit_has_connected_admins to indicate this. If it was googlesitekit_connected_admins, I'd expect it to e.g. contain an array of user IDs.

@felixarntz IB updated.

One question remains there: should we add proxySetupURL as a selector to the core/site store?

@aaemnnosttv IB โœ…

Yes, good point, let's add the following selectors to core/site:

  • getProxySetupURL
  • getProxyPermissionsURL

At the moment, the base data _may not_ include these values (if not using proxy). This also applies to the usingProxy field itself, which is already exposed via a selector, however behaves slightly weird right now, because it would only return true or undefined. Let's modify the PHP implementation of Authentication::inline_js_base_data so that these three fields are always present. If not using proxy, they should be false / an empty string respectively. Added this to the ACs.

@marrrmarrr one suggestion regarding the wording currently in the ACs:

Once you complete the 3 setup steps, you'll see stats from all already activated Google products.

I think we can remove the word "already" here as it reads a bit more naturally without it since that's implied by the past tense of "activated" ๐Ÿ™‚

What do you think?

@aaemnnosttv SGTM!

This one has proxy URLs missing in the core/site info store - not sure how this didn't fail tests before; taking a look now.

Tested

Installed, activated and setup latest develop SK.

Created a new admin:
image

Logged in as the new admin:
image

Confirmed new message

Checked browser sizes:
image

image

Confirmed functionality of setup as new admin:
image

Passed QA โœ…

Looks great! The messaging is much clearer! ๐Ÿ‘

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jsmshay picture jsmshay  ยท  3Comments

aaemnnosttv picture aaemnnosttv  ยท  5Comments

aaemnnosttv picture aaemnnosttv  ยท  5Comments

aaemnnosttv picture aaemnnosttv  ยท  4Comments

felixarntz picture felixarntz  ยท  4Comments