Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a "get help" link to the "reconnect_after_url_mismatch" notice. #5481

Closed
eugene-manuilov opened this issue Jun 30, 2022 · 17 comments
Closed
Labels
P1 Medium priority PHP Type: Enhancement Improvement of an existing feature

Comments

@eugene-manuilov
Copy link
Collaborator

eugene-manuilov commented Jun 30, 2022

Feature Description

We need to add a new "Get Help" link to the notification that appears when site has changed its URL. The URL for the new "get help" link should be obtained using the getDocumentationLinkURL selector introduced in #5423.

$content = '<p>' . sprintf(
/* translators: 1: Plugin name. 2: Message. 3: Proxy setup URL. 4: Reconnect string. */
__( '%1$s: %2$s <a href="%3$s">%4$s</a>', 'google-site-kit' ),
esc_html__( 'Site Kit by Google', 'google-site-kit' ),
esc_html__( 'Looks like the URL of your site has changed. In order to continue using Site Kit, you’ll need to reconnect, so that your plugin settings are updated with the new URL.', 'google-site-kit' ),
esc_url( $this->get_proxy_setup_url() ),
esc_html__( 'Reconnect', 'google-site-kit' )
) . '</p>';


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

Acceptance criteria

  • The "reconnect after url mismatch" notification should display a new "Get Help" link that leads to the {proxy}/support/?doc=url-has-changed URL.

Implementation Brief

  • In includes/Core/Authentication/Authentication.php:
    • Locate the get_reconnect_after_url_mismatch_notice method.
    • Inside the method's Notice class definition, in the second parameter array, locate the function in the content property.
    • Inside the function, when the $content variable is assigned, add a full stop after the closing </a> tag. After the full stop, add a new <a> tag with href attribute having the fifth placeholder (%5$s - the support link) and the content as the sixth placeholder (%6$s - the get help string). It should also have a target attribute with the value of _blank.
      __( '%1$s: %2$s <a href="%3$s">%4$s</a>', 'google-site-kit' ),
      • The fifth placeholder value should be /?doc=url-has-changed appended to the get_proxy_support_link_url() method of the same Authentication class. It should be escaped with esc_url().
      • The sixth placeholder should be Get Help (translatable).
  • In assets/js/components/setup/SetupUsingProxyWithSignIn.js:
    • Compose the "Get Help" link URL:
      • Use the getDocumentationLinkURL selector with the slug url-has-changed from the core/site store and assign it in a variable named urlHasChangedHelpLink (or something more relevant).
    • Create a new reassignable variable named getHelp.
    • After the description is displayed, check if getHelp is truthy and if so, display a <Link /> component with an href prop pointing to the getHelp variable and an external prop with true. The content of the component should be Get Help (translatable).
      <p className="googlesitekit-setup__description">
      { description }
      </p>
    • In the case where DISCONNECTED_REASON_CONNECTED_URL_MISMATCH === disconnectedReason is true and urlHasChangedHelpLink is truthy, assign urlHasChangedHelpLink to the getHelp variable.
      } else if (
      DISCONNECTED_REASON_CONNECTED_URL_MISMATCH === disconnectedReason
      ) {
      title = __( 'Reconnect Site Kit', 'google-site-kit' );
      description = __(
      'Looks like the URL of your site has changed. In order to continue using Site Kit, you’ll need to reconnect, so that your plugin settings are updated with the new URL.',
      'google-site-kit'
      );
      } else if ( isSecondAdmin ) {

Test Coverage

  • Verify the Setup -> Using Proxy with Sign-in -> Disconnected - URL Mismatch stories.
  • Update VRT images if required.

QA Brief

  • Locate the Setup -> Using Proxy with Sign-in -> Disconnected - URL Mismatch story in storybook
  • Verify that a new Get help-link has been added below the description
  • Verify that the link opens in a new tab and that the URL is <proxiedSupportURL>/?doc=url-has-changed

  • Create a new site and sign in with Site Kit.
  • Change the URL of the site so Site Kit shows the "URL Mismatch notice" when accessing the dashboard.
  • Verify there is now a "Get help" link beneath the URL mismatch notice message and it opens a ?/doc=url-has-changed link in a new tab.

Changelog entry

  • Add a help link to the "site URL has changed" notice when Site Kit detects your site URL has changed.
@eugene-manuilov eugene-manuilov added P1 Medium priority Type: Enhancement Improvement of an existing feature PHP labels Jun 30, 2022
@felixarntz
Copy link
Member

@eugene-manuilov Hmm, is this really what we should do here? Did that come up before? This is not an error I'd say, if anything we would put an actual documentation link here.

I think we should only consider things errors which are "unexpected" or variable. If we cover for a certain situation, and we want to make documentation for this specific topic available, I'd rather say this is not an error but a regular documentation link.

@eugene-manuilov
Copy link
Collaborator Author

@felixarntz this has been added to the error redirects mapping document. I agree with you that it is a regular documentation link. @bethanylang or @adamdunnage do you think we can move it from the errors document to the "learn more" redirects?

@adamdunnage
Copy link
Collaborator

@eugene-manuilov Is this the same as what we have documented here? Or is it slightly different?

@eugene-manuilov
Copy link
Collaborator Author

@eugene-manuilov Is this the same as what we have documented here? Or is it slightly different?

@adamdunnage, no this is different. This issue is for the case when the URL of the site has changed. The issue you are referring to happens during OAuth process.

@adamdunnage
Copy link
Collaborator

@eugene-manuilov Okay thanks for explaining. In which case I think that yes we should it from the errors document to the "learn more" redirects.

@bethanylang Do you agree?

@bethanylang
Copy link
Collaborator

Makes sense to me, thanks!

@eugene-manuilov
Copy link
Collaborator Author

Thanks, @adamdunnage and @bethanylang. Adam, will you remove the "reconnect_after_url_mismatch" row from the error redirects and add it to the "learn more" redirects?

I have updated AC to use the redirect link from the documentation redirects but left the same "get help" label. The only question is where do we need to put that "get help" link, right after the reconnect link?

@adamdunnage
Copy link
Collaborator

@eugene-manuilov I have added this to the "get help" document with the slug and documentation URL for this.

@eugene-manuilov
Copy link
Collaborator Author

@eugene-manuilov I have added this to the "get help" document with the slug and documentation URL for this.

@adamdunnage we need to add it to the "learn more" redirects (for documentation links) and remove it from the "get help" redirects (which is intended for errors). I don't see it's added to the correct redirects doc. Could you please check again?

@eugene-manuilov
Copy link
Collaborator Author

Ok, updated AC to use the url-has-changed slug for redirect since we already have in the redirects document for "learn more" links and it is used in another place as well.

@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Jul 23, 2022
@eugene-manuilov eugene-manuilov self-assigned this Jul 26, 2022
@eugene-manuilov
Copy link
Collaborator Author

Thanks, @nfmohit. Mostly looks good to me. Have a few comments:

extend the $content variable with concatenation and include a new <p> tag containing a link (<a>) tag.

No need to put the new link into a separate <p> tag, it will make the link appear below the notice message. We need to add the get help link after the reconnect one.

  • Use the getDocumentationLinkURL selector with the slug url-has-changed from the core/site store and assign it in a variable named alreadyConfiguredGetHelpLink (or something more relevant).

How about urlHasChangedHelpLink?

@eugene-manuilov
Copy link
Collaborator Author

  • add a new <p> tag containing a <Link> component.

Same here, no need to put it into a separate <p> tag. It should go in the same paragraph.

@nfmohit
Copy link
Collaborator

nfmohit commented Jul 26, 2022

Thank you @eugene-manuilov! I've updated the IB so that the Get Help link appears right after the Reconnect link.

@nfmohit nfmohit assigned eugene-manuilov and unassigned nfmohit Jul 26, 2022
@eugene-manuilov
Copy link
Collaborator Author

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Jul 26, 2022
@maciejost maciejost self-assigned this Jul 31, 2022
@maciejost maciejost removed their assignment Aug 3, 2022
@tofumatt tofumatt assigned tofumatt and maciejost and unassigned tofumatt Aug 3, 2022
@maciejost maciejost assigned tofumatt and unassigned maciejost Aug 5, 2022
tofumatt added a commit that referenced this issue Aug 8, 2022
@tofumatt tofumatt removed their assignment Aug 8, 2022
tofumatt added a commit that referenced this issue Aug 8, 2022
Add a get help link to mismatched url errors.
@mohitwp mohitwp self-assigned this Aug 9, 2022
@mohitwp
Copy link
Collaborator

mohitwp commented Aug 9, 2022

QA Update ⚠️

@eugene-manuilov @makiost

I tried to create this scenario using dev plugin. I enter correct URL in dev plugin first and after that 'sign in with google' and done set up for site kit. Again, change custom URL under dev plugin and navigated to site kit dashboard. Here on dashboard I'm getting 'insufficient permission error' instead of 'URL mismatch notice'.
Can you please guide me how can I create this scenario. Let me know if i missing any step ?

@eugene-manuilov
Copy link
Collaborator Author

@mohitwp I think the easiest way to replicate this scenario is to create a multisite instance with subdirectories. Such setup will allow you to create a subsite with a subfolder URL that you can change as you want on the general settings page. Just do something like this:

  1. Create a new site.
  2. Activate the multisite mode and configure it to be subdirectories.
  3. Create a new subsite.
  4. Install, activate and connect site kit on the subsite.
  5. Go to the general settings and change the URL to a different folder.
  6. Go back to the SK dashboard and verify that you see the get help link.

Let me know if it helps.

@mohitwp
Copy link
Collaborator

mohitwp commented Aug 10, 2022

QA Update ✅

Verified 👍

  • Verified Setup -> Using Proxy with Sign-in -> Disconnected - URL Mismatch story in storybook.

  • A new Get help-link has been added below the description

  • Link opens in a new tab and that the URL is /?doc=url-has-changed

  • Also, verified scenario on WP site.

  • Now There is a "Get help" link beneath the URL mismatch notice message and it opens a ?/doc=url-has-changed link in a new tab.

  • It navigates user to support link

  • Also, verified notices on WP dashboard.

image

image

image

@mohitwp mohitwp removed their assignment Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority PHP Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

8 participants