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

Update WP Consent API plugin detection to account for non-standard location #8307

Closed
9 tasks
techanvil opened this issue Feb 23, 2024 · 8 comments
Closed
9 tasks
Labels
P1 Medium priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@techanvil
Copy link
Collaborator

techanvil commented Feb 23, 2024

Feature Description

As mentioned in this #8305 (comment), it's possible for the WP Consent API plugin entry point to reside in a location other than wp-consent-api/wp-consent-api.php.

$slug = 'wp-consent-api';
$plugin = "$slug/$slug.php";

We should update the above code to account for alternative locations for the plugin.


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

Acceptance criteria

  • The Consent Mode controller should detect that the Consent API plugin is installed, even if it was installed via Composer or into a custom WordPress plugin folder and use the correct path to the plugin file.

Implementation Brief

  • Update the core/site/data/consent-mode REST route includes/Core/Consent_Mode/REST_Consent_Mode_Controller.php:
    • Replace the $slug and $plugin variables with a single new variable called $plugin_uri with the value of https://wordpress.org/plugins/wp-consent-api. This is the URI of the plugin which won't change even if the plugin folder or main file changes.
      $slug = 'wp-consent-api';
      $plugin = "$slug/$slug.php";
    • Create the $slug and $plugin variables as empty strings to set later based on dynamic plugin values.
    • Update the install check block:
      foreach ( array_keys( get_plugins() ) as $installed_plugin ) {
      if ( $installed_plugin === $plugin ) {
      $installed = true;
      break;
      }
      }
      • Get the key and value in the foreach get_plugins() as $plugin_file => $installed_plugin.
      • Remove the array_keys function so that the loop gets the full plugin objects.
      • Update the check condition to $installed_plugin["PluginURI"] === $plugin_uri.
        • Set the $slug variable using $installed_plugin["TextDomain"].
        • Set the $plugin variable using $plugin_file.
    • The response block will now use the $slug and $plugin variables set based on the WordPress plugin data above, no changes required:
      $activate_url = $nonce_url( self_admin_url( 'plugins.php?action=activate&plugin=' . $plugin ), 'activate-plugin_' . $plugin );
      $install_url = $nonce_url( self_admin_url( 'update.php?action=install-plugin&plugin=' . $slug ), 'install-plugin_' . $slug );
      $response['wpConsentPlugin'] = array(
      'installed' => $installed,
      'activateURL' => current_user_can( 'activate_plugin', $plugin ) ? esc_url_raw( $activate_url ) : false,
      'installURL' => current_user_can( 'install_plugins' ) ? esc_url_raw( $install_url ) : false,
      );

Test Coverage

  • Confirm existing phpunit tests pass.

QA Brief

  • Setup and configure a basic Site Kit installation
  • Manually download the WP Consent API plugin from https://wordpress.org/plugins/wp-consent-api/
  • Extract it to your plugins folder in a randomly named folder, i.e wp-content/plugins/test-consent-plugin
  • Navigate to Site Kit > Settings > Admin Settings
  • Under Consent Mode, enable said feature
  • Validate that the Activate CTA button shows as per the screenshot below:
image
  • Validate that, upon clicking said CTA, the WP Consent API plugin is successfully activated
  • Validate that, upon returning to Site Kit > Settings > Admin Settings, the Consent mode notice indicates the successful presence of the plugin as per the screenshot below:
image
  • Deactivate and delete the plugin
  • Ensure that the installation and activations steps for the plugin can be performed in Site Kit > Settings > Admin Settings

Changelog entry

  • Detect WP Consent API plugin even when it's installed in a non-standard folder name.
@techanvil techanvil added P1 Medium priority Type: Enhancement Improvement of an existing feature labels Feb 23, 2024
@binnieshah binnieshah added the Next Up Issues to prioritize for definition label Jun 10, 2024
@benbowler benbowler assigned benbowler and unassigned benbowler Jun 10, 2024
@eugene-manuilov eugene-manuilov self-assigned this Jun 12, 2024
@eugene-manuilov
Copy link
Collaborator

AC ✔️, I have adjusted it a bit to add a note that the controller should use the right path to the plugin.

@eugene-manuilov eugene-manuilov removed their assignment Jun 12, 2024
@benbowler benbowler assigned benbowler and unassigned benbowler Jun 12, 2024
@tofumatt tofumatt self-assigned this Jun 12, 2024
@tofumatt
Copy link
Collaborator

IB ✅

@tofumatt tofumatt removed their assignment Jun 12, 2024
@tofumatt tofumatt removed the Next Up Issues to prioritize for definition label Jun 12, 2024
@benbowler benbowler assigned benbowler and unassigned benbowler Jun 13, 2024
@binnieshah binnieshah added Next Up Issues to prioritize for definition Team S Issues for Squad 1 labels Jun 14, 2024
@binnieshah binnieshah removed the Next Up Issues to prioritize for definition label Jun 25, 2024
@10upsimon 10upsimon self-assigned this Jul 1, 2024
@10upsimon 10upsimon removed their assignment Jul 8, 2024
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Jul 9, 2024
@mohitwp mohitwp self-assigned this Jul 9, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented Jul 9, 2024

QA Update ✅

  • Tested on dev environment.
  • Tested using File manager plugin.
  • Tested by creating custom folder as mentioned in QAB.
  • Verified that the Consent Mode controller detect that the Consent API plugin is installed, even if it was installed via Composer or into a custom WordPress plugin folder and use the correct path to the plugin file.
  • Verified that plugin installation and activation is working as expected in case of installation through custom folder or directly through Site Kit > Settings > Admin Settings.

image

image

image

Recording.1155.mp4

@mohitwp mohitwp removed their assignment Jul 9, 2024
@wpdarren
Copy link
Collaborator

@mohitwp as per my conversation on Slack. I am a little paranoid with this feature due to past issues 😄 so could you test the sceario in the QAB, but also check that the snippet still appears in the source code and ensure that tracking is working as expected for EU and Non EU traffic. More info on the snippet can be found in the testing instructions. Let me know if you have any questions.

@mohitwp
Copy link
Collaborator

mohitwp commented Jul 10, 2024

QA Update ✅

    • Tested on dev environment.
    • Tested using File manager plugin.
    • Tested by creating custom folder as mentioned in QAB.
    • Verified that the Consent Mode controller detect that the Consent API plugin is installed, even if it was installed via Composer or into a custom WordPress plugin folder and use the correct path to the plugin file.
    • Verified that plugin installation and activation is working as expected in case of installation through custom folder or directly through Site Kit > Settings > Admin Settings.
  • Verified that tracking is working as expected for Non EU and EU members including UK.
  • Done E2E testing for consent mode functionality as per test instructions.

image

image

image

Recording.1155.mp4
  • Verified that if consent mode is enabled snippet is showing under source code.
Recording.1160.mp4

image

  • Verified that consent mode snippet is not showing if consent mode is not enabled.
Recording.1161.mp4
  • Verified GA events related to WP constant API plugin activation and other Consent mode GA events listed under testing instructions.

image

image

image

image

image

image

image

image

image

@mohitwp mohitwp removed their assignment Jul 10, 2024
@aaemnnosttv
Copy link
Collaborator

⚠️ Approval

There is a small change we should make to the implementation here. See #8959 (review)

I can raise a quick follow up here if needed.

@nfmohit
Copy link
Collaborator

nfmohit commented Jul 12, 2024

⚠️ Approval

There is a small change we should make to the implementation here. See #8959 (review)

I can raise a quick follow up here if needed.

I've created a follow-up PR.

@aaemnnosttv
Copy link
Collaborator

Thanks @nfmohit, this has been merged and tested so this is good to go 👍

It's worth noting that this issue's changes were somehow reverted (no longer in main or develop) in 9ab0412 after it had gone through QA, so the follow-up PR actually re-introduced the change while addressing the points I raised.

@aaemnnosttv aaemnnosttv removed their assignment Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

10 participants