-
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
Create infrastructure for detecting & installing WP Consent API. #8305
Create infrastructure for detecting & installing WP Consent API. #8305
Conversation
Build files for 8277b0f have been deleted. |
Size Change: +81 B (0%) Total Size: 1.49 MB
ℹ️ View Unchanged
|
$install_url = $nonce_url( self_admin_url( 'update.php?action=install-plugin&plugin=' . $slug ), 'install-plugin_' . $slug ); | ||
|
||
$response['wpConsentPlugin'] = array( | ||
'installed' => $installed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there's no need to include the active
field specified in the AC. This is because hasConsentAPI
indicates the active status.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @techanvil – this LGTM, I just had one thought about some of the plugin checks but it's not blocking and we can always address that in a follow-up.
$slug = 'wp-consent-api'; | ||
$plugin = "$slug/$slug.php"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could actually be different if it were installed via Composer. See https://packagist.org/packages/rlankhorst/wp-consent-level-api
Definitely an edge case for most of our users I think, but worth keeping in mind that the $plugin
can't always be assumed to be $slug/$slug
. Not just because of Composer, but the directory can be renamed, or sometimes even the plugin itself can change the name of its entry point php file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @aaemnnosttv, that's a good point. I've created #8307 to address this later. Seems like a post-MVP issue to me.
Summary
Addresses issue:
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist