Implementing business logic in class-based views - best practices

Using Django Forms & APIs
1

Hi! I’m working on a Django project and aiming to adhere to best practices, such as avoiding the service layer and keeping views thin .

I’m using class-based views for creating and updating objects, specifically by overriding the form_valid method to add business logic. Here’s an example:

class PetCreateView(CreateView):
    model = Pet
    fields = ['name', 'age', 'animal_type']
    template_name = 'pets/pet_create.html'
    success_url = reverse_lazy('pets')

    def form_valid(self, form):
        self.object = form.save(commit=False)
        self.object.owner = self.request.user
        self.object.clean()  # Question 2
        self.object.save()
        return redirect(self.get_success_url())

Question 1:

Instead of saving with commit=False, I could directly assign the owner in form.instance like this:

def form_valid(self, form):
    form.instance.owner = self.request.user
    form.instance.clean()  # Question 2
    return super().form_valid(form)

The Pet instance is then saved by the parent form_valid method. Would this be a better approach?

Question 2:

If I have validation logic inside the Pet model related to the user, I could call clean() in the view, and check if the Pet instance has an owner before validating it, to avoid an AttributeError in the initial clean call:

class Pet(models.Model):
    def clean(self):
        super().clean()
        if self._state.adding:
            self._validate_owner_can_adopt_pet()

    def _validate_owner_can_adopt_pet(self):
        if hasattr(self, 'owner') and not self.owner.can_adopt():
            raise ValidationError(_('This owner cannot adopt more pets.'))

Alternatively, I could skip the clean() call in the view and add the owner before the initial clean like this:

class PetCreateView(CreateView):
    def get_form_kwargs(self):
        kwargs = super().get_form_kwargs()
        if kwargs['instance'] is None:
            kwargs['instance'] = self.model()
        kwargs['instance'].owner = self.request.user
        return kwargs

Which approach would you recommend?

Question 3:

If the logic becomes more complex, for example:

class TeamCreateView(CreateView):
    model = Team
    fields = ['name', 'description']
    template_name = 'team/team_create.html'
    success_url = reverse_lazy('team')

    def form_valid(self, form):
        user: User = self.request.user
        with transaction.atomic():
            self.object = form.save()
            member = self.object.add_member(user)
            self.object.set_admin(member)
            return redirect(self.get_success_url())

Would it be better to move this logic into a custom manager? For example:

class TeamManager(models.Manager['Team']):
    @transaction.atomic
    def create_and_set_admin_member(self, user: 'User', **kwargs) -> 'Team':
        team = self.create(**kwargs)
        member = team.add_member(user)
        team.set_admin(member)
        return team

Then, in the view:

def form_valid(self, form):
    user: User = self.request.user
    self.object = Employer.objects.create_and_set_admin_member(user=user, **form.cleaned_data)
    return redirect(self.get_success_url())

Although I like the idea of separating the logic from the view, I’m unsure whether this is the right approach. The ModelForm.save() method has additional logic for many-to-many fields and error handling, and I’m not sure if bypassing it by using a custom manager is the best practice. Additionally, this creates a new instance from scratch, which seems wasteful, though I could pass the instance to the manager method to avoid this.

Question 4:

Since FormView doesn’t call form.save() in form_valid(), how would you recommend handling business logic in this case, compared to using create/update views?

Thank you!

2

Welcome @kashyl !

Keep in mind that these are “opinions”, and not necessarily recognized universally as “best practices”. Many (myself included), do consider them to be optimal, but not to the point of saying other approaches don’t have merit under particular circumstances.

Decisions such as these must be made in the context of the larger scope of the project - what works best for one project may not for another.

You could, but you’re calling “clean()” twice - it’s unnecessary in the general case.

My suggestion here is remove the self.object.clean() from your original.

Quoting from the docs for ModelForm:

When you use a ModelForm, the call to is_valid() will perform these validation steps for all the fields that are included on the form. See the ModelForm documentation for more information. You should only need to call a model’s full_clean() method if you plan to handle validation errors yourself, or if you have excluded fields from the ModelForm that require validation.

(The “validation steps” referenced here includes a call to Model.clean)

Regarding question 3 - it’s neither better nor worse, it’s a choice.
Personally, I don’t consider what you have here to be of sufficient size or complexity to make it worth moving to a manager method. (This doesn’t rise to the level of what I call “business logic”.)

Regarding question 4 - In the general case, FormView is intended for use with non-Model forms, where form.save() doesn’t make sense.

One perspective on this is that you need to look at how many different places you’re going to need this code to be executed.

Some people believe strongly that this type of work doesn’t belong in the views. I’m not one of them. If you’ve got a code fragment that is only going to exist once in your project, in one location for one specific view, then we put the code in the view. We find that easier to manage than having to navigate to the view - only to find out that the code being looked for is in a different file.

Again - this is an architectural decision, not a technical one, which means that different projects can do this different ways, and that the decisions between them are made more due “management” considerations than “technical”.

2 Likes