Set Label runs two times on node creation

Created on 21 August 2019, over 5 years ago
Updated 20 September 2024, 2 months ago

Problem/Motivation

When creating a new entity (e.g. a new node), "setLabel()" will be executed twice. First in hook_entity_presave() and after in hook_entity_insert(). This can potentially interfere with other third party modules, custom tokens or other custom logic.

Steps to reproduce

  • Install this module
  • Install another module implementing hook_entity_presave() (e.g. tgmnt)
  • Save the entity, that both modules modify
  • auto_entitylabel will override the logic of the other modules hook implementation.

Proposed resolution

Save the entity only once, while preserving the old module's logic supporting all of the tokens.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Needs review

Version

3.0

Component

Code

Created by

🇬🇷Greece Orestis Nerantzis Thessaloniki

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

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States euk

    Attaching a refactored patch to address the following:

    - coding standards (I hope I caught all of them),
    - added the missing AutoEntityLabelManager class reference,
    - re-rolled the patch against 3.x (it was something below 3.x it and was not applying),
    - fixed issue with info.yml file - the patch would not apply due to presence of the auto-generated package information.

    This patch applied cleanly and looks like it fixed our issue with duplicate aliases - since we don't rely on any funky tokens, this should work in our situation.

  • 🇺🇸United States euk

    Well, what do you know - it is failing here due to info.yml again. Attaching same file with a slight change around that hunk.

  • 🇺🇸United States euk

    Last attempt...

  • 🇦🇺Australia VladimirAus Brisbane, Australia

    Creating MR instead of patches.
    Make sure to push your changes to MR. 🎉

  • Status changed to Needs review almost 2 years ago
  • 🇦🇺Australia VladimirAus Brisbane, Australia

    Applied patch #77 to MR with few extra coding standards.

  • Status changed to Needs work almost 2 years ago
  • 🇺🇸United States SocialNicheGuru

    No longer applies to 3.0 version

  • 🇦🇺Australia VladimirAus Brisbane, Australia

    @SocialNicheGuru did you use diff from MR?

  • Status changed to Needs review almost 2 years ago
  • 🇦🇺Australia VladimirAus Brisbane, Australia

    All works for me

      - Installing drupal/auto_entitylabel (3.0.0): Cloning 8.x-3.0 from cache
      - Applying patches for drupal/auto_entitylabel
        https://git.drupalcode.org/project/auto_entitylabel/-/merge_requests/10.diff (Set Label runs two times on node creation https://www.drupal.org/project/auto_entitylabel/issues/3076302)
    
  • Status changed to Needs work almost 2 years ago
  • 🇺🇸United States euk

    From what I understand, at least locally (with Composer), when it comes to info.yml file failing the patch - it looks like it conflicts with the lines auto-generated by DO packaging script. However, I was able to figure out a workaround - just adding an extra context line... IDK

  • First commit to issue fork.
  • 🇺🇸United States SocialNicheGuru

    the MR does not apply to 3.0.

    - Installing drupal/auto_entitylabel (3.0.0): Extracting archive
    - Applying patches for drupal/auto_entitylabel
    https://git.drupalcode.org/project/auto_entitylabel/-/merge_requests/10.... (Set Label runs two times on node creation - https://www.drupal.org/project/auto_entitylabel/issues/3076302 🐛 Set Label runs two times on node creation Needs review )
    Could not apply patch! Skipping. The error was: Cannot apply patch https://git.drupalcode.org/project/auto_entitylabel/-/merge_requests/10....

  • 🇺🇸United States msielski

    To apply the diff (from the MR) with a composer install, #84 is correct you just need one more blank line of context. I'm not sure how to adjust this so it works for non-composer and composer installs, but my updated patch is attached and applies to 3.0.0. I am skipping submitting this change to the MR since it would break the patch for non-composer users. Anyone who knows the best way to handle this problem (I've seen it before with patching info.yml files) please let us know.

  • 🇮🇳India vikas shishodia

    Hi,
    I am using 3.0.0 version of 'Entity Auto Label' module. With this version i am facing issue with content moderation, when I create a content first time in Draft state it does not reflect in Moderated Content tab. However if I resave that content then it start reflecting in Moderated content tab.

    To fix this I used the Re roll patch provided in https://www.drupal.org/project/auto_entitylabel/issues/3076302#comment-1... 🐛 Set Label runs two times on node creation Needs review Using this patch my issue got resolved.

    But now when I am creating node first time then it is invoking hook_entity_insert, that is fine. But it also invoking hook_entity_update due to which, the code that i only want to run on node update is also triggering at the node create issue.

    So to fix both the above mentioned issue, I just use the 3.0.0 version and removed the hook_entity_insert() from module file then my both use cases are working perfectly.

    So need to verify is there any issue if I remove hook_entity_insert() for module file from module 3.0.0 version? Please clarify.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.1 + Environment: PHP 7.3 & MySQL 8
    last update 5 months ago
    Composer error. Unable to continue.
  • Status changed to Needs review 5 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.1 + Environment: PHP 7.3 & MySQL 8
    last update 5 months ago
    Patch Failed to Apply
  • 🇦🇺Australia VladimirAus Brisbane, Australia

    Resolved conflicts.

  • Status changed to Needs work 4 months ago
  • 🇩🇪Germany Anybody Porta Westfalica

    So to fix both the above mentioned issue, I just use the 3.0.0 version and removed the hook_entity_insert() from module file then my both use cases are working perfectly.

    That's getting resolved in: 🐛 auto_entitylabel_entity_insert saves the entity and that is not allowed Needs review

    PHPCS fails currently!

  • First commit to issue fork.
  • Pipeline finished with Failed
    3 months ago
    Total: 177s
    #248594
  • Pipeline finished with Success
    3 months ago
    Total: 166s
    #249153
  • 🇩🇪Germany Anybody Porta Westfalica

    @Grevil this is a quite heavy, but important change, as it also fixes issues like 🐛 Incompatible with tmgmt (auto-translation) Active and similar cases. Still I see some existing risks like 🐛 auto_entitylabel_entity_insert saves the entity and that is not allowed Needs review .

    How would you rate the new and existing test coverage?

    At least, I'd say we should keep this in dev for some time. Perhaps it might also be a good chance to switch to semver and push this into a 4.x branch and keep 8.x-3.x untouched?

    Activity here shows, that this is a really important fix!

  • 🇩🇪Germany Grevil

    @vikas shishodia its all commented in code now:

    // To support tokens that are only available after the entity has
    // been created (like id tokens) trigger a second save.
    // To do this without corrupting the entity run the
    // save operation at the end of the entity insert transaction.
    // To run code at the entity of the entity insert transaction
    // we need to register a transaction shutdown function.
    // Check the autolabel settings for the entity to see if we
    // need to register the shutdown function.

    "auto_entitylabel_entity_insert()" will only run, if we set the new "New content behavior" setting to "Create label after first save", which basically uses the pre-patch logic, but supports more tokens.

  • Pipeline finished with Canceled
    3 months ago
    Total: 79s
    #249224
  • Pipeline finished with Canceled
    3 months ago
    Total: 157s
    #249225
  • Status changed to Needs review 3 months ago
  • 🇩🇪Germany Grevil

    Ok, I hope the new update hook description makes this clearer:

    What do you guys think?

  • Pipeline finished with Failed
    3 months ago
    Total: 209s
    #249230
  • 🇩🇪Germany Grevil

    Alright, everything seems to work great! Just tested the update hook and updated its message a bit to users are not left in the dark with this new helpful feature!

    I also renamed the new config "new_entity_action" to "new_content_behavior" to synchronize it with its form label. And I feel like that makes it clearer.

    RTBC from my side! Final review by @Anybody regarding the changes I made.

  • Pipeline finished with Failed
    3 months ago
    Total: 169s
    #249238
  • Pipeline finished with Failed
    3 months ago
    Total: 463s
    #249248
  • Status changed to RTBC 3 months ago
  • 🇩🇪Germany Grevil

    Alright, that's it! RTBC @VladimirAus!

    Wonderful work everyone!

    I updated the issue summary.

  • Pipeline finished with Skipped
    3 months ago
    #251611
  • Status changed to Fixed 3 months ago
  • 🇦🇺Australia VladimirAus Brisbane, Australia

    Thank you, everyone!
    Commited! 🥂

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

  • Status changed to Needs review 3 months ago
  • 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin

    This one here has been merged and committed possibly far to quick. Follow up: 🐛 New changes to set label logic cause integrity constraint violation Active needs further investigation.

  • 🇺🇸United States justcaldwell Austin, Texas

    Since updating to 3.3, we're experiencing the issue described in 🐛 Pattern not being respected Active . I haven't had time to confirm, but is suspect it might also be related to the changes made here (?).

  • 🇩🇪Germany Anybody Porta Westfalica

    @jernejmramor yes, we're investigating the regressions, also see #100.

    In many cases, switching the New content behavior setting helps resolving the issue. Please let us know, if and which worked for you!

  • 🇺🇸United States justcaldwell Austin, Texas

    Yes, switching to 'Create label before first save' worked for us 🐛 Pattern not being respected Active .

  • 🇩🇪Germany Anybody Porta Westfalica

    @justcaldwell thanks!!

    So should we make that the default option for new installations for now, to mitigate the problem?
    I think that would make sense... (in a separate issue perhaps).

  • 🇩🇪Germany Grevil

    So should we make that the default option for new installations for now, to mitigate the problem?

    That's already the case! The old option is just used for exsiting installations.

  • 🇨🇭Switzerland michèle

    Same problem here with version 8.x-3.3:

    I need the "Create label AFTER first save", because I need the node ID:
    [node:field_usecase:entity:name] - [node:field_category:entity:field_shorttext] (#[node:nid])

    After the creation of a new node, the title is used as email subject:

    function mymodule_mail($key, &$message, $params) {
      ...
      $message['subject'] = t('New reservation: @title', array('@title' => $params['node_title']), $options);
      ...
    }
    

    The mail function is called in mymodule_node_insert().

    Resulting node title (correct!): Meeting - external (#2449)
    Resulting email subject: New reservation: %AutoEntityLabel: f0039c70-6fdc-4729-be7b-f834a23bf31a%

    With version 8.x-3.2, the Resulting email subject was correctly set to "New reservation: Meeting - external (#2449)".

    When I set "Create label BEFORE first save", the resulting email subject is correct (!), but the node title is wrong:
    Resulting node title (lacking ID): Team - internal (#)
    Resulting email subject (correct!): New reservation: Team - internal (#2450)

    Is it possible to go back to the old 8.x-3.2 version until this issue is fixed, or will I have problems due to a lack of backwards compatibility?

    Thank you very much!

  • 🇨🇭Switzerland michèle

    I can confirm that version 8.x-3.2 can be installed over 8.x-3.3 without any problem.

  • 🇨🇦Canada liquidcms

    Can anyone confirm what combination of code/patch works here?

    I have had https://git.drupalcode.org/project/auto_entitylabel/-/merge_requests/10.... running against 3.x-dev for quite some time; but it no longer applies.

  • 🇩🇪Germany luenemann Südbaden, Germany

    @liquidcms MR !10 got committed in #97 and released in 3.3.

Production build 0.71.5 2024