Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

EZP-30470: Filtered Content Types and Languages in ContentCreate widget based on user permissions #987

Merged
merged 2 commits into from
May 23, 2019

Conversation

mikadamczyk
Copy link
Contributor

@mikadamczyk mikadamczyk commented May 7, 2019

Question Answer
Tickets https://jira.ez.no/browse/EZP-30470
Requires ezsystems/ezpublish-kernel#2628
Bug fix? yes
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

This PR provides better filtering od Content Types and Languages in the content create widget. It depends on new method \eZ\Publish\SPI\Limitation\Target\Version::createFromAnyContentTypeOf. It also introduces new method in \EzSystems\EzPlatformAdminUi\Permission\PermissionCheckerInterface::getContentCreateLimitations to get Limitations specificaly for Content creation.

Checklist:

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

@mikadamczyk mikadamczyk added the Bug label May 7, 2019
@mikadamczyk mikadamczyk self-assigned this May 7, 2019
@mikadamczyk mikadamczyk marked this pull request as ready for review May 8, 2019 09:35
@mikadamczyk mikadamczyk force-pushed the EZP-30470 branch 3 times, most recently from c80e148 to 9bc4e4e Compare May 10, 2019 13:00
@mikadamczyk mikadamczyk changed the title EZP-30470: Filter Content Types and Languages in ContentCreate widget to those which the user has access [Don't merge] EZP-30470: Filter Content Types and Languages in ContentCreate widget to those which the user has access May 10, 2019
@mikadamczyk mikadamczyk force-pushed the EZP-30470 branch 4 times, most recently from d2c2256 to eeefb91 Compare May 13, 2019 09:43
@mikadamczyk
Copy link
Contributor Author

@mikadamczyk mikadamczyk changed the title [Don't merge] EZP-30470: Filter Content Types and Languages in ContentCreate widget to those which the user has access EZP-30470: Filter Content Types and Languages in ContentCreate widget to those which the user has access May 14, 2019
Copy link
Contributor

@webhdx webhdx left a comment

Choose a reason for hiding this comment

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

I see a huge problem here. In order to get list of restricted content types you need to load all content types. This happens in two places: in ContentCreateType and ContentRightSidebarBuilder. Full list of content types is also loaded by ContentTypeChoiceLoader because it's wrapping another choice loader.

It can cause a huge impact on Persistence cache layer. I'm not sure if this is possible to avoid but it looks like you don't need to wrap ContentTypeChoiceLoader just create new one that will load only needed content types.

*
* @throws \EzSystems\EzPlatformAdminUi\Exception\InvalidArgumentException
*/
public function getGroupedFlattenedLimitationsValues(
Copy link
Contributor

Choose a reason for hiding this comment

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

Name itself suggests that it should be split into smaller methods :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that the name of this method was missed

@mikadamczyk
Copy link
Contributor Author

mikadamczyk commented May 15, 2019

@webhdx I think we have an in-memory cache for Content Types. For the user with a broad set of allowed CT it would be not efficient to load CT, and then fetch ContentTypesGroups for them. Also in this PR, in

ContentCreateContentTypeChoiceLoader

we only filter existing list

$contentTypesGroups = $this->contentTypeChoiceLoader->getChoiceList();

and then

foreach ($contentTypesGroups as $group => $contentTypes) {
  $contentTypesGroups[$group] = array_filter($contentTypes, function (ContentType $contentType) {
    return \in_array($contentType->id, $this->restrictedContentTypesIds);
  });
}

also as @alongosz said #987 (comment) we can do optimization as follow-up

@mikadamczyk mikadamczyk force-pushed the EZP-30470 branch 2 times, most recently from a501ae6 to 2ac38c6 Compare May 16, 2019 08:08
Copy link
Contributor

@m-tyrala m-tyrala left a comment

Choose a reason for hiding this comment

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

QA approve

@alongosz alongosz changed the title EZP-30470: Filter Content Types and Languages in ContentCreate widget to those which the user has access EZP-30470: Filtered Content Types and Languages in ContentCreate widget based on user permissions May 23, 2019
@alongosz alongosz merged commit 7ef70dc into ezsystems:1.5 May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants