django-cms / djangocms-frontend

django CMS frontend is a plugin bundle for django CMS providing several components for the frontend, currently implemented with the popular Bootstrap 5 framework.
Other
55 stars 22 forks source link

[BUG] Broken copy/pasting for custom plugins with untangled fields extending CMSUIPlugin, FrontendUIItem and EntangledModelForm #194

Closed adlh closed 8 months ago

adlh commented 8 months ago

Description

A custom plugin (built as described in the djangocms-frontend docs, with new fields in the model), breaks when copy-pasting it on a page. After pasting it, no new item appears, as expected, and instead of it, the child plugins inside it are pasted into the original plugin (twice).

Steps to reproduce

Steps to reproduce the behavior:

  1. On a django-cms project (4.x) add a new app test_plugins.
  2. Create a new custom plugin, like the one described in the docs linked above, with own field(s) in the model:

    
    # test_plugins/models.py
    class CustomFrontendUIItem(FrontendUIItem):
    bla1 = models.CharField(blank=True, max_length=100)
    bla2 = models.CharField(blank=True, max_length=100)
    
    class Meta:
        verbose_name = _('A simple frontend plugin')
    
    def short_description(self):
        return f"'{self.bla1} | {self.bla2}"

test_plugins/forms.py

class CustomFrontendUIForm(EntangledModelForm): class Meta: model = CustomFrontendUIItem entangled_fields = { "config": [ "json_field", # This field will be stored in the config JSON ] } untangled_fields = ['bla1', 'bla2'] json_field = forms.CharField(max_length=100)

test_plugins/cms_plugins.py

@plugin_pool.registerplugin class CustomFrontendUIPlugin(CMSUIPlugin): model = CustomFrontendUIItem form = CustomFrontendUIForm module = ('Tests') name = _('FrontendUI Test') render_template = "test_plugins/custom_frontend_plugin.html" allow_children = True

fieldsets = [
    (None, {
        'fields': ['json_field', 'bla1', 'bla2']
    }),
]

def render(self, context, instance, placeholder):
    context.update({'instance': instance})
    return context
3. Then also add a template for it under `test_plugins/templates/test_plugins/custom_frontend_plugin.html`:
```html
{% load cms_tags easy_thumbnails_tags %}

<div class="card mx-3 my-2">
    <h2 class="card-header">Simple FrontendUI Test</h2>
    <div class="card-body">
        <p>{{ instance.bla1 }}, {{ instance.bla2 }}</p>
        <p>JSON: {{ instance.json_field }}</p>
        <div class="children">
            {% for plugin in instance.child_plugin_instances %}
                {% render_plugin plugin %}
            {% endfor %}
        </div>
    </div>
</div>
  1. Run makemigrations and then migrate.
  2. Start the server.
  3. Start editing a page (if necessary create it first) and then add an item of the custom plugin we just created, fill the 3 fields with some text and save it.
  4. The plugin is created and visible on the page.
  5. Inside the plugin add a child element, e.g. a Text plugin with some text.
  6. Now, on the Page Content sidebar (on the right) open the options of the custom plugin (not the child Text) and select 'Copy'.
  7. On the Page Content root options (on the top of the sidebar) select 'Paste'.
  8. Instead of now having a copy of the original plugin, the Text content was duplicated (twice) on the original plugin, so we now see 3 text items inside it. (On the sidebar we do see the original one at first and the 'new' one with 4 copies of the child, but after reloading the page, this updates into only one item with 3 children)

Expected behaviour

A new instance of the custom plugin should appear with a copy of the child Text plugin inside it.

Actual behaviour

No new plugin is created and the original instance gets duplicates of the child plugins instead.

Screenshots

Screenshot from 2024-03-06 20-42-58 Screenshot from 2024-03-06 20-44-05 Screenshot from 2024-03-06 20-47-08 Screenshot from 2024-03-06 20-48-03 Screenshot from 2024-03-06 20-48-40 Screenshot from 2024-03-06 20-49-00 Screenshot from 2024-03-06 20-49-23 Screenshot from 2024-03-06 20-54-46 Screenshot from 2024-03-06 20-55-04

Additional information (CMS/Python/Django versions)

Using virtualenv with python 3.10 and the following output of pip freeze:

asgiref==3.7.2
chardet==5.2.0
cssselect2==0.7.0
Django==5.0.3
django-appconf==1.0.6
django-classy-tags==4.1.0
django-cms==4.1.0
django-entangled==0.5.4
django-filer==3.1.1
django-formtools==2.5.1
django-fsm==2.8.1
django-parler==2.3
django-polymorphic==3.1.0
django-sekizai==4.1.0
django-select2==8.1.2
django-treebeard==4.7.1
djangocms-admin-style==3.3.1
djangocms-alias==2.0.0
djangocms-attributes-field==3.0.0
djangocms-frontend==1.2.2
djangocms-text-ckeditor==5.1.5
djangocms-versioning==2.0.0
easy-thumbnails==2.8.5
html5lib==1.1
lxml==5.1.0
packaging==23.2
pillow==10.2.0
reportlab==4.1.0
six==1.16.0
sqlparse==0.4.4
svglib==1.5.1
tinycss2==1.2.1
typing_extensions==4.10.0
webencodings==0.5.1

Do you want to help fix this issue?

adlh commented 8 months ago

An interesting fact here, is that almost the same plugin, but as a proxy model (without the added fields bla1 and bla2, works perfectly well, so maybe this has something to do with the 'untangled' fields.

I'm including my test project with 3 plugins, one non-frontendUI plugin, one using a proxy model and the one described in the bug. bug_report_example_project.zip

fsbraun commented 8 months ago

@adlh Thank you so much for the detailed bug report. This was extremely helpful! I looked into this.

Let me start with a few comments:

1) Currently, as described in the docs, only additional frontend items without any additional fields are possible (they need to be proxy models). 2) The reason for this is the way django CMS plugins are stored in the database in an abstract way, and how they are "downcasted" to concrete plugins. This only works over one level. Your implementation would require two downcast steps: a) from generic CMSPlugin to FrontendUIItem b) from FrontendUIItem to CustomFrontendUIItem 3) The solution for most cases is to move the additional database fields to the entangled form, effectively moving the fields from the database to the config JSON field.

However, I recognize that there is a legitimate use case for your situation, e.g., if you need a reference with guaranteed referential integrity.

There's a shortcoming in djangocms-frontend's set up which I would like to update. Besides FrontendUIItem which can be used as a base class for proxy models, I propose to introduce AbstractFrontendUIItem as a base which allows adding model fields.

Then you'd have two options to extend djangocms-frontend plugins:

  1. Using a proxy model (base class FrontendUIItem): The data is stored in the djangocms_frontend_frontenduiitem table and only form fields can be added to the JSON storage. No model fields to be added. (Probably the vast majority of use cases.)
  2. Subclassing AbstractFrontendUIItem: The data is stored in a separate table of your app. JSON storage can be used as with djangocms-frontend. You can add model fields.

@marksweb Do you have any thoughts on this?

fsbraun commented 8 months ago

@adlh And yes: The documentation is misleading, since it suggests you can just remove the proxy status of the model. 🤦‍♂️

adlh commented 8 months ago

Thanks @fsbraun for analyzing the problem. I think I can work with proxy models for the time being. Most of the plugins I'm migrating need only some options or text-fields, so that shouldn't be a problem.

And yes, the docs were really misleading on that part :-D

fsbraun commented 8 months ago

Fixed with #195