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

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: +Needs backport to D7
xjm’s picture

xjm
she/her
Primary language English
commented

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

sun
Primary language German
Location Karlsruhe
commented

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

catch’s picture

catch
he/him
Primary language English
commented

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

Anonymous (not verified) commented
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

dries commented

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

xjm’s picture

xjm
she/her
Primary language English
commented

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

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

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

dries commented
Status: Reviewed & tested by the community » Fixed

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

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: -Needs backport to D7

Follow-up #1382478: Cleanup {block_role} when deleting blocks

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs backport to D7

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

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.x-dev » 7.x-dev
David_Rothstein’s picture

David_Rothstein commented

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.