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

Add wpsc_cookies and wpsc_plugins filters to modify those settings #582

Merged
merged 2 commits into from Aug 9, 2018

Conversation

Projects
None yet
2 participants
@donnchawp
Contributor

donnchawp commented Aug 9, 2018

These filters will allow WordPress plugins to modify the cookies and
plugins lists used by WP Super Cache instead of the add/delete functions
introduced in #574 and #580.
Duplicate entries are removed so the filter function does not need to
worry about that. The filters fire on 'init'.

Example filter code:

function myplugin_add_wpsc_cookies( $cookies ) {
    $cookies[] = 'myplugin';
    return $cookies;
}
add_filter( 'wpsc_cookies', 'myplugin_add_wpsc_cookies' );
function myplugin_delete_wpsc_cookies( $cookies ) {
    if ( in_array( 'myplugin', $cookies ) ) {
        unset( $cookies[ array_search( 'myplugin', $cookies ) ] );
    }
    return $cookies;
}
add_filter( 'wpsc_cookies', 'myplugin_delete_wpsc_cookies' );

As suggested by @johnbillion and @andreasciamanna in #580.

Add wpsc_cookies and wpsc_plugins filters to modify those settings
These filters will allow WordPress plugins to modify the cookies and
plugins lists used by WP Super Cache instead of the add/delete functions
introducted in #574 and #580.
Duplicate entries are removed so the filter function does not need to
worry about that. Example code:

`function myplugin_add_wpsc_cookies_filter( $cookies ) {
    $cookies[] = 'myplugin';
    return $cookies;
}
add_filter( 'wpsc_cookies', 'myplugin_add_wpsc_cookies_filter' );`

`function myplugin_delete_wpsc_cookies_filter( $cookies ) {
    if ( in_array( 'myplugin', $cookies ) ) {
        unset( $cookies[ array_search( 'myplugin', $cookies ) ] );
    }
    return $cookies;
}
add_filter( 'wpsc_cookies', 'myplugin_delete_wpsc_cookies_filter' );`
@andreasciamanna

This comment has been minimized.

Show comment
Hide comment
@andreasciamanna

andreasciamanna Aug 9, 2018

As I've tried to explain here I think an action is more appropriate and safer than a filter.

If properly used, a hook to the filter could mess with the content of $cookies.

An action seems to me more straight-forward.

In a plugin, I could call do_action( 'wpsc_add_cookie', 'my-plugin-cookie-name' );

And in wp-super-cache, wpsc_add_cookie() could be hooked to that action:

function wpsc_add_cookie( $name ) {
	global $wpsc_cookies;
	if (
		! isset( $wpsc_cookies ) ||
		! is_array( $wpsc_cookies ) ||
		! in_array( $name, $wpsc_cookies )
	) {
		$wpsc_cookies[] = $name;
		wp_cache_setting( 'wpsc_cookies', $wpsc_cookies );
	}
}
add_action( 'wpsc_cookies' );

andreasciamanna commented Aug 9, 2018

As I've tried to explain here I think an action is more appropriate and safer than a filter.

If properly used, a hook to the filter could mess with the content of $cookies.

An action seems to me more straight-forward.

In a plugin, I could call do_action( 'wpsc_add_cookie', 'my-plugin-cookie-name' );

And in wp-super-cache, wpsc_add_cookie() could be hooked to that action:

function wpsc_add_cookie( $name ) {
	global $wpsc_cookies;
	if (
		! isset( $wpsc_cookies ) ||
		! is_array( $wpsc_cookies ) ||
		! in_array( $name, $wpsc_cookies )
	) {
		$wpsc_cookies[] = $name;
		wp_cache_setting( 'wpsc_cookies', $wpsc_cookies );
	}
}
add_action( 'wpsc_cookies' );
@donnchawp

This comment has been minimized.

Show comment
Hide comment
@donnchawp

donnchawp Aug 9, 2018

Contributor

Ah. I wondered about that. Your example makes sense and avoids running an apply_filters() on every page load if there's no outside code that wants to modify the cookie list. Thanks. I'll modify the PR.

Contributor

donnchawp commented Aug 9, 2018

Ah. I wondered about that. Your example makes sense and avoids running an apply_filters() on every page load if there's no outside code that wants to modify the cookie list. Thanks. I'll modify the PR.

Changed filters to actions and added add/delete actions.
Instead of using filters to change cookie/plugin lists I've added four
actions:
wpsc_add_plugin
wpsc_delete_plugin
wpsc_add_cookie
wpsc_delete_cookie

For example, to add a cookie name called 'euCookie':

`do_action( 'wpsc_add_cookie', 'euCookie' );`

Props @andreasciamanna.
@donnchawp

This comment has been minimized.

Show comment
Hide comment
@donnchawp

donnchawp Aug 9, 2018

Contributor

I updated the PR, changing the filters to actions so you can do do_action( 'wpsc_add_cookie', 'euCookie' ); to add a cookie called 'euCookie'.

Contributor

donnchawp commented Aug 9, 2018

I updated the PR, changing the filters to actions so you can do do_action( 'wpsc_add_cookie', 'euCookie' ); to add a cookie called 'euCookie'.

@andreasciamanna

This comment has been minimized.

Show comment
Hide comment
@andreasciamanna

andreasciamanna Aug 9, 2018

Thanks a lot!

We are a bit at the end of our development cycle, but I'll see if I can manage to include it in the next WPML bugfix release.

If not, it will be included in the following one.

andreasciamanna commented Aug 9, 2018

Thanks a lot!

We are a bit at the end of our development cycle, but I'll see if I can manage to include it in the next WPML bugfix release.

If not, it will be included in the following one.

@donnchawp donnchawp merged commit b9e1dfd into master Aug 9, 2018

@andreasciamanna

This comment has been minimized.

Show comment
Hide comment
@andreasciamanna

andreasciamanna Aug 9, 2018

Just tested it with add_cookie_plugin_filters branch and it works perfectly.

Found a bug in wp-cache-phase2.php though: I'll create a PR.

andreasciamanna commented Aug 9, 2018

Just tested it with add_cookie_plugin_filters branch and it works perfectly.

Found a bug in wp-cache-phase2.php though: I'll create a PR.

@donnchawp

This comment has been minimized.

Show comment
Hide comment
@donnchawp

donnchawp Aug 9, 2018

Contributor

Great. that's merged and I'll get a new release out this evening or tomorrow.

Contributor

donnchawp commented Aug 9, 2018

Great. that's merged and I'll get a new release out this evening or tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment