Opened 11 years ago
Closed 9 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)
Change History (20)
#1
@
11 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
@
9 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
@
9 years ago
@DJPaul @svenl77
We could use this kind of mechanism > https://gist.github.com/imath/91f2826cc9910f28b80e
#4
@
9 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.
#7
@
9 years ago
- Keywords needs-testing added
- Milestone changed from Future Release to 2.4
Let's do it.
#8
@
9 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
@
9 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
@
9 years ago
Ah! it seems you can use an url with copy
. That's why it was working with an url :)
#11
@
9 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
@
9 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
@
9 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
@
9 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.
set_group_avatar_by_group_id