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

Custom capability conflict with W3TC plugin #296

Closed
fjarrett opened this issue Mar 4, 2014 · 7 comments

Comments

Projects
None yet
3 participants
@fjarrett
Copy link
Contributor

commented Mar 4, 2014

A user reported in the forums that while running W3 Total Cache with the New Relic setting enabled the Stream Records page disappears.

screen shot 2014-03-04 at 12 01 01 pm

I've isolated the problem to these lines in the W3TC plugin.

It seems that $current_user->roles is just an empty array when Stream is enabled.

If you change the VIEW_CAP constant to manage_options everything works perfectly fine. So something about the way Stream is using a custom capability might be causing a conflict here, but I haven't been able to pinpoint exactly which plugin is to blame.

/cc @kucrut @shadyvb

@fjarrett fjarrett added the bug label Mar 4, 2014

@kucrut kucrut self-assigned this Mar 11, 2014

@fjarrett fjarrett added the reserved label Mar 11, 2014

@kucrut

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2014

It seems that $current_user->roles is just an empty array when Stream is enabled.

This is not Stream's fault. WT3C modified the global $current_user->roles when New Relic is enabled. So, even when Stream's not active, it will be empty.

@kucrut

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2014

@fjarrett The only way I can think of to work-around this, is to also use $user->caps in WP_Stream_Admin::_filter_user_caps(), so something like:

public static function _filter_user_caps( $allcaps, $caps, $args, $user = null ) {
    $user  = is_a( $user, 'WP_User' ) ? $user : wp_get_current_user();
    $roles = array_unique( array_merge( $user->roles, array_keys( $user->caps ) ) );

    foreach ( $caps as $cap ) {
        if ( self::VIEW_CAP === $cap ) {
            foreach ( $roles as $role ) {
                if ( self::_role_can_view_stream( $role ) ) {
                    $allcaps[ $cap ] = true;
                    break 2;
                }
            }
        }
    }

    return $allcaps;
}
@fjarrett

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2014

@kucrut Thanks for looking into this.

I feel like we shouldn't have to implement workarounds for other plugins not following best practices, and I assume that modifying this global would fall into that category.

@shadyvb Your thoughts?

@fjarrett fjarrett removed the reserved label Mar 12, 2014

@shadyvb

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2014

While that seems like a workaround, that's what core is actually doing:
https://github.com/WordPress/WordPress/blob/c67c9565f1495255807069fdb39dac914046b1a0/wp-includes/capabilities.php#L758

if ( is_array( $this->caps ) )
    $this->roles = array_filter( array_keys( $this->caps ), array( $wp_roles, 'is_role' ) );

Risking situations where those two keys are modified inappropriately, would be the fault of whatever code that changes them ( like W3TC ). But what @kucrut is suggesting seems pretty valid to me ( not sure if it'd work though, haven't tested ).

@shadyvb

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2014

Out of curiosity, has anyone reported this to W3TC yet ?

@fjarrett

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2014

@shadyvb Not that we know of, I'll ask the OP.

@fjarrett

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2014

The OP has reported this to W3TC to get their feedback on the matter as well.

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.