Problem/Motivation

This block has many issues as outlined in https://www.drupal.org/project/drupal/issues/3174990#comment-16035695

1. Has a block_count config which does nothing. This will need an update path to remove the config, etc. See #3223555: SyndicateBlock has unused block_count configuration
2. Config dependencies - the block should depend on the view.
3. Access controls - The block shouldn't render if the view or display doesn't exist - what happens if someone renames the display from feed_1? Should we just hide the block in those cases?
4. Cacheability - the block render cache needs to depend on the views config.
5. The block breaks if the url changes in the frontpage view #3174990: Test that SyndicateBlock works when views disabled
6. It has a configFactory class property that isn't used.

Proposed resolution

Deprecate the block with no replacement

Remaining tasks

Do it

User interface changes

Block removed from Olivero

API changes

SyndicateBlock is deprecated

Release notes snippet

SyndicateBlock is deprecated

CommentFileSizeAuthor
#18 3518990-nr-bot.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3518990

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

acbramley created an issue. See original summary.

annmarysruthy’s picture

Assigned: Unassigned » annmarysruthy

quietone’s picture

I noticed the change record for this issue published. Since this issue not fixed I un-published it.

acbramley’s picture

Status: Active » Needs work

I don't think we should be adding @legacy to all those tests. The block should instead be removed from the profile and theme.

annmarysruthy changed the visibility of the branch 3518990-deprecate-syndicateblock to hidden.

annmarysruthy’s picture

@acbramley Can I skip deprecating the block and directly remove it? But what if the block is currently being used in sites.

berdir’s picture

You want to deprecate the block *plugin* and remove the block configuration from standard profile so it is no longer used.

berdir’s picture

This also is related to #3039240: Create a way to declare a plugin as deprecated. Ideally, a deprecated block shouldn't show up anymore as a possible option in the UI, but you should still be able to view and edit a block that's already placed, possibly with a warning in the UI.

We had custom logic for this for the node type visibility plugin in D8 or 9.

annmarysruthy’s picture

Assigned: annmarysruthy » Unassigned

Deprecated the block and removed it from theme and profile. still getting a few test failures in merge request !11835.

acbramley’s picture

Most of the test failures were due to the block being in the update fixture which I've now removed.

The last failures are around performance tests.

StandardPerformanceTest is now failing with an additional query to the path alias table which is very strange since we're removing a block that renders a link to that:
SELECT 1 AS "expression" FROM "path_alias" "base_table" WHERE ("base_table"."status" = 1) AND ("base_table"."path" LIKE "/rss.xml%" ESCAPE '\\') LIMIT 1 OFFSET 0

acbramley’s picture

Status: Needs work » Needs review

Finally green.

Re #11 I think that'd be nice but I'm hoping since this block is most likely not used that much we probably don't need to block on that?

mstrelan’s picture

Opened #3522801: Deprecate RSS usage in core to discuss if we want to deprecate RSS more broadly in core

berdir’s picture

Do we want to do something about #11? Specifically, do we want to indicate or completely hide this block when placing a new block? We don't have an official way to do that anymore and the referenced issue is actually going to be tricky to implement now with typed classes and constructors, but we could add a a hardcoded workaround.

acbramley’s picture

Found a nice way to do #16 - there's a _block_ui_hidden key on the definition that BlockLibraryController uses to filter out definitions when adding a block. This doesn't affect existing blocks that have already been placed. Works perfectly for what we need here!

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

dcam made their first commit to this issue’s fork.

dcam’s picture

Status: Needs work » Needs review

Rebased MR 11853

berdir’s picture

Status: Needs review » Reviewed & tested by the community

I think this looks good. I agree that this should be removed as it's a relict of a time when the rss feed was hardcoded and not a view like it is now.

This does not remove any RSS capabilities in core, just a block that displays a hardcoded link to a path that might or might not actually be an RSS feed. #3522801: Deprecate RSS usage in core would be about removing that, which I disagree with as commented there.

  • catch committed b039cf93 on 11.2.x
    Issue #3518990 by annmarysruthy, acbramley, dcam, berdir: Deprecate...

  • catch committed 9e7e57dc on 11.x
    Issue #3518990 by annmarysruthy, acbramley, dcam, berdir: Deprecate...
catch’s picture

Version: 11.x-dev » 11.2.x-dev
Status: Reviewed & tested by the community » Fixed

Looks good to me too. There's going to be not-strictly-deprecated code to remove when this goes, but that's unavoidable, hopefully we'll find it when we need to remove it.

Committed/pushed to 11.x and cherry-picked to 11.2.x, thanks!

Status: Fixed » Closed (fixed)

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