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.
Comment | File | Size | Author |
---|---|---|---|
#9 | 1790398-9.patch | 7.1 KB | damiankloip |
#7 | 1790398-7.patch | 7.01 KB | damiankloip |
#5 | 1790398-5.patch | 5.78 KB | damiankloip |
#4 | 1790398-4.patch | 6.13 KB | damiankloip |
views-config-revert.patch | 5.75 KB | damiankloip | |
Comments
Comment #1
damiankloip CreditAttribution: damiankloip commentedTagging
Comment #2
dawehnerSo 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
Comment #3
dawehnerThe lonely s, i'm sorry ;)
"if there are changes" should probably better describe that there should be changes between the active and staging storage.
probably also named not perfect
... there is no parameter :)
what about mentioning the copying from the staging to the active storage?
Comment #4
damiankloip CreditAttribution: damiankloip commentedThanks @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?
Comment #5
damiankloip CreditAttribution: damiankloip commentedSorry, this patch please.
Comment #6
dawehnerAh i see the point why you can't use a listener, what about retrieving this information on cache clear or something like that?
Comment #7
damiankloip CreditAttribution: damiankloip commentedRerolled and added caching.
Comment #9
damiankloip CreditAttribution: damiankloip commentedWe need to clear the cache when we revert.
Comment #10
tim.plunkettWe've talked a lot about this since the last patch, it needs a new approach.
Comment #11
xjmI actually think we should fix this in CMI. See all the sibling issues at #1790610: [META] Ensure the Configuration and ConfigEntity systems fully support Views CRUD and status operations.
Comment #12
tim.plunkettAssigning to @damiankloip to get his feedback on this, I think he planned to work on this at the pre-BADCamp sprint.
Comment #13
damiankloip CreditAttribution: damiankloip commentedI'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.
Comment #14
damiankloip CreditAttribution: damiankloip commentedComment #15
xjmIt 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).
Comment #16
xjmComment #17
xjmNot having this is a regression from D7 Views, so bumping to major.
Comment #18
YesCT CreditAttribution: YesCT commentedWhat are the current blocking issues for this?
Comment #19
tim.plunkettNot sure, but #1515312: Add snapshots of last loaded config for tracking whether config has changed might help
Comment #20
mtiftRelated: #1497268: Add revert functionality to Config entities
Comment #21
dawehnerI doubt this will happen.
Comment #22
catch'Revert from default config' is something we could add in a minor release I think.
Comment #23
jhodgdonAs 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).
Comment #24
jhodgdonI 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?
Comment #25
jhodgdonJust 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".
Comment #29
kattekrab CreditAttribution: kattekrab at Creative Contingencies commentedI 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)
Comment #30
andypostUsing 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
Comment #31
jhodgdonThe 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.
Comment #32
andypostYep, this looks promising but when same view exists in more then one module (profile)
Also translation revert does not work in config_update
Comment #33
jhodgdonTranslations 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?