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:
- On a text format, set the Pathologic filter after the Markdown filter
- 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.
Is there a clever way to debug failed imports that I don't know about?
For debugging failed imports, you could activate the Feeds Log module (included with Feeds). This way you can see which items fail exactly. (If you have a lot of imports, be frugal on for which feed types you keep logging active, because Feeds Log can generate a lot of data.)
This issue is related to validation that happens during an import.
- Feeds performs validation to prevent invalid values from being imported. For example: if a list field only allows A, B and C, then the import of an item should fail if it tries to set D.
- Feeds also performs validation to minimize the chance of SQL errors happening, which are less friendly messages to display to users.
- The validation that some modules do depend on the current logged in user. So these modules assume that user performs an action in the UI, they don't account for the possibility that validation can also occur programmatically, like is the case for Feeds.
- During cron runs, the current logged in user is anonymous. This was problematic for Feeds users, because that user has usually less privileges and users often ran into validation issues because of the point above.
- To fix issue 4, a user switch to the feed owner was implemented during cron runs. The feed owner was chosen because during cron runs it is unknown which user exactly initiated the import.
- Users reported bugs that some imports ran fine in the UI, but not during cron - or vice versa.
- To fix issue 6, a user switch to the feed owner also happens when running imports in the UI. This way, the validation that happens during UI imports and cron imports is the same and that makes things consistent.
So, long story short, I think you ran into validation errors because one of the feed owners is blocked and therefore some modules think that a certain user is trying to do something it is not allowed to because they are blocked (or perhaps doesn't have the needed role anymore).
The origin of the issue is - in my opinion - that some validation depends on the current logged in user. I think that this cannot be fixed in Feeds or I don't know how to. So I think that you ran into an edge case here that is there due to limitations in the system.
You do have the possibility to configure on the feed type to skip validation (and also to only skip certain types of validation), but then you risk the import of invalid values or imports that are crashing because of SQL errors.
I hope this answers your question. Feel free to reopen this issue if you have follow-up questions. 🙂
Thanks for the review, I merged the code!
It looks like that this issue applied to coding standards for D7 only, so I think that it is outdated.
@kopeboy
I think you are seeing Tamper only as part of Feeds, while it is a standalone module.
You are somewhat right in this, but not completely. I try my best to make all Tamper plugins work outside of the Feeds context. There are functional tests for the configuration forms to ensure things can be configured so that they do not assume to be used with Feeds Tamper only. At the same time, when it comes to manually testing the plugins, most of them I only have tested using Feeds and Feeds Tamper. A few I have also used in a Migrate context with the the Migrate Tamper module (currently part of Migrate Plus Feeds → ).
I'm open to changes for the Hash plugin. I do think that when making the changes the md5()
method should be preserved, because else it would break backwards compatibility. So adding an option for other hash methods is fine, but probably md5 would need to be the default.
✨ Improve handling of empty data Active
This is addressed in ✨ Improve handling of empty data Active
Thanks for your contribution! It looks like that your issue is already handled in
✨
Improve handling of empty data
Active
. There is test coverage there for passing a NULL
value for every plugin, including the Boolean plugin. There is a similar fix applied to the Boolean plugin there as well.
So therefore I think this should be closed as a duplicate.
Hm, I thought I had fixed this issue sometime ago. Apparently this was double reported. So this is actually a duplicate of #3193610: Items going missing → .
Also some issues related to this one have been fixed a long time ago:
#3193610: Items going missing →
#3132198: Clean queue tasks when unlocking a feed to prevent potential data loss →
@anaconda777
Can it be that your issue is a configuration error?
Have you configured on the content type that new nodes should not be published by default?
Or are you mapping to "Published (status)" on the Feeds mapping page?
@ad0z
Thanks for your contribution! I think that you almost got it right. In EntityReference::defaultConfiguration()
the autocreate options are still defined (see the first lines of the method).
@ptmkenny
Thanks for the feedback! I think that adding an option to the Math plugin would then be best. Something like "Skip calculation when data is not numeric".
A fix has been implemented in ✨ Create separate Decode-plugin RTBC . Reviews there are welcome!
Updated issue title to reflect the current approach that is implemented.
I've now implemented a fix that makes the plugin handle multiples themselves. If an array is passed, but the selected encode method does not accept arrays, then the plugin iterates over all values.
I think that this approach is less desruptive for existing users of the plugin.
Let me know what you think of my approach to fix this issue and tests or code reviews are welcome! I'm aiming for a new Tamper release mid/end February, so it would be cool if you are able to test or review before that.
In order to make a target for this field appear, you'll need to write a FeedsTarget plugin.
Feeds only aims to support targets for fields provided by Drupal core, so therefore I'm moving this issue to the Time Field issue queue.
There appears to already exist an issue for that there, so closing this issue as a duplicate as well.
✨
TimeRange feeds compatibility
Needs review
After installing the patch I get the following error:
ValueError: Value "breakpoint" of "#modal_type" is not allowed in multiselect dropdown "edit-rie". in template_preprocess_multiselect_dropdown() (line 159 of modules/contrib/multiselect_dropdown/multiselect_dropdown.module).
call_user_func_array('template_preprocess_multiselect_dropdown', Array) (Line: 261)
Drupal\Core\Theme\ThemeManager->render('multiselect_dropdown', Array) (Line: 491)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 504)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 504)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 248)
Drupal\Core\Render\Renderer->render(Array, ) (Line: 238)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 231)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 128)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
call_user_func(Array, Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.view') (Line: 186)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 741)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
The form element is used in custom code and looks like this:
$form['fieldset_filter']['rie'] = [
'#type' => 'multiselect_dropdown',
'#title' => $this->t('Select'),
// '#default_value' => $this->getSelectedOptions($items),
'#options' => $rie_options,
'#label_aria' => 'Toggle the list of items',
'#label_none' => 'No RIE\'s selected',
'#label_all' => 'All RIE\'s selected',
'#label_single' => '%d RIE selected',
'#label_plural' => '%d RIE\'s selected',
'#modal_breakpoint' => ModalType::Modal->value,
'#ajax' => [
'callback' => '::getRieModulesAjax',
'disable-refocus' => FALSE,
'event' => 'change',
'progress' => [
'type' => 'throbber',
'message' => $this->t('Fetch modules..'),
],
'wrapper' => 'rieriemodules',
],
];
If I replace '#modal_breakpoint' => ModalType::Modal->value,
with '#modal_type' => ModalType::Breakpoint->value,
in the custom code, then the error is resolved and the dropdown renders as desired.
Based on my findings above, it looks to me that requiring "#modal_type" is an API change. I don't know enough about the module whether or not that is a desired adjustment. But if it is, it needs at least be documented in \Drupal\multiselect_dropdown\Element\MultiselectDropdown
. In the docblock there is a code example where #modal_breakpoint
is used instead of #modal_type
.
But I do wonder if requiring "#modal_type" is correct here as it apparently can break other modules that are not using the widget, but just the "multiselect_dropdown" form element.
In #105
if (preg_match('/^Autocreate/', $line->__toString())) {
__toString doesn't seem to be a valid method on render array.
@max.valetov Thanks for spotting! I think that the issue with the "Autocreate" option should be fixed in 🐛 Importing new files fails when "Autocreate entity" option is turned on. Active first. I think that one is easy to do.
Sorry for the late reply, I somehow did not get an email notification about this issue.
This is probably caused by the "minimum-stability" setting in the composer.json file in your Drupal root folder. By default, this setting is set to "stable". Because of that, Composer won't download the Tamper module (which is dependency of Feeds Tamper), because Tamper has no stable release yet.
To fix that:
- Either set "minimum-stability" to "alpha" in the composer.json of your Drupal root folder (because Tamper is currently in alpha);
- Or composer require the Tamper module as well with the following command:
composer require 'drupal/tamper:^1.0@alpha'
I hope that this helps!
I've checked again. I added code to support base urls that do not include the scheme. So previously, "example.com" was marked as incorrect, because parse_url()
does not see that as a url that contains a domain name. With the regular expression /^([^\/]+\.[a-z]+)\//i
, the code can accept "example.com" as a base url. Tests cases are added for this and it is checked that "www.example.com", "example.com/cat" and "example.com/foo.bar" are correctly parsed as well.
There is one remaining unresolved thread and that is that there is a inconsistency with other Tamper plugins in that it tries to convert the incoming data to a string, where other plugins would throw a TamperException. I'm not sure if that is a problem or if it is worth it to let this issue hang because of that. In my opinion, at least in the context of Feeds, I think it is a good thing if the Tamper plugin tries to be "friendly" and just see if it can use the data. Perhaps the other Tamper plugins need to be changed to become friendlier to or maybe there should exist a strict mode if it is important that the source data is of the correct data type. Changing other Tamper plugins are out of scope for this issue, however.
I think that this is ready to go, although it is possible that a small change is needed after ✨ Improve handling of empty data Active is resolved.
Tests are passing! This looks ready for a new review!
Highlights of the changes:
- Column selection: if a field has more than one 'column', you can select a column. For example, a link field has "uri" and "title"; a body field has "value", "summary" and "format".
- Selecting a bundle is optional. A field can be chosen without needing to select a bundle first. Selecting a bundle does narrow down the list of available fields.
- The list of entity types are grouped by provider (usually the module defining them), making it easier to find the one you need.
- Made updating the form via AJAX work outside of Feeds Tamper context.
- Lots of automated tests.
@hepabolu
Thanks for your contribution 🙂. I see that your plugin has one feature that looks useful that Entity Finder currently doesn't have: a return field. Entity Finder now always returns the entity ID, but being able to select a different field to return sounds very useful.
I'm not sure yet if we should add it to Entity Finder right now (since this issue has been open for a very long time already) or if it would be better to add it in a follow-up. Do you want to help adding that feature here?
I'm not sure, but the url for the image on the edit page looks wrong.
Instead of
example.com/system/files/styles/medium/http/artifact-info%3A8888/sites/default/files/feeds/auctions/images/06-09-19/1_2.jpg.webp?itok=fHyV7p0l
I would expect something like this to be the url:
example.com/system/files/styles/medium/private/feeds/auctions/images/06-09-19/1_2.jpg.webp?itok=fHyV7p0l
Or
/sites/default/files/styles/medium/public/feeds/auctions/images/06-09-19/1_2.jpg.webp?itok=fHyV7p0l
Perhaps it is an issue caused by ✨ Add a mapping target to media field Needs review ?