-
Notifications
You must be signed in to change notification settings - Fork 286
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
Install/activate the WP Consent API plugin in the background. #8521
Comments
The ACs say we should enable the WP Consent API plugin, but they don't say when/under what conditions. Enabling it automatically when it isn't enabled seems like a fine idea, but we shouldn't do it constantly, because then a user that explicitly disables it (for whatever reason) after we enabled it will return to the plugins page to see it "randomly" enabled again by us. So we probably want to specify the conditions to enable it, and also a mechanism to only do so once. |
Thanks, this makes sense 👍🏻 Moving to IB 👍🏻 |
Hey @benbowler, actually the AC for this needs a revision. It's not the case that we want the WP Consent API plugin to be installed in direct response to the user enabling Consent Mode via the switch in settings. We still want the trigger to be a click of the Install (or Activate) button, so the user is making an informed and deliberate choice to install the plugin. At present when the Install/Activate button is clicked, the plugins page is navigated to, taking the user away from Site Kit. What this issue should accomplish is to install/activate the plugin in the background when the button is clicked, showing an in-progress state while doing so (i.e. a spinner in the button). Please can you update the AC accordingly? |
Hi @benbowler, thanks for updating the AC.
|
@benbowler thanks for the update. Please note I made a small change to tweak the proposed error text and fix a couple of typos. That said - while I like the proposed error handling, I do have some reservations; for one, handling an error with "warning" styling is a bit out of step with other errors which tend to be displayed inline using the "error" red colour. Also by showing a fixed error message, the underlying error we receive from the WP API is hidden and this might contain some valuable information as to why the plugin couldn't be installed. Lastly the proposed message doesn't handle the activation case where the more appropriate guidance would be to navigate to the plugins page on the current WP instance and try activating there. To be honest, I think we might be better off initially taking a simple approach where we display the error received inline using the usual error styling (as seen in We can then iterate on it in a subsequent issue to provide more detailed error handling along the lines of what you have specced, but looking further into the possible errors received by the WP plugin API rather than having a single fixed message. How does that sound? |
Agreed @techanvil, I've simplified the AC removing implementation detail which can code in the IB based on the comments above. |
QA Update ❌
Issue-
Install Plugin: Gets stuck in a loading state- Activate Plugin : PASS CASES
Recording.1192.mp4 |
QA Update ✅
|
Feature Description
As discussed on Slack, we should install and/or activate the WP Consent API plugin in the background, rather than opening the WP plugins page in a new tab to facilitate the installation/activation.
This will improve both the general UX and our capability for tracking the WP Consent API activation state, which will help to further inform how to improve the feature.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
wp-admin/update.php
.wp-admin/plugins.php
.Implementation Brief
includes/Core/Consent_Mode/Consent_Mode.php
:POST:install-activate-wp-consent-api
, by implementing theget_datapoint_definitions
method.create_data_request
method. When this endpoint is called:https://github.com/WordPress/WordPress/blob/master/wp-admin/update.php#L110-L125
https://github.com/WordPress/WordPress/blob/master/wp-admin/update.php#L142-L145
activate_plugin
function, throwing aWP_Error
if activation fails.installActivateWPConsentAPI
.assets/js/components/consent-mode/WPConsentAPIRequirements.js
:SpinnerButton
component.installActivateWPConsentAPI
action.ErrorNotice
, see this comment.Test Coverage
QA Brief
*To test the erorr, you can use HTTP proxy addon, like Tweak, and replicate these settings:
Changelog entry
The text was updated successfully, but these errors were encountered: