🇳🇱Netherlands @megachriz

I've not seen an issue like this before. Is "entity already exists" the exact error message?

If you enable the Feeds Log module, does that reveal more information? With that module, after an import, you can see the exact data that Feeds receives and how the data looked like after parsing. Or did you already try that and is that where you got the "entity already exists" message from?

I tested the new update manually on a site with feed types for many different entity types. With one exception, all feeds_item and revision__feeds_item were updated and got the new index. The only table not updated on the site that I tested the update function on, was called "old_b9a028taxonomy_term" and that appears to be a left-over table from a very old Drupal database update, as described in https://www.drupal.org/node/3046576 → , so it makes sense that this one wasn't updated.

I removed the test as it wasn't testing the update properly and also because I used a whole Drupal 10 database dump for it, it could possibly cause issues in future Drupal versions - when Drupal Core has removed necessary updates to upgrade from Drupal 10.

I scheduled the merge. Thanks @klausi for the improvement!

This question has been answered a long time ago, marking this as fixed.

The code is scheduled to be merged!

The code is scheduled to be merged. Thanks @oleksiy for your contribution!

The code is scheduled to be merged!

I've been able to reproduce the issue and I can confirm that the change fixes the issue. Thanks for the fix! Scheduled to be merged.

megachriz → made their first commit to this issue’s fork.

Thanks for the patch. I committed it. Sorry for the very late response. I guess I overlooked this issue.

megachriz → created an issue.

@oleksiy
Thanks for your contribution! I've changed the setting name to "display_applied_protections_message" and I added test coverage.

D9 tests run again! Also repaired an other test issue.

megachriz → created an issue.

megachriz → created an issue.

Thanks for the reminder. I put tagging a release on my list.

Here is an introduction video about the Feeds Log module if you want to know more about how to use it:
https://www.youtube.com/watch?v=P3ipggf5Irs

@aitala
If you enable the module "Feeds Log" (included with Feeds) and then try to import again, you could perhaps get a hint about what might be wrong. The Feeds Log module keeps track of items that it imported and how these items looked like. By inspecting the logged item, you can see the result of the parsed source data.

Yes, "Characters to trim" looks indeed to be used to strip whitespace (or other characters) from the beginning and end of a string.
It is a direct interface to PHP's trim() function.

The plugin that comes the closest to what you need is "Truncate", but then you need to know the exact number of characters that the whole string should have. It is not usable if you need to remove an exact number of characters at the end of the string and the length of the string is not constant.

But it should be doable what you want with the plugin "Find replace REGEX". For example, to remove the last 3 characters from a string:

• REGEX to find: /.{3}$/
• Leave the field for "Replacement pattern" empty.

Sounds related to 🐛 Sticky action buttons from a dropdown button set can't be clicked Active .

Closed #3567328: Save or Save an import button does nothing → as a duplicate.

Yes, this is a known issue. It's a bug in Gin Admin Theme: 🐛 Sticky action buttons from a dropdown button set can't be clicked Active . You can try to disable sticky action buttons in the Gin settings. Or switch to a different admin theme.

See also this Feeds issue: 🐛 When using Navigation module, Save button doesn't trigger Active

I've written tests for this issue. However, I think that the error being displayed on subsequent imports is out of scope this issue. Because it turns out that on files missing the import still goes through. It looks like this wasn't always the case as reported in 🐛 Item fails to import when image for file target fails to download Needs work .
Perhaps it would be better if the item (optionally) fails validation if a file cannot be found, but that can be handled in an other issue, perhaps in 🐛 Item fails to import when image for file target fails to download Needs work .

I noticed the "fix" for this issue is also covered by #3565186: Add service for resolving files → . So we could just wait till that one is merged and then only commit the test that is provided here. I wrote the tests based on FileResolver. The test testImportWith404Url() will fail here, because the error message used in the MR here isn't specialized for HTTP error codes yet, but FileResolver's implementation is.

megachriz → created an issue.

The updates to the test look wonderful and easier to read. I've scheduled a merge for this fix. Thanks all for contributing!

This is scheduled to be merged!

It could also be done with a post update, but the implementation would probably be similar.

With manual testing I discovered that the index doesn't get added to revision tables when running database updates.
For new feeds_item fields, the index does get added to revision tables.

I've been trying to add tests for this update, but it is complicated. The test pass even when checking the node_revision__feeds_item table after applying the update. But with manual testing, the index is not added.

Yes, that would be great to add an index for feeds_item_guid!

And perhaps the fact that the feed ID was not included in the query was not intentional. Because in the D7 version of Feeds, GUID was not globally unique, but unique per importer. The discussion in ✨ Add support for unique fields to be unique site wide Closed: outdated also suggests that it wasn't intentional. But I think there's no going back now, because of the risk of breaking people's workflows.

That would also explain why my colleagues developed custom code to prefix the feed item guid with the feed ID.

I've done that as well on a few sites. 😅

I think that with the current implementation you can update content created by an other feed as well. So changing the query could break certain workflows.

In 2968671-tests I've added test coverage for this feature, but the implementation of the feature itself is currently in #3565186: Add service for resolving files → .

I'm working on moving file resolving logic out of the File target in #3565186: Add service for resolving files → . This is because the Media target also needs file resolving features. (The media target is handled in ✨ Add a mapping target to media field Needs review ).

In my enthousiasm I already experimented with allowing local file paths as source. I've been testing that feature the last couple of days and I think it works quite well. Though it may not cover some edge cases reported here:

• #12 Ignore directory setting and use file at originally location
  If the file to import is for example "public://foo/text.txt", but the configured destination directory on the file target is "public://bar", FileResolver will copy the file to "public://bar/text.txt". The file on "public://foo/text.txt" is not removed. This perhaps prevents the issue reported in #33? #33 reports that files get deleted on import.
• #34 cURL error 1: Protocol "private" not supported or disabled in libcurl
  Not sure when that happens.
• #54 Issues with private files
  With testing I did notice some validation issues on private files. The workaround seems to be to give the feed owner permission to access the private file. Because during my testing so far, if the file was new, the file owner was set equal to the feed owner.
• The implementation does not use mime_content_type() and does not set "filemime". It looks like that the File entity sets this by themselves in \Drupal\file\Entity\File::preCreate().

I feel like these edge cases would be better handled in follow-ups. Let's first get absolute paths and public uri's supported! 😃

@andriy khomych
Are you already working on the tests? I'm working on moving file downloading/resolving to a separate service in #3565186: Add service for resolving files → . That's because the Media target (see ✨ Add a mapping target to media field Needs review ) needs file resolving features too. However, even when that file resolver issue gets completed, this issue would still need tests.

megachriz → created an issue.

Merging the code is scheduled to be merged. Thanks all for testing!

Merged the code!

Yes, having a static XML file instead of a generated one is fine to me for the tests.

I do think returning the XML earlier (when there is no xmlns) is better for the readability of the code, because it requires less nesting.

I wasn't aware of the feature to exclude certain files from releases. That would make the Feeds download a lot smaller, because there are many files related to tests.

Scheduled for merge.

A challenge in this is that Feeds treats multiple mappings to the same target as setting values for a multivalue field. For example, if a source has fields called like ‘author1’, ‘author2’, ‘author3’, etc.

An alternative approach that I have been working on (but not published) is a way to temporary override mappings. I used this in combination with a custom built import form (client specific) so with its current implementation I think it is not usable out of the box. Though I’m not completely sure, it has been a few years since I last worked on it.

If I’m not mistaken, this functionality is already provided by the Entity Clone module.
https://www.drupal.org/project/entity_clone →

I’m on mobile right now, so I cannot check that. Can you try Entity Clone module?
If that doesn’t work properly, you can reopen this issue.

Setting to "Needs review" so others have a chance to review the code if they want.

Thanks for your contribution! I fixed a RuntimeException class not found error in the code and added test coverage. Since I wasn't sure how to write a test for this case, the test was generated by AI and then slightly adjusted by me.

megachriz → made their first commit to this issue’s fork.

megachriz → created an issue.

I exported the definitions from before this change and after this change and then compared the two files to see if I spotted any differences that would be an error and did not see errors. There were slight differences in the plugin's descriptions. For some plugins, single quotes in the description were replaced with double quotes. But that's no error.

I used the following code for exporting the definitions:

drupal_flush_all_caches();

$keys = [
  'id',
  'label',
  'description',
  'category',
  'handle_multiples',
  'itemUsage',
];

$rows = [];
foreach (\Drupal::service('plugin.manager.tamper')->getDefinitions() as $definition) {
  $row = [];
  foreach ($keys as $key) {
    if (!array_key_exists($key, $definition)) {
      $row[$key] = 'undefined';
    }
    elseif (is_object($definition[$key])) {
      $row[$key] = (string) $definition[$key] .  ' (' . get_class($definition[$key]) . ')';
    }
    else {
      ob_start();
      var_dump($definition[$key]);
      $row[$key] = ob_get_clean();
      $row[$key] = trim($row[$key]);
    }
  }
  $rows[$definition['id']] = $row;
}
ksort($rows);

file_put_contents('/tmp/defs2.txt', print_r($rows, TRUE));

This is good, so I've scheduled to merge the changes.

And I see lots of test failures with Drupal Core 11.3.x-dev, while all tests pass with Drupal Core 11.2.8. 🤔
I hope my client will give me some time to help with this.

Importing multilingual content with Feeds is indeed not perfect yet. There are more language related Feeds issues. I tag these with "multilanguage", so I can find them back as soon as I plan to focus on Feeds multilingual issues again.

Closing this one as a duplicate of 💬 Invalid enclosure Active

Let me see if I understand things correctly:

1. An external process places a XML file and a jpg file into public://feeds.
2. The image file is always called "art-00.jpg".
3. You use Filefield Paths module to rename the file, but that is configured in a way that previous files could get overwritten.
4. Because of that you, you've disabled the Filefield Paths configuration and are now trying to rename the file with Feeds instead.

I think that the issue is this:

1. Feeds only supports renaming a file during the process stage and only by numbering them. At the process stage, Feeds writes the data that it received.
2. Tampers (including the Rewrite Tamper) are applied during the parse stage. If you apply a Tamper on a source that contains a reference to a file, you are instructing Feeds where to look for the file to download. This does not rename the file itself.
   You are saying to Feeds something like: instead of looking for a file at "public://feeds/art-00.jpg", look for a file at "public://feeds/Hania Rani - Portico Quartet & Hania Rani EP.jpg" instead. And Feeds reports back "Hm, I cannot find that file.". The file that is there is still called "art-00.jpg".

I don't know if there is a way to configure Filefield Paths that it will not overwrite existing files. I think your best option is to include something in the file path which would make the image name unique. That could be the node ID for example or something else that uniquely defines the album image.

But it sounds like that files getting a new ID on every import is an issue that you encounter. I don't see a direct solution for that problem. I know there exists an unresolved issue when using Feeds and Filefield Paths together: namely that on every import a new file gets created. It is an issue in the 8.x-3.x version of Feeds too.

I hope this helps you further.

foxy-vikvik
Yes, you are right that an example for QueryPath should be added to the documentation.
Do you want to add one on https://www.drupal.org/docs/contributed-modules/feeds-extensible-parsers... → ? I could add one too, but it might take a while before I'll get to it.

A working solution for this feature currently exists in my sandbox Feeds DEV → . I've been using that for years with success. Only thing what needs to be done is moving the code to an official project (probably my other sandbox Feeds Base64 images → [which would need to become an official project]) and fix any code style issues.

When importing files, you need two source fields:

• A source field with the base64 string
• A source field with the file name

There is one caveat with the solution provided there: all file names need to be unique - even if the files are saved to different folders. That's because all files are saved to a temporary folder first - and if file names are not unique, it would overwrite previously assembled files.

megachriz → created an issue.

I've updated the conversion:

• category is made translatable.
• new property itemUsage is now converted too.
• plugins that were added this year ('absolute_url', 'entity_finder' and 'twig') have now also conversions.

Do the changes in the MR help to resolve the issue?

Thanks for merging!

Just so you know, on the MR pipeline there is now an option to run tests without the fix. It also runs only the tests in files that were modified.

Is this still an issue?

If so, I need more information on how you configured the feed type in question and how you configured any tampers. I think that it is most likely caused by a configuration error.

I've provided a test and a possible fix in the MR.

megachriz → created an issue.

Hm, for this particular plugin, the defaults should be NULL. I think that's why defaultConfiguration() is not defined. The method getSetting() would return NULL if the setting key is not defined.

Is a checkbox field not allowed to have NULL as default? Then it should probably become FALSE if $this->getSetting(static::SETTING_FALLBACK) returns NULL.

Current code for the checkbox field:

$form[static::SETTING_FALLBACK] = [
  '#type' => 'checkbox',
  '#title' => t('Fallback to strtotime() if the date could not be parsed with the provided date format.'),
  '#default_value' => $this->getSetting(static::SETTING_FALLBACK),
  '#states' => [
    'visible' => [
      'input[name="plugin_configuration[date_format]"]' => ['filled' => TRUE],
    ],
  ],
];

(note: I see it is calling t() instead of $this->t().)

But I'm willing to define defaultConfiguration() just with both setting keys set to NULL. Would that help?

@foxy-vikvik
Based on the error message I see that you are using one of the QueryPath parsers. I see that an example for these parsers are not yet documented on https://www.drupal.org/docs/contributed-modules/feeds-extensible-parsers... → .

I don't use the QueryPath parsers myself, but based on the information in the tests it looks like to work in a similar way as how you would select elements with CSS on a HTML page.

For example in the following HTML the div could be selected by .post, by div, by html body div or in a few other ways.

<html>
  <body>
    <div class="post">

So instead of using /rss/channel/item you could use rss channel item or just simply item.

You can also use the XML Xpath parser instead. Then the context value that you provided in the screenshot should be valid.

It seems this module is not actively maintained.

Yes, I saw I hadn't enabled notifications for this project yet. But it is true that I give this project very minimal attention. Most of my attention goes to Feeds. For Commerce Feeds I mostly only come back for when it's time to make it up to date for the next version of Drupal Core.

This bug does look worthwhile to fix. Does this occur when there are no currencies configured in Commerce?

I think the fix and the tests are good enough.

I cannot remove febbraro as he is the owner of the project node.

I went ahead and removed those that haven't been active in the last 10 years in this project. @twistor was last active I think in 2016 and @franz did contribute to Feeds 1.5 years ago. So I haven't remove them.

I went ahead and removed those that haven't been active in the last 10 years in this project. @twistor was last active I think in 2016 and @franz did contribute to Feeds 1.5 years ago. So I haven't remove them.

Oh, I see I had created an issue for this as well: 📌 Cleanup list of Feeds maintainers Active . I thought last commit 10 years ago was a fair reason to remove them from the list.

I assume that the cause was that the available update information wasn't updated yet on your site. 8.x-3.2 was only released yesterday. And not long before that, 8.x-3.0 was the latest release. (The 8.x-3.1 release was the latest release for only one minute.)

Thanks for closing this issue. 🙂

I mean the "Check manually" link on /admin/reports/updates:

drush cr does not refresh the list of available updates I think.

By the way, when I install Feeds (not update it) on an existing Drupal site, I get this:

Ah, that's weird. Can you try to refresh the update data? (There is a button somewhere in the admin interface for that.)

Merged.

In the issue fork no GitLab issues. Go figure. 🤨

Well, let's fix and merge 📌 Fix PHPStan issues Active and then we'll see again.

megachriz → created an issue.

megachriz → created an issue.

@nicolasgraph
I have created a 8.x-3.1 → release that restores Drupal 10.1 compatibility. This is basically what is in the 8.x-3.0 release + the changes from the 3553772-10.1-support-2 branch. So this includes the changes that you posted in #8.

I also released 8.x-3.2 → that now requires at least Drupal 10.2.

I've scheduled the merge.

Alright!

Interesting request. I do wonder if you could also do the following instead: add the "Default value" plugin where you enter the values separated by comma in a long string and then follow it up with the "Explode" Tamper plugin, where you split the values by comma. I think that would also work?

PHP 8.4 testing is now enabled on the project.

The tests do pass at least on PHP 8.4 + Drupal 11.2.5. Maybe there was a hickup in the installation process - one that doesn't happen every time. If PHP 8.4 is passing code style checks as well, I merge the MR and mark this as "Works as designed".

Alright, thanks for reporting that back.

I do see that Feeds isn't automatically tested on PHP 8.4 yet. So we could check if Feeds tests pass on it.

megachriz → made their first commit to this issue’s fork.

For your interest, in 3135359-temp I made an attempt to run tests for both CommonMark v1 and v2, so that we could get test coverage for ✨ Add support for Commonmark v2 Active . Since the tests weren't merged before, I temporarily experimented with it in this issue instead.

So ideally, the commits from that temp branch that are worth it, should move to ✨ Add support for Commonmark v2 Active .

@joelpittet
Ah, thanks for merging! That work project that I talked with you about on Slack last week has been demanding all my work time in the past 6+ months, so I wasn't able to continue with Markdown during office hours.

@damienmckenna
Thanks for reporting that back. If it happens incidentally, but it does continue to happen, maybe the Feeds Log could perhaps give an indication what might go wrong when it happens again? "Feeds Log" is a module included with the Feeds project. For every import, it logs which items were created, updated, cleaned (deleted or unpublished) or ignored. By default, the logs are kept for one week because it can potentially store a lot of data (when you have very large imports).

@damienmckenna
Could your issue perhaps be related to 🐛 Wrong clean action applied when running imports for multiple feeds in one request Active ?

This issue is not fixable at the moment. The library does indeed have marked the interface as deprecated, but it also doesn't work without it yet.
See https://github.com/laminas/laminas-feed/issues/85

@andriy khomych
Thanks for confirming! Also thanks for your contribution and your patience to get this issue resolved. I scheduled the MR for merging.

I think that this is good to go! Changes that I made:

• Expanded description of CronTest::testImportPeriodPerFeed() a bit more and also added a few more comments in the test.
• When creating or editing a feed, I noticed the UI showed an option called "- None -". I think this was confusing because that option did essentially the same as "Use the feed type default", so I removed that option in FeedForm. I also set the default value of the field to "Use the feed type default" in case no value was set yet.

Is it good to go for you too?

megachriz → created an issue.