Set Label runs two times on node creation

Attaching a patch for one possible approach, as the insert hook is called only when an entity/node is created for the first time, so on edit it will not go into the insert hook.
Second option would be to leave the check, but add a getLabel() method to the decorated_entity, so the check for old label could be done, without generating/setting a new label and not calling the setLabel() twice.

Thanks.

Added to beta4. If you'd like to see a stable release, please help with issues in #3153250: Roadmap for a stable 8.x-3.x release.

Hi,

I composer updated to the 8.x-3.0-beta4 version which includes this patch. I have a custom module which generates an automatic email to the logged in user using hook_node_insert when a node is created (the content type uses auto entity label). After this update, when creating a node, it now sends two emails. Reverting to beta3 resolves the issue. I suspect this patch is causing the node to save twice and therefore fire the hook_node_insert twice when saving a new node (or something like that). Could you take a look and suggest a solution? If it is helpful, the module code is here. Also, I am not sure if it is better to post here or create a new issue, happy to do that that if preferred. Many thanks, -c

If the above "fix" is bad, feel free to reopen. Otherwise, if it's a new problem, create a new ticket. Thanks.

I suspect it is the former, but I've never reopened an issue, how do I do that?

HI @charlie1volley. Could you please be so kind and have the beta 4 version and only change back the part that is changed in this patch, to test if this is really what is causing your issue? As a custom module and code is involved I want to be sure the issue is in the entity label code and with this patch particular, rather then with the custom code or other error somewhere else.

#11: Updating the status does the trick. Looking forward to seeing you results. (If it doesn't look like the problem is here, we can close again.)

Thanks very much DeaOm and colan (I missed that one can edit the issue via the green button up top, sorry about that). Sure, I will upgrade to beta4, test as described and report back.

OK, updated to beta4 and replaced the code that was eliminated by this patch and the custom module now works as designed (only one email sent) for both Node Add and Node Edit forms. The auto label setting is "Automatically generate the label and hide the label field" and revisions are enabled on the content type.

After spending some more time with it, I think what is happening is that with the patch, the auto_entitylabel_entity_insert hook is saving the node twice because the "old label" check is not in place. My understanding of the insert/update hooks is that by virtue of calling it, it will save (so not required to $entity->save(); when using the hook). So when this hook fires without the old label check, it is saving twice: once driven by $entity->save(); and then once driven by the hook itself. Saving the node twice would then result in firing my custom module hook_node_insert twice and creating a duplicate email.

So I reverted to the patch code but commented out $entity->save(); and it appears now to be working as designed (one email generated for either of the node add or edit forms when saved). What do you think, does this solve it without impacting anything else?

BTW thanks very much for your review and support/development of this module, this functionality is critical to my site. -c

Hi @charlie1volley. I suggest creating a new patch without the $entity->save(); and then it can be tested and confirmed it is indeed working/not impacting anything else.

Sorry, although I can kind of work out what the code means looking at it, I've never actually created a patch. I'd be happy to learn how (I imagine this one is probably as simple as they get and a good one to learn on) but I'm having trouble finding what I think is appropriate documentation on how to do this. Could you point me to a resource?

Maybe this can help https://www.drupal.org/patch/create?

OK, thanks, I'll give it a shot...

OK, patch created...

Shoot, sorry, I see now I should have named the file "-20" instead of "-19". I couldn't figure out a way to rename and I didn't want to confuse the issue by uploading a new file, but let me know if it would be better to make another comment and rename.

Also: I could not preview patch application with 'composer update --dry-run' or 'composer install --dry-run' but it applied successfully on 'composer update'. Is there a "--dry-run" type command that can be used for patches?

I had a similar issue because of the node being saved twice and the patch fixed the issue in the sense that it is saved only once.

However I believe that hook_entity_insert won't automatically save the node, so I think manipulating the node doesn't matter if we don't save it.

My question is: do we need the hook_entity_insert?

It seems to decorate (which is already done in auto_entitylabel_entity_presave, so useless) + prevent a new revision creation on insert (not sure to get the purpose of that)

This double save issue appears to be causing a WSOD when using auto entity labels with the core book module. In that case if you try to create a new book on a completely new node you get an SQL duplicate primary key error on save (adding a book outline to an existing node doesn't trigger this issue).

Digging around the issue queue it looks like the hook_entity_insert was added to add support for the [node:nid] token: https://www.drupal.org/project/auto_entitylabel/issues/2920695. Unfortunately, I don't have any suggestions for resolving both these issues.

Disregard, this was in beta3 and has been removed in beta4 by the patch #2 in this thread

We have this in hook_entity_insert:

if ($entity->label() != $old_label) {

So we shouldn't get multiple saves - unless we are using something available only after the initial save like the nid. In that case, I think it's normal behavior to save it twice (not sure why it is causing this issue with books though)

I haven't investigated further, so I don't know why this comparison always fails

I've done a bit more digging and found out why there's a WSOD with the books module.

While the second save in hook_entity_insert is not forbidden it is not recommended (according to the hook_entity_insert documentation at api.drupal.org). This stackexchange post explains the issue https://drupal.stackexchange.com/questions/225612/update-title-field-in-hook-entity-type-insert/225627#225627 (it talks about node_save but I think this still applies to entity->save()). The entity insert hooks run after the entity runs in the middle of the database transaction to save the entity, therefore the node id is available but not everything in saved.

The problem with the core book module is that it uses the node id as the primary key for its own book entities. I'm not quite sure why but book still thinks that there is no book entity associated with the node when the save in hook_entity_insert is called, therefore it tries to create a second outline, causing the duplicate primary key error.

According to the stackexhange post the way to avoid this is to run the second save after the initial database transaction has finished, and some code to do this. I've created a patch with this code in it.

I've only tested on my site. This removes the WSOD for book nodes. I don't use the [node:id] token so I haven't tested if this resolves that issue. Since this code doesn't remove the second save, only delays it, I don't think this will fix the double email issue.

I have thought a bit more and it is becoming a more complex issue:

1. with the patch #2 committed in beta4, we have removed the check which only saves if the node label is different -> we will ALWAYS save twice new nodes, but the second save from hook_insert isn't actually doing anything (not doing decoration) -> we need to fix it
2. if we want to check the label (which was removed by the patch #2) to set the labels, then we have to run setLabel() and this can cause some issues in the case of auto-incrementing token (which was the initial issue reported) - but I don't see any way to check if the label has changed without running setLabel()
3. in some other cases, we need to run setLabel() in hook_entity_insert because the id is only set after the save -> will trigger a double save but I don't see any way around it
4. then there is your (very valid imo) issue Denma, but I think it should be a separate topic

So we have a bit of a catch 22 situation. How can we handle the 2 (incompatible) use cases:
- I need the hook_entity_insert() to run because my title contains elements which are set only after the node is saved
- I need to run setLabel() only once to prevent custom token executed twice?

Should we add an option in the admin:
Option #1 - default: setLabel() would run only on presave hook -> setLabel() will always be ran only once + 1 save
Option #2: setLabel() would run only on insert for new node creation (and presave hook for updated nodes) -> setLabel() will always be ran once, but 2 saves (only use this if the label depends on elements only available after the save)

What do you think?

Probably the question was more directed to the module maintainers/experts, but from my perspective/use, I would say the option suggested in #25 looks like a nice solution to me...

Split the book issue into a separate issue.

I've created a patch which implements @vdsh's suggestion in comment #25 but I've made some tweaks:

This patch adds a setting to each auto entity label configuration page with the two options mentioned in the comment to control when the label gets generated for new entities. In the patch the options are called before first save (Option #1) and after first save (Option #2).

However I've tweaked how Option #2 works. When Option #2 is selected the hook_entity_insert function doesn't run setLabel() but instead just triggers a second save. This second save is run via a shutdown callback so that it runs once the first save transaction is finished. In hook_entity_presave setLabel() is only run on existing node updates when Option #2 is selected (so never on new nodes). The magic part here is that the second save will trigger a rerun of hook_entity_presave but because the second save is deferred until after the first save is finished this second save is updating an existing node, causing the setLabel() function in the second call to hook_entity_presave() to run. In this way setLabel() gets run only once by auto_entitylabel even when we're saving twice.

In addition this also fixes as the deferred save doesn't cause books to trigger a duplicate primary key error. In addition because the entity isn't being altered during hook_entity_insert it eliminated a warning we received during our automated tests which said that we shouldn't alter entities after the save transaction has started.

Note that this patch doesn't take into account cases where another contributed module triggers another save of the entity, in that case setLabel() may run twice. What do you think of adding a third option, one where the automatic label is never created on new entities? That would catch the case where there is another module triggering a second save and the label uses an auto-incrementing token.

I haven't installed the patch, I just read it but it looks good & clean.

However, for the upgrade, if the user doesn't go to select either of the 2 options, $decorated_entity->getNewEntityAction() will return null, and autoentitylabel will not be ran, which seems a bit harsh...

Maybe the patch should provide a migration to set 'before save' to all existing already, or consider nothing set as 'before save'?

As I haven't installed it, I may have missed something

I also wouldn't go with option #3 - not that it's a bad idea - but because I don't have any example in mind of when such a module depending on data only available after the first save would need to resave the opportunity - so may be better to not include it and wait to see usecases for that

Thanks for reviewing the patch @vdsh. I did actually test this case on my site (I've got a lot of node types and I didn't update the configuration for all of them), even with the option not set it still created the label. I think it works because the comparison uses the double equals '==' not triple equals '===', there it sees zero and NULL as equivalent.

To make things more obvious I can update the patch to explicitly set a default for all existing configuration entities and to use the triple equals.

In addition should the default behaviour be changed to Option #2 (the double save)? This means existing sites won't break because their labels use tokens that aren't available until after the first save.

As for option #3 why I thought of it was because of the design of the current site I'm working on. We use the Group module to control authoring access to nodes but we've customised it so that content authors can add new content to their groups from the node/add page. To allow that we need to save new nodes twice, first to get the node id and again when adding it to the group. Since groups is triggering the second save we don't need auto_entitylabel to trigger another save. But since we don't use auto-incrementing tokens nor do we use the node id token we don't need this option (Option #1 is enough). I suggested it in case there were any other examples similar to this. I'm happy to leave it off until there's a need for it.

I think using === is clearer, and would avoid the same misunderstanding that I had by not paying enough attention (plus, it may prevent issues in the future)

So either a migration, or else setting explicitely somewhere (in the get method?) that if we have null, then we return 'after_save' (I personnally think before_save makes more sense for the majority of use cases, but you are right, it is probably better to keep the current behavior (which is double save so always doing after_save)

For your use case (option #3), if you were using NID in auto_entitylabel, I think that you can use before_save. As it will save a first time (without the nid), and then the second time with your additional module in the hook_entity_presave(). So you will only have an issue IF you need autoentity label with nid + you have an autoincrementing token + you are using another module which is doing an additional save on insert() It seems far fetched... and also would not work if you have 2 othe modules doing an additional save on insert()

That's only my take, it would be great if a maintainer could review it as well

Thanks for the feedback @vdsh. I've updated the patch with that feedback:
- Swapped the double equals in both the hook_entity_insert and hook_entity_presave functions to be triple equals when comparing to the new entity action setting.
- Added an update hook to update all existing configurations to have the new entity action configuration option set. Existing configuration sets it to after save (preserving the old behaviour). New configurations default this to before save.
- Added the new setting to the unit test and removed the deprecation notices.

I tested the patch in #32 but am getting below error any time I try to create any content type. I tried changing a config to save once and also not to change existing labels, but same error.

Error: Call to a member function get() on null in Drupal\auto_entitylabel\AutoEntityLabelManager->getConfig() (line 359 of /home/customer/www/rms2.net/public_html/modules/contrib/auto_entitylabel/src/AutoEntityLabelManager.php)
#0 /home/customer/www/rms2.net/public_html/modules/contrib/auto_entitylabel/src/AutoEntityLabelManager.php(234): Drupal\auto_entitylabel\AutoEntityLabelManager->getConfig('status')
#1 /home/customer/www/rms2.net/public_html/modules/contrib/auto_entitylabel/auto_entitylabel.module(111): Drupal\auto_entitylabel\AutoEntityLabelManager->getStatus()
#2 /home/customer/www/rms2.net/public_html/modules/contrib/auto_entitylabel/auto_entitylabel.module(54): auto_entitylabel_prepare_entityform(Array, Object(Drupal\auto_entitylabel\AutoEntityLabelManager))
#3 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/Extension/ModuleHandler.php(539): auto_entitylabel_form_alter(Array, Object(Drupal\Core\Form\FormState), 'node_ct20_form')
#4 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/Form/FormBuilder.php(836): Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object(Drupal\Core\Form\FormState), 'node_ct20_form')
#5 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/Form/FormBuilder.php(279): Drupal\Core\Form\FormBuilder->prepareForm('node_ct20_form', Array, Object(Drupal\Core\Form\FormState))
#6 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/Controller/FormController.php(91): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\node\NodeForm), Object(Drupal\Core\Form\FormState))
#7 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch))
#8 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#9 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/Render/Renderer.php(573): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#10 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#11 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#12 /home/customer/www/rms2.net/vendor/symfony/http-kernel/HttpKernel.php(151): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#13 /home/customer/www/rms2.net/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#14 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#15 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#16 /home/customer/www/rms2.net/public_html/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#17 /home/customer/www/rms2.net/public_html/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#18 /home/customer/www/rms2.net/public_html/core/modules/ban/src/BanMiddleware.php(50): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#19 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\ban\BanMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#20 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#21 /home/customer/www/rms2.net/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /home/customer/www/rms2.net/public_html/core/lib/Drupal/Core/DrupalKernel.php(708): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /home/customer/www/rms2.net/public_html/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#24 {main}
.

Whoops. Looks like I deleted the wrong chunk of code.

charlie1volley try this new patch.

No worries. I tried this patch, a few things:

I first tried with option 2 (saves twice) and created a new node. Works as expected: node label automatically generated and two emails were generated. However, the message that is displayed after creating a new node ("XXXX ...has been created.") does not show the label, instead it has {{ PLACEHOLDER LABEL }} in place of the label.

I then tried with option 1 (saves once) and created a new node. Works as expected: node label automatically generated and one email generated. In this case, the message displays the label as normal.

I then tried with option 1 but edited an existing node. Did not work as expected: no email generated. It seems the hook_node_update that should be generating an email when the node is edited and saved does not fire.

charlie1volley. I had to put the placeholder title in for option 2 because Drupal wouldn't let me save a node without a title. Unfortunately that does affect the new node created message. What I can do is update the patch to include some messages when auto_entitylabel performs its actions (for example, when it's putting in the placeholder and when it's updating the label).

I tested the case where Option 1 was used and an existing node was updated, including updates to the tokens generating the label. The node was updated and the new label was created.

As for your email not being generated when you edited the node did you make any changes to your node before pressing save? If not try making a change, saving and then check if your hook_node_update fires. I've noticed that when Drupal doesn't detect a change in a node between old and new revisions it will not save a new revision of the node, this caused issues with other modules on my site. For those cases I had to force Drupal to create a new revision.

I'm very sorry, I thought I had posted a retraction regarding "not sending an email on node edit" last night. I wanted to see if it editing the node would send the email if the save twice option was selected (I had only tested with save once option) so I ended up testing with each option and it is working fine. Good point about the revisions but for my email module, the user has to check a "Send me an email" box each time for the email to be generated, so the node is actually changed each time. I have no clue otherwise why the emails were not being generated when I tested the first time but I'm pretty confident it didn't have to do with the patch. Thanks for all of your assist & efforts!

This new version of the patch adds some info messages showing when this module updates the label. In addition I've added the token module as a test dependency in an effort to get the kernel tests to run.

Try again. This time the patch was built against the dev branch.

Tested patch and is working as designed. One suggestion on the placeholder message:

"Using placeholder {{ PLACEHOLDER LABEL }} as a temporary title. This will be replaced after the second save."

I think someone who is new to the module might interpret this to mean the node will need to be manually resaved for the auto title to generate and replace the placeholder. Might I suggest something like:

"Automatic title successfully generated. The temporary title {{ PLACEHOLDER LABEL }} has been replaced and can be ignored."

Also, for my own personal use case, the message:

"Automatically changed title from XXXX to XXXX"

is extraneous and I would actually need to remove it since it might cause confusion with the users who aren't admin types (or just be info that would be ignored). However, I can see that it would be a useful feature for knowledgeable users/administrators to confirm the auto title update took place, so would defer to module maintainers/consensus and can make a patch should this feature be committed.

I would also agree regarding the messages. Shouldn't we have some kind of debug option which would display those messages only if it's turned on?

@charlie1volley and @vdsh. Drupal's logger has a debug message mode, I can move the changed message to there.

The original message that sparked adding the messages 'node_type {{ PLACEHOLDER LABEL }} has been created' comes from the core node module (it is added as part of NodeForm's save function). Therefore not directly under our control.

I've had a look at Drupal's messenger API and it looks like we've got the option to delete messages (though in a bit of a roundabout way). What I can do is if option #2 is selected during the entity insert hook look for the status message with the placeholder, delete it and then add it back after the second save. That way the user only sees a single message with the right title.

Here's another version of this patch. This time when the label is created after the first save the original status message with the placeholder is removed.

A few points to note about this patch:

• To account for multiple automatic labels on a page (eg nodes with paragraphs) each placeholder now includes the entity uuid to make it unique.
• The original node creation status message included a hyperlink to the new node. I've removed this link in the new message as it may contain a out-of-date URL (this happens when url aliasing rules use the entity label)

Also be aware the unit tests still don't work yet.

I tried to test the new patch but the patch fails to apply. The previous patch also is now failing to apply...

@charlie1volley. Please try this one.

Finally got some time to work on this. Here's a version of the patch which should have the kernel tests working. In addition the patch fixes some deprecation warnings in the tests.

Unfortunately I could not get kernel tests which test the AFTER_SAVE option to run. The problem seems to be that when the shutdown callback gets called the test container is no longer available. This only appears to affect the Kernel tests, testing with the full site up and running appears to work.

If I get some more time I'll write a functional test for this.

Sorry, I could not get the patch to apply using composer (on Drupal 8.9.13).

@charlie1volley give this version of the patch a try.

In the patch I tried to remove some of the deprecation notices from the tests but it looks like there's some breaking changes between the test infrastructure of D8 and D9.

Sorry, I could not get this one to apply...

@charlie1volley what versions of PHP and Drupal are you using? Also what errors are you receiving when you try to apply the patch?

Drupal: 8.9.14
PHP: 7.3.27

All Composer gives me is: Could not apply patch! Skipping. The error was: Cannot apply patch

@charlie1volley I've requeued testing for this patch against Drupal 8.9.x development (I can't select 8.9.14, only 8.9.13). My site has moved to Drupal 9 making it difficult to debug this on Drupal 8.

Do you have any other patches applying to the auto_entitylabel module? If so try removing those patches and applying this one on its own and see if the patch will apply.

Whenever I've received that message on my site from composer I've found that the actual patch failure is reported further up the output, and on my site it's usually when applying multiple patches which have shuffled the lines too far away from their original positions that the patch tool can no longer resolve it.

Another thing I can suggest trying is instead of running composer try to manually patch the module using:

patch -p1 < patchfile.patch

from your auto_entitylabel directory (suggest checking out a fresh copy of auto_entitylabel-8.3-x-dev that has had no past patches apply). That's the command composer is running under the hood and that will print the errors to the console.

Hey Charlie1volley,

The patch applies cleanly to the latest dev, but not to the 3.0-beta4

So in your composer, you need to allow for dev packages and set "drupal/auto_entitylabel": "^3.x-dev" (instead of ^3.0 that I guess you have)

Hope it helps!

vdsh: Thanks! Worked like a charm!

Demma10: Thank you for your perseverance and apologies for my novice-ness. I was able to apply patch, tested, and works as designed (now it is Drupal 8.9.15).

No worries @charlie1volley, I'm just really glad the patch worked in the end.

To get a patch included in a module it usually needs to be written against the dev version, that's why I switched to the dev version. It was just unfortunate that for this module the dev version is so different from the released version that the patch couldn't apply to it.

Demma10: I was doing some other things on the site and ran across a couple of things:

1. The PLACEHOLDER message shows up if "after save" is selected. Maybe that is intended, but just wanted to point out. If "before save" is selected, it does not show (as expected).

2. I noticed #1 because one of the other content types on my site that I had not specifically tested with the latest patch (I had just been testing the automatic email generating ones), unexpectedly generated the PLACEHOLDER message. When I looked at the auto label settings, it had "after save" selected when I had expected it to be "before save" based on #32 comment "Existing configuration sets it to after save (preserving the old behaviour). New configurations default this to before save." I have another dozen or so content types and they all had the same thing if the content type was using auto labels (not disabled). I cannot explain why the two content types with auto emails I had been testing apparently defaulted to "before save", but it looks like if the content type had been using auto labels, it defaulted to "after save." If the content type did not use auto labels (disabled), it defaulted to "before save." I created a new content type and it correctly defaulted to "before save."

I hope this makes sense...

patch from #53 applies to 3.x-dev and fixes the issue that was caused by this when we upgraded from beta3 to beta4. Thanks.

I have a content type with an entity_reference to a paragraph
I was using auto_entitylabel
The tokens for the fields within the paragraph referenced are no longer available once this patch is applied

After updating from beta3 to beta4, our project's automated tests where we set moderation_state when creating a node started failing - I tracked it down to this:

      if ($entity->getEntityType()->isRevisionable()) {
        $entity->setNewRevision(FALSE);
      }

It was working previously in beta3, so noting here for anyone else who is using moderation that beta4 could break things. Using 3.x-dev with the #53 patch did allow our tests to pass again, but not keen on all the debug logger messages that are coming through now.

Hi all, i just found that if i enable "preserve already created node titles" and will use atutomaticall generate the label with [node:nid] it wont generate nid as title! However if i reverted back patch#2 and added all as it was before it started to work and now preserve works fine.

Unfortunately I didn't had success applying the patch to the latest version in a D9 latest version.

I got the patch, put into the auto_entitylabel directory and did the "patch --dry-run -p1 < 3076302-50-d8.patch" and I got the messages:

checking file auto_entitylabel.info.yml
checking file auto_entitylabel.install
checking file auto_entitylabel.module
Hunk #1 FAILED at 11.
Hunk #2 succeeded at 169 (offset 6 lines).
Hunk #3 succeeded at 236 (offset 6 lines).
1 out of 3 hunks FAILED
checking file config/schema/auto_entitylabel.schema.yml
checking file src/AutoEntityLabelManager.php
checking file src/AutoEntityLabelManagerInterface.php
checking file src/Form/AutoEntityLabelForm.php
Hunk #2 FAILED at 306.
1 out of 2 hunks FAILED
checking file tests/src/Functional/PrefilledOptionTest.php
checking file tests/src/Functional/PreserveTitlesOptionTest.php
checking file tests/src/FunctionalJavascript/AutoEntityLabelBatchTest.php
checking file tests/src/Kernel/AutoEntityLabelFormTest.php
checking file tests/src/Kernel/AutoEntityLabelManagerTest.php
checking file tests/src/Kernel/AutoEntityLabelPermissionControllerTest.php
checking file tests/src/Kernel/AutoEntityLabelTest.php

I remove the module from my installation with a composer remove drupal/auto_entitylabel and re-add with a composer require 'drupal/auto_entitylabel:3.x-dev@dev', just to be sure that I had the latest version. Trying the patch again, the same problem.

I reach this issue coming from another one in https://www.drupal.org/project/auto_entitylabel/issues/3239799 where we believed this patch could fix that issue but, the problem remains the same there.

There's something that I made wrong?

Thanks

Finally got some time to work on this issue :).

Since it's been so long and the old patch no longer applies to the latest dev I've done a complete rewrite.

This new patch adds the module Hook Post Action as a dependency. Hook Post Action provides hooks for running operations after the entity has finished being saved (when it is safe to save the entity again). It too calls the drupal_register_shutdown_function to run its post-insert hooks, but unlike the other patch it doesn't seem to interfere with the Kernel tests (there were some kernel tests patch 53 couldn't run because it would error out).

If adding the new module is overkill I can just copy the important parts of the code from the hook_post_action module.

The patch fixes being able to save with books and the node id tokens work.

There are a few things to be aware of when installing this patch:

- You may need to install the Hook Post Action module before applying the patch.
- I've updated the tests to remove the deprecation notices, this breaks test compatibility with Drupal 8. The tests will need to run under Drupal 9.
- This patch DOES NOT fix the issue of Set Label running twice. I'm releasing this version because I wanted to test if this new patch fixes the other issues with the patch in comment 53 (paragraph and nested entity labels).

Re-rolled the patch to remove the dependency on the hook_post_action module. Was able to move the relevant parts of hook_post_action into this module and keep the kernel tests working.

Again, this patch DOES NOT fix this specific issue, setLabel() still runs twice. However, this does fix #3196672: WSOD when trying to create a book outline on a new node and I'm hoping it also fixes %AutoEntityLabel% title when "Preserve already created node titles." option is enabled and Node creation fails with "This entity (node: %AutoEntityLabel%) cannot be referenced."

As for fixing the issue of setLabel() running twice, after reading some of the comments in this issue I'm now not sure the before and after save settings is very usable. The run setLabel before the first save option does remove support for tokens that aren't available before the first save of the entity but it isn't always obvious what those tokens are (going from an earlier comment these tokens might also include paragraph tokens and entity reference field tokens). The run setLabel after save option required placeholder text because entities cannot be saved without their label, but this caused issues with the insert messages (and possibly one of the two issues above).

What does everyone think about going back to the original solution for this of comparing the old and new labels and only triggering the second save if there's a difference?

Hi Demma10,

Thanks for the patch. Now, at least, I could apply it. Unfortunately the %AutoEntityLabel% with the preserve already created node titles option enabled remains the same. The only way that I found to "fix" it was adding a check into the auto_entitylabel_entity_presave hook with a "|| $entity->isNew()" before to set the label.

I did had time to test the other changes but I will try to do this weekend.

Again, thanks for your effort.

any progress guys?

Sorry this has taken so long, I ended up not having as much time to work on this as I thought.

I did do some experiments with automatically detecting if any post-save tokens were used and then triggering the second save only when detected, but I didn't have a lot of success (at the time I couldn't get any of the token manager services to detect if there were actually tokens in the string).

Instead I've just ported the before save and after save settings from the previous patch into this new patch. In addition I've reworked the logic to include a fix for %AutoEntityLabel% title when "Preserve already created node titles." option is enabled. The code isn't very optimal, but hopefully it makes sense. Apologies for all the comments, there are a lot of different conditions and branches to account for. Also note that the update hook to set the new_entity_action setting only applies to entities which already have autolabel settings, entities which do not have autolabel settings will not be updated.

Just be warned that I haven't been able to test this yet. There may be another patch once I get a chance to test this.

Forgot to set the issue status.

Apart from the bunch of code standard violations which I'm not going to describe because they are visible on the latest test run, the patch from #71 has an issue .

I tried to apply on beta4 which failed. So I updated to latest dev version, 0db913e commit, and run drush updb , which failed with Class "AutoEntityLabelManager" not found.

Can't comment on the test coverage quality, this needs a deep dive into the issue. Feels like everyone would benefit if the description would be updated to clearly state the problem and what is suggested solution. This would help with the patch review.

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.

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

Last attempt...

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

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

No longer applies to 3.0 version

@SocialNicheGuru did you use diff from MR?

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)

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

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)
Could not apply patch! Skipping. The error was: Cannot apply patch https://git.drupalcode.org/project/auto_entitylabel/-/merge_requests/10....

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.

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

Resolved conflicts.

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: #3367643: auto_entitylabel_entity_insert breaks hook_entity_insert

PHPCS fails currently!

@Grevil this is a quite heavy, but important change, as it also fixes issues like #3452242: Incompatible with tmgmt (auto-translation) and similar cases. Still I see some existing risks like #3367643: auto_entitylabel_entity_insert breaks hook_entity_insert.

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!

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

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

What do you guys think?

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.

Alright, that's it! RTBC @VladimirAus!

Wonderful work everyone!

I updated the issue summary.

Thank you, everyone!
Commited! 🥂

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

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

@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!

Yes, switching to 'Create label before first save' worked for us.

@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).

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.

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!

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

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.

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

Adding related issue that maybe could help #3260103: Automatic entity label does not take into account the field constraints

Was also running into a similar issue as @michèle in #106 as title was calculated based on a saved value (via serial module) so "Before save" is not an option. I had a hook_node_insert implementation that checked isNew(), which no longer passed because of this issue. Luckily I no longer needed to do that extra check.

Should this maybe be closed and another ticket opened? The change of status to "Needs review" implies that the changes are not yet released, but from what I can tell that's not true. I got the impression that 3.4 was a rollback of this, but it doesn't seem to be.