Editable layout does not work well when portal_type has a Schema

Describe the bug Editable blocks layout works fine on the standard Document type, but it has problems on types that have a Schema:

When you add or edit an instance of such a type, the blocks behavior seems completely absent: you get a "normal" form with fields and field sets. With the standard blocks behavior all is fine.
The control panel for editing the layout never shows up, it keeps saying the blocks behavior is read-only.

The first problem led me to believe that I should enable both the volto.blocks and volto.blocks.editable.layout behaviors. But that is not what the "Enable editable blocks" button in the Layout control panel does: it switches off the standard volto.blocks (if this is enabled) and enables volto.blocks.editable.layout. And that works fine for Documents.

Let me first say how to solve this, and then explain how to reproduce this.

In plone.restapi.behaviors the IBlocksEditableLayout class needs an extra line @provider(IFormFieldProvider), just like IBlocks already has. This provider does not get inherited. Without this provider, the editable layout behavior seems to have no fields because plone.dexterity.utils.getAdditionalSchemata finds no form schema for it. (I don't know why this part does work for the standard Document type.)

This change leads to two differences:

The add/edit forms of our portal_type with schema properly show blocks. (The standard Document remains working as well.)
There is a change in the properties that are returned in the json for the FTI. Originally the blocks (and blocks_layout) field got reported as belonging to a behavior with a name like plone.dexterity.schema.generated.Plone_5_1721241704_2_695599_0_Document (an on the fly Schema for Documents) or whatever "real" Schema you have in the Schema portal type. But now it belongs to the behavior that you would expect: volto.blocks.editable.layout

That last change means that in Volto the check for readOnlyBehavior in ContentTypeLayout.jsx needs to be updated. It should continue to check for generated as it does now (so it keeps working for Documents when someone uses a current plone.restapi release). But it should also check for volto.blocks.editable.layout so it works when the plone.restapi fix has landed. So something like:

this.setState({
    readOnlyBehavior: !(blocksBehavior === `volto.blocks.editable.layout` || blocksBehavior.includes('generated'))
    ? blocksBehavior
    : '',
});

I can make PRs later.

Wait, now it seems I can go to the layout page, I can add blocks and save the layout, but when I reload the page, I have a blank layout again ?? I need to check what is going on there... :-/

To Reproduce Steps to reproduce the behavior:

Go to http://localhost:3000/controlpanel/dexterity-types
Create a new type, let's call it Bug.
Go to its FTI in the ZMI: http://localhost:8080/Plone/portal_types/bug/manage_propertiesForm
In the Schema line put: plone.supermodel.model.Schema.
Go to the Layout page of this type in Volto: http://localhost:3000/controlpanel/dexterity-types/bug/layout
You see: "Can not edit Layout for Bug content-type as it doesn't have support for Volto Blocks enabled"
Click 'Enable editable blocks'
You get an info box with "Changes saved", but then you see a message: "Can not edit Layout for Bug content-type as the Blocks behavior is enabled and read-only"

Software (please complete the following information):

Volto 18.0.0-alpha.30 (tested with latest alpha 41 as well)
Plone 6.0.11.1
Plone REST API 9.6.1

@mauritsvanrees As discussed this afternoon, this issue looks similar to a previous one that was reported and fixed on plone.restapi as that's where the behaviors are implemented: https://github.com/plone/plone.restapi/issues/1476.

There the inheritance was fixed so that IEditableBlocks inherits from IBlocks if I understand it correctoy, but as you write the decorators are not inherited.

I am beginning to fear that what we want is not possible. The problem is:

In a standard Plone Site when you go to http://localhost:3000/controlpanel/dexterity-types/Document/layout you see "Can not edit Layout for Page content-type as the Blocks behavior is enabled and read-only". As expected.
You click the "Enable editable blocks" button.
What happens then? Three things:
- The volto.blocks behavior is removed.
- The volto.blocks.editable.layout behavior is added.
- The model source attribute is changed. You get the following (ignoring the namespaces):

  <schema>
    <fieldset name="layout" label="Layout">
      <field name="blocks" type="plone.schema.jsonfield.JSONField">
        <default>{}</default>
        <description/>
        <required>False</required>
        <title>Blocks</title>
      </field>
      <field name="blocks_layout" type="plone.schema.jsonfield.JSONField">
        <default>{'items': []}</default>
        <description/>
        <required>False</required>
        <title>Blocks Layout</title>
      </field>
    </fieldset>
  </schema>

This is done by Volto in onEnableEditableBlocks.

So when editable blocks are enabled, the two fields are taken from the schema. This explains why it is fine that the IBlocksEditableLayout class is not defined as an IFormFieldProvider: it does not need to provide fields, because they are defined in the model_source.

And they must be defined in the model_source attribute, because that is the only model-related FTI attribute that you can edit through the web. Where else would you save these field settings?

That explains why the current way is fine for the standard Document type, which does not have a schema class.

In our use case, with a portal_type that has a filesystem schema class (like the empty plone.supermodel.model.Schema), this does not work. The same changes are made when you click "Enable editable blocks": the FTI gets a model_source attribute with exactly the schema copied above, with blocks and blocks_layout fields. So that part is actually good. And when I temporarily simplify the readOnlyBehavior check to this.setState({readOnlyBehavior: ''}) (so making sure readOnlyBehavior is false) I can see the layout page. And I can add blocks there and save it, and I can reload this page and the new blocks stay. And they show up on the Add-form of this type.

But the model_source in the FTI is not actually changed. So I think this is only in memory. Indeed: when I restart, the layout page no longer loads. It complains that the type has no support for Volto Blocks. The blocks are gone from the Add-form of this type as well.

Note that an FTI has a lookupSchema method that first checks if there is a valid schema class and then uses it. Only when there is no such schema, do we get end up with a "generated" schema where lookupModel is used, which first looks in the model_source, else model_file, else schema.

In other words: when an FTI defines a schema class, the model_source gets ignored, and that is the only place where the custom layout of blocks could be stored.

In fact, you get the same problem when you try to add a simple field, not blocks related, even just in Classic UI:

Create an FTI
In the ZMI set the schema property to plone.supermodel.model.Schema (or whatever real schema you want).
In ClassicUI in the control panel of this type add a field. (I did not realise until this week that Volto does not have the option of adding a field.)
This seems to work, the model_source gets updated, although strangely in the UI you see the field three times, both in the control panel and on an add-form.
But restart the site and the field is gone.

So: this is not going to work. :-(

I am beginning to fear that what we want is not possible. The problem is:

I've scanned your analysis, but am not grasping it yet fully. The stranges part: If it is not possible, why has apparently worked for 3-4 years since this feature has been implemented? Or is the implementation used in customer projects different from the core implementation? Also: this is Plone backend, assuming this did work

@avoinea @ichim-david @tiberiuichim Would you be so kind to check or ask around with colleagues how you have been using this feature in past projects? I assume it was developed as a functional requirement at the time.

@mauritsvanrees theory/workaround if we would add the FTI model as an xml model that gets loaded on startup instead of Python schema class, then that is loaded in the model_source. Is that the way this could still work in current implementations?

Yes, switching to an XML model instead of a Python schema would work.

Or start with standard blocks behavior, hack Volto the way I do above (forcing readOnlyBehavior to false), click around to get a nice layout, save it, probably get an error but ignore it, and check what was submitted on the network tab as a PATCH request. Take the defaults of blocks and blocks_layout from there, and put them in those fields in your own Python schema. In a client project I now have this as proof of concept:

from plone.dexterity.content import Container
from plone.restapi.behaviors import BLOCKS_SCHEMA
from plone.restapi.behaviors import LAYOUT_SCHEMA
from plone.schema import JSONField
from plone.supermodel import model
from zope import schema
from zope.interface import implementer

# sample blocks with a carousel
BLOCKS_DEFAULT = {
    "a483a3d3-145c-4b58-a060-6ffb5af8f8f2": {"@type": "title"},
    "94e5f97b-0962-4ce0-8f74-ac0937911314": {
        "data": {
            "blocks": {
                "eada8880-cf8a-4167-9e6a-d4ccdc6674a4": {
                    "blocks": {
                        "ba3b0085-17a5-427b-92c2-0bdd0adfefce": {"@type": "slate"}
                    },
                    "blocks_layout": {
                        "items": ["ba3b0085-17a5-427b-92c2-0bdd0adfefce"]
                    },
                },
                "25f1c942-4f5a-4717-adf2-682f707c4372": {
                    "blocks": {
                        "096d972e-e20a-423c-b86f-940a5b082217": {"@type": "slate"}
                    },
                    "blocks_layout": {
                        "items": ["096d972e-e20a-423c-b86f-940a5b082217"]
                    },
                },
                "68c839ed-16cf-4589-bbb0-8848553fa9dc": {
                    "blocks": {
                        "b8b23b0c-c527-4a67-a34f-01e815950191": {"@type": "slate"}
                    },
                    "blocks_layout": {
                        "items": ["b8b23b0c-c527-4a67-a34f-01e815950191"]
                    },
                },
            },
            "blocks_layout": {
                "items": [
                    "eada8880-cf8a-4167-9e6a-d4ccdc6674a4",
                    "25f1c942-4f5a-4717-adf2-682f707c4372",
                    "68c839ed-16cf-4589-bbb0-8848553fa9dc",
                ]
            },
        },
        "right_arrows": True,
        "collapsed": True,
        "non_exclusive": True,
        "filtering": False,
        "styles": {},
        "@type": "accordion",
    },
    "undefined": {
        "@type": "slate",
        "value": [{"type": "p", "children": [{"text": ""}]}],
        "plaintext": "",
    },
    "6d32b823-bf5f-4d32-b9f3-2290b4ca9386": {"@type": "slate"},
}
LAYOUT_DEFAULT = {
    "items": [
        "a483a3d3-145c-4b58-a060-6ffb5af8f8f2",
        "94e5f97b-0962-4ce0-8f74-ac0937911314",
        "6d32b823-bf5f-4d32-b9f3-2290b4ca9386",
    ]
}

class ICijfer(model.Schema):
    """Marker interface and Dexterity Python Schema for Cijfer"""

    # ... other fields ...

    # next lines copied from plone.restapi.behaviors,
    # but with our own default values.
    model.fieldset("layout", label=_("Layout"), fields=["blocks", "blocks_layout"])

    blocks = JSONField(
        title="Blocks",
        description="The JSON representation of the object blocks information. Must be a JSON object.",  # noqa
        schema=BLOCKS_SCHEMA,
        # default={},
        default=BLOCKS_DEFAULT,
        required=False,
    )

    blocks_layout = JSONField(
        title="Blocks Layout",
        description="The JSON representation of the object blocks layout. Must be a JSON array.",  # noqa
        schema=LAYOUT_SCHEMA,
        # default={"items": []},
        default=LAYOUT_DEFAULT,
        required=False,
    )

@implementer(ICijfer)
class Cijfer(Container):
    """Content-type class for ICijfer"""

I've scanned your analysis, but am not grasping it yet fully. The stranges part: If it is not possible, why has apparently worked for 3-4 years since this feature has been implemented?

Just to be clear on this point still. It has probably always only worked for portal_types that have their schema defined in XML. Or that do not have a schema at all yet, and only have behaviors. Or types that have a model_file on file system, because the model_file is the one that is checked last.

Corrolary is also: if the type has a model_file with some fields in it, and you enable the editable layout, then the fields in the model_file will be ignored, at least after restart. (I did not try this, but this is what I expect based on my current knowledge.)

Document has plone.app.contenttypes.schema:document.xml as model_file, but it is basically empty, so is not really affected.

I am beginning to fear that what we want is not possible. The problem is:

I've scanned your analysis, but am not grasping it yet fully. The stranges part: If it is not possible, why has apparently worked for 3-4 years since this feature has been implemented? Or is the implementation used in customer projects different from the core implementation? Also: this is Plone backend, assuming this did work

@avoinea @ichim-david @tiberiuichim Would you be so kind to check or ask around with colleagues how you have been using this feature in past projects? I assume it was developed as a functional requirement at the time.

I would say that enabling the the Editable Layouts has been this flaky from the beginning. I mean: it has worked when the relevant conditions were met, but when you have the conditions as explained by Maurits it was working as explained.

We have faced several issues when enabling the Volto blocks TTW, and we have always seen this kind of behavior. We ended going to the ZMI, and removing/adding the relevant properties in the model_source.

I would say that we would need to identify when is supossed to work OK, and allow only setting the behavior on that cases.

I am beginning to fear that what we want is not possible. The problem is:

In a standard Plone Site when you go to http://localhost:3000/controlpanel/dexterity-types/Document/layout you see "Can not edit Layout for Page content-type as the Blocks behavior is enabled and read-only". As expected.

You click the "Enable editable blocks" button.

What happens then? Three things:

The volto.blocks behavior is removed.

The volto.blocks.editable.layout behavior is added.

The model source attribute is changed. You get the following (ignoring the namespaces):
  <schema>
    <fieldset name="layout" label="Layout">
      <field name="blocks" type="plone.schema.jsonfield.JSONField">
        <default>{}</default>
        <description/>
        <required>False</required>
        <title>Blocks</title>
      </field>
      <field name="blocks_layout" type="plone.schema.jsonfield.JSONField">
        <default>{'items': []}</default>
        <description/>
        <required>False</required>
        <title>Blocks Layout</title>
      </field>
    </fieldset>
  </schema>
This is done by Volto in onEnableEditableBlocks.

So when editable blocks are enabled, the two fields are taken from the schema. This explains why it is fine that the IBlocksEditableLayout class is not defined as an IFormFieldProvider: it does not need to provide fields, because they are defined in the model_source.

And they must be defined in the model_source attribute, because that is the only model-related FTI attribute that you can edit through the web. Where else would you save these field settings?

That explains why the current way is fine for the standard Document type, which does not have a schema class.

@mauritsvanrees Indeed, this is exactly how it works :see_no_evil:

We also use this as a behavior in one of our projects https://github.com/eea/eea.dexterity.indicators/blob/master/eea/dexterity/indicators/interfaces.py#L179-L662

The volto.blocks.editable.layout is there just as a marker interface in order that the transformers work properly. As you can see they are registered on IBlocks interface:

https://github.com/plone/plone.restapi/blob/9.7.1/src/plone/restapi/blocks_linkintegrity.py#L13-L29

plone / volto

Editable layout does not work well when portal_type has a Schema #6170