Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 of 3 tasks
adlh opened this issue Mar 6, 2024 · 5 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request

Comments

@adlh
Copy link

adlh commented Mar 6, 2024

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.register_plugin
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
  1. Then also add a template for it under test_plugins/templates/test_plugins/custom_frontend_plugin.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?

  • Yes, I want to help fix this issue and I will join #workgroup-pr-review on Slack to confirm with the community that a PR is welcome.
  • No, I only want to report the issue.
  • I'm not sure if I can contribute with a PR, but I can try to help in what I can. I already opened a thread for this on the #support channel.
@adlh
Copy link
Author

adlh commented Mar 6, 2024

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 fsbraun transferred this issue from django-cms/django-cms Mar 7, 2024
@fsbraun fsbraun added the bug Something isn't working label Mar 7, 2024
@fsbraun
Copy link
Member

fsbraun commented Mar 7, 2024

@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
Copy link
Member

fsbraun commented Mar 7, 2024

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

@fsbraun fsbraun added documentation Improvements or additions to documentation enhancement New feature or request labels Mar 7, 2024
@adlh
Copy link
Author

adlh commented Mar 7, 2024

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
Copy link
Member

fsbraun commented Mar 25, 2024

Fixed with #195

@fsbraun fsbraun closed this as completed Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants