Class-based views #22

Miseracle · 2018-01-28T16:13:56Z

According to #20 , changed functional-based views to cb approach.

webknjaz · 2018-01-28T17:05:58Z

blog/views.py

@@ -3,115 +3,136 @@
 - post_list - returns post list;
 """

-from django.shortcuts import render, get_object_or_404, redirect
+from django.shortcuts import get_object_or_404, redirect


You shouldn't need these shortcuts as well.

webknjaz · 2018-01-28T17:10:30Z

blog/views.py

+    template_name = 'blog/post_edit.html'
+
+    def post(self, request, *args, **kwargs):
+        """Get author-field from request and save post to DB."""
        form = PostForm(request.POST)


I believe, this check is already encapsulated into CreateView.

You don't need this

webknjaz · 2018-01-28T17:10:50Z

blog/views.py

+    template_name = 'blog/post_edit.html'
+
+    def post(self, request, *args, **kwargs):
+        """Get author-field from request and save post to DB."""
        form = PostForm(request.POST)
        if form.is_valid():


I believe, this check is already encapsulated into CreateView.

webknjaz · 2018-01-28T17:11:52Z

blog/views.py

+    template_name = 'blog/post_edit.html'
+
+    def post(self, request, *args, **kwargs):
+        """Get author-field from request and save post to DB."""
        form = PostForm(request.POST)
        if form.is_valid():
            post = form.save(commit=False)
            post.author = request.user


You can inner this via form_valid method or so

webknjaz · 2018-01-28T17:12:45Z

blog/views.py

+    template_name = 'blog/post_edit.html'
+
+    def post(self, request, *args, **kwargs):
+        """Get author-field from request and save post to DB."""
        form = PostForm(request.POST)
        if form.is_valid():
            post = form.save(commit=False)
            post.author = request.user
            post.save()


It's done automatically, you don't have to implement post method at all

I quess, I have to use construction on 55-57 anyway - not in post, but in form_valid then.

No, there should probably be a way to even encapsulate that into save method of form

webknjaz · 2018-01-28T17:14:47Z

blog/views.py

+    fields = ['title', 'text']
+    template_name = 'blog/post_edit.html'
+
+    def get_success_url(self):


Better move this to model

webknjaz · 2018-01-28T17:15:42Z

blog/views.py

+class PublishPost(TemplateView, Protected):
+    """View for publishing post."""
+
+    def get(self, request, *args, **kwargs):


It should be post

webknjaz · 2018-01-28T17:16:12Z

blog/views.py

+    queryset = Post.objects.filter(published_date__isnull=True).order_by('created_date')
+
+
+class PublishPost(TemplateView, Protected):


Use updateview

webknjaz · 2018-01-28T17:18:20Z

blog/views.py

+    fields = ['author', 'text']
+    template_name = 'blog/add_comment.html'
+
+    def post(self, request, *args, **kwargs):


Same as above: read the docs of what each of base classes implement/encapsulate

webknjaz · 2018-01-28T17:20:34Z

blog/views.py

+        comment = get_object_or_404(Comment, pk=kwargs['pk'])
+        post_pk = comment.post.pk
+        comment.delete()
+        return redirect('post_detail', pk=post_pk)


How about get_success_url?

Tried, but didn't figure out, how to get post.pk.
get_success_url evaluates after the action is done, so I can't get it from self.object.

I'd save it in form_valid and then reused that to create success url

Actually, I was wrong - we can get self.object.
https://code.djangoproject.com/ticket/19044

But right now it fails on template rendering (cuz post object is missing). I can write separate template for deleting or redefine get method (weird solution, I guess)...

Alright. Let's leave it as it. It would be cleaner in case of doing normal CRUD for some JS client.

webknjaz · 2018-02-03T16:55:47Z

blog/models.py

+
+    def get_absolute_url(self):
+        """Redirect to view with detail info about parent post."""
+        return reverse('post_detail', args=[str(self.post.pk)])


I think, pk=self.post.pk should work just fine

It doesn't, reverse() accepts only args / kwargs for additional parameters.

oh then you should pass kwargs, singe you use named group pk in that regexp

webknjaz · 2018-02-03T16:56:12Z

blog/models.py

@@ -59,3 +59,7 @@ def approve(self):
    def __str__(self):
        """Represent a comment as a string by its text."""
        return self.text
+
+    def get_absolute_url(self):
+        """Redirect to view with detail info about parent post."""


It's a like. No redirect here at all.

it's rendering/returning url, nothing more

webknjaz · 2018-02-03T16:59:04Z

blog/views.py

+    def form_valid(self, form):
+        """Pass parent post pk to model."""
+        post = self.get_object(Post.objects.filter(pk=self.kwargs['pk']))
+        form.save(post)


This is pointless: you get object from the db and save it back? That's weird

I get Post object and save it as a field of Comment.
Though, working with keys instead of objects should be simplier.

webknjaz · 2018-02-03T16:59:18Z

blog/views.py

+        """Pass parent post pk to model."""
+        post = self.get_object(Post.objects.filter(pk=self.kwargs['pk']))
+        form.save(post)
+        return super(AddComment, self).form_valid(form)


In Python 3, super is simpler

webknjaz · 2018-02-03T17:00:05Z

.gitignore

@@ -1,8 +1,10 @@
 *.pyc
 __pycache__
-tvenv
+djenv


why not just use pyenv's virtualenv plugin and forget about in-project env dir?

webknjaz · 2018-02-03T17:01:13Z

blog/forms.py

@@ -13,6 +13,14 @@ class Meta:
        model = Post
        fields = ('title', 'text',)

+    def save(self, user=None):
+        """Save post to DB, assigning author field."""
+        post = super(PostForm, self).save(commit=False)


it's not py2

webknjaz · 2018-02-03T17:02:47Z

blog/urls.py

+    url(r'^post/(?P<pk>\d+)/comment/$', views.AddComment.as_view(), name='add_comment'),
+    url(r'^comment/(?P<pk>\d+)/approve/$', views.ApproveComment.as_view(), name='comment_approve'),
+    url(r'^comment/(?P<pk>\d+)/remove/$', views.RemoveComment.as_view(), name='comment_remove'),
+    url(r'^admin/', admin.site.urls),


isn't it already added in mysite?

webknjaz · 2018-02-03T17:06:15Z

blog/views.py

+        """Publish post."""
+        post = self.get_object(Post.objects.filter(pk=kwargs['pk']))
+        post.publish()
+        return HttpResponseRedirect(reverse('post_detail', kwargs={'pk': post.pk}))


If you use UpdateView, there's a plenty of logic encapsulated there. You should just inject a small change into some of already existing methods. It will get redirected automatically. BTW, there should be self.object available, which is handled so you don't have to do manual db lookup.

self.object assigns in BaseUpdateView.get(), so I should write smthg like:

super.get(...) DoSmthg (self.object.publish etc.) return HttpResponseRedirect(...)

I think you're missing the idea of pre-existing base classes. post/get/put etc. methods are low-level here. But those classes like UpdateView and similar encapsulate lots of logic and provide you with custom hook methods, which you should modify to inject some changes into existing flow.

I think, I miss the idea, where should I implement logic in general - or just couldn't find an appropriate method to make an injection.

self.object instead of get_object.

webknjaz · 2018-02-07T08:24:54Z

blog/forms.py

+    def save(self, user=None):
+        """Save post to DB, assigning author field."""
+        post = super().save(commit=False)
+        if user:


Why exactly do you forbid setting NULL for the author?

webknjaz · 2018-02-07T08:26:14Z

blog/models.py

@@ -27,6 +28,10 @@ def publish(self):
        self.published_date = timezone.now()
        self.save()

+    def get_absolute_url(self):
+        """Return url for view with detail info about post."""
+        return reverse('post_detail', args=[self.pk])


I'd do kwargs instead

webknjaz · 2018-02-07T08:33:29Z

blog/views.py

+
+    def form_valid(self, form):
+        """Pass request.user to model."""
+        form.save(self.request.user)


No, form.save() is already being called after form_valid() by the base view's logic. I think, you probably should only modify the instance like form.instance.author = self.request.user and that's it:
https://docs.djangoproject.com/en/2.0/topics/class-based-views/generic-editing/#models-and-request-user

webknjaz · 2018-02-07T08:36:34Z

blog/views.py

+    fields = ['title', 'text']
+    template_name = 'blog/post_detail.html'
+
+    def get(self, request, *args, **kwargs):


That's weird: why not just POST the change and let update view to handle all internal logic?

Should I remove publish() method from model then? And just pass published_date change?

no, that method is handy

webknjaz · 2018-02-07T08:37:05Z

blog/views.py

+
+    def form_valid(self, form):
+        """Pass parent post pk to model."""
+        post = self.get_object(Post.objects.filter(pk=self.kwargs['pk']))


(same as above)

webknjaz · 2018-02-07T08:37:51Z

blog/views.py

+    model = Comment
+    fields = ['author', 'text']
+
+    def get(self, request, *args, **kwargs):


decide whether you use UpdateView or go for smth simpler.

Rewrite views to class-based. Add debug-logging.

37c4d6a

Miseracle added the enhancement label Jan 28, 2018

Miseracle self-assigned this Jan 28, 2018

Miseracle requested a review from webknjaz January 28, 2018 16:14

webknjaz requested changes Jan 28, 2018

View reviewed changes

Miseracle added 2 commits February 3, 2018 14:32

Remove shotcuts, move logic to forms and built-in CBV methods.

b55b24e

Move comment saving logic from views. Add absolute_url for comments

eda6faa

webknjaz reviewed Feb 3, 2018

View reviewed changes

Remove unused admin-pages from urls, change super() to Py3 style, use

7346ad9

self.object instead of get_object.

webknjaz requested changes Feb 7, 2018

View reviewed changes

		queryset = Post.objects.filter(published_date__isnull=True).order_by('created_date')


		class PublishPost(TemplateView, Protected):

Class-based views #22

Are you sure you want to change the base?

Class-based views #22

Conversation

Miseracle commented Jan 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment