Add a 'machine_name' widget for string field types with a UniqueField constraint

Why do you need a machine name for a content entity ? You already have the an id and uuid ?

Well, most content entities don't have machine name field as most of them use a serial field and a uuid. That's already unique enough to be fair.

The entity_subqueue content entity type from the Entityqueue module also needs a 'machine name' field widget, so I think it's worth giving this a try :)

Looks like we might need this for #2784921: Add Workspaces experimental module

Here's an initial WIP patch which shows the complexity of this widget. And there are two separate issues that need to be fixed as well, as can be seen in the first two hunks from the patch.

Not sending this patch to the testbot because it's useless at the moment.

I finally managed to get this working. It was a lot trickier than I expected but I'm pretty happy with its behavior now and we have complete test coverage as well :)

Looks very awesome!

I can imagine it was tricky, but it's quite an elegant solution.

Small nitpick which can be fixed on commit, if it's an issue at all:

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
@@ -0,0 +1,196 @@
+    parent::__construct($plugin_id, $plugin_definition, $field_definition, $settings, $third_party_settings);
+
+    $this->entityFieldManager = $entity_field_manager;
+    $this->elementInfo = $element_info;

I wouldn't usually expect a blank line here.

Oh, I always leave a blank line in the constructor between the parent call and local variable assignments, I like the code to have enough "breathing room" :)

1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
   @@ -0,0 +1,196 @@
   + *   field_types = {
   + *     "string"
   + *   }
   

   I think this should be no ui. Pretty sure the UX/product team won't want to present 'regex' to users. Given we're going to use it on base fields for workspaces - I think that would be fine. @Sharique - can you confirm that would suit PET module too?

2. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
   @@ -0,0 +1,196 @@
   +class MachineNameWidget extends WidgetBase implements ContainerFactoryPluginInterface {
   

   Does this need to provide a config dependency on the field it references?

3. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
   @@ -0,0 +1,196 @@
   +        'exists' => $this->getSetting('exists'),
   

   Is there a test for this?

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
@@ -0,0 +1,196 @@
+ *     "string"

I really think this should be its own Field Type that has a constraint to validate unique. We're validating unique in the form layer, but not for things like REST.

Thanks @larowlan for a great review! I've started to work on your points and I'll post a detailed reply tomorrow.

Ok, here it is :)

Re #15:

1. Totally agreed there, it's exactly what I did initially only to find out that the "no ui" concept does not exist for widgets and formatters, only for field types. So we can do the next best thing and use isApplicable() to make this widget available only to entity ID fields. This is actually the 99% use case and it satisfies all the modules that need this widget so far: Workspaces, Entityqueue and the PET module from the IS.

2. It doesn't need to declare a config dependency because the source field has to be configured in the form display if it wants to be available as a source and the entity form display config entity already depends on all its components (aka. configured fields).

3. There is now :)

4. Sadly, this can not really be its own field type because, for example, @alexpott asked in #2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002) to allow setting an entity ID explicitly only for string ID fields. And we also have an existing issue to deal with this in REST: #2870649: REST should respond with a 409 for a POST request to an existing entity

Since we're limiting this widget to be available only to ID fields, we're effectively blocked on #2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002) for now because the tests will fail without that patch.

@timmilwood pointed me to this issue while talking about #2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002). Adding issues linked from #17 to related issues to improve discoverability.

#2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002) is in, so we can unpostpone this one. It would allow us to clean up some form code in the workspaces module :)

The patch from #17 looks good.

I think the work around in #17 to not having no ui available looks good. Given it does not does do exactly what @larowlan asked for in #15 I'm going to ping @larowlan but I'm +1 on commit.

1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
   @@ -193,4 +190,35 @@ public static function processMachineNameSource($element, FormStateInterface $fo
   +      ->condition(reset($element['#array_parents']), $value, '=')
   

   I don't think this will work with IEF - so we need something less brittle here.

2. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
   @@ -193,4 +190,35 @@ public static function processMachineNameSource($element, FormStateInterface $fo
   +    // This widget is available only to entity identifier fields.
   

   This is a reasonable compromise

Re #25.1: That's right and also easy to fix: we can just put all the info we need in the widget's form element.

Tested with IEF and everything works perfectly :)

I really think this should be its own Field Type that has a constraint to validate unique. We're validating unique in the form layer, but not for things like REST.

I think this comment still stands?

@larowlan, I answered that point in #17.4.

Testbot was acting up.

Actually, now that the workspace module has been committed, we have the first use case for this widget in core!

The interdiff shows what kind of ugly code can be removed :)

A couple more tests needed to be updated.

#31 was a simple change to remove the current custom code from from Workspaces to use the new widget, which also brings an indirect level of test coverage on top of the existing dedicated one, so I think it's ok to move this back to RTBC.

Random fail.

Rerolled.

As this is passing on 8.7 and it likely won't make it into 8.6, I think it should be RTBC as per #34

#15 and #27 are still open: larowlan asked for an additional unique FieldType.

The current patch adds a FieldWidget (in itself already an improvement).

Could we RTBC and then create a follow-up for the unique FieldType?

@ndf, I replied to #15 and #27, see #17.4. And we're already RTBC :)

@amateescu Sorry I missed 17.4
Then RBTC from me too.

Still think this patch looks good. Realised though that the issue summary is out-of-date and there is no change record. Needs work for those things.

Updated issue summary.

Thanks, @Pancho! Here's a CR too: https://www.drupal.org/node/3028311

Further refined issue summary.

edit: Oops, now we have two change records. We might want to combine the best of both.

@Pancho, oops indeed :) I added the usage example from your CR to the one linked in #47.

added another followup task

Think this should actually be postponed to that other issue which is creating a whole new separate ‘machine_type’ field type.

@Pancho, as far as I see, that issue introduces a 'machine_name' config data type, while this one is about providing a field widget for content entity types, so they're not related at all regarding their functionality.

@amateescu You're completely right, sorry. Remains RTBC.

+  public static function isApplicable(FieldDefinitionInterface $field_definition) {
+    // This widget is available only to entity identifier fields.
+    $entity_type = \Drupal::entityTypeManager()->getDefinition($field_definition->getTargetEntityTypeId());
+
+    return $field_definition->getName() === $entity_type->getKey('id');
+  }

Do we need to enforce this? What if I want a 'machine_name' widget on some other kind of field? I might want all the same behavior of the machine name widget for giving machine names to some other kind of content on a site. E.g. I need my projects to have a machine name, even though they're nodes that officially have integer IDs. Why can't I define my own string field for this and reuse the 'machine name' widget?

@dww, that was my attempt to resolve @larowlan's request from #15 (last paragraph), but now that I think about it again, maybe it's better to enforce that the field definition has a UniqueField constraint rather than being the entity identifier.

A couple of self-review fixes :)

Updated change record. I only added a sentence in the end, because the widget will still primarily be there for string-type identifiers.

This one now depends on #3027745: UniqueFieldConstraint doesn't work for entities with string IDs being committed first to ensure the machine_name really is unique, am I right? In that case, we should actually see a failing test here.

Better title for the new approach in #55.

This one now depends on #3027745: UniqueFieldConstraint doesn't work for entities with string IDs being committed first to ensure the machine_name really is unique, am I right? In that case, we should actually see a failing test here.

Not necessarily :) There are two use cases for this widget:

1) for the identifier field of an entity type with string IDs
2) for a string field with a UniqueField constraint, on an entity type with either integer or string IDs

The first case is what we're doing for the workspace entity type, and it works just fine. The second case is the "problematic" one, but only in the case where the entity type uses a string ID.

So I'm not sure whether there's a hard dependency, that issue is a bug that needs to be fixed regardless of this new widget.

That issue was just committed, does anyone want to RTBC this one? :)

This looks good to me. Just some tiny nits below and then I think it's RTBC.

1. +++ b/core/modules/workspaces/tests/src/Functional/WorkspaceTest.php
   @@ -52,23 +52,24 @@ public function setUp() {
   -    $page = $this->getSession()->getPage();
   
   +++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/Field/Widget/MachineNameWidgetTest.php
   @@ -0,0 +1,228 @@
   +   * Node is required because the machine name callback checks for the
   +   * "access content" permission.
   

   I'd hoped that machine names wouldn't always require node, so in digging I found that actually the `access content` permission is defined by the system module, so either just that module can be installed, or this comment can be updated.

2. +++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/Field/Widget/MachineNameWidgetTest.php
   @@ -0,0 +1,228 @@
   +    entity_get_form_display('entity_test_string_id', 'entity_test_string_id', 'default')
   ...
   +    entity_get_form_display('entity_test_string_id', 'entity_test_string_id', 'default')
   ...
   +    entity_get_form_display('entity_test_string_id', 'entity_test_string_id', 'default')
   ...
   +    entity_get_form_display('entity_test_string_id', 'entity_test_string_id', 'default')
   

   Should new usages of deprecated functions be added?

@jhedstrom, thanks for the review!

Re #63:

1. Very good point, we can clean that up and just assign the access content permission to the test user because the system module is installed by default.

2. Those functions are not really deprecated because they don't have any replacements. That problem will be fixed in #2367933: Move entity_get_(form_)display() to the entity display repository :)

This looks good to me.

Those functions are not really deprecated because they don't have any replacements.

Good to know! I'd always thought the recommended replacement in the deprecation notice was incomplete compared to the old functionality :)

Added one more example. These are all borderline-configuration ContentEntityTypes.

Fixed usages of entity_get_form_display() which is properly deprecated now.

1. +++ b/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php
   @@ -170,8 +170,15 @@ public function buildForm(FieldableEntityInterface $entity, array &$form, FormSt
   +    // Sort the components by their weight in order to allow a form element from
   +    // a widget to depend on the processed elements of another form element from
   +    // another widget. For example, the 'machine_name' widget needs the
   +    // processed '#id' property of its source field.
   +    $components = $this->getComponents();
   +    uasort($components, ['Drupal\Component\Utility\SortArray', 'sortByWeightElement']);
   +
        // Let each widget generate the form elements.
   -    foreach ($this->getComponents() as $name => $options) {
   +    foreach ($components as $name => $options) {
   

   Note to other reviewers, this happens in the machine name element plugin (not in the widget)

2. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
   @@ -0,0 +1,209 @@
   +    foreach (array_intersect_key($available_fields, $displayed_fields) as $field_name => $field) {
   +      // The source field can only be another string field.
   +      if ($field->getType() === 'string' && $field->getName() !== $this->fieldDefinition->getName()) {
   +        $options[$field_name] = $field->getLabel();
   +      }
   +    }
   

   We could also limit these by weight in the display here right?

   I.e we're already filtering out items that aren't enabled, but could we also inspect their weight relative to the weight of this item?

3. +++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php
   @@ -28,7 +28,7 @@ public function validate($items, Constraint $constraint) {
   +    if (isset($entity_id) && !$entity->isNew()) {
   

   if the entity is new, then why would it have a value in the database that would impact the query?

   i.e if the entity is new, adding a <> against its ID won't return any results right? Because its not saved yet.

   I'm not convinced we need this change - can you elaborate on why it was needed?

+++ b/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php
@@ -170,8 +170,15 @@ public function buildForm(FieldableEntityInterface $entity, array &$form, FormSt
+    uasort($components, ['Drupal\Component\Utility\SortArray', 'sortByWeightElement']);

Only nit SortArray:class

Looks good overall.

Just one small thing:

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
@@ -0,0 +1,209 @@
+    // This widget is only available to fields that have a 'UniqueField'
+    // constraint.

This is a detail that could do to be also mentioned in the class docs, for DX. Imagine a developer trying to figure out why this widget doesn't show in the UI.

Thanks everyone for the reviews :)

Re #69:

2. That's a good idea! Done and updated the test coverage to take this new behavior into account.

3. That change fixes a bug introduced in #3027745: UniqueFieldConstraint doesn't work for entities with string IDs, which didn't take into account that the UniqueField constraint could be set on the ID field itself. In that case, when adding a new entity with a string ID, the entity ID is set by the form (this doesn't happen for entities with integer IDs), so $entity->id() will never be NULL, which essentially makes the query ignore other pre-existing entities which have the same ID as the newly created one.

I've updated the comment and the code to make it more clear when we need to add that condition to the query.

#70: heh, fixed.

#72: good point, fixed as well.

Setting back to RTBC because all the reviews were addressed.

This is looking good. In prep for this getting into core, I've rolled a 2.x branch of the machine_name_widget module that contains most of this code.
Some of the core features like checking uniqueness isn't there, but the basic ability to add machine names is.

https://www.drupal.org/project/machine_name_widget/releases/8.x-2.0-alpha1

The 9.1.x tests failed: https://www.drupal.org/pift-ci-job/1672843

I requeued it to be sure, but this issue may need a different patch for 9.x. Thanks!

Fixed that :)

1. +++ b/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php
   @@ -170,8 +171,15 @@ public function buildForm(FieldableEntityInterface $entity, array &$form, FormSt
   +    // Sort the components by their weight in order to allow a form element from
   +    // a widget to depend on the processed elements of another form element from
   +    // another widget. For example, the 'machine_name' widget needs the
   +    // processed '#id' property of its source field.
   +    $components = $this->getComponents();
   +    uasort($components, [SortArray::class, 'sortByWeightElement']);
   +
        // Let each widget generate the form elements.
   -    foreach ($this->getComponents() as $name => $options) {
   +    foreach ($components as $name => $options) {
   

   Does this mean that you can break the form by re-ordering the fields in the UI? It looks like it. We prevent you from choosing an impossible source field but can you re-order the fields after doing this.

2. +++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php
   @@ -25,11 +25,10 @@ public function validate($items, Constraint $constraint) {
   -    $entity_id = $entity->id();
   -    // Using isset() instead of !empty() as 0 and '0' are valid ID values for
   -    // entity types using string IDs.
   -    if (isset($entity_id)) {
   -      $query->condition($id_key, $entity_id, '<>');
   +    // If the entity already exists in the storage, ensure that we don't compare
   +    // the field value with the pre-existing one.
   +    if (!$entity->isNew()) {
   +      $query->condition($id_key, $entity->id(), '<>');
        }
   

   Should these changes have test coverage in \Drupal\KernelTests\Core\Validation\UniqueFieldConstraintTest?

3. +++ b/core/modules/workspaces/tests/src/Functional/WorkspaceTestUtilities.php
   @@ -57,13 +57,11 @@ protected function getOneEntityByLabel($type, $label) {
   -    $this->getSession()->getPage()->hasContent("$label ($id)");
   

   Should we at least assert a 200?

Can anybody explain why we use closure function in MachineNameWidget class?

'exists' => function () {
  return FALSE;
}

The Drupal\Component\Serialization\PhpSerialize can't serialize the MachineNameWidget class, because we have closure function.

I have the same issue as vbory said in #81. The issue occurs while I was trying to create a new entity_reference item via inline_entity_form (complex widget).

I copied patch from #79 but removed this Closure. I hope it's gonna work too.

Small fix for #82

Apologies for last two, not working patches. I spend some time, check where this Closure is used and find some solution.

Like @bvory noticed a Closure can't be serialized. But as it's described in docs https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21... the "exists" parameter should be a callable.

For this issue, I have a workaround. We can create method witch always return false and set it as a callback. This solution is working for me, but I'm not sure is it a good solution. Maybe we just should check "MachineName::validateMachineName" does exist callback function is set and leave it empty in MachineNameWidget.

I believe the issues @alexpott mentioned in #80 are still not resolved. So this still needs work?

Hello, Any update?

Working properly.

reroll
added option "Standalone" to the widget settings. see api.drupal.org

reroll

Patch from #92 wasn't working for us against 9.3.0.
So rerolled patch attached.

hi, I tried, the rerolled patch #94 is not working for me.

Patch #73 for D8.9.x is partially working. Throwing issue "Serialization of Closure is not allowed in Serialize()" when trying to modify a custom content block having paragraph type entity reference field. Creating new patch here.
Note: This is for D8.9.x only.

Updating patch. Refer to above comment ^^.

Rerolling the 94 patch for 9.3.0 / 9.3.x.

Hm, rerolling the #92 patch for 9.3.0 / 9.3.x!

Fixing this:

FILE: /Users/chart/Projects/drupal/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 33 | ERROR | [x] Expected "\Drupal\Core\Entity\EntityFieldManagerInterface" but found "\Drupal\Core\Entity\EntityFieldManagerInterface;" for @var tag in member variable comment
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Adding patch for specific 9.2.x Drupal core version following #101

Ignore the above ^^ patch in #102 as I mistakenly did not add the untracked files.
Adding new patch for D9.2.x here.

FILE: /var/www/html/core/modules/workspaces/src/Form/WorkspaceForm.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | WARNING | [x] Unused use statement

PHPCS is not happy

The field widget should have "Textfield size" option.

Addressed #105 in patch #101, please review.

Thanks!

I think we need a usability review on this ticket before we tag it with "usability review" let's add all the screenshots for the widget and its setting to the IS.

I'm working on this.

I'm guessing this won't make it to 9.x, so a D10 patch would be necessary (#106 seems to be applying well enough on 10.1)

In The D10 version

+++ b/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php
@@ -173,8 +174,15 @@ public function buildForm(FieldableEntityInterface $entity, array &$form, FormSt
+    uasort($components, [SortArray::class, 'sortByWeightElement']);

I think according to #3259716: Replace usages of static::class . '::methodName' to first-class callable syntax static::method(...) this should be: uasort($components, SortArray::sortByWeightElement(...));

setting to NW for the screenshots at least

Creating patch for 10.1.x and address #111

attached reroll file

Patch #112 (a re-roll of #106) omitted the new files that were being added in #106. (And the interdiff for it, posted in #113, therefore showed those files removed.)

Here's a replacement patch that includes the new files. This is a re-roll of #106, including the suggestion from #111 to use the new callable syntax. I include an interdiff of this vs #106. Drupal 9.5 sites may not support the callable syntax new to PHP 8.1, so I include a patch for D9.5.x too (without interdiff - I've just retained the previous uasort($components, [SortArray::class, 'sortByWeightElement']); line).

I've also included some screenshots of the new widget settings (collapsed+expanded), and the widget itself in action (before+after clicking the 'edit' link). I trust those might be enough screenshots? Since this is new functionality, I presume we don't need screenshots of how it looked before the change? More can be made if needed of course!

Fixed the test failure which was due to case sensitivity. (To my surprise!)

I also spotted that the widget's isApplicable() relies on the options having been set when adding the constraint. I don't see anywhere documented that requires that; so I've replaced an isset() for an array_key_exists() so that calls like ->addConstraint('UniqueField') will work rather than requiring an empty second parameter ->addConstraint('UniqueField', []). (There are instances in core of other constraints being added without setting the options parameter, such as in \Drupal\Core\Entity\EntityType::__construct().)

Previous test failure was for a hopefully unrelated Ckeditor5 test. Tests all passed on a retest!

According to the issue summary #2905594: Missing entity validation constraint: don't allow new entities when there is an existing one with the same ID is still remaining. So my question is should this ticket be postponed on that one? Or is #2905594: Missing entity validation constraint: don't allow new entities when there is an existing one with the same ID not needed for this to move forward?

That issue talks about using a "duplicate ID" - if that's talking about a field with a unique value constraint, it could actually be solved by the work that has now been done here. All that would be needed (on top of this work) is for the constraint to be added to that field, I believe.

Or it's talking about using the entity ID field itself, rather than a field using a machine name constraint/field/widget. Which would be related, but I don't think blocks this issue at all. It might even still be able to make use of it in the same way - by simply adding the constraint to the appropriate entity ID field.

So I could be wrong, but I'm updating the IS based on that belief that it's a related issue, not a blocker to this one. Correct me if I am indeed wrong!

Thanks for updating that! This one may need more eyes but noticed the change record was written for D8 and this probably will only make it in 10.2. So if that could be updated.

Ok; updated the change record to now target 10.2.0.

10.2.x-dev isn’t an option for the issue metadata here though. I guess we just wait to get it into that or main or whatever becomes available?

So tested by enabling entity_test module and going to entity_test_string_id/structure/entity_test_string_id/form-display
Verifying the ID field can switch between textfield and machine_name.

Verified that regular Text(only) field doesn't get machine_widget.

Not sure how other to test this one. But lets get into committer eyes. May be easier to get into 10.2 early

Thanks to everyone here for moving this 7 year old issue along!

I triaged this and updated the remaining tasks accordingly. Here are the details.

The issue summary states that there are no user interface changes but this is adding a new widget. And in #107, screenshots were asked for. I eventually found them in #144. @james.williams, thanks for making them. It really helps reviewers/committers if the screenshots are in the IS or a link to the comment so they can be found. Also, with a note saying which patch/MR version they were made with. (I just added it)

There were many questions raised in the reviews, that is good to see and read. However, #80 has not been answered. I have added that to the remaining tasks.

#124 says "Not sure how other to test this one. But lets get into committer eyes." Instead, before setting to RTBC ask in #contribute for first. An issue that is Reviewed & tested by the community ("RTBC") should not have any questions unanswered, it should be ready for commit.

I read the CR and made changes to use plain English. There should be no changes to the content but someone should check that. I am adding a review of the CR to the remaining tasks. The CR mentions the benefits to site builders but site builder' is not selected in the 'Impacts'. Should it be? The CR should also include a screenshot. These two look the most informative,

https://www.drupal.org/files/issues/2023-05-16/widget-settings-collapsed...
https://www.drupal.org/files/issues/2023-05-16/widget-settings-expanded.png

The latest patch is failing tests and is on 10.1x. This needs to be on 11.x with passing tests. Setting to NW.

I am updating credit, always a challenge on issues with > 100 comments.

A reminder to all that when making a patch, an interdiff or diff is needed. There are instructions for creating an interdiff. If you think a diff is not needed, add a comment stating why.

Hello.

You can try using this module https://www.drupal.org/project/machine_name_widget

The patch in #2685749-116: Add a 'machine_name' widget for string field types with a UniqueField constraint for Drupal 9.5 is broken.
It adds two unnecessary files
core/b/core/tests/Drupal/FunctionalJavascriptTests/Core/Field/Widget/MachineNameWidgetTest.php
core/b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/MachineNameWidget.php

They do not differ from the ones at the correct path, so I just removed them.

It should look like the one attached.

Re-roll of the patch from 116 for 11.x. Fixing the testPublishWorkspace as well.

Fix failure of custom commands

Thanks @a.dmitriiev! :) Still 1 failing test left.

Could you perhaps use a MR to make reviews easier?

Here is the patch the same as MR. I figured out the failing test: after changing machine name to client side only, the trailing replace char is also removed. I hope now the tests will pass.

Still the same test fails. However, the expected string is exactly the same as widget produces in admin UI when checking the same thing manually.

Looks like the machine name is wrong. When I add a new field anywhere in Drupal, naming it Test value !0-9@, the machine name gets auto-created as field_test_value_0_9_ with an underscore (_) at the end.

Which would be strange, though, as the machine name test says also says Test value !0-9@ translates to test_value_0_9: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/Drupal/... 🧐

Okay, I reverted the commit. That wasn't it.

Okay, I found a way to output the machine name value from the field and it's a completely different value than the one that's expected: jvfavt8z == test_value_0_9 wich of course is false. Same can be seen in the test artefacts.

It's also the same what I experienced after I put $settings['extension_discovery_scan_tests'] = TRUE; into my settings.php and enabled the entity_test module. I opened URL /entity_test_string_id/add and it always contained some random name in the name field and the machine name field was always pre-filled with this machine name. When I changed the name, the machine name didn't change accordingly. It always stayed the same.

(It also looks really different from all other machine name elements I know in Drupal.)

By the name of Dries I couldn't find where this arbitrary name value was coming from. On every page load it was a different one. I also don't understand why all other asserts succeed and only this one doesn't.

Does anybody here have an idea why when accessing /entity_test_string_id/add the name is always pre-filled with a random value? Where does it come from?