Problem/Motivation

BlockPluginInterface defines build() method that is supposed to return an array.
However, AggregatorFeedBlock returns NULL when configured feed is not loaded.

Proposed resolution

Return empty array when feed cannot be loaded.

CommentFileSizeAuthor
#10 3212354-10.patch2.49 KBguilhermevp
#9 3212354-9.patch2.49 KBguilhermevp
#7 3212354-7.patch439 bytesguilhermevp
#5 3212354-5.patch460 bytesguilhermevp
#3 3212354-3.patch4.5 KBguilhermevp

Comments

Chi created an issue. See original summary.

chi’s picture

chi commented
Title: AggregatorFeedBlock » AggregatorFeedBlock::build must always return array
guilhermevp’s picture

guilhermevp commented
Status: Active » Needs review
StatusFileSize
new 3212354-3.patch4.5 KB

A patch for it.

chi’s picture

chi commented
Status: Needs review » Needs work

The patch includes a bunch of unrelated changes.

guilhermevp’s picture

guilhermevp commented
Status: Needs work » Needs review
StatusFileSize
new 3212354-5.patch460 bytes

Sorry for wrong patch.

chi’s picture

chi commented

@guilhermevp I would use early return instead of "else".
Example: https://git.drupalcode.org/project/drupal/-/blob/9.1.8/core/modules/help...

guilhermevp’s picture

guilhermevp commented
StatusFileSize
new 3212354-7.patch439 bytes

Right, makes sense. Thanks for the feedback!

chi’s picture

chi commented

@guilhermevp That's not early return but rather the opposite.

https://pear.php.net/manual/en/standards.bestpractices.php#standards.bes...

guilhermevp’s picture

guilhermevp commented
StatusFileSize
new 3212354-9.patch2.49 KB

Sorry, thanks again for the feedback.
Sending patch.

guilhermevp’s picture

guilhermevp commented
StatusFileSize
new 3212354-10.patch2.49 KB

removed indentation error.

chi’s picture

chi commented
Status: Needs review » Reviewed & tested by the community

Thank you.

  • catch committed ebf0c95 on 9.3.x 
    Issue #3212354 by guilhermevp, Chi: AggregatorFeedBlock::build must...

  • catch committed ff99906 on 9.2.x 
    Issue #3212354 by guilhermevp, Chi: AggregatorFeedBlock::build must...

  • catch committed d39e25d on 9.1.x 
    Issue #3212354 by guilhermevp, Chi: AggregatorFeedBlock::build must...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Don't think it's useful to add an explicit test for this.

Committed/pushed to 9.3.x, cherry-picked to 9.2.x and 9.1.x, thanks!

Status: Fixed » Closed (fixed)

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