We can pretty easily provide language support for any entity.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | interdiff-2529538-11-13.txt | 1.64 KB | megachriz |
| #13 | feeds-generic-entity-language-2529538-13.patch | 21.56 KB | megachriz |
| #11 | interdiff-2529538-8-11.txt | 840 bytes | megachriz |
| #11 | feeds-generic-entity-language-2529538-11.patch | 21.52 KB | megachriz |
| #8 | interdiff-2529538-6-8.txt | 18.51 KB | megachriz |
Comments
Comment #1
twistor commentedComment #3
twistor commentedComment #4
megachriz@twistor
For my understanding, so I know where to focus on in this issue, does this issue deals with all the issues that I had found with the language setting on the processor in #1183440-234: Multilingual Feeds - Make field import language-aware or only some of them? Additionally, I think I see in the code that if the processor's language is set to a language that is not enabled, Feeds will import the items in LANGUAGE_NONE instead of the configured language. If so, this would answer my question if Feeds should import items in languages that are not available (answer being "no").
My assumption now is that this issue is aimed to fix the following:
Let me know, then I know which automated tests I can write for this issue.
Comment #5
twistor commentedYou've got it, except for one point:
Entity translation doesn't enable entity language support for entities that don't have it. Confusing, I know. It is more of a field translation module.
As to whether we should import disabled/removed languages. That's debatable, so if you have any opinion, let me know. My stance is to avoid invalid configuration if possible.
Comment #6
megachrizHere are some tests using the language setting on the node processor. I hope to write tests for the taxonomy processor later.
The tested scenarios are based on the manual tests I did earlier in #1183440-234: Multilingual Feeds - Make field import language-aware. The following scenarios are tested:
The tests are currently failing locally. The only thing that failed was that Feeds still imports in a language when the locale module is disabled. All the other scenarios are passing! I do have to verify though, if the tests are making the right assertions.
I imagine me two possible "bug" reports:
In conclusion, I think it's better that Feeds doesn't import entities in non-existent language. It would be good though to validate the importer configuration when the importer is installed (either as default importer [in a Feature module], or when using the "Import importer" feature). I think importer validation would be out of scope for this issue. This should probably be handled in #2320781: Validate feed importer configuration: check for invalid bundle and invalid language instead.
Comment #8
megachrizAnd here are also tests for importing multilingual terms. I figured that for node and term some performed tests are the same, so I created a base class called
Feedsi18nTestCasefor testing multilingual entities and two classes that extends that class: one for node and one for term.The test for multilingual terms depends on i18n_taxonomy (thanks twistor for pointing out that the Entity translation module doesn't make vocabularies translatable. That's one complexity less). It's possible that the testbot doesn't execute that one cause I believe the testbot doesn't check the patch for additional dependencies. If this is the case, I will do a separate commit just for adding that extra dependency (since I can do commits now, yay!).
Let's see how this goes. Will there still be just two failed assertions, like in #6? We'll see.
Comment #11
megachrizI've looked at the reason why Feeds is still asigning a language when the locale module is disabled and it appears to be that
language_list()returns a list of languages even when the locale module is disabled. Therefore I added a check for the availibility of the locale module toFeedsProcessor::entityLanguage(). The reason whylanguage_list()returns a list of languages when the locale module is disabled is that it must be able to return these early in the bootstrap process at a point that this module isn't yet loaded. See #173227: Optional languages not available.I've added the test dependency to i18n_taxonomy now in a commit in order for the testbot to execute also the test "Feedsi18nTaxonomyTestCase".
Comment #13
megachrizThe reason that the test failed was that Feedsi18nTestCase::setUp() tried to set the processor settings at a point were no taxonomy vocabularies yet were available, thus resulting in a test failure. The attached patch moves setting the processor settings to Feedsi18nNodeTestCase and Feedsi18nTaxonomyTestCase. Let's see if this fixes all the test failures.
Comment #15
stefan.r commentedThis seems like a nice patch, at first sight this looks good. I don't see any big problems and tests make sense.
Comment #17
megachrizCommitted! Thanks all. In the commit I have also credited contributors from #1183440: Multilingual Feeds - Make field import language-aware from which I think had a part on this side of the language support issue (language support on the processor).
On to the second part of the language support issue!
#1183440: Multilingual Feeds - Make field import language-aware