Regression: feed block titles are empty when migrated from D7

Created on 29 November 2017, over 6 years ago
Updated 7 September 2023, 10 months ago

Ideally the aggregator's blocks' titles would be inherited from the title of the feed.
This matters less when creating a block manually because the author can just write a title.
However migrated blocks have no title at all.

This would probably do it:
On Drupal\aggregator\Plugin\Block\AggregatorFeedBlock

  public function label() {
    if ($feed = $this->feedStorage->load($this->configuration['feed'])) {
      return $feed->label();
    }
    return parent::label();
  }
πŸ› Bug report
Status

Fixed

Version

2.0

Component

Code

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States dcam

    I've been trying to understand the nature of this issue because it didn't make sense that blocks would be missing a title. And I didn't know how this related to a D7 migration. It started to sound more like a feature request to make block titles the same as the feed name. But I think that I figured out what the root cause of this issue is: In D7 the block title was the feed title. See https://git.drupalcode.org/project/drupal/-/blob/7.x/modules/aggregator/.... But in D8 Aggregator blocks in D8+ got a title of "Aggregator feed." So from the perception of a person migrating a D7 site, they might describe the block title as being "missing" as in "it lost the title at it had in D7."

    Back in 2013 when Aggregator blocks were derivatives, the derivatives did have the feed title as the block title. But then #1888702: Use configuration selection instead of derivatives for some blocks β†’ happened and the derivatives were eliminated in favor of having the feed selector setting. The feed-block titles were deleted along with them. I read through the issue to see if there was any consideration to that, but there wasn't. There was no test coverage. So this is a feature regression.

    We definitely can't implement the existing patch as it is. I just tested it. I placed a block with the default "Aggregator feed" title. Then I applied the patch. The rendered block title didn't change, but when I went into the block's config form the title field contained the feed title.
    So this is not desirable behavior. Oddly enough, I tested changing the block title to something else and and it changed on the front end. But when I went back into the config form again the field reverted to the feed title.

    Personally, I don't think that any change should be made to the current behavior for blocks being placed in D9/10. I don't know that it makes any sense because you configure which feed is displayed at the same time that you set the title. So I say we forget the current patch. But maybe we need to set the titles of blocks that are migrated, if that's possible.

  • Status changed to Active 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States dcam

    Nope. I was wrong. The block titles are literally empty after being migrated from D7. I think the problem is that in D7 the block title was set or altered by Aggregator to be the feed title. That doesn't happen in D8+. It just ends up having no H2. I haven't checked D6 yet.

    On the surface this seems like an easy fix. We need to alter the migration and set the block label to the feed title. I just have to find out how to query for it.

  • Status changed to Needs review 10 months ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update 10 months ago
    146 pass, 4 fail
  • πŸ‡ΊπŸ‡ΈUnited States dcam

    I added hook_migrate_prepare_row() in order to alter the block's label as it's being migrated.

    I copied the D6 block migration test for D7. The Aggregator feed block had to be enabled in the fixture.

  • The last submitted patch, 20: 2927014-20.patch, failed testing. View results β†’
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update 10 months ago
    146 pass, 4 fail
  • πŸ‡ΊπŸ‡ΈUnited States dcam

    I accidentally included changes from another patch. I don't think this will fix the failures though.

  • The last submitted patch, 22: 2927014-22.patch, failed testing. View results β†’
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update 10 months ago
    148 pass
  • πŸ‡ΊπŸ‡ΈUnited States dcam
  • Status changed to RTBC 10 months ago
  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    Nice one πŸ‘Œ

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update 10 months ago
    148 pass
    • dcam β†’ committed b5046b54 on 2.x
      Issue #2927014 by dcam, chiranjeeb2410, ranjith_kumar_k_u, larowlan:...
    • dcam β†’ committed 27f470db on 1.x
      Issue #2927014 by dcam, chiranjeeb2410, ranjith_kumar_k_u, larowlan:...
  • Status changed to Fixed 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States dcam

    Thank you to everyone who helped work on this patch!

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

Production build 0.69.0 2024