Take the 2-minute tour ×
WordPress Development Stack Exchange is a question and answer site for WordPress developers and administrators. It's 100% free, no registration required.

Which one is correct when adding actions/filters/etc.... Both methods work fine and I've seen people do things both ways, but I'm assuming one is correct. My gut tells me that wrapping the if statement around the action only is better but I'm not sure...

Method A: Wrapping if around add_action:

function unregister_default_wp_widgets() {
    unregister_widget('WP_Widget_Pages');
    unregister_widget('WP_Widget_Calendar');
    unregister_widget('WP_Widget_Archives');
    unregister_widget('WP_Widget_Links');
    unregister_widget('WP_Widget_Meta');
    unregister_widget('WP_Widget_Search');
    unregister_widget('WP_Widget_Text');
    unregister_widget('WP_Widget_Categories');
    unregister_widget('WP_Widget_Recent_Posts');
    unregister_widget('WP_Widget_Recent_Comments');
    unregister_widget('WP_Widget_RSS');
    unregister_widget('WP_Widget_Tag_Cloud');
    unregister_widget('WP_Nav_Menu_Widget');
}

if( !current_user_can('administrator') ) {
    add_action('widgets_init', 'unregister_default_wp_widgets', 1);
}

---- OR -----

Method B: Wrapping if around both function definition and add_action:

if( !current_user_can('administrator') ) {

    function unregister_default_wp_widgets() {
        unregister_widget('WP_Widget_Pages');
        unregister_widget('WP_Widget_Calendar');
        unregister_widget('WP_Widget_Archives');
        unregister_widget('WP_Widget_Links');
        unregister_widget('WP_Widget_Meta');
        unregister_widget('WP_Widget_Search');
        unregister_widget('WP_Widget_Text');
        unregister_widget('WP_Widget_Categories');
        unregister_widget('WP_Widget_Recent_Posts');
        unregister_widget('WP_Widget_Recent_Comments');
        unregister_widget('WP_Widget_RSS');
        unregister_widget('WP_Widget_Tag_Cloud');
        unregister_widget('WP_Nav_Menu_Widget');
    }

    add_action('widgets_init', 'unregister_default_wp_widgets', 1);
}
share|improve this question
1  
Just a quick heads up, you don't want to check against a role, but rather a specific capability (like update_plugins) with current_user_can(). –  Andrew Bartel Feb 3 at 23:23
    
I've heard that before. Do you have any examples of why? Maybe my answer on the bottom and how my site broke pertains to this.... –  willow Feb 20 at 4:25
    
Actually I tried update_plugins with my situation on my answer below, but it didn't fix it. Can you give a use case where if( !current_user_can('administrator') ) { } wouldn't work because it's far easier to manage in my opinion. Someone posted here a way around it wpbandit.com/code/check-a-users-role-in-wordpress, but I've still never seen a case where it hasn't worked. –  willow Feb 20 at 4:35
    
If you read the codex for the function it specifically says not to use a role and cites a trac ticket (core.trac.wordpress.org/ticket/22624). –  Andrew Bartel Feb 20 at 16:29
    
On line 1999 core.trac.wordpress.org/browser/tags/3.4.2/wp-includes/… it says for 'Capability or role name' –  willow Feb 22 at 1:36

3 Answers 3

up vote 5 down vote accepted

There is no real difference between the two methods.
When using method A, only the hooking is bound to the condition (i.e., the function is defined no matter what), while in method B the definition of the function as well as the hooking is.

Another thing is the following method (which you did not include):

function my_hooked_function() {
    if (! current_user_can('administrator')) {
        $some_var = 'some value';
        some_function($some_var);
    }
} // function my_hooked_function

add_action('widgets_init', 'my_hooked_function');

Since the complete condition can already be evaluated outside the function (i.e., directly in your functions.php file) method A/B should be preferred. Otherwise you would hook your function every time and for all users, while it is being used by administrators only.

There are, however, conditions, which cannot be put (i.e., evaluated) outside the hooked function, as some variable and/or object that is utilized in the condtion is not yet defined/accessible. In such a case you have to put the condition inside the function (as shown in this answer).

An example:

// NOT working
if (is_front_page())
    add_action('shutdown', function() { echo 'Front'; });

// WORKING
add_action('shutdown', function() { if (is_front_page()) echo 'Front'; });
share|improve this answer
    
+1 for your first example. I always put my conditions inside the function. –  helgatheviking Feb 3 at 23:39
    
+1 this. You should not put complex conditional evaluations outside of functions. You don't know for sure when your code runs or is loaded or what have you, and for safety, running the conditional as late as possible is generally better. –  Otto Feb 4 at 0:12
    
This was confusing at first but I came across a situation (which I posted on the bottom that made it make sense. –  willow Feb 20 at 4:26

Here's another way.

function unregister_default_wp_widgets() {
if( !current_user_can('update_core') ) {
    unregister_widget('WP_Widget_Pages');
    unregister_widget('WP_Widget_Calendar');
    unregister_widget('WP_Widget_Archives');
    unregister_widget('WP_Widget_Links');
    unregister_widget('WP_Widget_Meta');
    unregister_widget('WP_Widget_Search');
    unregister_widget('WP_Widget_Text');
    unregister_widget('WP_Widget_Categories');
    unregister_widget('WP_Widget_Recent_Posts');
    unregister_widget('WP_Widget_Recent_Comments');
    unregister_widget('WP_Widget_RSS');
    unregister_widget('WP_Widget_Tag_Cloud');
    unregister_widget('WP_Nav_Menu_Widget');
    }
}
add_action('widgets_init', 'unregister_default_wp_widgets', 1);
share|improve this answer
    
This is a better solution. Putting the condition (current_user_can) outside any scope requires the availability of that function, and it might not when you load plugin/theme. –  Rilwis Feb 5 at 3:00

Sorry for the late response but none of it really made sense to me still until I came across a situation today.

I ended up adding the following function in the mu-plugin directory that would redirect non admins to the dashboard when accidentally hitting certain pages they weren't suppose to be at.

function wp_admin_pages_redirect_to() {
  global $pagenow;
  $admin_pages = array(
                            'themes.php',
                            'edit-tags.php?taxonomy=post_tag',
                            'options-general.php',
                    );
  if(in_array($pagenow, $admin_pages)) {
    wp_redirect( admin_url('/') ); exit;
  }
}

if( !current_user_can('administrator') ) {
add_action('admin_init', 'wp_admin_pages_redirect_to');
}

However, this would give me the white screen of death. So after reading @Rilwis comment I tried the following and it worked fine.

function wp_admin_pages_redirect_to() {
if( !current_user_can('administrator') ) {
  global $pagenow;
  $admin_pages = array(
                            'themes.php',
                            'edit-tags.php?taxonomy=post_tag',
                            'options-general.php',
                    );
  if(in_array($pagenow, $admin_pages)) {
    wp_redirect( admin_url('/') ); exit;
  }
}
}
add_action('admin_init', 'wp_admin_pages_redirect_to');

Anyway, reading @Rilwis comment on Brads suggestion made me finally (kind of) realize what @t f was talking about and how certain conditions cannot be evaluated outside the function. I hoping this is what he was talking about.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.