[buddypress-trac] [BuddyPress Trac] #6210: Create New Invitations API

[buddypress-trac]

#6210: Create New Invitations API
-----------------------------------------+-----------------------
 Reporter:  dcavins                      |       Owner:  dcavins
     Type:  enhancement                  |      Status:  reopened
 Priority:  low                          |   Milestone:
Component:  Core                         |     Version:
 Severity:  normal                       |  Resolution:
 Keywords:  dev-feedback trac-tidy-2018  |
-----------------------------------------+-----------------------

Comment (by boonebgorges):

 Thanks for bringing this one back to life, @dcavins !

 I've done a casual review and I have some questions and feedback. In no
 particular order:

 - Can you explain how `component_name` and `component_action` work?
 Specifically, it feels like `component_name` is enough to differentiate
 between, eg, group invitations and site invitations.
 - Regarding naming: I'm not a huge fan of `BP_Invitations_Invitation`. IMO
 it's better to go with `BP_Invitation` or `BP_Core_Invitation`. And
 `BP_Invitations` is not very clear to me. Maybe something like
 `BP_Invitation_Manager`, `BP_Group_Invitation_Manager`, etc?
 https://stackoverflow.com/questions/1866794/naming-classes-how-to-avoid-
 calling-everything-a-whatevermanager
 - Otherwise, the general pattern of having a base manager class and then
 extending it per component seems OK.
 - Because `run_send_action()` and `run_acceptance_action()` seem to be the
 critical integration points for specific components, perhaps it'd make
 sense (from the point of view of self-documentation) to make the base
 class `abstract` and to make these two methods `abstract`.
 - I see that, in various places, you're deprecating the use of
 `membership_id` in the case of acceptance/rejection. Obviously, I
 understand why you have to do this, and some level of compatibility
 breakage is unavoidable in this kind of change. But I'm concerned that
 it's going to cause backward compatibility concerns of an awkward type.
 For example, in `bp_group_request_accept_link()` you are swapping
 `membership_id` with `user_id` in the existing link format. If a third-
 party plugin is manually building a link of this sort, and accidentally
 puts in a "membership id" that matches up with a valid user_id, it could
 result in a link that accepts the wrong invitation. This is bad. In cases
 like this, perhaps it's better to break those old links altogether, and
 change the format: `admin/membership-requests/accept-request/` or
 something like that. More generally, it'll be necessary to have a high-
 level summary of compatibility breaks, so we can share with BP devs.

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