jrief / djangocms-cascade

Build Single Page Applications using the Django-CMS plugin system
MIT License
165 stars 85 forks source link

Creating a BootstrapRowPlugin fails because of incorrect `empty_form` value #390

Open Xiphoseer opened 3 years ago

Xiphoseer commented 3 years ago

When trying to create Bootstrap 4 Row plugins, we ran into an issue where no settings were displayed in the creation form, but the creation failed with an error that told us to fix one of the settings.

After some investigation, I found that the num_columns field from the form was missing from the request and that there's a weird form_empty context value in templates/cascade/admin/change_form.html.

It turns out that for a form that doesn't use any django-entangled fields, this line in plugin_base.py sets that value but ignores any untangled_fields that may be present. Thus, no form fields appear in the admin view and consequently aren't included in the request to the server.

I would personally consider values like empty_form to be an anti-pattern, because they produce hard-to-debug cases like this one, but I appreciate that the intent is to inform the user that this form is intentionally blank if it were to work correctly.

jrief commented 3 years ago

OK. I will have a look at it.

Xiphoseer commented 3 years ago

Is there any update to this? I can try to create a PR, but I'm not sure I understand the details of django-entangled enough for that.

svandeneertwegh commented 3 years ago

Same problem here. Any updates?

jrief commented 3 years ago

@Xiphoseer Just one additional question: Did you create your own plugin with its own form, or did you use the BootstrapRowPlugin as provided by djangocms-cascade?

Xiphoseer commented 3 years ago

@Xiphoseer Just one additional question: Did you create your own plugin with its own form, or did you use the BootstrapRowPlugin as provided by djangocms-cascade?

We did create our own djangocms-cascade plugins (here) but none of them change, override or derive from BootstrapRowPlugin. So as far as I can tell, we used the BootstrapRowPlugin as provided.

I made sure to verify this behavior on a page without any of our custom plugins before filing this bug.

jrief commented 3 years ago

But I absolutely agree, that I should not render an "Empty form message", if there are any untangled_fields in the form's Meta class. This should be fixed.

jrief commented 3 years ago

Could you please retry with the latest version from this repository.