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-30344: Allowed limiting Content management to specific translations #2585

Merged

Conversation

@alongosz
Copy link
Member

commented Mar 25, 2019

Question Answer
JIRA issue EZP-30344
Required by ezsystems/ezplatform-admin-ui#934
Bug/Improvement yes
New feature yes
Target version 7.5.1 for eZ Platform 2.5.1 LTS
BC breaks yes, minor (see JIRA issue)
Tests pass yes
Doc needed yes

Summary

This PR:

  • Improves Language Limitation by limiting user's ability to create, edit and publish content in a given language to a list of translations set by that Limitation.
  • e6e5aa0 implements PermissionResolver::lookupLimitations which provides more standardized and API-oriented way to fetch information about existing Limitations for the given object in the context of the given policy (by @mikadamczyk).

Squashed into one PR due to time constraints.

Implementation details

Many Limitations throw exception when dealing with unknown objects or targets, which makes it impossible to set some specific mixes of Limitations on a given policy. This had hit us in the past and now makes current changes impossible to implement.

To solve this issue we're introducing two new interfaces:

  • \eZ\Publish\SPI\Limitation\Target which is a marker for a new Target object which is designed in a new way and provides more information about an intent of a canUser check.
  • \eZ\Publish\SPI\Limitation\TargetAwareType for LimitationType (service which implements Limitation evaluation logic) which does not throw exception when dealing with an unknown object. Such LimitationType should at the end return ACCESS_ABSTAIN. TargetAwareType LimitationType expects $targets to be an array of instances of Target, however if different object is passed, it won't throw exception (behaves as described).

We've created the first class which objects follows the described behavior:

  • \eZ\Publish\SPI\Limitation\Target\Version

and a builder for DX and readability

  • \eZ\Publish\SPI\Limitation\Target\Builder\VersionBuilder
    which provides behavioral, intent-driven API.

Consider the following updateContent use case:

        $this->repository->getPermissionResolver()->canUser(
            'content',
            'edit',
            $content,
            [
                (new Target\Builder\VersionBuilder())
                    ->updateFieldsTo(
                        $contentUpdateStruct->initialLanguageCode,
                        $contentUpdateStruct->fields
                    )
                    ->build(),
            ]
        )

it checks if a user is able to edit the Content if new Version would contain new initial language and given fields (actually used only to extract language codes of modified translation).

Limitation\Target\Version is passed as one of the targets to the following ContentService methods:

  • createContentDraft // from existing Version
  • updateContent
  • publishVersion // not needed

We could try handling ContentUpdateStruct, but this Target is also designed to provide Limitation lookup for which creating artificial update struct is an overkill.

To provide BC for existing Limitations other than Language and avoid known issues when mixing Limitations for a single policy, the logic of PermissionResolver::canUser is changed as follows:

  • for LimitationTypes not implementing Limitation\TargetAwareType only targets which are not instances of Limitation\Target are passed (array is filtered prior passing). If an array is empty, null is returned as some Limitations rely on that.
  • for Target-aware Limitation types (implementing Limitation\TargetAwareType) only targets which are instances of Limitation\Target are passed.

TODO:

  • Cover COTF requirements mix (Translation, Content type, Parent Location) with integration tests.
  • Implement Languages diff to determine if publishing should be allowed.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@mikadamczyk mikadamczyk referenced this pull request Mar 27, 2019

Merged

EZP-30344: Improved handling Language Limitation #934

2 of 2 tasks complete

@alongosz alongosz force-pushed the alongosz:ezp-30344-language-limited-content-mgm branch 4 times, most recently from e15470c to b1d2838 Mar 27, 2019

@alongosz alongosz changed the title EZP-30344: Implemented Translation Limitation EZP-30344: Improved Language Limitation Apr 4, 2019

@alongosz alongosz changed the title EZP-30344: Improved Language Limitation EZP-30344: Allowed limiting Content management to specific translations Apr 4, 2019

@alongosz alongosz force-pushed the alongosz:ezp-30344-language-limited-content-mgm branch from b1d2838 to 01a240a Apr 4, 2019

@alongosz

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

@mikadamczyk refactored to use VersionBuilder according to @adamwojs request. For changed usages please see 6217d15 (hint: see extra build method there).

@alongosz alongosz force-pushed the alongosz:ezp-30344-language-limited-content-mgm branch from 9e61496 to 15cd9b6 Apr 9, 2019

@alongosz

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

Fixed code review issues and added more flexibility to Target\Version via a9b7c2d for better permission lookup.

@micszo micszo self-assigned this Apr 11, 2019

alongosz and others added some commits Mar 19, 2019

Improved LanguageLimitationType implementation
Improved implementation takes care about a target
which describes future intended changes to a Content item
Improved ContentService permission checks by passing instances of Target
Instance of Target provides to TargetAwareLimitationType better target
which describes the actual intent which cannot be inferred from
a content item itself

changed canUser invocation for:
* createContentDraft
* updateContent
* publishVersion

@alongosz alongosz force-pushed the alongosz:ezp-30344-language-limited-content-mgm branch from c67e9d3 to f0c45ea Apr 12, 2019

@micszo micszo added QA approved and removed Ready for QA labels Apr 17, 2019

@micszo micszo removed their assignment Apr 17, 2019

@alongosz alongosz merged commit 0f847fd into ezsystems:master Apr 18, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alongosz alongosz deleted the alongosz:ezp-30344-language-limited-content-mgm branch Apr 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.