(This is technically a feature request, but one that affects huge portions of contrib)
In D7 the assumption was that the module owns its configuration.
In D8, the assumption is that the site owns its configuration. After the module is installed, the configuration it supplied is never updated.
Most common example: Views. In D7 you supply a default view. If the user hasn't modified it, it will always match the version in the module.
If the module adds a filter to that view, that filter will be picked up after a cache clear.
Most non-trivial modules will need the "update this configuration if the user hasn't modified it" functionality.
Admin views, frontend views, rules, descriptions on field instances, etc.
It's also possible to need to create configuration added to the module. For example, a module update that ships a new action
to be used on the admin view.
Without this the module users would get drastically different UX based on when they installed the module.
We solved this in Commerce by implementing a ConfigUpdater class which can be used in hook_post_update_NAME().
It was influenced by the config_update contrib which also attacks the same use case.
It has methods for importing config, deleting config (rare use case), and reverting config (with $skip_modified being TRUE by default).
Here's the commit: https://github.com/drupalcommerce/commerce/commit/af4e4bb3507cc1119e92ea...
And here's a usage example: https://www.drupal.org/node/2677906#comment-10947049
I want us to place that code in core, cause contrib shouldn't need to copy around 200 lines of code, or depend on another contrib in order to ship with an up to date View.
Comments
Comment #2
bojanz commentedHere's the config_update version, the ConfigReverter: http://cgit.drupalcode.org/config_update/tree/src/ConfigReverter.php
Differences:
1) Requires $type, $name, instead of an array of $names (which I think is more user friendly).
2) Doesn't have the $skip_modified / isModified() functionality.
3) Doesn't have the result object with success/failure messages (which we added because of our update hook focus)
4) Throws events, which is useful.
Otherwise, close. I'm happy to do some bikeshedding around the API.
Comment #3
alexpottComment #4
dawehner@bojanz
So what about adding the events to your version?
Comment #5
bojanz commented@dawehner
I have nothing against, it just wasn't done in the initial commit.
What we need to discuss is the Updater VS Reverter focus, and the result object (succeeded/failed) that goes with an updater focus.
Comment #6
alexpottOne question I have about this is this... if a view title X and the module author changes it to Y but the site was qa'd with it being X but the module author decides to update unmodified view which this is... why should it change? If I was the qa team / site owner I'd be really annoyed something I qa'd just changed on the module author's whim.
Comment #7
catchYes this was exactly my concern when this was brought up in irc. We can't distinguish between things that weren't changed because an explicit decision was made to keep them as is, vs. things that weren't changed because no-one was bothered either way. Only in the latter case does updating make sense.
The issue summary covers the ctools/features case, but it doesn't cover things like taxonomy vocabularies, node types and other core config entities where default content was usually created via API calls rather than hook implementations. Those cases are closer to how things are treated in 8.x, so it's not as if 8.x has completely reversed what 7.x does, 7.x was extremely inconsistent.
We have the same problem with static configuration.
In 7.x, if you save a configuration form, then regardless of whether the values in the form match the defaults or not, they'd all get saved to the {variable} table and used regardless of changes to the default variable_get() argument. For that matter the same happens with Views if you save anything in the UI regardless of whether changes are made.
There's also config dependencies. Let's say a module provides a default field. If I've created a View depending on that field, then changing the field type might break it, changing the field description won't. If it's an image style, changing the dimensions could definitely break my theme.
So providing the API for this is OK, but it was never a clear cut situation in 7.x core or contrib either. I'm also not clear whether this is supposed to be for updating configuration or for 'reverting to defaults'. It's quite possible to update configuration via an update hook, and core has several examples of this. Reverting to defaults seems like something the user can be given a choice for.
Comment #8
bojanz commentedYes. And it needs 200 lines of helper code, which mglaman and I wrote.
I am not asking core to do anything automatically. Each module writes its own hook_post_update_NAME() hooks.
If Commerce decides it wants to keep its admin views up to date, then it can write the hook, use the updater, and do it.
But it's wrong for the only answer to be "this is not possible via the current APIs".
Sure. Keep in mind your examples are all mostly frontend. On the backend they are much more likely to be a non-issue (and keeping people's admin UIs out of date is much more likely to earn me stern bug reports than the opposite).
So, let's continue then with the ConfigUpdater focus, not the ConfigReverter one.
Comment #9
agoradesign commentedAbsolutely agree with bojanz!
I think the most important aspect of this discussion is, that the pure existence or non-existence of a certain helper functionality in the API does not encourage nor prevent a module maintainer from updating configuration entities. If I'm module maintainer and decide that i want/need to change a views title to "xyz", then I will do it anyway. I will write an update function that will change the view, even if I have to write 200 lines of helper code.
But - and that's the point - if I always have to write that much extra code, that's bad in every sight. It's complicated, you need far more time, you end up in redundant code (at least through different modules and/or projects) and there's always a big risk of introducing some bugs. Instead it would be far more better and easier to have a couple of solid API helper functions, that help me doing my changes, that I want to do anyway. It's in the responsibilty of the module author to use them correctly and wisely. But this applies to many other things. Just think of any kind of database operations,...
Comment #10
agoradesign commentedI have a concrete example, where the lack of these API functions caused a problem, as I'm 99% sure that the concerning update hook would have used these functions instead. At least it is a similar problem, as it does not have to do with configuration updates, but with entity definition updates. There's a \Drupal::entityDefinitionUpdateManager()->applyUpdates() function, which applies all pending entity definition updates, but there's nothing to call when you only want to apply updates of certain entities. You can read the following issue to see what problems can arise here: #2671194: Call to \Drupal::entityDefinitionUpdateManager()->applyUpdates() in update 8106 can cause problems
So the basic message behind that issue is the same as here with updating config entities. A concrete API function is missing, so you help yourself with finding alternatives, which may end up in unwanted problems you weren't aware of!
Comment #11
catch@agoradesign, have you seen https://www.drupal.org/node/2554097? There's a full update API for updating configuration entity schema down to the level of individual fields etc. specifically to avoid just updating all entities at once. That was one of the last update path critical issues during the 8.x beta and very painful to resolve.
Comment #12
agoradesign commentedNo, thanks! That's really cool :-))) I'll refer to that CR in the mentioned issue.
Ok, under these circumstances you have to re-interpret my example. Like so: it would be very important to have some great API functions for configuration updates like we have already for entity schema updates.
I even think that the description of that CR delivers perfect reasons, why we'd need that functionality. The automatic updates were removed because the behaviour was unpredictable. Instead, we now have a great toolset for handling schema updates, that module authors may use. => why not offer similar functionality for configuration updates? I think, we all agree that automatic updates are both impossible and unwanted here for the same reasons. But why not offer the helper functions here too to eliminate the pain of handling such updates for module maintainers?
Comment #13
mglamanHere is where I find the ideas behind ConfigUpdater useful for developers. At my previous workplace, I developed a SaaS built on top of Drupal. We worked in about three-week sprints, which mean we progressively added new configurations, or configuration changes to fix bugs. However, we also allowed people to customize their websites (it was a WYSIWYG site building platform.)
Features, as it was, made importing new config easy (as does ConfigUpdater.) However updating config was atrocious as we had to ensure it was not overridden, or else the customer would lose changes. That's where ConfigUpdater's update is useful, as it checks if something is modified and will update accordingly.
Yes, the current API supports all of this, but it would be lengthy to implement each time. I would have to re-create my steps for each sprint release, or shoot, even each ticket which implemented change.
In my opinion, the DI container in Drupal 8 makes it an even better candidate for SaaS solutions. However we need to ensure the developer experience for delivering configuration updates is there as well.
Comment #14
jhodgdonAt this point, I think this is 8.2.x material.
So... I would be happy for some or all of the API portions of the Config Update contrib project to get into Core. Besides ConfigReverter, there is also a ConfigDiffer class that provides some diff functionality that is (in my not-so-humble opinion) vastly better than the diffs that Core is currently displaying when looking at config synch. And there is a ConfigLister class that aids in figuring out what is different between currently active config and config provided by modules and other projects.
Features module is using several of these API classes, and maybe some other modules are as well -- I've been getting a lot of questions/patches in the issue queue lately.
I would also be happy to see the UI portions of the Config Update module get into Core, but that is perhaps a separate issue from this one. Basically, it provides a report and some Drush functionality that lets users see a list of what is missing/added/changed, and you can see this by module/theme/profile that provided the config originally, or by config type (like all Views configs, independent of which module provided them).
Anyway... if you'd like I could make a patch here that would add the API portions of the Config Update project to Core. I do think that all of them would be beneficial for contrib, not just the Reverter class/service. We can open a separate issue about the UI stuff.
Comment #15
jhodgdonAlso, check out these Core issues that could perhaps be resolved if we had this in Core:
#1790398: Re introduce revert functionality for views using the config system
#1924202: Tour tips are provided as configuration, so never get updated
#1398040: Detect if default configuration of a module has been changed, and allow to restore to the original
#1497268: Add revert functionality to Config entities
Comment #16
bojanz commented@jhodgdon
Based on the feedback so far it seems we'll want to limit scope to hook update helpers, as it looks in commerce.
See #1 for my comparison between ConfigUpdater and ConfigReverter.
Other API classes will need issues of their own.
Comment #17
jhodgdonAnd RE #2... In Config Update, I separated out the "revert some config" functionality from the "decide if it's modified" functionality, which is why the "skipUpdated" is missing from my Revert class. See the ConfigDiffer class.
http://cgit.drupalcode.org/config_update/tree/src/ConfigDiffer.php
I think that separation is a good idea, and am against mixing the two together in one class. It's cleaner this way.
I'm undecided on the other API differences.
Comment #18
jhodgdonI guess my point is that if we go the way of the config_update project, modules would need the functionality of both ConfigDiffer (to see if there are differences) and ConfigReverter (to apply the changes). The functionality in the Commerce project combines the two.
They wouldn't need ConfigLister probably, although that class has some methods that return a list of all the changed config objects, which can also be useful.
Comment #23
bircherComment #35
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #36
pameeela commentedThis has been a simmering product need for Drupal CMS for a while now, and I came across this when searching for any related requests/discussions. I'm not sure whether this issue is the right place to revive it since it's so old, but I don't think the fundamentals have changed very much.
Since Drupal CMS is basically a pre-configured Drupal install, and it is aimed at less expert users, the need for shipping product improvements is more pronounced.
I pretty much agree with the original issue summary, and I wonder if the existence of Drupal CMS provides a way to address some of the concerns, when it comes to "I haven't modified this because I don't care" vs "I haven't modified this because I specifically like it and don't want any upstream changes".
E.g. what about a toggle for "allow upstream config changes to unmodified config" that we could enable by default in CMS, but disable in core?
Fundamentally though there is a requirement for Drupal CMS to (easily) ship updates to config that has not been modified, or adding new config.
Comment #37
catchThe issue title here is specifically about modules, but Drupal CMS config mostly comes from recipes. At the moment Drupal CMS goes out of its way to remove recipes from the site after they're applied, since #3524255: Automatically unpack recipes on project creation so that even the code does not get updated.
Updating configuration in recipes is going to be more or less incompatible with making any significant changes to those recipes. e.g. if a recipe starts to require a module that it previously didn't, then updating the config by itself won't work, would also have to composer require and install that module. Or if module A is swapped for module B then it's not a case of updating configuration but deleting it and replacing it with something else. But that won't work if a field type is changed because content already exists depending on that field type etc.
So whether site admins have modified something is only part of the problem, the other issue is whether they've created config or content that depends on the config being changed (views, canvas templates etc.).
It would be possible at that point to fully replace the recipe with a different one in Drupal CMS, so that only new sites get the recipe, but then both need to be maintained, especially once the expectation is set that recipes are maintained over time for sites that previously applied them. And that also undoes the use case here in that older sites won't get the new recipe - although it might keep them going for a few months at a time across smaller changes.
There are non-update-path approaches to this problem like https://www.drupal.org/project/config_sync in contrib, and #1497268: Add revert functionality to Config entities in core - but that core issue is pretty stalled too and I've never used config_sync.
Comment #38
pameeela commentedMy intention was not to get into a discussion about updating recipes. Drupal CMS ships with many contrib modules that could leverage this functionality if it were available. I agree the recipe side of things is a can of worms and not necessarily relevant to this issue.
Comment #39
catchOK that's not how I read:
since the pre-configuration comes from recipes?
Are there specific examples of where this has come up in contrib modules recently?
Comment #40
smustgrave commentedWould one example be the content view? If we add a new filter existing sites wouldn’t get it. Could be way off for that example though
Comment #41
pameeela commentedThat's why I found the issue, among several others, but there are going to be a few different threads to pull.
A good example of this is #3086883: Support limiting the exposed language filter to the languages used by the content in the list (from core, not contrib, but the same principles apply and it's almost exactly the same as what is described in the IS). That is honestly one of the weirdest user-facing behaviours that we have and it is a shame that it will only apply to new sites without manual intervention.
I think we should try to support shipping these improvements to existing sites and that Drupal CMS provides a good differentiation point to opt in to them. It would be good to gauge consensus on this point though and this issue seemed like a decent place to start.
Comment #42
mglamanAny time someone wants to deploy configuration changes to customers (so SaaS) and not break their configuration.
Drupal Commerce or any contrib shipping Views alternative for their entity list builder.
Comment #43
catch#3086883: Support limiting the exposed language filter to the languages used by the content in the list is probably a good example , where it's theoretically possible to update the view but we currently don't because in the past whenever we did that, something would break somewhere in someone's view and it's been much easier not to.
I think somewhere there's a core issue to try to introduce 'choose your own adventure updates' but all I could find was this comment referencing it obliquely from ten years ago #2732393-6: Decide if and how to try to save incorrectly orphaned images uploaded in text_with_summary fields from #2725415. But the idea would be that updates could have a description that explains what the update will do, and then the site owner can select during the update process whether it should run or not. A bit like in ubuntu/debian updates when it tells you a modified file is going to get updated and gives you options whether to update it or not. That could probably be in turn put behind an 'always accept' global setting. However that doesn't cover being able to check whether the configuration was ever changed or not, and to do that we'd need to be able to filter out config that's been updated due to schema changes vs. config that's been explicitly changed. Or make it so that those updates instead of trying to update config, completely replace it with the shipped version (so a revert), and the text warns you that this is what will happen and any customisations will be lost.
Comment #44
catchMaybe it would be possible to use a hash comparison.
Load the config from file or the db, remove site-specific keys like uid, and then hash it. The module doing the update would need to generate the hash with the old default config value, and then compare that against the hash from the live config, and then only if that matches, do the update (probably still with the confirm form step). By comparing the live config against the previous version of the default config that should handle schema updates and other automated updates to config but also not match if the site owner has made intentional changes.