- Issue created by @wim leers
- Issue was unassigned.
- ๐ฌ๐งUnited Kingdom alexpott ๐ช๐บ๐
Wim Leers โ credited alexpott โ .
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Closed ๐ Improve core.extension:profile validation Postponed as a duplicate. Crediting @alexpott.
- Status changed to Active
10 months ago 1:03pm 21 March 2024 - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
- Status changed to Needs review
10 months ago 2:50pm 21 March 2024 - Status changed to Needs work
10 months ago 3:27pm 21 March 2024 The Needs Review Queue Bot โ tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide โ to find step-by-step guides for working with issues.
- ๐ฎ๐ณIndia kunal.sachdev
kunal.sachdev โ made their first commit to this issueโs fork.
- ๐ฎ๐ณIndia kunal.sachdev
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 inThemeInstaller
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.
- ๐ฎ๐ณIndia kunal.sachdev
- @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 thinks that
- ๐ฎ๐ณIndia yash.rode pune
yash.rode โ made their first commit to this issueโs fork.
- Status changed to Needs review
6 months ago 7:56am 11 July 2024 - ๐ง๐ชBelgium borisson_ Mechelen, ๐ง๐ช
I feel that ExtensionExists should be whether the extension exists and we should have a separate constraints - the other constraint should be called ExtensionInstalled. However this is unfortunate because we already have ExtensionExists and it's current meaning is that it is installed.
Not sure what to do.
This is a difficult question. In my opinion ExtensionExists/ExtensionInstalled makes sense, but renaming this and after that reusing the name for the other meaning is difficult. Can we use ExtensionPresent to indicate that should be present on the file system?
- Status changed to Needs work
4 months ago 10:49pm 13 September 2024 - ๐บ๐ธUnited States smustgrave
Believe this needs work for the feedback mentioned in #16? Also is MR 7107 needed anymore?