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

Files: 
CommentFileSizeAuthor
#11 vdc-1834828-11.patch1.87 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 48,771 pass(es).
[ View ]
#7 drupal-1834828-7.patch12.04 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 48,580 pass(es).
[ View ]
#1 drupal-1834828-1.patch30.58 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 48,021 pass(es), 3 fail(s), and 3 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new30.58 KB
FAILED: [[SimpleTest]]: [MySQL] 48,021 pass(es), 3 fail(s), and 3 exception(s).
[ View ]

Let's see how much is broken at the moment.

Status:Needs review» Needs work

The last submitted patch, drupal-1834828-1.patch, failed testing.

See 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.

Well ... from my standpoint it's a totally different kind of snapshot :)

Yeah, 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.

One general question: Could we bring just the controller/entity type in, and fix the UI later?

Status:Needs work» Needs review
StatusFileSize
new12.04 KB
PASSED: [[SimpleTest]]: [MySQL] 48,580 pass(es).
[ View ]

Let's make this issue small and just get the API working.

Issue summary:View changes

create a real summary

I 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.

I'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.

Clarified 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.

Issue tags:+Needs tests
StatusFileSize
new1.87 KB
PASSED: [[SimpleTest]]: [MySQL] 48,771 pass(es).
[ View ]

One 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.

Hmm yeah the original idea is "nice" but feels very contrib to me.

Issue tags:-Needs tests

Hmm, 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

Status:Needs review» Closed (won't fix)

Issue summary:View changes

update