Problem/Motivation

Drupal 10 no longer ships with the library "laminas/laminas-feed". To make Feeds usable without using Composer on Drupal 10, we need to add Ludwig support by adding a ludwig.json file. Note: Feeds Extensible Parsers already has Ludwig integration, so it make sense to add it for Feeds too.

Steps to reproduce

  1. Get a Drupal 10 installation.
  2. Checkout Feeds via git or ">download it manually.
  3. Install Feeds on the D10 site.
  4. Create a feed type with the RSS/Atom parser.
  5. Try to import a RSS feed, this will likely fail because of the missing library.

Proposed resolution

Add a ludwig.json file that specifies the "laminas/laminas-feed" library. For instructions, see https://www.drupal.org/docs/contributed-modules/ludwig/ludwig-integratio...

Manually test with installing the Ludwig module if the library can get successfully installed. Then check if importing a RSS feed can succeed by following the steps to reproduce this issue.

Remaining tasks

  • Add a ludwig.json file.
  • Test if the library "laminas/laminas-feed" can be installed with the Ludwig module.
  • Test if a RSS feed can be imported.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#8 feeds-missing-library-message.png138.32 KBmegachriz
#8 interdiff-3313864-2-8.txt874 bytesmegachriz
#8 feeds-ludwig-3313864-8.patch2.11 KBmegachriz
#2 3313864-02.patch1.25 KBdevad

Issue fork feeds-3313864

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:

  • 3313864-ludwig Comparecompare
About issue forks

Comments

MegaChriz created an issue. See original summary.

devad’s picture

devad commented
Status: Active » Needs review
StatusFileSize
new 3313864-02.patch1.25 KB

Hi.

This is the Composer-generated library dependancy tree of the laminas/laminas-feed package:

  - Installing laminas/laminas-stdlib (3.13.0): Extracting archive
  - Installing laminas/laminas-servicemanager (3.15.0): Extracting archive
  - Installing laminas/laminas-escaper (2.10.0): Extracting archive
  - Installing laminas/laminas-feed (2.18.2): Extracting archive

This patch should do the job.

The .module file code addition is needed due to "files" autoload type inside laminas/laminas-servicemanager package. It is an one-time addition and it does not need the maintenance usually.

The ludwig.json file will need to be updated accordingly every time the composer.json file's laminas/laminas-feed library version is updated.

irinaz’s picture

irinaz commented
Assigned: Unassigned » irinaz
irinaz’s picture

irinaz commented

Tested during BADCamp, patch works as expected!

irinaz’s picture

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

irinaz commented
Status: Reviewed & tested by the community » Needs review
devad’s picture

devad commented

@irinaz thank you for review.

Is there a reason why you have downgraded this issue from RTBC back to "needs review" afterwards?

megachriz’s picture

megachriz
Primary language Dutch
commented
StatusFileSize
new feeds-ludwig-3313864-8.patch2.11 KB
new interdiff-3313864-2-8.txt874 bytes
new feeds-missing-library-message.png138.32 KB

@devad
The reason was that I wanted to leave it open for someone else to review during BADCamp.

I checked it myself too and I noticed that the import still failed after the libraries were downloaded. I figured out that a cache clear was needed for the libraries to be detected. Maybe this is something that could be improved in the Ludwig module? Automatically clear caches after downloading new libraries?

Then I thought that it would be nice if an import fails gracefully if the library is missing. The import was now failing with an AJAX error when importing using the UI.

So in the updated patch I added code that throws an exception during parsing if the library is missing. That results into the following in the UI:

devad’s picture

devad commented

Thank you @MegaChriz.

The reason was that I wanted to leave it open for someone else to review during BADCamp.

Sure. It makes sense.

I checked it myself too and I noticed that the import still failed after the libraries were downloaded. I figured out that a cache clear was needed for the libraries to be detected. Maybe this is something that could be improved in the Ludwig module? Automatically clear caches after downloading new libraries?

Good catch! I was in a BIG illusion that the automatic clear caches after missing packages are downloaded exists in Ludwig module already. The cache clear is added now and a new Ludwig 2.0.5 release is created:

#3315051: Automatic clear caches after downloading missing libraries

Thanks again!

  • MegaChriz committed 5c688d8 on 8.x-3.x 
    Issue #3313864 by devad, MegaChriz, irinaz: Added integration with the...
megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs review » Fixed

Committed #8. Thanks @devad for the improvement on Ludwig as well!

devad’s picture

devad commented

Thanks @MegaChriz and welcome.

I have added the Feeds project to the Ludwig module ecosystem list.

Good job!

Status: Fixed » Closed (fixed)

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