EZP-29734: As an Administrator I want to add Content Type limitation for Section/Assign policy #696

mikadamczyk · 2018-10-19T09:19:12Z

Question	Answer
Tickets	https://jira.ez.no/browse/EZP-29734
Bug fix?	no
New feature?	yes
BC breaks?	no
Tests pass?	yes
Doc needed?	no
License	GPL-2.0

Checklist:

Coding standards ($ composer fix-cs)
Ready for Code Review

dew326 · 2018-10-19T09:43:17Z

src/lib/UniversalDiscovery/Event/Subscriber/SectionAssign.php

+    public function onUdwConfigResolve(ConfigResolveEvent $event): void
+    {
+        $configName = $event->getConfigName();
+        if ('single' !== $configName && 'multiple' !== $configName) {


I'm not sure if we need to check for single - I think we have only multiple.

andrerom · 2018-10-21T10:53:14Z

src/lib/UniversalDiscovery/Event/Subscriber/SectionAssign.php

+        $allowedContentTypesIds = array_unique(array_merge(...$allowedContentTypesIds));
+        foreach ($allowedContentTypesIds as $allowedContentTypeId) {
+            try {
+                $identifier = $this->contentTypeService->loadContentType($allowedContentTypeId)->identifier;


This won't be very efficient, better if you can use ContentTypeService->loadContentTypeList($allowedContentTypesIds), to do that we need a second review of ezsystems/ezpublish-kernel#2444

Besides repeated loads that would avoid the loop and the try block (invalid content type id's will simply not be returned, just like the logic here already does)

src/lib/Util/PermissionUtil.php

+     *
+     * @return array
+     */
+    public function flattenArrayOfLimitations(array $hasAccess): array


andrerom · 2018-10-21T10:55:49Z

src/lib/UniversalDiscovery/Event/Subscriber/SectionAssign.php

+        $this->permissionUtil = $permissionUtil;
+        $this->contentTypeService = $contentTypeService;
+        $hasAccess = $permissionResolver->hasAccess('section', 'assign');
+        $this->allowedContentTypesIdentifiers = is_array($hasAccess) ? $this->checkAllowedContentTypes($hasAccess) : [];


shouldn't this be filled with all content types if $hasAccess is true? otherwise layer above would need to second guess if empty array means false or true, or?

andrerom · 2018-10-21T11:03:16Z

src/bundle/Controller/SectionController.php

+        foreach ($this->permissionUtil->flattenArrayOfLimitations($hasAccess) as $limitation) {
+            if ($limitation instanceof NewSectionLimitation) {
+                // If one of user limitation is NewSectionLimitation we check if user can assign Content to $section
+                return in_array($section->id, $limitation->limitationValues);


As this is iterating several roles and policies, this should not return here unless value is true.

Afaik you can do like in checkAllowedContentTypes() and collect all allowed sections before you do a in_array.

andrerom · 2018-10-21T11:04:19Z

src/bundle/Controller/SectionController.php

@@ -490,9 +507,25 @@ private function getSectionsNumbers(array $sections): array
     * @param \eZ\Publish\API\Repository\Values\Content\Section $section
     *
     * @return bool
+     *
+     * @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
     */
    private function canUserAssignSectionToAnyContent(Section $section): bool


naming, but something like canUserAssignSectionToSomeContent might be more fitting, given a true here is more like a "maybe", while a false is pretty accurate.

mikadamczyk · 2018-10-30T07:38:27Z

@adamwojs can you take a look at it?

barbaragr · 2018-10-30T12:03:30Z

Add policies to the Editor role:

Setup/Administrate
Section/View
User/Login
Content/*
Section/Assign with Content Type: Article limitations

Log in as an Editor
Go to Content/Structure and create an Article
Go to Admin/Sections, click assign button next to Design section
Select created article, click (+) button and click Confirm

Actual result: Error 403 is show: Access denied

barbaragr · 2018-10-30T12:10:08Z

For Admin user after publishing an Article error 500 is thrown:

Go to Content/Content structure
Click Create and select Article
Fill Title and Intro, click Publish

Actual result: Error 500 is thrown: None of the routers in the chain matched url '/admin/content/location/61' but article was published. For editor user (with policies mentioned above) everything works fine.

lserwatka · 2018-10-30T14:30:56Z

@mikadamczyk could you rebase here, as I merged #695

…for Section/Assign policy

mikadamczyk · 2018-10-30T14:38:54Z

@lserwatka done

mikadamczyk added the Ready for review label Oct 19, 2018

mikadamczyk self-assigned this Oct 19, 2018

mikadamczyk requested review from adamwojs, Nattfarinn, webhdx, ViniTou, sunpietro and dew326 October 19, 2018 09:19

sunpietro approved these changes Oct 19, 2018

View reviewed changes

ViniTou approved these changes Oct 19, 2018

View reviewed changes

dew326 approved these changes Oct 19, 2018

View reviewed changes

mikadamczyk force-pushed the EZP-29734 branch from 2346772 to 5c88119 Compare October 19, 2018 13:49

ezsystems deleted a comment from ezrobot Oct 19, 2018

mikadamczyk force-pushed the EZP-29734 branch from 5c88119 to 3b31fe0 Compare October 19, 2018 13:56

ezsystems deleted a comment from ezrobot Oct 19, 2018

andrerom suggested changes Oct 21, 2018

View reviewed changes

andrerom mentioned this pull request Oct 21, 2018

EZP-29223: Implement permissions for "Content/Translate" #695

Merged

2 tasks

mikadamczyk force-pushed the EZP-29734 branch from 3b31fe0 to fb83d01 Compare October 22, 2018 11:36

andrerom approved these changes Oct 30, 2018

View reviewed changes

lserwatka added the Ready for QA label Oct 30, 2018

barbaragr self-assigned this Oct 30, 2018

mikadamczyk force-pushed the EZP-29734 branch 3 times, most recently from a27a265 to 01b629e Compare October 30, 2018 13:34

barbaragr added QA approved and removed Ready for QA labels Oct 30, 2018

barbaragr approved these changes Oct 30, 2018

View reviewed changes

EZP-29734: As an Administrator I want to add Content Type limitation …

682adaf

…for Section/Assign policy

mikadamczyk force-pushed the EZP-29734 branch from 01b629e to 682adaf Compare October 30, 2018 14:38

lserwatka merged commit 74a82b5 into ezsystems:master Oct 30, 2018

ViniTou pushed a commit that referenced this pull request May 12, 2023

[Behat] Increased timeout when searching for subitem headers (#696)

0fa6f1d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-29734: As an Administrator I want to add Content Type limitation for Section/Assign policy #696

EZP-29734: As an Administrator I want to add Content Type limitation for Section/Assign policy #696

mikadamczyk commented Oct 19, 2018

dew326 Oct 19, 2018

andrerom Oct 21, 2018 •

edited

Loading

This comment was marked as resolved.

andrerom Oct 21, 2018

andrerom Oct 21, 2018 •

edited

Loading

andrerom Oct 21, 2018 •

edited

Loading

mikadamczyk commented Oct 30, 2018

barbaragr commented Oct 30, 2018 •

edited

Loading

barbaragr commented Oct 30, 2018 •

edited

Loading

lserwatka commented Oct 30, 2018

mikadamczyk commented Oct 30, 2018

EZP-29734: As an Administrator I want to add Content Type limitation for Section/Assign policy #696

EZP-29734: As an Administrator I want to add Content Type limitation for Section/Assign policy #696

Conversation

mikadamczyk commented Oct 19, 2018

Checklist:

dew326 Oct 19, 2018

Choose a reason for hiding this comment

andrerom Oct 21, 2018 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

andrerom Oct 21, 2018

Choose a reason for hiding this comment

andrerom Oct 21, 2018 • edited Loading

Choose a reason for hiding this comment

andrerom Oct 21, 2018 • edited Loading

Choose a reason for hiding this comment

mikadamczyk commented Oct 30, 2018

barbaragr commented Oct 30, 2018 • edited Loading

barbaragr commented Oct 30, 2018 • edited Loading

lserwatka commented Oct 30, 2018

mikadamczyk commented Oct 30, 2018

andrerom Oct 21, 2018 •

edited

Loading

andrerom Oct 21, 2018 •

edited

Loading

andrerom Oct 21, 2018 •

edited

Loading

barbaragr commented Oct 30, 2018 •

edited

Loading

barbaragr commented Oct 30, 2018 •

edited

Loading