Raise error when slug already exists

I looked into possible ways to raise an error to generate an error when a slug already exist.

This is the model:

# models.py
from django.db import models
class Question(models.Model):
    title = models.CharField(max_length=100)
    slug = models.SlugField(max_length=100, db_index=True, unique=True)

for which I use a generic view:

# views.py
from .models import Question
class QuestionCreate(generic.CreateView):
    model = Question
    fields = ['title', 'text']
    def form_valid(self, form):
        instance = form.save(commit=False)
        slug = slugify(form.cleaned_data['title'])
        if Question.objects.filter(slug=slug).exists():
             form.add_error("title","This Question already exist")
             return self.form_invalid(form)

The slug is generated from the question title.

Then it was brought to my attention that I don't need to save the form first:

class QuestionCreate(generic.CreateView):
    model = Question
    fields = ['title', 'text']
    def form_valid(self, form):
        slug = slugify(form.cleaned_data['title'])
        if Question.objects.filter(slug=slug).exists():
             form.add_error("title","This Question already exist")
             return self.form_invalid(form)

Is this the best practice?

Other possibilities I considered:

# models.py
from django.db import models, IntegrityError
from django.forms import ValidationError
class Question(models.Model):
    title = models.CharField(max_length=100)
    slug = models.SlugField(max_length=100, db_index=True, unique=True)
    def save(self):
        slug = slugify(self.title)
        try:
            self.slug = slug
            super().save()    
        except IntegrityError:
            raise ValidationError("Question already exist")

but it seems not possible to display this error message to the user.

The final option I considered:

```
# models.py
from .forms import QuestionForm
class QuestionCreate(generic.CreateView):
model_f

The good

So, you're trying to allow users to create new questions, but those questions must have a slug which is unique. You're enforcing this on the database level with unique=True, which is a good start.

You are using a form to handle the creation of new questions, which is definitely the right way to do this. It'll also be really useful when you need to build in the ability to edit a question, since you can just reuse the same form.

The bad

But you're enforcing the uniqueness of a question outside of the form. The form ultimately has the business logic that determines whether or not the input is valid, which is why Django has built-in support for validating forms. If you find issue with isolating your form validation logic inside of a form, I would recommend breaking it out as general utilities which can be easily tested.

Right now you're enforcing these constraints either on the model level (through save, which is definitely not the right place) or on the view level (through form_valid, which is not right but also not horrible).

Django designed the generic views to be easy to implement, assuming you follow the standard Django conventions. If you don't follow the conventions, you end up with cases like your form_valid where you have to do additional validation and then decide that it's actually invalid, forcing them to form_invalid. You shouldn't have made it to form_valid, since the state is not actually valid.

The fix

You should move your uniqueness logic into the form. Since you are only validating a single field, the title field, you only need to handle the logic for that field. This means that you can isolate your logic inside a custom cleaning method.

class QuestionForm(ModelForm):

    class Meta:
         model = Question
         fields = ['title', 'text']

    def clean_title(self, title):
        from django.utils.text import slugify
        from django.core.exceptions import ValidationError

        slug = slugify(title)

        if Question.objects.filter(slug=slug).exists():
            raise ValidationError('A question with this title already exists.')

        return title

class QuestionCreate(generic.CreateView):
    form_class = QuestionForm

This means that you no longer need the form_valid override and you don't need to worry about having to replicate that validation logic in other places which use this form.

StackExchange Code Review Q#154820, answer score: 9