We are working on being able to create snapshots of a view. This will work much the same as a revision would on a content entity.
The sandbox is at http://drupal.org/sandbox/tim.plunkett/1799554
Problem/Motivation
Editing a view is a rather complex thing due to many possible options. For example often people don't really know the performance consequences of their changes. To better document what changed on a view, but also really important have a way to revert
changes to previous versions snapshots of views are introduced.
Proposed resolution
Create a new configuration entity type called view_snapshot which extends a normal view by adding the create date,
a note/message for the change and an snapshot it which will could look like $view_name:$number. Prefixing the view-name
allows us to easily list snapshots by view. This will be used a new ViewSnapshotStorageController to create new snapshots.
POSTPONED: Additional we introduce new form controllers for all the different options on a snapshot (delete/revert).
Remaining tasks
- code review
- POSTPONED: Get a good UX experience based on a review
- Maybe more?
User interface changes
POSTPONED: A new "view snapshots" link in the view edit UI
Comment | File | Size | Author |
---|---|---|---|
#11 | vdc-1834828-11.patch | 1.87 KB | tim.plunkett |
#7 | drupal-1834828-7.patch | 12.04 KB | dawehner |
#1 | drupal-1834828-1.patch | 30.58 KB | dawehner |
Comments
Comment #1
dawehnerLet's see how much is broken at the moment.
Comment #3
xjmSee also: #1515312: Add snapshots of last loaded config for tracking whether config has changed. Maybe we should focus our efforts over there?
Notice that the current direction there is to store snapshots in the DB.
Comment #4
dawehnerWell ... from my standpoint it's a totally different kind of snapshot :)
Comment #5
damiankloip CreditAttribution: damiankloip commentedYeah, I think we talked about this before (maybe in SF), this is definitely not the same thing. That issue is about automatically creating a snapshot before each save operation, but only ever having one. This is more user driven I think.
Comment #6
dawehnerOne general question: Could we bring just the controller/entity type in, and fix the UI later?
Comment #7
dawehnerLet's make this issue small and just get the API working.
Comment #7.0
dawehnercreate a real summary
Comment #8
gddI was talking to xjm and dawehner about this earlier and I guess I don't really get why this is useful. It is essentially providing one level of undo/comparison, which seems very limited. Either you want a big pile of levels or don't bother. xjm suggested this might be a problem better solved by contrib and I think I agree.
As for #3 it is made to copy the entire contents of one storage to another, but it could probably be modified to take an optional prefix which would only combine the matching config from one to the other. Then you would need someplace to store it but that isn't really an issue.
Comment #9
damiankloip CreditAttribution: damiankloip commentedI'm not sure what you mean by one level? You mean just one snapshot? If so, this is more like revisions and not the related issue to do with creating a snapshot just of the previous config.
Comment #10
gddClarified this with timplunkett, who informed me this is actually more like a real revisioning system with multiple snapshots.
However I'm still against this, albeit for different reasons. Second, I've always believe we shouldn't be building revision control into core for config. That's what a VCS is for. If people really want this in other storage, then it should be a contrib. Second, if we did actually do this, we should do it core-wide, not just for one subsystem.
Comment #11
tim.plunkettOne possibility is just to allow the config prefix to be altered/overridden by a storage controller, which is the only part of this patch that isn't new code. Which should have its own explicit (unit) test coverage anyway.
Comment #12
aspilicious CreditAttribution: aspilicious commentedHmm yeah the original idea is "nice" but feels very contrib to me.
Comment #13
damiankloip CreditAttribution: damiankloip commentedHmm, ok, this is a fair point, I also think we will run out of time to try and get this in anyway :)
I remember we talked about creating a getConfigPrefix method a few times before anyway, so this definitely makes sense to get in regardless. I don't think this issue is the place to do this however. We should keep this for the snapshots, even if we move this later (contrib).
@dawehner, what do you think? contrib project? If so, we can just move the work we have been doing in Tim's sandbox to a new project/sandbox.
I have created a new issue to add the getConfigPrefix() method to the controller, I meant to do this ages ago, I think me and @tim.plunkett talked about this (If I remember right).
New issue is: #1849792: Abstract usage of 'config_prefix' on ConfigStorageController into getConfigPrefix method
Comment #14
dawehnerSee http://drupal.org/sandbox/dereine/1862386
Comment #14.0
dawehnerupdate