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

Issue #132 : Allow filtering to disable 'posts' component. #219

Merged
merged 5 commits into from Aug 17, 2016

Conversation

Projects
None yet
2 participants
@kienstra
Copy link
Contributor

commented Aug 17, 2016

@westonruter,
Could you please review this pull request for Issue #132?

The new filter add_posts_to_customize_loaded_components adds 'posts' to the array of components in the Customizer.
And WP_Customize_Posts is only instantiated if 'posts' is still present.
So a later filter may remove 'posts', to disable it.

One issue I noticed is that if a filter does remove 'posts', it causes a fatal error:

Notice: Undefined property: WP_Customize_Manager::$posts in /srv/www/wordpress-develop/src/wp-content/plugins/wp-customize-posts/php/class-customize-posts-plugin.php on line 179 Fatal error: Call to a member function add_support() on a non-object in /srv/www/wordpress-develop/src/wp-content/plugins/wp-customize-posts/php/class-customize-posts-plugin.php on line 179

Removing the action on line #71 removed that error, but there was another fatal error.

I could have missed something here.

Fixes #132.

Issue #132 : Allow filtering to disable 'posts' component.
A new filter adds 'posts' to the array of components in the Customizer.
WP_Customize_Posts is only instantiated if 'posts' is still present.
So a filter may remove 'posts', to disable it.
@westonruter

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2016

@kienstra this is looking good. Regarding the fatal error, the fix should be simple. At the top of load_support_classes try adding:

if ( ! isset( $wp_customize->posts ) ) {
    return;
}

That should do it. We just have to account for the same situation as another plugin that looks for whether the posts component is enabled.

@westonruter westonruter added this to the 0.8 milestone Aug 17, 2016

kienstra added some commits Aug 17, 2016

Issue #132 : Prevent error from using an undefined property.
$wp_customize->posts is now only set if 'posts' isn't removed with a filter.
And before, removing it caused an error.
So if #wp_customize->posts is not set, return from load_support_classes.
Issue #32 : Avoid an error by using class_exists().
WP_Customize_Posts may not be instantiated if a filter removes 'posts'.
So the class WP_Customize_Postmeta_Setting may not exist.
In this case, it would cause a fatal error, when accessing its property.
So begin the conditional with class_exists().
@kienstra

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2016

Hi @westonruter,
Thanks for helping with this, especially so quickly. The commits above took care of errors locally, when 'posts' is removed from $components.

*
* @param array $components Components.
* @param WP_Customize_Manager $wp_customize Manager.
* @return array Components.
*/
function filter_customize_loaded_components( $components, $wp_customize ) {
require_once dirname( __FILE__ ) . '/class-wp-customize-posts.php';
$wp_customize->posts = new WP_Customize_Posts( $wp_customize );
if ( in_array( 'posts', $components ) ) {

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 17, 2016

Contributor

Add a third true argument to in_array() for strict checking.

* @param WP_Customize_Manager $wp_customize Manager.
* @return array Components.
*/
function add_posts_to_customize_loaded_components( $components, $wp_customize ) {

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 17, 2016

Contributor

Remove $wp_customize arg and corresponding phpdoc, as the param is not used.

kienstra added some commits Aug 17, 2016

Issue #132 : Apply Weston's feedback - arguments and strict checking.
Remove $wp_customize param  and its DocBlock line--it's not used.
And reduce argument count in add_filter to 1.
Also, add 3rd argument true to in_array for strict evaluation.
@kienstra

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2016

Thanks For Review
Commit To Apply Feedback

Hi @westonruter,
Thanks for your feedback. I should have caught those points with a PHPCS check. The commit above makes the corrections.

@westonruter westonruter merged commit 68d00aa into develop Aug 17, 2016

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 96.533%
Details

@westonruter westonruter deleted the feature/allow-removing-posts-component branch Aug 17, 2016

@westonruter westonruter modified the milestones: 0.7.1, 0.8 Sep 1, 2016

@westonruter westonruter restored the feature/allow-removing-posts-component branch Sep 1, 2016

@westonruter westonruter modified the milestones: 0.8, 0.7.1 Sep 1, 2016

@valendesigns valendesigns deleted the feature/allow-removing-posts-component branch Aug 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.