Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 5 years ago

#5202 closed enhancement (fixed)

Create a new Group Avatar outside of the Group via PHP

Reported by: svenl77 Owned by: imath
Milestone: 2.4 Priority: normal
Severity: normal Version:
Component: Groups Keywords: has-patch commit
Cc: imath

Description

Right now it is not possible to create a new avatar for a group easy via php.

I have created a workaround function to solve this problem. But I think, there should be something like this in core.

I have attached the function as php file. I'm new to trac, if submitting a function is done in another way, please let me know.

Attachments (4)

set_group_avatar_by_group_id.php (1.7 KB) - added by svenl77 7 years ago.
set_group_avatar_by_group_id
5202.patch (2.8 KB) - added by imath 5 years ago.
5202.02.patch (3.3 KB) - added by imath 5 years ago.
missing crop_w/crop_h arguments
5202.03.patch (5.2 KB) - added by imath 5 years ago.

Download all attachments as: .zip

Change History (20)

@svenl77
7 years ago

set_group_avatar_by_group_id

#1 @boonebgorges
7 years ago

  • Keywords needs-refresh added; dev-feedback removed
  • Milestone changed from Awaiting Review to Future Release

svenl77 and I talked about and worked through this problem a bit at WCEU. I agree it's very problematic that there's no simple, procedural way to set an avatar for a group or a user.

The crux of the issue is that our avatar upload handler function relies on wp_handle_upload(), which itself calls wp_upload_dir(). We need to filter the output of wp_upload_dir() dynamically, since we store in different directories for different user/group IDs. The problem is that, the way that WordPress filters work, the filtering has to happen in a separate callback function. And that callback function has to have *some* way of figuring out what the intended group/user ID is. The method we're currently using is to assume that the avatar is intended for the *current* group/user, information that is available in the callback function by (eventual) reference to the $bp global. (If WP filters were set up in such a way that we could pass parameters to callbacks at the time that we called add_filter(), we could solve this ticket by passing the group ID manually to the wp_upload_dir filter callback.)

svenl77, the solution you have proposed in set_group_avatar_by_group_id.php is good enough for use in a plugin, but we can't adopt it for BP core. It's dangerous to be manually modifying superglobals like $_POST and $_FILES, because of possible name conflicts/overwrite (among other reasons). Likewise, it's potentially problematic to change the value of $bp->groups->current_group manually - in part, because you might want to change the avatar of one group while looking at another group's page, and making the change you suggest would mess up the *actual* current group.

A more effective solution in BuddyPress would be to create a new global key where we could temporarily stash the avatar item id/type, so that it'll be accessible later on for the wp_upload_dir filter. Something like:

function bp_core_upload_avatar_to_item( $avatar, $item_id, $item_type ) {
    if ( empty( buddypress()->avatar_admin ) ) {
        buddypress()->avatar_admin = new stdClass;
    }

    buddypress()->avatar_admin->item_id = $item_id;
    buddypress()->avatar_admin->item_type = $item_type; // 'user', 'group'

    add_filter( 'upload_dir', 'bp_core_avatar_dir_filter', 10, 0 );

    wp_handle_upload( $avatar, array( 'action' => 'bp_avatar_upload' ) );

    remove_filter( 'upload_dir', 'bp_core_avatar_dir_filter', 10, 0 );
}

function bp_core_avatar_dir_filter() {
    $item_id = buddypress()->avatar_admin->item_id;
    $item_type = buddypress()->avatar_admin->item_type;

    $subdir = 'group' == $item_type ? 'group-avatars' : 'avatars';
    $path = bp_core_avatar_upload_path() . '/' . $subdir . '/' . $item_id;

    // etc
}

This is all off the top of my head, and some different error-checking etc will probably be required, but it gives an idea of a safer method to do what's being suggested here.

Ideally, once we had something like this in place, we would refactor our existing avatar upload process so that it uses the new functions. Also, it should be applied to users as well as groups.

#2 @DJPaul
5 years ago

  • Cc imath added

@imath can you take a look at this since you're the expert? I am talking to Sven about it at wceu :D

#3 @imath
5 years ago

@DJPaul @svenl77

We could use this kind of mechanism > https://gist.github.com/imath/91f2826cc9910f28b80e

#4 @imath
5 years ago

  • Keywords needs-refresh removed

If as discussed in slack, we only need to set an avatar for an object given an absolute path to an image, it's easier to achieve see 5202.patch.

@imath
5 years ago

@imath
5 years ago

missing crop_w/crop_h arguments

#5 @Jessy Marco
5 years ago

Will you implement this into next release?

Many thanks.

#6 @imath
5 years ago

Jessy Marco, i hope so :) It would be very useful for blog avatars, see #6544

#7 @DJPaul
5 years ago

  • Keywords needs-testing added
  • Milestone changed from Future Release to 2.4

Let's do it.

#8 @svenl77
5 years ago

Hi @imath, I use your patch in a plugin with a prefix to create group avatars from featured image until your patch gets into core ;)

I run into an issue with file_exists.

I needed to remove the

! file_exists( $r['image'] )
Line 1269
 if ( empty( $r['item_id'] ) || ! file_exists( $r['image'] ) || ! @getimagesize( $r['image'] ) ) { 

change to 
if ( empty( $r['item_id'] ) || ! @getimagesize( $r['image'] ) ) {

after I removed this check all is working.

#9 @imath
5 years ago

Hi @svenl77

I've just checked your plugin, and the reason is you are sending an url when we expect an absolute path. The docblock is mentioning it:
@type string $image The absolute path to the image

I think it's justified/safer to check if the file exists before trying to copy it in another place.

I've tried to find a WordPress function to get the attachment path, and it seems to only exist for thumbnails > wp_get_attachment_thumb_file(). Maybe you can get some inspiration out of this function and get the absolute path to the 'post-thumbnail' size before using the 'copy' function.

It's weird it's working with an url though... I need to understand why as PHP's copy function requires a path.

#10 @imath
5 years ago

Ah! it seems you can use an url with copy. That's why it was working with an url :)

#11 @svenl77
5 years ago

Hi @imath

thanks for the quick response and checking my code ;)

Yes I understand the issue. I could use get_attached_file() but than the image would be in original size and the cropped part would display just a small peace of the complete image.

I do check if the image exists before I call the copy function.

Many thanks for your help and make me understand the reason.

#12 @imath
5 years ago

You're welcome, i've commented on your plugin's commit about it and shared a gist containing a function to get the thumbnail file path and fall back to original one.

Because I think you cannot be sure there Will Be a thumbnail size on the site your plugin is activated. Believe me ;)

#13 @imath
5 years ago

mm if we need to create avatars this way, i can imagine the same need will concern Cover Images ? Am i wrong ?

#14 @imath
5 years ago

  • Keywords commit added; needs-testing removed

In 5202.03.patch:

  • I've improved a bit the function and "move it" at the BP Attachments level.
  • It's now possible to set an avatar or a cover image for a user or a group.
  • Also it's now making sure the image type is allowed for avatars or cover images.

I think we're good to go :)

If no objections will commit it tonight.

@imath
5 years ago

#15 @DJPaul
5 years ago

looks ok

#16 @imath
5 years ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 10193:

Introduce a new function to generate an Avatar or a Cover Image for a given User or Group.

You can now use the absolute path to an existing image of your WordPress site to dynamically set the avatar or the cover image of a Group or a User.

Props svenl77, boonebgorges, DJPaul

Fixes #5202

Note: See TracTickets for help on using tickets.