feincms/feincms3

ContentEditor admin requires TemplateMixin

Closed this issue · 5 comments

It appears from the documentation that all the plugins(mixing) are optional including TemplateMixin which appears to be required only for multiple template formats. However the ContentEditor admin requires instance.regions which are only available from TemplateMixin

Have I missed something?

EDIT: Although the documentation does state that the ContentEditorAdmin uses the template regions but it should either make the TemplateMixin a dependency or we should move the region related functions from the TemplateMixin to the AbstractPage model or another ContentEditor related Mixin since forcing a template_key in all models just to get ContentEditorAdmin seems to be superflous

The ContentEditor requires a regions attribute on the model, and the TemplateMixin is one way to provide this attribute. The django-content-editor documentation contains a static regions definition which works as well. Copy-pasted from http://django-content-editor.readthedocs.io/en/latest/#example-articles-with-rich-text-plugins:

class Article(models.Model):
    title = models.CharField(max_length=200)
    pub_date = models.DateField(blank=True, null=True)

    regions = [
        Region(key='main', title='main region'),
        Region(key='sidebar', title='sidebar region',
               inherited=False),
    ]

    def __str__(self):
        return self.title

Maybe you don't even need two regions, then regions = [Region(key='main', title='Main')] would already be sufficient.

I agree that forcing a template_key field would be superfluous.

Agreed that TemplateMixin has to provide that as an override but my two cents say that the documentation for ContentEditorAdmin should mention the regions setting as a compulsory requirement (esp since the documentation is quite good and thorough otherwise)
With that done, we can close this query. Thanks

Thank you! I added a note to the django-content-editor docs. Feedback is very welcome!

@matthiask, adding this note here because the more I use this, it appears that an Admin requiring a field on a model is backwards. If we think about it, the ContentEditor admin should use a default regions setting of [] if none are provided and only use regions if the model has the same. An admin throwing an error due to a missing field on the model isn't intuitive. If something is added into the admin which is not valid for the model then an error makes sense. What say?

The list of regions is both used in the ContentEditor and also in content_editor.contents.Contents. The region CharFields exist on all plugins.

The model is available everywhere, so I didn't give this too much thought. Do you have a proposal how the regions list should be made available to all the places it's being used instead? Or maybe we should just introduce a system check here: https://github.com/matthiask/django-content-editor/blob/master/content_editor/admin.py#L19 ?