From the original port issue:
- Entity pages should not be provided in the first step, or be configurable on the while bundle level

CommentFileSizeAuthor
#7 add_enable_pages-2858154-7.patch8.44 KBarchnode
#5 add_enable_pages-2858154-5.patch6.86 KBarchnode
#3 add_enable_pages-2858154-3.patch6.14 KBarchnode

Comments

fago created an issue. See original summary.

fago’s picture

fago
Primary language German
Location Vienna
commented
Title: Add entity pages support » Make entity pages support configurable

Noted that this is already there, but we need this to be configurable by bundle.

archnode’s picture

archnode commented
Status: Active » Needs review
StatusFileSize
new add_enable_pages-2858154-3.patch6.14 KB

I added the possibility to enable the entity pages based on a setting in entity type.

In the EntityList Builder i changed the link on the title back to the entity page if it's activated to be consistent with the other listings.

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Needs work

Thanks - here a review:

  1. +++ b/src/Controller/WhileEntityController.php
@@ -201,4 +203,21 @@ class WhileEntityController extends ControllerBase implements ContainerInjection
+   * Undocumented function
+   * ¶
+   * @param [type] $while_entity
+   * @return void

    That needs work. 

  2. +++ b/src/Controller/WhileEntityController.php
@@ -201,4 +203,21 @@ class WhileEntityController extends ControllerBase implements ContainerInjection
+    $entity_type = WhileEntityType::load($while_entity->bundle());

    Needs to use a dependency-injected EntityTypeManager. But easier way to do it would be $while_entity->type->entity 

  3. +++ b/src/Entity/WhileEntityType.php
@@ -27,7 +27,8 @@ use Drupal\Core\Config\Entity\ConfigEntityBundleBase;
+ *     "entity_pages_active" = "entity_pages_active",

    No, that's not an entity key- do not add this here. 

  4. +++ b/src/Entity/WhileEntityType.php
@@ -47,4 +48,18 @@ class WhileEntityType extends ConfigEntityBundleBase implements WhileEntityTypeI
+   * The state of entity page display

    Missing trailing point. 

  5. +++ b/src/Entity/WhileEntityType.php
@@ -47,4 +48,18 @@ class WhileEntityType extends ConfigEntityBundleBase implements WhileEntityTypeI
+   * @var Boolean

    bool 

  6. +++ b/src/Entity/WhileEntityType.php
@@ -47,4 +48,18 @@ class WhileEntityType extends ConfigEntityBundleBase implements WhileEntityTypeI
+  protected $entity_pages_active;

    A default value would make sense to me here - what's the default? 

  7. +++ b/src/Entity/WhileEntityTypeInterface.php
@@ -9,5 +9,11 @@ use Drupal\Core\Config\Entity\ConfigEntityInterface;
+   * Gets the entity pages active state

    Missing trailing point. 

  8. +++ b/src/Entity/WhileEntityTypeInterface.php
@@ -9,5 +9,11 @@ use Drupal\Core\Config\Entity\ConfigEntityInterface;
+   * ¶

    Whitespace. 

  9. +++ b/src/Entity/WhileEntityTypeInterface.php
@@ -9,5 +9,11 @@ use Drupal\Core\Config\Entity\ConfigEntityInterface;
+   * @return Boolean

    bool 

  10. +++ b/src/WhileEntityHtmlRouteProvider.php
@@ -23,6 +24,12 @@ class WhileEntityHtmlRouteProvider extends AdminHtmlRouteProvider {
+    // Remove universal canonical route.

    that seems not what the code does. 

  11. +++ b/src/WhileEntityListBuilder.php
@@ -102,14 +102,20 @@ class WhileEntityListBuilder extends EntityListBuilder {
+    }    ¶

    Whitespace.

archnode’s picture

archnode commented
StatusFileSize
new add_enable_pages-2858154-5.patch6.86 KB

Updated the patch, thanks for the review! I added a conditional redirect for entity form to avoid redirecting the user to an access denied.

fago’s picture

fago
Primary language German
Location Vienna
commented 
  1. +++ b/src/Controller/WhileEntityController.php
@@ -201,4 +203,21 @@ class WhileEntityController extends ControllerBase implements ContainerInjection
+   * ¶

    whitespace here 

  2. +++ b/src/Controller/WhileEntityController.php
@@ -201,4 +203,21 @@ class WhileEntityController extends ControllerBase implements ContainerInjection
+   * @param WhileEntity $while_entity

    Missing description 

  3. +++ b/src/Controller/WhileEntityController.php
@@ -201,4 +203,21 @@ class WhileEntityController extends ControllerBase implements ContainerInjection
+   * @return AccessResult

    Missing NL before @return. AccessResult needs a fully qualified namespace. 

  4. +++ b/src/Entity/WhileEntityTypeInterface.php
@@ -9,5 +9,11 @@ use Drupal\Core\Config\Entity\ConfigEntityInterface;
+   * ¶

    Whitespace (space char) 

  5. +++ b/src/Entity/WhileEntityTypeInterface.php
@@ -9,5 +9,11 @@ use Drupal\Core\Config\Entity\ConfigEntityInterface;
+   * @return bool

    Missing description. 

  6. +++ b/src/Form/WhileEntityTypeForm.php
@@ -47,6 +51,9 @@ class WhileEntityTypeForm extends EntityForm {
+    $while_entity_type->set('entity_pages_active', $form_state->getValue('entity_pages_active'));

    Nope - changing entity content is done in buildEntity(). However, in this case this might even work without any custom code? 

  7. +++ b/src/WhileEntityListBuilder.php
@@ -102,14 +102,20 @@ class WhileEntityListBuilder extends EntityListBuilder {
+    $access_manager = \Drupal::service('access_manager');

    This needs to be injected. 

  8. +++ b/src/WhileEntityListBuilder.php
@@ -102,14 +102,20 @@ class WhileEntityListBuilder extends EntityListBuilder {
+    if ($access_manager->checkNamedRoute('entity.while_entity.canonical', ['while_entity' => $entity->id()], \Drupal::currentUser())) {

    currentUser needs to be injected 

  9. +++ b/src/WhileEntityListBuilder.php
@@ -102,14 +102,20 @@ class WhileEntityListBuilder extends EntityListBuilder {
+    }    ¶

    whitespace

archnode’s picture

archnode commented
StatusFileSize
new add_enable_pages-2858154-7.patch8.44 KB

I adressed the issues in this updated patch.

archnode’s picture

archnode commented
Status: Needs work » Needs review

Forgot to change status!

  • fago committed a8153bd on 8.x-1.x authored by archnode 
    Issue #2858154 by archnode: Make entity pages support configurable
fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Fixed

thx, committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.