🇮🇳India @kunal.sachdev

I need help with the test failures -

1. \Drupal\Tests\field_ui\Functional\ManageFieldsTest::testAddField() - In this test we are testing that field descriptions appear, both 1 line and multiple lines. But the failure is that the single line description is not showing on the screen. And when I change the code
   description: new TranslatableMarkup("This one-line field description is important for testing"), to

   description: [
     new TranslatableMarkup("This one-line field description is important for testing"),
     ],

   in the plugin core/modules/field/tests/modules/field_test/src/Plugin/Field/FieldType/TestItemWithSingleDescription.php then it works.

2. All the failing FunctionJavascript tests - All the tests are failing with the error 'SyntaxError: Identifier 'DrupalDialogEvent' has already been declared'. I tried reverting all the changes done in core/misc/dialog/dialog.js in https://git.drupalcode.org/project/drupal/-/commit/9b2d61c6e159ffdb62d5b... then it works fine without errors.

kunal.sachdev → made their first commit to this issue’s fork.

kunal.sachdev → created an issue.

alexpott → credited kunal.sachdev → .

The current test failure is in tests/src/Nightwatch/Tests/keyboardTest.js in which there is a part where we are testing that when we are on the categories filter and we click tab then it shifts to next filter drop down i.e security coverage. This test is failing now because in this MR we are rendering the filters as it is which is defined from the backend and not hardcoding them. The previous hardcoding was done in a way such that the filters were displayed in the order :- categories -> security coverage -> maintenance status -> development status but now we accidentally in the backend have the filters defined in the order :- categories ->maintenance status -> security coverage -> development status. For now to fix the failure we can fix the order of the defined filters in backend but I think the major problem is that somewhere in the code we have hard coded the tab index for each filter which needs to be fixed (maybe in a new issue).

kunal.sachdev → made their first commit to this issue’s fork.

Now that ✨ Certain filters should only be shown if they are actually defined Needs work is fixed.

Yes, this issue is pre-existing. Create a separate issue for this 🐛 The functionality of showing results behaves abnormally on tab switch sometimes Active

kunal.sachdev → created an issue.

kunal.sachdev → made their first commit to this issue’s fork.

kunal.sachdev → made their first commit to this issue’s fork.

Added test coverage for the changes to ExtensionExistsConstraintValidator. Also we have update path and its test so removing the related issue tags.

I also tried to use AltLeastOneOf constraint in 📌 Add validation constraints to core.menu.schema.yml Needs work and I have mentioned the problems I faced in https://www.drupal.org/project/drupal/issues/3441434#comment-15686742 📌 Add validation constraints to core.menu.schema.yml Needs work .

I tried to use AtLeastOneOf constraint here but I faced two problems here :-

1. AtLeastOf constraint requires an array of constraint objects and we were getting constraint arrays here. So what I did was implemented a plugin class for AtLeastOneOf, and converted the constraint arrays into constraint objects.
2. RecursiveContextualValidator prevents us from using custom groups. But we want to use AtLeastOneOf constraint there and I think it's validator requires every constraint to have a group. So for that I created this follow-up 📌 Remove the restriction from RecursiveContextualValidator that prevents using custom groups. Needs review .

kunal.sachdev → created an issue.

Change the status of CR to be published.

Looks good.

Looks good. Just have one minor doubt.

Change record created [#3458722].

kunal.sachdev → made their first commit to this issue’s fork.

• @phenaproxima thinks that ExtensionExists constraint doesn't make a ton of sense for core.extension see ( 3441503-#18 📌 Add validation constraints to system.theme Needs work ) for more details.
• Also discussed this with @phenaproxima in slack and concluded that if we want to use ExtensionExists constraint for core.extension then ExtensionExistsConstraintValidator should be completely refactored and use the ModuleExtensionList and ThemeExtensionList, which can report on existing-but-not-yet-installed things.

@phenaproxima I think this comment ( #18 📌 Add validation constraints to system.theme Needs work ) was meant for 📌 Add validation constraints to core.extension Needs review .

The test is failing because in RecipeRunner::installTheme() we do

\Drupal::service('theme_installer')->install([$theme]);

and in ThemeInstaller we have

$extension_config
        ->set("theme.$key", 0)
        ->save(TRUE);

which also tries to validate the config and it breaks for ExtensionExistsContraint with error message 'Theme stark is not installed', which is true.

kunal.sachdev → made their first commit to this issue’s fork.

Most tests are failing because in TestBase setup() we do

$this->container->get('theme_installer')->install(['stark']);

and in ThemeInstaller we have

$extension_config
        ->set("theme.$key", 0)
        ->save(TRUE);

which also tries to validate the config and it breaks for ExtensionExistsContraint with error message 'Theme stark is not installed', which is true. I think we could add a check in ThemeInstaller before saving the key in the config, something like this-

if (!isset($installed_themes[$key])) {
        $extension_config
        ->set("theme.$key", 0)
        ->save(TRUE);
 }

, but I am not really sure about this.

znerol → credited kunal.sachdev → .

Looks good. 👍🏻

kunal.sachdev → made their first commit to this issue’s fork.

Looks good, just some minor docs and test changes required.

Same validation is being done in 📌 Add validation constraints to core.menu.schema.yml Needs work .

kunal.sachdev → created an issue.

For #9 📌 Add validation constraints to system.mail Needs work we already have a constraint

        constraints:
          PluginExists:
            manager: plugin.manager.mail
            interface: 'Drupal\Core\Mail\MailInterface'

to check if the plugin name is valid.

quietone → credited kunal.sachdev → .

kunal.sachdev → created an issue.

The tests are passing now.

kunal.sachdev → created an issue.

There is one remaining unanswered thread in the MR https://git.drupalcode.org/project/drupal/-/merge_requests/4780#note_280791.

kunal.sachdev → made their first commit to this issue’s fork.

and about this in

but if I look at the merge request, that file is not updated at all:

#116 ✨ Add UUID to sections Needs work in the file LayoutBuilderEntityViewDisplayStorage is not updated but the method LayoutBuilderEntityViewDisplayStorage::mapFromStorageRecords() uses a method \Drupal\layout_builder\Section::fromArray(). which is updated here in the MR.

It's LayoutBuilderEntityViewDisplayStorage and not LayoutBuilderEntityViewDisplay, corrected the IS.

Updated CR and IS.

Created CR [#3428313]

@Wim Leers I was referring to #3208766-83 ✨ Add UUID to sections Needs work in #109 ✨ Add UUID to sections Needs work .

Yes, I was waiting for someone to answer the first question in https://www.drupal.org/project/drupal/issues/3208766#comment-15354646 ✨ Add UUID to sections Needs work before removing the post-update hook. But now I think I'll remove the post-update hook for now and we can add it back if we need it. I am adding the question to the remaining tasks in IS.
Also, updated the IS in general.

Updated the IS as before screenshots added in #68 📌 Improve menu parent link selection Needs work .

The changes looks good.

Yes, if the answer to the first question in https://www.drupal.org/project/drupal/issues/3208766#comment-15354646 ✨ Add UUID to sections Needs work is that it's not a concern then we will not need any post-update/update hooks. Hence, marking it again to "Needs review".

kunal.sachdev → made their first commit to this issue’s fork.

The performance test is failing because this is introducing an additional cache query in core/modules/user/user.module::user_form_process_password_unmask(). I think we can increase the query counts by one to resolve the test failure.

#20 🐛 Display the page title, even if "0" in olivero Active : Instead of creating a follow-up to do testing in a loop I changed the test in this issue only.

Tests are passing and all feedback is addressed, but there are 26 unresolved threads and most of them can be resolved.

#223 🐛 Display status message on configuration forms when there are overridden values Fixed : This is happening because there are two forms ClearCacheForm and PerformanceForm on Performace Page and it shows the status messages of the both the forms on the top of the respective forms. I discussed this with @lauriii and we think it's not a problem, so we can keep it it as it is for now.

Going to work on resolving the test failures.

Worked on addressing feedback but there are 26 unresolved threads and most of them can be resolved. Also there is an unrelated CSpell failure in MR.

Addressed all the feedback. Also there is an unrelated CSpell failure in MR fatal: bad object c574a6e4f0867d27a49a244ee9648d7adfaf9f9b.

#218 🐛 Display status message on configuration forms when there are overridden values Fixed : No, this not what is expected. It is working like this because there are two forms ClearCacheForm and PerformanceForm and it shows the status message on the top of the PerformanceForm below the ClearCacheForm.