-
Notifications
You must be signed in to change notification settings - Fork 812
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
Adds unit tests and is_module_active method to SAL. #11010
Conversation
D22570-code. (newly created revision) |
This is automated check which relies on |
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.
Thank you @zinigor 🙇
@@ -96,6 +96,14 @@ function get_jetpack_modules() { | |||
return null; | |||
} | |||
|
|||
function is_module_active( $module ) { | |||
if ( is_user_member_of_blog() ) { |
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.
Why do we need the membership check here?
@@ -89,11 +89,11 @@ function after_render_options( &$options ) { | |||
} | |||
|
|||
function get_jetpack_modules() { | |||
if ( is_user_member_of_blog() ) { |
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.
if we remove the check from here we will want to add it in json-endpoints/class.wpcom-json-api-get-site-endpoint.php
This is already deployed on the other side. Thanks, Miguel! |
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.
looks good. Merging.
In order to synchronize code between Jetpack and WordPress.com better one of the little pieces we needed was the
is_module_active
method present in the Site Abstraction Layer. This change adds it as well as enforcing its existence in all extending classes.Changes proposed in this Pull Request:
is_module_active
method for SAL Sites.Testing instructions:
Proposed changelog entry for your changes:
This blocks #10945