Skip to content
This repository has been archived by the owner on Sep 23, 2020. It is now read-only.

added VersionSubscriber with creation of versions #97

Merged
merged 20 commits into from
Jan 18, 2017
Merged

added VersionSubscriber with creation of versions #97

merged 20 commits into from
Jan 18, 2017

Conversation

danrot
Copy link
Contributor

@danrot danrot commented Nov 21, 2016

This PR adds a new subscriber, which creates a new version everytime a document is published. In addition to that it keeps documents checked out if necessary.

danrot
danrot commented Dec 22, 2016
View reviewed changes
composer.json
@@ -14,7 +14,8 @@
"jackalope/jackalope-doctrine-dbal": "~1.2.3",
"phpunit/phpunit": "^4.5",
"monolog/monolog": "^1.1",
"phpbench/phpbench": "0.10.*"
"phpbench/phpbench": "0.10.*",
"symfony/phpunit-bridge": "~2.7"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wachterjohannes What version should this dependency be set to now? I need it because of the ClockMock class.

@danrot
Copy link
Contributor Author

danrot commented Jan 10, 2017

Tests are failing because I had to reintroduce the phpunit-bridge from Symfony for the ClockMock class, and some dependencies are containing deprecations again. Should we add the deprecation helper weak option` again?

@danrot
Copy link
Contributor Author

danrot commented Jan 10, 2017

Nevermind, fixed it 🙈 Had just to use the correct slugifier class. However, I am still in favour of not using the Symfony CMF slugifier at all, since it doesn't add any value. If we would just use the Ferrandini/Urlizer wrapped with our custom interface we would keep control over these things... But that's probably another discussion and out of scope of this PR anyway.

@danrot danrot requested a review from chirimoya January 10, 2017 13:47
chirimoya
chirimoya reviewed Jan 16, 2017
View reviewed changes
lib/DocumentManager.php
@@ -137,6 +139,17 @@ public function removeDraft($document, $locale)
/**
* {@inheritdoc}
*/
public function restore($document, $locale, $version, array $options = [])
{
$options = $this->getOptionsResolver(Events::FIND)->resolve($options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Events::RESTORE

lib/Subscriber/Behavior/VersionSubscriber.php
'locale' => $versionInformation['locale'],
'version' => $version->getName(),
'author' => $versionInformation['author'],
'authored' => date('c', time()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use only date('c')

lib/Subscriber/Behavior/VersionSubscriber.php
$contentPropertyPrefix = $this->propertyEncoder->localizedContentName('', $event->getLocale());
$systemPropertyPrefix = $this->propertyEncoder->localizedSystemName('', $event->getLocale());

$node = $event->getNode();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment

lib/Subscriber/Behavior/VersionSubscriber.php
$version = $this->versionManager->getVersionHistory($node->getPath())->getVersion($event->getVersion());
$frozenNode = $version->getFrozenNode();

foreach ($frozenNode->getPropertiesValues() as $name => $value) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment

@chirimoya
Copy link
Member

created and changed date is missing in the live session

@chirimoya chirimoya merged commit a622b1b into sulu:develop Jan 18, 2017
@danrot danrot deleted the feature/versioning branch January 18, 2017 15:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chirimoya chirimoya chirimoya left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danrot @chirimoya