# Is this a good class based view?

**URL:** <https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195>\
**Category:** Forms & APIs\
**Created:** [August 26, 2024, 2:54pm UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195 "2024-08-26T14:54:02Z")\
**Posts on this page:** 10\
**Page:** 1

<div class="post-metadata">

**Author:** ![Rachit74](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/rachit74/32/23323_2.png) [@Rachit74](https://forum.djangoproject.com/u/Rachit74)\
**Post date:** [August 26, 2024, 2:54pm UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/1 "2024-08-26T14:54:02Z")

</div>

Recently I found out about class based views and started implementing them in my social media Django app.  
I was wondering if this view that I have written is a good way to write views and if not what can i improve on this.

```auto
# Read post Class Based View
class ReadPostView(DetailView, FormMixin):
    model = Post
    template_name = 'posts/read_post.html'
    context_object_name = 'post'
    # the id that the class will look for in the url
    pk_url_kwarg = 'post_id'

    form_class = CommentForm
    success_url = 'home'

    def get_context_data(self, **kwargs):
        context = super().get_context_data(**kwargs)
        post = self.object
        comments = Comment.objects.filter(post=post).order_by('-comment_at')
        context['comments'] = comments
        # Because we need to pass the form in the get request too
        context['form'] = self.get_form()            
        return context
    
    @method_decorator(login_required)
    def post(self, request, *args, **kwargs):
        self.object = self.get_object()
        form = self.get_form()
        if form.is_valid():
            comment = form.save(commit=False)
            comment.post = self.object
            comment.comment_author = request.user
            comment.save()
            messages.success(request, "Comment Added!")
            return redirect(self.get_success_url())

    def get_success_url(self) -> str:
        return reverse('read-post', kwargs={'post_id': self.object.id})

```

Also I request you guys to keep the english simple as it is not my first language, Thankyou!

---

<div class="post-metadata">

**Author:** ![onyeibo](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/onyeibo/32/11494_2.png) [@onyeibo](https://forum.djangoproject.com/u/onyeibo)\
**Post date:** [August 26, 2024, 3:57pm UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/2 "2024-08-26T15:57:04Z")

</div>

Does it perform as intended? If so, the rest can improve over time

> [@Rachit74](#):
>
> `@method_decorator(login_required)`

Have you considered subclassing `LoginRequiredMixin` and providing a `login_url` rather than using a decorator?

Example:

```auto
from django.contrib.auth.mixins import LoginRequiredMixin

class ReadPostView(LoginRequiredMixin, FormMixin, DetailView):
    login_url = "/user/login/"

```

Why was it necessary to override `get_success_url()` completely? Alternatively, set `self.success_url` then call `super().get_success_url()`

---

<div class="post-metadata">

**Author:** ![KenWhitesell](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/kenwhitesell/32/280_2.png) [@KenWhitesell](https://forum.djangoproject.com/u/KenWhitesell)\
**Post date:** [August 26, 2024, 3:57pm UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/3 "2024-08-26T15:57:57Z")

</div>

Welcome @Rachit74 !

In addition to the comments above, any mixin classes should be listed before the base class in the class definition.

Second, for any class showing additional detail with a form, I always recommend that you use the `FormView` as the base class. The `DetailView` doesn’t really add any functionality in this situation.  
You can then add any additional detail to be displayed with that form as part of your `get_context_data` method. That way, you don’t need to do anything special with your form handling unless it’s required.

---

<div class="post-metadata">

**Author:** ![onyeibo](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/onyeibo/32/11494_2.png) [@onyeibo](https://forum.djangoproject.com/u/onyeibo)\
**Post date:** [August 26, 2024, 4:03pm UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/4 "2024-08-26T16:03:11Z")

</div>

In short,

`FormView` can effectively replace `FormMixin` and `DetailView` in @Rachit74’s case.  
Very cool answer.

---

<div class="post-metadata">

**Author:** ![white-seolpyo](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/white-seolpyo/32/22386_2.png) [@white-seolpyo](https://forum.djangoproject.com/u/white-seolpyo)\
**Post date:** [August 27, 2024, 12:51am UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/5 "2024-08-27T00:51:05Z")

</div>

you can use prefetch queryset instead override get\_context\_data method.

and you are not need success\_url, because you override get\_success\_url method.

```auto
form django.db.models import Prefetch
class ReadPostView(DetailView, FormMixin):
    # model = Post
    queryset = Post.objects.all().prefetch_related(
        Prefetch('{comment reverse related name}', Comment.objects.filter(post=post).order_by('-comment_at'))
    )

```

---

<div class="post-metadata">

**Author:** ![Rachit74](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/rachit74/32/23323_2.png) [@Rachit74](https://forum.djangoproject.com/u/Rachit74)\
**Post date:** [August 27, 2024, 11:03am UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/6 "2024-08-27T11:03:26Z")

</div>

Hey @onyeibo thanks for the `LoginRequiredMixin` tip didn’t knew about that, but in my case I want the user to be logged in if they are going to comment, so I think that `@method_decorator(login_required)` makes more sense, because applying the mixin makes the complete read post function view to require a login.

---

<div class="post-metadata">

**Author:** ![KenWhitesell](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/kenwhitesell/32/280_2.png) [@KenWhitesell](https://forum.djangoproject.com/u/KenWhitesell)\
**Post date:** [August 27, 2024, 11:22am UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/7 "2024-08-27T11:22:42Z")

</div>

> [@Rachit74](#):
>
> but in my case I want the user to be logged in if they are going to comment,

I’m not sure that really make sense from a UI/UX perspective, because someone who isn’t logged in will still see the form. So they might get the impression that they _can_ comment - but only get an error or redirected after typing in their comment and submitting.

---

<div class="post-metadata">

**Author:** ![Rachit74](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/rachit74/32/23323_2.png) [@Rachit74](https://forum.djangoproject.com/u/Rachit74)\
**Post date:** [August 27, 2024, 11:33am UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/8 "2024-08-27T11:33:42Z")

</div>

Hey @KenWhitesell I was just experimenting with your recommendation of `FormView` and using it does make more sense here is an snippet

```python
class ReadPostView(FormView):
    template_name = 'posts/read_post.html'
    form_class = CommentForm

    def get_context_data(self, **kwargs: Any) -> dict[str, Any]:
        context = super().get_context_data(**kwargs)
        post_id = self.kwargs['post_id']
        context['post'] = get_object_or_404(Post, id=post_id)
        return context
    
    def form_valid(self, form: Any) -> HttpResponse:
        post = self.get_context_data()['post']
        comment = form.save(commit=False)
        comment.comment_author = self.request.user
        comment.post = post
        comment.save()

```

Also regarding about the form UI/UX I actually use a logic in my template that for a user that is not logged in the template will show `login to comment`

```html
{% if user.is_authenticated %}
<a href="#" class="show-link" onclick="toggleBox(event, 'comment-box')">Add a Comment</a>
{% else %}
<a href="{% url 'login' %}" class="hi">Login</a> to comment
{% endif %}

```

Hopefully this makes sense of why i can’t use LoginMixin on the class 🙂

---

<div class="post-metadata">

**Author:** ![KenWhitesell](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/kenwhitesell/32/280_2.png) [@KenWhitesell](https://forum.djangoproject.com/u/KenWhitesell)\
**Post date:** [August 27, 2024, 12:24pm UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/9 "2024-08-27T12:24:07Z")

</div>

> [@Rachit74](#):
>
> ```auto
> def form_valid(self, form: Any) -> HttpResponse:
> post = self.get_context_data()['post']
> comment = form.save(commit=False)
> comment.comment_author = self.request.user
> comment.post = post
> comment.save()
> 
> ```

Did you truncate your post before the end of the `form_valid` function? If not, your `form_valid` method here is not returning an `HttpResponse`. You _typically_ will return an `HttpResponseRedirect` after a successful submission.

> [@Rachit74](#):
>
> Hopefully this makes sense of why i can’t use LoginMixin on the class

Yes, this makes sense.

---

<div class="post-metadata">

**Author:** ![Rachit74](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.djangoproject.com/rachit74/32/23323_2.png) [@Rachit74](https://forum.djangoproject.com/u/Rachit74)\
**Post date:** [August 27, 2024, 12:27pm UTC](https://forum.djangoproject.com/t/is-this-a-good-class-based-view/34195/10 "2024-08-27T12:27:06Z")

</div>

Yes I am redirecting after the form has been submitted.

also i am doing this to prevent any unauthenticated user form commenting

```python
 def form_valid(self, form: Any):
        if not self.request.user.is_authenticated:
            return redirect('login')

```