Problem/Motivation

In discussion with @bojanz we felt it would be useful to have a method to know if a piece of configuration is provided by a module and if it has not being changed by the user. This method could be used in post update functions by modules to change configuration (for example).

Steps to reproduce

Proposed resolution

Add ConfigCompartor class with method isModified

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-2656370

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

dawehner’s picture

This would also make the life of https://www.drupal.org/project/config_update a bit easier

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mglaman’s picture

Assigned: Unassigned » mglaman

Working on a patch to add a utility method to ConfigManager.

mglaman’s picture

Assigned: mglaman » Unassigned
Status: Active » Needs review
FileSize
2.9 KB

Here is a new isModified method which compares the current _core.default_config_hash against the actual configuration's value. This hash is created when the configuration is first installed, allowing us to tell if the configuration object was modified.

Status: Needs review » Needs work

The last submitted patch, 5: create_a_method_to-2656370-5.patch, failed testing.

mglaman’s picture

Status: Needs work » Needs review
FileSize
841 bytes

+1 me, somehow deleted namespace in test when making patch. Fixed.

Status: Needs review » Needs work

The last submitted patch, 7: create_a_method_to-2656370-7.patch, failed testing.

mglaman’s picture

#7 turned out to be the interdiff. Here's proper patch.

bojanz’s picture

Status: Needs review » Needs work
+  public function isModified($config_name) {
+    $active = $this->activeStorage->read($config_name);

$active could return FALSE if $config_name is invalid, the code doesn't account for that.
In Commerce we ended up accepting array $config instead of $config_name instead, since the calling code usually needs to load the active config anyway (and can handle failures). We should at least do that here as well.

I also dislike the entire idea of ConfigManager, which "provides helper functions for the configuration system". That means it has no clear purpose, and a license to grow indefinitely. I've opened #2684081: Modules need a way to keep their configuration up to date to address the full use case that isModified() is needed for.

mtodor’s picture

Status: Needs work » Needs review
FileSize
4.7 KB

I have modified patch with some changes and added additional tests.

So what I have changed is following:
1. I have added Exception in case that config is not found (could happen that config never existed or that it's deleted) - how that exception will be handled depends on module that uses functionality of isModified method.
2. I didn't change parameter type, because ConfigManager works with config names mainly and introducing array $config could make confusion.
3. Tests are covering following cases: when it's updated and not changed, was already covered with previous patch - I have added: when configuration never existed, when configuration is deleted, when it's updated and marked as "installed by module", this is case when module update unmodified config and mark it as default installed one.

Just to clarify things here, this method just provides information is configuration changed since last installation of module or not. How module will handle that information is up to module update implementation.

dawehner’s picture

Just a quick comment for now ...

  1. --- a/core/lib/Drupal/Core/Config/ConfigManager.php
    +++ b/core/lib/Drupal/Core/Config/ConfigManager.php
    
    @@ -481,4 +482,27 @@ public function findMissingContentDependencies() {
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function isModified($config_name) {
    

    IMHO we could just add this to its own helper class and avoid some BC break with this. Also, "Manager" is just an anti-pattern IMHO, as it is a lazy way to group just whatever you want into one class. Maybe having something like a "ConfigComparer" or so, would help a bit.

  2. +++ b/core/lib/Drupal/Core/Config/ConfigManager.php
    @@ -481,4 +482,27 @@ public function findMissingContentDependencies() {
    +    if (FALSE === $active) {
    

    Nitpick: We don't use yoda notation, but rather just $active === FALSE. Nice strict check, even afaik not neeeded.

jhodgdon’s picture

See also https://www.drupal.org/project/config_update
The base module has some classes that do exactly what this module is trying to do.

chr.fritsch’s picture

1. I introduced a new class ConfigComparator and moved the isModified function into it.

2. I also changed the active check

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
Ankit.Gupta’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
620 bytes

Rerolled the patch #14 in drupal 10.1.x

geek-merlin’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

The last patch misses the most and is not a reroll. So nw.

karishmaamin’s picture

Status: Needs work » Needs review
FileSize
5.62 KB

Re-rolled patch against 10.1.x. Please review

smustgrave’s picture

Issue tags: -Needs reroll
FileSize
1.53 KB
5.97 KB

Thank you for the patch just fyi

Commit check failures
The last patch doesn't pass commit checks, could you make sure to run ./core/scripts/dev/commit-code-check.sh before uploading a patch to make sure there are no issues with code formatting. see https://www.drupal.org/docs/develop/development-tools/running-core-devel...

Fixed issue which was around setExpectedException()

Status: Needs review » Needs work

The last submitted patch, 32: 2656370-32.patch, failed testing. View results

smustgrave’s picture

Status: Needs work » Needs review

Appears to be a random layout builder failure.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
FileSize
144 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Bhanu951 made their first commit to this issue’s fork.

Bhanu951’s picture

Assigned: Unassigned » Bhanu951

Bhanu951’s picture

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs change record

Self review.

This will need a change record for the new service.

joegraduate’s picture

Status: Needs work » Needs review

Created a draft CR for review:

https://www.drupal.org/node/3350329

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: -Needs change record

Thanks @joegraduate for the CR.

Moving to NW for the open threads on the MR 3338

Bhanu951’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Needs work

2 Nitpicky ones.

Bhanu951’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Threads appear to be answered.

quietone’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Bhanu951’s picture

Status: Needs work » Needs review

Review Comments addressed.

smustgrave’s picture

Status: Needs review » Needs work

Failure in MR seems legit to the issue.