Problem/Motivation

We recently added behavior plugins to Paragraphs, unfortunately the serialized field where the values are stored is not revisionable and can create compatibility problems with diff module.

Proposed resolution

Make the field revisionable, since the field can be used already we need to provide an update function adding field tables manually.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yongt9412 created an issue. See original summary.

johnchque’s picture

This should work. :).

miro_dietiker’s picture

Priority: Normal » Critical
Status: Needs review » Needs work
Related issues: +#2836010: Paragraphs Diff behavior plugin support

Let's make sure we have this stuff connected.

Berdir pointed out that this update is not enough if data exists.

Berdir’s picture

Yes, the standard update functions don't work if there is data and they need to make altering changes.

If you run this with data in there then it will fail.

You need to make the schema changes by hand and then update the stored metadata. See poll.install for an example.

Or maybe we could delete the data and then do the update, but we need to migrate the data to the new storage, so getting it out first.

miro_dietiker’s picture

I don't think anyone has data we need to migrate currently, so deleting the plugin storage column if already existing and recreating it would also be OK?
(Risk: If someone reruns this update method later accidentally, data is truly lost.)

Berdir’s picture

I was thinking about that. If we do it before the next release, then yes, maybe. chances of someone already using this seem to be very small.

Then we could just do an update query to set the field to NULL, an then call that update, that should work. Make sure to test it with having data.

johnchque’s picture

Not sure if this is the way to make this update work. Wouldn't we instead drop everything related with the field and add it again?

Status: Needs review » Needs work

The last submitted patch, 7: make_the_behavior-2838657-7.patch, failed testing.

johnchque’s picture

Oops, sorry, didn't have to remove the field.

Status: Needs review » Needs work

The last submitted patch, 9: make_the_behavior-2838657-9.patch, failed testing.

johnchque’s picture

Status: Needs work » Needs review

Test-bot problem. :/

Berdir’s picture

Assigned: Unassigned » Berdir

Nope, that's not what I meant. I'll give it a try.

Berdir’s picture

This is, much simpler.

This is why we need to empty the data first, so we can get around the limitation of the API of not supporting changes if there is data. But it works perfectly if the column is empty.

Also noticed that we didn't had the default value of the field set to a serialized array, so new paragraphs created in the API or when nobody called setBehaviorSetting() were NULL.

Berdir’s picture

Assigned: Berdir » Unassigned

  • miro_dietiker committed 36cf9b7 on 8.x-1.x authored by Berdir
    Issue #2838657 by yongt9412, Berdir: Make the behavior plugins field...
miro_dietiker’s picture

Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.