teppokoivula / VersionControl

Version Control Module For ProcessWire CMS/CMF
GNU General Public License v2.0
26 stars 10 forks source link

Restoring revisions #14

Closed happy-kaseem closed 7 years ago

happy-kaseem commented 7 years ago

I am wondering if I misunderstand the concept of this module or if this is an actual bug.

Let's say, I have a template with two fields A and B which I have added to VersionControl. Then I apply the following changes:

  1. Change field A to the value A2
  2. Change field B to the value B2

This results in the following revision history:

Revision  Field A Field B
3 B2
2 A2
1 A1 B1

If I now restore (or preview) Revision 2, I would assume that I get the following values A = A2 and B = B1.

But what I get is A = A2 and B = B2. Which means no change at all and VersionControl ignores the request to restore to Revision 2.

As far as I understand from the code, this happens, because VersionControl in order to restore a revision, clones the current page and applies the requested revision (2 in this case). The result is the above A = A2 and B = B2. And not what I would expect A = A2 and B = B1.

All would work fine if I had changed both fields at every step. But this is generally not the case.

With the current setup, I can only see one solution to the problem on how to restore a revision:

  1. clone the page
  2. apply all the revisions from revision 1 to the requested revision, in this way we would end up with A = A2 and B = B1.
teppokoivula commented 7 years ago

Thanks for reporting this, it does sound like a bug to me. The original implementation of this module was very much focused on "field level version control", and the "restore page to revision x" feature may indeed be "incomplete" in some ways.

I'll take a closer look at this (and create a test case in the VersionControlTests repository) as soon as possible :)

happy-kaseem commented 7 years ago

Thanks @teppokoivula. If this helps, I am trying to reword the SQL statement which creates the list of changes to be applied when restoring a page. Currently this seems to happen here in file PageSnapshot.module.

I hope to be able to submit the code suggestion soon.

teppokoivula commented 7 years ago

@happy-kaseem, thanks for reporting the issue and figuring out a fix – and sorry for taking so long to get back to you! Wanted to write a test case for this first, but then something else came up and I got distracted. Anyway, your PR has now been merged :)