Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
In #2148199: Use snapshot to warn users if the configuration has changed since last import, we added functionality to warn the user when configuration has changed since the last config import, to protect site owners from unintentionally overwriting their live configuration with older configuration (for example, if content authors changed configuration on a production site and then additional changes were deployed from the development environment). This warning is displayed when the config differs from the last snapshot, and a snapshot is created on installation and again whenever the user synchronizes their configuration.
Because the first snapshot is created immediately on installation, any configuration change after installation results on the message being "stuck" on the config synch page.
Steps to reproduce
- Install the standard profile.
- Enable (e.g.) the simpletest module.
- Visit
admin/config/development/configuration. You will see this warning:
Your current configuration has changed. Changes to these configuration items will be lost on the next synchronization:
Screenshot:
It's unclear to the user what they're supposed to do because of this warning. Displaying the warning when configuration has never been exported or imported doesn't make sense, and the site owner also has no way to get rid of it. Synching config would remove the error... but there's no changed config to import and so neither drush nor the UI will allow the user to synch. The only workaround would be to export the config, hack it by hand, synch the hacked config, then revert the hack and re-synch.
Beta phase evaluation
| Issue category | Bug because the purpose of the warning is to help the user understand what will happen when configuration is synchronized, but the current behavior instead obfuscates that. |
|---|---|
| Issue priority | Major because this is an issue that will affect every single site owner the first time they try to synchronize configuration, and there is no way to get rid of the misleading warning in a normal workflow (i.e. without hacking the config export). |
| Disruption | TBD |
Proposed resolution
There are a couple ways to address this:
- Provide the user with some way to deliberately create a new snapshot.
- Create a new snapshot on export as well as import.
- Don't create a snapshot on installation.
- Display a different warning when the user has never synchronized configuration.
@alexpott and I discussed #2 and ruled it out, because the purpose of the warning is to protect site owners from accidentally reverting live configuration changes. There is no guarantee that the export ends up being incorporated into whatever might be subsequently imported. (For example, a site owner could export configuration on production, then stage configuration from dev over that, and would be back in the situation of not having any way of knowing what's being overwritten.)
#1 could be useful generally, but might introduce its own usability problems, as the user might interpret it as a prompt to create a new snapshot, and then be in the same situation as #2. It could also be combined with other solutions. #1 has been ruled out as adding additionally complexity and features to an already complex UI. Also atm the moment in HEAD the snapshot is only used to produce this warning message. Adding this functionality means that this warning message would then become unreliable.
#3 makes a certain sense, since there's no indication that the site is part of a deployment workflow until a synch happens. However, it would result in sites being in an ambiguous state when the site owner is first setting up their workflow (especially since core does not currently support installing a site from configuration). For example, the site owner might create a copy of the configured site for production, make the production site live, make one change in production, and then still end up unwittingly overwriting that on the first synch. #4 would help avoid that problem.
#4 was implemented as part of writing the patch but it turns out that warning the user at this point actually just detracts from reviewing the changes that are already listed. And in this instance you have no idea what might be different on this site instance since the snapshot is taken on site installation and not when the user actually copied the site. Therefore this solution was rejected.
The implemented solution is #3 which means that message warning that you might be about to overwrite local changes only can occur after an initial import of configuration.
Remaining tasks
Review
Commit
User interface changes
- The warning is not "stuck" on the config synch page after the initial installation.
- A different warning may be added.
API changes
TBD.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2388867.7.patch | 9.06 KB | alexpott |
| changed_from_what.png | 40.28 KB | xjm | |
Comments
Comment #1
xjmIn general, it's also confusing to see "your configuration has changed" and "there are no configuration changes" on the same screen. (Like, which is it?) We're using the word "changes" to mean two different things on the same screen. That might be a separate (normal) usability bug to resolve.
Comment #2
xjmComment #3
xjmComment #4
Wim LeersHah, I thought this was happening because I installed beta 1 many weeks ago and then kept upgrading to latest 8.0.x with `drush cr`! Glad to see this issue opened, also helped me understand this better.
Comment #5
jhodgdonI think this should be in the Config Manager module component, not the base config system, since it's about warning user on the config synch page?
Comment #6
yoroy CreditAttribution: yoroy commentedThis configuration page indeed handles two different things that make it hard to understand what is going on. As for the warning message, I think the best strategy would be to introduce a message for the 'blank slate' state. Similar to the empty message we apply to tables (https://www.drupal.org/node/1146122).
Comment #7
alexpottThe patch attached is a implementation of option 3 from the issue summary. I think this makes the most sense.
Here's what it does.
I thought about adding a message If there is no snapshot and there is configuration to import (option 4 from IS). The message would say something like Configuration has never been imported to this instance of the site. Any changes made solely to the configuration of this instance will be lost on import.. I'm not entirely convinced this message is actually useful - since the changes a configuration import is going to make are detailed on the screen anyways.
Comment #8
jhodgdonSo... This seems like a good start, but it still seems like we also need option #1 from the issue summary, which would allow someone to update the snapshot at any time. This would allow them to:
- See what has changed in their configuration since the last time they made a snapshot.
- Have a baseline for comparing "current" or "snapshot" to what they're importing from staging.
The UI for this needs to make it clear that there is only one single snapshot stored. The name "snapshot" might otherwise imply to people that they could make a whole series (like content revisions that you could revert back to anything in the sequence). Or maybe "baseline" would be a better name for the UI, because I think a button that says "Establish the baseline" or "Update the baseline" or something to that effect would communicate the concept more clearly than "Take a snapshot".
Comment #9
jhodgdonAlso... So this page is still displaying two different things, and only allowing operations on one of them.
The top part displays a list of which config objects have changed in active config since the last snapshot/baseline, and I don't think there is any way to act on these differences or see the details.
And then the bottom part displays a list of which config objects have been added, renamed, changed, etc. between staging and active, and allows you to see detailed line-by-line differences and also to import the staging to be your active config. And when you do an import, it implicitly does a snapshot afterwards.
Do these even belong together?
Comment #10
Berdir@jhodgdon: The way to see details is check the files below in the diffs. The problem it is trying to solve is that you don't know which configs changed in staging and in active, so it is telling you which objects you probably want to look at specifically.
There might be other ways to display that, e.g. with a marker inside the list. But I'm not sure how much this issue should change, I think that getting rid of the message on installations that you never imported into is the most annoying part right now, trying to improve it is IMHO a different issue.
Comment #11
alexpottSo the whole point of this message is to warn that you might be about to lose configuration changes made on the site by importing configuration. That is all. So it definitely belongs on the screen where you are about to do the import.
We can detect that you might be about to lose local changes by comparing active to a snapshot of configuration taken after the last time configuration was imported. The patch only changes the fact that this comparison is only made after the first import - which is the only way we can know if the user intends this particular site instance to be a target for config imports. It is a warning message and only part of the page if you are at risk of overwriting local changes.
I don't think is should be core's responsibility to add functionality to act on the snapshot storage - this is something that can be explored in contrib - also having multiple snapshot storages too. This patch does not add any mentions of the word snapshot to the UI and I think it would be a mistake to do so.
Making the UI even more complex by being able to compare staging to snapshot is not going to help either.
Comment #12
jhodgdonOK. I agree that this patch is a good step, and probably other discussions of the usability of this UI, or lack of help, and other issues, should be discussed at #2247291: Reorder tabs in configuration UI. However, since item 1 (ability to make a snapshot) is part of this issue summary, should it not be discussed here?
Comment #13
alexpottWhilst reading the issue summary and writing and testing the patch I discounted possible resolution 1 as just adding more complexity. I will update the issue summary to reflect the solution in #7.
Comment #14
alexpottComment #16
jhodgdonI took a deeper look at the patch file today, and also applied it on simplytest.me to try it out, side by side with an unpatched Drupal 8 (in another browser).
On both sites, I did the following:
- Installed using Standard profile.
- Went to admin/config/development/configuration - nothing reported there so far on either site.
- Went to admin/modules and turned on the Testing module.
- Back to admin/config/development/configuration - see the warning message about config changed on the unpatched site, not on the patched site. Note: I think you have to go back to this page to see the message. I don't see it on admin/config/developing/testing. Updating summary's Steps to Reproduce with this change. Anyway, that is good. The patch takes away this confusing warning, as advertised.
- Made a config export zip file, from admin/config/development/configuration/full/export [made a different file for each site]
- Went to admin/config/system/site-information and changed the site name and saved.
- Imported the zip file on admin/config/development/configuration/full/import
- That took me back to the Synchronize page, where I could view differences, which was only the change to the site name that I expected on the patched site. [On the unpatched site, I still had the confusing message, but I ignored it.]
- Clicked Import All. The site name is back to what it was. Message is gone on unpatched site. Also verified the Testing module is still enabled.
- Uninstalled the Testing module and then went back to admin/config/development/configuration. The unpatched site does not tell me there are any changes (??!? that is wrong). The patched site tells me that there are changes, which make sense (simpletest.settings and core.extension). I even tried on the unpatched site to do the Import again to get back to my "simpletest is installed" config state, and I still don't get notification that there are changes.
So. This patch works well, and is definitely needed.
Looking at the code in the patch, I can't see any problems. Let's do this.
Comment #18
webchickGreat, thanks so much for the thorough testing job on this. Agreed that this seems super confusing.
Committed and pushed to 8.0.x. Thanks!