Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 6 years ago

#4333 closed enhancement (maybelater)

Allow User avatar functions to pass through arguments to bp_core_fetch_avatar

Reported by: cklosows's profile cklosows Owned by:
Milestone: Priority: normal
Severity: normal Version: 1.6
Component: Core Keywords: needs-refresh, trac-tidy-2018
Cc: kurtpayne

Description

If an argument is passed to bp_loggedin_user_avatar that is accepted by bp_core_fetch_avatar, it is not passed through to bp_core_fetch_avatar.

bp_get_loggedin_user_avatar and bp_get_displayed_user_avatar accepts the following arguments:

'type' => 'thumb',
'width' => false,
'height' => false,
'html' => true,
'alt' => ( 'Profile picture of %s', 'buddypress' )

bp_core_fetch_avatar accepts the following arguments:

'item_id' => false,
'object' => $def_object, user/group/blog/custom type (if you use filters)
'type' => $def_type,
thumb or full
'avatar_dir' => false, Specify a custom avatar directory for your object
'width' => false,
Custom width (int)
'height' => false, Custom height (int)
'class' => $def_class,
Custom <img> class (string)
'css_id' => false, Custom <img> ID (string)
'alt' => $def_alt,
Custom <img> alt (string)
'email' => false, Pass the user email (for gravatar) to prevent querying the DB for it
'no_grav' => false,
If there is no avatar found, return false instead of a grav?
'html' => true, Wrap the return img URL in <img />
'title' =>
Custom <img> title (string)

If I were to send 'no_grav' => true as an argument for bp_loggedin_user_avatar it would not send this to bp_core_fetch_avatar due to hard coded values in the 'bp_get_loggedin_user_avatar' filter call.

I think we should send any arguments through to bp_core_fetch_avatar that are supplied to bp_loggedin_user_avatar, while keeping the defaults for bp_loggedin_user_avatar in place.

Attachments (7)

4333.diff (2.2 KB) - added by cklosows 12 years ago.
Removes hard coded arguments for bp_core_fetch_avatar and uses parsed arguments given.
4333.02.diff (4.6 KB) - added by cnorris23 12 years ago.
4333.2.diff (4.6 KB) - added by cklosows 12 years ago.
Correcting syntax issues in 4333.02.diff
4333.03.diff (5.4 KB) - added by cnorris23 12 years ago.
Add note about 'item_id' and 'email' args being overwritten
4333.04.patch (71.4 KB) - added by cnorris23 11 years ago.
4333.04.2.patch (71.4 KB) - added by cnorris23 11 years ago.
4333.05.patch (72.6 KB) - added by cnorris23 11 years ago.

Download all attachments as: .zip

Change History (28)

@cklosows
12 years ago

Removes hard coded arguments for bp_core_fetch_avatar and uses parsed arguments given.

#1 @kurtpayne
12 years ago

  • Cc kurtpayne added

#2 @cnorris23
12 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

I could argue both why we should/shouldn't do this, but since the bp_(loggedin/displayed)_user_avatar functions are just user specific wrappers for bp_core_fetch_avatar, it makes sense that they should accept the same arguments. However, the patch as it stands, won't work. Rather than being user specific functions, they're not just pass-through functions for bp_core_fetch_avatar. The 'item_id' param needs to be hard coded at a minimum. I'll put up a more comprehensive patch in a few hours.

@cnorris23
12 years ago

#3 @cnorris23
12 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Attached patch allows all bp_core_fetch_avatar() arguments to be passed through (except of course for the user specific args). Removes unnecessary default args, as they are the same as bp_core_fetch_avater(). Adds bp_(get_)loggedin_user_email() functions, so that bp_(get_)displayed_user_email() functions aren't so lonely. Also PHPDoc.

#4 @cklosows
12 years ago

There are syntax issues with your patch when setting the item_id and email elements of the $r array.

$r = ['item_id'] = bp_loggedin_user_id();
$r = ['email']   = bp_get_loggedin_user_email();
$r = ['item_id'] = bp_displayed_user_id();
$r = ['email']   = bp_get_displayed_user_email();

I've corrected and this seems to work.

@cklosows
12 years ago

Correcting syntax issues in 4333.02.diff

#5 follow-up: @cnorris23
12 years ago

Good catch. Not even sure how that happened.

#6 in reply to: ↑ 5 @cklosows
12 years ago

Replying to cnorris23:

Good catch. Not even sure how that happened.

Probably just staring at the glowing screens too long like the rest of us ;).

This seems to work well and be a sufficient replacement for passing through the arguments. I'll try and do an audit on things calling bp_core_fetch_avatar and see if we can make any other improvements before having this move forward.

@cnorris23
12 years ago

Add note about 'item_id' and 'email' args being overwritten

#7 follow-up: @cnorris23
12 years ago

  • Keywords dev-feedback added

Replying to cklosows:

This seems to work well and be a sufficient replacement for passing through the arguments. I'll try and do an audit on things calling bp_core_fetch_avatar and see if we can make any other improvements before having this move forward.

Apparently we had the same idea, because that's what I just got finished doing. This is an "issue" in a number of places. Before we go digging around and changing what adds up to be 10+ functions, in addition to the ones already patched, we should probably wait for one of the devs to weigh in.

#8 in reply to: ↑ 7 @cklosows
12 years ago

Yeah I found the same thing. Agreed on the dev feedback after seeing this.

#9 @boonebgorges
12 years ago

  • Component changed from Members to Core
  • Keywords 1.7-early added
  • Milestone changed from Awaiting Review to Future Release

I don't have a problem with this. Go ahead and write up the patch for the rest of the relevant functions (group avatar functions, etc).

#10 @boonebgorges
12 years ago

  • Keywords 1.7-early removed
  • Milestone changed from Future Release to 1.7

#11 @johnjamesjacoby
11 years ago

  • Milestone changed from 1.7 to 1.8

Fine by me. Let's tie up the loose ends and get it in. Punting to 1.8 for now, feel free to move back to 1.7 if you're compelled to patch right away.

#12 follow-up: @cnorris23
11 years ago

  • Keywords needs-patch added; has-patch removed

I'll have a more comprehensive patch up tonight.

#13 in reply to: ↑ 12 @cklosows
11 years ago

Replying to cnorris23:

I'll have a more comprehensive patch up tonight.

Sounds like a plan. Will review and test once complete.

@cnorris23
11 years ago

@cnorris23
11 years ago

#14 @cnorris23
11 years ago

  • Keywords has-patch added

First off, ignore the 4333.04 varieties. Guess I got a little trigger happy with the agree and upload button, so it's a duplicate upload, plus they won't apply correctly for some reason.

Attached patch includes the following:

1) allows relevant functions to accept all bp_core_fetch_avatar arguments (except those specific to the function's, uh, function)
2) removes use of extract (TODO: bp_core_fetch_avatar(). Big under taking and I need to get some shut eye)
3) removes redeclaration of default bp_core_fetch_avatar arguments where possible

I couldn't get any errors to appear, but it was also limited testing. Will definitely need some eyes.

#15 @cnorris23
11 years ago

  • Keywords needs-patch removed

#16 @boonebgorges
11 years ago

  • Keywords needs-refresh added; needs-testing dev-feedback removed
  • Type changed from defect (bug) to enhancement

Hey cnorris23 - I like most of the stuff in this patch, but it's kind of a mess because there's so much going on. Could we get separate patches for your 1, 2, and 3? The original issue of this ticket was 1, I believe, and should just require a patch a line or two long.

#17 @boonebgorges
11 years ago

  • Milestone changed from 1.8 to Future Release

#18 @johnjamesjacoby
9 years ago

  • Version changed from 1.6-beta to 1.6

#19 @DJPaul
9 years ago

  • Keywords has-patch removed

#20 @DJPaul
6 years ago

  • Keywords trac-tidy-2018 added

We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.

Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.

If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.

For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/

#21 @DJPaul
6 years ago

  • Milestone Awaiting Contributions deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.