Updated comment #237.
Problem/Motivation
Field collection does not support node translation properly. Multilingual sites using the node translation method are not able to use Field collection.
Drupal 7 has two methods for translation: the node translation method used in previous versions of Drupal, and the Field translation method that will be used in future versions of Drupal (see Drupal 7's new multilingual systems (part 4) - Node translation). For the issue covering Field Translation, see #1344672: Field Collection: Field translation (entity_translation) support..
As stated in the module's README.txt
* As of now, the field-collection field does not properly respect different
revisions or languages of the host entity. Thus, for now it is suggested to
only use the field for entities that are not revisionable and translatable.
Proposed resolution
Provide support to have Field collections work with note translation and utilize the $obj->language parameter.
Remaining tasks
* Commit the latest patch (comment #251).
* Notify users that node_clone and i18n_node is now supported and possibly link them to this issue / the fixup-script.
* Identify why FieldCollectionBasicTestCase didn't fail on comment #176
User interface changes
None at present, though some administrative configuration options may be relevant or necessary. For example: whether files used by image fields or file fields should be physically duplicated or whether translated fields should reference the same fid.
API changes
* Use of $langcode and LANGUAGE_NONE would change to be relevant to the host entity.
* Duplication of files for image fields and file fields may occur.
Original (related) reports by dbravender and FiNeX
#1156468: New translations don't inherit parent field collections
#1186878: Node reference inside field_collection doesn't work with i18n
Comment | File | Size | Author |
---|---|---|---|
#300 | field_collection-1316162-300.patch | 1.42 KB | carsato |
#251 | field_collection-1316162-251.patch | 8.75 KB | goron |
#242 | interdiff-227-242.diff | 916 bytes | torotil |
#242 | field_collection-fixup-broken-copies.txt | 3.22 KB | torotil |
#239 | Screen Shot 04-23-14 at 01.58 PM.PNG | 22.21 KB | PhilY |
Comments
Comment #1
zilverdistel CreditAttribution: zilverdistel commentedAlso check these issues. For us, solving these issues solved all our problems.
Comment #2
chymz CreditAttribution: chymz commentedHere is a quick fix for Clone field collections as new, separate entities when creating a new translation.
Create a new module with this :
Comment #3
mordonez CreditAttribution: mordonez commentedHere is a quick patch to separate entities when creating or updating a translation.
Comment #4
mordonez CreditAttribution: mordonez commentedAdding a check only for node entity types
Comment #5
Nexsoft CreditAttribution: Nexsoft commentedSuper! Works great so far! Thanks!
Comment #6
JulianH CreditAttribution: JulianH commentedPatch works only if view mode is embedded
Comment #7
JulianH CreditAttribution: JulianH commentedsomeone has a solution?
Comment #8
mordonez CreditAttribution: mordonez commentedSorry JulianH I'll take a look asap and I'll test with embeddeds views. By the way I think the last patch it's a quick win but there are much work to do with i18n fields.
Comment #9
Lukas von BlarerWorks for me. I am implementing two sites using this patch. Is this a clean solution and can I go ahead and create content using this?
Comment #10
gregglesJust confirming this works on a site I'm working on. Even if it's not perfect it seems like it helps.
Comment #11
Andreas Radloff CreditAttribution: Andreas Radloff commentedWorks for me too. Thanks!
Comment #12
elefter@alfasoftware.gr CreditAttribution: elefter@alfasoftware.gr commentedif the field collection has a field collection in it the parent collection gets a new copy but the child one doesnt
if think its cause the
if ($entity_type == 'node')
fails for the child collectionComment #13
mathiasgmeiner CreditAttribution: mathiasgmeiner commentedThanks! It works perfect!
Comment #14
drupov CreditAttribution: drupov commentedIn my case if I have a translated node with field collections, when updating one of the translations the changes applied also to the all other nodes in the translation set.
Applying the patch from #4 solved this.
Is this going to be commited?
Comment #15
Jonah Ellison CreditAttribution: Jonah Ellison commented+1 for patch #4. Here's an update to address #12 (field collections within field collections), though not sure if these is a better way to implement this.
Upping priority since this bug can cause data loss (through content overwriting).
Comment #16
fagoSounds good. Maybe we can even solve this generally though?
If a new host-entity is saved which already contains field-collections, those collections must be new too. Else, we've to clone them. This should generally solve problems with cloning entities, regardless whether its content translation, node clone or custom code. thoughts?
Comment #17
mineaeliza CreditAttribution: mineaeliza commentedI tesed the function with Field collection 7.x-1.0-beta3 and it worked! thanks!
Comment #18
Lukas von BlarerStill works... I've been using it the past few months. Can we get this commited?
Comment #19
Sborsody CreditAttribution: Sborsody commentedI'm looking for entity_translation support. This patch won't interfere with entity_translation, will it?
See #1344672: Field Collection: Field translation (entity_translation) support.
Comment #20
jdanthinne CreditAttribution: jdanthinne commentedUnfortunately, the patch doesn't work at all when Widget is "Hidden".
Comment #21
lukas.fischer CreditAttribution: lukas.fischer commentedThis patch is actually a copy of but adapted for version field_collection beta 4:
Comment #22
jdanthinne CreditAttribution: jdanthinne commentedStill not working when Widget is "Hidden".
Comment #23
iamEAP CreditAttribution: iamEAP commented@jdanthinne, could you be more specific when you say "when Widget is 'Hidden' "? Do you mean the display widget for the collection, or the display widget on fields within the collection? I can't imagine how a display widget would interfere.
Here's the same as #21 with some comment cleanup as a real patch.
I did some testing and this seems to work well for me.
Comment #24
jdanthinne CreditAttribution: jdanthinne commentedI mean that in the content type that contains a Field Collection, the field widget type is "Hidden".
Comment #25
iamEAP CreditAttribution: iamEAP commentedAh, now I'm able to reproduce.
This is because when Field Collections are set to "hidden" (rather than embedded), they're edited in a context completely separate from the host entity.
One option is to add code to check for this case in the hook_field_presave, but it might make more sense to get rid of the case entirely (e.g., when you translate content, it shouldn't automatically get the source node's collections).
Comment #26
iamEAP CreditAttribution: iamEAP commentedHere's #23 with additional code that checks for the hidden widget case.
Comment #27
jdanthinne CreditAttribution: jdanthinne commented#26 working better now with hidden widget: before this patch, the old collections (in the original language), were still linked to the translated node, so when I change one instance, it changed in both languages (unwanted!).
Now, the old collections are unlinked from the translated node (fine!), but they're not duplicated, assigned to a new language and linked to the new node… that would be nice(r) because as now, you have to recreate the collection in the translated node (instead of translating them).
Despite that workflow detail, #26 is already great!
Comment #28
mareks CreditAttribution: mareks commentedI had an issue with field-collection translations where one translation sort of over-wrote the other translations. At least this is how it looked.
Anyway, patch #26 fixed the problem for me. Thumbs up!
Comment #29
tim.plunkettI'm against adding node specific code like that. Can't this be abstracted more?
Comment #30
iamEAP CreditAttribution: iamEAP commented@tim.plunkett Do you know of other ready-made entity types that make use of the content translation (vs. entity field translation) paradigm that could be used to make/test a more abstracted patch?
Comment #31
damontgomery CreditAttribution: damontgomery commentedI am having an issue with #26, but not with #15.
The issue may have to do with this patch for field_collection, http://drupal.org/node/1239946#comment-5410010. This patch ensures that "empty" or "duplicate" field collection items are not saved every time you save / edit a node with a field collection. That patch has worked well.
I am also using Workbench with Workbench Moderation.
My field collections are using the embedded widget.
With #15, everything works as expected!
With #26, if I made a new translation of a node (content type page) with a field collection and save as "published" the first time, the reveals are all gone on the translated page. They remain on the original page. If I instead save the content as "draft" the first time, the field collections are kept. If I then change the state from "draft" to "published", the field collections are kept. The only time the field collections are removed is when I first save the translation as "published".
This isn't a huge issue, but one to consider.
I may have time to look into it a bit more. My guess though is that it has something to do with the two field collection patches and Workbench Moderation.
Comment #32
derPaolo CreditAttribution: derPaolo commentedsame as pandaeskimo. #15 works fine with the patch he mentions. thanks a lot!
Comment #33
chaskype CreditAttribution: chaskype commented#26: field_collection-support_content_translation-1316162-26.patch queued for re-testing.
Comment #34
gagarine CreditAttribution: gagarine commented#26 works for me. I didn't have problem when i save the first time like explain in #31.
In Europe translation is not a feature request... it's a critical bug ;)
Comment #35
ParisLiakos CreditAttribution: ParisLiakos commented#26 works here as well, thanks
Comment #36
mgiffordTagging
Comment #37
amandine_m CreditAttribution: amandine_m commentedSuscribe
Comment #38
JurgenR CreditAttribution: JurgenR commentedAny updates on this one?
Using patch #26 fixed the problem, how will this evolve?
Comment #39
gagarine CreditAttribution: gagarine commentedFor #26.
Comment #40
jimmyko CreditAttribution: jimmyko commented#26: field_collection-support_content_translation-1316162-26.patch queued for re-testing.
Comment #41
AaronBaumanPatch #26 corrects the user-facing behavior, but "language" is still stored as 'und' for all translations in both the field collection and the fields within the field collection.
Comment #42
iamEAP CreditAttribution: iamEAP commentedBelieve that's expected behavior. This is about the "content translation" paradigm, the fields you're referring to are related to the "field translation" paradigm; believe there's a separate ticket for this.
Comment #43
AaronBaumanCan you point me to that ticket?
I thought "field translation" referred to things like labels and help text.
Comment #44
iamEAP CreditAttribution: iamEAP commented@aaronbauman: #1344672: Field Collection: Field translation (entity_translation) support.
Comment #45
fagoI and tim.plunkett have both stated that we'd prefer a more general solution, see #16 and #29. Let's do so:
Comment #46
Morpheu5 CreditAttribution: Morpheu5 commented#26 saved my life, for now. I applied it to 7.x-1.x-dev 2012-Jul-28 and it seems to work as expected (i.e. translates the fields' content).
Comment #47
Pomliane CreditAttribution: Pomliane commentedAccording to the Priority handbook page "Issues which have significant repercussions but do not render the whole system unusable are marked major."
Comment #48
siramsay CreditAttribution: siramsay commented#26 works for me,
I didn't have the problem as outlined i #31 so I guess as @pandaeskimo say it is a Workbench Moderation problem
I am using beta 4
Comment #49
Pomliane CreditAttribution: Pomliane commentedComment #50
joel_osc CreditAttribution: joel_osc commentedI have recently updated to the latest dev release from Sept. 12 for another issue and have found that when I create a new translation the field collection does not get cloned as a new entity, meaning that when I change one node the other one changes as well - which is bad for multilingual. It seems this patch did not make it into the latest dev and the new dev release _field_presave function is quite different since the introduction of revisioning.
Comment #51
joel_osc CreditAttribution: joel_osc commentedHere is a patch for the latest dev, please give it a try as I have only done limited testing.
Comment #52
joel_osc CreditAttribution: joel_osc commentedDoh! Same patch as #51 without the dpm() in it.
Comment #53
ParisLiakos CreditAttribution: ParisLiakos commentedjust triggering testbot
Comment #54
paolomainardi CreditAttribution: paolomainardi commentedThis patch brokes the node revision revert feature:
EntityFieldQueryException: Motore di archiviazione files non trovato in EntityFieldQuery->queryCallback() (line 1180 of /Users/paolo/Zend/workspaces/DefaultWorkspace/twinbit/hart/tim/includes/entity.inc).
Comment #55
paolomainardi CreditAttribution: paolomainardi commentedPlease, find attached a patch to make this patch works also on node revisions reverting.
Comment #56
petabyte CreditAttribution: petabyte commentedA big THANK YOU! to everybody who contributed to this issue. I already feared that this will break my website's neck when I discovered that field_collection doesn't support full content translation. But then I found this issue. :-)
I only noticed one side effect: in field_collection.module on line 1611 the result of field_collection_item_load isn't tested before it's used as object. When no item could be loaded (e.g. it doesn't exist yet, what is the case for every newly created node translation) PHP raises a notice. By ensuring $entity isn't FALSE we could prevent this. Attached is a tiny patch for this particular detail.
Comment #57
gagarine CreditAttribution: gagarine commentedTheir is a problem with the test bot?
Comment #58
kytom CreditAttribution: kytom commentedIt should be noted that the patch needs the latest dev of Entity API installed, otherwise it will break the site.
When trying to save a node with a field collection:
Fatal error: Call to undefined method FieldCollectionItemEntity::isDefaultRevision() in [...]/sites/all/modules/field_collection/field_collection.module on line 538
Installing latest dev of Entity API fixed it.
Comment #59
carn1x CreditAttribution: carn1x commented#55 works perfectly for me using Embedded widget.
Comment #60
bradjones1#55 and #56 working for me as well. Though I don't think this can RTBC without a test pass? Does this need a new test or is testbot just stuck on those patches for some reason?
Comment #61
iamEAP CreditAttribution: iamEAP commentedIt's been repeated several times in this thread that any patch with node-specific code will not be accepted (#16, #29, #45); needs to be a more generic solution.
Going to mark this as needs work, as such.
Comment #62
jrglasgow CreditAttribution: jrglasgow commentedI combined #55 and #56 together in this patch which worked for me.
Comment #63
AdamGerthel CreditAttribution: AdamGerthel commentedI've tried the patch in #55, but when changing language of a node the reference to the field collections (from the node) seems to break. I.e the row in the field collection field (on the node) disappears after saving the node. The field collection is still in the database, just not referenced any more.
Comment #64
yannickooYou should remove the whitespace.
Comment #65
c4rl CreditAttribution: c4rl commentedRerolled
Comment #66
mgiffordWhat's up with the bot? It's postponed after 2+ days.
Comment #67
c4rl CreditAttribution: c4rl commentedAttaching again?
Comment #68
tim.plunkettIt's because of http://qa.drupal.org/pifr/test/125674
Comment #70
bonchak CreditAttribution: bonchak commented#55: field_collection-support_content_translation-1316162-55.patch queued for re-testing.
Comment #71
Lukas von BlarerDo we need to adapt the tests?
Comment #72
Barry Tielkes CreditAttribution: Barry Tielkes commentedSuper!
#26 worked for me.
Finally content in field collections don't get overwritten between different languages.
Thx!
Comment #73
Lukas von BlarerComment #74
amandine_m CreditAttribution: amandine_m commentedComment #75
amandine_m CreditAttribution: amandine_m commented#65: field_collection-1316162-65.patch queued for re-testing.
Comment #77
SeyV CreditAttribution: SeyV commented#65 worked perfect for me. Applied clean & fixed the problem even with collections that were previously translated with the patch in #26.
Retriggering bot.
Comment #78
SeyV CreditAttribution: SeyV commented#65: field_collection-1316162-65.patch queued for re-testing.
Comment #79
jrglasgow CreditAttribution: jrglasgow commentedI was having a problem with this patch when making a revision. When the new revision is created the field_collection gets reset to empty.
This patch solves this problem for me. It probably won't pass the unit tests, I haven't had a chance to take a look at that yet.
Comment #81
Mender CreditAttribution: Mender commentedPatch #65 worked for me. #26 failed.
Absolutely agree.
Comment #82
Hydra CreditAttribution: Hydra commentedConfirm that #65 works
Comment #83
rich.3po CreditAttribution: rich.3po commentedTried the patch in #65.
It seems to work, ie the field collections on my node are translatable, however it seems to have introduced a bug with the node title. If you update the title on the node, it does not get updated (regardless of language) :-/
Reverting the patch this problem goes away
Comment #84
rich.3po CreditAttribution: rich.3po commentedUpdate: the problem seems to be on line 36 of the patch, ie:
If i comment out the line
$item['entity']->item_id = '';
this fixes the node title problem, but will most likely break something else with the patch.... Unfortunately I dont understand the module/patch in great enough detail to fix both issuesc4rl - can you help?
Comment #85
idflood CreditAttribution: idflood commentedI tried the patch in #79.
- After translating a node, it first looks ok. But If I edit the translation then the collection get emptied. Editing it again and putting some content in the collection "fixes" the problem since the content is saved. However, editing it again, the collection is empty again... (same if I edit the original node, the collection is empty)
- The node title is correctly translated
edit: If i comment the same line as #84 then the problem is fixed. It looks like this portion has been added to handle revisions if I'm right.edit3: Commenting the lines worked because before that I didn't comment them out and translated. Then the "empty collection" issue appearead so I commented these lines and if fixed this issue. But with these lines commented, if I create a new node and try to translate it it doesn't work (the text stay the same between the languages, if the French translation is edited, the english version get the same text).
edit2: Commenting these lines make the test pass locally. Without the comment I have the same 5 errors as reported by the testbot.
Comment #86
idflood CreditAttribution: idflood commentedSo I did some more test and tried some things with the code.
I never saw a "nid" property on the entity. On my tests there are "item_id" and "revision_id" among other properties but I never saw a "nid". Another thing, is that they are always defined, even for a new translation. On the other hand, the "$host_entity" has a "nid" field, and it's NULL on new translation.
Given that the $entity doesn't have a nid, the following line should be changed too. It should be either $entity->item_id or $host_entity->nid. In the following patch I used $host_entity->nid but I'm not sure about that.
With these changes it works well on my little test without revisions. I did not test this with revision.
edit: I quickly tested with revision and it looks like it's working. However I never really used revisions so I may be missing something here.
Comment #87
FiNeX CreditAttribution: FiNeX commentedHi, patch #86 works fine on my latest test site. I'm not using node revision too.
Comment #88
fledev.com CreditAttribution: fledev.com commentedHi, can confirm that #86 solved the translation issue for me.
Just to be sure, I've removed the previously initialized translations (let the newly updated module make a new #items id copy).
Thank you!
Comment #89
yannickoo#86 works great! Thank you! Because two other confirmed that #86 works I set the status to RTBC :)
Comment #90
tne_ CreditAttribution: tne_ commented#86 works here aswell!
Comment #91
ParisLiakos CreditAttribution: ParisLiakos commentedmost of a coding standards review
no need for = here, i think its the default, but this is just minor
i suppose we should use <> here instead of != since it is compatible with all databases
AND
should be&&
and FALSE should be in the second part of comparison:
$entity !== FALSE
Comment #92
idflood CreditAttribution: idflood commentedThanks for the review : )
fieldCondition and addFieldCondition don't have a default value for the $operator so it may be safer to keep it.
For the "<>", the addFieldCondition function take care of replacing the operator. I think that the '!=' is more readable and common so I left it.
So in the following patch I only fixed the error described in the last remark about the !== comparaison.
Comment #93
ParisLiakos CreditAttribution: ParisLiakos commentedah..nice
was not aware of that:)
Setting back to rtbc, even though i am not sure maintainer's concerns @#45 are resolved
Comment #94
lennyaspen CreditAttribution: lennyaspen commentedThis is not working for me. I tried the last and the #86 patch. I also tried Entity Translation but it didn't work either.
I have a field collection with multiple values which should be translated in three languages but its impossible
If I try to translate the fields they just take the new value and substitute the old one, thus erasing it.
Comment #95
yannickooSee the issue title "Support content translation". Content translation is the thing when creating new nodes for every language and as you can see the patch there is special handling for nodes.
Comment #96
Elli-Sofi CreditAttribution: Elli-Sofi commentedPatch #86 works perfect.
Thank you very much!
Comment #97
jibize CreditAttribution: jibize commentedPatch #92 worked for me too.
Thank you!!
Comment #98
RobKoberg CreditAttribution: RobKoberg commentedThe patch #92 did not work for me after updating to 7.x-1.x-dev. field_collection fields are always saved as LANGUAGE_NONE. They are associated correctly with their node parent. Not sure why, but this makes the latest version (the translated version) of the field's value show for the original language as well as the translated version. In the attached screen grab of a table that stores the field value of a field in a field collection, you can see that after switching to to the translate/add or edit creates the translate set with LANGUAGE_NONE.
I am not using entity_translation -- just the core modules and i18n (is there something else required?).
A little background: There are three field_collection_items for the node, so you are seeing the results for one node and its translated node. The first three are for are for english and second three are for the spanish version. I have only set the value for the first one in the second set (been working on this for a while -- restore from backup, try something, get same result, restore from backup, lather, rinse, repeat...)
I have also attached the field_collection_item table along with the field_collection's field table. These show that the IDs are of the items are mapped correctly to their respective nodes, but the latest version shows for both the translated and original node.
Any ideas?
Comment #99
netsensei CreditAttribution: netsensei commented#98: I've tried to reproduce this with a basic test setup with content translation:
I created a new basic page node in English and translated it to Dutch (2 nodes => 1 tnid) The database reflects those changes: the field collection entity itself will be LANGUAGE_UND but field_data_field_collection which contains the actual relation between the node entity and the FC entity makes the correct mapping: the EN & NL nodes each will each be related to separate FC entities. To me, there doesn't seem any need to assign languages to the FC entities themselves as those are defined through the host entity to which they are assigned.
In your screenshots, I see the same configuration, but the FC entities assigned to the ES node do have the same, duplicated, content as the onces assigned to the EN node. Try changing the content of the FC field on the ES node and you should see how those get reflected correctly. I didn't see any NL content appear on my EN nodes when trying to reproduce this.
I do believe we need to make sure that this patch needs to meet these conditions:
1. Work with Content translation, Entity translation & Field translation.
2. Be entity type agnostic (as pointed out by Fago et al.)
I don't think we've established those goals are met, yet.
Comment #100
RobKoberg CreditAttribution: RobKoberg commented"Try changing the content of the FC field on the ES node and you should see how those get reflected correctly."
That is what I did. For the english node, I had a value of "Login or Sign up". For the spanish version I had "Ingresa o regístrate". ( http://drupal.org/files/Screen%20Shot%202013-03-30%20at%208.24.08%20AM.png )
Using the language switcher and viewing in either English or Spanish showed "Ingresa o regístrate". One thing that might be an issue is that I am not using URL for language detection (/admin/config/regional/language/configure). I am using Session, User, Browser, and Default -- in that order. I have the same URLs for the en and es nodes.
That said, I switched back to using node fields rather than FC entities.
I was also having problems with entityreferences, which also might be related to not using URL for detection. I worked around that with:
This is not possible with field_collection because of having LANGUAGE_NONE.
Comment #101
wizonesolutionsWould be great to get a summary of testing steps. I ran into this issue on a Content Translation-using site and applied the patch. My use case is simply to translate a new node and check in the database that the FCs are not using the same IDs.
Before #92: the `value`s would be the same.
After #92: Still seeing the translations having the same `value`s.
I tweaked the condition that resets the `item_id` on the entity. It's clear that it should work for nodes without `nid`s. However, it was checking that `$host_entity->nid` was always set, and this was not the case when using content translation (`isset()` returns `FALSE` on `NULL`s by design).
After doing that, new field collection items were created for the translated node, as should happen. The content of the newly-created field collection items is the same as in the original node (I didn't change anything), also as expected.
I instead check that `$host_entity_type == 'node'` now. I don't see any problems with this change, but I am not fully educated on the issue, so let me know if there are any.
Re. netsensei #99: This issue is to support content translation only. I think that #2 is currently a non-issue. Entity translation is being handled in a separate issue.
I think this patch should be reviewed and concrete steps given if it still needs work. We need something working in Field Collection sooner than later, even if it's just in dev.
Comment #103
kelvintran CreditAttribution: kelvintran commented#101 worked for my case.
I use content translation for my content in Drupal. I have one content type and have two field collections inside. I applyed patch #101 and now I can translate it. It work for node template. I did not checked on view and panel.
Thank wizonesolutions
Comment #104
zterry95 CreditAttribution: zterry95 commentedI have test the patch of #65, #92, #101
all failed. I think I am missing something. However below is my steps:
1: create a field collection field for content type : article.
2: add a textfield for field collection.
3: i18n enabled, and the article enable for translation too.
4: create an article in English, with field collection field "test string"
5: go to translation tab, and add translation for this article.
I pay attention for field collection field specially and confirm it has been translated.
suppose the translation string is "测试数据"
6: then I visit the both english and chinese.
both language, the field collection field, the value becomes “测试数据”
Am I do something wrong? thanks for help.
BTW, I update my entity to dev. see below:
http://drupal.org/node/1782664
Comment #105
zterry95 CreditAttribution: zterry95 commentedah.. ignore my post above.
I apply the patch in wrong place.
After I found this problem and apply the patch of #101, the field collection field can be translated now.
Thanks for great work of guys.
Hope this patch can be embeded into field_collection core in soon:)
Comment #106
bderubinat CreditAttribution: bderubinat commentedVery good field_attach_prepare_translation alter function to copy content of node's field_colection when translating. Merci Etienne.
http://www.lapetitepausetechnique.net/2012/09/field_collection-et-i18n/
It works for me with two fixes
Comment #107
artis CreditAttribution: artis commented#101: field_collection-1316162-101.patch queued for re-testing.
Comment #109
wizonesolutionsFor anyone still getting burned by this bug (or for that matter getting burned by using Node Clone on nodes with field collections), first apply my previous patch (or bderubinat's, but I haven't tested that one). That will stop further damage.
To repair your previous entries and ensure that, for example, removing a field collection entry doesn't remove it from other nodes from which you didn't want it removed, you can implement this script I've detailed in a gist: https://gist.github.com/wizonesolutions/5567549
It will give new
item_id
s to field collection items that it detects as appearing more than once. All my testing has indicated this is safe, but if not, please leave a comment on the gist. I hope this helps someone.Thanks to Project Ricochet for sponsoring my time on this.
Comment #110
akalam CreditAttribution: akalam commented#101 worked for me, but after changed the file paths to the relative ones to the module folder. Here is my version that applyes cleanly from inside the field_collection module directory, aganst last dev
Comment #111
wizonesolutionsdesarrollo2.0: Thanks for the reroll. My version has been working well for quite some time, and we realized it also fixes the same problem occurring when cloning nodes. I think we need another reviewer before this could be RTBC'd, but it's looking pretty good to me. Would love this to make it into the module.
Comment #112
Watergate CreditAttribution: Watergate commentedThanks wizeonesolutions and desarrollo2.0, it works for me!
However, I'm curious why checking if $host_entity_type == 'node'. If it's because the entity's id is unkown, you/we could use entity_extract_ids.
Comment #113
jamesmorrish CreditAttribution: jamesmorrish commented#110 worked for me, although I had to recreate the patch for Field Collections 7.x-1.0-beta5.
Here you go:
Comment #114
PMassoels CreditAttribution: PMassoels commented#101: field_collection-1316162-101.patch queued for re-testing.
Comment #115
Fugazi-2 CreditAttribution: Fugazi-2 commented#113 worked for me, thanks.
Comment #116
jimmyko CreditAttribution: jimmyko commented#113: field_collection-1316162-113_0.patch queued for re-testing.
Comment #117
handrus CreditAttribution: handrus commentedPatch #113 just saved my ass.
Works like a charm.
Comment #118
derMatze CreditAttribution: derMatze commented- ignore this -
Comment #119
torotil CreditAttribution: torotil commentedThis still doesn't work if "Create new revision" is checked when the new node is created. Test case:
The result is: Instead of a copy of the field_collection a new revision (item_id=1, revision_id=2) is created that is linked to node B. The data for the older revision is lost. Editing those nodes later on leads to more odd behavior like data not being saved …
This also happens if the revisioning module is enabled. Since it forces new revisions when editing a node.
Comment #120
DuttonMa CreditAttribution: DuttonMa commentedPatch #101 worked fine for me, but I was working in an environment where I wasn't able to use patched modules.
I therefore created my own module and implemented the following hook:
(see http://theduttons.org/?p=7)
This worked perfectly, and field collection translation worked correctly once the new module was enabled.
.. hope that helps someone out !
Comment #121
interdruper CreditAttribution: interdruper commentedPatch #113 (rewrap of #101) applies fine over 7.x-1.0-beta5+1-dev, and works for me.
I do not use node revisions, so I cannot say anything about #119.
For those looking for field translation (entity translation module support),
remember that other issue is the appropriate: #1344672: Field Collection: Field translation (entity_translation) support.
Comment #122
maartendeblock CreditAttribution: maartendeblock commented#113 works for me with i18n and revisions (workbench). I could not replicate the problem described in #119.
Comment #123
badrange CreditAttribution: badrange commentedIs it possible that the problem described in #119 was caused by testing the #113 patch on field_collection items that were created before the patch was applied?
Some weird things happened to field_collections that were made prior to installing the #113 patch on my local development environment.
Comment #124
torotil CreditAttribution: torotil commentedI can still confirm the issue mentioned in #119 using beta5+#113. Although it's not apparent from the UI the state in the database is still inconsistent.
Prerequisites
Steps to reproduce
Expected result
Two independent entities of type field_collection are created.
Actual result
The value of the field_collection in the translated node is stored in a new revision of the same field_collectiont entity (entity_id=1).
Actual result in the database
Expected result in the database
Comment #125
torotil CreditAttribution: torotil commentedAfter that field_collection behaves weird:
After editing the translated node again - this time without checking the "Create new revision" checkbox - the just edited value is lost and the node is displayed as if the field_collection is empty.
Indeed the field_data_field_test table now has two entries with two entities - but the newly created one has no value in field_data_field_textfield.
Comment #126
torotil CreditAttribution: torotil commentedThis patch fixes it for me regardless of the revision setting. It simply forces the creation of a new field_collection entity when the host_entity is new.
At this point field_collection is unable to handle multiple host-entities pointing to the same field_collection-entity anyway.
This patch comes without any special-cases for the node module.
Comment #128
torotil CreditAttribution: torotil commentedFixed notices and whitespace.
Comment #129
torotil CreditAttribution: torotil commentedComment #130
prdctvtxt CreditAttribution: prdctvtxt commentedadding consolidated patch for beta5 release
Comment #131
torotil CreditAttribution: torotil commented@prdctvtxt: There is no need to consolidate the patch. Mine works instead of #110 or #113 for both 1.x-dev and 1.0-beta5 :)
Comment #132
Lukas von BlarerThis works for me as well. RTBC?
Comment #133
yannickooWhitespaces!
Comment #134
torotil CreditAttribution: torotil commentedYeah right. We finally have a working RTBC patch after 133 comments. And what is the reaction? Setting it back to Needs Work because there is one an empty line is aligned with the rest of the block (which can be changed easily before commiting). That's the way to lose contributors ;)
Comment #135
torotil CreditAttribution: torotil commented@yanickoo: the patch that is RTBC is not even the patch that has the whitespace. #128 should be committed instead of #130
Comment #136
yannickooThe patch in #128 does not look complete. Compare it with the patch from #130.
Removed the whitespaces :)
Comment #137
tim.plunkettComment #138
torotil CreditAttribution: torotil commented@yanickoo: it is complete. it just handles a more general case and therefore is much simpler. Test it!
It's not code-size that counts, it's functionality ;)
Comment #139
torotil CreditAttribution: torotil commentedComparing #128 with #130:
My tests showed that the underlying problem is that field_collection is unable to handle a field_collection-entity that's shared between multiple host-entities. #128 avoids this situation in general while #130 and the patches before avoid it only for one special case.
Also there was an error in the logic of the original code: If $host_entity->revision was set a new field_collection-entity revision was created even if $host_entity->is_new was set to true (=the new revision is the first).
Essentially with the code from #128 in place the rest of #130 or #136 is dead code: Special cases where the general case already handles everything.
Comment #140
torotil CreditAttribution: torotil commentedHere is a test that tries to create copies of a node with field_collections. It fails without #128.
Comment #141
torotil CreditAttribution: torotil commentedThis patch includes #128 and the tests from #140
Tests should pass.
Comment #142
torotil CreditAttribution: torotil commentedI'm pretty sure #141 would also make #1416416: Add support for node_clone work.
Comment #143
gregory_kapustin CreditAttribution: gregory_kapustin commentedHi ; is there a #136 vs #141 doc. ?
Comment #144
torotil CreditAttribution: torotil commented@gregory_kapustin take a look at #139: #136 is essentially #128+#113 which has been put in in #130 and re-rolled without whitespace errors in #136. Both @prdctvtxt and @yannickoo didn't notice respectively believe me that #128 was indeed meant instead of #113 not additionally to it. This caused additional confusion …
Essentially there are two ways to tackle this issue in this bug-report:
Can we finally get over this confusion and have people test things before they make any presumptions about what "looks complete" or not? I feel like I keep repeating myself over and over and nobody cares to read / test.
Comment #145
ParisLiakos CreditAttribution: ParisLiakos commentedso. patch in #141 takes care of #45 and has a test.
it is rtbc. all other patches/approaches are node-specific
thanks @torotil
Comment #146
gregory_kapustin CreditAttribution: gregory_kapustin commented====> French (original) node field collection values were english's ones. / Field collections IDs were the same.
:(
Comment #147
ParisLiakos CreditAttribution: ParisLiakos commented:(
#146 means its NW
Comment #148
torotil CreditAttribution: torotil commented@gregory_kapustin: I've just tried exactly that and I can't reproduce your problem. Are you sure you've applied the patch correctly?
Comment #149
gregory_kapustin CreditAttribution: gregory_kapustin commented@torotil sorry I just counter-checked and problem's still there. Drupal 7-22.
Did it work on your installation ? New translation node ===> New field collection items ids ?
Comment #150
torotil CreditAttribution: torotil commentedYes it worked fine on my installation: node in one language with a field_collection, translate node -> add new translation -> save: The field_collections have different item_ids and I can change their contents independently.
How exactly do you create the translation?
… and could you post the code of field_collection_field_presave() in the version you are using?
Comment #151
gregory_kapustin CreditAttribution: gregory_kapustin commentedHi,
_presave has
$force_new_entity = !empty($host_entity->is_new);
andI translate with node/add/MYCONTENTTYPE?translation=NID&target=en.
Whole _presave is
Comment #152
torotil CreditAttribution: torotil commentedOk, then I'm at a loss: It works for me exactly like that.
It seems that in your setup $node->is_new is not set although in my setups it always is set on node/add/…
… and node_save() has …
Comment #153
torotil CreditAttribution: torotil commented@gregory_kapustin: could you run the unit-test that's included in the patch?
Comment #154
gregory_kapustin CreditAttribution: gregory_kapustin commentedNevermind, it worked on a fresh install. I'll find what was troubling on my website, but it must not delay the rtbc.
Comment #155
torotil CreditAttribution: torotil commentedSetting to RTBC again due to #154.
Thanks for testing, anyway!
Comment #156
gregory_kapustin CreditAttribution: gregory_kapustin commentedGot it. I dpm'd $host_entity on field_presave, and, as weird as it is, the node entity when I add a translation to a node, "is_new" is not set on the host.
So I bypassed it with
$force_new_entity = !empty($host_entity->is_new) || !$host_entity->nid;
Of course, it's not compatible with others entity types...... I give you my patch anyway.
Comment #157
torotil CreditAttribution: torotil commented@gregory_kapustin what version of drupal core / i18n_node are you using? In my tests $node->is_new was always set.
Comment #158
gregory_kapustin CreditAttribution: gregory_kapustin commentedDrupal 7.22, i18n 7.x-1.8
Comment #160
torotil CreditAttribution: torotil commentedCould you get a backtrace for such calls where this happens? This would be really helpful!
Something like:
might help …
Comment #161
yvesvanlaer CreditAttribution: yvesvanlaer commentedHello fellow Drupalistas
When I save my node with an unlimited field collection on one language, it synchronises with the other language.
I tried every patch on this issue list and on https://drupal.org/node/1683784
But nothing is resolving my problem.
I started using the dev version in hope of finding a solution.
Aren't there any clear instructions how to get it translated?
Thank you so much.
Comment #162
yvesvanlaer CreditAttribution: yvesvanlaer commentedI solved my own problem.
There was something wrong in the /admin/config/regional/language/configure.
It now has a "Content language detection" section...
- Order of language detection methods for content. If a version of content is available in the detected language, it will be displayed.
I only had enable some of them.
Maybe this information could help somebody else.
Comment #163
cjoy CreditAttribution: cjoy commentedtested with a fc (unlimited values), but can not confirm #141 works
Problems encountered:
Adding a new translation (update: also when editing the original content)
Expected result:
node form inheriting the original field collection values
Actual result:
node form showing empty field collection items, matching the count of the original language version.
Removing field collection item on node edit from (add translation)
Expected result:
fc item removed from multi value form item.
Actual result:
AJAX callback fails
Environment:
Drupal 7.23 (standard install)
Entity 7.x-1.2
Field Collection 7.x-1.0-beta5+1-dev
Entity Translation 7.x-1.0-beta3
That said, thanks to everyone working on the issue and providing feedback and fixes.
Field Collections are a very powerful tool for Sitebuilders and the i18n incompatibility is a major blocker.
Comment #164
gregory_kapustin CreditAttribution: gregory_kapustin commented@cjoy
Concerning your newly created translations, I bypassed the problem with that patch.
Comment #165
cjoy CreditAttribution: cjoy commented@gregory_kapustin
thank you - I just realised that the same problematic behaviour can be observed when editing the original content, so I assume the issue is more deeply rooted or I have missed something elementary here.
I was assuming that patch #141 does not rely on other patches or modules (such as node_clone). If that is not so, it would be really helpful if someone could provide a detailed description on how to reproduce a working test case for this fix with a vanilla Drupal install.
Comment #166
torotil CreditAttribution: torotil commented@cjoy @gregory_kapustin: I have exactly the inverse problem: I can't reproduce your problems.
I'm testing on current versions drupal 7.23, i18n 1.10, field_collection-dev + patch from #141. Two languages, one field_collection (unlimited) containing one text field (unlimited), content type with multilingual "enabled with translation".
I create a node in the first language with multiple field_collection items with one or more entries for the text-field. Then I go to Translate -> add translation. I can delete, modify, … all values without error independently from the others.
Are you using additional modules that might play a role? Can you try to reproduce this on a minimal installation? Can you try to get a backtrace like described in #160.
Regarding #163: This seems like an unrelated issue. This is about i18n_node and not about entity_translation.
Comment #167
cjoy CreditAttribution: cjoy commented@torotil
you are right - sorry for adding confusion here.
I tested it again on a vanilla Drupal install using i18n instead of entity_translation and the patch posted in #141.
Unfortunately, it does not work as expected either.
Environment:
Drupal 7.23 (standard install)
Field collection 7.x-1.0-beta5+1-dev + patch #141
Internationalization (i18n) 7.x-1.10
Multilingual content (i18n_node) 7.x-1.10
Test case
Node with a field collection (unlimited values) containing an image field and a text field
Problem
Editing the field collection items in the translated node alters the original field collection.
The translated node references the same field collection entity as the original.
Condition to reproduce
The above problem occurs only when "Create new revision" is not checked for the content type.
Workarround
Enable "Create new revision" when creating the host entity and/or translations.
Comment #168
torotil CreditAttribution: torotil commented@cjoy does the newly introduced unit-test (included in the patch from #141 work for you? Could you try to get a backtrace by putting something like #160 in your field_collection_field_presave()?
This is exactly what I'd expect to happen if the patch wasn't applied …
Comment #169
cjoy CreditAttribution: cjoy commentedI'm afraid I fail to wrap my head around how to execute that unit test.
As for the debug info you requested:
I stepped through field_collection_field_presave() and the only difference to be noted is that the following block is fully executed for adding a translation _with_ revisions enabled and not executed otherwise:
$force_new_entity remains FALSE in all scenarios.
Comment #170
ParisLiakos CreditAttribution: ParisLiakos commentedRe @#152
This happens after
field_attach_presave('node', $node);
though:/Comment #171
ParisLiakos CreditAttribution: ParisLiakos commentedthats the best i can come up
interdiff vs #141
Comment #173
ParisLiakos CreditAttribution: ParisLiakos commentedyes that was stupid..$host_entity != $entity :P
Comment #174
torotil CreditAttribution: torotil commented@cjoy take a look at https://drupal.org/node/291740
Comment #175
torotil CreditAttribution: torotil commented@ParisLiakos: Could you provide a simpletest (that emulates node translation) that fails with #141 but works with #173?
Comment #176
ParisLiakos CreditAttribution: ParisLiakos commentedsure, i think that should do it
Comment #177
ParisLiakos CreditAttribution: ParisLiakos commentedand a small fix, plus README.txt update
Comment #178
torotil CreditAttribution: torotil commented@ParisLiakos: Where is field_attach_presave() called before $node->is_new is being set?
node_save has the following:
So it can't be node_save(). What else leads to field_attach_presave() without is_new being set?
Comment #179
ParisLiakos CreditAttribution: ParisLiakos commentedoh, i see what got you there. the $op variable is either insert or update, not presave. thats where field_attach_presave is being called:
Comment #180
torotil CreditAttribution: torotil commentedOk let's take a look at field_attach_insert() and field_attach_update():
$op being 'update' or 'insert' is perfectly fine.
Comment #181
torotil CreditAttribution: torotil commentedAh now I see: I'm on a version of D7 where a patch for #1994594: hook_field_presave() not called reliably is applied. Never mind.
Comment #182
torotil CreditAttribution: torotil commentedRTBC works for me on a vanilla Drupal install. - I'm still unsure why the unit-test works with #141 only though. But I won't hold this up any longer.
Comment #183
ParisLiakos CreditAttribution: ParisLiakos commentedit works because you manually set the is_new to TRUE before calling node_save()
#1994594: hook_field_presave() not called reliably would be a nice link to add in the comments of node special casing
Comment #184
fagoGood work - the patch seems to solve the issues around cloning field collections and even comes with tests!
This got removed, but it does not look like the patch changes the behaviour around field translation? Thus, that sentence should stay or become more precises. Actually revision got solved already.
Some other minor remarks:
Shouldn't be in here.
Create needs to start upper-case.
Here as well.
Comment #185
ParisLiakos CreditAttribution: ParisLiakos commentedso thats good news, #1781190: Field Collection saved on presave went in, so we dont need to special case for nodes anymore.
updated patch that fixes #184
Comment #186
ParisLiakos CreditAttribution: ParisLiakos commentedthis will most likely fix #1416416: Add support for node_clone, i marked it as postponed on this one.
Comment #187
FiNeX CreditAttribution: FiNeX commentedPatch #185 works fine on latest -dev version :-)
Comment #188
fagoI was about to commit this, then I figured it's not complete:
1. The comment says the host entity is being cloned, but a regular insert could look the same. -> It's being cloned when we have an field collection entity in there that's not new, but the host is suddenly new.
2. Then the comment says we need to clone the field collection entity, what is right - but we do not do that. We just save the same object with a new ID. However, it should do a PHP "clone" operation to ensure the new field collection entity has received a new identity - else the wrong item sits in the original un-cloned entity.
3. We need to improve tests to cover that. Best clone the node object also and then also assert that the two field collection entities do have different object identities (notIdentical) *and* IDs.
Comment #189
FiNeX CreditAttribution: FiNeX commentedWhat about the case with old nodes which share the same field collection item due to the bug? Would be possible to fix them? ( ref to comment #27 and #94 )
Comment #190
gregory_kapustin CreditAttribution: gregory_kapustin commented@FiNeX I used this script to clone my field collections and attribute them to nodes ; worked fine.
Comment #191
ParisLiakos CreditAttribution: ParisLiakos commentedwell i got to say i am not sure i got #188 correctly, but here is an attempt..
is that what you mean?
Comment #192
ParisLiakos CreditAttribution: ParisLiakos commentedsigh
Comment #194
ParisLiakos CreditAttribution: ParisLiakos commentedoh yes, this caught a bug:)
Comment #195
torotil CreditAttribution: torotil commentedHere is an update hook, that tries to fixup host_entities that share one field_collection_item by copying the items. This can't work in all cases though as data might get lost permanently due to this bug.
Comment #196
Mark_L6n CreditAttribution: Mark_L6n commentedTried to update Issue summary, but it didn't work. Changes attempted:
In Problem/Motivation, change
Field collection does not support content translation properly. Multilingual sites are not able to use Field collection.
to
In Proposed resolution, change
Provide support to have Field collections be multilingual
to
Provide support to have Field collections work with note translation
Comment #197
klonos@Mark_L6n: don't worry mate, I did that on your behalf ;)
...just a pointer: when you want to reference other issues use
[#issue_id]
. This way, when the issue node is rendered, besides the issue title it also reflects its status (color code and hover tooltip) as well. If you want to reference a specific comment in an issue, use [#issue_id-comment_number]. Examples:#1316162: Support content translation and host entity cloning
renders like so: #1316162: Support content translation and host entity cloning#1316162-197: Support content translation and host entity cloning
renders almost the same, but links directly to this comment here ;)Comment #198
Darren Clark CreditAttribution: Darren Clark commentedThere is crossover with issue #1344672 I've rolled in these into the patch to get translation and cloning functioning albeit with some warnings.
https://drupal.org/comment/8229149#comment-8229149
Comment #199
marksmith CreditAttribution: marksmith commentedWhat can I do if I want to localize only the parent item in a nested field collection, and want the child field collection to remain the same through all languages? (see #12)
Use case:
Field collection 1 (parent): A list of course titles that need to be localized.
Field collection 2 (child): A list of bibliography items that should be the same in all languages.
Thanks for any suggestions.
(I also use the field_collection_views module)
Comment #200
miiimooo@ParisLiakos
I'm testing the patches from #194 with the current dev version.
When I enabled entity translation on a field collection field it doesn't seem to have any effect on the field. In the UI it still says "(all languages)" next to the field (other fields are translate-able at this point)
Update: enabling field translation on the field in the host entity (node) has the desired effect.
Comment #201
ParisLiakos CreditAttribution: ParisLiakos commented@miiimooo well you should probably check this issue then #1344672: Field Collection: Field translation (entity_translation) support.. this issue is not about field translation
Comment #202
miiimoooOops!
Comment #203
torotil CreditAttribution: torotil commentedComment #204
lukusHi - wondering if there's any update on this issue?
I'm very keen to use field collections on a site, with i18n translations.
I'm aware that this might be problematic due to this issue .. can anyone offer any advice on how to proceed?
Comment #205
ParisLiakos CreditAttribution: ParisLiakos commentedcheck the patch in #194
i use it on production for months now, without problems
Comment #206
klonos...hiding deprecated/past patches helps not to confuse people ;)
PS: I've left the patch at #195 by @torotil visible because from what I get it is an addition to the one at #194.
Comment #207
math3usmartins CreditAttribution: math3usmartins commentedHi guys,
Thanks for sharing these solutions -- really helpful.
I think it's worthy placing a note/disclaimer on the project's page.
Comment #208
mgifford@math3usmartins - what would you have the disclaimer say?
Comment #209
wizonesolutionsFor those looking for an interim solution, I discovered https://drupal.org/project/field_collection_node_clone
I opened an issue in their issue queue to track this issue. I don't maintain that module, but it looks like it was born out of frustration from this issue.
Comment #210
FMB CreditAttribution: FMB commentedPatch #194 works perfectly for me.
Comment #211
jrglasgow CreditAttribution: jrglasgow commentedI have combined the patches in 194 and 195 into one patch for easier patching.
Comment #212
interestingaftermath CreditAttribution: interestingaftermath commentedUpdated patch #211 seems to work great for me.
Comment #213
ianthomas_ukWhy is this check being removed? The change was made in #185. I assume the check was originally added for a reason.
This could do with some better variable names, and maybe some comments. $node? $entity? $item? Which is which?
I assume $entity is the field collection? So how about $original_node, $original_field_collection, $duplicate_node etc. It wouldn't really make a difference to the test, but it would be clearer if we cloned $node before passing it to node_save too, and safer if this is refactored in the future.
Shouldn't FieldCollectionBasicTestCase have failed on #176?
Comment #214
ianthomas_ukComment #215
jrglasgow CreditAttribution: jrglasgow commentedI have made some changes based on the recommendation in #213.
Please Review
Comment #217
jrglasgow CreditAttribution: jrglasgow commentedLet's try this again
Comment #219
KalyaniK CreditAttribution: KalyaniK commented86: field_collection-1316162-86.patch queued for re-testing.
Comment #221
dariogcode CreditAttribution: dariogcode commentedThe issued I had is when tranlsating a node. The field collection keep "referencing" to original node, causing an editing in one node change the value in the other. Then editor need to delete collection element and add a new one.
The patch provided in #211 fixed this for NEW CONTENT. However for existing content I still have the same issue. I guess field_collection_update_7005 should take care of this but not in my case.
Comment #222
torotil CreditAttribution: torotil commentedHi tilion,
this bug may lead to permanent data-loss. Namely users might overwrite content from other languages without knowing. The update hook from #211 tries to fix existing field_collections with a reasonable heuristic. It also restructures everything so that future edits don't lead to data loss. However it can't do magic. Data that's already lost will stay lost.
Comment #223
Airborn CreditAttribution: Airborn commentedHi,
I got an error while updating the module after patching with #211. It says that a table is not existing. I found out that we should check if the field_config.deleted is 0 at the beginning of field_collection_update_7005()
Alex
Comment #224
hkirsman CreditAttribution: hkirsman commentedTx very many! #211 fixed the bug where field collections where overwritten in translated nodes.
Comment #225
KalyaniK CreditAttribution: KalyaniK commented86: field_collection-1316162-86.patch queued for re-testing.
Comment #227
torotil CreditAttribution: torotil commented@ #1316162-213: Support content translation and host entity cloning
I didn't find any reason why field_collection_get_field_entity() should be unable to deal with an empty $item['entity']. Maybe it was some (undocumented) shortcut in the logic. Since the logic seems to have changed a lot and non of the testers (and non of the automated tests) experience any problems with that I don't see any reason do hold this issue up.
Yes it could. I took a closer look though: The naming is consistent throughout the test-class. So either rename them all which should be done in a separate issue - or leave it consistent. Not having descriptive variable names is bad - having inconsistent variable names is worse.
@ #1316162-223: Support content translation and host entity cloning I've added a check for deleted fields.
Let's get this commited finally!
Comment #228
torotil CreditAttribution: torotil commentedComment #229
torotil CreditAttribution: torotil commentedI don't think that the rather complicated update hook should be holding up this patch either: It was created to somehow fix sites that have been in an unsupported state (using i18n_node with field_collection) anyway. So it might be provided as an extra script instead of as update hook.
Comment #230
mducharme CreditAttribution: mducharme commentedAmazing work everyone. It's been a long road but I think that this patch should be held up until the update hook is implemented though. If not - we need to make sure that it is clear that data WILL BE LOST if users translate existing field collections that contain data but are currently untranslated.
Comment #231
ianthomas_ukThat's the wrong way round to look at things - the patch is making a change, so it should be possible to justify that change, rather than just say that it doesn't cause any obvious problems. The logic hasn't actually changed that much when you undo this change - lots of lines were marked as changed just because the indentation changed.
Good point about keeping consistency with the other tests in the file. That means we don't want those changes from #215/217, but please make it clear what a patch is based on if it isn't based on the latest patch (ideally with an interdiff). #227 is based on #211 and the changes are sensible.
This new patch just undoes the unexplained
removal.
Comment #234
ianthomas_uk231: field_collections-1316162-231.patch queued for re-testing.
Comment #236
ianthomas_uk227: field_collections-1316162-227.patch queued for re-testing.
Comment #237
ianthomas_ukOK, the test failures explain the removal of that check - field_collection_field_get_entity() allows the entities to be lazy loaded, but with that check in place they are skipped instead, causing the test failures. Looking at the history of those lines, they were added at different times so this presumably wasn't considered.
#227 is therefore better than #231.
I've tidied the remaining tasks, adding:
* Address comment #211
* Identify why FieldCollectionBasicTestCase didn't fail on comment #176
Comment #238
d.sibaud CreditAttribution: d.sibaud commented#227 work like a charme!!
Comment #239
PhilYHello,
Got problem after applying patch #227 when running database update (see attached screenshot) maybe related to table prefix?
Comment #240
ianthomas_ukYep,
field_config
should be{field_config}
to take account of table prefixes.Comment #241
marcoliverFurthermore shouldn't this also apply to all those SELECT statements in the patchfile? Something like
SELECT f1.{$name}_value FROM field_data_{$name} f1;
would fail on an installation with prefixed tables, no?Comment #242
torotil CreditAttribution: torotil commentedThere is actual two issues being dealt with at once that complicates / delays things:
1. has a working patch including unit tests since months - at least since #192. Which should be committed ASAP. #211 combined the two patches which is IMHO wrong.
2. has a script that was originally posted by me in #195 that worked for us. I've posted it here because it might be helpful to others. It was never intended to be part of this patch or feature request.
Note that there isn't even a complete solution to 2. since data is going to get lost permanently.
Having 2. block 1. is just going to produce more broken installations and additional data-loss. Again The fixup script is only needed if your setup was in an unsupported state in the first place.
I've split the patch again into two parts:
Comment #243
ParisLiakos CreditAttribution: ParisLiakos commentedthank you for splitting the patches!
Can we commit this? we are close to 300 comments:/
Comment #244
dariogcode CreditAttribution: dariogcode commentedPlease commit to dev version so we can have this advantage.
Comment #245
joetsuihk CreditAttribution: joetsuihk commentedtested against 7.x-1.x dev, works great!
Comment #246
avner CreditAttribution: avner commenteddosnt work for me, still unable to translate nodes without the field_collection being synced across
Comment #247
torotil CreditAttribution: torotil commented@avner/#246: Are you sure you have applied the patch correctly and you are using the patched version of field_collection? If your issues persist: What version of field_collection and i18n are you using?
Comment #248
idflood CreditAttribution: idflood commentedTested patch in #242 and it's working perfectly here. I will come back here if I encounter any issue but it is looking good.
Comment #249
killua99 CreditAttribution: killua99 commentedNow this patchs need reroll.
This is a 248 comments issue, please commit this.
Comment #250
killua99 CreditAttribution: killua99 commentedActually idk why drush fails to apply this patch. I did try on my local and the patch still working.
And I double test it, see like is working without problem. this patch should be committed already then.
Comment #251
goron CreditAttribution: goron commentedHere's a reroll of the patch in #242 (the main one - "field_collection-1316162-242.patch") to work with latest 7.x-1.x of this module.
(Sorry my editor also added a few lines of whitespace removal)
I can also +1 that this works. Definitely seems RTBC.
Comment #252
ThomasH CreditAttribution: ThomasH commented2The patch provided by goron in #251 solved my problem of having field collections sync over node translations. Thanks a lot!
Comment #253
torotil CreditAttribution: torotil commentedThomasH: That's funny. There were no functional changes between #242 and #251 -- only whitespace.
Comment #254
rich.3po CreditAttribution: rich.3po commented+1 for patch in #242. This fixed the 'shared' field collections issue i was experiencing when cloning nodes (and translating)
Many thanks - please roll
Comment #255
goron CreditAttribution: goron commentedtorotil: I don't think ThomasH was saying the #242 was not working for him. He just tried the latest patch on the thread and it worked.
To be clear, I rerolled the patch because because the one from #242 would not apply to the latest code. There were no functional changes.
Comment #256
miro_dietikerHere some questions / minor additional inputs from my review.
What exactly are these (negative) asserts checking? Please comment in code.
And note that the code does NOT check any successful copying.
Variables $field_name, $field, $instance IMHO should be more explanatory (field or field collection?). IMHO the test code is too hard to read with this naming.
Comment #257
torotil CreditAttribution: torotil commented@miro_dietiker:
ad 2. was already addressed in #227. In short: the variable naming is consistent with the rest of the tests in the file. If you want to change the naming please make a patch that does so for all tests.
ad 1. the assertions check that distinct field collection entities are created when cloning a node.
$item['value']
contains the entity-id of the referenced field collection (see field_collection_field_schema()).Comment #258
miro_dietiker@torotil: Sounds fine for me. Thank you for the inputs. We can always improve things (naming, add comments) in a followup.
Comment #259
RaulMuroc CreditAttribution: RaulMuroc commented.
Comment #260
Martijn Houtman CreditAttribution: Martijn Houtman commented+1 for the patch in #251, works as expected for me.
@RaulMuroc: Sure you are using the latest dev version with the patch applied? I just did a fresh install, and it works fine here.
Comment #261
RaulMuroc CreditAttribution: RaulMuroc commentedFor those lost: You have to set up each field of each Field collection translatable through: Structure > Field collection > Manage fields.
Comment #262
dariogcode CreditAttribution: dariogcode commented@raulmuroc it is supposed to work like you described as desired. Probably something is wrong in your setup, Patch in this issue solve the 'shared' data issue when translating, but no one reported what you described.
Comment #263
RaulMuroc CreditAttribution: RaulMuroc commentedOtherwise works nicely.
Comment #264
jmuzz CreditAttribution: jmuzz commented@RaulMuroc To answer the question about entity translation, they are two different approaches and shouldn't be combined for translating node content. Content translation makes a separate copy of a node for each language, so every translation can have completely different values for every field. Entity translation uses one copy of a node but allows some of its fields to be translated, while other fields will be the same for every translation.
Comment #266
jmuzz CreditAttribution: jmuzz commentedThanks everybody for your work on this. Sorry if I missed any names.
If there's any remaining issues please open a new ticket for them.
Comment #267
f0ns CreditAttribution: f0ns commented+1 for the patch in #251, works perfectly on the latest stable release!
Thanks a lot!
Comment #268
f0ns CreditAttribution: f0ns commented+1 for the patch in #251, works perfectly on the latest stable release!
Thanks a lot!
Comment #269
harshadananjaya CreditAttribution: harshadananjaya commentedI am running Drupal 7.19 I use field collections extensively and facing a problem to support content translation. After following this thread, I checked my code to make sure the patch has been already applied to my version of Drupal. Even though the patch is already applied, it doesn’t work as expected. Here is the current behavior.
1) First instance I save data; the database language column stores ‘und’. It should be English (en) since it’s the default language.
2) Then I go to edit and the fields are blank. That is because it’s looking for data saved as ‘en’. But the database has a record written with the language ‘und’
3) If I re-save entering the same data again, the database language column gets saved properly as ‘en’ (so there is a stale record at this point with language ‘und’)
Bootstrap.inc seems to have ‘und’ defined as the default language. If I change it to ‘en’, you won’t get the [translate] tab for content types.
Since this patch has worked for others, I am wondering why it doesn’t in this case.
Comment #270
jmuzz CreditAttribution: jmuzz commentedI think bootstrap should define en as the default language. It returns the result of the variable, you can check it with:
select * from variable where name='language_default';
About the missing translate tab, do you have something set up for content language detection? If only default is checked you won't get the translate tab. It is usually recommended to use URL detection. /admin/config/regional/language/configure
Comment #271
harshadananjaya CreditAttribution: harshadananjaya commented@ jmuzz:
in fact, in this case your answer not working for my issue. Although I done as u described, the problem is still there
Comment #272
remaye CreditAttribution: remaye commentedAfter reading such a long and technical thread I'm steel a bit confused of what as been fixed (or not) exactly and in which conditions. It would be great if someone could make a little summary, like :
What content translation method it is covering (in this case, Node Translation method)
Had this patch already be committed to last dev version: date/version
Is there any minimum version or patch for other module required (like for Entity API, i18n ...)
Is there any special thing to do (or not to do) while configuring the multilingual settings ("en" as default ? language detection method ? ...)
I think it would help everyone new to the issue.
Thanks.
Comment #274
panche CreditAttribution: panche commentedThis is not yet fixed, I'm experiencing this as well in two different sites.
Comment #275
netsensei CreditAttribution: netsensei commented#274: This has been fixed in development (committed july 3rd) The last stable version was released on march 13th. This is not part of a stable release yet. You should check what version of Field Collection you have running. If it is the latest stable version, you could try either one of these options:
1/ Apply the patch committed in #265 directly to the code and move the module from sites/all/modules/contrib to sites/all/modules/contrib_patched folder.
2/ Upgrade to the development snapshot (7.x-1.x-dev) (... only if you feel really lucky)
Please leave this closed. Submit any particular problems as a new follow-up issue and use the "related issues" field in the issue summary form to refer to this issue.
Comment #276
giupenni CreditAttribution: giupenni commentedWith latest dev version the problem persist.
I can't translate.
Comment #277
nicolasg CreditAttribution: nicolasg commentedharshadananjaya / #269
Did you ever fix it ?
I have the exact same problem.
If i manually change 'und' to 'en' in the database then it loads the collection.
I think somewhere in the field_collection or entity_translation modules there is a line that sets the language in the database based on the constant LANGUAGE_NONE when it should be using the nodes language, or at the very least the system language.
Ive trued going thru field_collection.module and changing all the LANGUAGE_NONE's but to no avail - changing the bootstrap.inc default does work, but that causes other problems, so thats not a solution for me.
Comment #280
ParisLiakos CreditAttribution: ParisLiakos commentedComment #281
carn1x CreditAttribution: carn1x commentedSorry if I'm being a noob but why was this closed if the last patch failed?
EDIT: Also I note that the latest release and dev (4 Nov 2014) do not resolve this issue, so I'm guessing it shouldn't be marked fixed.
Comment #282
carn1x CreditAttribution: carn1x commentedComment #283
carn1x CreditAttribution: carn1x commentedOK, after some investigation it's clear the patch in #251 is already in dev, and works for newly created field collection entities after updating.
However shouldn't this issue also address existing broken field collections or is that for another issue, as those are still broken. I'll review field_collection-fixup-broken-copies.txt and see if that's still in good shape.
EDIT: After applying field_collection-fixup-broken-copies.txt I still have field collection entities synchronised across languages.
Comment #284
ParisLiakos CreditAttribution: ParisLiakos commentedplease open new issues, this is about to go to 300 comments
Comment #287
alcroito CreditAttribution: alcroito commentedComment #288
RaulMuroc CreditAttribution: RaulMuroc commentedHas it been committed? Otherwise I would not set it as closed (fixed) but only fixed.
Comment #289
alcroito CreditAttribution: alcroito commentedYes, the patch for content translation support (node translation support) has been committed on July 3rd 2014, as seen in comment #265.
It is part of the latest DEV release, and as of 4th November 2014, it is part of the 7.x-1.0-beta8 release.
Comment #290
gge CreditAttribution: gge commentedUsing Field collection 7.x-1.0-beta8+2-dev and i18n 7.x-1.11, I'm having problems showing translated taxonomy terms in the correct language.
When a node is translated from english to german, the english node will display terms in german even if the displaying format is Link (localized). Terms are translated using "Translate. Different terms will be allowed for each language and they can be translated.", not "Localize. Terms are common for all languages, but their name and description may be localized."
Any advice?
Thanks.
Comment #299
carsato CreditAttribution: carsato commentedI've used the most recent version of field_collection module (beta11) and patched it (with no luck) with comment #251 patch. I haven't acomplished the field_collection translation of referenced entities, so I wrote my own workaround.
Basically I translate referenced entities on load just when on node translation form.
My solution consist of modifying the load function of field collections:
Not sure if it's the right solution, but I coudn't apply previous solutions based on comment #251 and beta11
Comment #300
carsato CreditAttribution: carsato commentedPatch rolled here for previous comment function.
Comment #301
carsato CreditAttribution: carsato commentedComment #302
victoriachan CreditAttribution: victoriachan at Torchbox for University of Oxford commented@carsato Can you please clarify what is your patch supposed to do, and perhaps link to the related comment? This issue has become way too long, and is very difficult to follow.
I'm using Field collection 7.x-1.0-beta11, i18n 7.x-1.13 *without apply any patches to field collection*, and the committed changes on this ticket works for me. I have 2 languages, English (default) and Welsh. I'm not using revision.
I tested creating a new node (language neutral), editing the field collections on the node, and all works well. I then changed the node to English, edited it, still works well. I then added a Welsh translation, edited it, and edited the English version again too, and both worked well.
Thanks everyone for your hard work! Hopefully this issue can be closed soon.
Comment #303
jmuzz CreditAttribution: jmuzz commentedThe patch in this issue has already been committed. There is no longer any need to apply it as long as you use the latest dev version of field collection. This issue has been closed (See #266). Please create a new issue if there are any further problems related to content translation and host entity cloning in the latest dev version of field collection and do not reopen this issue for additional support.