bp_get_group_avatar() needs to work outside the loop

[slaFFik]

Currently bp_get_group_avatar() function doesn't check $groups_template global var, although using it extensively.
If you try to use this function not in a loop scope - it will generate lots of notices.

It makes it impossible to use outside of the loop more relevant bp_get_group_avatar() comparing to bp_core_fetch_avatar().

[boonebgorges]

I agree that there should be a way to get a group avatar outside of a group loop.

I'm not 100% sure that bp_get_group_avatar() is the way to do it. Our -template.php functions are pretty inconsistent throughout BuddyPress. In some places, these functions accept an argument so that they can be used outside of the loop, and in other places they do not. Many of the bp-groups-template.php functions, for example, accept a $group object. It's easy for us to add this functionality to bp_get_group_avatar(), but this might also be a good time to take a moment and see whether we'd like to take a step toward greater consistency here. What if, for example, we introduced a function like this:

function bp_get_group_from_param( $group = false ) {
    global $groups_template;

    $group_obj = null;

    if ( $group instanceof BP_Groups_Group ) { // or is_object()
        $group_obj = $group;
    } elseif ( is_numeric( $group ) ) {
        $group_obj = groups_get_group( array( 'group_id' => $group ) );
        // maybe some error checking to make sure the group exists
    } elseif ( isset( $groups_template->group ) ) {
        $group_obj = $groups_template->group;
    }

    return $group_obj;
}

Then, within each template function:

function bp_get_group_avatar( $group = false ) {
    if ( ! $group = bp_get_group_from_param( $group ) ) {
        return '';
    }

    // Now we can assume $group is correct
}

This way, devs have the option of passing a group object, passing a group ID, or passing nothing and having the group determined from the group loop context.

[r-a-y]

The bp_get_group_from_param() function would be pretty awesome.

Also, check out a related, crappy patch here - ticket:6387#comment:6 - that only adds groups_get_current_group() as a fallback and doesn't do any group lookups.

[espellcaste]

I like @boonebgorges suggestions and I'll work on a patch. On the BP REST API and BP CLI, I opted to use bp_core_fetch_avatar precisely because I noticed you could not use bp_get_group_avatar properly outside of the loop.

[espellcaste]

This is the first patch, of a few more I'm sure, introducing the new helper function, bp_get_group_from_param, to grab a group using one of the identifiers: slug, ID, BP_Groups_Group which defaults to the current group in the BP_Groups_Template loop.

This is a preliminary patch which accounts to:

• creating the helper function
• updating a few functions to use this new function (more needs to be updated)
• adding basic support to unit tests (support will be improved in future patches)

Some notes:

• PHPDoc was updated to remove references of getting the ID from the current loop;
• bp_group_is_visible was updated to check against a specific user ID;
• bp_get_group_avatar was updated to pass other parameter to bp_core_fetch_avatar which were missing.

The goal is to gather preliminary feedback before diving even deeper. =P

[imath]

Hi @espellcaste

Thanks a lot for your work and patch about this improvement. I'll look at it in detail asap. Here's some first thoughts:

• what about bp_get_group_by( $field, $value ) like the WP get_user_by() function?
• It seems weird to have the from_param suffix and being able to pass the entire Group object. I would probably have used a bp_get_group( $group_id_or_group_object ) and used bp_get_group_by( 'id', $value ) function inside it in case a Group ID was passed.

I've changed the version to 1.0 as this was the BuddyPress version when bp_get_group_avatar() was introduced.

[espellcaste]

I think those are good suggestions, particularly the bp_get_group_by one (other objects/components would benefit from having similar instances).

I'll update the patch while the rest of the diff is reviewed. :)

[espellcaste]

Second patch with the code suggestions applied. Minor tweaks were made to the PHPDoc and unit tests. :)

[imath]

Hi @espellcaste

Thanks a lot for your work on this second patch. It looks nice, you'll find some suggestions mainly about DocBlocks into the patch I suggest to use to compliment yours below.

NB: the patch also fixes an issue with how you edited bp_get_group_avatar(): the html argument needs to default to true otherwise the avatar image is not output, and we get instead the avatar URL (which can be retrieved using bp_get_group_avatar_url().

Once you've fixed the bp_get_group_avatar() issue and If you're confident with the patch, feel free to commit it.

[espellcaste]

In 13085:

We are introducing in this commit two new helper functions to get a group: bp_get_group_by and bp_get_group. The former allows for getting a group by id/ID or slug.
And the latter allows for getting a group by id/ID, group object, slug. And when used in the context of a Group loop built by the BP_Groups_Template class, it defaults to the Group being iterated on.

• The new helper functions were applied to several functions in the Group component.
• bp_group_is_visible was updated to check against a specific user ID. Previously, only the current logged in user was verified.
• bp_get_group_avatar was updated to use the new functions and new arguments that are passed to bp_core_fetch_avatar were added.
• PHPDoc were updated to reflect those changes.

Props imath, boonebgorges and DJPaul

Fixes #6749 (trunk)

[espellcaste]

I mistakenly fixed this ticket where I had other patches to send.

[espellcaste]

6749-4.diff is a follow up patch. For the purposes of this ticket, that's the only patches I'm gonna suggest. :)

[espellcaste]

In 13097:

Adding the newly created helper functions bp_get_group_by and bp_get_group for Groups related functions
and template files.

Also, adding several PHP fixes/tweaks. And unit tests.

Fixes #6749

[espellcaste]

In 13103:

Improving the group member count routine and the function helper.

The group member count routine was updated to avoid direct, uncached, SQL query and unnecessary cache refresh when a group's Members page was viewed.

is now being used to get the group member count which takes into account users' existence in the site,

the query is now cached and filterable.

was also updated to get the current group from if available.

Props imath
Fixes #7614 and see #6749