CreateView: Am I doing this right?

Using Django Forms & APIs
1

Let’s say I have a song model and I want to allow users to post comments on a song. This is what the comment model looks like (I omitted the choices for rating for the sake of brevity):

class Comment(models.Model):
    profile = models.ForeignKey(Profile, on_delete=models.CASCADE)
    song = models.ForeignKey(Song, on_delete=models.CASCADE)
    text = models.TextField(max_length=5000)
    rating = models.PositiveSmallIntegerField(choices=Ratings.choices)
    create_date=models.DateTimeField(auto_now_add=True)

And the form:

class AddCommentForm(ModelForm):
    class Meta:
        model = Comment
        fields = ("rating", "text")

It has two foreign keys, profile (which is connected to the authenticated user) and song. The only fields the user should see in the form are text and rating (which is a score from 1-10). These are both required to save a comment since they’re non-nullable, but they’re not in the form - which means I will need to add them in the view.

There are also two constraints: a user cannot comment on their own song, and they can’t comment on a song they’ve already commented on.

Now, here is my implementation using CreateView. Is this the best way for me to use it?

To verify my constraints: I override dispatch to check if the user has commented on the song already or if it’s the user’s own song. While I’m at it, I store the song into extra_context so I don’t have to do another database query for it later.

To add the foreign keys before saving: I override form_valid to add the profile and song IDs to the comment that is about to be saved.

To provide context data: I override get_context_data to add the song to the context data. If the song was loaded in dispatch, it will already be in extra_context, so I can just pull it from there instead of querying the database again. The context data is only used in the template, so this is only relevant to the GET request.

My questions:

Is dispatch the best place for me to check my constraints? Or should I do that in form_valid, or somewhere else?

Is my override of form_valid the right way of adding the foreign key values to the comment? I know I could put both the user id and song id in the form itself as hidden fields, but I don’t want to allow for the possibility of the user changing those by modifying the HTML locally.

class AddCommentView(LoginRequiredMixin, CreateView):
    form_class = AddCommentForm
    template_name = "add_comment.html"

    def dispatch(self, request, *args, **kwargs):
        if not request.user.is_authenticated:
            return super().dispatch(request, *args, **kwargs)

        song_id = kwargs['pk']
        song = Song.objects.get(id=song_id)
        profile_id = request.user.profile.id
        is_own_song = song.artist_set.all().filter(profile_id=profile_id).exists()
        has_commented = song.comment_set.all().filter(profile_id=profile_id).exists()

        # Storing the song into extra_context to save a database request
        self.extra_context={'song': song}

        if (is_own_song or has_commented):
            return redirect('comment_rejected')

        return super().dispatch(request, *args, **kwargs)

    # Add profile and song ID to the comment in order to save it
    def form_valid(self, form):
        comment_instance = form.save(commit=False)
        comment_instance.profile = self.request.user.profile
        comment_instance.song_id = self.kwargs['pk']
        comment_instance.save()

        return redirect('view_song', self.kwargs['pk'])

    def get_context_data(self, **kwargs):
        context = super().get_context_data(**kwargs)
        context['song'] = self.extra_context['song'] if self.extra_context['song'] else Song.objects.get(self.kwargs['pk'])
        return context
2

In this case I think you should not make validations in the View, you should do it in the ModelForm’s clean() method as you can do it in any Form.

If you plan to use this validation in every case (not just for this view but in all your project) I think is better encapsulate in the model´s clean() method. Just remember that a ModelForm calls the model´s clean() method, but if you create a Form you should call it in the code.

In any case you should pass in your view the Profile form the request.user:

class AddCommentoView(LoginRequiredMixin, CreateView):
    ...
     def form_valid(self, form):
         profile = self.request.user.profile
         return super(AddCommentoView, self).form_valid(form)

The the first option (ModelFormValidation) should look like this:


class AddCommentForm(ModelForm):
    ...
    def clean(self):
        # Validation here, I am not sure of the relations between classes

In you will use model validation:

class Comment(models.Model):
    ...
    def clean(self):
        if self.profile in song.artist_set.all(): # not sure about these 
            raise ValidationError("Comment your own songs is not allowed.")

I have no tested the code. But I hope it helps.