Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎮Implement the functionality to create and edit blog posts #39

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions blog/forms.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
"""The forms for the Blog application."""

from django import forms

from .models import Post


class PostForm(forms.ModelForm):
"""Form for editing posts in the Blog application."""

class Meta:
"""Metadata for the post form."""

model = Post
fields = ('title', 'text')
16 changes: 12 additions & 4 deletions blog/templates/blog/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,19 @@
</head>
<body>
<header class="page-header">
<div class="container">
<h1><a href="/">Django Girls Blog</a></h1>
</div>
<div class="container">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change the indentation here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edited

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What did you edit exactly and how?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry I didn't understand what I should correct

Copy link
Member

@webknjaz webknjaz Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restore the original indentation, I suppose.

{% if user.is_authenticated %}
<a href="{% url 'post_new' %}" class="top-menu">
{% include './icons/file-earmark-plus.svg' %}
</a>
<p class="top-menu">Hello {{ user.username }} <small>(<a href="{% url 'logout' %}">Log out</a>)</small></p>
{% else %}
<a href="{% url 'login' %}" class="top-menu"><span class="glyphicon glyphicon-lock"></span></a>
{% endif %}
<h1><a href="/">Django Girls Blog</a></h1>
</div>
</header>
<main class="container">
<main class="content container">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this relate to editing the posts?

<div class="row">
<div class="col">
{% block content %}
Expand Down
4 changes: 4 additions & 0 deletions blog/templates/blog/icons/file-earmark-plus.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions blog/templates/blog/icons/pencil-fill.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions blog/templates/blog/post_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

{% block content %}
<article class="post">
<aside class="actions">
{% if user.is_authenticated %}
<a class="btn btn-secondary" href="{% url 'post_edit' pk=post.pk %}">
{% include './icons/pencil-fill.svg' %}
</a>
{% endif %}
</aside>
{% if post.published_date %}
<time class="date">
{{ post.published_date }}
Expand Down
9 changes: 9 additions & 0 deletions blog/templates/blog/post_edit.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{% extends 'blog/base.html' %}

{% block content %}
<h2>New post</h2>
<form method="POST" class="post-form">{% csrf_token %}
{{ form.as_p }}
<button type="submit" class="save btn btn-secondary">Save</button>
</form>
{% endblock %}
24 changes: 24 additions & 0 deletions blog/templates/registration/login.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{% extends "blog/base.html" %}

{% block content %}
{% if form.errors %}
<p>Your username and password didn't match. Please try again.</p>
{% endif %}

<form method="post" action="{% url 'login' %}">
{% csrf_token %}
<table>
<tr>
<td>{{ form.username.label_tag }}</td>
<td>{{ form.username }}</td>
</tr>
<tr>
<td>{{ form.password.label_tag }}</td>
<td>{{ form.password }}</td>
</tr>
</table>

<input type="submit" value="login" />
<input type="hidden" name="next" value="{{ next }}" />
</form>
{% endblock %}
2 changes: 2 additions & 0 deletions blog/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@
urlpatterns = [
path('', views.post_list, name='post_list'),
path('post/<int:pk>/', views.post_detail, name='post_detail'),
path('post/new/', views.post_new, name='post_new'),
path('post/<int:pk>/edit/', views.post_edit, name='post_edit'),
]
37 changes: 36 additions & 1 deletion blog/views.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
"""The views for the Blog application."""

from django.shortcuts import get_object_or_404, render
from django.contrib.auth.decorators import login_required
from django.shortcuts import get_object_or_404, redirect, render
from django.utils import timezone

from .forms import PostForm
from .models import Post


Expand All @@ -20,3 +22,36 @@ def post_detail(request, pk):
"""Display a blog post based on the post's primary key."""
post = get_object_or_404(Post, pk=pk)
return render(request, 'blog/post_detail.html', {'post': post})


@login_required
def post_new(request):
"""Create a new post."""
if request.method == 'POST':
form = PostForm(request.POST)
if form.is_valid():
post = form.save(commit=False)
post.author = request.user
post.published_date = timezone.now()
post.save()
return redirect('post_detail', pk=post.pk)
else:
form = PostForm()
return render(request, 'blog/post_edit.html', {'form': form})


@login_required
def post_edit(request, pk):
"""Edit an existing post."""
post = get_object_or_404(Post, pk=pk)
if request.method == 'POST':
form = PostForm(request.POST, instance=post)
if form.is_valid():
post = form.save(commit=False)
post.author = request.user
post.published_date = timezone.now()
post.save()
return redirect('post_detail', pk=post.pk)
else:
form = PostForm(instance=post)
return render(request, 'blog/post_edit.html', {'form': form})
6 changes: 6 additions & 0 deletions mysite/urls.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
"""mysite URL Configuration."""

from django.contrib import admin
from django.contrib.auth import views
from django.urls import include, path

urlpatterns = [
path('admin/', admin.site.urls),
path('accounts/login/', views.LoginView.as_view(), name='login'),
path(
'accounts/logout/', views.LogoutView.as_view(next_page='/'),
name='logout',
),
path('', include('blog.urls')),
]
10 changes: 5 additions & 5 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
#
# pip-compile requirements/base.in
#
asgiref==3.7.2
asgiref==3.8.1
# via django
django==3.2.21
# via -r base.in
pytz==2023.3.post1
django==3.2.25
# via -r requirements/base.in
pytz==2024.2
# via django
sqlparse==0.4.4
sqlparse==0.5.2
# via django
20 changes: 10 additions & 10 deletions requirements/dev.txt
Copy link
Member

@webknjaz webknjaz Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps perform package version bumps in a separate PR?

Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,31 @@
#
# pip-compile requirements/dev.in
#
asgiref==3.7.2
asgiref==3.8.1
# via django
astroid==3.0.2
astroid==3.3.6
# via pylint
covdefaults==2.3.0
# via -r requirements/dev.in
coverage==7.3.4
coverage==7.6.9
# via
# -r requirements/dev.in
# covdefaults
dill==0.3.7
dill==0.3.9
# via pylint
django==3.2.23
django==3.2.25
# via -r requirements/base.in
isort==5.13.2
# via pylint
mccabe==0.7.0
# via pylint
platformdirs==4.1.0
platformdirs==4.3.6
# via pylint
pylint==3.0.3
pylint==3.3.2
# via -r requirements/dev.in
pytz==2023.3.post1
pytz==2024.2
# via django
sqlparse==0.4.4
sqlparse==0.5.2
# via django
tomlkit==0.12.3
tomlkit==0.13.2
# via pylint
110 changes: 110 additions & 0 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ def setUpTestData(cls):
published_date=datetime(2030, 3, 1, tzinfo=cls.tm_zone),
)

cls.valid_form_post = {
'title': 'Test Title',
'text': 'Test Text',
}

cls.invalid_form_post = {
'title': '',
'text': 'Test Text',
}

def test_post_list_rendering(self):
"""Check correct posts displayed based on mocked current time."""
post_list_url = reverse('post_list')
Expand Down Expand Up @@ -86,3 +96,103 @@ def test_post_detail_failed(self):
reverse('post_detail', kwargs={'pk': 31}),
)
self.assertEqual(404, http_response.status_code)

def test_post_new_get_authorized(self):
"""Ensure authorized users can access the new post page."""
self.client.force_login(self.user)
url = reverse('post_new')
http_response = self.client.get(url)
self.assertEqual(http_response.status_code, 200)
self.assertTemplateUsed(http_response, 'blog/post_edit.html')

def test_post_new_get_unauthorized(self):
"""Ensure unauthorized users are redirected when add the new post."""
self.client.logout()
url = reverse('post_new')
http_response = self.client.get(url)
self.assertRedirects(http_response, f'/accounts/login/?next={url}')

def test_post_new_valid_form_authorized(self):
"""Ensure authorized users can correctly fill out the form for post."""
self.client.force_login(self.user)
url = reverse('post_new')
initial_post_count = Post.objects.count()
http_response = self.client.post(
url, self.valid_form_post, follow=True,
)
self.assertEqual(http_response.status_code, 200)
self.assertTemplateUsed(http_response, 'blog/post_detail.html')
new_post = Post.objects.last()
self.assertRedirects(
http_response,
reverse(
'post_detail',
kwargs={'pk': new_post.pk},
),
)
self.assertEqual(new_post.title, 'Test Title')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit difficult to connect the dots when this constant string is defined here, but the data it compares to is defined elsewhere. Why not reuse the same value saved into a var?

self.assertEqual(new_post.text, 'Test Text')
self.assertEqual(new_post.author, self.user)
self.assertIsNotNone(new_post.published_date)
self.assertEqual(Post.objects.count(), initial_post_count + 1)

def test_post_new_invalid_form_authorized(self):
"""Test handling of invalid form submission for new post."""
self.client.force_login(self.user)
url = reverse('post_new')
http_response = self.client.post(url, self.invalid_form_post)
self.assertEqual(http_response.status_code, 200)
self.assertTemplateUsed(http_response, 'blog/post_edit.html')
self.assertContains(
http_response, 'This field is required.', html=True,
)
Comment on lines +146 to +148
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


def test_post_edit_get_authorized(self):
"""Ensure authorized users have access to edit the post."""
self.client.force_login(self.user)
post = self.current_post
url = reverse('post_edit', kwargs={'pk': post.pk})
http_response = self.client.get(url)
self.assertEqual(http_response.status_code, 200)
self.assertTemplateUsed(http_response, 'blog/post_edit.html')

def test_post_edit_get_unauthorized(self):
"""Ensure unauthorized users are redirected when edit a post."""
self.client.logout()
url = reverse('post_edit', kwargs={'pk': self.current_post.pk})
http_response = self.client.get(url)
self.assertRedirects(http_response, f'/accounts/login/?next={url}')

def test_post_edit_valid_form_authorized(self):
"""Check the correctness of updating the post after editing."""
self.client.force_login(self.user)
post = self.current_post
url = reverse('post_edit', kwargs={'pk': post.pk})
updated_form_data = self.valid_form_post.copy()
updated_form_data['title'] = 'Updated Title'
updated_form_data['text'] = 'Updated Text'
initial_published_date = post.published_date
http_response = self.client.post(url, updated_form_data, follow=True)
self.assertEqual(http_response.status_code, 200)
self.assertTemplateUsed(http_response, 'blog/post_detail.html')
self.assertRedirects(
http_response, reverse(
'post_detail', kwargs={'pk': post.pk},
),
)
post.refresh_from_db()
self.assertEqual(post.title, 'Updated Title')
self.assertEqual(post.text, 'Updated Text')
self.assertEqual(post.author, self.user)
self.assertNotEqual(post.published_date, initial_published_date)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is a useful check, the best way of dealing with the time changes is freezing it. There are libraries like freezegun and similar that monkey-patch the stdlib that could be used. But since you did it manually in the past already, I suggest repeating the same technique here: https://github.com/kpi-web-guild/django-girls-blog-OlenaEfymenko/blob/f6ccd09/tests/test_views.py#L60-L63.


def test_post_edit_invalid_form_authorized(self):
"""Test handling of invalid form submission when editing a post."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's best to avoid using the vague "test doing something generic and abstract" and focus on declaring the expectation. The outcome of this test is probably not that you executed a test for the sake of executing a test that mentions handling the form. Instead, you probably expect that using a form in a specific way would result in a specific reaction of the server, don't you? Why not mention that specific thing in the docstring?

self.client.force_login(self.user)
url = reverse('post_edit', kwargs={'pk': self.current_post.pk})
http_response = self.client.post(url, self.invalid_form_post)
self.assertEqual(http_response.status_code, 200)
self.assertTemplateUsed(http_response, 'blog/post_edit.html')
self.assertContains(
http_response, 'This field is required.', html=True,
)
Loading