Field collection doesn't play nice with workbench moderation (patch)

Patch works well for me on the node edit form against latest dev version. I am having trouble with the Embedded option and revision control (see: #1811222: 'In Content' add/edit actions don't respect Node Revisions).

I am experiencing issues as well. When saving a node with fieldcollection i get an error like this in my drupal log:
Exception: Unable to save a field collection item without a valid reference to a host entity. 1401 revision:54037 in FieldCollectionItemEntity->save() (line 473 of ../sites/all/modules/field_collection/field_collection.module).

Patch at #1 works for me when applied to 7.x-1.0-beta5, using embedded forms.

See also:

#1844322: Unable to save draft when adding new field values on published node (workbench moderation)
#1443604: Ability to Incorporate -- Field collections -- Into Workbench Moderation

Let's at least see if we can get this committed, since the added test does no harm.

If there is any concern about adding module-specific code, well, there are only two revision moderation modules in general use for Drupal 7, and unlikely to be any more now that revision management looks like it will be in core for Drupal 8.

@kscheirer said on January 28, 2013 at 8:52pm in a different issue that has since moved to this issue...

"@TheSasquatch - If you're willing to work with "new module, either a release or dev" I'm not sure what you think the difference is between applying a patch and using a dev version of a module."

Our department policy it to use releases, betas, and even alphas and devs if necessary, because they are released by the official maintainer of the module, for which reputation is also a factor. But patches, if I understand correctly, can be contributed by anyone, are typically experimental, and are often a less vetted developer's way of saying, "Here's a proposed fix," that you may or may not find acceptable/reliable.

...just a little background into our policy.

In this case, since I'm desperate, and since it might help solve the problem, I would be willing to try the patch as long as...
* It is your opinion that the patch is reasonably safe.
* You are reasonably sure I would be able to revert back without database issues if the patch turns out to cause problems down the road.

If I go ahead with this patch, can I apply it to the current dev? If not, and if I must first revert back to beta5, do you forsee any issues with reverting from current dev to beta5? Anything special I should do, like run an uninstall, or can I just go through the usual module "update" process (reverting in this case)?

++TimLeytens Patch works for us.

I understand that, and I like that community model. I was just explaining our internal decision. Some orgs don't even do alphas and betas in production. For us, we made an exception because we understand the Drupal community, and often alphas and betas are viable. We just draw the line at patches. Another reason we avoid patches is because they are often removed with security patches and other updates, and then have to be reapplied, which increases our maintenance overhead.

But like I said, we're desperate, and so we'll make an exception this time. We also like the idea of testing the patch on behalf of the rest of the community.

Sorry for the tangent.

I'll give the patch a try and report back on our (hopeful) success.

Thanks!

Field Collection and Workbench Moderation works great together for me using the latest version (7.x-1.0-beta5).

Exact steps to reproduce the problem?

Patch #1 is required if you create a NEW node with a field collection and store NO values in it.
Then you revise the node to set the initial values for that node. The Field collection data, doe not save.

Another way to look at it is

You have a whole bunch of published content. Then you add a new field collection to the

/admin/structure/types/manage/XXX/fields/

When you then try add the content to those nodes via a new revision the values are not saved.

Patch #1 worked great for me! Not only did it fix the most recent bug, but it also seems to have fixed the bug that made field collections *NOT* revision-able. (Maybe that was fixed with the latest beta, too, but I'm using the dev plus the patch.) It also fixed a bug that was affecting only some of our content types with fields collections in which we could only add one new instance of the collection per save. So if we wanted to add 5 more instances in a field collection field, we had to save 5 times between each new instance. That is now fixed. I'm not sure at what point it was fixed, but with our current dev + patch #1 setup, it is fixed. *PLEASE* put this latest patch into a new beta or release. THANK YOU!!!!

Patch works for us! Thanks @TimLeytens!

Patch worked for me against the DEV of 26 Dec 2012. ThanksTim

Because workbench moderation sets the node revision to '0' in the shutdown function 'workbench_moderation_store' and does a node_save, triggering 'field_collection_field_update', where in case $entity->revision is empty, field_collection starts deleting the revisions.

What is workbench trying to achieve here? If work bench issues a wrong node_save() the bug is over there.

That is the only way we have to save forward revisions in D7 as node_save is not flexible as it need to be. Node is saved twice. This will change all in D8.

Grazie!

+1 looks like this one is ready to go in!

@fago - can we get this committed?

The patch at #1 works but adds workbench_moderation specific code. Over at #1781190: Field Collection saved on presave I have proposed a more generic alter() hook that will allow other modules to stop field_collection deleting newly added items.

Field Collection Workbench Moderation implements that hook to prevent deletion, and also ensures moderated field values are displayed correctly when accessed through a Views relationship on the field collection.

The attached patch against current versions of field_collection also provides the same hook.

This may not really work for other modules. I was also required to add workbench specific code to linkchecker to stop it from processing. We cannot add to every module in the Drupal world a new hook just for this reason. Checking for $entity->workbench_moderation['updating_live_revision'] is the easiest solution. The only helper function I can think of is like _linkchecker_isdefaultrevision() that allows the module maintainers to maintain a list of workbench like modules and to make life easier it we get isDefaultRevision in Drupal 8.

Reattaching the RTBC patch from #1.

Fair enough, if there are other modules that get screwed up by Workbench Moderation. I'll remove my proposal from #1781190: Field Collection saved on presave as well. When/if that patch is committed then #22 may need to be re-rolled - though if this check is moved to immediately before the if ($original_by_id) check, then it may still apply?

The issue is not Workbench itself in general - if it may sounds sooo. The underlying issue is forward revisions. Every module that implements forward revisions will run into this issue. Workbench is only one of the most popular from my point of view.

Understood. It would be desirable if there was an agreed common entity property that any module implementing forward revisions could set if doing the re-save, rather than every affected Entity API module needing to implement a check for the existence of each module's property. I guess it would need to be implemented by Entity API so the property is accessible from an entity wrapper.

In D8 the version that is saved "sets" isDefaultRevision property to true as I know. See my link and D8, please.

I was just thinking that for D7, which will need to be supported for a long time yet, and we don't know whether the D8 functionality will ever get backported, it would make sense to provide that sort of functionality in a temporary common API.

_linkchecker_isdefaultrevision() checks whether the entity is in the process of being updated from the Workbench Moderation shutdown function, not that the entity is the default revision. If called outside the context of the shutdown function then $entity->workbench_moderation['updating_live_revision'] is not set, so you'll get FALSE for any revision. Perhaps the function name is misleading?

(removed code that works for field collection but may screw up other modules)

Thanks to @hass for patient application of the clue-bat to my head.

This patch fixed some pretty serious bugs I was encountering when saving as draft with workbench_moderation while adding a new item to the field collection on that node (I've also noticed that changing the "Hide empty values" field setting sometimes has an affect on this). My issues were that if I had a published revision and edited it and ONLY added a new collection item the node have it's published revision set to draft and no new revision would be created.

Patch #1 applied to Field Collection 7.x-1.0-beta5+1-dev with Entity 7.x-1.1 and Workbench Moderation 7.x-1.3 installed fixes the following scenario:

SCENARIO:
Create new node and add one field collection group. Save as draft. Publish this draft.
Create new draft of published node. Add an additional field collection group. Save.
- This unpublishes this node.
- If I edit the draft, the 2nd field collection group I added in revision of published node is gone.
- If I attempt to add the field collection group again: Workbench Moderation state is draft but Publish is ON. Also view draft is gone and view published does not show new field collection group.

APPLIED PATCH in #1:
- [Action] Created new draft of published node.
- [Action] Added new field collection group.
- [Action] Save (without publishing this revision)
- [Result] New field collection group appears in View Draft as expected (PASS)
- [Result] Node remains published (PASS)
- [Result] In Published view, field collection group added in unpublished revision does not show as expected (PASS)
- [Action] Published revision with new field collection group.
- [Action] View Published
- [Result] New field collection group appears as expected. (PASS)

I have more testing to do on 2 other instances of field collections on my particular content type. Also need to do further testing in the context of Panelizer, which I believe will be out of the scope of this issue.

Applied patch in #1. Everything seems to check out fine on our end.
Using Workbench 7.x-1.2, Field Collection 7.x-1.0-beta5

@TimLeytens, Thanks for the patch, would've taken our team a while to figure that one out.

Has the latest version of the patch been tested with a nested field collection, that is, a field collection contains another field collection?

I tested that very briefly and it appears to work. The patch runs its check whenever any field collection is saving itself, so if a nested field collection is being saved as a result of its parent being saved from workbench moderation's re-save of the node, then the deletion of revisions should also be prevented for the nested field collection.

But test it yourself, before you do anything with real data.

Patch #22 works for me.

I did have to add the extra lines manually though.

function field_collection_field_update($entity_type, $entity, $field, $instance, $langcode, &$items) {
  // Prevent workbench moderation from messing up our field_collection
  if (!empty($entity->workbench_moderation['updating_live_revision'])) {
    return;
  }
$items_original = .....................

@ robearls -- why the extra line?

Can you update and provide a new patch?

Are you saying that the patch didn't apply cleanly? This is how I interpret your comment, but I think clarification is in order.

Thanks for the patch, works for me.

The patch from #22 applied cleanly. I also installed the Field Collection Workbench Moderation module from John Pitcairn's sandbox. Now everything seems to be working as expected.

I've applied the patch in #1807460-22: Field collection doesn't play nice with workbench moderation (patch) and #1901892-19: Most recent Field Collection revision always appears in View when using Workbench Moderation, with a slight amendment to the patch from the other issue (removing 'wbm' so that the hook fires).

Things seem to be OK when creating a new field collection.

But, if you apply this on an existing site, the behaviour is not as expected.

Test steps:

• Create a new draft of a published node with an attached field collection
• When viewing the draft version, things look OK.
• When viewing the published version, a different field collection is displayed. I think it is the relevant field collection with the lowest ID.

If you then publish your new revision edit the node again, normal service is resumed - creating new drafts will not affect the published version.

So I think the workaround is to apply the patches, then save new (identical) revisions of all published nodes with field collections and publish the new revisions.

I'll look at a patch where this saving happens in an update hook (the manual fix isn't feasible on a site with as much content as the one I'm working on).

removed duplicate post

+++ b/field_collection.module
@@ -923,6 +923,12 @@ function field_collection_field_presave($host_entity_type, $host_entity, $field,
+  // Prevent workbench moderation from messing up our field_collection

Misses trailing point and should explain why this is necessary.

So why is it?

That is the only way we have to save forward revisions in D7 as node_save is not flexible as it need to be. Node is saved twice. This will change all in D8.

Yes, I know. But why does that lead to field collection behaving wrong? Saving the node twice shouldn't be problematic per se?

Updated comment and re-rolled against current dev release.

I happen to agree with @fago on this one. Field collection doesn't seem to be doing anything wrong in here.

If anything the fix should really be in the other modules.

That being said this is a temporary patch if you happen to be using workbench_moderation-7.x-2.x-dev and state_machine-7.x-3.x-dev.

This is mostly here if anyone needs it for these specific version of the module.

I don't think it's a question of whether field collection is doing something "wrong" or not. Core doesn't support content moderation, so technically contrib modules don't have to support it either. However, IMHO, this is a major, no good, very bad oversight in Drupal. Workbench Moderation (among other modules) attempts to correct that oversight, but is forced to apply some unfortunate hacks to do so. Namely, it must effectively save each node twice on every save—once to create a forward revision, and again to set the node table back to the previously published revision. The second time it saves the node it has to set $node->revision = 0, to suppress the creation of yet another revision. There's simply no getting around that.

Unfortunately, when you edit a field collection and save a draft, the Field Collection module gets very confused. It makes some assumptions that simply don't hold and, during Workbench Moderation's second node_save(), it deletes the new field collection revision, thinking it's unused.

As far as I can tell (and I don't really consider myself an expert here), there is no way for Workbench moderation to fix this error. The best it can do is announce its intentions, which it does by setting $node->workbench_moderation['updating_live_revision'] to TRUE during that second save. The only way to get Field Collection to work with Workbench Moderation is for Field Collection to change its assumptions. I know this is not ideal, but it seems like a necessary evil.

It looks like there was an error in #42: $entity should be $host_entity. Here's a re-roll.

Commit this now, please. As jstoller said there is no other way to solve this in D7. Core node_save() cannot changed and is the root cause.

I can confirm that patch in #46 works.

#46 working.

Also need this one or you will run into some nasty stuff when deleting revisions:
#2000690: Deleting revisions via node_revision_delete wrongly deletes entire field collection
Patch #9

I can confirm that this patch works, at the latest Field collection dev version. I have not used the sandbox project mentioned above (after all it is not available anymore).

I agree this should be committed.

Whether the root problem is in workbench moderation or core, the field collection module should not allow its data to be removed as a side effect of the operations workbench moderation performs.

@nasia123, the Field Collection Workbench Moderation module from John Pitcairn's sandbox can still be downloaded from the git repository, and I myself have it on a couple sites right now, but to be honest I'm not sure if its necessary. I'm just afraid to remove it.

@John Pitcairn, can you shed any light on this? Is your sandbox module necessary? Is it adding anything I'll miss if I remove it? Between your summary on the module page and your comments on various issues, I'm now quite confused.

I added the sandbox module, I saw no difference, perhaps someone needs to test this more

The sandbox module is still necessary to resolve the Views issue for me, but there appear to be unwanted side effects if you are using a Content Revision view.

Hi,

The sandbox module work nice for me but i face to another problem.

When node is marqued as "Published", it become impossible to save any change in the field collection.

Is someone have the same problem ?

Thanks !

I had the same issue as mentioned in #55.
When I have a published node and try to edit the filed collection items in the node, it retains the published revisions values.I use workbench moderation for content moderation, when I remove the check box for moderate revision it works fine.

ok, if it can be solved only this way let's add it then. Committed!

46: field_collection_with_workbench_moderation-1807460-46.patch queued for re-testing.

The patch was committed, it doesn't need to be applied now.

This is just an update to the patch in #43

Ignore me...

ignore #62 ...

I should probably open a new issue, but I'm having an issue with this.. It works fine, unless you have a field collection that has a field collection in it. Then, any changes to the nested field collection on a new node revision changes all revisions.

Thoughts?

I'll just open a new issue: #2243595: Nested field collections don't play nice with workbench moderation (revisions)

46: field_collection_with_workbench_moderation-1807460-46.patch queued for re-testing.

Hello guys,

I think my issue is related to it.
However I applied all the last patch and it does not seem to work.

My issue is as follow:

I have a content type having a field_collection. This field collection contains text and one picture. Admin can create as much as field collection wanted. => collec01 (txt+img) ; collec02 (txt+img) ; collec02 (txt+img)........

After deleting the picture from this collection, it is impossible to make a new one saved.
This is not the case with the text. When the text is updated, the change is taken into account.
When this is the image, the field stay as deleted, that is the new image is never saved.

As I said I tried the patches but nothing change. The only way to make it work is to remove the collection itself and create a new one...(quite obvious by the way, since the revisokn doesn't exist for the newly created)

I don't see anything in my logs. FYI, I use AWS S3 with Storage API, but I don't think this is the root of the issue.

Did someone experienced the same issue? may be I missed something...

Best regards gentlemen/women.

@adrientoulousain - that does sound different though. I'd open up an new issue for this, as this one was marked closed, so there won't be much response to this one anymore.

Hi

To work it out the https://www.drupal.org/node/1807460#comment-7908001 version of 7.x-1.0-beta11 for field_collection module. have updated the patch.

Not sure I'm on the right track, but wondering why the hook_workbench_moderation_transition call from https://www.drupal.org/node/1807460#comment-11059599 (and https://www.drupal.org/node/1807460#comment-7908001) can't be put in a custom module

@alimc29: See my sandbox module, field_collection_wbm. I believe that is where @manishmore has taken the code in the patch from, without crediting me.

I think it unlikely this code will ever be included in the field collection module, for reasons explained in the other related issue, #1901892: Most recent Field Collection revision always appears in View when using Workbench Moderation.

Note that this issue is marked closed (fixed), since the original problem as outlined in the summary has indeed been fixed.