Skip to content
Snippets Groups Projects

3336994 StringFormatter access

3 unresolved threads

Co-authored-by: Dezső BICZÓ mxr576@gmail.com

Closes #3336994

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
45 48 $this->installConfig(['filter']);
46 49 $this->installEntitySchema('node');
47 50 $this->installEntitySchema('user');
51 $this->setUpCurrentUser(['uid' => 0]);
  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • 22 23 */
    23 24 class FieldGroupRowsTest extends ViewsKernelTestBase {
    24 25
    26 use UserCreationTrait;
  • Dezső Biczó added 3 commits

    added 3 commits

    • 0390b8ab - Resolve https://git.drupalcode.org/project/drupal/-/merge_requests/8317#note_338223
    • 5d0e6409 - Resolve https://git.drupalcode.org/project/drupal/-/merge_requests/8317#note_347254
    • 31602914 - Removed unnecessary use of UserCreationTrait

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • 139 ? [
    137 140 '#type' => 'link',
    138 141 '#title' => $view_value,
    139 142 '#url' => $url,
    140 ];
    141 }
    142 else {
    143 $elements[$delta] = $view_value;
    144 }
    143 ]
    144 : $view_value;
    145 145 }
    146
    147 (new CacheableMetadata())
    148 ->addCacheableDependency($access)
    149 ->applyTo($elements);
    • Comment on lines 125 to +149

      I find it easier to be more consistent to avoid creating a meaningless AccessResult::forbidden() object and calling isAllowed() in a loop.

      Suggested change
      127 $url = NULL;
      128 $entity = $items->getEntity();
      129 $entity_type = $entity->getEntityType();
      130
      131 if ($this->getSetting('link_to_entity') && !$entity->isNew() && $entity_type->hasLinkTemplate('canonical')) {
      132 $url = $this->getEntityUrl($entity);
      133 }
      134
      135 $access = $url?->access(return_as_object: TRUE) ?? AccessResultForbidden::forbidden();
      136 foreach ($items as $delta => $item) {
      137 $view_value = $this->viewValue($item);
      138 $elements[$delta] = $access->isAllowed()
      139 ? [
      140 '#type' => 'link',
      141 '#title' => $view_value,
      142 '#url' => $url,
      143 ]
      144 : $view_value;
      145 }
      146
      147 (new CacheableMetadata())
      148 ->addCacheableDependency($access)
      149 ->applyTo($elements);
      127 $entity = $items->getEntity();
      128 $entity_type = $entity->getEntityType();
      129
      130 $cacheability = new CacheableMetadata();
      131 if ($this->getSetting('link_to_entity') && !$entity->isNew() && $entity_type->hasLinkTemplate('canonical')) {
      132 $url = $this->getEntityUrl($entity);
      133 $access = $url->access(return_as_object: TRUE);
      134 $cacheability->addCacheableDependency($access);
      135 if (!$access->isAllowed()) {
      136 $url = NULL;
      137 }
      138 }
      139 else {
      140 $url = NULL;
      141 }
      142
      143 foreach ($items as $delta => $item) {
      144 $view_value = $this->viewValue($item);
      145 $elements[$delta] = $url
      146 ? [
      147 '#type' => 'link',
      148 '#title' => $view_value,
      149 '#url' => $url,
      150 ]
      151 : $view_value;
      152 }
      153
      154 $cacheability->applyTo($elements);
    • Dezső Biczó changed this line in version 8 of the diff

      changed this line in version 8 of the diff

    • @kksandr thx for your suggestion regarding code readability, you made a valid point on that. Please check my counter offer for improving that.

      What I did not like in your solution is unsetting $url and making decisions based on that.

    • Please register or sign in to reply
  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • Please register or sign in to reply
    Loading