warfare-plugins / social-warfare

This is a public repository for the Social Warfare WordPress plugin created primarily for the purpose of publishing and maintaining a public list of bugs, known issues, and feature requests with the community at large.
35 stars 27 forks source link

Move the Pinterest conditional to inside the filter functions in url-processing.php #487

Closed Nicholas-Cardot closed 6 years ago

Nicholas-Cardot commented 6 years ago

Currently the filters are run inside of these conditionals. Instead, we need to move these conditionals to the inside of the functions that are being called by those filters.

if( ( $network === 'pinterest' && isset( $swp_user_options['utm_on_pins']) && true === $swp_user_options['utm_on_pins']) || $network !== 'pinterest' ):
                $array = apply_filters( 'swp_analytics' , $array );
            endif;

            // Run the link shortening hook filters, but not on Pinterest
            if( $network !== 'pinterest' ):
                $array = apply_filters( 'swp_link_shortening' , $array );
            endif;
ckmahoney commented 6 years ago

I'm not sure that I understand what you mean. For example, if I put the the conditional check in swp_link_shortener(), the call to apply_filter is then inside the function which gets called for the corresponding add_filter.

function swp_link_shortener( $array ) {
    global $swp_user_options;

    if ( $array['network'] == 'totes' ) :
        return $array;
    endif;

    // Run the link shortening hook filters, but not on Pinterest
    if( $network !== 'pinterest' ):
        $array = apply_filters( 'swp_link_shortening' , $array );
    endif; 
    . . . 
}

Am I interpreting this incorrectly @Nicholas-Cardot ?

Nicholas-Cardot commented 6 years ago

Yeah, so right now, the conditional blocks all functions that are attached to the "swp_link_shortenting" filter. Instead, the filter should run as is, and the contents of the function should be wrapped in the conditional.

from:

if ( our conditonal )
    apply filter
endif;
our funtion() {
    do some stuff;
}

to something like this:

apply filter
our function() {
   if (our conditional ) {
        do some stuff
    }
}
Nicholas-Cardot commented 6 years ago

This way we can apply that conditional on a function by function basis instead of blocking all functions on the filter by bypassing the filter altogether.

ckmahoney commented 6 years ago

Please review before closing @Nicholas-Cardot just to make sure it is as you mean. Also please verify adding return $array before the end of swp_link_shortner() and swp_google_analytics() is okay.

Nicholas-Cardot commented 6 years ago

I'll review this and test it tomorrow before closing it out.

Nicholas-Cardot commented 6 years ago

This issue should now be resolved.