added VersionSubscriber and necessary node types and behaviors #3042

danrot · 2016-11-21T15:49:32Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Fixed tickets	fixes parts of #2067, fixes #3062
Related issues/PRs	none
License	MIT
Documentation PR	none

What's in this PR?

This PR adds versioning support for pages.

TODO

danrot · 2017-01-09T09:03:06Z

@chirimoya I am working on introducing the flag, what should be the default? I would go for false, since it is not working with jackalope-doctrine-dbal.

danrot · 2017-01-09T09:49:19Z

src/Sulu/Bundle/ContentBundle/Controller/NodeController.php

@@ -776,6 +776,17 @@ public function postTriggerAction($uuid, Request $request)
                    $this->getDocumentManager()->removeDraft($data, $language);
                    $this->getDocumentManager()->flush();
                    break;
+                case 'restore':


Should we somehow disable this action if versioning is disabled? I'd say we should, the question is how. Put all of the code in this case in a if, and let the code fall through, or throw the exception in this branch? As Unrecognized action or something like versioning is disabled?

@chirimoya When thinking about it... Should we move that functionality to the VersionController, and deactivate the version controller as a whole?

danrot · 2017-01-09T09:55:22Z

src/Sulu/Bundle/ContentBundle/Resources/config/routing_api.yml

@@ -11,6 +11,11 @@ sulu_content.node.seo:
    parent: sulu_content.node
    resource: Sulu\Bundle\ContentBundle\Controller\SeoController

+sulu_content.node.versions:


Should probably not be registered if versioning is disabled.

Should be possible with custom route loaders.

danrot · 2017-01-09T09:57:49Z

src/Sulu/Bundle/ContentBundle/Resources/public/js/components/content/settings/main.js

@@ -259,6 +259,39 @@ define([
                    }
                ]);

+                this.sandbox.start([


Don't show if versioning is disabled.

danrot · 2017-01-09T09:58:16Z

src/Sulu/Bundle/ContentBundle/Resources/views/Template/settings.html.twig

                    <span class="name">...</span> (<span class="date">...</span>)
                </div>
            </div>
+        </div>
+
+        <div id="versions">


Don't output if versioning is disabled.

danrot · 2017-01-09T09:59:21Z

src/Sulu/Bundle/DocumentManagerBundle/Resources/config/behaviors.xml


+        <service id="sulu_document_manager.suscriber.behavior.version"


Don't register when versioning is disabled.

danrot · 2017-01-09T10:01:23Z

src/Sulu/Component/PHPCR/NodeTypes/Content/ContentNodeType.php

@@ -49,6 +49,7 @@ public function getDeclaredSupertypeNames()
    {
        return [
            'sulu:base',
+            'mix:versionable',


Don't include mixin directly in NodeType, otherwise it won't be usable in PHPCR implementations without versioning.

danrot · 2017-01-10T11:04:02Z

src/Sulu/Bundle/ContentBundle/Controller/VersionController.php

+     *
+     * @return \Symfony\Component\HttpFoundation\Response
+     */
+    public function postAction(Request $request, $uuid, $version)


The cool thing here is that we don't need the @Post annotation as we have in the NodeController the $version parameter will also be added to the route from the method signature.

The bad thing is that the security is not working correctly then, since it is recognizing the POST as create, which means that every who is allowed to add a page can also restore it...

That's probably part of a bigger issue, we've got several ways to handle this.

Find a different way to distinguish between add and create in security (would have to think about that, maybe if the last part excluding the query string is number or uuid?)

Handle security without the fancy helpers Sulu gives us.

Rename that action to postTrigger action as we are used to it, and add a @Post annotation. The drawback is that we have to include the parent route in the annotation: /nodes/{uuid}/versions/{version}

use postAction and validate query parameter action or url last segment is placeholder

danrot · 2017-01-10T11:10:29Z

src/Sulu/Bundle/ContentBundle/Tests/app/AppKernel.php

@@ -18,6 +18,10 @@ public function registerContainerConfiguration(LoaderInterface $loader)
    {
        parent::registerContainerConfiguration($loader);

+        if (getenv('SYMFONY__PHPCR__TRANSPORT') === 'jackrabbit') {


I don't have a better (and fastly implementable) idea how to solve this. It's about enabling versioning only for jackrabbit... A solution might be to add another environment variable. Would allow us to also test jackrabbit without versioning. Then we would have another build more, and the tests are taking longer, ... I think I would leave it like that.

danrot · 2017-01-10T11:12:32Z

src/Sulu/Bundle/DocumentManagerBundle/Document/Subscriber/SecuritySubscriber.php

+/**
+ * Responsible for integrating the security part of Sulu to the DocumentManager.
+ */
+class SecuritySubscriber implements EventSubscriberInterface


This subscriber makes detecting the user more reusable. It's always set as an option, so that it can be reused in all the different events.

chirimoya · 2017-01-25T17:00:33Z

@danrot ready to merge?

danrot · 2017-01-26T07:08:07Z

@chirimoya yes 😃

danrot force-pushed the feature/versioning branch 6 times, most recently from 5b8ea29 to a691e8f Compare November 29, 2016 07:32

danrot force-pushed the feature/versioning branch 3 times, most recently from dae5da1 to dc46dc6 Compare December 12, 2016 13:17

danrot force-pushed the feature/versioning branch 4 times, most recently from 9ae0de6 to e00d978 Compare December 13, 2016 09:32

chirimoya self-requested a review December 21, 2016 08:00

danrot force-pushed the feature/versioning branch from e00d978 to cb778ed Compare December 22, 2016 09:30

danrot force-pushed the feature/versioning branch from cb778ed to 5ba1738 Compare January 9, 2017 08:16

danrot commented Jan 9, 2017

View reviewed changes

danrot force-pushed the feature/versioning branch 4 times, most recently from ad4e4e2 to 0d4db89 Compare January 10, 2017 10:17

danrot commented Jan 10, 2017

View reviewed changes

danrot force-pushed the feature/versioning branch 2 times, most recently from 08270b4 to 81d7d6c Compare January 18, 2017 14:43

chirimoya approved these changes Jan 18, 2017

View reviewed changes

danrot added 18 commits January 25, 2017 09:43

added VersionSubscriber and necessary node types and behaviors

4e761f5

added subscriber to set page to draft after restoring

3c720d8

added restore functionalit to BlameSubscriber

be4c7d1

extract user default from BlameSubscriber

e19b551

removed creator on settings tab

a7748a7

added method to get multiple users at once

3a20a91

added restore functionality to UI

fd61831

implemented feature flag

e1ba613

added changelog

0b12f30

use non-deprecated slugifier

13d843c

fixed minor code style

4cbb0df

added webspace parameter to VersionController

9987200

update version table after save

ed9ec2a

readded creator if versioning is disabled

6345b4b

updated translation for changelog in settings tab

f2d9f28

fixed tests

ed252f0

remove repository from composer.json

ed5ea9a

added missing test condition

cf210af

danrot force-pushed the feature/versioning branch 2 times, most recently from a13d0e6 to cf210af Compare January 25, 2017 14:15

danrot added this to the Release 1.5 milestone Jan 26, 2017

chirimoya merged commit ac6f6c6 into sulu:develop Jan 31, 2017

danrot deleted the feature/versioning branch January 31, 2017 16:30

danrot mentioned this pull request Feb 6, 2017

added documentation for versioning sulu/sulu-docs#313

Merged

danrot mentioned this pull request Jun 14, 2017

remove exception for non-Sulu user #3404

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

added VersionSubscriber and necessary node types and behaviors #3042

added VersionSubscriber and necessary node types and behaviors #3042

Uh oh!

danrot commented Nov 21, 2016 •

edited

Loading

Uh oh!

danrot commented Jan 9, 2017

Uh oh!

danrot Jan 9, 2017

Uh oh!

danrot Jan 9, 2017

Uh oh!

danrot Jan 9, 2017

Uh oh!

danrot Jan 9, 2017

Uh oh!

danrot Jan 9, 2017

Uh oh!

danrot Jan 9, 2017

Uh oh!

danrot Jan 9, 2017

Uh oh!

danrot Jan 9, 2017

Uh oh!

danrot Jan 10, 2017

Uh oh!

chirimoya Jan 16, 2017

Uh oh!

danrot Jan 10, 2017

Uh oh!

danrot Jan 10, 2017

Uh oh!

chirimoya commented Jan 25, 2017

Uh oh!

danrot commented Jan 26, 2017

Uh oh!

Uh oh!


		<service id="sulu_document_manager.suscriber.behavior.version"

added VersionSubscriber and necessary node types and behaviors #3042

added VersionSubscriber and necessary node types and behaviors #3042

Uh oh!

Conversation

danrot commented Nov 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's in this PR?

TODO

Uh oh!

danrot commented Jan 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chirimoya commented Jan 25, 2017

Uh oh!

danrot commented Jan 26, 2017

Uh oh!

Uh oh!

danrot commented Nov 21, 2016 •

edited

Loading