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

Conditionally Enqueue Admin Scripts and Styles for Enhanced Authorize Application Screen Visibility #8504

Closed
18 tasks done
hussain-t opened this issue Apr 9, 2024 · 7 comments
Labels
P0 High priority Squad 2 (Team M) Issues for Squad 2 Type: Enhancement Improvement of an existing feature

Comments

@hussain-t
Copy link
Collaborator

hussain-t commented Apr 9, 2024

Feature Description

Implement custom logic to conditionally load admin scripts and styles to enhance the WordPress Authorize Application screen based on the domain name within the success_url parameter, ensuring that enhancements are only applied for users with Site Kit for Google services.

For more information, refer to the Custom Style Application and Identification Strategy sections in the design doc.


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

Acceptance criteria

  • Class Implementation: A new class named Authorize_Application must be successfully implemented within the Google\Site_Kit\Core\Admin namespace. This class will encapsulate all conditional logic for loading scripts and styles on the Authorize Applications screen.
  • Functionality of Authorize_Application Class: The Authorize_Application class must control the conditional enqueuing of scripts and styles based on two key criteria: the domain name in the success_url parameter and confirmation that the current page is the authorize-application page.
  • Specific Page Detection: The Authorize_Application class must confirm that the current page is the authorize-application page before enqueueing any scripts or styles. This ensures that enhancements are targeted and do not affect other areas of the admin dashboard.
  • Path Detection: The class should validate that the success_url parameter contains the specific path /settings/authorization/wordpress, indicating that the user interaction is related to the intended integration.
  • Conditional Script and Style Enqueueing: Scripts and styles should only be enqueued when both conditions are met: the page is authorize-application and the success_url domain is correctly identified as a *.google.com domain.

Implementation Brief

In the includes/Core/Admin directory:

  • Create a new Authorize_Application class within the Google\Site_Kit\Core\Admin namespace.

Constructor Method

  • Implement a __construct method within the Authorize_Application class that receives Context $context and Assets $assets as parameters.

Methods

  • register():
    • Register the enqueue_assets method to the WordPress admin_enqueue_scripts action, ensuring the Authorize Application styles are conditionally loaded.
  • is_authorize_application_screen:
    • Checks if the current admin screen is the authorize-application page. Utilize get_current_screen()->id to determine the screen identity.
    • Return: Boolean. True if on the correct screen, false otherwise.
  • is_google_service:
    • Validate the path part of the success_url parameter to ensure it matches the specified path /settings/authorization/wordpress.
    • Extract the domain using parse_url() and check it against a regex pattern.
    • Return: Boolean. True if the domain is valid, false otherwise.
  • enqueue_assets():
    • Utilize the Assets::enqueue_asset method to enqueue the stylesheet.
    • Conditionally enqueue the googlesitekit-authorize-application stylesheet if both is_authorize_application_screen and is_google_service return true.

Add the Stylesheet to the Assets Class

In includes/Core/Assets/Assets.php:

  • Modify the Assets::get_assets method to include the googlesitekit-authorize-application stylesheet using the Stylesheet constructor.

Register the Authorize_Application Class

In includes/Plugin.php:

  • Register the Authorize_Application class within the Plugin::register method.
  • Ensure it is registered after the Assets class.

Test Coverage

  • Add basic test coverage for the Authorize_Application class.

QA Brief

  • Ensure that Site Kit is installed and activated on your WordPress installation.

Testing with Site Kit Activated and Google Domain:

  • Navigate to the following URL on your WordPress site, replacing example.com with your actual site domain:
https://example.com/wp-admin/authorize-application.php?app_name=GoogleServiceIntegration&app_id=123e4567-e89b-12d3-a456-426614174000&success_url=https%3A%2F%2Fexample.google.com%2Fsettings%2Fauthorization%2Fwordpress&sitekit=true
  • Confirm that the Yes, I approve of this connection button displays with the color #0B57D0.

Testing with Non-Google Domain:

  • Modify the success_url Domain: Change the success_url to a non-Google domain and access the same URL:
https://example.com/wp-admin/authorize-application.php?app_name=GoogleServiceIntegration&app_id=123e4567-e89b-12d3-a456-426614174000&success_url=https%3A%2F%2Fexample.com%2Fsettings%2Fauthorization%2Fwordpress&sitekit=true
  • Ensure that the Yes, I approve of this connection button reverts to the default WordPress button color.
  • Confirm that no custom styles are applied.

Testing with Site Kit Deactivated:

  • Deactivate Site Kit.
  • Access the URL again with the original success_url pointing to a Google domain:
https://example.com/wp-admin/authorize-application.php?app_name=GoogleServiceIntegration&app_id=123e4567-e89b-12d3-a456-426614174000&success_url=https%3A%2F%2Fexample.google.com%2Fsettings%2Fauthorization%2Fwordpress&sitekit=true
  • Verify that the Yes, I approve of this connection button displays the default WordPress styles, showing that Site Kit enhancements are not applied without the plugin activated.

Changelog entry

  • Enqueue stylesheet specific to the Authorize Application screen.
@hussain-t hussain-t added P0 High priority Type: Enhancement Improvement of an existing feature Squad 2 (Team M) Issues for Squad 2 labels Apr 9, 2024
@hussain-t hussain-t assigned hussain-t and unassigned hussain-t Apr 12, 2024
@hussain-t
Copy link
Collaborator Author

I've added the AC and IB and have moved directly to IBR.

@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Apr 12, 2024
@techanvil techanvil self-assigned this Apr 12, 2024
@techanvil
Copy link
Collaborator

Hi @hussain-t, please can you take a look at this #8503 (comment), and make appropriate changes to this issue too?

@techanvil techanvil assigned hussain-t and unassigned techanvil Apr 12, 2024
@hussain-t
Copy link
Collaborator Author

hussain-t commented Apr 12, 2024

Thanks, @techanvil. "Authorize Application" sounds good to me. I've updated the AC and IB accordingly.

@hussain-t hussain-t assigned techanvil and unassigned hussain-t Apr 12, 2024
@hussain-t hussain-t changed the title Conditionally Enqueue Admin Scripts and Styles for Enhanced Application Passwords Screen Visibility Conditionally Enqueue Admin Scripts and Styles for Enhanced Authorize Application Screen Visibility Apr 15, 2024
@techanvil
Copy link
Collaborator

IB LGTM, nice one @hussain-t! ✅

@techanvil techanvil removed their assignment Apr 15, 2024
@nfmohit nfmohit self-assigned this Apr 15, 2024
@hussain-t
Copy link
Collaborator Author

@techanvil, I have updated the AC and IB to align with the latest confirmation regarding the success_url. As Gilad mentioned:

so you should expect the domain to be different, but the path to be the same: /settings/authorization/wordpress.

Based on this, our checks are now focused on the specific static path rather than the domain.

@nfmohit nfmohit assigned hussain-t and unassigned nfmohit Apr 15, 2024
@techanvil
Copy link
Collaborator

techanvil commented Apr 15, 2024

@hussain-t cool, thanks for the heads up. On a minor related note, the IB specifies checking using a regex pattern - we might find a substring match is sufficient, but we can figure that out at implementation time too.

@ivonac4 ivonac4 removed the Next Up Issues to prioritize for definition label Apr 16, 2024
hussain-t added a commit that referenced this issue Apr 17, 2024
@hussain-t hussain-t removed their assignment Apr 18, 2024
@nfmohit nfmohit self-assigned this Apr 18, 2024
nfmohit added a commit that referenced this issue Apr 19, 2024
@nfmohit nfmohit removed their assignment Apr 19, 2024
@kelvinballoo
Copy link
Collaborator

QA Update ✅

Tested on develop branch on WP 6.5.2 and PHP 8.0 and results are as expected:

  • Testing with Site Kit Activated and Google Domain:
    Reviewed this link and I can confirm the button colour is #0B57D0.
    SK active + Google dom
  • Testing with Non-Google Domain:
    Reviewed this link and I can confirm the button 'Yes, I approve of this connection button' is following the default WordPress button colour: #3858E9
    This is the same as what is used on the Prod version.

    Screenshot 2024-04-23 at 15 33 26
  • Testing with Site Kit Deactivated:
    Reviewed this link and I can confirm the button 'Yes, I approve of this connection button' is following the default WordPress button colour: #3858E9

    Screenshot 2024-04-23 at 15 37 10

This is working well @hussain-t . I am moving this ticket to Approval column.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Squad 2 (Team M) Issues for Squad 2 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants