Contrib.social

call to AutoEntityLabelManager->setLabel() when no label field exists

Open on Drupal.org →
Created on 16 September 2023, over 1 year ago
Updated 30 August 2024, 7 months ago

Problem/Motivation

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.

Steps to reproduce

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.

Proposed resolution

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.

Remaining tasks

Identify how to determine whether the label field actually exists before trying to save, or gracefully exit when a save fails.

User interface changes

Probably none, unless you want to hide the "Manage auto entity labels" local task if the label isn't setValue()able.

API changes

Not an API change, just one of more internal logic changes.

Data model changes

None. I don't think there is a need to track whether the label is writeable in the data model.

🐛 Bug report
Status

Postponed: needs info

Version

3.0

Component

Code

Created by

🇺🇸United States josephcheek Provo, UT

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @josephcheek
  • Comment over 1 year ago →
    🇺🇸United States josephcheek Provo, UT

    I 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...

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.7 + Environment: PHP 7.3 & MySQL 8
    last update over 1 year ago
    Composer error. Unable to continue.
  • @josephcheek opened merge request.
  • Comment over 1 year ago →
    🇺🇸United States josephcheek Provo, UT

    ok I couldn't figure out how to get a diff to patch composer without creating a MR, so MR created 8-).

  • Status changed to Needs review over 1 year ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 7.3 & MySQL 8
    last update over 1 year ago
    Patch Failed to Apply
  • Comment over 1 year ago →
    🇮🇳India viren18febs

    Hi @josephcheek
    I have fixed the issue and created a patch file for your reference. please review

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India AditiVB

    Moving this to NW as patch failed to apply .

  • Assigned to AditiVB
  • Comment over 1 year ago →
    🇮🇳India AditiVB

    Working on this issue

  • Status changed to Needs review over 1 year ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 7.3 & MySQL 8
    last update over 1 year ago
    Composer error. Unable to continue.
  • Comment over 1 year ago →
    🇮🇳India viren18febs

    Added new patch!

  • Issue was unassigned.
  • Comment over 1 year ago →
    🇮🇳India AditiVB
  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India AditiVB

    Hi @viren moving this to NW as some composer error is showing in your patch file .

  • Assigned to AditiVB
  • Comment over 1 year ago →
    🇮🇳India AditiVB

    i am working on this .

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 7.3 & MySQL 8
    last update over 1 year ago
    Composer error. Unable to continue.
  • Comment over 1 year ago →
    🇮🇳India AditiVB

    Hi ,
    please reivew

  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India AditiVB
  • Comment over 1 year ago →
    🇮🇳India viren18febs

    This 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'.

  • Status changed to Needs work 8 months ago
  • Comment 8 months ago →
    🇩🇪Germany Anybody Porta Westfalica

    Please update the MR to be up to date!

  • Status changed to Postponed: needs info 7 months ago
  • Comment 7 months ago →
    🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin

    This 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.

    But since this isn't my module, I don't get to decide the logic that goes into it 8-/.

    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.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024