On a particular site, when using the code committed in #1730678: Provide a "complex" widget instead of "Primary" and "Secondary" fields., this code is always resulting in empty $other_group_ids.

  $entity_gids = og_get_entity_groups($entity_type, $entity);
  $entity_gids = !empty($entity_gids[$target_type]) ? $entity_gids[$target_type] : array();

  $user_gids = og_get_entity_groups();
  $user_gids = !empty($user_gids[$target_type]) ? $user_gids[$target_type] : array();

  $other_groups_ids = array();
  $other_groups_ids = array_diff(array_values($entity_gids), array_values($user_gids));

which in turn causes an exception to be thrown further down when the set() method is called:

        if ($field_mode == 'admin') {
          // Keep only the hidden group IDs on the entity, so they won't
          // appear again on the "admin" field, for example on an autocomplete
          // widget type.
          $valid_ids = $other_groups_ids ? entityreference_get_selection_handler($field, $mocked_instance, $entity_type, $dummy_entity)->validateReferencableEntities($other_groups_ids) : array();
          $valid_ids = $field['cardinality'] == 1 ? reset($valid_ids) : $valid_ids;
          $wrapper->{$field_name}->set($valid_ids);
        }

Oddly, neither $entity_gids or $user_gids are empty. I'm continuing to dig into this.

CommentFileSizeAuthor
#10 og-default-widget-exception-1774294-10.patch627 bytesjhedstrom
#7 og-default-widget-exception-1774294-07.patch797 bytesjhedstrom
#4 1774294-og-widget-full-diff-4.patch670 bytesamitaibu
#1 og-admin-widget-1774294-01.patch904 bytesjhedstrom
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Active » Needs review
FileSize
og-admin-widget-1774294-01.patch904 bytes

So, in one example, the value of $entity_gids is

array(
  53 => 26,
);

while the value of $user_gids is

  array(
    34 => 26,
    39 => 29
  );

Since array_diff() will only return values from array1 that are not present in subsequent arrays, this is always returning empty, rather than returning, in the above example, 

array(0 => 29);

A simple change resolves this (attached), but I'm not sure I'm fixing the correct problem.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented

> Since array_diff() will only return values

What's the array_diff() you refer to -- validateReferencableEntities()?

Sorry, I see the above example, $other_groups_ids = array_diff(array_values($entity_gids), array_values($user_gids));

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented

Looking at array_diff comments, Maybe the correct diff should be something like this:

return array_diff(array_merge($left, $right), array_intersect($left, $right));

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented
FileSize
1774294-og-widget-full-diff-4.patch670 bytes

Can you try this?

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented

Patch in #4 fixes this for me.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented
Title: Unhandled exception in og_field_widget_form() » Use correct diff to get 'other groups' in og_field_widget_form()
Component: og-field-access » og.module
Status: Needs review » Fixed

committed.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Fixed » Needs review
FileSize
og-default-widget-exception-1774294-07.patch797 bytes

Re-opening because I'm now seeing the same behavior in the conditional below the one we just fixed, regarding the $my_group_ids always being empty since the array_intersect call only includes values that are present in subsequent arguments, but in this case, $user_gids has values that are distinct from $entity_gids.

          // Keep only the groups that belong to the user and to the entity.
          $my_group_ids = array_values(array_intersect($user_gids, $entity_gids));
          $valid_ids = $my_group_ids ? entityreference_get_selection_handler($field, $mocked_instance, $entity_type, $dummy_entity)->validateReferencableEntities($my_group_ids) : array();

          $valid_ids = $field['cardinality'] == 1 ? reset($valid_ids) : $valid_ids;
          $wrapper->{$field_name}->set($valid_ids);

Again, not sure if the attached patch is the way to go, but unlike the previous error, if these 2 arrays are mutually exclusive, I don't see a way to reconcile that and get a non-null $my_group_ids.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented

I don't see a way to reconcile that and get a non-null $my_group_ids.

But NULL is a valid value, do you get WSOD?
btw, Are you using latest Entity API?

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented
Status: Needs review » Fixed

I've pushed a fix, apparently wrapper doesn't like array() and prefers NULL.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Fixed » Needs review
FileSize
og-default-widget-exception-1774294-10.patch627 bytes

This requires the same fix for the non-admin widget.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented
Status: Needs review » Fixed

Thanks, committed.

Status: Fixed » Closed (fixed)

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

GiorgosK’s picture

GiorgosK
Location Chios, Greece 🇬🇷
CreditAttribution: GiorgosK commented
Status: Closed (fixed) » Active

There exactly the same error on the latest openatrium distribution
coming from OG #2073229: array_diff(): Argument #2 is not an array in og_field_widget_form
it feels that its very relevant to this issue thus I reopen this
the patch provided in #10 is not at all appropriate but its providing enough info about the problem line
if you can provide any appropriate patch will be glad to try it out

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented
Status: Active » Closed (fixed)

If needed, please open a new issue.