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
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | 3518990-nr-bot.txt | 91 bytes | needs-review-queue-bot |
Issue fork drupal-3518990
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:
- 3518990-
changes, plain diff MR !11853
- 3518990-deprecate-syndicateblock
changes, plain diff MR !11835
Comments
Comment #2
annmarysruthy commentedComment #4
quietone commentedI noticed the change record for this issue published. Since this issue not fixed I un-published it.
Comment #5
acbramley commentedI don't think we should be adding
@legacyto all those tests. The block should instead be removed from the profile and theme.Comment #8
annmarysruthy commented@acbramley Can I skip deprecating the block and directly remove it? But what if the block is currently being used in sites.
Comment #9
berdirYou want to deprecate the block *plugin* and remove the block configuration from standard profile so it is no longer used.
Comment #11
berdirThis 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.
Comment #12
annmarysruthy commentedDeprecated the block and removed it from theme and profile. still getting a few test failures in merge request !11835.
Comment #13
acbramley commentedMost 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 0Comment #14
acbramley commentedFinally 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?
Comment #15
mstrelan commentedOpened #3522801: Deprecate RSS usage in core to discuss if we want to deprecate RSS more broadly in core
Comment #16
berdirDo 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.
Comment #17
acbramley commentedFound a nice way to do #16 - there's a
_block_ui_hiddenkey 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!Comment #18
needs-review-queue-bot commentedThe 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.
Comment #20
dcam commentedRebased MR 11853
Comment #21
berdirI 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.
Comment #24
catchLooks 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!