New theme structure doesn't work with parent theme not being a BP framework

[Detective]

In my local installation of BuddyPress I use the following theme structure:

• Parent theme: a regular WP theme.
• Child theme: a custom BP framework.

So, the child theme is like the "member theme" of the previous versions: it just contains what the parent theme doesn't have to be BP-compliant.

If I use the default BP theme, when viewing a group page the following is the value of $bp_path in the function bp_core_do_catch_uri:

string(18) "groups/single/home" 

However, if I use the setup commented at the beginning, the following is the value of $bp_path:

string(17) "groups/group-home"

[apeatling]

This is because your BP framework does not have the new group template files. If you include the template files in /groups/single/ from the default theme this will change.

[Detective]

I forgot to say: when I tested this, I renamed my groups folder to old-groups and copied the groups folder from the bp-parent theme. And before I had tested moving my own files to the new locations (inside single/ with their new names).

I don't understand enough BP to find where $bp_path is set. If you give me some pointer I'll try to make a patch.

Thanks.

[Jason_JM]

Replying to Detective:

I forgot to say: when I tested this, I renamed my groups folder to old-groups and copied the groups folder from the bp-parent theme. And before I had tested moving my own files to the new locations (inside single/ with their new names).

I don't understand enough BP to find where $bp_path is set. If you give me some pointer I'll try to make a patch.

Thanks.

Please:

• Use the default templates, assure default works first.
• Copy the default template, modify as necessary.
• You are either missing files, or your template file is not defined correctly.
• Please evaluate the template file and compare to the default.

This will be flagged 'works for me' for now. If your still having issues, provide more information and re-open, but please only after you try the recommended, provide some extra info (like files). Attach your files so that they can be evaluated, it's a little hard to debug something with no info on this side.

About $bp_path:

With $template being a specific template file
*bp_core_load_template(...) is called, accepting and passing $template as $pages to bp_catch_uri.

->calls bp_catch_uri(...) is called, accepting $template as $pages and tries to load the first template file that can be found.

->It is here that $bp_path gets set:

$bp_path = $pages;

[Detective]

Well, the default templates work ok, this ticket is about a problem using a custom bp framework.

I found the problem. I'll provide a patch this weekend (here in Chile we're on holydays :) Independence Day(s) ). However, if someone want to pull this earlier, better :)

This is the problem:

function groups_screen_group_home() {
   // ...
   if ( file_exists( TEMPLATEPATH . '/groups/single/home.php' ) )
      bp_core_load_template( apply_filters( 'groups_template_group_home', 'groups/single/home' ) );
   else
      bp_core_load_template( apply_filters( 'groups_template_group_home', 'groups/group-home' ) );
}

bp_core_load_template calls locate_template to load the right file. This works ok almost always, except when the template files have new names, because the existance of the new template filename is being checked only on the parent theme by using TEMPLATEPATH. Since in this setup the parent theme doesn't contain that file, the old filename is passed to bp_core_load_template.

A simple solution is to replace all these instances with something like this:

$located = locate_template( array( 'groups/single/home.php' ), false );
if ( $located != '' )
   bp_core_load_template( apply_filters( 'groups_template_group_home', 'groups/single/home' ) );
else
   bp_core_load_template( apply_filters( 'groups_template_group_home', 'groups/group-home' ) );

This duplicates some work (the filesystem is searched twice for the correct template) but it's easier to code and more future proof (since in a few versions the old themes will be discarded, the only modification needed is to delete "the else branch".

[Detective]

patch attached. I tested it locally both with the default theme and my custom bp framework, it seems to work ok.

[Jason_JM]

I would re-do the string comparisons the other way around.

( "string" != Some.other.member.accessor.data() )

Keep things consistent with the code.

add the has-patch, needs-testing keywords when done

I'll try to hop on it for you

[Detective]

I don't agree. If you search for "!=" in the BuddyPress code you'll find that the prominent string comparison seems to be the opposite of what you propose. Even in the same file you can find both ... example:

bp-xprofile-classes.php

if ( $message != '' ) {

if ( '' != $option_value ) { 

if ( $this->exists() && $this->value != '' ) {

I don't think it's worth to rewrite the patch just because you think it lacks code styling consistency.

I still propose the original patch, since in my setup it works ok (I have kept testing my bp framework extensively with the patch applied, always comparing with tbe standard bp framework).

[Jason_JM]

Ok. Testing now. Should be done in a bit. I have about 30min from now to test, so I'll post up what I've tested.

I don't want you to think i'm being harsh or anything. We're all helping out, this is good stuff. One step at a time, we're all here to help to make that lean mean buddy pressing machine. oh ya!

Concerning the conventions, BP may not fully be safe checking everything and I'm sorry that I've used that as an example. The only perfect code I've ever seen was Windows Vista RC1. JUST KIDDING! But It's more of a type-safety issue.

Though in the code it looks like we always have control over things like ($message) you really never know what can pop in there. Run-time errors, security issues, etc. With starting out using the strict string we are assured an object to pivot the comparison with. So if $message say has some nasty garbage, it will always fail. No need for any exception monitoring, etc. Now the likely hood of $message really ever having garbage? Who knows. But it's the practice that we should use, so when there are times it may matter things won't be so bad. Hopefully...

[Detective]

English isn't my first language so sometimes I sound angry or something like that :p I'm sorry it my message looks harsh or insulting.

I think it's best to just use !empty( locate_template... ). But since the old theme support will disappear in future versions, this code will vanish, so maybe it's pointless to argue about the truly perfect solution.

Regards,

Eduardo

[Jason_JM]

Your right. It's pointless.

I have tested only (no time remaining, must run out) the following. It looks ok so far to me:
bp-groups/bp-groups-classes.php
bp-blogs/deprecated/bp-blogs-deprecated.php
bp-core/bp-core-signup.php
bp-blogs/admin-tabs/bp-blogs-comments-tab.php
bp-groups/deprecated/bp-groups-deprecated.php

[Jason_JM]

Anyone else want finish testing? It's almost done!

[apeatling]

The style of your patch is important, just because all of BuddyPress does not yet adhere to the WordPress coding standards does not mean patches don't need to either. I'm doing my best to fix areas of code up, but good patches will help that process.

[apeatling]

(In [1906]) Fixes #997 props Detective, Jason_JM