🇳🇱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?

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.

Thanks for your contribution! I merged the code.

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

The Tamper logo is added to the project. :)

@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.

megachriz → changed the visibility of the branch 8.x-1.x to hidden.

megachriz → changed the visibility of the branch 3517986-entity-finder-plugin to hidden.

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

I think that we have enough tests to start with.

It took some effort, but all tests are passing again! Merged.

megachriz → created an issue.

I made a start with writing the tests. The tests are currently failing though.

megachriz → changed the visibility of the branch 2928904-add-a-mapping to hidden.

megachriz → changed the visibility of the branch 2928904-add-a-mapping-test to hidden.

No more cspell errors! Merged.

megachriz → created an issue.

I merged the changes!

megachriz → created an issue.

Merged.

megachriz → created an issue.

I experienced this issue too when working on tests in 🌱 [markdown] Add Tests Needs work . In the test I've used the following workaround:

$config = $this->parser->getConfiguration();
$config['render_strategy']['type'] = RenderStrategyInterface::NONE;
$this->parser->setConfiguration($config);

I think that the default config should be loaded as soon as using \Drupal::service('markdown') or, as the test uses, \Drupal::service('plugin.manager.markdown.parser')->createInstance('commonmark') and not at the moment getRenderStrategy() is called. So therefore I move the status of this issue to "Needs work".

I experience that a cache rebuild is enough to make the parser available. I think that that behavior is okay, though it would be cool if that is mentioned on /admin/config/content/markdown as also said in #3259945: Help text at /admin/config/content/markdown should say that a cache clear is needed to pick up new parsers → .

Shall we close this one as "works as designed"? Or does a cache rebuild not resolve the issue for you?

I can no longer reproduce this issue. Maybe it has been fixed in the mean time. In 🌱 [markdown] Add Tests Needs work I added test coverage that follows the steps listed in the issue summary and the test passes.

Feel free to reopen if this is still an issue.

I've written a few tests to render a text with each markdown parser. Only for the CommonMark PECL parser I marked the test as incomplete because that one requires a PECL extension. Without the markTestIncomplete() call, the fails with the error Call to undefined function CommonMark\Parse().

The proposed Feeds Tamper logo in 📌 Create logo for Feeds Tamper for Project Browser Initiative Active :

I've updated the Feeds Tamper logo design by using the purple design of the Tamper logo and I made the basket purple accordingly:

The proposed Tamper logo in 📌 Project Browser: Create logo for Tamper Needs review :

I still like the purple variant of the Tamper logo a bit better, but I did make the circle a bit bigger and greener. In the previous version I think that the green was a bit to light. The color looked less well in the Feeds Tamper logo.

I committed the logo as logo.png on the project.

I reverted a commit. I think it would also be a good idea to revert anything that tries to fix deprecations from Markdown itself. However, if fixing these deprecations is necessary for Drupal 11 support, they should of course be done, but even in that case I think it would still be better to handle these in an other issue first. Because it looks like that some attempts to fix these deprecations here introduced new bugs.

I got a fatal error on a D10 site:

Error: Call to undefined method League\CommonMark\CommonMarkConverter::convert() in Drupal\markdown\Plugin\Markdown\CommonMark\CommonMark->convertToHtml() (line 183 of /Users/youri/Websites/drupal/modules/wip/markdown/src/Plugin/Markdown/CommonMark/CommonMark.php).

Is it possible that the code drops support for CommonMark v1? And is that necessary? I would say that should be handled in ✨ Add support for Commonmark v2 Active . So I think that commit 3283349 should be reverted.

GitLab CI has been enabled, though only for D10 testing for now. D11 testing should be enabled in 📌 Automated Drupal 11 compatibility fixes for markdown Active .

I've added test coverage. However, in order to make InstallableLibrary unit testable, I had to make some more changes to that class. The class lacks dependency injection.

There is a BC break in HttpClientTrait. That's not great, but is a consequence of not using dependency injection.

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

In 3463119-abort-validation I've added code to abort validation of the Markdown settings and also remove the Markdown settings from the form state when the markdown filter is not enabled. In my case, this allows a text format with CKEditor enabled but with Markdown disabled to be saved again.

It does not solve the underlying error, because the reported error message is still there when you both enable CKEditor and Markdown. But it looks like they aren't meant to be enabled together.

This should fix 🐛 Unable to save text format without enabling Markdown filter Active as well.

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

Good idea indeed. I merged the change.

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

I experience this issue when I haven't any markdown libraries installed. When I have the library "league/commonmark:^1.6" installed, I experience 🐛 Error when saving text format - configuration property id doesn't exist Active instead.

I've implemented a different approach in 3470570-remove-non-config-keys-from-form-state, because I'm not sure if it is correct that ParserConfigurationForm should be assumed to only be used in Filter form context. So instead, I tried to fix it within the FilterMarkdown class.

Also added test coverage that was meant for 🐛 Error when saving text format - configuration property id doesn't exist Active , but that test passes as long as "league/commonmark" is not installed, apparently.

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

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

I was able to reproduce the error this way:

1. On a text format, set the Pathologic filter after the Markdown filter
2. Try to render a text using the configured text format.

I added an unit test (oof) and I merged the changes!

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

I applied for co-maintainership and I merged the changes! Thanks all!

Additionally, I credited "dan kolbas" who had worked on a fix too in 🐛 SubformState contains 2 abstract methods Closed: duplicate (as requested by @tyler36 in #6).

The only thing I can see on the screenshot is that it says it is operating in maintenance mode, but I doubt that's the cause: I tried to import a RSS feed while in maintenance mode and that worked fine for me. I also do not think that this issue is caused by certain Feeds configuration.

When I search on the web for the given error, it hints me that it might be related to apache configuration.
https://stackoverflow.com/questions/10873295/error-message-forbidden-you...

The only other thing I can think of is that Feeds may not be able to write or read from certain directories, like private://feeds/in_progress.

We could perhaps also delete SubformState and SubformStateInterface from the BcSupport folder, because it says it was supposed to be removed from Markdown 3.0. But to not delay this issue on that, we could also handle that in a follow-up.

I've added basic test coverage. To demonstrate that the tests fail without the fix, I've added a separate MR that should not be merged.

Note: there does exist a GitLab CI feature for running tests only, but I think that feature works only with the current Drupal version (which is Drupal 11) and Markdown isn't compatible with Drupal 11 yet. So therefore I created a separate MR for it.

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

I experience this issue too. But with error reporting on, I get the following error:

Fatal error: Class Drupal\markdown\BcSupport\SubformState contains 2 abstract methods and must therefore be declared abstract or implement the remaining methods (Drupal\Core\Form\FormStateInterface::setIgnoreDestination, Drupal\Core\Form\FormStateInterface::getIgnoreDestination) in /web/modules/contrib/markdown/src/BcSupport/SubformState.php on line 16

For that error, there already is an other issue: 🐛 Subformstate incorrect interface error Active
So assume that this issue is a duplicate of that one.

@poakpong
Feel free to reopen this issue if the reason that you saw a blank page was caused by a different error. (Check the logs on /admin/reports/dblog after you faced the blank page or enable error reporting on /admin/config/development/logging.)

I agree that this is useful. On a site that I use Commerce Abandoned Carts on, I've embedded a table in the CAC email template. That table consists of two columns: the left column contains product images, the right column the product names with a short description.

For each product, I use a custom view mode that I called 'mail'. A template for a certain product type looks like this in my case:

<tr valign="top">
  <td>{{ product.field_product_image }}</td>
  <td>
    <h2>{{ product.title }}</h2>
    {{ product.field_auteurs }}
  </td>
</tr>

I think that how I implemented displaying products in the CAC mail is a bit to fancy to use as a default, as it would require a lot of setup. We could indeed go for a simpler approach as default, like you propose in the issue summary.

I see that this issue is addressed in 📌 Automated Drupal 11 compatibility fixes for markdown Active , so closing this as a duplicate.

See this line from MR !37:
https://git.drupalcode.org/project/markdown/-/merge_requests/37/diffs?fi...

@finex
I did not manually test the 2.1.x version on Drupal 11 yet, while the dev version says it is Drupal 11 compatible. See latest comment from 📌 Add Drupal 11 support for 2.1.x branch Needs review . This the main reason that I did not create a release yet.

Additionally, in #2993374-9: Use queue for sending emails → there were some suggestions for improvements that would be nice to haves for 2.1.x. I had suggested to open new issues for these before closing the issue. Not for every suggestion a new issue was created yet.

I also liked ✨ Link to the cart in the email could be non-session dependent Needs review to get in 2.1.x, but that one is also a nice to have.

@bkosborne
Thanks for your contribution. This could be a viable solution too, though it is possible that it will no longer be supported in a future version of PHP (PHP 9?). I'm not sure yet though if it is going to be removed, it's not clear from https://www.php.net/manual/en/language.oop5.properties.php#language.oop5...

Meanwhile, I've updated the other MR:

• I've addressed my own remarks from #11.
• I made sure that the data property cannot be get or set directly. An UnexpectedValueException will be thrown when trying to do so. Added test coverage for this change.

Thanks for your contribution! It looks like a valuable addition to me.

Do you want to add test coverage for it too? This way we can ensure that no mails are being sent when the option is enabled.

I'm also thinking if it would be better UX to reverse the option label, so that it says 'Send abandoned cart emails' (and have it enabled by default). Because I believe that I read somewhere that enabling an option to disable a feature can be confusing.

@hswong3i
I understand that people could need this and ✨ File target: add support for local file path as source Needs review together, but for keeping the issues focussed, it is better to keep them separated. Instead, maybe it is better to open a separate branch called '2928904-2968671-media-and-local-file-do-not-merge' for that.

For people would like to see this issue to be resolved, we need to have automated tests - and there appears to be an issue with updating existing configuration. See Remaining tasks for details.

Thanks for fixing! I enabled PHP 8.4 testing and I see that the Role Delegation module has a similar issue. But there also exists an issue for that already: 📌 Nullable types must be explicit Active .

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

Great, tests are passing again. Merged the code.

megachriz → created an issue.

Can it be that there exists a content entity type here that violates \Drupal\Core\Entity\EntityInterface contract? Allowed return types for \Drupal\Core\Entity\EntityInterface::label() are string, \Drupal\Core\StringTranslation\TranslatableMarkup or null.

With the entity types I tested, all labels are instances of \Drupal\Core\StringTranslation\TranslatableMarkup.

It is the case that \Drupal\tamper\Plugin\Tamper\EntityFinder::getEntityTypes() returns a multidimensional array, but this is intentional. The list of content entity types are grouped by provider:
$entity_types[$entity_type->getProvider()][$machine_name] = $entity_type->getLabel();

This makes it easier to find the content entity type that you want to use:

@jens peter

I cannot understand why this is not sorted a long time ago

Well, it is because I'm unable to address every issue. Feeds has more than 800 open issues and I also am maintainer of a few other projects on drupal.org. While I would like to get everything fixed, I can only do so much. 🤷‍♂️

but guess the developers are from the USA

Nope, I'm from the Netherlands. 🙂 I import date values too, but apparently I'm not affected by this issue. I think that it is PHP who assumes American date formats by default.

If you like to get this issue addressed, you can help in the following way:

• By providing an automated test that demonstrates the issue.
• After that, by providing a fix that make that test pass.
• By joining the weekly Feeds meeting on Slack and help with reviewing or testing other issues that are being discussed during that meeting. The meeting is every Thursday on 19:00 UTC from November till March and 18:00 UTC from April till October.

@GGH
I see that PHPCS fixes are already being picked up in other issues, for example in 📌 Fix remaining phpcs, cspell issues Needs review and 📌 \Drupal calls should be avoided in classes, use dependency injection instead Needs review . Maybe not worth of your time to do that here again. Also, you are adding commits to a branch that can get wiped out by the update bot, see issue summary:

Warning: The 'project-update-bot-only' branch will always be overwritten. Do not work in that branch!

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

Coding standard fixes have been done. Setting to "Needs review" for now, until it is clear what changes exactly are requested in #64.

All Tamper from the D7 version of Feeds Tamper plugins are ported!

I've opened a follow-up for adding an additional configuration option (called "Return field") for the Entity Finder plugin: ✨ Entity Finder: add return field option Active

The idea comes from @hepabolu who added this option to a plugin similar to Entity Finder in https://github.com/hepabolu/feeds_tamper_lookup_entity, so it would be great if we could port that option to the Entity Finder plugin.

megachriz → created an issue. See original summary → .

I merged the code! Thanks all who contributed to this issue.

I've rerolled the patch from #62 into a MR and I made the following changes:

• Updated trait with changes from SimpleGMapFormatter.
• Use attribute instead of annotation for SimpleGMapAddressFormatter.
• Removed unused .orig file (simple-gmap-output.html.twig.orig).
• Updated tests.

Probably needs coding standard fixes (phpcs, phpstan, etc.).

@smustgrave
What's the duplicate code that you refer to? Most code of SimpleGMapFormatter is moved to a trait, so SimpleGMapAddressFormatter can make use of it. This is done to reduce code duplication. But perhaps I missed code duplication somewhere else?

Yes, with Feeds it is possible to update content that was not originally created by Feeds. In order to update content, you need to mark at least one mapping target on the mapping page as unique. Furthermore, the CSV column names configured on the mapping page needs to match exactly with the column names used in the CSV file.

How does your source look like? How did you configure your feed type? Is there a mapping target that you configured to be unique? If so, which one?

Needs a reroll (and MR) now that 🐛 Importing new files fails when "Autocreate entity" option is turned on. Active is in.

Also, I suggested a change, see remaining tasks from the issue summary.

Thanks ad0z for your contribution! I made a few more changes and I merged the code.

#3316065: Images imported from external feed often map to the incorrect node entity when several feeds are imported during the same cron run → looks like a duplicate of this one. For now I'm just linking it as a related issue.

This needs a test to demonstrate what is exactly going wrong. After that, we can work out what the correct way is to fix it.

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

It looks like that 🐛 URL validation to accept `public` and `private` urlscheme Fixed is resolved, so this is probably no longer an issue here.

No further info provided.

No further info provided.

No further info provided.

No further info provided.

Well, that demonstrates at least that the module was not properly installed. This error says that a database column is missing in the feeds_feed table.

With phpmyadmin I've exported all Feeds Log tables and the feeds_log field for the feeds_feed table. See attachment.
After you have ran this SQL, I expect that you can properly uninstall and then install Feeds Log again.

I hope this fixes your issue. If not, feel free to reopen.

I merged the code! Thanks all who contributed to this issue.

The only one remaining to be merged is 📌 D7 EFQ Finder to D8 Entity finder Needs work ! I plan to merge that one later this week.

Thanks for reviewing @ericgsmith! I merged the code.

Can you share your configuration and the source data?

There is a known issue at least with the Rewrite plugin that it cannot iterate through values properly. See #3224959: Rewrite plugin: iterate through values to rewrite → .

Did you perhaps use the other Feeds Log → module before? If so, try to reinstall the Feeds Log module that comes with Feeds.

Reinstalling the Feeds Log module is what you can try first anyway.

I noticed this too. There appears to be a flaw in the design for making this intuitive. I found out that if I manually edit the weights of the Tamper instances, you can get a chain like this work in the right order.

The flaw is that you order Tamper instances per source. But if tampering one source is dependent of tampering of an other source, then you would need to manually adjust the weights to get things executed in the right order. So the weight of the tamper of source one would for example need to be 0 and then the weight of the replace tamper would need to be 1. If they are both 0, it is possible that the replace tamper gets handled first.

Suggestions for making this process more intuitive are welcome. 🙂
I am thinking of a preview feature myself, but I'm not sure yet how that would need to look like in the Feeds Tamper interface. And if it makes sense to provide a preview feature there. Because my idea was to add a preview feature to the mapping page that would look like the Feeds Import Preview → module that I wrote for the D7 version of Feeds.

I found the issue that introduced renderRoot() in the code:
#2969259: Drush cron : can't print error messages →

@jrochate
I added the task "When importing media images via a media field on a node, make sure that the image is visible on the media edit form." to the remaining task list. Does that summarize the issue well?

@jurgenhaas
Thanks for the review! Not for every mode iteration happens. Only for the modes where 'handle_multiples' is set to FALSE. See Encode::getModes().

The following modes have set 'handle_multiples' to TRUE. These modes accept arrays as input:

• serialize
• json_encode
• yaml_encode

When for these modes an array is passed, the array will be passed directly to the function and not be iterated over.

The following modes have set 'handle_multiples' to FALSE. These modes do not accept arrays as input.

• unserialize
• json_decode
• base64_encode
• base64_decode
• yaml_decode

When for these modes an array is passed, there will be iterated over the array and each value of the array will passed to the function.

Demonstration

Given the following input:

$input = [
  [
    'title' => 'foo',
    'body' => 'footsie',
  ],
  [
    'title' => 'bar',
    'body' => 'barkruk',
  ],
  [
    'title' => 'qux',
    'body' => 'quxious',
  ],
];

When executing var_dump($plugin->tamper($input));:

• 'serialize' will return
  

  string(177) "a:3:{i:0;a:2:{s:5:"title";s:3:"foo";s:4:"body";s:7:"footsie";}i:1;a:2:{s:5:"title";s:3:"bar";s:4:"body";s:7:"barkruk";}i:2;a:2:{s:5:"title";s:3:"qux";s:4:"body";s:7:"quxious";}}"
  

• 'json_encode' will return
  

  string(100) "[{"title":"foo","body":"footsie"},{"title":"bar","body":"barkruk"},{"title":"qux","body":"quxious"}]"
  

• 'yaml_encode' will return
  

  string(105) "-
      title: foo
      body: footsie
  -
      title: bar
      body: barkruk
  -
      title: qux
      body: quxious
  "
  

base64_encode actually fails on the above, because it cannot handle multidimensional arrays, but for the following input:

$input = [
  'title' => 'foo',
  'body' => 'footsie',
];

It returns:

array(2) {
  ["title"]=>
  string(4) "Zm9v"
  ["body"]=>
  string(12) "Zm9vdHNpZQ=="
}

But the point is, modes 'serialize', 'json_encode' and 'yaml_encode' will always result into a string if an array, a scalar value or null is passed (classed objects might throw errors, depending on their implementation).

Yes, I got some of these errors as well. I don't know the cause of the file permissions error, but the others are deprecation warnings. This means that some of the code will no longer work in future PHP versions, probably in PHP 9.
So it is wise to make a plan for moving off from Drupal 7 if you can, because at some point things will no longer work, unless you can keep using older MySQL and PHP versions.

I' happy to merge MR's if you can confirm that these don't break things. This way there would be a tiny chance that things keep working on future PHP/MySQL versions. I say tiny, because you'll probably face similar errors in other D7 modules. That could potentially become a lot of work to maintain. Well, PHP 9 isn't here yet, so you might be good for a while.

libxml_disable_entity_loader() is somewhat easy to fix, and I think there was already a fix for this made in 🐛 Function libxml_disable_entity_loader is deprecated on PHP8 Fixed . It looks like that this did not got into a release yet.

I just tried to import a simple XML file using the XML Xpath parser and that worked fine. (PHP 8.3.10)

I have meanwhile ran the remaining tests and these did not fail on my local install.

When I try to load your importer on a D7 test site, I get the following warnings:

• The plugin feeds_fetcher_directory_fetcher is unavailable.
• Processor: Invalid value music_artists for config option Bundle.

I see that "feeds_fetcher_directory_fetcher" is coming from Feeds directory fetcher → . I don't know if that module got PHP 8 compatibility.

Can you perhaps try if an import does work with a different fetcher? For example, with the File upload fetcher? That way we can perhaps rule out that the issue is with the Feeds directory fetcher module.
According to #2537090: Clarify differences between this module and feeds built-in fetcher → , I see that the file upload fetcher can also fetch from a directory if you enable the option "Supply path to file or directory directly".

If you like me to try to replicate the issue, can you provide your importer in a feature module and add a sample file to import?

Have you got any ideas why this is not working anymore? Or how to enable and where to find maximum logs where I can find out what is wrong?

No, I don't know yet. If the errors are not in the Drupal logs, you could try if you can find any messages in the server logs. I experienced that not every error ends up in Drupal's watchdog. On my local machine I can find the server logs in /opt/homebrew/var/log/httpd/error_log. I see there why the Rules test failed, for example: [Thu Feb 06 20:56:36.686383 2025] [php:error] [pid 9646] [client ::1:63512] PHP Fatal error: During inheritance of RecursiveIterator: Uncaught Error: Class "RulesActionContainer" not found in /Users/youri/Sites/drupal/modules/d7/wip/rules/includes/rules.plugins.inc

I just tried to run all D7 tests on a local site on PHP 8.3.10 and I noticed the following:

• Most tests that ran were passing.
• The following tests failed:
  • Fetcher: HTTP
  • Mapper: File field
  • Rules integration
• At some point, the test runner crashed. I think it was at about test class 36 of 46 (probably the Rules integration test). So 10 tests did not run yet.
• There were lots of deprecation notices like 'Creation of dynamic property' and 'Optional parameter $data declared before required parameter $info is implicitly treated as a required parameter'.

So this implies that there are problems with the HTTP Fetcher and with mapping to file fields on PHP 8.3.

Maybe that I run the remaining 10 tests tomorrow, now it's time for me to head to bed.

Unfortunately, automated testing for Drupal 7 looks like to be disabled (or I can no longer find it). What I can tell, there have been changes to Feeds to make it compatible with PHP 8.2 (see notes of the latest release → ). What I can remember from the last time I looked at the test results, is that there were some errors in modules that Feeds depend on (Job Scheduler) or integrate with (Date). I think that these errors were only PHP notices, not things that would break Feeds.

So that being said, I expect that the D7 version of Feeds should work on at least PHP 8.0, PHP 8.1 or PHP 8.2. Not sure about PHP 8.3 or newer.

I hope I can mark the D7 version of Feeds as supported again soon. Because I still need to maintain a few D7 sites for some time longer and on one of these Feeds is being used. The support would be quite minimal though, like responding to issues like this one.

If you haven't done so yet, I recommend to install the D7 security client module. Some D7 projects will still receive security support by a community effort. I plan to join the effort to get Feeds as one of these on the list. Read more about this at https://www.d7security.org/

Closed this issue as a duplicate of #3193610: Items going missing → .

If you still encounter this issue and you think it is not related to a configuration error, feel free to reopen this issue or leave a comment to request it for being reopened.