- last update
over 1 year ago 29,381 pass, 1 fail - ๐ฌ๐งUnited Kingdom james.williams
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!
- last update
over 1 year ago 30,327 pass, 1 fail - last update
over 1 year ago 30,360 pass, 1 fail The last submitted patch, 114: drupal-10-1x-machine-name-widget-2685749-114.patch, failed testing. View results โ
- last update
over 1 year ago 29,382 pass, 2 fail - ๐ฌ๐งUnited Kingdom james.williams
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 anisset()
for anarray_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()
.) The last submitted patch, 116: drupal-10-1x-machine-name-widget-2685749-116.patch, failed testing. View results โ
- last update
over 1 year ago 29,389 pass - last update
over 1 year ago 30,368 pass - ๐ฌ๐งUnited Kingdom james.williams
Previous test failure was for a hopefully unrelated Ckeditor5 test. Tests all passed on a retest!
- Status changed to Needs work
over 1 year ago 5:39pm 5 June 2023 - ๐บ๐ธUnited States smustgrave
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?
- Status changed to Needs review
over 1 year ago 12:06pm 6 June 2023 - ๐ฌ๐งUnited Kingdom james.williams
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!
- Status changed to Needs work
over 1 year ago 6:03pm 15 June 2023 - ๐บ๐ธUnited States smustgrave
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.
- Status changed to Needs review
over 1 year ago 8:10pm 15 June 2023 - ๐ฌ๐งUnited Kingdom james.williams
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?
- Status changed to RTBC
over 1 year ago 10:12pm 15 June 2023 - ๐บ๐ธUnited States smustgrave
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
- last update
over 1 year ago 29,430 pass - last update
over 1 year ago 29,431 pass - last update
over 1 year ago 29,431 pass - last update
over 1 year ago 29,437 pass 37:34 33:54 Running- last update
over 1 year ago 29,437 pass - last update
over 1 year ago 29,442 pass - last update
over 1 year ago 29,445 pass - last update
over 1 year ago 29,444 pass - last update
over 1 year ago 29,444 pass - last update
over 1 year ago 29,440 pass - last update
over 1 year ago 29,440 pass - last update
over 1 year ago 29,442 pass - last update
over 1 year ago 29,445 pass - last update
over 1 year ago 29,447 pass - last update
over 1 year ago 29,447 pass - last update
over 1 year ago 29,447 pass - last update
over 1 year ago 29,447 pass - last update
over 1 year ago 29,447 pass - last update
over 1 year ago 29,452 pass - last update
over 1 year ago 29,454 pass - last update
over 1 year ago 29,455 pass - last update
over 1 year ago 29,447 pass, 2 fail - last update
over 1 year ago 29,449 pass, 2 fail - last update
over 1 year ago 29,449 pass, 2 fail - last update
over 1 year ago 29,450 pass, 2 fail - last update
over 1 year ago 29,451 pass, 2 fail - Status changed to Needs work
over 1 year ago 11:53pm 8 August 2023 - ๐ณ๐ฟNew Zealand quietone
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.
- ๐บ๐ฆUkraine ankondrat4 Lutsk
Hello.
You can try using this module https://www.drupal.org/project/machine_name_widget โ
- ๐ฆ๐นAustria tgoeg
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.
- last update
about 1 year ago 30,331 pass, 3 fail - last update
about 1 year ago Patch Failed to Apply - Status changed to Needs review
about 1 year ago 3:55am 15 December 2023 - ๐ฉ๐ชGermany a.dmitriiev
Re-roll of the patch from 116 for 11.x. Fixing the testPublishWorkspace as well.
- last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago 30,764 pass, 1 fail - Status changed to Needs work
about 1 year ago 5:53am 15 December 2023 - ๐ฉ๐ชGermany Anybody Porta Westfalica
Thanks @a.dmitriiev! :) Still 1 failing test left.
Could you perhaps use a MR to make reviews easier?
- Status changed to Needs review
about 1 year ago 6:28am 15 December 2023 - ๐ฉ๐ชGermany a.dmitriiev
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.
- Merge request !5812Issue #2685749 by amateescu, james.williams, charginghawk, Marcin Maruszewski,... โ (Open) created by a.dmitriiev
- Status changed to Needs work
about 1 year ago 6:44am 15 December 2023 - ๐ฉ๐ชGermany a.dmitriiev
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.
- last update
11 months ago Build Successful - leymannx Berlin
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 asfield_test_value_0_9_
. - last update
11 months ago 25,809 pass, 1,822 fail - leymannx Berlin
Which would be strange, though, as the machine name test says also says
Test value !0-9@
translates totest_value_0_9
: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/Drupal/... ๐ง - last update
11 months ago CI aborted - leymannx Berlin
leymannx โ changed the visibility of the branch 11.x to hidden.
- leymannx Berlin
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 isfalse
. 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 theentity_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?
- ๐ฉ๐ชGermany a.dmitriiev
Re-rolled patch for Drupal 10.3, still tests are not fixed.