Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Steps to repeat:
1. Add a Text (plain) field to a config entity (Contact Form)
2. Edit the display of the field and check "Link to the Content" setting in the format settings.
3. Display the field in the default view mode
3. Submit such form .
Expected Results:
User successfully submits contact form.
Actual Results:
PHP Error: Drupal\Core\Entity\EntityMalformedException: The "contact_message" entity cannot have a URI as it does not have an ID in Drupal\Core\Entity\EntityBase->toUrl() (line 191 of /app/core/lib/Drupal/Core/Entity/EntityBase.php).
Comments
Comment #2
KittenDestroyer CreditAttribution: KittenDestroyer as a volunteer commentedI think the solution must be implemented in \Drupal\Core\Field\Plugin\Field\FieldFormatter\StringFormatter::viewElements()
This error happens also in case of Preview of an new entity as seen in:
This issue
Comment #3
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedComment #4
KittenDestroyer CreditAttribution: KittenDestroyer as a volunteer commentedComment #5
KittenDestroyer CreditAttribution: KittenDestroyer at EPAM Systems commentedAdding a ptch for both mentioned issues.
Comment #6
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedPatch #5 didn't pass the test so move to need work.
Comment #7
KittenDestroyer CreditAttribution: KittenDestroyer at EPAM Systems commentedComment #8
KittenDestroyer CreditAttribution: KittenDestroyer at EPAM Systems commentedComment #9
KittenDestroyer CreditAttribution: KittenDestroyer at EPAM Systems commentedThere was some issue with file formating on my side. Fixed.
Comment #10
rensingh99 CreditAttribution: rensingh99 at Red Crackle commentedHi @KittenDestroyer,
The message entity doesn't have the links like canonical that is being used in getEntityUrl.
That's why this error is coming.
The condition which you added "!$entity->isNew()" will affect all entity. So, it will be good to hide the "Link to the Content" option for that entity, which doesn't have canonical links.
Thanks,
Ren
Comment #11
KittenDestroyer CreditAttribution: KittenDestroyer at EPAM Systems commentedHi @rensingh99,
Yes you are right. I've added checks for this case in StringFormatter
Comment #12
KittenDestroyer CreditAttribution: KittenDestroyer at EPAM Systems commentedComment #13
KittenDestroyer CreditAttribution: KittenDestroyer at EPAM Systems commentedComment #14
edmund.dunn CreditAttribution: edmund.dunn at Aten Design Group commented@KittenDestroyer #13 worked for me.
Comment #15
rensingh99 CreditAttribution: rensingh99 at Red Crackle commentedHi @KittenDestroyer
There are no need to check the entity is a new condition. So, it should be removed.
Thanks,
Ren
Comment #17
BerdirYes, we do need the !isNew() check, that fixes node preview with a field is configured like that.
This does need a test.
Comment #18
mohit_aghera CreditAttribution: mohit_aghera as a volunteer and at QED42 commented@Berdir
I've kept
isNew()
check as it is.Added following items:
- Add test only patch
- Add actual test cases.
Comment #19
mohit_aghera CreditAttribution: mohit_aghera at QED42 commentedFixing coding standard issues.
Taken diff from comment #13 only.
Comment #21
mohit_aghera CreditAttribution: mohit_aghera at QED42 commentedComment #22
BerdirA bit unsure about adding a dependency ton contact module in field module, even if it's just a test.
What about doing it with a test entity type in a kernel test, like the existing test coverage in \Drupal\Tests\field\Kernel\String\StringFormatterTest::testStringFormatter? We can either use a test entity type or any of the content entity types in web/core/modules/system/tests/modules/entity_test/src/Entity, it doesn't actually need to be a config entity type.
Comment #23
mohit_aghera CreditAttribution: mohit_aghera at QED42 commented@Berdir I think we probably need a config entity for the same.
I tried to reproduce for node and user by adding a field and following the steps, I was not able to reproduce it.
I'll try to refactor it and implement it with Kernel tests.
Comment #24
BerdirSure, node/user is fine, those have a canonical link template, you need something that doesn't. For example \Drupal\entity_test\Entity\EntityTestLabel.
Comment #25
mohit_aghera CreditAttribution: mohit_aghera at QED42 commented@Berdir
I've refactored the test cases and now we are using kernel tests.
Currently, I've just added test cases for a testing entity by doing render.
Here in the above snippet, we are displaying link_to_entities checkbox for the entities which has a canonical path.
Do you think if we need to cover this under functional tests and check if the checkbox is visible to certain entities only?
Comment #26
BerdirGood question. As far as I see, we don't have existing test coverage for that checkbox nor the summary output and generally rarely do for formatter settings UI I think. So maybe we can get this in without, but will have to be decided by the core maintainers, lets see what they think :)
I did run the the test locally and verified that it fails without the fix.
Comment #29
catchAgreed we don't need to test the individual formatter settings form here, testing the entity display config seems fine.
Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!