Problem/Motivation

\Drupal\aggregator\Entity\Item::getLink() says it returns strings but it can return a NULL.

Steps to reproduce

Run Drupal\Tests\aggregator\Functional\AggregatorDisplayConfigurableTest on PHP 8.1

Proposed resolution

Return an empty string instead of a NULL.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review

Here's the fix from the meta. The deprecation occurs due to template_preprocess_aggregator_item()

alexpott’s picture

alexpott’s picture

larowlan’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

Status: Reviewed & tested by the community » Needs work

Aggregator's on its way out of core, but could we have an issue for the @todo to link to?

alexpott’s picture

Status: Needs work » Needs review
FileSize
465 bytes
452 bytes

I investigated whether or not we could replace the $variables['url'] = UrlHelper::stripDangerousProtocols($item->getLink()); with $variables['url'] = $item->toUrl()->toSting();. That didn't work because a NULL link value triggers an exception in $item->toUrl(). I think the fix here is fine. Gonna remove the @todo

andypost’s picture

Status: Needs review » Reviewed & tested by the community

++ no todo needed

andypost’s picture

larowlan’s picture

Status: Reviewed & tested by the community » Fixed

I wonder if there's merit in a follow-up to make the link field required.

Committed c9ec5e4 and pushed to 9.3.x. Thanks!

  • larowlan committed c9ec5e4 on 9.3.x
    Issue #3240148 by alexpott: \Drupal\aggregator\Entity\Item::getLink()...

Status: Fixed » Closed (fixed)

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