UHF-9738: Fix block access hook

helfi_platform_config.module

@@ -22,6 +22,7 @@ use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Url;
 use Drupal\helfi_api_base\Environment\Project;
 use Drupal\helfi_platform_config\DTO\ParagraphTypeCollection;
+use Drupal\helfi_platform_config\EntityVersionMatcher;
 use Drupal\paragraphs\Entity\ParagraphsType;
 use Drupal\user\Entity\Role;
 
@@ -247,44 +248,51 @@ function helfi_platform_config_block_access(Block $block, $operation, AccountInt
   // Handle page title block access based on field_has_hero value.
   if ($operation === 'view' && $block->getPluginId() === 'page_title_block') {
     /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
-    $entity = Drupal::service('helfi_platform_config.entity_version_matcher')
-      ->getType()['entity'];
+    $entity = Drupal::service(EntityVersionMatcher::class)->getType()['entity'];
 
     if (
       !$entity instanceof ContentEntityInterface ||
-      !$entity->hasField('field_hero')
+      !$entity->hasField('field_has_hero')
     ) {
       return AccessResult::neutral();
     }
 
-    // Show title block if the "has hero" checkbox is not checked (=false).
+    // Hide title block if the "has hero" checkbox is checked and
+    // the Hero paragraph is found.
     if (
-      $entity->hasField('field_has_hero') &&
-      $entity->get('field_has_hero')->value
+      $entity->get('field_has_hero')->value &&
+      $entity->get('field_hero')->entity
     ) {
-      return AccessResult::allowed()->addCacheableDependency($block);
+      return AccessResult::forbidden()
+        ->addCacheableDependency($block);
     }
 
-    // Show title block if the Hero paragraph is not found.
-    return AccessResult::forbiddenIf($entity->get('field_hero')->entity)
+    return AccessResult::neutral()
       ->addCacheableDependency($block);
   }
 
   // Handle hero block access based on field_has_hero value.
   if ($operation === 'view' && $block->getPluginId() === 'hero_block') {
     /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
-    $entity = Drupal::service('helfi_platform_config.entity_version_matcher')
-      ->getType()['entity'];
+    $entity = Drupal::service(EntityVersionMatcher::class)->getType()['entity'];
 
     if (
       !$entity instanceof ContentEntityInterface ||
       !$entity->hasField('field_has_hero')
     ) {
-      return AccessResult::neutral();
+      return AccessResult::forbidden();
+    }
+
+    // Hide the hero block if the "has hero" checkbox is checked (=true)
+    // or the field_hero field is missing.
+    if (
+      !$entity->get('field_has_hero')->value ||
+      !$entity->get('field_hero')->entity
+    ) {
+      return AccessResult::forbidden();
     }
 
-    // Hide the hero block if the "has hero" checkbox is checked (=true).
-    return AccessResult::forbiddenIf(!$entity->get('field_has_hero')->value);
+    return AccessResult::neutral();
   }
 
   return AccessResult::neutral();

helfi_platform_config.services.yml

@@ -1,4 +1,8 @@
 services:
+  _defaults:
+    autoconfigure: true
+
+  Drupal\helfi_platform_config\Helper\BlockInstaller: '@helfi_platform_config.helper.block_installer'
   helfi_platform_config.helper.block_installer:
     class: Drupal\helfi_platform_config\Helper\BlockInstaller
     arguments:
@@ -10,8 +14,6 @@ services:
     class: Drupal\helfi_platform_config\Menu\FilterByLanguage
     arguments:
       - '@router.admin_context'
-    tags:
-      - { name: event_subscriber }
 
   helfi_platform_config.translation_writer_command:
     class: Drupal\helfi_platform_config\Commands\TranslationWriterCommand
@@ -22,9 +24,8 @@ services:
   helfi_platform_config.route_subscriber:
     class: Drupal\helfi_platform_config\EventSubscriber\RouteSubscriber
     arguments: ['@current_route_match']
-    tags:
-      - { name: event_subscriber }
 
+  Drupal\helfi_platform_config\EntityVersionMatcher: '@helfi_platform_config.entity_version_matcher'
   helfi_platform_config.entity_version_matcher:
     class: Drupal\helfi_platform_config\EntityVersionMatcher
     arguments: ['@entity_type.manager', '@current_route_match', '@language_manager']
@@ -34,6 +35,7 @@ services:
     tags:
       - { name: twig.extension }
 
+  Drupal\helfi_platform_config\ConfigUpdate\ConfigUpdater: '@helfi_platform_config.config_update_helper'
   helfi_platform_config.config_update_helper:
     class: Drupal\helfi_platform_config\ConfigUpdate\ConfigUpdater
     arguments:
@@ -43,23 +45,20 @@ services:
 
   helfi_platform_config.term_route_subscriber:
     class: Drupal\helfi_platform_config\Routing\TermRouteSubscriber
-    tags:
-      - { name: event_subscriber }
 
   helfi_platform_config.filter_disabled_translations:
     class: Drupal\helfi_platform_config\Menu\FilterDisabledTranslations
     arguments:
       - '@entity_type.manager'
       - '@language_manager'
       - '@router.admin_context'
-    tags:
-      - { name: event_subscriber }
 
   logger.channel.helfi_platform_config:
     parent: logger.channel_base
     arguments:
       - 'helfi_platform_config'
 
+  Drupal\helfi_platform_config\Token\OGImageManager: '@helfi_platform_config.og_image_manager'
   helfi_platform_config.og_image_manager:
     class: Drupal\helfi_platform_config\Token\OGImageManager
     arguments:
@@ -79,5 +78,3 @@ services:
 
   helfi_platform_config.user_create_route_subscriber:
     class: Drupal\helfi_platform_config\Routing\UserCreateRouteSubscriber
-    tags:
-      - { name: event_subscriber }

src/Plugin/Block/HeroBlock.php

@@ -4,18 +4,19 @@
 
 namespace Drupal\helfi_platform_config\Plugin\Block;
 
+use Drupal\Core\Block\Attribute\Block;
 use Drupal\Core\Entity\ContentEntityInterface;
+use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Drupal\helfi_platform_config\EntityVersionMatcher;
 use Drupal\paragraphs\ParagraphInterface;
 
 /**
  * Provides a 'HeroBlock' block.
- *
- * @Block(
- *  id = "hero_block",
- *  admin_label = @Translation("Hero block"),
- * )
  */
+#[Block(
+  id: "hero_block",
+  admin_label: new TranslatableMarkup("Hero block"),
+)]
 class HeroBlock extends ContentBlockBase {
 
   /**

tests/src/Kernel/HeroBlockTest.php

@@ -0,0 +1,184 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\helfi_platform_config\Kernel;
+
+use Drupal\block\Entity\Block;
+use Drupal\Core\Field\FieldItemListInterface;
+use Drupal\helfi_platform_config\EntityVersionMatcher;
+use Drupal\KernelTests\Core\Entity\EntityKernelTestBase;
+use Drupal\node\Entity\Node;
+use Drupal\node\NodeInterface;
+use Prophecy\PhpUnit\ProphecyTrait;
+
+/**
+ * Tests the hero block access.
+ *
+ * @group helfi_platform_config
+ */
+class HeroBlockTest extends EntityKernelTestBase {
+
+  use ProphecyTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'system',
+    'field',
+    'block',
+    'config_rewrite',
+    'helfi_platform_config',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() : void {
+    parent::setUp();
+
+    $this->installEntitySchema('block');
+  }
+
+  /**
+   * Test block access.
+   *
+   * @dataProvider getTestNodes
+   */
+  public function testBlockAccess(array $values, bool $showHero) {
+    $hero = Block::create([
+      'id' => $this->randomMachineName(),
+      'plugin' => 'hero_block',
+      'region' => 'content',
+      'theme' => 'stark',
+      'weight' => 99,
+    ]);
+
+    $title = Block::create([
+      'id' => $this->randomMachineName(),
+      'plugin' => 'page_title_block',
+      'region' => 'content',
+      'theme' => 'stark',
+      'weight' => 100,
+    ]);
+
+    $node = $this->mockNode($values);
+
+    $versionMatcher = $this->prophesize(EntityVersionMatcher::class);
+    $versionMatcher->getType()->willReturn(
+      ['entity' => $node, 'entity_version' => 'canonical'],
+    );
+    $this->container->set('helfi_platform_config.entity_version_matcher', $versionMatcher->reveal());
+
+    $titleAccess = $title->access('view', NULL, TRUE);
+    $heroAccess = $hero->access('view', NULL, TRUE);
+
+    $this->assertEquals($showHero, $titleAccess->isForbidden());
+    $this->assertEquals(!$showHero, $heroAccess->isForbidden());
+  }
+
+  /**
+   * Data provider for tests.
+   *
+   * @return array[]
+   *   The data.
+   */
+  public function getTestNodes() : array {
+    return [
+      [
+        // Has no hero field.
+        [],
+        FALSE,
+      ],
+      [
+        // Hero hidden.
+        [
+          'field_has_hero' => FALSE,
+          'field_hero' => NULL,
+        ],
+        FALSE,
+      ],
+      [
+        // Hero visible but field value is missing.
+        [
+          'field_has_hero' => TRUE,
+          'field_hero' => NULL,
+        ],
+        FALSE,
+      ],
+      [
+        // Hero visible and evaluates to TRUE.
+        [
+          'field_has_hero' => TRUE,
+          'field_hero' => TRUE,
+        ],
+        TRUE,
+      ],
+      [
+        // Hero evaluates to TRUE but is hidden.
+        [
+          'field_has_hero' => FALSE,
+          'field_hero' => TRUE,
+        ],
+        FALSE,
+      ],
+    ];
+  }
+
+  /**
+   * Create mocked node.
+   */
+  private function mockNode(array $values): NodeInterface {
+    $node = $this->getMockBuilder(Node::class)
+      ->disableOriginalConstructor()
+      ->getMock();
+
+    $fields = [];
+
+    foreach ($values as $key => $value) {
+      $field = $this
+        ->getMockBuilder(FieldItemListInterface::class)
+        ->disableOriginalConstructor()
+        ->getMock();
+
+      $field
+        ->expects($this->any())
+        ->method('__get')
+        ->willReturnCallback(function ($property) use ($value) {
+          return match ($property) {
+            'value', 'entity' => $value,
+            default => throw new \LogicException(),
+          };
+        });
+
+      $fields[$key] = $field;
+    }
+
+    $get_callback = function ($property) use ($fields) {
+      if (array_key_exists($property, $fields)) {
+        return $fields[$property];
+      }
+
+      throw new \LogicException();
+    };
+
+    $node
+      ->expects($this->any())
+      ->method('__get')
+      ->willReturnCallback($get_callback);
+
+    $node
+      ->expects($this->any())
+      ->method('get')
+      ->willReturnCallback($get_callback);
+
+    $node
+      ->expects($this->any())
+      ->method('hasField')
+      ->willReturnCallback(fn ($property) => array_key_exists($property, $fields));
+
+    return $node;
+  }
+
+}