Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Refactoring Django class-based views, clean up 18 repetitive classes.

https://github.com/AnthonyBRoberts/fcclincoln/blob/master/apps/story/views.py

I'm a little embarrassed to admit that this is mine. But it is.

class FrontpageView(DetailView):
    template_name = "welcome_content.html"
    def get_object(self):
        return get_object_or_404(Article, slug="front-page")
    def get_context_data(self, **kwargs):
        context = super(FrontpageView, self).get_context_data(**kwargs)
        context['slug'] = "front-page"
        events = Article.objects.filter(slug="events")
        context['events'] = events
        return context

So this is a pretty normal class-based detail view in Django.

It's assigning a template, getting an Article object, and adding some things to the context_data.

Then I copied this class 17 times. Each time, there's a different template, and a different slug, and different stuff added to the context_data.

The idea is that there's a WYSIWYG editor for administrators to change the web content, and a user authentication system, to allow multiple people access to the site content. Basically, a super-simple CMS, so no one has to edit html to update the site.

But I really wish I could refactor this so I don't have these nearly identical 18 classes. Any suggestions on where I should start on this would be most welcome.

like image 574
Anthony Roberts Avatar asked Mar 20 '23 07:03

Anthony Roberts


2 Answers

Squash all of your classes down to a single class that inherits from TemplateResponseMixin, as DetailView does, (also check out the SingleObjectTemplateResponseMixin) and override its get_template_names() method to return the template appropriate for the current situation.

A beautiful example of this being used is in the django-blog-zinnia project

def get_template_names(self):
    """
    Return a list of template names to be used for the view.
    """
    model_type = self.get_model_type()
    model_name = self.get_model_name()

    templates = [
        'zinnia/%s/%s/entry_list.html' % (model_type, model_name),
        'zinnia/%s/%s_entry_list.html' % (model_type, model_name),
        'zinnia/%s/entry_list.html' % model_type,
        'zinnia/entry_list.html']

    if self.template_name is not None:
        templates.insert(0, self.template_name)

    return templates

Django will take that list of names and try each item to see if it exists in the templates folder. If it does, that template is used.

Update

After looking at your code a little more closely, perhaps something like this:

In your main urls.py

# convert each url
url(r'^$', FrontpageView.as_view()),
url(r'^history/$', HistoryView.as_view()),
url(r'^calendar/$', CalendarView.as_view()),
url(r'^news/$', NewsView.as_view()),
url(r'^visitors/$', VisitorsView.as_view()),
...
# to just
url(r'^(?P<slug>[\w\d/-]+)/$', SuperSpecialAwesomeView.as_view()),
# but, put this at the end of urls list after any routes that don't use this view

DetailView, after setting the class attribute model, will check to see if slug is in the url's kwargs and if it is, it will use the slug to do a model lookup just like what you are already doing: Article.ojects.get(slug=self.kwargs['slug'])

models.py

You could add a type field to your Article model. The type will specify what type of article it is. For example, your ChildrenView, YouthView, and AdultView could all have a type of music (since the templates are all music, I'm assuming that's how they are related).

ARTICLE_TYPE_CHOICES = (
    (0, 'music'),
    (1, 'weddings'),
    (2, 'outreach'),
    ...
)

class Article(models.Model):
     ...
     type = models.IntegerField(choices=ARTICLE_TYPE_CHOICES)
     ...

Then, in your views.py

class SuperSpecialAwesomeView(DetailView):
    template_name = None
    model = Article
    def get_template_names(self):
        slug = self.kwargs.get('slug', '')
        templates = [
            # create a template based on just the slug
            '{0}.html'.format(slug),
            # create a template based on the model's type
            '{0}.html'.format(self.object.get_type_display()),
        ]
        # Allow for template_name overrides in subclasses
        if self.template_name is not None:
            templates.insert(0, self.template_name)

        return templates

Given an article instance with a type of music and a slug of ministry/children, Django will look for a template named ministry/children.html and a template named music.html.

And if you need to do some special stuff for other views (like you will probably need to for SermonsView), then subclass SuperSpecialAwesomeView

class SermonsView(SuperSpecialAwesomeView):
    paginate_by = 2
    queryset = Article.objects.order_by('-publish_date')
like image 131
OozeMeister Avatar answered Apr 06 '23 22:04

OozeMeister


A quick approach I would think: Add a template field in the model with a list of predefined template choices (those can be created dynamically). Override the default DetailView methods, override the get_template_names method to assign the proper template to the view (if not available fallback, that can be done through a try: except:). Apart from that you can alter the View behaviour with any kind of model flags. This way you can have a single entry point for a model, rather than defining repeatable views all over the place. I tend to keep a FrontPageView independent from other views though, for easiness and because it serves a different purpose. If you need repeatable context entries, consider a context processor, if you need repeatable context entries for specific views consider Mixins.

like image 35
petkostas Avatar answered Apr 06 '23 23:04

petkostas