When trusted contacts was created, it added an additional group that people can be referenced by. Unfortunately, this means that without a group_type filter, a group reference with a user of id=2 will also show up in node groups of id=2.

By adding an OG group type filter, only users who are referencing that group type (node or user depending on the view) should show up.

CommentFileSizeAuthor
#19 2037417-group_type_filterout-19.patch2.23 KBjaperry
#18 2037417-group_type_filterout-18.patch2.28 KBjaperry
#14 2037417-group_type_filterout-14.patch1.04 KBjaperry
#15 2037417-group_type_filterout-15.patch1.98 KBjaperry
#5 2037417-commons-groups-og-grouptype-5.patch4.29 KBjaperry
#2 2037417-commons-groups-og-grouptype-2.patch3.69 KBjaperry

Comments

japerry’s picture

japerry
Location KOMK
commented
Issue tags: +Commons 7.x-3.3 radar

adding to radar

japerry’s picture

japerry
Location KOMK
commented
StatusFileSize
new 2037417-commons-groups-og-grouptype-2.patch3.69 KB

This patch also includes the fix for #2037419: Pending group users are shown as contributors (because doing them as two different views is somewhat annoying)

japerry’s picture

japerry
Location KOMK
commented
Status: Active » Needs review

This patch also includes the fix for #2037419: Pending group users are shown as contributors (because doing them as two different views is somewhat annoying)

devin carlson’s picture

devin carlson commented

Marked #2025413: Duplicate people in group contributors pane as a duplicate.

japerry’s picture

japerry
Location KOMK
commented
StatusFileSize
new 2037417-commons-groups-og-grouptype-5.patch4.29 KB

This adds an update hook in the install file to revert the view

ezra-g’s picture

ezra-g commented

Awesome catch & diagnosis, japerry!

A grep from profiles/commons/modules/contrib suggests that there may be more places we need to fix this:

 grep -r og_membership_rel * | grep commons
commons_activity_streams/commons_activity_streams_groups/commons_activity_streams_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_activity_streams/commons_activity_streams_groups/commons_activity_streams_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_activity_streams/commons_activity_streams_groups/commons_activity_streams_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_activity_streams/commons_activity_streams_groups/commons_activity_streams_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['relationship'] = 'field_target_nodes_target_id';
commons_activity_streams/commons_activity_streams_groups/commons_activity_streams_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['required'] = TRUE;
commons_activity_streams/commons_activity_streams_groups/commons_activity_streams_groups.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_documents/commons_documents.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_documents/commons_documents.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_documents/commons_documents.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_documents/commons_documents.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_events/commons_events.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_events/commons_events.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_events/commons_events.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_events/commons_events.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['required'] = TRUE;
commons_events/commons_events.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'users';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['fields']['og_roles']['relationship'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'users';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['required'] = TRUE;
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['required'] = TRUE;
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_node_group']['id'] = 'og_membership_related_node_group';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_node_group']['table'] = 'og_membership';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_node_group']['field'] = 'og_membership_related_node_group';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_node_group']['required'] = TRUE;
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['fields']['field_group_logo']['relationship'] = 'og_membership_related_node_group';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['fields']['title']['relationship'] = 'og_membership_related_node_group';
commons_groups/commons_groups.views_default.inc:  $handler->display->display_options['fields']['group_group']['relationship'] = 'og_membership_related_node_group';
commons_notices/commons_notices.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_notices/commons_notices.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_notices/commons_notices.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_notices/commons_notices.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['required'] = TRUE;
commons_notices/commons_notices.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_polls/commons_polls.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_polls/commons_polls.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_polls/commons_polls.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_polls/commons_polls.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_posts/commons_posts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_posts/commons_posts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_posts/commons_posts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_posts/commons_posts.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_q_a/commons_q_a.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_q_a/commons_q_a.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_q_a/commons_q_a.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_q_a/commons_q_a.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_radioactivity/commons_radioactivity_groups/commons_radioactivity_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_radioactivity/commons_radioactivity_groups/commons_radioactivity_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_radioactivity/commons_radioactivity_groups/commons_radioactivity_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_radioactivity/commons_radioactivity_groups/commons_radioactivity_groups.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['required'] = TRUE;
commons_radioactivity/commons_radioactivity_groups/commons_radioactivity_groups.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_search/commons_search.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_search/commons_search.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_search/commons_search.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_search/commons_search.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['required'] = TRUE;
commons_search/commons_search.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user_group']['id'] = 'og_membership_related_user_group';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user_group']['table'] = 'og_membership';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user_group']['field'] = 'og_membership_related_user_group';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user']['id'] = 'og_membership_related_user';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user']['table'] = 'og_membership';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user']['field'] = 'og_membership_related_user';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['fields']['name']['relationship'] = 'og_membership_related_user';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['arguments']['uid']['relationship'] = 'og_membership_related_user_group';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['arguments']['uid_1']['relationship'] = 'og_membership_related_user';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user_group']['id'] = 'og_membership_related_user_group';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user_group']['table'] = 'og_membership';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user_group']['field'] = 'og_membership_related_user_group';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user']['id'] = 'og_membership_related_user';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user']['table'] = 'og_membership';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['relationships']['og_membership_related_user']['field'] = 'og_membership_related_user';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['fields']['name']['relationship'] = 'og_membership_related_user';
commons_trusted_contacts/commons_trusted_contacts.views_default.inc:  $handler->display->display_options['arguments']['uid']['relationship'] = 'og_membership_related_user_group';
commons_wikis/commons_wikis.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['id'] = 'og_membership_rel';
commons_wikis/commons_wikis.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['table'] = 'node';
commons_wikis/commons_wikis.views_default.inc:  $handler->display->display_options['relationships']['og_membership_rel']['field'] = 'og_membership_rel';
commons_wikis/commons_wikis.views_default.inc:  $handler->display->display_options['arguments']['gid']['relationship'] = 'og_membership_rel';
japerry’s picture

japerry
Location KOMK
commented

These are all OG relationships by node, not user -- so those don't apply to this patch.

ezra-g’s picture

ezra-g commented
Status: Needs review » Reviewed & tested by the community

Seems RTBC to me. Let's get additional testing in 7.x-3.x dev. Thanks!

devin carlson’s picture

devin carlson commented
Status: Reviewed & tested by the community » Fixed

This has been committed to Commons Groups 7.x-3.x.

http://drupalcode.org/project/commons_groups.git/commit/d89418a

devin carlson’s picture

devin carlson commented

Marked #2037419: Pending group users are shown as contributors as a duplicate.

Status: Fixed » Closed (fixed)

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

japerry’s picture

japerry
Location KOMK
commented
Status: Closed (fixed) » Needs work

This issue has popped back up within content. If you create 'private content' as user 1 (admin), and create a group with a nid=1, you'll see private content show up within all the group content listing views.

This bug is a little more challenging for content since the view for the homepage is shared with group content listing pages. We need to remove the view filter for the homepage.

ezra-g’s picture

ezra-g commented
Priority: Normal » Critical

Marking as critical since this causes several listings to be fairly incorrect.

japerry’s picture

japerry
Location KOMK
commented
StatusFileSize
new 2037417-group_type_filterout-14.patch1.04 KB

I've made the view changes to commons_groups, commons_activity_streams_ and commons_radioactivity

http://drupalcode.org/project/commons_groups.git/commit/53fbd2d
http://drupalcode.org/project/commons_activity_streams.git/commit/ad39822
http://drupalcode.org/project/commons_radioactivity.git/commit/c9483e9

do not use the patch below. Use the patch in comment 15.

japerry’s picture

japerry
Location KOMK
commented
Status: Needs work » Needs review
StatusFileSize
new 2037417-group_type_filterout-15.patch1.98 KB

Also the CTs need to be updated, as they show up in the tabs on each home page and group page...

Here are the views, and an updated patch with an update script to refresh the views.

http://drupalcode.org/project/commons_posts.git/commit/4dd0f98
http://drupalcode.org/project/commons_wikis.git/commit/ae5d844
http://drupalcode.org/project/commons_q_a.git/commit/b676963
http://drupalcode.org/project/commons_polls.git/commit/f28cdbf
http://drupalcode.org/project/commons_groups.git/commit/6e4e8d1

ezra-g’s picture

ezra-g commented

Thanks, japerry! A thorny problem, indeed.

A couple of questions about the implementation here.

A) If we need to alter views defined on behalf of all of the content types that appear in the group browsing widget, could we do this with a single alter in Commons Groups or Commons_BW module, and simplify the code a bit?

@@ -887,6 +887,22 @@ function commons_trusted_contacts_form_commons_bw_partial_node_form_alter(&$form
+function commons_trusted_contacts_views_pre_view(&$view, &$display_id, &$args) {
+  if (empty($args)) {
+    if (isset($view->display_handler->options['filters']['group_type'])) {
+      $filters = $view->display_handler->get_option('filters');

B) This appears to not check against any particular View ID. Perhaps we should be checking against a specific view ID or some other aspect of the view?

C) 

@@ -887,6 +887,22 @@ function commons_trusted_contacts_form_commons_bw_partial_node_form_alter(&$form
+ * Implements views_pre_view

Nit: The format here is "Implements hook_views_pre_view()." -- hook_[hook name], parens and period. This helps when grepping for all implementations of a particular hook :).

japerry’s picture

japerry
Location KOMK
commented

I'm not sure I quite follow A. Basically hook_views_pre_view is an alter that runs during execution of a view. When you mentioned last week looking at commons_bw module, it looked like it was just making a row count for the tab. So whether you put this hook_views_pre_view code in trusted contacts or in groups, it doesn't really matter. I put it in trusted contacts because that caused the regression of the users and nodes having the same ID.

For B, we are looking for the group_type filter and removing it if the corresponding argument (which relies on the relationship to OG) is not there. This should apply to all views that contain this combination, because we shouldn't ever need to see a view that needs a group_type filter on when the corresponding OG reference is missing (because it'd be an invalid filter). We could put in a clause for all the different views, but I think this would be more fragile than the current approach.

C. darn, you noticed that! I saw it after commit and already fixed it locally. Heh sharp eye ;-)

japerry’s picture

japerry
Location KOMK
commented
StatusFileSize
new 2037417-group_type_filterout-18.patch2.28 KB

Here is another slight revision of the previous patch. Functionally it adds a check to make sure you're only looking at groups with a gid argument and the args array is false. Also added some comments to better describe what the function does.

japerry’s picture

japerry
Location KOMK
commented
StatusFileSize
new 2037417-group_type_filterout-19.patch2.23 KB

Okay we're moving this to commons groups. it fits better there.

don't use any other patches above!

japerry’s picture

japerry
Location KOMK
commented
Status: Needs review » Fixed

Fixed!

http://drupalcode.org/project/commons_groups.git/commit/8eea522

Yes there is a syntax change in the imagecache labels when I re-dumped the feature. its minor but in this commit as well.

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