In my opinion according to the best drupal practices the better way to realize dependency injection to PollRecentBlock plugin.

Comments

myLies created an issue. See original summary.

mylies’s picture

Status: Active » Needs review
StatusFileSize
new203 bytes

And a patch for this)
please, have a look

Status: Needs review » Needs work

The last submitted patch, 2: text_resize-missing-config-error-2937097-2.patch, failed testing. View results

berdir’s picture

Look like the wrong patch?

You don't need to add multiple test runs, only when that really makes sense, like it is something that is different for sqlite/postgresql or different drupal core versions.

mylies’s picture

Status: Needs work » Needs review
StatusFileSize
new2.54 KB

eps, sorry - I miss the project folder
and a correct patch

mylies’s picture

berdir’s picture

Version: 8.x-1.1 » 8.x-1.x-dev
Status: Needs review » Needs work
  1. +++ b/src/Plugin/Block/PollRecentBlock.php
    @@ -44,9 +51,36 @@ class PollRecentBlock extends BlockBase {
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    +    return new static(
    +      $configuration,
    +      $plugin_id,
    +      $plugin_definition,
    +      $container->get('entity_type.manager')
    +    );
    +  }
    

    The create and construct method should be above all other methods and usually __construct() is first.

  2. +++ b/src/Plugin/Block/PollRecentBlock.php
    @@ -44,9 +51,36 @@ class PollRecentBlock extends BlockBase {
    +   * @param array $configuration
    +   * @param string $plugin_id
    +   * @param mixed $plugin_definition
    +   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
    +   */
    

    those need descriptions, start with what the parent has.

mylies’s picture

Status: Needs work » Needs review
StatusFileSize
new3.1 KB

I changed a patch according to #7 comment

nkoporec’s picture

Status: Needs review » Reviewed & tested by the community

Tested the latest patch and it applies cleanly.

berdir’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/src/Plugin/Block/PollRecentBlock.php
@@ -15,7 +18,45 @@ use Drupal\Core\Session\AccountInterface;
+
+  /** @var \Drupal\Core\Entity\EntityTypeManagerInterface  */
+  private $entityTypeManager;
+
+  /**
+   * Overrides \Drupal\Component\Plugin\PluginBase::__construct().
+   *
+   * Overrides the construction of context aware plugins to allow for
+   * unvalidated constructor based injection of contexts.
+   *

Do not use private properties, use protected.

The docblock of the property is not following coding standards.

Also, the description on the construct method is too much. Everyone knows how constructor injection works, just say "Construct a new X object" or something like that.

nkoporec’s picture

Status: Needs work » Needs review
StatusFileSize
new1005 bytes
new4.7 KB

Updated the patch from @myLies and fixed the issues that @Berdir has found.

  • Berdir committed 01a3a57 on 8.x-1.x authored by nkoporec
    Issue #2944695 by myLies, nkoporec: PollRecentBlock dependency injection
    
berdir’s picture

Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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