[buddypress-trac] [BuddyPress Trac] #6932: Emails: real unsubscribe functionality

[buddypress-trac]

#6932: Emails: real unsubscribe functionality
----------------------------------------+---------------------------
 Reporter:  DJPaul                      |       Owner:  tharsheblows
     Type:  enhancement                 |      Status:  assigned
 Priority:  normal                      |   Milestone:  2.6
Component:  API - Emails                |     Version:
 Severity:  normal                      |  Resolution:
 Keywords:  has-patch needs-unit-tests  |
----------------------------------------+---------------------------

Comment (by tharsheblows):

 Here is the next version. Except for the things below, the mistakes are
 typos or complete misunderstandings, so if you could, please point them
 out explicitly again!

 >It looks like you removed $link from the signature of
 bp_email_get_unsubscribe_link(), but then didn't fix the function :)
 Putting all the paramaters into a single $args array - 'link' (or maybe
 better 'redirect_to'), 'notification_type', 'user_id' - seems better to me
 than separating them out into separate arguments.

 I've gone back and forth on where this should be. It was in the schema
 (not explicitly but you could filter it in) but I think that was way too
 confusing and I hadn't documented it (even when I tried it was still too
 confusing) so it's back in the individual functions with a default of the
 activity directory. It's better there anyway because it has access to the
 user id and notification type. I have been having a long discussion with
 myself about this.


 >bp_get_request_unsubscribe doesn't appear to be hooked to anything.

 This picks up action=unsubscribe in bp_get_request()


 >You're running URLs through esc_url() before sending them to email.

 Eek, I couldn't find this discussion. I've left it in because I'd rather
 it fail that way than the other if that makes sense!

 I've used https://github.com/WordPress-Coding-Standards/WordPress-Coding-
 Standards but only done the stuff I wrote.

 Also, the unsubscribe functionality needs unit tests like
 bp_activity_at_message_notification_email_unsubscribe, is that right? I
 was going to do one for each notification type but tell me if that's not
 the way to do it.

 Also also, I was looking at this
 https://buddypress.trac.wordpress.org/ticket/7036 by @r-a-y and this
 ticket https://buddypress.trac.wordpress.org/ticket/7038 and think it
 might make sense to put all the email details in one schema, ie combine
 bp_email_type_schema() and bp_email_get_schema(). This current patch does
 *not* do that.

 And finally (!!) if the function names are bad, could you suggest new
 ones? I am out of ideas!

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