Add validation constraints to core.extension

Created on 20 March 2024, 10 months ago
Updated 13 September 2024, 4 months ago

Problem/Motivation

Over at ๐Ÿ› Install profile is disabled for lots of different reasons and core doesn't allow for that Fixed , the schema for core.extension:profile is being refined.

Once that lands, we'll be able to make it validatable too, and make the entire config object validatable!

Conversation that triggered this issue to be created:

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

๐Ÿ“Œ Task
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Baseย  โ†’

Last updated about 5 hours ago

Created by

๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @wim leers
  • Merge request !7107Outline what needs to happen here. โ†’ (Open) created by wim leers
  • Issue was unassigned.
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Draft MR created.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • Pipeline finished with Failed
    10 months ago
    Total: 177s
    #124087
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Closed ๐Ÿ“Œ Improve core.extension:profile validation Postponed as a duplicate. Crediting @alexpott.

  • Status changed to Active 10 months ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • Pipeline finished with Failed
    10 months ago
    Total: 165s
    #125256
  • Pipeline finished with Failed
    10 months ago
    Total: 487s
    #125262
  • Status changed to Needs review 10 months ago
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • Pipeline finished with Failed
    10 months ago
    Total: 184s
    #125286
  • Status changed to Needs work 10 months ago
  • 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.

  • Pipeline finished with Failed
    8 months ago
    Total: 179s
    #184758
  • Pipeline finished with Failed
    8 months ago
    Total: 266s
    #184796
  • Pipeline finished with Failed
    8 months ago
    Total: 263s
    #191502
  • Pipeline finished with Failed
    8 months ago
    Total: 574s
    #191509
  • ๐Ÿ‡ฎ๐Ÿ‡ณ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 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.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia kunal.sachdev
    • @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.
  • Pipeline finished with Failed
    7 months ago
    Total: 602s
    #202678
  • Pipeline finished with Failed
    7 months ago
    #203544
  • Pipeline finished with Failed
    7 months ago
    Total: 571s
    #203550
  • Pipeline finished with Failed
    7 months ago
    Total: 597s
    #203574
  • Pipeline finished with Failed
    7 months ago
    Total: 476s
    #208437
  • Pipeline finished with Failed
    7 months ago
    Total: 566s
    #210465
  • Pipeline finished with Failed
    7 months ago
    Total: 483s
    #210505
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    yash.rode โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Success
    6 months ago
    Total: 540s
    #221448
  • Status changed to Needs review 6 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    Tests are passing now

  • Pipeline finished with Success
    6 months ago
    Total: 1171s
    #224663
  • ๐Ÿ‡ง๐Ÿ‡ช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
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Believe this needs work for the feedback mentioned in #16? Also is MR 7107 needed anymore?

Production build 0.71.5 2024