Jump to content

Version Control


teppo

Recommended Posts

@SteveB: I might've figured this out.

The problem is related to the way Spex sets up itself. It's a singular module and keeps track of the active page via a property called activePage, the value of which is defined the first time Spex is initiated. In this cases the first loaded page is the admin page for VersionControl. Spex sets that as the active page, and since it isn't meant for admin usage, this prevents some of the necessary methods from being triggered properly.

The quick and dirty fix is adding the following to your /site/templates/_init.php file, before any calls to Spex are made:

if (!$spex) {
    $spex = $modules->get('Spex');
    $spex->setActivePage($page);
}

This is based on some quick testing on a module I'm not familiar with, so please take your time testing this properly, but so far it does seem to work for me. Another approach would be detecting Spex in the ProcessVersionControl::executePreview method, but to be honest I don't like the idea of adding module-specific workarounds there. From my point of view this is not a problem of VersionControl, but rather related to the way Spex works.

Link to comment
Share on other sites

10 hours ago, teppo said:

@SteveB


if (!$spex) {
    $spex = $modules->get('Spex');
    $spex->setActivePage($page);
}

 From my point of view this is not a problem of VersionControl, but rather related to the way Spex works.

I agree it's not a VersionControl problem exactly but I'd like to find a way to use both of these useful modules.

Putting that code in Iinit.php made no difference. Putting it in the template file seemed a step in the right direction but then I found that apparently _init.php wasn't loaded at all. At least, calls to functions in a file I include from there failed (does not exist). Including _init.php from the template file gets us a bit closer but Spex does not appear to be fully working in this mashup and the layout is very incomplete.

The field based way of looking at changes (clock icon) is not working well with PageListSelectMultiple and I use a lot of those.

Thinking about a plan B.

Thanks

Link to comment
Share on other sites

Thanks for the update, @SteveB. Like I said, I've never really used Spex, but this change made the bundled example site work as expected. I'm curious about why _init.php wouldn't be loaded at all in your case and why it got loaded in my tests, but then again: it's not a huge surprise that a real-world use case doesn't behave exactly as a minimal example setup :)

If you could provide me with a stripped down setup that doesn't work as expected, I could test this further, but until then I'm probably not going to be of much help here.

Link to comment
Share on other sites

13 hours ago, SteveB said:

Putting that code in Iinit.php made no difference. Putting it in the template file seemed a step in the right direction but then I found that apparently _init.php wasn't loaded at all.

Taking another look at this after a cup of coffee. While I still have no idea what's going on in your site, here's another workaround you could try, though you'll first have to grab the latest version of ProcessVersionControl.module from GitHub. Then add the following to the beginning of the /site/templates/admin.php file:

$this->addHookBefore('ProcessVersionControl::executePreview', function(HookEvent $event) {
    wire('modules')->get('Spex')->setActivePage(null);
});

This resets the active page for Spex right before ProcessVersionControl renders it, which in turn should make Spex behave as expected in this situation. If this works, it's probably the cleanest you can get without modifying either Spex or ProcessVersionControl manually (neither of which I'd recommend doing).

Hope this helps.

  • Like 1
Link to comment
Share on other sites

Yes, that did it. The Spex layout is working, all the JS & CSS loads, etc. Must have been good coffee.

I ended up adding a line to set a variable so I can know in a template file that a preview is being rendered.

$this->addHookBefore('ProcessVersionControl::executePreview', function(HookEvent $event) {
    wire('modules')->get('Spex')->setActivePage(null);
    wire('modules')->get('Spex')->addTemplateVar('vcpvw', 1);
});

For this particular project where many of the pages being edited have lots of fields (24), I'm not entirely sure a visual preview is best but I've only just gotten it working so too soon to say.

This next observation may or may not have to do with the fact that I have not turned on Version Control for any of the image fields (I have a different kind of image archive for that) and I'm using PageImageAssistant (a.k.a. Pia). I noticed that in the template where I'd normally show a thumbnail from a page chosen in a page field, The value for $page->myField->image in the preview is not  the full data. Kind of makes sense, this is for a page that is not currently selected but was selected at the time the preview is representing. I get the image basename and I can work with that.

As I'm testing now I had a weird thing happen where I changed a page field (multiple values, checkboxes) and the change showed up in previews for earlier revisions. In fairness, this is a test page in my local dev server. I'll make some brand new content and try again. (a misunderstanding)

I made a minor change (@619) so that in the editor clock icon only shows up on fields with more than one version. Otherwise, there's nothing to look at.

    protected function renderHTML(array $data) {

        $markup = "";
        foreach ($data as $field => $field_data) {
            $diff = " diff";
            if ($this->diff_disabled) $diff = "";
            if ($diff && wire('fields')->get($field)->type instanceof FieldtypeFile) $diff = "";
            $revision = count($field_data) ? " data-revision='{$field_data[0]['revision']}'" : "";
            $markup .= "<div class='field-revisions$diff' data-field='$field'$revision>";
            //if (count($field_data)) {
            if (count($field_data) > 1) {
                $markup .= "<ul class='ui-widget-content'>";

Thanks for taking an interest and solving my Spex problem.

Link to comment
Share on other sites

Thought I'd mention this here too. Both VersionControl.module and ProcessVersionControl.module have been updated to version 1.1.0:

  • VersionControl 1.1.0 includes new public method getHistory($page) for fetching the full history of a specific page as an array (supports pagination and filters, arguments can be checked from the code). The primary goal here was to separate the core logic of the module from display stuff.
  • ProcessVersionControl 1.1.0 greatly improves the diff output for Page fields. Instead of IDs and string diff it will now use actual labels and custom diff logic. This not only looks better, but also makes more sense than comparing a list of numbers and displaying the smallest possible change set between them :)

page-diff.png

The Page field update was thanks to this pull request, though it ended up being a bit more complicated than that: https://github.com/teppokoivula/VersionControl/pull/2

  • Like 12
Link to comment
Share on other sites

On 8/10/2016 at 2:34 PM, tpr said:

Looks like a great module, however, I got an error on a multilanguage install " Missing required GET param pages_id ".  Using PW 3.029.

Thanks for reporting this, @tpr. Any suggestions for reproducing this issue would be welcome, though – when and where did this happen, what were you doing then, does it happen consistently or just sometimes, etc.

So far everything seems to be working just fine for me. Installed a fresh multi-language site and using the latest version from GitHub (3.0.29 with some off-version fixes).

Link to comment
Share on other sites

I've tried to disable modules and also tried it on another site which is non-multilanguage and almost a fresh install, and I got the same error. They are PW 3.028 & 29. They are on different servers, both of them running PHP 7.

I installed the module by entering "VersionControl" on the Install module page and Download. After that I enabled one template and one field, that's all. When I load a page in the admin of this template, the error appears.

Link to comment
Share on other sites

8 minutes ago, tpr said:

Update: it appears to be PHP 7. If I downgrade to 5.6.22 the clock icons appear on the fields.

Thanks! I don't have an environment with PHP7 at the moment, so might have to set one up later, but for the time being it would be cool if you could check your browsers dev tools (net panel) for me :)

Once you open the admin page, there should be an AJAX request for an URL like this:

http://www.example.com/processwire/setup/version-control/page?pages_id=1031&settings%5Bempty%5D=true&settings%5Brender%5D=HTML

Are you seeing this there? Is it returning status code "200 OK" or something else?

Link to comment
Share on other sites

@tpr: thanks. With your test environment this was easy to reproduce and I've just pushed a fix to GitHub :)

To be honest I'm not entirely sure why this was broken in the first place, but it has something to do with the way isset() implementation has changed in PHP 7.0.6. Found a bunch GitHub issues related to other frameworks where this has also caused issues. Ditching isset() works in this case, since nothing that evaluates to false should be a valid value here anyway.

@ryan: doing isset($this->input->get->pages_id) in a Process module resulted in "false" in PHP 7, even though $this->input->get->pages_id returns a value ('1234' or something along those lines). Unless I'm mistaken here, this might be something that the core should account for. Seems to be related to https://bugs.php.net/bug.php?id=72117.

  • Like 3
Link to comment
Share on other sites

Thanks! 

Would it be possible to show the version control icon only for fields that are actually enabled? Now they appear on all fields and that seems a bit misleading to me (why's there the icon if it does nothing?).

  • Like 1
Link to comment
Share on other sites

On 8/13/2016 at 9:44 PM, tpr said:

Would it be possible to show the version control icon only for fields that are actually enabled? Now they appear on all fields and that seems a bit misleading to me (why's there the icon if it does nothing?).

This should've already been the case, but in certain situations things weren't working as expected. The icons were displayed if there was at least one row of data in the database for that page and field combination, and a bug in building the base dataset often resulted in *all* fields for enabled templates (and not just the enabled ones) having that one row of data.

I pushed a fix for the config issue a while ago and I've just pushed another fix that skips non-enabled fields even if there is data for them. Still need to make some updates to the automatic data (re)generation part, but for now things should appear normal :)

  • Like 2
Link to comment
Share on other sites

I might found a bug:

On a multilanguage CKEditor field, if I try to restore a revision, always the last language tabs' value is restored (not sure if this is true, I have only 2 languages).

Furthermore, the loading span doesn't go away, I had to set display: none to see the results. Or does it remain there to avoid manual editing? If not, I would use display: not or perhaps pointer-events: none to enable clicking. Now I can't even switch language tabs.

Maybe only setting a lower opacity (with pointer-events: disabled) and using :after pseudo to add the loader animation would do. This could be achieved simply by toggling a class on the element. I can help you with this if you need.

  • Like 1
Link to comment
Share on other sites

5 minutes ago, tpr said:

I might found a bug:

On a multilanguage CKEditor field, if I try to restore a revision, always the last language tabs' value is restored (not sure if this is true, I have only 2 languages).

Furthermore, the loading span doesn't go away, I had to set display: none to see the results. Or does it remain there to avoid manual editing? If not, I would use display: not or perhaps pointer-events: none to enable clicking. Now I can't even switch language tabs.

Maybe only setting a lower opacity (with pointer-events: disabled) and using :after pseudo to add the loader animation would do. This could be achieved simply by toggling a class on the element. I can help you with this if you need.

Thanks. Currently working on some pending fixes, but I'll take a closer look at these soon :)

  • Like 2
Link to comment
Share on other sites

@tpr: finally found time to take a closer look at the language tabs issue. So far there appear to be two issues behind the scenes:

  • VersionControl stores all language values in the form of "data[language-id]", but the field name for the default language is actually just "data". I'm pretty sure this used to work at one point, so perhaps it was a core change somewhere between 2.4 and 2.6 (have to setup a test site with earlier version to make sure).
  • When built-in language tabs are enabled, CKEditor instances are not loaded before a specific tab is opened. This is a bit of a problem, since I can't set a value of a CKEditor instance unless it's already there.

This requires a bit of testing, but I'll try to get these sorted out soon.

  • Like 2
Link to comment
Share on other sites

  • 1 month later...

I dont know why, but when I try to install this module I have an 500 error and the Process Version Control doesnt install.. The two other module are installed but not de Process Version Control.

I run the Processwire 3.0.35. Any idea?

Thank you

 

 

Link to comment
Share on other sites

  • 2 weeks later...

Either I'm missing something or the module is not working properly in version 3.0.33 devns.

I installed it, configured my basic page to be included and sure enough started seeing page edits appear listed on the history tab, even identifying the changed fields correctly. However, preview always shows the current version and revert has no effect.

Link to comment
Share on other sites

  • 3 months later...

Today I made the update for VersionControl from version 1.2.7 to version 1.2.8 using the ProcessWireUpgrade module.

My test site runs PW core version 3.0.52. 

After the update the site was producing the following error which I found in the error log file:

Compile Error: Cannot redeclare class VersionControl (line 18 of /home/chr/ac-test-pw/site/assets/cache/FileCompiler/site/modules/PageSnapshot/VersionControl.module)

I noticed that there are now two folders containing the file "VersionControl.module": "VersionControl" and "PageSnapshot"

After removing the folder "PageSnapshot" with the old version of "VersionControl.module", everything seems to work properly again.

It actually appears that installing the module using "Modules > New > Add module from URL ..." will install it in the folder /site/modules/PageSnapshot where as the "ProcessWire Upgrade" module will create a new folder /site/modules/VersionControl which is the recommended folder for the installation of the module according to the module page http://modules.processwire.com/modules/version-control/

  • Like 1
Link to comment
Share on other sites

  • 3 months later...

Hi @teppo, I recently started using this great module.

Tracy is picking up some error notices when opening a page containing a Repeater in Page Edit.

PHP Notice: Trying to get property of non-object in ...\VersionControl\ProcessVersionControl.module:658

This seems to be due to the field names ($field) containing a "_repeater1234" suffix so wire('fields')->get($field) does not find a match and returns null.

  • Like 1
Link to comment
Share on other sites

  • 5 weeks later...

Hi.

I was wondering if it is possible use some of the version control data on the frontend? I tried to use this code:

$versioncontrol = new VersionControl();
$limit = 25;
$start = 0;
$page->template->enabled_templates = true;
$data = $versioncontrol->getHistory($page, $start, $limit);

However, it will not register the template as enabled, even though the template is enabled for version control.

What I am trying to do is get the dates for a specific field every time it has been modified.

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...