Account created on 20 November 2009, over 15 years ago
  • Media technologist at WebCoo 
#

Merge Requests

More

Recent comments

🇳🇱Netherlands megachriz

I plan to work on this in the next few weeks.

🇳🇱Netherlands megachriz

This was already reported before in 🐛 Rewrite plugin does not work after Explode Closed: duplicate . However, I just closed that one as a duplicate too because a fix for this issue is now available in a combination of two other issues:

  • 🐛 "Rewrite" plugin doesn't work right Active
    This ensures that the data of previously applied Tamper plugins (thus before the Rewrite plugin) is written back to the item. The Rewrite plugin then finds the most up to date data on the item.
  • Rewrite plugin: iterate through values to rewrite Needs work
    Among other things, this adds support for arrays for the Rewrite plugin. If your initial source value is for example Foo,Bar,Qux and you explode that using a comma as separator and then use the Rewrite plugin to add a "x" to it, you'll get ["Foox","Barx","Quxx"] instead of ["Foox","Foox","Foox"].
🇳🇱Netherlands megachriz

A fix for this issue is now available in a combination of two other issues:

  • 🐛 "Rewrite" plugin doesn't work right Active
    This ensures that the data of previously applied Tamper plugins (thus before the Rewrite plugin) is written back to the item. The Rewrite plugin then finds the most up to data on the item.
  • Rewrite plugin: iterate through values to rewrite Needs work
    Among other things, this adds support for arrays for the Rewrite plugin. If your initial source value is for example Foo,Bar,Qux and you explode that using a comma as separator and then use the Rewrite plugin to add a "x" to it, you'll get ["Foox","Barx","Quxx"] instead of ["Foox","Foox","Foox"].

Closing this as a duplicate now.

🇳🇱Netherlands megachriz

I've provided a fix and I've updated the issue summary to clarify what the problem is.

🇳🇱Netherlands megachriz

Moving this to Feeds Tamper now.

🇳🇱Netherlands megachriz

@starlight-sparkle
Thanks for your contribution! Can you put your changes in a MR? The testbot on drupal.org no longer looks at patches, so changes need to be in a MR now if you want that the code eventually gets added to the module.

🇳🇱Netherlands megachriz

I've paused moving projects to Drupal 11 at the moment. I have planned to look at this again in September and October this year. In my notes I see that 🐛 Allow premium message pseudo-field to be hidden Needs review should be resolved.

🇳🇱Netherlands megachriz

@codebymikey
If you are interested, I am giving the Rewrite plugin a major overhaul in Rewrite plugin: iterate through values to rewrite Needs work . It supports array replacements, and using nested values as replacements. There is probably some overlap now with the Twig plugin in terms of functionality. Maybe I went a bit too far in expanding the Rewrite plugin, but in my defense the majority of it I had already written before this issue was opened, it just took me some time to polish it.

🇳🇱Netherlands megachriz

I think it would be good to add tests for this. The tests should cover the following:

  • When periodic import is configured for a feed type, the date for the next import should be displayed.
  • When periodic import is NOT configured for a feed type, it should say "Not scheduled".
  • When periodic import is NOT configured for a feed type, but an import in background is started, it should say "On next cron run" (or something similar, refer to the D7 version of Feeds for that).

Since I think that case 3 is currently not working, setting this issue to "Needs work".

🇳🇱Netherlands megachriz

It depends on the current user time zone for how the date is displayed. For example, I have for one feed displaying the following:

do, 01/01/1970 - 00:59

I think it is better to fix this in FeedViewBuilder and replace the date with the text 'Not scheduled'. This is also consistent with how a similar issue was fixed in the D7 version of Feeds.

It will then look like this:

🇳🇱Netherlands megachriz

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

🇳🇱Netherlands megachriz

Setting the action to resolve the issue in the issue title.

🇳🇱Netherlands megachriz

I've tested the MR this way:

  • Tried to import a CSV file with nothing in it: no items were cleaned. This is good.
  • Tried to import a CSV file that did not exist (404): no items were cleaned. This is good.
  • Tried to import a CSV file that did not change (304): no items were cleaned. This is good.
  • Tried to import a CSV file that has only a header row: all items were cleaned. This is good.
  • Tried to import a XML file with zero items in it: all items were cleaned. This is good.

I went through the code one more time and only made some small changes to code comments.

So I think all is good. Merged! Thanks all for contributing.

🇳🇱Netherlands megachriz

I had the code running for more than a week on a live site, I did not notice any regressions so far. I merged the code!

🇳🇱Netherlands megachriz

Setting this issue back to "active", because there is a way to reproduce it.

🇳🇱Netherlands megachriz

It sounds like this is an issue in the Gin Admin theme:
🐛 Sticky action buttons from a dropdown button set can't be clicked Active

As far as I can see, there is no mistake in Feeds in the definition of the dropbutton. But I leave this issue open, should a change in Feeds be needed to fix it.

I do see that in Gin, the submit buttons are rendered outside of <form>, so perhaps that causes issues with reapplying the right form actions.

🇳🇱Netherlands megachriz

Perhaps the Gin Admin theme (version 5.0.3) with the "Enable sticky action buttons" feature turned on doesn't work well with dropbuttons?

On Gin 4.0.6 (also with the "Enable sticky action buttons" feature turned) I noticed that the import buttons weren't displayed at all.

🇳🇱Netherlands megachriz

I think that the issue your reported here is covered by Rewrite plugin: iterate through values to rewrite Needs work . Can you confirm?

Closing this as a duplicate for now. Feel free to reopen if Rewrite plugin: iterate through values to rewrite Needs work doesn't help to resolve this. (I realize I reply two years later, but I only now get to address issues related to the Rewrite plugin.)

🇳🇱Netherlands megachriz

I think that the issue was that previously the Rewrite plugin displayed the labels of sources instead of keys. This has been resolved in #3185029: Rewrite plugin: use keys of source definition in replacement patterns in november 2022, so I assume that this issue is no longer a thing. Feel free to reopen if I'm wrong.

🇳🇱Netherlands megachriz

Array replacing is now supported.

If the source data is:

[
  'name' => [
    'foo',
    'bar',
    'qux',
  ],
  'ext' => [
    'jpg',
    'png',
    'gif',
  ],
]

And you apply a the Rewrite plugin to the source 'name' using the text [name].[ext]

You will get:

[
  'name' => [
    'foo.jpg',
    'bar.png',
    'qux.gif',
  ],
  'ext' => [
    'jpg',
    'png',
    'gif',
  ],
]

I've also been working on supporting nested data, if you use the text [name].[ext.1] instead, you should get:

[
  'name' => [
    'foo.png',
    'bar.png',
    'qux.png',
  ],
  'ext' => [
    'jpg',
    'png',
    'gif',
  ],
]

For this I've not added test coverage yet. Marking as "needs work" for additional test coverage.

🇳🇱Netherlands megachriz

I think that the issue actually is in Feeds Tamper:

  1. Say you have a source item with the following data:
    [
      'phone' => '123/456-7890',
    ]
    
  2. On the source "phone", you apply the Tamper "find_replace", replacing / with )
  3. Also on the source "phone", you apply the Tamper "rewrite", with the following text: ([phone]

With Feeds, the result becomes:

[
  'phone' => '(123/456-7890',
]

That's because Feeds Tamper now writes the result back to the source at the end of the chain and not in between.

  • Branch 3323381-rewrite-tests_only changes ChainedTamperTest to mimic Feeds Tamper's current behavior.
  • Branch 3323381-rewrite-test-updated changes ChainedTamperTest to mimic what Feeds Tamper's behavior probably should be.
🇳🇱Netherlands megachriz

Merged!

🇳🇱Netherlands megachriz

@lindsay.wils
Can you try to reproduce your issue on a clean install? I cannot help fix the issue if I don't know how it occurs.

🇳🇱Netherlands megachriz

megachriz created an issue.

🇳🇱Netherlands megachriz

Nice work so far! I've some remarks:

  1. I think that we should avoid <code> tags in translatable strings.
  2. On the feed form, it would be useful to see what the default configured headers are, maybe as a placeholder for the textarea.
  3. I think it would be better if the configured http headers are saved as an array instead of a string. For both the feed type and the feed.
  4. Needs tests.

Test coverage

  • Functional test for configuring a feed type with http headers:
    • Assert that misconfigured headers result into a validation error.
    • Assert that empty lines are ignored and not saved as configuration.
    • Assert that configured headers are saved.
  • Functional test for configuring a feed with http headers override: test the same things as for configuring a feed type with http headers.
  • Unit test for asserting the expected headers when they are configured on the feed type.
  • Unit test for asserting the expected headers when they are configured on the feed.
  • Unit test for asserting the expected headers when they are configured on both the feed type and the feed.
🇳🇱Netherlands megachriz

To summarize:

  • You have three feed nodes using the same feed type.
  • The content item that got unpublished was only imported by one feed, not by the other two.
  • The content item that got unpublished was present in the source file and even updated during the import.

Is there a way for you to reproduce the issue on a clean Drupal install? Because else it would be very hard to find out what exactly is causing this bug.

🇳🇱Netherlands megachriz

@joelpittet
Good catch about the exception throwing during clean(). I think if one of the plugins throws an exception, the remaining plugins should still be called. I have adjusted this. (In practice, only the processor plugin in Feeds implements CleanableInterface, but in theory, there could exist contributed modules that implement it for the other plugins).

Also, is there any risk that one plugin’s clean() call could mutate shared state on the feed in a way that affects how later plugins behave within the same loop?

Yes, each clean() receives the same state object, so they could manipulate that. I'm not sure yet if that is a concern. At least, that behavior doesn't change with the code changes.

🇳🇱Netherlands megachriz

Wow! This looks great. In the code I could only spot that some of the code comments and names of methods could be improved. And that there are a few unrelated fixes. Ideally these should be handled in a separate issue, but I do appreciate getting these fixed too. 😀

I'm going to apply the changes on a live site that uses Feeds just to see if that results into any regressions.

Thanks for the work so far!

🇳🇱Netherlands megachriz

Very well made! I merged it.

Possible improvement for a follow-up: allow arrays as replacement patterns. The Rewrite plugin also is limited in this regard.

Let's say you have two sources called "name" and "extension" and you have the following lines in the CSV:

name,extension
foo|bar,jpg|png
baz,gif

You might want that to result into:

I assume that is what is 🐛 The rewrite plugin renames multiple values with the same (first) row Active about.

🇳🇱Netherlands megachriz

This is a great addition! I do wonder if we should fallback to strtotime() when parsing the date fails. Maybe provide that as an option? "Fallback to strtotime() if the date could not be parsed.". What do you think?

If you have applied the Tamper "String to Unix Timestamp" to the source "date" and set the date format to "d/m/Y" and then import the following CSV:

title,date
Foo,2020-09-01
Bar,01/11/2011
Qux,01-10-2011

This is the result after import (using the latest dev releases of Feeds and Feeds Tamper):

🇳🇱Netherlands megachriz

Alright, so the following is a different issue for you:

title,url,status,title2
I see the issues here are:

  • title2 appears at last, even though it is used on the first mapping row.
  • title still appears in the template, even if it is no longer used in the mappings.

Expected CSV template should have been:

title2,url,status

Can you be more specific with the issue that you experience and provide a step by step plan with how to reproduce it?

🇳🇱Netherlands megachriz

I merged the code! This issue took a long time to resolve, because in the past years I had put most of the time into getting Feeds to a stable release.

For the most optimal experience the latest dev of Feeds is required. Allow parsers and event subscribers to mark an item as invalid Needs work has been merged as well.

When this gets into a release of Feeds Tamper I plan to create a new Feeds release on the same day.

🇳🇱Netherlands megachriz

Navigation Extra Tools doesn't make any difference for me. The button "Save and import" still works for me. Could it be theme related? Or are there perhaps JS errors on the page?

🇳🇱Netherlands megachriz

I also tried to reproduce the issue on Drupal 11.2.2, but I also did not encounter the reported issue with that version.

🇳🇱Netherlands megachriz

I tried to reproduce this issue on Drupal 11.1.8 and PHP 8.3.10, on a clean install, but for me the button "Save and import" works fine on the feed add page.

Steps that I took:

  1. Installed Drupal using the standard profile (via drush si).
  2. Enabled modules "Navigation", "Feeds" and dependencies.
  3. Added a feed type with the HTTP fetcher and the RSS/Atom parser.
  4. Added mappings: "Title" to "Title (title)", "Item GUID" to "Feeds item (feeds_item): Item GUID", "Description" to "Body (body): Text".
  5. Added a feed with title "Feeds" and feed URL " https://www.drupal.org/project/issues/rss/feeds ".
  6. Clicked "Save and import".

The import went fine and imported 50 items.

So I think this needs more steps for reproducing the issue, try to reproduce it on a clean install.

🇳🇱Netherlands megachriz

Interesting issue. As I'm not familiar with the Trash module yet, I'm not sure what the best way is to solve this problem. If possible, I would like to avoid the soft dependency on the Trash module, because making sure the Trash specific code keeps working would require tests and a dev dependency on the Trash module. Also, I'm aiming for a pass on PHPStan. With the current implementation, I think that will mean that eventually all child classes of FieldTargetBase would need to pass the Trash service instance to the parent. See 📌 (PHPStan) Add dependency injection for FieldTargetBase and add deprecation warnings Active for other services I've planned to inject in FieldTargetBase in the future (and require them in Feeds 4.x).

One thing we could do instead, but I'm not sure if that's enough, is add a query tag to the query that Feeds executes, for example:

$query->addTag('feeds_unique_query');

This tag would then always be added.

In theory, you could then implement hook_query_TAG_alter() to perform the Trash specific code, though I don't know if adding another query tag in such implementation has the desired effect.

🇳🇱Netherlands megachriz

Feeds only aims to support targets for fields provided by Drupal core, so therefore I'm moving this issue to the Redirect issue queue.

There is already an issue there about it, so closing this as a duplicate as well.
Feeds: redirects_source field target class. RTBC

🇳🇱Netherlands megachriz

I see that the unit tests for the Copy plugin were a bit weak. Thanks for the improvements! I merged the code.

I hope to look at your other contributions soon.

🇳🇱Netherlands megachriz

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

🇳🇱Netherlands megachriz

Thanks for providing code and tests! Looks good at first glance.

I think that the tests fail because of the following warning:

1 test triggered 1 PHP warning:
1) /builds/issue/tamper-3530101/vendor/twig/twig/src/Template.php:345
Array to string conversion
Triggered by:
* Drupal\Tests\tamper\Unit\Plugin\Tamper\TwigTest::testTwig
/builds/issue/tamper-3530101/tests/src/Unit/Plugin/Tamper/TwigTest.php:93

🇳🇱Netherlands megachriz

I'm kinda waiting for a new release of Role Delegation, because tests currently still fail on PHP 8.4 most likely (but not 100% sure) only because of PHP 8.4 errors in that module. 📌 Nullable types must be explicit Active was recently merged. But if a release on their side takes too long, then perhaps it would be good to create a new release that is at least partly PHP 8.4 compatible.

🇳🇱Netherlands megachriz

If you could add tests for it too, that would be great! (A unit test and a functional test are required in order for the code to be merged.)

🇳🇱Netherlands megachriz

Sounds like a good idea! Do you need any pointers to start with writing such a plugin?

🇳🇱Netherlands megachriz

A feed gets in a locked state as soon as an import for it gets started. It gets unlocked again when the import successfully finishes. When an import ends abruptly (for example because of a PHP fatal error), the feed remains locked.

The easiest way to get a feed in a locked state (and have it be in that state for a bit longer) is to go to a feed, click on "Import in background" (/feed/x/schedule-import) and click the button "Schedule import". This will lock the feed and create a queue task. The queue task will be performed on cron runs. So long that you don't cron, the feed remains locked (unless you unlock it manually).

You can also manually lock a feed using the drush command 'feeds:lock':

drush feeds:lock 1

'1' is the ID of the feed to lock.

🇳🇱Netherlands megachriz

Not that I know of. The file target in Feeds can currently only download the image from a url. So without custom code or additional modules the image would need to be downsized at the source.

I do have written an alternative file target that accepts a base64 string as input in my Feeds DEV sandbox project. But then you would still need to write Tamper plugins to download the image, downsize it and convert it to a base64 string.

It would probably be better idea to make a change to the image target in Feeds to respect the maximum image dimension setting. Too bad that setting is not evaluated automatically when storing images programmatically.

🇳🇱Netherlands megachriz

There should be a button called "Update" on the settings form. Theming issue? Try if you can replicate the issue with a different admin theme. If you then see the update button, the issue is likely theme related.

Screenshot from a Dutch site:

🇳🇱Netherlands megachriz

I see that this feature was completed when #2989279: Add a target to entity ID was done.

🇳🇱Netherlands megachriz

Thanks for working on this!

I think that the Expire feature should be able to work without requiring an import. So it should get triggered on cron runs by itself. Do you think that makes sense? Is that doable without loading all feed types on each cron run?

🇳🇱Netherlands megachriz

The Tamper "Unix timestamp to Date" requires indeed a number as input.

What you could do is the following:

  1. Map a blank source to your date (or text) field.
  2. Add the Tamper "Set value or default value" and set the value to "now".
  3. Add the Tamper "String to Unix Timestamp", this will convert "now" to the current time.
  4. Add the Tamper "Unix Timestamp to Date" (with for example the format "Y-m-d"). You'll get the current date.

If you set the mapping target as unique, I think you'll get one item each day.

Do note that you could possibly get duplicate items imported in case the item on day 1 is the same as on day 2.

🇳🇱Netherlands megachriz

Yes, this is possible. You just need to have a field on your content type that can be used as a unique identifier. That can be any text or number field. Using the node ID is also possible. Feeds Item GUID may not be possible, because I'm not sure if Devel Generate sets a value for that field, I would assume it does not.

On the mapping page you need to map to the field that can be used as a unique identifier and for that target, be sure to click the checkbox in the column "Unique":

🇳🇱Netherlands megachriz

This feature has recently be added to Feeds: Change RSS default importer to accomodate multiple values in Active

Currently it is only available in the dev version.

🇳🇱Netherlands megachriz

Feeds only aims to provide mapping targets for fields defined by Drupal Core. This looks like to request support for a field type by a contrib module, so the FeedsTarget plugin should be added to that module instead.

I see an issue already exist there, so closing this as a duplicate as well.
Feeds mapper Active

🇳🇱Netherlands megachriz

Alright. I tested the code myself by using chmod 000 on certain directories to see if the directory got displayed in the error message on import and it did.

I merged the code.

🇳🇱Netherlands megachriz

This is now ready to be tested in combination with 🐛 Catch TamperException to prevent import from crashing Needs work for Feeds Tamper.

🇳🇱Netherlands megachriz

I looked through the code one more time, did not spot anything suspicious. Merged it. Thanks all who helped on this issue.

🇳🇱Netherlands megachriz

Upon an exception, an item can now get marked as invalid. This causes the item to fail validation and the item will not be imported.

Depends on Allow parsers and event subscribers to mark an item as invalid Needs work .

Result of applying the Tamper plugin "Filter items" while the source data is a string instead of an array:

And in the log (Feeds Log module):

🇳🇱Netherlands megachriz

@fonant
Do not that this is a patch for the D7 version of Feeds Tamper. Did you try to apply it on the D7 version of Feeds Tamper?

I'm still willing to commit code to the D7 version of this project, but I won't put much time in testing it myself.

🇳🇱Netherlands megachriz

In two places I've added the directory name to the error message. I think that's all, or did I miss one?

🇳🇱Netherlands megachriz

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

🇳🇱Netherlands megachriz

@max.valetov
I did not take a deep dive into the Media target issue yet, mostly only worked on the tests so far.
I do know that the source that you should map to the Media target must be an url using the http or https scheme.
Other source types, like local path or not yet supported, but there is being worked on: File target: add support for local file path as source Needs work .
The source url must contain an extension as well, but there is an issue open for supporting file urls that do not specify the extension: 💬 Failed to validate remote image with no file extension Needs work

If you are not sure what value Feeds receives, I recommend to enable the Feeds Log module (included with Feeds). With that module you can see what items were processed and what the source item looked like at the time of processing.

🇳🇱Netherlands megachriz

If you can write a test that demonstrates the bug, that would be great! The test should be added to \Drupal\Tests\tamper\Unit\Plugin\Tamper\ConvertBooleanTest.

Since there is a patch, I set the issue status to "Needs work". "active" indicates that there's only a report and no existing code.

🇳🇱Netherlands megachriz

I've added a possible fix, still needs work because a new dependency called 'plugin.manager.field.field_type' is not injected yet. It might not be possible yet to inject it without either a BC break or a deprecation warning.

🇳🇱Netherlands megachriz

No fix yet, but I made a start with test coverage.

🇳🇱Netherlands megachriz

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

🇳🇱Netherlands megachriz

@thomasmurphy
I have not tested it, but I think that one, two, three doesn't work because there are spaces used. So the column names effectively became "one", " two" and " three". " two" is not equal to "two", because the first one starts with a space and the second doesn't.

In 🐛 CSV headers with extra space don't get mapped to correct fields Needs work there is being worked on a feature to ignore leading or closing spaces in CSV column names.
There also exists an issue to implement a feature to detect mismatches in configured CSV column names vs actual CSV column names: Allow parsers to validate source before import Active .

🇳🇱Netherlands megachriz

I've added a source called 'authors'. It returns an array. If you want to format this as a comma-seperated string, use the Feeds Tamper plugin "Implode".

🇳🇱Netherlands megachriz

Thanks for reporting back! I'll mark this issue as fixed then. The dependency implementation that should help to prevent this issue can be handled in 🐛 Field feeds_item is unknown when looking up a taxonomy item that doesn't exist Active .

🇳🇱Netherlands megachriz

No, importing from relative paths is not supported yet. Only urls using the http or https scheme are supported for importing files at the moment.

Adding support for relative paths is high on the wishlist, there is being worked on in File target: add support for local file path as source Needs work .

🇳🇱Netherlands megachriz

Thanks for filing a bug report. Perhaps the bug is that the plugin has set 'handle_multiples' to 'TRUE' in the annotation. Can you check that when you remove that if that also fixes the problem?

🇳🇱Netherlands megachriz

@dudeweb
This module does get very little attention from me, so indeed also for the docs. Great that you were able to figure out how to get products imported. It is indeed product variations first and products second. I don't know of an easier way.

If you run into issues like these, it's okay to ask a question on Drupal Slack . For Feeds specific questions, there is the #feeds channel. You are not guaranteed to always get an answer quickly, but the community is quite active on Slack, so it's worth a shot. 🙂

🇳🇱Netherlands megachriz

Did you try to reproduce the issue on a clean install? Because when I import two items with the same exact file (with the same name), then I get as result that both items reference the same file entity. In your case, you get an error. So there must exist a difference between my setup and your setup. The most likely difference is that you:

  • Have a module installed that I have not.
  • And/or have configured a file related setting differently.

To find out what that difference is, try to replicate the error on a clean install so we know which module or setting causes a different behavior.

Based on the current info, the only possible solution that I see is making sure that file names are unique. But there could perhaps exist an other solution if we can find out what causes a different behavior.

🇳🇱Netherlands megachriz

Can it be that some records reference text files that are in a different directory, but with the same name?

The error message says that a file from the temporary directory could not be written to either the public or private file system because on that file system there already exists a file with the same name.

I'm unable to reproduce the error however. Even when I try to import a CSV file like the following, I don't get an error:

guid,title,file
1,A,http://example.com/feeds/txt/a/sample.txt
2,B,http://example.com/feeds/txt/b/sample.txt

Or this:

guid,title,file
1,A,http://example.com/sites/default/files/bijlagen/sample.txt
2,B,http://example.com/sites/default/files/bijlagen/sample.txt

Can you try to reproduce the issue on a clean install? Maybe a file related module is configured in a way that causes this issue.

Using private:// or public:// as file source is indeed not yet supported. Only http sources are supported at the moment. There is a request to support the local file path a source: File target: add support for local file path as source Needs work .

🇳🇱Netherlands megachriz

Perhaps that resaving the feed type will fix the problem for you - if the cause is indeed a missing feeds_item field.

🇳🇱Netherlands megachriz

Can it be that you accidentally deleted the feeds_item field? Or did you not deploy the field? This field gets created automatically when you save a feed type. It holds metadata about the import, such as when an item was imported last.

There is a related issue about this where I propose to create a dependency on the feeds_item field on the feed type, so that the issue can no longer occur on deployment.
🐛 Field feeds_item is unknown when looking up a taxonomy item that doesn't exist Active

🇳🇱Netherlands megachriz

I'm trying to figure out from your bug report what you are trying to do and what problem you actually face. It seems related to importing data into a file field on a node.

Can you explain further with CSV examples what you are importing and at which moment something goes wrong and what goes wrong exactly?

For example:

  1. Create a feed type using the CSV parser with mapping to Feeds Item Guid, title and a file field. Set Feeds Item guid as unique.
  2. Import the following CSV file:
    id,title,file
    1,Foo,http://www.example.com/file1.txt
    2,Bar,
    

    If I read correctly, this import goes fine. New records with a text file reference and new records without text a file reference are getting imported.

  3. Change the CSV file and import again.
    id,title,file
    1,Foo Updated,http://www.example.com/file1.txt
    2,Bar Updated,
    

Record 2 updates fine, but for record 1 you get an error? Or do you get an error when on an update you reference a different text file? Or do you get an error if on an update the text file doesn't exist on the source?

🇳🇱Netherlands megachriz

I see that RNG has no gitlab CI support enabled yet. Do you want to add that, perhaps in an other issue, and then fix any test failures that may occur?

🇳🇱Netherlands megachriz

If you want to make a start on this, that would be great! It will take a while until the site that I use RNG on is ready for Drupal 11, so that's why I gave porting this module to D11 a low priority.

🇳🇱Netherlands megachriz

I see that the new properties are also exported when calling BaseItem::toArray(), which I think we don't want. So these properties should not be exported. So that should get fixed, but perhaps before we do that it makes sense to finalize 🐛 Creation of dynamic property Drupal\feeds\Feeds\Item\SyndicationItem::$parent:fid is deprecated Needs work first, because in that issue BaseItem::toArray() gets also changed. And I think that issue is good to go, just waiting a few more days for possible feedback on that one.

🇳🇱Netherlands megachriz

For backwards compatibility I have moved the new methods to a new interface called ValidatableItemInterface. Feeds Tamper can then check if the item implements that interface before trying to call the new methods. This way, Feeds can still be compatible with older Feeds versions.

Let's see if the test passes now.

🇳🇱Netherlands megachriz

I tried to reproduce the issue using the following steps:

  1. Created a taxonomy called 'tags'.
  2. Created a content type called 'article' with a taxonomy reference field for the taxonomy 'tags' and allowing an unlimited number of values.
  3. Created a feed type using the CSV parser and the node processor with content type 'Article'.
  4. Add mappings to 'Title' and 'Tags'. Configured the mapping target 'Tags' to reference by name and to autocreate terms in the vocabulary 'tags'.
  5. Added a Tamper Explode plugin to the 'Tags' mapping target and using | as delimiter.
  6. Imported the following CSV file:
    title,tags
    Lorem,Lehigh|Activities
    Ipsum,Lehigh|Residence Camp
    

My result was that two articles got imported, both with two terms.

Can you share your configuration and provide a sample source file? This way I can probably quicker try to reproduce the issue. One way to share configuration is by packing it inside a feature module.

Or else can you provide the exact steps to reproduce the issue?

🇳🇱Netherlands megachriz

Thanks for creating the issue. At first I could not reproduce it. I created several feed types and for each one the CSV template matched the order of the mappings.

But then I decided to edit the mappings of a feed type and created a new CSV source for the first mapping.

First the mappings where this:

And that resulted into the following CSV template:

title,url,status

Then I mapping a source called "title2" to "Title (title)", and I got the following CSV template:

title,url,status,title2

I see the issues here are:

  • title2 appears at last, even though it is used on the first mapping row.
  • title still appears in the template, even if it is no longer used in the mappings.

Expected CSV template should have been:

title2,url,status

Am I correct that this is what the issue is about?

I also sense a request here to be able to change the order of mappings. That should be handled in a different issue. Or is it the order of custom sources?

🇳🇱Netherlands megachriz

I adjusted the code a bit, because I did not like that 'data' became a preserved key. The changes ensure that a field called 'data' can be set without overwriting the complete $data property.

I also expanded the test coverage a bit.

🇳🇱Netherlands megachriz

The Feeds Tamper logo is added to the project. :)

🇳🇱Netherlands megachriz

The Tamper logo is added to the project. :)

🇳🇱Netherlands megachriz

@baikho
Can you provide a test that demonstrates the bug? This way we can ensure that the bug doesn't accidentally return in the future. The test case should be added to \Drupal\Tests\tamper\Functional\Plugin\Tamper\EntityFinderTest.

Note: I moved the code to a new branch called '3517986-entity-finder-fieldable', because I had some trouble pushing/pulling from/to 8.x-1.x, which is a branch name already used in the main repo.

Production build 0.71.5 2024