Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

EZP-30973: Made siteaccess aware repository to be used as default #2786

Merged
merged 3 commits into from
Dec 3, 2019

Conversation

adamwojs
Copy link
Member

@adamwojs adamwojs commented Oct 1, 2019

Question Answer
JIRA issue EZP-30973
Bug/Improvement no
New feature yes
Target version master
BC breaks yes/no
Tests pass yes
Doc needed yes

TODO:

QA:

  • Make sure that site access aware repository layer is used when for instance ezpublish.api.service.content service is injected
  • Run all possible sanities

@adamwojs adamwojs requested a review from ViniTou November 8, 2019 08:44
@@ -224,7 +224,7 @@ public function updateObjectStateGroup(APIObjectStateGroup $objectStateGroup, Ob
throw $e;
}

return $this->buildDomainObjectStateGroupObject($spiObjectStateGroup);
return $this->buildDomainObjectStateGroupObject($spiObjectStateGroup, $objectStateGroup->prioritizedLanguages);
Copy link
Member

Choose a reason for hiding this comment

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

Please add @property-read string[] $prioritizedLanguages to \eZ\Publish\Core\Repository\Values\ObjectState\ObjectStateGroup

@@ -380,7 +380,7 @@ public function updateObjectState(APIObjectState $objectState, ObjectStateUpdate
throw $e;
}

return $this->buildDomainObjectStateObject($spiObjectState);
return $this->buildDomainObjectStateObject($spiObjectState, null, $objectState->prioritizedLanguages);
Copy link
Member

Choose a reason for hiding this comment

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

Please add @property-read string[] $prioritizedLanguages to \eZ\Publish\Core\Repository\Values\ObjectState\ObjectState

@@ -1,19 +1,19 @@
services:
# API Aliases
ezpublish.api.repository:
alias: eZ\Publish\Core\Event\Repository
alias: ezpublish.siteaccessaware.repository
Copy link
Member

Choose a reason for hiding this comment

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

using named services instead of FQCN is a step back IMHO

@adamwojs
Copy link
Member Author

PR updated according to @alongosz code review suggestions. Additionally:

  • Replaced [] to \eZ\Publish\API\Repository\Values\Content\Language::ALL in tests
  • Added BC note

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Tests with ExampleController for ezpublish.api.service.content service OK.
Regression suite OK.
Sanities OK.

@micszo micszo removed their assignment Nov 29, 2019
@adamwojs adamwojs merged commit 9385bc7 into master Dec 3, 2019
@adamwojs adamwojs deleted the ezp_30973 branch December 3, 2019 08:40
# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging this pull request may close these issues.

5 participants