@mercime @hnla
Sorry I posted in the wrong place, can you kindly move it to creating and extending.
Thanks!
Even though your plugin is BP related, it might be worth also posting the question in:
https://wordpress.org/support/forum/plugins-and-hacks
Your reach will be extended to the WP plugin dev community
But you haven’t really run a check to see if bp is loaded, at least not in the loader file which is why I think you have that response ( although the response does direct you to our codex entry showing the approach you have followed, so that doesn’t really make sense it smacks of an automated response or worse someone just zapping off the message without thinking? ), you simply require your main file without any checks then do a add_action on bp_include, in your main file you do use that function I’ve seen before to try and require the bp-loader and if not post a message which is fine it works but I wonder whether it’s better to move all your checks to your main loader file?
I have to admit I’ve always puzzled the best approach, rather annoyingly you can check half a dozen of the better BP developers and they all do something different, I would favour a consensus as to the best approach and have that written up as a best practice to follow in the codex – personally I’ve tried a variety of approaches all work, on my one wp repo plugin I used same loader.php approach you have requiring and add_actioning on bp_include hook.
Also originally the check was not so much to prevent ones plugin from loading but to prevent white screens if BP was deactivated with your plugin still running.
@hnla
I just checked using the advice you have given and moved the check to the main loader file and for some reason it does make a difference because now the message is displayed as expected if BP is not installed so I’ll consider this the best practice in the future. I wonder if I should contact someone that does the plugin reviews so they can re-check it now that its up to par?
Thanks again for your advice Hugo! Thank you as well @henrywright-1 !
It all depends on how the plugin functions. if you have WP options saved and then use bp_include to load plugin your settings won’t load fast enough for BuddyPress to use options.
bp_include is used for code that accesses BuddyPress code. So you know that BuddyPress is loaded before you execute code. It should not be used to detect if BuddyPress is active.
you can use https://codex.wordpress.org/Function_Reference/is_plugin_active
or check if ( class_exists( ‘BuddyPress’ ) )
Well think the point here is going to be someone sorting out that codex entry then with a more complete and in depth examination of the check and load process, especially given people are receiving messages from WP repo pointing to the original post stating to use bp_loaded.
@hnla @mercime @modemlooper
I agree there needs sorting out and consistency in how to check for BP etc.
I just got an email from Mika E one of the plugin reviewers and she says
But … why?
BuddyPress has loader actions you can hook into: https://plugins.trac.wordpress.org/browser/buddypress/trunk/bp-core/bp-core-actions.php#L34
(I’m sitting next to JJJ right now and he pointed this out). So you don’t have to call loader.php, which is slower in the long run, and instead you hook into BP at the appropriate point.
The whole thing it looks like you’re doing is adding this action:
add_action( ‘admin_notices’, ‘private_community_for_bp_lite_install_buddypress_notice’ );
And you want to only add that notice IF BuddyPress is loaded, right? So why not the smaller check? If you do that, you won’t have to call bp-loader, because you’re hooking in after it’s loaded.
Why would I want my install buddypress notice to display if they have buddypress installed and the requirement for BP is met? That makes no sense. Right now it is setup to display only if someone using wordpress does not have BP installed and activated which is what it does and should do correct?
I don’t get it! Any of you guys have advice?
Thanks again!
/*** Make sure BuddyPress is loaded ********************************/
if ( class_exists( 'BuddyPress' ) ) {
add_action( 'bp_include', 'private_community_for_bp_lite_init' );
} else {
add_action( 'admin_notices', 'private_community_for_bp_lite_install_buddypress_notice' );
}
function private_community_for_bp_lite_init() {
require( dirname( __FILE__ ) . '/private-community-for-bp-lite.php' );
}
function private_community_for_bp_lite_install_buddypress_notice() {
echo '<div id="message" class="error fade"><p style="line-height: 150%">';
_e('<strong>Private Community For BP Lite</strong></a> requires the BuddyPress plugin to work. Please <a href="https://buddypress.org/download">install BuddyPress</a> first, or <a href="plugins.php">deactivate Private Community For BP Lite</a>.');
echo '</p></div>';
}
@modemlooper
So is this a new requirement to get a plugin approved to the repository? It would be nice to have this in the codex as a reference in the future if so. Thanks!
not a requirement. my suggestion is just another possible way to check if BP is active.
@modemlooper
Yeah it pretty much does the same thing in a simpler form, but hopefully they will approve it as I took your advice. I really don’t see why my plugin is being scrutinized because there is lots of simpler, and even more complex BP plugins on the repo that doesn’t check for BP, or give a notice that BP is required for it to work. Some of the plugins are basically just a function with an add action or filter hook with the required plugin header. Maybe the standards are being raised which I suppose is a good thing. Anyway thank you for the advice given.
Mika and John were just pointing out you didn’t need to load that extra file to check for BuddyPress. The code you were using has been copy & pasted over the years from some early plugins.
@modemlooper
What extra file? Are you referring to the loader.php? If that is the case then the example you provided above makes no sense. Why would I require private-community-for-bp-lite.php? Are you suggesting I do away with the loader.php and add the example you provided in the main plugins private-community-for-bp-lite.php file as well as the required plugin header info or am I missing something? This is different than my usual workflow so it seems strange to me. I guess you can require a file from within a file but doesn’t it seem redundant? I really do not see what difference it makes. The plugin works as expected either way. So the hold up with the review panel is kinda tedious. Is this some new way of streamlining plugins for consistency or am I missing something that is required? To me it seems any advantages to this approach you offered are negligible and it apparently still has not made any difference in the approval status. Keep in mind this is the first plugin I have released to the WP repo because I am still new to adding the admin settings but what I have works so far and saves the settings to the database so I don’t see what the problem is! Thanks again!
this: require_once( ABSPATH . ‘/wp-admin/includes/plugin.php’ );
The review proccess can and will point out better coding practices. If they see a cleaner way to write it then they will say something. If you ever submit a theme for review you will have this happen 10 fold.
The plugin review team is actually easy on the reviews.
In other words the function that essentially attempted to call into itself other core files from WP core & BP core for no real good reason, I’ve seen that function before as well and puzzled as to why someone thought that was necessary or good.
The fundamental approach of loader.php as the top level file still holds good as common and best practise, with your header,main checks, language local, and any functions to run on activation or deactivation etc.
@modemlooper @hnla
Thank you guys for pointing that out. I will start using this as a standard practice in the future. I still believe there needs to be concise documentation in the codex so other new developers can follow these practices and to be consistent and it should really come from someone with more experience when they have time to update it. I really appreciate all the invaluable advice you have given.
Thanks a million guys, you rock!
You can also wrap everything in a class. Look at bp-loader.php to get an idea of how to do that. I’m guilty of not following the best way out of laziness but I always go over my code and try and perfect as much as possible. I look at how BuddyPress core code is written and look over others plugins that I feel are doing it right.
@modemlooper
Yes I am familiar with wrapping everything in a class. I have used this method in a small plugin I have on github. I appreciate the advice again!
P.S. I have your site listed on my blog as a place to get awesome premium BP plugins so I hope you don’t mind. http://bphelpblog.wordpress.com/
Actually just wrap notice check in if class because bp_include won’t run if bp isn’t active.
add_action( ‘bp_include’, ‘private_community_for_bp_lite_init’ );
function private_community_for_bp_lite_bp_check() {
if ( !class_exists( 'BuddyPress' ) ) {
add_action( 'admin_notices', 'private_community_for_bp_lite_install_buddypress_notice' );
}
}
add_action('plugins_loaded', 'private_community_for_bp_lite_bp_check', 999);
@modemlooper
I see your point but I don’t really see the point of over-complicating this very simple plugin without a valid reason. The functionality is there so I think it should be left as is unless you have an argument with some valid points to make me understand it a little better. Either way I really do appreciate your input and I am not saying that method is not good, I just want to know how it really benefits the end user or the difference this would make? Thanks again!
@bphelp
Note to myself:
Never dabble in building themes, because 10 fold sounds excruciating! LOL!
The reason of my last suggestion is you can not test if BuddyPress class if you do not call it after plugins_loaded. Previous code doesn’t work right because plugins load in the order they are listed in the admin. If your plugin is above BuddyPress then the BP class hasn’t loaded and the error notice will show even if BuddyPress is active. The last code fixes that by running function after all plugins are loaded.
@modemlooper
I have tested the plugin:
https://github.com/bphelp/private_community_for_bp_lite
As is it works as expected. If BuddyPress is installed and activated then the notice does not appear, however if you try installing the plugin without BuddyPress installed and activated then the notice appears. Can you test it so you will see what I mean because it simply works!
You really need to wrap the if class in a function that loads after all plugins just to be on the safe side. The reason it works is because your plugin name is alphabetically after BuddyPress so it loads after but the proper way is to wait until all plugins are loaded and then give the error notice.
@modemlooper
I probably will but this shouldn’t be a requirement especially when people fart out little dinky plugins and they don’t get half of the hassle I have gotten from the review panel. I don’t get why they are telling me how to code my plugin. It is my choice how to to code it and as long as it works and does not contain malicious code then it should be approved. I think I have made a valid point! Is there some high expectation of a plugin that does what mine does because conceptually it was pretty simple?
