Problem/Motivation

Currently, if you create bundle with name equal to entity type (for example, create content type = "node"), the node form id will be 'node_form' instead of 'node_node_form'.

Proposed resolution

Change logic in Drupal\Core\Entity\EntityForm::getFormId() line 78:

if ($bundle != $entity_type) {

Remaining tasks

write tests

User interface changes

No.

API changes

No.

CommentFileSizeAuthor
#11 interdiff.txt581 bytesandypost
#11 2279071-entity-form-id-11.patch3.53 KBandypost
#11 2279071-entity-form-id-11-fail.patch2.73 KBandypost
#6 2279071-entity-form-id-6.patch3.38 KBandypost
#6 2279071-entity-form-id-6-fail.patch2.58 KBandypost
#1 2279071-1.patch749 bytesandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Title: Fragile form id for bundlable entity forms » Fragile form_id generation for bundlable entity forms
Issue summary: View changes
Status: Active » Needs review
FileSize
2279071-1.patch749 bytes

Seems we have no tests at all

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: -Needs tests +Entity Field API

I think no reason in tests

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: +Needs tests

Test should make sure the proper name for for form when bundle name is the same as entity type name

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Should be possible to use the entity_test form for that, the default bundle there is entity_test and it has a bundle key.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Needs work

This absolutely needs tests. It should be easy enough to write a unit test for this, right?
It should cover both entity types with and without a bundle key.

The change looks fine.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs work » Needs review
FileSize
2279071-entity-form-id-6-fail.patch2.58 KB
2279071-entity-form-id-6.patch3.38 KB

Here's a unit test

The last submitted patch, 6: 2279071-entity-form-id-6-fail.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Looks good to me, thanks @andypost!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/tests/Drupal/Tests/Core/Entity/EntityFormTest.php
@@ -0,0 +1,92 @@
+      array('article_node_form', array(
+        'bundle' => 'article',
+        'entity_type' => 'node',
+        'operation' => 'default',
+      )),
+      array('article_node_delete_form', array(
+        'bundle' => 'article',
+        'entity_type' => 'node',
+        'operation' => 'delete',
+      )),
+      array('user_form', array(
+        'bundle' => '',
+        'entity_type' => 'user',
+        'operation' => 'default',
+      )),
+      array('user_delete_form', array(
+        'bundle' => '',
+        'entity_type' => 'user',
+        'operation' => 'delete',
+      )),

There is no test where the entity and bundle have the same name?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

The failed test shows the case:

+++ b/core/tests/Drupal/Tests/Core/Entity/EntityFormTest.php
@@ -0,0 +1,92 @@
+        'bundle' => '',
+        'entity_type' => 'user',
...
+        'bundle' => '',
+        'entity_type' => 'user',

this is the case, because the test uses Entity and it's method bundle() returns entity name

\Drupal\Core\Entity\Entity::bundle() {
return $this->entityTypeId;
}
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs work » Reviewed & tested by the community
FileSize
2279071-entity-form-id-11-fail.patch2.73 KB
2279071-entity-form-id-11.patch3.53 KB
interdiff.txt581 bytes

Added case for user with bundle user.
Now fail test shows 3 failures (1 user bundle and 2 for empty bundle)

The last submitted patch, 11: 2279071-entity-form-id-11-fail.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • webchick committed 673e1e5 on 8.0.x 
    Issue #2279071 by andypost: Fixed Fragile form_id generation for...

Status: Fixed » Closed (fixed)

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