/**
   * Automatic label is optional. Will only be generated if no label was given.
   */
  const OPTIONAL = 2;

But it generates label even if label is defined.

    $node = Node::create([
      'type' => 'page',
      'uid' => 0,
      'title' => '',
    ]);
    $node->save();
    // 'title' generated: OK.


    $node = Node::create([
      'type' => 'page',
      'uid' => 0,
      'title' => $this->randomMachineName(20),
    ]);
    $node->save();
    // 'title' generated: WRONG.
CommentFileSizeAuthor
#14 3077387-14.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch2.92 KBredndahead
#12 screenshot.png50.45 KBsuper_romeo
#11 screenshot.png58.54 KBsuper_romeo
#5 3077387-5.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch3 KBsuper_romeo
#3 3077387-3.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch3.08 KBsuper_romeo

Comments

super_romeo created an issue. See original summary.

perpignan’s picture

perpignan commented

+1 - Same issue ! downgrade to auto_entitylabel 8.x-2.1-beta1 fix the problem

super_romeo’s picture

super_romeo commented
Status: Active » Needs review
StatusFileSize
new 3077387-3.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch3.08 KB

Patch + first test for this module FOR FREE :)

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Needs work

Sadly the patch needs work, it contains ugly characters. I didn't test it yet for functionality.

super_romeo’s picture

super_romeo commented
Status: Needs work » Needs review
StatusFileSize
new 3077387-5.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch3 KB
kaszarobert’s picture

kaszarobert
Primary language Hungarian
commented

Patch #5 works for me. Thank you!

chrotto’s picture

chrotto commented

This issue is the same in 3.x-bet2. I solved it with #2 but this version is flagged as deprecated so I hope a new version without this issue soon is available.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs review » Reviewed & tested by the community

I reviewed the code and the change in the logic looks correct to me. The test looks correct too. Together with #6, marking RTBC.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Priority: Normal » Major

Raising prio as of data loss.

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Reviewed & tested by the community » Needs work

@geek-merlin: agreed.

/web/modules/contrib/auto_entitylabel$ git apply -v 3077387-3.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch
3077387-3.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch:25: trailing whitespace.
<?php
3077387-3.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch:26: trailing whitespace.
3077387-3.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch:27: trailing whitespace.
namespace Drupal\Tests\auto_entitylabel\Kernel;
3077387-3.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch:28: trailing whitespace.
3077387-3.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch:29: trailing whitespace.
use Drupal\auto_entitylabel\AutoEntityLabelManager;
Checking patch auto_entitylabel.module...
Checking patch tests/src/Kernel/AutoEntityLabelTest.php...
Applied patch auto_entitylabel.module cleanly.
Applied patch tests/src/Kernel/AutoEntityLabelTest.php cleanly.
warning: squelched 74 whitespace errors
warning: 79 lines add whitespace errors.

Anyone interested to reroll. Please beware of #2856225: Replace all usages of deprecated EntityManagerInterface already committed to dev.

super_romeo’s picture

super_romeo commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new screenshot.png58.54 KB

Applies ok with cweagans/composer-patches.

super_romeo’s picture

super_romeo commented
StatusFileSize
new screenshot.png50.45 KB

Wrong screenshot.

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Reviewed & tested by the community » Needs work

THanks for your work on this @super_romeo. But - Please do not set RTBC as patch provider and let others test your patch. And I actually would prefer a patch working without cweagans/composer-patches. Patch needs to be able to apply to latest dev git cloned from D.O. Thanks for understanding.

redndahead’s picture

redndahead commented
Status: Needs work » Needs review
StatusFileSize
new 3077387-14.auto_entitylabel.OPTIONAL-generates-label-even-if-label-is-defined.patch2.92 KB

Been a while since I've created a patch, but hopefully this re-roll is correct.

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Needs review » Reviewed & tested by the community

Dreditor code review looks good so far and ...

/d8/web/modules/contrib/auto_entitylabel$ git apply -v 3077387-14.auto_entitylabel.OPTIONAL-generates-label-ev en-if-label-is-defined.patch
Checking patch auto_entitylabel.module...
Checking patch tests/src/Kernel/AutoEntityLabelTest.php...
Applied patch auto_entitylabel.module cleanly.
Applied patch tests/src/Kernel/AutoEntityLabelTest.php cleanly.

Patch applies cleanly.

Manual test on latest Drupal core installation with patch applied to latest auto_entitylabel:8x-dev and testing 2 articles, one with no title set and one with title set, this issue seems to be fixed. Also, I do not see any arguable code injections here which would need further discussion. RTBC from me.

Thanks to all for the hard work on this.

  • diqidoq committed 8d6761b on 8.x-3.x authored by redndahead 
    Issue #3077387 by super_romeo, redndahead: OPTIONAL generates label even...
dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Reviewed & tested by the community » Fixed

Thanks @all! 1++

Status: Fixed » Closed (fixed)

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