It appears that when an entity maps to the same taxonomy from several different fields, and the Feeds importer is configured to auto-create terms, some of the reference fields might not be populated if they contain the same value in the source data.
Steps to Reproduce
- Create a taxonomy vocabulary called "Values".
- Create a content type called "Test".
- Create a taxonomy reference field in the "Test" content type called "Test reference 1" that points to terms from the "Values" vocabulary.
- Create a second taxonomy reference field in the "Test" content type called "Test reference 2" that points to terms from the "Values" vocabulary.
- Create a CSV feeds importer that imports nodes of type "Test".
- Map CSV field "test1" to the "Test reference 1" field, and enable "Auto create: Create the term if it doesn't exist."
- Map CSV field "test2" to the "Test reference 2" field, and enable "Auto create: Create the term if it doesn't exist."
- Import a CSV file with the following data:
test1 test2 Alpha Beta Kiwi Kiwi
Expected Results
Two nodes are created with the following data:
- Node 1 - Test reference 1: Alpha, Test Reference 2: Beta
- Node 2 - Test reference 1: Kiwi, Test Reference 2: Kiwi
Actual Results
Two nodes are created with the following data:
- Node 1 - Test reference 1: Alpha, Test Reference 2: Beta
- Node 2 - Test reference 1: Kiwi, Test Reference 2: (NULL)
Additional Notes
It looks like the allowed values for each taxonomy are statically cached per field in taxonomy_feeds_set_target(). This means that a snapshot of the allowed values is taken on the first row, and then that cached data is used when populating each additional row. The cached data gets updated for a particular field when a term is auto-created while populating that field, but caches for other fields that might point to the same taxonomy are not updated in the process. Only the cache for the field that created the new term is updated.
In the repro steps above, since "Kiwi" is not an allowed value in row 1, and the "Test Reference 2" field is not responsible for auto-creating the new term, it unnecessarily rejects the term during import. The value will get populated if the import is run a second time; this seems to be purely a consequence of a stale per-field cache.
Comments
Comment #2
guypaddock commentedComment #3
guypaddock commentedComment #4
guypaddock commentedI'm working on tests and a fix for this.
Comment #5
guypaddock commentedAttached are tests which should demonstrate the issue.
Comment #6
guypaddock commentedPrevious test patch failed for the wrong reason (missing base field). Revised tests attached that (should) fail for the correct reason this time.
I'd be running this locally but I'm having issues getting my local test setup working.
Comment #7
guypaddock commentedThe attached patch should correct the issue.
Comment #8
guypaddock commentedSetting to NR.
Comment #9
guypaddock commentedAdjusted issue title.
Comment #10
megachrizThanks for the patch and test! I hope to give it a review this Thursday.
Comment #11
megachrizThanks for the clear issue description and the step to reproduce the bug! I have followed the steps and can confirm the existence of the bug and the patch fixes it.
I've made some minimal changes in the code comments of the patch in #7 and committed that. See the interdiff for the changes.
Feed item vs Feed node
For example, you changed the comment
// Add term reference field to feed item bundle.to// Add a term reference field to the "article" node bundle.. What was meant with "feed item bundle" and "feed node bundle" is the difference in two ways content types can be used by Feeds. One can use a content type to attach an importer to, which means that a node of type needs to be created for importing items (useful to import multiple sources of the same type or to allow your editors to perform imports). These type of nodes are called "feed nodes" in Feeds terminology. The "feed item" refers to the type of the data to create, this is the bundle you select on the processor settings. I reworded the comments a bit to hopefully make this more clear.Comment #13
guypaddock commentedAll of your changes LGTM as well. Thanks, @MegaChriz!