Adds basic UI for adding, editing and removing group memberships

[jludwig]

#209 #223

This adds Field UI integration w/Membership Types and Memberships, allowing admins to add/edit/remove both membership types and memberships in groups.

Note: This work was sponsored by BioRAFT. https://www.drupal.org/bioraft

Todo

• Tests... Will get at at least a test base setup before getting this merged in. I know, I should have worked on the tests during development but that's not a habbit for me yet.
• Form validation for the membership UID currently only works with the OG group context autocomplete, not the standard Drupal autocomplete. This can be fixed by applying the constraint (already create) on the Entity level rather than just the widget level
• No way to tell someone's membership right now. Adding the bundle column to the view would be easy, but it would just clutter a membership view on a site with only one type of membership
• The last member can be removed from a group. Not sure how to approach this. Should it enforce that the group owner is always a member?
• A membership can be a group/group content (lol)
• The add member link from the membership page only works on nodes, because it uses og.links.action.yml rather than a dynamic way to achieve that
• Some performance tweaks, especially in the OG autocomplete where it loads the memberships associated with the current group context

What it looks like in action

membership_ui.pdf

[jludwig]

@amitaibu A lot more work has gone into this. I just need to go through and do some code cleanup etc.

I also merged in #222 because the two play very well together, and there would be some conflicts otherwise.

Notes:

• Some documentation/code cleanup needed.
• The updated members view currently has 'Membership Type' as a column, but this doesn't make sense if it's Default is the only type available. I can get to it, but I'm a bit lazy.
• The OG Autocomplete element takes group context into account and may be useful for things other than referencing users. However, the membership reference selection handler is a bit inefficient currently because it loads all the group memberships. I can get to this too, but laziness
• A lot of functionality has been introduced both here and in OG master, so I need to rebase/catchup w/master and post some screenshots.

[amitaibu]

Haven't forgotten about this, in the meanwhile, here are a few answers to question from above:

Not sure how to approach this. Should it enforce that the group owner is always a member?

in D7 the group owner (e.g. node author) cannot leave the group. Which sounds like the right thing to keep.

A membership can be a group/group content (lol)

We should explicitly disallow, or that lol will become a big cry 😉

[zerolab]

Nice work on this one, @jludwig

FWIW, I had to modify og.routing.yml as follows:

@@ -51,7 +51,8 @@ entity.og_membership.canonical:
    _entity_access: 'og_membership.view'
  options:
    parameters:
-      type: entity:{entity_type_id}
+     group:
+        type: entity:{entity_type_id}

entity.og_membership.add_form:
  path: 'group/{entity_type_id}/{group}/admin/members/add/{membership_type}'
@@ -62,7 +63,8 @@ entity.og_membership.add_form:
    _og_membership_add_access: 'TRUE'
  options:
    parameters:
-      type: entity:{entity_type_id}
+      group:
+        type: entity:{entity_type_id}

entity.og_membership.edit_form:
  path: 'group/{entity_type_id}/{group}/admin/members/{og_membership}/edit'

to fix an Fatal error: Uncaught TypeError: Argument 1 passed to Drupal\content_moderation\ParamConverter\EntityRevisionConverter::hasForwardRevisionFlag() must be of the type array, string given, called in core/modules/content_moderation/src/ParamConverter/EntityRevisionConverter.php on line 44 and defined in core/modules/content_moderation/src/ParamConverter/EntityRevisionConverter.php:59 error on a 8.2.x build with Content Moderation enabled.

One more thing to add to the To-Do: the ability to change the membership state. When a user joins the group using the "Request group membership" link, their membership state is set to "pending". Whereas adding them through the new "add members" form sets them as active.

[AronNovak]

That PR is very exciting, as it's going to add a critical functionality to OG 8.x, thanks for all the efforts!

So here's my feedback after a walkthrough via the UI, many of the things are really small and far from being critical:

• I am at group/node/1/admin/members/add, the first field is Member User ID, which is true from a developer perspective, but in fact, if i fill the user ID (a number, right?), i get a validation error. Drupal usually refers it with the Username phrase (at admin/people/create for instance).

• I am at group/node/1/admin/members/2/edit?destination=/group/node/1/admin/members, I expect to see the administrative theme (seven, plain D8), but I see bartik themed page. Same applies to group/node/1/admin/members/2/delete?destination=/group/node/1/admin/members, as these are the two exceptions to the rule that all the usermanagement happens under administrative theme.

• I am at group/node/1/admin/members/add, I am super-admin, UID1, but I see the field Request membership. I expect this field to be hidden for those users who can and will add members as active. This is an addition to the point of @zerolab 's comment at Adds basic UI for adding, editing and removing group memberships #231 (comment) , the very last part. Indeed a Status field would be nice to be able to alter status of existing membership. And then, this Request membership field visibility could depend on the fact if the new user is going to be active or not.

• I am at group/node/1/admin/members/add/aaa, that's a URL that I hand-crafted for fun. I get a fatal PHP error:

  Fatal error: Call to a member function label() on null in /drupal-8.2.6/modules/og/src/Form/OgMembershipForm.php on line 33
  

• I am at /group/node/2/admin/members/add/aa, that's a URL that I hand-crafted for fun, node/2 is a group content, not a group. I get The website encountered an unexpected error. Please try again later., and in the watchdog

  Exception: The entity context found in the route match is not a valid group. in Drupal\og\Controller\OgAdminMembersController->getGroupFromRoute() (line 163 of /home/aaron/gizra/drupal-8.2.6/modules/og/src/Controller/OgAdminMembersController.php).
  

  On contrary, if I visit /group/node/2/admin/members/add, I get a meaningful access denied: You are not authorized to access this page.

• I am at /group/node/1/admin/members/add, I submit the form by typing 1 in the Member User ID field, then I submit it. After that, I see a PHP notice at admin/reports/dblog:

  Notice: Undefined offset: 0 in Drupal\og\Plugin\Validation\Constraint\UniqueOgMembershipConstraintValidator->validate()
  

• I created a membership type called <script>alert("XSS");</script> (credits to @amitaibu). At /group/node/1/admin/members/add/_script_alert_xss_script_, I expect to see the same membership type name as at admin/structure/membership-types, but it's sanitized differently.
  

• I created a membership type called <script>alert("XSS");</script>. At /group/node/1/admin/members/add/_script_alert_xss_script_, I try to add a member with this membership type. I receive an error The website encountered an unexpected error. Please try again later.. At admin/reports/dblog, I see this:

  Drupal\Component\Plugin\Exception\PluginNotFoundException: The "entity:og_membership:_script_alert_xss_script_" plugin does not exist.
  

More later on the actual implementation.

[AronNovak]

@jludwig
To determine $entity_type_id, it's not entirely clear why we need to check both entity_type_id and _og_entity_type_id, when this check is used exactly at one route: entity.og_membership.add_form.

[AronNovak]

@jludwig
In the function, it seems there's nothing about nodes. but about og_memberships. list of the node types and later is only one node type should be in-line with the actual entity that we deal with.

[damienmckenna]

Any plans to commit this soon?

[amitaibu]

As far as I can see there's still jludwig#9 that needs to be merged

[damienmckenna]

Spotted a couple of minor items.

[pfrenssen]

I have unfortunately no availability any more to review large PRs. My employer no longer gives me time to work on OG since we have long completed our goals, and my family and house renovations take up practically all of my spare time. But I am very happy to see all the progress here, and can't wait to try it out!

[MPParsley]

@damienmckenna, I'll be merging this in the coming days, let me know if there's any more feedback.

[MPParsley]

This has been a huge effort by a number of people, among other: @jludwig, @damienmckenna, @zerolab, @joachim-n, @pwolanin, @AronNovak, @amitaibu, @pfrenssen, @MPParsley, @DiDebru, @bboro, @lennartvava, and more, thank you all!

PS: this PR was initially opened on 11 Jan 2017 and may be the oldest PR merge in history :-)

[amitaibu]

🥳 Thank you @MPParsley (et al) for getting this finally merged!