We have a node with an entityreference field that only allows one other node type as reference values. When creating content in this node the autocomplete is showing valid values. However it's possible to bypass the type by pasting a valid NID in the field.
So practical example:
Title | NID | Node type
Test 1 | 1234 | X
Test 2 | 5678 | Y
When creating a node of type Z with the node reference field set to only allow node type X. It's possible to reference to node "Test 2" of type Y by pasting the value "Test 2 (5678)" into the field. Actually the title value doesn't matter, if a valid NID is supplied you can save the entity.
What I think is happening is because we supplied a valid node id it's bypassing the check for the type. Or was the "allowed types" option in the field settings never intended for validation on the save event and only for the dropdown values.
Our users discovered this when copying over the data from different environments (the node ID's differ of course on each environment) hence the weird linking occurred.
Field configuration used: Simple (with optional filter by bundle).
Comment | File | Size | Author |
---|---|---|---|
#5 | interdiff.txt | 1.24 KB | BramDriesen |
#5 | saving_entity_ignores-2894475-5.patch | 1.22 KB | BramDriesen |
|
Comments
Comment #2
BramDriesenCorrected a typo.
Comment #3
BramDriesenIf I have time today I'll try to dig deeper into the issue and try to propose a patch for it.
Comment #4
BramDriesenFinally had time to debug the issue, turns out it was introduced a long time ago. (see linked issue). Basically what's happening is when you supply a random string and a valid id like `foo (456)` of a different bundle as allowed, the `$valid_ids` array would be empty and thus skipping adding the warnings. This is only a problem when you try to add one bogus item or all bogus items. Patch attached.
Comment #5
BramDriesenActually, I was not happy with the patch. Refactored it a bit so we don't have the foreach duplicated. Interdiff and patch attached.
Comment #6
hgoto CreditAttribution: hgoto as a volunteer commentedThank you for the patch. I could reproduce the problem. Also I tested the patch manually and it works well.
Though, it may be possible to make the patch simpler. For example, replacing the following line in
entityreference_field_validate()
(the function you're patching) seems to work.What do you think?
Comment #7
BramDriesenYour change won't work because the function will then not process the ID's. The array_flip on the valid_ids array (which is empty in my use case) will fail. That's the reason why the if wrapper was added in the linked issue. So I still think my patch is the best solution.
Comment #8
hgoto CreditAttribution: hgoto as a volunteer commentedHmm. I don't understand what you told well. Which function do you mean by "the function"? And what does "not process the ID's" mean concretely?
Seeing all the code of
validateReferencableEntities()
in Entity Reference module again, it seems to always return an array andarray_flip($valid_ids)
works. Is there a case thatarray_flip($valid_ids)
doesn't work?Comment #9
BramDriesen@hgoto Check the errors thrown in the linked issue.
In my scenario there is no valid data, thus the
$valid_ids
array is empty. If we apply your change to the if we will go inside the if and then crash on this line.$invalid_entities = array_diff_key($ids, array_flip($valid_ids));
Because we then do again an array_flip() on an empty array like in the linked issue.
That, is why your solution is not going to work and I prefer my patch.
PS: With "the function" I mean the function I'm patching.
Comment #10
BramDriesenSmall bump. It's still a major issue...
Comment #11
BramDriesenWe're using this patch now for over a year in production without issues. Can this be reviewed once again and maybe put into the next release?
Comment #12
maurizio.ganovelliThe patch works well for me also; i've applied it on a production platform last week and no problem so far.
Comment #13
webadpro CreditAttribution: webadpro commentedWAnted to chip in saying the patch works for me also.
Comment #14
BramDriesenI guess we can set it to RTBC then and just hope someone might commit it :-)
Comment #15
BramDriesenComment #16
BramDriesenUpdating to dev version. Not sure if the patch still applies.
Comment #17
drumm#2516716: Entity selection target bundle settings are not enforced looks like the same issue, and that patch has a test.
Comment #18
BramDriesenSeems related indeed.
Comment #19
minorOffense CreditAttribution: minorOffense at Coldfront Labs Inc. commentedClosing as duplicate. Feel free to reopen if you add a valid test before the other issue resolves their test problem (or preferably fix their test).