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 :-
AtLeastOf
constraint requires an array of constraint objects and we were getting constraint arrays here. So what I did was implemented a plugin class forAtLeastOneOf
, and converted the constraint arrays into constraint objects.RecursiveContextualValidator
prevents us from using custom groups. But we want to useAtLeastOneOf
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 forcore.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 forcore.extension
thenExtensionExistsConstraintValidator
should be completely refactored and use theModuleExtensionList
andThemeExtensionList
, 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.