amitaibu / og

A fork to work on OG8
https://github.com/Gizra/og
29 stars 16 forks source link

OgComplex::massageFormValues() should not assume the location of the "other groups" field #180

Open pfrenssen opened 8 years ago

pfrenssen commented 8 years ago

In OgComplex::massageFormValues() the values of the "other groups" field are appended to the values of the "groups audience" field. This is not implemented correctly. It is currently assumed that the widget is always located in the same location as in the entity edit form, but this is obviously not always true:

  public function massageFormValues(array $values, array $form, FormStateInterface $form_state) {
    // It is not at all certain that the other_groups field is located
    // at $form['og_group_ref*']['other_groups'] (*or similar).
    foreach ($form[$this->fieldDefinition->getName()]['other_groups'] as $key => $value) {
      // ...
    }
    // ...
  }

The documentation of the method also indicates that the $form parameter does not necessarily represent the entire form, it also might be a form element that is part of a larger form. We need to rethink the way we are dealing with this situation.

One place where this is currently causing errors is in the field edit form. The widget is rendered here so you can provide default values.

For example these steps will trigger this false behaviour:

  1. Turn the 'article' content type into a group content type.
  2. Go to the field edit form of the groups audience field (admin/structure/types/manage/article/fields/node.article.og_group_ref).
  3. Submit the form.
  4. The following error occurs and the default value of the "other groups" field is not saved:
Notice: Undefined index: og_group_ref in Drupal\og\Plugin\Field\FieldWidget\OgComplex->massageFormValues() (line 316 of modules/contrib/og/src/Plugin/Field/FieldWidget/OgComplex.php).
Drupal\og\Plugin\Field\FieldWidget\OgComplex->massageFormValues(Array, Array, Object) (Line: 375)
Drupal\Core\Field\WidgetBase->extractFormValues(Object, Array, Object) (Line: 319)
Drupal\Core\Field\FieldItemList->defaultValuesFormValidate(Array, Array, Object) (Line: 158)
Drupal\field_ui\Form\FieldConfigEditForm->validateForm(Array, Object)
call_user_func_array(Array, Array) (Line: 88)
Drupal\Core\Form\FormValidator->executeValidateHandlers(Array, Object) (Line: 275)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object, 'field_config_edit_form') (Line: 124)
Drupal\Core\Form\FormValidator->validateForm('field_config_edit_form', Array, Object) (Line: 574)
Drupal\Core\Form\FormBuilder->processForm('field_config_edit_form', Array, Object) (Line: 319)
Drupal\Core\Form\FormBuilder->buildForm('field_config_edit_form', Object) (Line: 79)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 128)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 577)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 129)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 102)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 139)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 62)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 62)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 103)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 82)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 55)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 637)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Let's wait until the big refactoring in #161 is done before tackling this.

RoySegall commented 8 years ago

Yesterday I managed to process the values from the dyanmic field and the other groups.