Aggregator does not depend on Block module, it merely provides blocks as one way to list/view aggregated feed items.

This is a stop-gap fix for #1373142: Use the Testing profile. Speed up testbot by 50%. There is actually more in the Aggregator module settings as well as category and feed configuration forms pertaining to blocks (which possibly makes little sense without Block module being enabled - unless you argue that those block settings could be re-used by other modules than Block module), but fixing that is for another, less critical issue.

CommentFileSizeAuthor
drupal8.aggregator-block.0.patch1.4 KBsun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Issue tags: +Needs backport to D7
xjm’s picture

Hmm, the module_exists() is kinda icky. I guess as a stopgap it could be okay, but it seems to me that it would be better to refactor the block stuff properly throughout the module in one go.

sun’s picture

Any refactoring cannot be backported; but this stop-gap fix should be.

catch’s picture

I opened #1378354: Properly remove dependency of aggregator module on block module. I'm also OK with not adding explicit tests for this, since those are getting added in the testing profile issue. Looks RTBC to me.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

yes, this is a simple, clearly justified workaround with follow ups to fix it properly.

Dries’s picture

I'm OK with committing this stopgap solution. I'd recommend that we add a @todo though. Yes or no? :)

xjm’s picture

Well, if we have the followup issue that catch posted, do we still need a @todo as well?

sun’s picture

No. :) We discussed alternative options, but all of them basically run into the same trap:

The @todo would have to be "Completely rewrite this module to bring it on par with our current APIs and standards."

#1378354: Properly remove dependency of aggregator module on block module is sufficient, and after staring at the insanity of code for a couple of minutes, I'd even say we should start to think about a hard deadline for people to work on such major issues. Otherwise, remove it from core, as we'd risk to release D8 with an Aggregator module that has a technical maturity of Drupal 4.7.0.

But well, in the end, we need to have this discussion in #1136482: [policy] Deprecate aggregator.module in D9 core and remove it in D10, not here.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x and 8.x. Thanks!

andypost’s picture

xjm’s picture

Issue tags: +Needs backport to D7

I believe the backport tag stays on:
http://drupal.org/node/1207020

xjm’s picture

Version: 8.x-dev » 7.x-dev
David_Rothstein’s picture

I think the bigger question here is why is aggregator module trying to reach into the {block} table and delete stuff in the first place. Isn't that something Block module should take care of on its own?

Issue: #1227966: Dynamically-defined blocks are never removed from the site, even when they disappear from hook_block_info()
See also: #1273544: Media Gallery Block remains after deleting node

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