Hello! Spent some time with ECK today and it works great as I quickly created a Web Links entity type on a website for users to manage links similar to the Web Links module.
I stumbled upon a bug when attempting to set Base Field Title Overrides for a bundle while creating it step by step. However, saving the bundle without Base Field Title Overrides works and then editing the bundle to set the Base Field Title Overrides does work.
Problem/Motivation
Important: I've only received this bug while creating a new bundle type, not when editing an entity type bundle.
Steps to Reproduce
Create a new bundle step by step.
Go to Structure -> ECK Entity Types
Click on the button labelled "Add entity type"
In the field labelled "Label" entered the text "Test1" and checked all of the boxes for "Available Base Fields". Clicked the button labelled "Create entity type".
For Entity Type "Test1" clicked the button labelled "Add bundle"
Entered in the fields "Name":"B1" and "Description":"Test Bundle 1". Clicked on the fieldset "Base Field Title Overrides" and for the "Title" entered "Sensible Default Here". Clicked the button labelled "Save Bundle".
Almost the WHITE SCREEN OF DEATH ... "This website encountered an unexpected error. Please try again later." Checked the Recent logs for the following:
LogicException: Missing bundle entity, entity type web_link_type, entity id default_web_link. in Drupal\Core\Entity\EntityType->getBundleConfigDependency() (line 914 of /PROJECT-PATH/core/lib/Drupal/Core/Entity/EntityType.php).
Proposed resolution
Workaround: Save the bundle first and then edit the entity type bundle to set defaults.
Continuing from the error above
Stepped back a page and removed the value for the "Title" field. Clicked the "Save bundle" button.
It saved!
Clicked Edit for the "Test Bundle 1".
Under the fieldset of "Base Field Title Overrides" entered "Sensible Default Here on Edit" in the "Title" field. Clicked "Save bundle" button.
It saved!
Comment | File | Size | Author |
---|---|---|---|
#10 | interdiff-3064353-9-10.txt | 1.64 KB | tetranz |
#10 | eck-title-override-on-new-3064353-10.patch | 9.08 KB | tetranz |
#9 | interdiff-3064353-7-9.txt | 2.64 KB | tetranz |
#9 | eck-title-override-on-new-3064353-9.patch | 8.75 KB | tetranz |
#7 | interdiff-3064353-6-7.txt | 2.38 KB | tetranz |
Comments
Comment #2
tetranz CreditAttribution: tetranz at Third and Grove commentedI think this fixes it. I moved the field update code below where we know the bundle exists.
I'm planning to do a patch to make the descriptions editable but ran into this.
Comment #3
manuel.adanMakes sense. #2 works, but we need test coverage on this, it was not added in the original issue #2839979: Custom label for Title (D8 edition)
Comment #4
tetranz CreditAttribution: tetranz at Third and Grove commentedok, I'll look at doing some tests.
Comment #5
tetranz CreditAttribution: tetranz at Third and Grove commentedHere's an improved BundleCRUDTest which includes title overrides. Without my fix so this test would fail.
Unfortunately the test fails anyway because of deprecations. I have addressed that here https://www.drupal.org/project/eck/issues/3066452
Comment #6
tetranz CreditAttribution: tetranz at Third and Grove commentedHere is the test including my fix so this will pass if 3066452 is merged.
This test only proves that we can create and edit bundles with title overrides. I will now take a look at the EntityCRUDTest. That needs to check that we really do get the correct labels when editing an instance of the entity.
Comment #7
tetranz CreditAttribution: tetranz at Third and Grove commentedThis adds a test to EntityCRUDTest which creates an entity with title overrides and checks that the overridden labels appear on the page.
It's still a work in progress. Places where I've used $this->randomMachineName() there really should be $this->randomString() but that runs into escaping issues. I don't think there are real escaping issues with the module but the rendering does not escape quotes but $this->assertSession()->assertEscaped() does escape quotes so it's hard to test for correctness. I'll study what Node does because it allows you to edit the title label much like we're doing here.
I think there's a separate problem with the 'changed' base field not appearing on the edit form. I'll verify that and create an issue.
Comment #8
tetranz CreditAttribution: tetranz at Third and Grove commentedI created https://drupal.org/project/eck/issues/3077297 for the 'Changed' field issue. These tests will fail while 'Changed' is presented as an editable field.
Comment #9
tetranz CreditAttribution: tetranz at Third and Grove commentedI looked at a few other modules and most seem to use randomMachineName() for labels etc so I'll stay with that. I increased the length of the randoms to 16.
Comment #10
tetranz CreditAttribution: tetranz at Third and Grove commentedThe "Changed" field does nor appear on the entity form. At first I thought that was a bug but it is not.
I think I'm done for now. This will pass if https://www.drupal.org/project/eck/issues/3066452 is also applied.
Comment #11
sim_1Manually tested on Drupal 8.8.x-dev with the latest eck-1.x-dev. I was able to reproduce the issue before applying the patch. And then it appears to be fixed after applying the patch.
Like @tetranz said, I applied the patch from https://www.drupal.org/project/eck/issues/3066452 and ran the module tests locally and they all passed.
Marking as RTBC unless anyone things further changes are needed.
Comment #12
pifagorComment #14
pifagor