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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
30.58 KB

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.

xjm’s picture

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.

dawehner’s picture

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

damiankloip’s picture

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.

dawehner’s picture

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

dawehner’s picture

Status: Needs work » Needs review
FileSize
12.04 KB

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

dawehner’s picture

Issue summary: View changes

create a real summary

gdd’s picture

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.

damiankloip’s picture

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.

gdd’s picture

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.

tim.plunkett’s picture

Issue tags: +Needs tests
FileSize
1.87 KB

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.

aspilicious’s picture

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

damiankloip’s picture

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

dawehner’s picture

Status: Needs review » Closed (won't fix)
dawehner’s picture

Issue summary: View changes

update