From #1927608-197: Remove the tight coupling between Block Plugins and Block Entities by @catch:

It's also weird that we specify both the plugin and the module in block config but that predates the patch so meh.

I agree with catch. It seems weird that block plugins require 'module' to be passed as configuration rather than figuring it out some other way.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benjy’s picture

Assigned: tim.plunkett » Unassigned

$plugin_definition contains a "provider" key and a "module" key. Are they ever different? Maybe we could just use the provider key instead?

jibran’s picture

Status: Active » Closed (duplicate)
tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett
Status: Closed (duplicate) » Active

I don't thinking actually fixed it, I'll post a patch showing what I mean.

jibran’s picture

In all fairness the title of the issue is confusing. I realized that after reading the original comment by @catch.

tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett
Status: Active » Postponed

The 'module' needs to be stored due to how BlockPluginBag handles exceptions for disabled modules.
Postponing on #1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed

tim.plunkett’s picture

Issue summary: View changes
Status: Postponed » Needs review
Issue tags: +Plugin system
FileSize
18.36 KB

This is what I meant.

Status: Needs review » Needs work

The last submitted patch, 6: block-1991442-6.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
6.46 KB
24.82 KB

Well yeah, I guess that would be a problem :)

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Perfect!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: block-1991442-8.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
24.37 KB

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: block-provider-1991442-11.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community

Can we turn off the auto retest until the known bot issues are resolved?

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

block-provider-1991442-11.patch no longer applies.

error: patch failed: core/modules/views/tests/Drupal/views/Tests/Plugin/Block/ViewsBlockTest.php:125
error: core/modules/views/tests/Drupal/views/Tests/Plugin/Block/ViewsBlockTest.php: patch does not apply

benjy’s picture

Status: Needs work » Needs review
FileSize
24.64 KB

Re-roll.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

  • Commit 41b6df5 on 8.x by alexpott:
    Issue #1991442 by tim.plunkett, benjy: Remove 'module' from block plugin...
alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs reroll

Committed 41b6df5 and pushed to 8.x. Thanks!

Fixed whitespace errors on commit

diff --git a/core/modules/views/tests/Drupal/views/Tests/Plugin/Block/ViewsBlockTest.php b/core/modules/views/tests/Drupal/views/Tests/Plugin/Block/ViewsBlockTest.php
index 135a5b3..eb46cb3 100644
--- a/core/modules/views/tests/Drupal/views/Tests/Plugin/Block/ViewsBlockTest.php
+++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/Block/ViewsBlockTest.php
@@ -128,7 +128,7 @@ public function testBuild() {
 
     $definition['provider'] = 'views';
     $plugin = new ViewsBlock($config, $block_id, $definition, $this->executableFactory, $this->storage, $this->account);
-    
+
     $this->assertEquals($build, $plugin->build());
   }
 
@@ -150,7 +150,7 @@ public function testBuildFailed() {
 
     $definition['provider'] = 'views';
     $plugin = new ViewsBlock($config, $block_id, $definition, $this->executableFactory, $this->storage, $this->account);
-    
+
     $this->assertEquals(array(), $plugin->build());
   }
tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned

No idea how that got in there, thanks.

Status: Fixed » Closed (fixed)

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