No action for group photo upload?

[bhargavbhandari90]

We have this action for user profile picture upload function "bp_avatar_ajax_set()" into “buddypress/bp-core/bp-core-avatars.php”

do_action( 'xprofile_avatar_uploaded', (int) $avatar_data['item_id'], $avatar_data['type'] );

This action is only for user photo upload.

What if we want to add some hook for group photo..?

[boonebgorges]

Hi @bhargavbhandari90 - Thanks for the ticket! You're right that we should have an action here.

7371.diff​ introduces a new action in the AJAX and non-AJAX contexts. I don't love the action name 'groups_avatar_uploaded', but it parallels the existing user profile action.

@bhargavbhandari90 Does it seem like this would serve your purpose?

[bhargavbhandari90]

Replying to boonebgorges:

Hi @bhargavbhandari90 - Thanks for the ticket! You're right that we should have an action here.

7371.diff​ introduces a new action in the AJAX and non-AJAX contexts. I don't love the action name 'groups_avatar_uploaded', but it parallels the existing user profile action.

@bhargavbhandari90 Does it seem like this would serve your purpose?

Thank you @boonebgorges . It's good one.

But I have second thought on this.

What if we send whole array to that action instead of those two parameters only?

I am also giving my patch for this, so let me know if it's possible or not.

And I also don't like the action name 'groups_avatar_uploaded', so renamed action name is also in the patch.

[bhargavbhandari90]

Some code formatting.

[boonebgorges]

Thanks for the patch, @bhargavbhandari90. A few comments:

• I agree that it would be good to send the whole array to the action. However, we can't change the existing parameters sent to the action, as this change will break existing uses. We could add the whole array as a new parameter.\
• The new action appears in two places: groups_screen_group_admin_avatar() and bp_avatar_ajax_set(). The parameters passed to the function must be the same (or at least compatible) in each of these places.
• The name 'xgroup_avatar_uploaded' doesn't match our naming conventions. 'xprofile' is short for 'Extended Profile'. I'd prefer somethting like 'bp_groups_uploaded_avatar', but this is fairly different from 'xprofile_avatar_uploaded', so I feel like we should just go with 'groups_avatar_uploaded'.

[bhargavbhandari90]

Replying to boonebgorges:

Thanks for the patch, @bhargavbhandari90. A few comments:

• I agree that it would be good to send the whole array to the action. However, we can't change the existing parameters sent to the action, as this change will break existing uses. We could add the whole array as a new parameter.\
• The new action appears in two places: groups_screen_group_admin_avatar() and bp_avatar_ajax_set(). The parameters passed to the function must be the same (or at least compatible) in each of these places.
• The name 'xgroup_avatar_uploaded' doesn't match our naming conventions. 'xprofile' is short for 'Extended Profile'. I'd prefer somethting like 'bp_groups_uploaded_avatar', but this is fairly different from 'xprofile_avatar_uploaded', so I feel like we should just go with 'groups_avatar_uploaded'.

Ok. Agree with you. So for action name I'll go with the name 'groups_avatar_uploaded'.

And I added that third parameter which will be array as I said before. I checked that and will not make harm to existing users.

[bhargavbhandari90]

Changed action name and added new parameter

[bhargavbhandari90]

@boonebgorges I am changing the milestone. I think this should be in next version. What you say?

[boonebgorges]

Thanks for the updated patch, @bhargavbhandari90. I'll look more closely at it in the upcoming days.

The 2.7.4 release is only for security fixes and regressions. Your request is for an enhancement, so it belongs in the 2.8 milestone. 2.8 is due out in late January 2017.

[bhargavbhandari90]

Replying to boonebgorges:

Thanks for the updated patch, @bhargavbhandari90. I'll look more closely at it in the upcoming days.

The 2.7.4 release is only for security fixes and regressions. Your request is for an enhancement, so it belongs in the 2.8 milestone. 2.8 is due out in late January 2017.

@boonebgorges All right buddy. Thanks for the update. :-)

[boonebgorges]

@bhargavbhandari90 Thank you for the updated patch!

[boonebgorges]

In 11314:

Improve actions fired after avatar upload.

• Introduce the groups_avatar_uploaded action, which paralells the xprofile_avatar_uploaded filter, and runs after either an AJAX or a non-AJAX group avatar upload.
• Update instances of the xprofile_avatar_uploaded action so that they receive the array of arguments passed to the upload/crop handler.
• Improve documentation for existing actions.

Props bhargavbhandari90.
Fixes #7371.

[bhargavbhandari90]

@boonebgorges Thank you sooooooooo much.

[Offereins]

@boonebgorges I noticed your commit does not include the $avatar_data parameter in the documented version of the xprofile_avatar_uploaded action. It is in the docblock, but not in the function call.

[boonebgorges]

In 11315:

After [11314], ensure that promised data is passed to xprofile_avatar_uploaded action.

Props Offereins.
Fixes #7371.

[jdgrimes]

Also fire the action when an avatar is uploaded as part of group creation

[jdgrimes]

I was just looking for an action like this, and I stumbled upon this ticked. I realized though, that avatars can also be uploaded as part of the group creation process, and it looks like this action isn't being fired there. I think that probably this was just an oversight, so I've uploaded 7371-group-create.diff​, which fires the action in groups_action_create_group() as well.

[jdgrimes]

Fix a typo: s/atavar_data/avatar_data/

[jdgrimes]

I've also just discovered that there is a typo in bp_avatar_ajax_set(), after getting "Undefined variable: atavar_data". See 7371-typo.diff​.

[bhargavbhandari90]

Replying to jdgrimes:

I've also just discovered that there is a typo in bp_avatar_ajax_set(), after getting "Undefined variable: atavar_data". See 7371-typo.diff​.

Good one. We need to check the entire avatar system now.

[DJPaul]

The typo is only two weeks old; introduced in r11314 with the patch in this very ticket.

[djpaul]

In 11392:

Groups: fix typo in filter arg.

See #7371

Props jdgrimes

[djpaul]

In 11393:

Groups: fire groups_avatar_uploaded action after uploading avatar during group creation process.

Fixes #7371

Props jdgrimes