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
- Add two node types and make them groups
- Add two group audience fields to the user entity, each one referencing one of the node types
- Create a group for one of node types
- Go to /group/node/%nid/admin/people/add-user where %nid is the node ID of the group node that was just created
- 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.
Comment | File | Size | Author |
---|---|---|---|
#8 | 2042163-7-og-add-user-multiple-audience.patch | 8.84 KB | RoySegall |
#5 | 2042163-5-og-add-user-multiple-audience.patch | 8.84 KB | tstoeckler |
#1 | 2042163-1-og-add-user-multiple-audience.patch | 957 bytes | tstoeckler |
Comments
Comment #1
tstoecklerHere is a patch. I'm working on tests as we speak, so to say. :-)
Comment #3
tstoecklerYeah, I failed #git on that one. Still should be reviewable. I'll provide a better patch once I get the tests in order.
Comment #4
Frank Ralf CreditAttribution: Frank Ralf commentedTested the patch and it works as promised. Can be set to RTBC when patch also has passed test bot.
Comment #5
tstoecklerI 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.
Comment #6
amitaibuWow, even a test, cool! :) Someone from Gizra will review soon.
Comment #7
RoySegall CreditAttribution: RoySegall commentedI can confirm the patch: i was able to restore the problem before applying the patch. After applying the patch and the problem fixed.
Comment #8
RoySegall CreditAttribution: RoySegall commentedMinor 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.
Comment #9
tstoecklerWell 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().Comment #10
amitaibuCommitted, thanks.
Comment #11.0
(not verified) CreditAttribution: commentedUpdated issue summary: Notes are no longer valid.