Add support for any operation that requires entity status (published/unpublished)

Created on 4 August 2022, about 2 years ago
Updated 18 September 2023, about 1 year ago

Problem/Motivation

Currently the GroupContentAccessControlHandler for entity access check supports status check (published/unpublished) for the 'view' operation only.

    // We only check unpublished vs published for "view" right now. If we ever
    // start supporting other operations, we need to remove the "view" check.
    $check_published = $operation === 'view'
      && $entity->getEntityType()->entityClassImplements(EntityPublishedInterface::class);

In some cases (e.g. comment entity), the 'approve' operation requires the status check.

Proposed resolution

Add new method to the GroupContentPermissionProvider which should return the operations that require status check (Default is ['view']).

  /**
   * Get operations that require entity status check.
   *
   * @return array
   *   The operations.
   */
  public function getEntityStatusOperations() {
    return ['view'];
  }

Then custom/contrib modules could override the method if is needed to support other/more operations for an entity type.

Amend the check for the operation in GroupContentAccessControlHandler

    $check_published = in_array($operation, $this->permissionProvider->getEntityStatusOperations())
      && $entity->getEntityType()->entityClassImplements(EntityPublishedInterface::class);

Remaining tasks

Feature request
Status

Needs review

Version

3.0

Component

Code

Created by

🇧🇪Belgium msnassar

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇧🇪Belgium keszthelyi Brussels

    Patch from commit 6352b64

  • 🇧🇪Belgium msnassar

    Patch from commit 6352b64 with the tests fixed... It is not possible to test MR!

  • 🇧🇪Belgium msnassar

    As we are in the process of migrating from group 1 to group 2, I would like to add this feature to group 2 & 3. But I would like to hear from the maintainers first?
    I have 2 options:

    • Use the same concept as the patch in #5
    • Use the same concept as the patch in #5 BUT move the method to AccessControlTrait:
        public function getEntityStatusOperations() {
          if (!isset($this->parent)) {
            throw new \LogicException('Using AccessControlTrait without assigning a parent or overwriting the methods.');
          }
          return $this->parent->getEntityStatusOperations();
        }
      

      And then, in the AccessControl:

        public function getEntityStatusOperations() {
          return ['view'];
        }
      

    Any other alternatives are very welcomed...

  • 🇧🇪Belgium msnassar

    Here is a proposed patch for group 2 and 3. It is different from what I proposed in #6.
    I would like to hear from the maintainers whether this solution is acceptable. If yes, then I will add tests.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    9,628 pass
Production build 0.71.5 2024