Problem/Motivation

Currently, Configuration Revert offers a specialized UI for analyzing the state of configuration sitewide.

This is definitely useful. However, for many users, it could be more intuitive to access such information in the context of the regular UI for managing a given type of configuration. This would be similar to for example the Drupal 7 Views admin list UI.

Proposed resolution

Work with ConfigEntityListBuilder to conditionally add diff and revert links where applicable.

Remaining tasks

User interface changes

API changes

Comments

jhodgdon’s picture

Status: Active » Closed (won't fix)

I don't think this is even possible. There is no way to find out what the "regular UI" is for a given config type, that I know of.

I also think that even if it were possible, it would be a very difficult thing to manage. The regular UI is for editing config, not for diffing it.

jhodgdon’s picture

Oh I see that maybe you're suggesting that Config Revert add diff/revert links to the regular listings of config.

This is maybe possible. ConfigEntityListBuilder invokes hook_entity_operation() and _alter() to build operations links, passing in the entity, and you can call $entity->getType() to get the entity type object, and presumably figure out that it's a config entity.

However, I really don't think this is a good idea. The diff and revert operations will not be very understandable to people outside the context of the report, in my opinion, unless they had much longer names on those links. Also, it would take some time to calculate which operations are valid for a given entity: you'd need to load the entity from config/install as well as active storage and figure out if it's different or not, to see if there is even anything to diff or revert. This would need to be done for every entity in the listing, adding quite a bit to the time to build what could be a pretty long page.

So I really don't think this is a good idea. Sorry.

jhodgdon’s picture

Just as a note, I am thinking here partly about the support burden, on site administrators as well as the maintainer of this module. Whenever you introduce things that could be confusing, you create a support problem. I am pretty sure this would be confusing. At least if someone goes to the report page and has the Help module enabled, there is some text in the help box that tells them what they're seeing. But if you're for instance on the Views listing page, and you see a "revert" link, which really means "update this view to what's on the disk right now", or a "diff" link, which shows you a diff that as you've pointed out on other issues, may not be understandable by the average Views administrator, I really think you're just asking for trouble.

The management of web site updates is complicated. It should, in my opinion, be handled by someone who really knows the site and understands what happens when you update a module. Blindly applying updates is never a good idea on any site, and the operations offered by Config Revert are no exception. Therefore I think having the report and its associated operations on this separate page and not merged in with the rest of the UI for the individual entities makes the most sense.

There are also several config entities that don't even have a UI, such as Tours, and simple config also wouldn't work with this. So you'd be left with the ability to manage some types of config in one way and others in a different way, diffs that would not match the format of the other operations, etc.

Keeping it separate is also consistent with what the core Config Manager module does -- you don't get Export links added to the operations on config management UI screens either. Both Config Manager (core) and this module are for experts who know what they're doing. They shouldn't be mixed up in the generic UI.

nedjo’s picture

The diff and revert operations will not be very understandable to people outside the context of the report

I agree that trying to navigate diffs and reverting for arbitrary types of configuration sitewide is an expert task.

But reverting a piece of configuration you've edited seems like a reasonable task for a configuration editor. Currently in Drupal 7, reverting is a common workflow step for configuration editors that's built into the editing UI. In core, for instance, an overridden image style will offer a 'revert' operation in the listing at admin/config/media/image-styles. It's true that reverting a view will mean "update this view to what's on the disk right now", but that's exactly what it means now in D7, and that fact appears to be understood and accepted.

The draft patch in #1790398: Re introduce revert functionality for views using the config system added reverting to the list of operations for a view in the views listing.

Diffs are indeed a more complex question, and offering them in e.g. a view listing could be confusing. That said, doing so might help with "learnability". I'm more likely to be able to grok a diff if it's of a piece of configuration that I myself have edited, and I'm much more likely to find the link if it's in my regular editing UI.

In sum: revert links, at least, seem to be a good fit for config entity lists. If the implementation would be made complex by not being in core, possibly it would be better to wait until if/when this module is a candidate for core. Diff links are less of an obvious fit, but seem worth considering.

jhodgdon’s picture

The problem is it is not really just a "revert", and you really need to look at the differences before you decide if it is OK to revert. When you "revert", you are possibly updating to a new version, and possibly reverting your edits. I don't think (as you've pointed out on other issues) that looking at the diffs is for the average admin, and it doesn't match what someone who, for instance, knows how to make views in the UI would understand, so I don't think putting the diff and revert on the Views list as operations will be satisfactory. Not going to do it.

jhodgdon’s picture

Just to be clear, the reason I'm not willing to add the "revert" links to other UIs is mostly that I don't want to support them and I think it's too dangerous for the average "I know how to make views" user. I also disagree that people actually understood what the views revert links were doing in D7.

nedjo’s picture

I see this issue as depending on an answer to #2414339: Consider adding to Core (8.1.x). Would postponed work?

I don't mean that everyone using D7 intuits the details of what happens when you revert a configuration item. I just mean that I personally haven't run into signs that anyone's too fussed about whether reverting restores an original or an updated version. A quick scan of the views issue queue doesn't show any obvious signs of confusion or frequent support requests.

For diffs, I wonder if we could offer a form of visual diff. That is, rather than displaying the text diff, bring up the configuration item in edit mode with the extension-provided values. For views, at least, this would show graphically what has changed in a form familiar to views admins (deleted displays crossed out, new displays outlined in red, changed displays starred, and so on).

jhodgdon’s picture

I'm really not considering adding this to this module, which is why I put the status to Won't Fix. But I don't really care if you want to reopen it and put it to Active. Rather than that argue further about why, which I think I have said enough times.

Regarding diffs in the config item, that will not show you the differences or what has changed, it will show you the new values. I don't think what you are imagining for Views will work.

jhodgdon’s picture

Status: Closed (won't fix) » Active

Here, happy?

nedjo’s picture

Version: » 8.x-1.x-dev
Status: Active » Closed (won't fix)

Won't fix is fine, I'm just thinking this is a question to come back to and potentially reconsider if/when this is considered for core, when questions like maintenance burden will still be relevant but at least won't fall on a single maintainer.

jhodgdon’s picture

Status: Closed (won't fix) » Postponed

Sure, then let's go ahead and leave it open but postponed, so it doesn't get lost, as you suggested in an earlier comment. I'll add a note to the "add to core" issue.

jhodgdon’s picture

Component: Code » Reports module
Pasqualle’s picture

Status: Postponed » Closed (won't fix)
Related issues: +#1497268: Add revert functionality to Config entities

Drupal core has its own issue for the config revert functionality #1497268: Add revert functionality to Config entities