With ECK, it is possible to create an entity that has no Title (label) field. Auto Entity Label still tries to save a label for content of this type and generates an error in the dblog. If this is attempted during a batch run, the batch UI freezes.
1. Install ECK and Auto_EntityLabel
1. Create an Entity with ECK. Don't enable the Title field.
1. Create a Bundle within your new Entity, give it a field or two, and then create a piece of content for that bundle.
1. Manage that Bundle's Automatic Entity Labels, choosing to enable one of the "Automatically" options. Fill in a pattern.
1. Optional: tick "Resave All Labels". This will cause AEL to regen the label for the content you just created.
1. Verify Batch Operation starts, and never completes.
1. Verify error in dblog:
Error: Call to a member function setValue() on null in Drupal\auto_entitylabel\AutoEntityLabelManager->setLabel() (line 177 of /app/web/modules/contrib/auto_entitylabel/src/AutoEntityLabelManager.php).
A similar dblog entry is entered when a piece of content is saved from its edit dialog.
Tracing through the code, it appears that AutoEntityLabelManager::hasLabel()'s call to $definition->hasKey('label') is returning a false positive, because the entity has a label definition but does not indeed have a label field. So AELM::hasLabel() returns TRUE.
Even $label_name = $this->getLabelName() on line 176 succeeds with a $label_name of "title", but $this->entity->$label_name->setValue($label) still fails with the setValue() on null error when called.
Identify how to determine whether the label field actually exists before trying to save, or gracefully exit when a save fails.
Probably none, unless you want to hide the "Manage auto entity labels" local task if the label isn't setValue()able.
Not an API change, just one of more internal logic changes.
None. I don't think there is a need to track whether the label is writeable in the data model.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3387731_12.patch | 764 bytes | aditi saraf |
| #12 | interdiff_8_12.txt | 687 bytes | aditi saraf |
| #8 | fixed-error-when-no-label-field-exists-3387731.patch | 540 bytes | viren18febs |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
josephcheekI added a bit of code to check whether the field definition for the label field exists before attempting to set the label. No MR yet because it needs checking, testing, rewriting, debating, rewriting again, etc. https://git.drupalcode.org/issue/auto_entitylabel-3387731/-/commit/5f124...
Comment #4
josephcheekok I couldn't figure out how to get a diff to patch composer without creating a MR, so MR created 8-).
Comment #5
viren18febs commentedHi @josephcheek
I have fixed the issue and created a patch file for your reference. please review
Comment #6
aditi saraf commentedMoving this to NW as patch failed to apply .
Comment #7
aditi saraf commentedWorking on this issue
Comment #8
viren18febs commentedAdded new patch!
Comment #9
aditi saraf commentedComment #10
aditi saraf commentedHi @viren moving this to NW as some composer error is showing in your patch file .
Comment #11
aditi saraf commentedi am working on this .
Comment #12
aditi saraf commentedHi ,
please reivew
Comment #13
aditi saraf commentedComment #14
viren18febs commentedThis is the reason behind composer error!
Drupal core development requires Composer 2.3.6, but Composer 2.2.7 is installed. Please run 'composer self-update'.Comment #15
anybodyPlease update the MR to be up to date!
Comment #16
dqdThis should be at least "Normal" not "Minor" if we decide to go that route... And actually - in this state of solution - it would be rather a Feature request, not a bug. Why? Well because it wasn't in the project's initial scope to deal with badly managed bulk actions where content with no label has been added to AEL routines or to deal with crash testing the module against types with have no label at all. Types without label shouldn't even have the settings for Auto EntityLabel available TBH. That would be the bug then for me. But OK, there are always corner cases.
Short off-topic: There is no such thing like your or my module. This is an Open Source project based on community work under GPL license which lives from the support of every contributor and their motivations and ideas. This can cause different opinions in how things need to be solved, yes, and I agree with you regarding the LabelManager. But I would even go further and argue why do we fix a module called "Auto EntityLabel" regarding an issue appearing when no label exist? Why do you try to even set up AEL for this type?
Thanks for all the efforts and reports and patches in here but please elaborate more on why this is a required feature or bug fix. The patch looks great (#4 and #12) but to place a condition like this in here increases performances hogging, especially on large bulk requests like mentioned in this issue here multiple times. I would argue we rather need a condition which disables AEL completely on types without label. Because these types are completely out of the scope and meaning of this module.
I think this needs further discussion before re-rolling and committing.