Contrib.social

AccessResult::orIf() has meaningless checks

Open on Drupal.org →
Created on 31 May 2024, over 1 year ago
Updated 17 June 2024, about 1 year ago

Problem/Motivation

From the codebase:

    if ($this->isForbidden() || $other->isForbidden()) {
      // Removed for brevity, either or both are forbidden
    }
    elseif ($this->isAllowed() || $other->isAllowed()) {
      // Removed for brevity, either or both are allowed
    }
    else {
      // Removed for brevity, both have to be neutral at this point
      // as we already checked for allowed and forbidden.
    }

Yet in that final branch, we have the following check:

      if (!$this->isNeutral() || ($this->getCacheMaxAge() === 0 && $other->isNeutral()) || ($this->getCacheMaxAge() !== 0 && $other instanceof CacheableDependencyInterface && $other->getCacheMaxAge() !== 0)) {
        $merge_other = TRUE;
      }

This can be simplified to:

      if ($this->getCacheMaxAge() === 0  || ($other instanceof CacheableDependencyInterface && $other->getCacheMaxAge() !== 0)) {
        $merge_other = TRUE;
      }

Steps to reproduce

See code, read code, see bug.

Proposed resolution

See above.

Remaining tasks

Create MR with changes above.

User interface changes

/

API changes

/

Data model changes

/

Release notes snippet

/

🐛 Bug report
Status

Fixed

Version

10.4 ✨

Component
Base  →

Last updated 7 days ago

Created by

🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

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

Merge Requests

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024