[buddypress-trac] [BuddyPress Trac] #7506: friends_add_friend() should convert accepted arguments

buddypress-trac noreply at wordpress.org
Fri Apr 21 15:00:56 UTC 2017


#7506: friends_add_friend() should convert accepted arguments
--------------------------+------------------------------
 Reporter:  slaFFik       |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Friends       |     Version:
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |
--------------------------+------------------------------
Changes (by boonebgorges):

 * keywords:  dev-feedback has-patch => has-patch


Comment:

 [attachment:7506.01.patch] seems good to me.

 Using `is_numeric()` when handling `$email_or_user` seems problematic to
 me, because it's possible to have numeric user logins in WordPress. This
 is going to be a problem whenever we do "fuzzy" lookups like this. A
 better practice would probably be for `BP_Email_Recipient` to have
 something like a `set_user( $field, $value )`, which would use
 `get_user_by()` internally, and then for BuddyPress to use this
 exclusively rather than relying on the loosey-goosey checks (which we
 probably have to leave in for BC, but whose use could be discouraged
 through best practices in our own codebase). Not sure if this matches
 Paul's vision for how the email API works, so I'd let him weigh in on this
 idea, and keep the fix more localized and conservative for the time being.

 > bp_email_set_default_headers() should have $user check to prevent
 generating those notices in case user ID is a string passed to
 bp_send_email(), because $user->ID doesn't exist.

 Yes, because it's possible to send emails to non-members, right? The
 unsubscribe link should only be added to the email footer if the
 `get_user_by()` check succeeds.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7506#comment:6>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac


More information about the buddypress-trac mailing list