In case there is a duplicate issue of this, I'm sorry, I couldn't find one. This is certainly related to #1263588: Allow having multiple group-audience type fields but I couldn't find any reference of this exact problem there, so I'm opening a new issue.

Problem description

On group/%group_type/%gid/admin/people/add-user, given that there are multiple group audience fields on the user entity that each reference different bundles on the same group type (e.g. different node types), a select list is shown to select the group audience field to use. Only one field should be shown, though: the one that references the bundle that the current group (%gid) belongs to. Surely enough, trying to select any other fields yields a fatal error.

Steps to reproduce

  1. Add two node types and make them groups
  2. Add two group audience fields to the user entity, each one referencing one of the node types
  3. Create a group for one of node types
  4. Go to /group/node/%nid/admin/people/add-user where %nid is the node ID of the group node that was just created
  5. Select the field that belongs to the other node type, fill out the rest of the form and submit

You should see the following error:

OgException: OG membership can not be created in entity user and bundle user using the field field_interest_group_membership as the field does not reference company bundle in node entity type. in OgMembership->save() (line 62 of /og/includes/og.membership.inc).

Proposed resolution

When fetching the list of group audience fields check each one whether it references the given bundle of the group.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

Status: Active » Needs review
FileSize
957 bytes

Here is a patch. I'm working on tests as we speak, so to say. :-)

Status: Needs review » Needs work

The last submitted patch, 2042163-1-og-add-user-multiple-audience.patch, failed testing.

tstoeckler’s picture

Yeah, I failed #git on that one. Still should be reviewable. I'll provide a better patch once I get the tests in order.

Frank Ralf’s picture

Tested the patch and it works as promised. Can be set to RTBC when patch also has passed test bot.

tstoeckler’s picture

Assigned: Unassigned » tstoeckler
Priority: Normal » Major
Status: Needs work » Needs review
FileSize
8.84 KB

I noticed that there are already some changes in 7.x-2.x vs. 2.2 in og_get_group_audience_fields() that introduce checking on the target entity type. I added a check for target bundles to that function. I also added test-coverage, including for the target entity type case.

Tentatively marking major. This is probably not the most common use-case, but getting fatals from the UI is also not very nice. Feel free to disagree and downgrade again.

amitaibu’s picture

Wow, even a test, cool! :) Someone from Gizra will review soon.

RoySegall’s picture

I can confirm the patch: i was able to restore the problem before applying the patch. After applying the patch and the problem fixed.

RoySegall’s picture

Minor fix - you don't need to write:
if (isset($group_bundle) && !empty($field_info['settings']['handler_settings']['target_bundles']) && !in_array($group_bundle, $field_info['settings']['handler_settings']['target_bundles'])) {

you can just write:
if ($group_bundle && !empty($field_info['settings']['handler_settings']['target_bundles']) && !in_array($group_bundle, $field_info['settings']['handler_settings']['target_bundles'])) {

I'm attachign a patch with that minor fix.

tstoeckler’s picture

Well that disallows the theoretical possibility of a bundle named '0' because in PHP (bool) '0' === FALSE . That's really an academic use-case, though, so I don't really care either way. That's just the reason I usually stick to isset().

amitaibu’s picture

Status: Needs review » Fixed

Committed, thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated issue summary: Notes are no longer valid.