Problem/Motivation

After #2412569: Allow setting the auto-create bundle on entity reference fields with multiple target bundles selecting the bundle(s) of any entity type the ajax request is triggered and calls \Drupal\Core\Entity\Plugin\EntityReferenceSelection\DefaultSelection::validateConfigurationForm which expects form states values to be set as ER Field setting and it broke the DER field setting form.

Proposed resolution

Fix the DER field setting form.

Remaining tasks

Fix the test fails and bug.

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#16 field_setting_form_is-2712545-16.patch11.65 KBjibran
#15 settings-entity-test-base-field-display-handler-settings-target-bundles-entity-test-base-field-display-7.jpg607.58 KBjibran
#12 field_setting_form_is-2712545-12.patch9.69 KBjibran
#2 field_setting_form_is-2712545-2-test-only.patch6.35 KBjibran
#2 field_setting_form_is-2712545-2.patch9.81 KBjibran

Comments

jibran created an issue. See original summary.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Assigned: Unassigned » larowlan
Status: Active » Needs review
StatusFileSize
new field_setting_form_is-2712545-2.patch9.81 KB
new field_setting_form_is-2712545-2-test-only.patch6.35 KB

While working on #2469609: Convert all WebTestBase to BrowserTestBase I found this bug. @larowlan can you please have look at it?

The last submitted patch, 2: field_setting_form_is-2712545-2.patch, failed testing.

The last submitted patch, 2: field_setting_form_is-2712545-2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: field_setting_form_is-2712545-2-test-only.patch, failed testing.

The last submitted patch, 2: field_setting_form_is-2712545-2-test-only.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Ok

OnkelTem’s picture

OnkelTem commented

I have a related issue.

$triggering_element = $form_state->getTriggeringElement();
$parents = $triggering_element['#parents'];
if (array_shift($parents) === 'settings') {
  // ...
}

the if-part never triggers here on Drupal 8.0.5.
Particularly, $parents = $triggering_element['#parents']; results to ['submit'] array i.e. w/o 'settings'.

OnkelTem’s picture

OnkelTem commented

I've checked the patch with the latest Drupal 8.1.x (thinking that the patch logic rely on some latest changes) and it doesn't seem to work properly - the if-content is still unreachable.

For example, if you create a DER field referencing a non-bundled entity like "User", then settings>user>handler_settings>target_bundles will get value of "[]", which prevents autocomplete to work as it expects either not empty array of bundles or NULL (which obviously would happen in the 320th line if it was called).

OnkelTem’s picture

OnkelTem commented

What do you think:

diff --git a/src/Plugin/Field/FieldType/DynamicEntityReferenceItem.php b/src/Plugin/Field/FieldType/DynamicEntityReferenceItem.php
index 0860a99..c85f26c 100644
--- a/src/Plugin/Field/FieldType/DynamicEntityReferenceItem.php
+++ b/src/Plugin/Field/FieldType/DynamicEntityReferenceItem.php
@@ -341,6 +341,16 @@ class DynamicEntityReferenceItem extends EntityReferenceItem {
         'handler_submit',
       ]);
     }
+    // Addition
+    $field = $form_state->getFormObject()->getEntity();
+    // @todo inject this
+    $entity_types = \Drupal::entityManager()->getDefinitions();
+    $settings = $field->getSettings();
+    foreach (array_keys(array_intersect_key($settings, $entity_types)) as $entity_type_id) {
+      if ($form_state->getValue(['settings', $entity_type_id, 'handler_settings', 'target_bundles']) === []) {
+        $form_state->setValue(['settings', $entity_type_id, 'handler_settings', 'target_bundles'], NULL);
+      }
+    }
   }
 
   /**
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Thanks @OnkelTem. I think #10 makes a lot of sense. If you can do a patch it'd be great but if not I'll have a look at it on weekend if Lee won't beat me to it.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review
StatusFileSize
new field_setting_form_is-2712545-12.patch9.69 KB

@OnkelTem can you try this?

Status: Needs review » Needs work

The last submitted patch, 12: field_setting_form_is-2712545-12.patch, failed testing.

The last submitted patch, 12: field_setting_form_is-2712545-12.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
StatusFileSize
new settings-entity-test-base-field-display-handler-settings-target-bundles-entity-test-base-field-display-7.jpg607.58 KB

:/

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review
StatusFileSize
new field_setting_form_is-2712545-16.patch11.65 KB

Here is a green patch.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/tests/src/FunctionalJavascript/DynamicEntityReferenceTest.php
@@ -0,0 +1,118 @@
+   * Tests if the toolbar can be toggled with JavaScript.

c/p issue

other than that, rtbc

great work

  • jibran committed 03a37a7 on 8.x-1.x 
    Issue #2712545 by jibran, OnkelTem, larowlan: Field setting form is...
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Reviewed & tested by the community » Fixed

Thanks @larowlan for the review.
Fixed on commit

diff --git a/tests/src/FunctionalJavascript/DynamicEntityReferenceTest.php b/tests/src/FunctionalJavascript/DynamicEntityReferenceTest.php
index 5262e92..52eb284 100644
--- a/tests/src/FunctionalJavascript/DynamicEntityReferenceTest.php
+++ b/tests/src/FunctionalJavascript/DynamicEntityReferenceTest.php
@@ -66,7 +66,7 @@ class DynamicEntityReferenceTest extends JavascriptTestBase {
   }
 
   /**
-   * Tests if the toolbar can be toggled with JavaScript.
+   * Tests field settings of dynamic entity reference field.
    */
   public function testFieldSettings() {
     $assert_session = $this->assertSession();

Status: Fixed » Closed (fixed)

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

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Assigned: larowlan » Unassigned