I think it would be awesome to get back the ability to revert views. With the config system this is possible, not exactly as before, but we can essentially revert changes that have been made to the active storage before they are pushed to the staging storage.

Here is an initial patch for this; I have added a revert link to the list controller, and updated the logic for displaying this too, based on an isOverridden() method on the ViewStorage object. I have also added a setOverridden method as the setter for this property, as well as a getConfigName helper method, which I think should possibly be moved later on to the config entity base controller.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

Issue tags: +VDC

Tagging

dawehner’s picture

+++ b/lib/Drupal/views/ViewStorageController.phpundefined
@@ -42,7 +42,17 @@ class ViewStorageController extends ConfigStorageController {
+    // Get config changes.
+    $source_storage = drupal_container()->get('config.storage');
+    $target_storage = drupal_container()->get('config.storage.staging');
+    $config_changes = config_sync_get_changes($source_storage, $target_storage);

So as written on IRC, isn't there a way to react on config changes? Manually determine all the changes all the time could be a performance issue

dawehner’s picture

Status: Needs review » Needs work
+++ b/lib/Drupal/views/ViewStorage.phpundefined
@@ -192,6 +199,55 @@ class ViewStorage extends ConfigEntityBase implements ViewStorageInterface {
+   * Returns whether the view configuration s overridden.

The lonely s, i'm sorry ;)

+++ b/lib/Drupal/views/ViewStorage.phpundefined
@@ -192,6 +199,55 @@ class ViewStorage extends ConfigEntityBase implements ViewStorageInterface {
+   *   TRUE if there are changes, FALSE otherwise.

"if there are changes" should probably better describe that there should be changes between the active and staging storage.

+++ b/lib/Drupal/views/ViewStorage.phpundefined
@@ -192,6 +199,55 @@ class ViewStorage extends ConfigEntityBase implements ViewStorageInterface {
+   * Sets the overridden status of a view.

probably also named not perfect

+++ b/lib/Drupal/views/ViewStorage.phpundefined
@@ -192,6 +199,55 @@ class ViewStorage extends ConfigEntityBase implements ViewStorageInterface {
+   * @param bool $status
+   *   The overridden status to set, defaults to TRUE.
...
+  public function revert() {

... there is no parameter :)

+++ b/lib/Drupal/views/ViewStorage.phpundefined
@@ -192,6 +199,55 @@ class ViewStorage extends ConfigEntityBase implements ViewStorageInterface {
+      // Revert the config staging changes.

what about mentioning the copying from the staging to the active storage?

damiankloip’s picture

Status: Needs work » Needs review
FileSize
6.13 KB

Thanks @dawehner! I have made changes based on your comments in #3, I'm still not sure about the performance issue of gettings this diff on each load. I'm not ure we can rely on having a listener on the config storage (If that is what you mean) as we would only know about changes when something changes?

damiankloip’s picture

FileSize
5.78 KB

Sorry, this patch please.

dawehner’s picture

Ah i see the point why you can't use a listener, what about retrieving this information on cache clear or something like that?

damiankloip’s picture

FileSize
7.01 KB

Rerolled and added caching.

Status: Needs review » Needs work

The last submitted patch, 1790398-7.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
7.1 KB

We need to clear the cache when we revert.

tim.plunkett’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

We've talked a lot about this since the last patch, it needs a new approach.

xjm’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » config.module
Issue tags: +Configuration system
tim.plunkett’s picture

Assigned: Unassigned » damiankloip

Assigning to @damiankloip to get his feedback on this, I think he planned to work on this at the pre-BADCamp sprint.

damiankloip’s picture

I'm not sure we can just move this issue to the config system, as we will still need to implement whatever is provided. So I guess this issue should be for that, like it was. The last patch implements (un ideally) what we can do now. This should evolve when we get new shiny features provided by the config system.

So we want two things I guess; a) the ability to revert to the original config provided by a module and b) revert/select a 'revision'.

In a nutshell, this definitely sounds like something we should hash out at the sprint.

damiankloip’s picture

Component: config.module » views.module
xjm’s picture

It actually sounds like we need one (or more) issues to fix it in CMI, and then one (or more) followups to implement it in Views. I'm happy to discuss it at BADCamp, though. In fact, it would be good to flesh out and sprint on #1790610: [META] Ensure the Configuration and ConfigEntity systems fully support Views CRUD and status operations at our sprint so that we can discuss that more when we have access to heyrocker (and get help for those issues at the sprint following BADCamp, which I will be attending).

xjm’s picture

Issue tags: +feature freeze
xjm’s picture

Priority: Normal » Major

Not having this is a regression from D7 Views, so bumping to major.

YesCT’s picture

What are the current blocking issues for this?

tim.plunkett’s picture

mtift’s picture

dawehner’s picture

Version: 8.0.x-dev » 9.x-dev
Issue summary: View changes

I doubt this will happen.

catch’s picture

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

'Revert from default config' is something we could add in a minor release I think.

jhodgdon’s picture

As a note, both Tour (maybe) and Configurable Help (definitely) [which was moved to contrib] need this type of "revert" or at least "see if things are different or new" functionality. See #2371439: Figure out what to do when module updates configurable topic for Configurable Help and #1924202: Tour tips are provided as configuration, so never get updated for Tour (not sure whether the Tour module is really this or not, but I've marked the Config Help issue as having this one as Related).

jhodgdon’s picture

I just made a sandbox/contrib module that will do this for any config entity:
https://www.drupal.org/sandbox/jhodgdon/2391835

I haven't specifically tested it on Views and it is a bit rough still but you may want to try it out?

jhodgdon’s picture

Just a note here that the sandbox referenced in the previous comment is considerably matured since a month ago:
https://www.drupal.org/sandbox/jhodgdon/2391835

On #1398040: Detect if default configuration of a module has been changed, and allow to restore to the original there is a discussion of whether this sandbox functionality should go into Core. I think it would resolve this issue if it did; if not, the resolution could be "use contrib".

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.

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.

kattekrab’s picture

I reckon this is something we should revisit. Unless it's already been addressed elsewhere?

I'm working on a talk that compares Views as contrib in D7 and in D8. Adding this as a gotcha for D8 (for now)

andypost’s picture

Assigned: damiankloip » Unassigned
Priority: Major » Normal
Status: Needs work » Closed (works as designed)

Using config.sync as source to compare for revert looks bad idea, but there's no way to find what is origin for view (it can live in modules & profiles same time)

Feel free to reopen if any ideas

jhodgdon’s picture

The thing to do, IMO (biased!) is to use the Config Update Manager module:
https://www.drupal.org/project/config_update

It is not in Core. See also #1398040: Detect if default configuration of a module has been changed, and allow to restore to the original for Core.

andypost’s picture

Yep, this looks promising but when same view exists in more then one module (profile)
Also translation revert does not work in config_update

jhodgdon’s picture

Translations are not configuration, so they cannot be reverted by the Config Update module, that is true. They are stored using some other mechanism, and they are not provided by modules/themes/profiles, so they don't work the same way at all as configuration.

Regarding two modules providing the same view... why would that be? I don't think two modules/profiles should be defining the same view, should they?