I do not know whether this is out of scope but did anyone ever consider integrating with the paragraphs module?
At the moment I am building a replication workflow with nodes having paragraphs attached to them. For this cause I had to create a paragraphs storage class using ContentEntityStorageTrait and implementing a custom database update method to keep its parent's target_revision_id in sync with the paragraph's revision_id (else any change to the parent node entity through Entity API would trigger ContentEntityStorageTrait's save logic and create a new revision each time) which is naturally prone to error.
Also I had to replace the EntityReferenceRevisions field type class (which the paragraphs module uses) with my own implementation which has a similar functionality to multiversion's EntityReferenceItem class. I think creating a trait and using it in the EntityReferenceRevisions and EntityReferenceItem classes would be a great follow-up issue.

Comments

alexej_d created an issue. See original summary.

DamienMcKenna’s picture

Might you be able to share some of your code so we could review it? Thanks.

alexej_d’s picture

Here is a link to my branch on Github. The code is a bit messy because I was short on time. But it seems so be working now. You also need to patch replication modules ContentEntityNormalizer for it to set the target_revision_id for entity_reference_revisions fields on denormalization.

DamienMcKenna’s picture

Title: Integration with paragraphs module » Integration with Paragraphs module

The fork isn't adding the appropriate record to the entity_reference_revisions field, is that what you're talking about? It is finally copying the Paragraph's field data though, but without the entity_reference_revisions record it's like a detached limb.

DamienMcKenna’s picture

I made a PR of your branch's changes: https://github.com/dickolsson/drupal-multiversion/pull/89

DamienMcKenna’s picture

The branch *does* handle revisions properly within the paragraph entities, which is good..

alexej_d’s picture

That's what I meant. You need to patch the ContentEntityNormalizer in the Replication module. Add this after this line here and it should work properly:

if($type === 'entity_reference_revisions') {
    $translation[$field_name][$delta]['target_revision_id'] = $target_entity->revision_id->value;
}
DamienMcKenna’s picture

DamienMcKenna’s picture

Here are @alexej_d's changes in patch format with minor code formatting changes.

Status: Needs review » Needs work

The last submitted patch, 9: multiversion-n2756417-9.patch, failed testing.

The last submitted patch, 9: multiversion-n2756417-9.patch, failed testing.

The last submitted patch, 9: multiversion-n2756417-9.patch, failed testing.

DamienMcKenna’s picture

To the maintainers - what should we do about tests? I'd love to get some added, but I'm not sure where they should go given the scenario involves about six modules (full Deploy suite plus Paragraphs and Entity Reference Revisions).

DamienMcKenna’s picture

Every single test passes locally:

$ simpletest8 --verbose multiversion

Drupal test run
---------------

Tests to be run:
  - Drupal\multiversion\Tests\CommentStatisticsTest
  - Drupal\multiversion\Tests\ConflictTrackerTest
  - Drupal\multiversion\Tests\EntityQueryTest
  - Drupal\multiversion\Tests\EntityStorageTest
  - Drupal\multiversion\Tests\MenuLinkTest
  - Drupal\multiversion\Tests\MigrationTest
  - Drupal\multiversion\Tests\MultiversionManagerTest
  - Drupal\multiversion\Tests\RevisionFieldTest
  - Drupal\multiversion\Tests\RevisionTreeIndexTest
  - Drupal\multiversion\Tests\SequenceIndexTest
  - Drupal\multiversion\Tests\UninstallTest
  - Drupal\multiversion\Tests\UuidIndexHooksTest
  - Drupal\multiversion\Tests\UuidIndexTest
  - Drupal\multiversion\Tests\Views\DeletedTest
  - Drupal\multiversion\Tests\Views\WorkspaceTest
  - Drupal\multiversion\Tests\WorkspaceTest
  - Drupal\relaxed\Tests\StubTest
  - Drupal\Tests\multiversion\Kernel\MultiversionIndexFactoryTest
  - Drupal\Tests\multiversion\Unit\DefaultWorkspaceNegotiatorTest
  - Drupal\Tests\multiversion\Unit\WorkspaceManagerTest

Test run started:
  Tuesday, August 2, 2016 - 21:55

Test summary
------------

Drupal\multiversion\Tests\CommentStatisticsTest               43 passes                           10 messages
Drupal\multiversion\Tests\ConflictTrackerTest                 17 passes                            2 messages
Drupal\multiversion\Tests\EntityQueryTest                     75 passes                            2 messages
Drupal\multiversion\Tests\EntityStorageTest                  351 passes                            2 messages
Drupal\multiversion\Tests\MenuLinkTest                        13 passes                            2 messages
Drupal\multiversion\Tests\MigrationTest                       57 passes                                      
Drupal\multiversion\Tests\MultiversionManagerTest             35 passes                                      
Drupal\multiversion\Tests\RevisionFieldTest                  162 passes                            4 messages
Drupal\multiversion\Tests\RevisionTreeIndexTest               42 passes                            4 messages
Drupal\multiversion\Tests\SequenceIndexTest                   22 passes                            2 messages
Drupal\multiversion\Tests\UninstallTest                       15 passes                            2 messages
Drupal\multiversion\Tests\UuidIndexHooksTest                  13 passes                            2 messages
Drupal\multiversion\Tests\UuidIndexTest                       15 passes                            2 messages
Drupal\multiversion\Tests\Views\DeletedTest                   30 passes                            6 messages
Drupal\multiversion\Tests\Views\WorkspaceTest                 23 passes                            4 messages
Drupal\multiversion\Tests\WorkspaceTest                       83 passes                           16 messages
Drupal\relaxed\Tests\StubTest                                  5 passes                                      
Drupal\Tests\multiversion\Kernel\MultiversionIndexFactoryTes   1 passes                                      
Drupal\Tests\multiversion\Unit\DefaultWorkspaceNegotiatorTes   2 passes                                      
Drupal\Tests\multiversion\Unit\WorkspaceManagerTest            6 passes                                      

Test run duration: 12 min 55 sec
DamienMcKenna’s picture

Lets see if adding a test dependency for entity_reference_revisions will fix the problem.

Status: Needs review » Needs work

The last submitted patch, 15: multiversion-n2756417-15.patch, failed testing.

The last submitted patch, 15: multiversion-n2756417-15.patch, failed testing.

The last submitted patch, 15: multiversion-n2756417-15.patch, failed testing.

jeqq’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 15: multiversion-n2756417-15.patch, failed testing.

The last submitted patch, 15: multiversion-n2756417-15.patch, failed testing.

The last submitted patch, 15: multiversion-n2756417-15.patch, failed testing.

The last submitted patch, 15: multiversion-n2756417-15.patch, failed testing.

jeqq’s picture

@DamienMcKenna, @alexej_d Thank you for working on this!

There should be a check if the field exists, otherwise it always will require the Entity Reference Revisions module in EntityReferenceRevisionsItem.

jeqq’s picture

Status: Needs work » Needs review
DamienMcKenna’s picture

Thanks for working out what the problem was with the tests. One minor thing - I don't think there's anything wrong with adding entity_reference_revisions as a test dependency, it'll make it easier to add tests for the integration.

DamienMcKenna’s picture

IMHO it might be worth adding test_dependencies lines for each of the supported modules - entity_reference_revisions, uuid, media, etc. That way when someone writes tests for them you won't run into chicken-and-egg problems because of the new test_dependencies. (see https://www.mediacurrent.com/blog/how-fix-testbot-bug-when-adding-new-tests for more details)

dixon_’s picture

In general I would like contrib modules to integrate with Multiversion within their own modules (not the other way around as with this patch). This makes more sense as Multiversion will get into core in the comings months and cease to exists. See #2721129: Workflow Initiative.

So it's better that the integrations live where they can be fully maintained moving forward.

However, Paragraph module have a lot of usage and I could be ok with committing this patch if someone goes ahead and writes decent test coverage for this patch :)

dixon_’s picture

Issue tags: +Needs tests
DamienMcKenna’s picture

Title: Integration with Paragraphs module » Integration with the Multiversion module
DamienMcKenna’s picture

Title: Integration with the Multiversion module » Integration with the Paragraphs module
Grimreaper’s picture

Status: Needs review » Needs work

Hello,

I have tested patches from the issues #2780877-4: Add Multiversion support to Paragraphs and #2674882-7: Add Multiversion compatibility to Entity Reference Revisions (for Paragraphs, etc). It does not work.

I have tested on Drupal 8.1.10 and with the dev version of paragraphs and the deploy suite.

Also it seems that the patch introduce a dependency on the entity_storage_migrate module, no?

friera’s picture

The patch doesn't work for me.

undertext’s picture

Here is the patch for paragraphs integration, new one, with another approach.

Small description what have been done:

The «Entity reference revisions» field is like «Entity reference» field with extra handling.
This extra handling is related to fact that "entity reference revisions» fields assume that referenced entity can store information about its parent. So Paragraphs entity has columns «parent_id» and «parent type» in the database.
And the logic of the postSave() method in EntityReferenceRevisionsItem.php is pretty simple - update the entity with parent information without creating new revision after the parent entity has been saved . The more clear explanation:
1) We create new node with new paragraph field
2) When paragraph field is saving it creates paragraph entity without parent information (because we do not know the parent node id on this step)
3) Node is saved, know we know the parent id.
4) Paragraph entity is updated in postSave with parent node information.

The current multiversion storage forces new revision on every save() call. But we do not need this on postSave.
So the method has been added to ContentEnityStorageInterface and ContentEntityStorageTrait - saveWithoutForcingNewRevision().

Also the default EntityReferenceRevisionsItem class was replaced with multiversion-supported version.
The changes comparing to default EntityReferenceRevisionsItem class are next:
save() call was replaced by saveWithoutForcingNewRevision() call.
Added code for supporting stubs and handling references to already created nodes.

undertext’s picture

Status: Needs work » Needs review
undertext’s picture

Change namespace of test to right one.

Status: Needs review » Needs work

The last submitted patch, 37: integration_with-2756417-36.patch, failed testing.

undertext’s picture

Add @requires annotation for test bot.

undertext’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 39: integration_with-2756417-37.patch, failed testing.

undertext’s picture

Status: Needs work » Needs review
FileSize
19.4 KB

Try one more time.

Status: Needs review » Needs work

The last submitted patch, 42: integration_with-2756417-38.patch, failed testing.

undertext’s picture

Status: Needs work » Needs review
FileSize
19.67 KB

Ok, changing patch according to this one
https://www.drupal.org/node/2692407#comment-11833737

Status: Needs review » Needs work

The last submitted patch, 44: integration_with-2756417-39.patch, failed testing.

undertext’s picture

Assigned: Unassigned » undertext
Status: Needs work » Needs review
FileSize
19.87 KB

Adding missing drupal modules dependencies to composer.json.