google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 292 forks source link

Update SiwG "Get your Client ID" link and login URLs #9621

Closed tofumatt closed 1 day ago

tofumatt commented 3 weeks ago

Feature Description

The "Get your Client ID" URL should be updated to the new SiwG-specific URL for Site Kit.


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

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

aaemnnosttv commented 2 weeks ago

Thanks @zutigrm – I actually meant to put this into IBR but no problem. In light of new developments on the external side, I've made an update to the AC. The main change is that we'll no longer be passing a redirecturi to the external project provisioning service. See also #9677 which I just opened although I think this issue can happen in parallel since this one is more about changes to the setup.

I've also removed the point about changing the action parameter, as this is already happening/done in https://github.com/google/site-kit-wp/pull/9577/files#diff-5d9448f7515540d9d39cb7f666fd5dc594af902a96aef39a181a6de355263e3aR58

  • Add public static method get_google_login_url I'd suggest calling this get_auth_url since the prefix is redundant being on the "SiwG" class :) It also probably doesn't need to be static since it will only be used internally as of #9677

Update Google\Site_Kit\Core\Assets\Assets::get_inline_base_data

  • In $inline_data add new item to the array: loginURL, with value of Google\Site_Kit\Modules\Sign_In_With_Google::get_google_login_url()

I don't think this will be necessary now since we'll only need the login endpoint in PHP when rendering the button.

Add getLoginEndpointURL selector

  • Wrap it with createRegistrySelector and retrieve following data:

    • siteName using getSiteName selector from CORE_SITE store homeURL using getHomeURL selector from CORE_SITE store supportEmail using getEmail selector from CORE_USER store* redirecturi using newly created selector getLoginURL from CORE_SITE store
  • Define new constant loginEndpointURL and assign it value of https://developers.google.com/identity/site-kit

There seems to be some confusion here, but at the least, it's confusing to have very similarly named selectors for different things. The login endpoint is what we mean by the URL constructed from wp_login_url that login requests will be sent to. The developers.google.com/identity/site-kit based URL is really the SiwG module's "service URL" similar to other modules, so we can follow that pattern here as those all use AccountChooser URLs as well.

All in all, I think it will be substantially smaller to do so the estimate should be able to come down as well.

zutigrm commented 2 weeks ago

@aaemnnosttv Thanks, I update the IB following the feedback and AC change.

Add public static method get_google_login_url I'd suggest calling this get_auth_url since the prefix is redundant being on the "SiwG" class :) It also probably doesn't need to be static since it will only be used internally as of https://github.com/google/site-kit-wp/issues/9677

Since we no longer need login URL within the scope of this issue, I removed this part, as it can be done in the referenced issue if needed.

aaemnnosttv commented 2 weeks ago

Thanks @zutigrm

IB ✅

aaemnnosttv commented 2 weeks ago

@tofumatt @jimmymadon we need to make a very small change to this issue as it isn't worth creating a new one over. I'll tweak the AC accordingly but essentially the only change is as follows

- siteurl: The home URL of the site (front page root): use home URL (not reference URL)
+ siteorigin: The origin of the site root/home URL
mohitwp commented 5 days ago

QA Update ✅

![Image](https://github.com/user-attachments/assets/afa04a9d-2ecd-49fe-a81f-ae3e63ff8a4b) ![Image](https://github.com/user-attachments/assets/fca1a38c-2f8a-4ae6-b4c3-8e7d16776139)