I'm using OG 7.x-2.x, currently using the alpha3 but I know this applies to dev as well.

I have 3 content types:
- Group (group type)
- Contact (group content type)
- Project (group content type)

These are all nodes.

Project has a noderef field (references module) called "Primary Contact". I am using a view to provide the values, and I am only showing the Contacts posted to the same Group as the Project (ie. the Primary Contact for the Project is a Contact from the same Group).

This is generally working fine, until there is a bit more going on with other fields. Where I have noticed it is with filefields. When I click "Upload" to upload a filefield, and there is already a Primary Contact set, I get an error "An illegal choice has been detected. Please contact the site administrator."

When I debug a bit, I see that the file upload button is calling the url http://mydomain/file/ajax/..., and within that callback, a call is being made to og_context, and it's not returning anything for gid. When this happens, the valid Primary Contact value is not present in the allowed values (since there is no group context in that validation request).

I'm not sure why unrelated fields are being run through validation routines when files are uploaded. I don't know if this is a core issue or what.

Comments

Cottser’s picture

I'm having a very similar issue as well, with a very different setup: OG 7.x-1.x, and entityreference instead of references module. In my case the "Add another item" and "Remove" buttons in a multiple cardinality field are triggering errors (so AJAX again). I'm using Views to provide the values for the entityreference dropdown.

See comment #3 here for a bit more information as to why AJAX triggers these types of errors: #1263840: Adding multiple field collection field instances containing a node reference produces errors on node add form (required field)

There are some core issues that are similar, but nothing exact:
#1162712: AJAX submissions try to validate other form fields: "An illegal choice has been detected."
#811542: Regression: Required radios throw illegal choice error when none selected

If I set the contextual filter on my view to a fixed value and give it a gid, everything works fine. This quick and dirty code in a view exhibits the same behaviour as "Current OG group from context" though.

if (is_numeric(arg(1))) {
  $node = node_load(arg(1));
  return $node->group_audience[LANGUAGE_NONE][0]['gid'];
}

This could definitely end up being a core issue.

stevector’s picture

I am having the same issue with a multivalue field collection and it seems to boil down to the ajax handler not know the appropriate context. What about using $_GET params to carry the context to the ajax callback? This concept is used with Entity Reference Prepopulate.

stevector’s picture

Status: Active » Needs review
FileSize
4.08 KB
PASSED: [[SimpleTest]]: [MySQL] 723 pass(es). View

Here's a patch that seems to get the job done though I'm not particularly happy with the details of the implementation and there is probably some cleanup that should happen before this is committed.

The basic idea is:

Adding an additional handler to og_context_og_context_negotiation_info()
This is currently named specific to ajax however there may be a case to make it more global. For instance, the media module browser could benefit from GET variables. Currently there is the "url" handler but this is hardcoded to only work with entityreference_prepopulate. Should that handler get expanded instead of added a new one?

Overriding Drupal.ajax.prototype.beforeSubmit()
Even though misc/ajax.js suggests overriding this function, I don't like it. I've included in this patch a commented out alternate approach. I tried to just set the url during the attach behaviors phase but that didn't work and I'm not sure why.

To test this patch you'll have to enable the additional handler at /admin/config/group/context

amitaibu’s picture

I think I understand some but not completely. Can you provide some steps to see the error (i.e. the context not set properly).

stevector’s picture

I ran into this error because I have a "news" content type with the following configuration:

1. A group audience field.
2. An entity reference field that points a a single slideshow node. The allowed values are determined a view. The View filters down to slideshows that are in the same OG as the news node being edited.
3. A multivalue field collection field (based on the above error reports I think any mutlivalue field will yield the same error).

When the "add" and "remove" buttons are clicked on the multivalue field collection an ajax request is made to paths like "system/ajax" and "field_collection/ajax." At these URLs the form is rebuilt why causes the slideshow reference field to rebuild it's allowed values. Because this rebuild is happening on an ajax path (/field_collection/ajax) instead of a node edit path ("/node/123/edit") og_context doesn't return the desired value and the view that determines the allowed values for the slideshow field has different results.

When the form is submitted, an error message occurs. The selected slideshow is no longer in the list that FAPI considers to be the allowed values.

stevector’s picture

FileSize
4.08 KB
PASSED: [[SimpleTest]]: [MySQL] 723 pass(es). View

Here's an update patch. It switches one function call from array_shift() to reset().

amitaibu’s picture

@stevector,
Any chance you can upload a db-dump with the scenario in #5 -- I'd be very happy to be able to reproduce on my local?

stevector’s picture

FileSize
108.77 KB

Yeah, here's a database that just uses a file field instead of a a field_collection because there were already enough contrib modules in play :)

Here's a screenshot of what the error looks like:

Only local images are allowed.

Here's what I get from running drush pml --status=enabled --type=module

 Package           Name                                           Version     
 Chaos tool suite  Chaos tools (ctools)                           7.x-1.2     
 Core              Block (block)                                  7.14        
 Core              Contextual links (contextual)                  7.14        
 Core              Dashboard (dashboard)                          7.14        
 Core              Database logging (dblog)                       7.14        
 Core              Field (field)                                  7.14        
 Core              Field SQL storage (field_sql_storage)          7.14        
 Core              Field UI (field_ui)                            7.14        
 Core              File (file)                                    7.14        
 Core              Filter (filter)                                7.14        
 Core              Help (help)                                    7.14        
 Core              Image (image)                                  7.14        
 Core              List (list)                                    7.14        
 Core              Menu (menu)                                    7.14        
 Core              Node (node)                                    7.14        
 Core              Number (number)                                7.14        
 Core              Options (options)                              7.14        
 Core              Path (path)                                    7.14        
 Core              RDF (rdf)                                      7.14        
 Core              Search (search)                                7.14        
 Core              Shortcut (shortcut)                            7.14        
 Core              System (system)                                7.14        
 Core              Taxonomy (taxonomy)                            7.14        
 Core              Text (text)                                    7.14        
 Core              Toolbar (toolbar)                              7.14        
 Core              Update manager (update)                        7.14        
 Core              User (user)                                    7.14        
 Fields            Entity Reference (entityreference)             7.x-1.0-rc5 
 Organic groups    Organic groups (og)                                        
 Organic groups    Organic groups context (og_context)                        
 Organic groups    Organic groups UI (og_ui)                      7.14        
 Other             Entity API (entity)                            7.x-1.0-rc3 
 Views             Views (views)                                  7.x-3.3     
 Views             Views Bulk Operations (views_bulk_operations)  7.x-3.0     
 Views             Views UI (views_ui)                            7.x-3.3 

To reproduce this error with this database:

1. Make a new page node and put it in the Biology Group.
2. Edit the node and select a slideshow.
3. Save the node.
4. Edit the node again and upload a txt file to the file field using the ajax upload button.
5. See error.

amitaibu’s picture

Ok, I see the problem - thanks for the DB.

How about doing something like this (untested) inside an existing/ new OG-context handler -- it's hardcoded to file/ajax, but looks nicer I think:

  $item = menu_get_item();
  if ($item['path'] == 'file/ajax') {
    list($form, $form_state) = ajax_get_form();
    $entity_type = $form['#entity_type'];
    if (empty($form_state[$entity_type])) {
      return;
    }

    $entity = $form_state[$entity_type];
     list($id) = entity_extract_ids($entity_type, $entity);
     if (!$id) {
       return;
     }
    
    // Check if the entity is itself a group.
    if ($group = og_is_group($entity_type, $id)) {
      $contexts[$entity_type][] = $id;
    }
    elseif ($gids = og_get_entity_groups($entity_type, $entity)) {
      $contexts = $gids;
    }
  
    return $contexts;    
  }

amitaibu’s picture

Title: OG Context not returning proper values during validation calls » Make OG Context aware to parent form in file upload ajax calls
Status: Needs review » Needs work

Better title.

stevector’s picture

Hi Amitaibu,

Thanks for the review. I think an approach like that would be cleaner. A potential downside would be having to account for all the various ajax paths used. There's also system/ajax and field_collection module has it's own path. And come to think of it, entity reference autocompletes too. For instance that slideshow select field in this example only works on page load because it is a select list and the og_context is available. If you switch that to autocomplete, it'll break because the context isn't available at the autocomplete path. I think this patch solves for that case too.

What about expanding the GET variable approach used in entity reference prepopulate to also be the same GET variables used here? So there would be only one GET variable handler that works for all cases.

amitaibu’s picture

I think this is a similar issue -- #1502972: Issues with ajax file upload fields
We basically need to make sure that after the ajax call we refresh the form.

And come to think of it, entity reference autocompletes too

No, as far as I can see only ajax calls to menu items that are defined as $item['delivery_callback'] == 'ajax_deliver' cause the problem. ER's autocomplete doesn't cause the same problem.

mxh’s picture

FileSize
9.64 KB

I'm sorry to ask for it, but I'm looking for a workaround for this problem, which is exactly the same as posted by dtarc. I'm currently using 7.x-2.2.

I have a view which is catching the OG group form the context in the contextual filter and use this view for my ER fields.

I think that's a problem with ER fields and ajax in general. When i don't use OG's context function in the view by using an other method to get the group I want (I attached the view for if you'd like to see what I mean), which also works when the node id is in the URL, then I get the same error with "illegal choice..".
Maybe there has to be a fix for that general problem, not by fixing it only for the OG context.

mxh’s picture

Okay just for those who are looking for a possible workaround when having this particular problem with OG and ER views:
If you're OK with it that the view will return all results when OG won't find the group by context (which it won't when you're in the path file/ajax/field_file_attach/...), you could set up your with contextual filter 'OG membership: group id' and instead of using "Current OG group from context", you select "PHP" and insert following code:

$context = og_context('node');
if (empty($context)) {
  return 'all';
}
else {
  return $context['gid'];
}

Assuming your group is a node. Otherwise write og_context('user').

Then it works with ajax file uploads.

Attached my view as example.ö

Definetly not a good workaround, but it works until a good solution is found.

paolomainardi’s picture

Issue summary: View changes
FileSize
1.57 KB

Attached a patch to fix a fatal error of #6.

syastrov’s picture

Here is an approach which is entirely server-side. It handles 'file/ajax' paths and parses the parameters to the form's action to set the group context.

syastrov’s picture

Fixed my previous patch in #16 to work with all machine names for destination group types: [a-z0-9_]+
Was previously matching only [\w\-]+, which was my mistake.

syastrov’s picture

Component: og.module » og-context
Status: Needs work » Needs review
FileSize
2.91 KB
PASSED: [[SimpleTest]]: [MySQL] 1,079 pass(es). View

Here is a patch which combines Amitaibu's approach in #9 with my approach from #17.
It handles the case where you try to upload a file into a field attached to a node you are creating with an og_group_ref. If you upload a file, then when it rebuilds the form, it will have the correct og_context (it determines the context from the query string of the form's action). It will also have the correct og_context during a /file/ajax request when editing/removing an uploaded file (due to Amitaibu's code).

It is an entirely server-side solution.

I hope that people can test this patch and we can commit a fix to this issue.

ranroz’s picture

@syastrov

+++ b/og_context/og_context.module
@@ -124,6 +124,13 @@ function og_context_og_context_negotiation_info() {
+  $providers['file-ajax'] = array(
...
+  );

Why not use the existing node handler instead?.

+++ b/og_context/og_context.module
@@ -490,6 +497,54 @@ function og_context_handler_node($node = NULL) {
+  // Based on https://drupal.org/comment/6628310#comment-6628310

The comment needs to change. An explanation of what is done here would be good.

+++ b/og_context/og_context.module
@@ -490,6 +497,54 @@ function og_context_handler_node($node = NULL) {
+      $group_id = $matches[1]; ¶

Remove extra space at the end of the line.

+++ b/og_context/og_context.module
@@ -490,6 +497,54 @@ function og_context_handler_node($node = NULL) {
+    if (preg_match("/og_group_ref=(\d+)/", $form['#action'], $matches)) {

Don't hardcode og_group_ref. It's better to use OG_AUDIENCE_FIELD.

Also remove white spaces at the beginning of lines 506, 510 and 512.

amitaibu’s picture

> Don't hardcode og_group_ref. It's better to use OG_AUDIENCE_FIELD.

No, you can't use OG_AUDIENCE_FIELD as you can have multiple group audience fields. You need to og_get_group_audience_fields()

no.exploit’s picture

FileSize
3.5 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch og_context_file_ajax-1635750-20.patch. Unable to apply patch. See the log in the details link for more information. View

Attached a patch to fix #20 and for normal ajax call

Status: Needs review » Needs work

The last submitted patch, 21: og_context_file_ajax-1635750-20.patch, failed testing.

kaare’s picture

FileSize
3.5 KB

Patch from #21 rerolled as proper git diff. No change = no interdiff.

kaare’s picture

Title: Make OG Context aware to parent form in file upload ajax calls » OG Context handler for ajax calls (system, file upload)
FileSize
2.84 KB
PASSED: [[SimpleTest]]: [MySQL] 1,236 pass(es). View

Here is a cleaner aproach closer to @amitaibu's suggestion in #9. It doesn't even try to negotiate og context during entity creation. The suggested code for this so far seems fragile.

OG context negotiation of ajax calls during entity creation should be combined with the logic in og_context_handler_url(), which only works for nodes.

kaare’s picture

Status: Needs work » Needs review
kaare’s picture

FileSize
2.5 KB
PASSED: [[SimpleTest]]: [MySQL] 1,236 pass(es). View
905 bytes

This one utilizes _group_context_handler_entity().

JayDarnell’s picture

Please forgive the oversight - didn't read the comments for karee's patch close enough.. only works for editing existing entities. I wish my coding chops were better so I could figure out how to also make it work for new entities.

JayDarnell’s picture

Please keep in mind I'm not the most skilled drupal developer as of this moment. I still have heaps to learn but I thought I'd take a crack at extending what kaare provided in #26 - I attempted to extract the logic from the og_context_handler_url to make the patch from #26 work when editing existing nodes as well.

This seems to work although given it is my first attempt at editing a contributed module I imagine my contributions are probably not solid enough to pass a code review.

/**
 * Context handler; Shared between file upload and system ajax calls.
*/
function og_context_handler_ajax() {
  $item = menu_get_item();

  if (strpos($item['path'], 'file/ajax') !== 0 && strpos($item['path'], 'system/ajax') !== 0) {
    return;
  }
  
  list($form, $form_state) = ajax_get_form();

  if (empty($form['#entity_type'])) {
    return;
  }
  $entity_type = $form['#entity_type'];
  if (empty($form_state[$entity_type])) {
    return;
  }
  
  if(!isset($form['nid']['#value'])) {
    
    // We are adding a new node
    $node_type = $form_state[$entity_type]->type;
    
    if (!$fields = og_get_group_audience_fields('node', $node_type)) {
      return;
    }
    $gids = array();
    foreach ($fields as $field_name => $label) {
      $field = field_info_field($field_name);
      $instance = field_info_instance('node', $field_name, $node_type);
      
      if ($ids = ($_SESSION['og_context']['group_type'] == $entity_type) ? array($_SESSION['og_context']['gid']) : NULL) {
      
        // We need to validate those values ourself, as we called
        // entityreference_prepopulate_get_values_from_url() directly, bypassing
        // entityreference_prepopulate_get_values().
        $target_type = $field['settings']['target_type'];
        $valid_ids = array();
        $entities = entity_load($target_type, $ids);
        foreach ($entities as $id => $entity) {
          if (!entity_access('view', $target_type, $entity)) {
            // User can't access entity.
            continue;
          }

          if (!og_is_group($target_type, $entity)) {
            // Entity is not a group.
            continue;
          }

          $valid_ids[] = $id;
        }

        if ($valid_ids) {
          $gids += array($target_type => array());
          $gids[$target_type] = array_merge($gids[$target_type], $valid_ids);
        }
      }
    }
    return $gids;
    
  } else {
    
    // We are editing an existing node
    return _group_context_handler_entity($entity_type, $form_state[$entity_type]);
  }
  
}
JayDarnell’s picture

I opted to move my custom code into a standalone module of its own until og_context can be officially patched to address these two scenarios.

clemens.tolboom’s picture

Checking with views #2358049: Contextual filter - Provide default value - Content ID from URL not working on ajax requests and it's solution it only replies on the server side. Why is client side js needed?

In #9 @amitaibu suggest to use https://api.drupal.org/api/drupal/includes!ajax.inc/function/ajax_get_fo... which I agree. This is used in #18

In #11 @stevector suggests that would be a pain to check for all ajax path

@JvE in #2358049: Contextual filter - Provide default value - Content ID from URL not working on ajax requests added a nice ajax check

+  protected function is_ajax() {
+    $ajax = false;
+    // jQuery sets a header we can check.
+    if (isset($_SERVER['HTTP_X_REQUESTED_WITH']) && $_SERVER['HTTP_X_REQUESTED_WITH'] == 'XMLHttpRequest') {
+      $ajax = true;
+    }
+    else {
+      // Module ajax callbacks have an ajax delivery callback.
+      $router_item = menu_get_item();
+      if (!empty($router_item['delivery_callback']) && $router_item['delivery_callback'] == 'ajax_deliver') {
+        $ajax = TRUE;
+      }
+    }
+    return $ajax;

I don't think we need new contexts as the current form with whatever fields on it triggering ajax breaks og_context.

(my 2 cents)

grndlvl’s picture

Status: Needs review » Needs work
FileSize
4.46 KB
4.89 KB

OK This adds in the support for when creating new entities. It's a little bit messy with some todo's

Obviously I feel that this patch would not be complete without this feature, however, we should open discussion about it's implementation and how we might want to address the issues mentioned in the uploaded patch.

Thanks,

Jonathan

LittleRedHen’s picture

The patch from @grndlvl in #31 is working nicely for me: fixes the problem I was having with needing entity reference fields to be prepopulated when using the inline_entity_form to create them. Thanks!