Skip to content

Commit c0652e3

Browse files
IdirLISNDidayolo
andauthored
Fix redirection when removing a forum post or a thread (#2257)
* debug redirection OK * forums unit test rewrite OK * Flake8 fix * Clean code --------- Co-authored-by: Adrien Pavão <adrien.pavao@gmail.com>
1 parent e2a8481 commit c0652e3

2 files changed

Lines changed: 150 additions & 43 deletions

File tree

Lines changed: 131 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,159 @@
1+
# apps/forums/tests/test_smoke_tests.py
12
from django.test import TestCase
23
from django.contrib.auth import get_user_model
34
from django.urls import reverse
45

5-
from competitions.models import Competition
6+
from competitions.models import Competition, CompetitionParticipant
67
from ..models import Forum, Thread, Post
78

8-
99
User = get_user_model()
1010

1111

12-
class ForumSmokeTests(TestCase):
12+
class ForumTests(TestCase):
1313

14-
def setUp(self):
15-
self.admin_user = User.objects.create_superuser("admin", "admin@example.com", "pass")
16-
self.regular_user = User.objects.create_user("regular", email="regular@example.com", password="pass")
14+
@classmethod
15+
def setUpTestData(cls):
16+
cls.admin_user = User.objects.create_superuser(
17+
username="admin",
18+
email="admin@example.com",
19+
password="pass"
20+
)
21+
cls.regular_user = User.objects.create_user(
22+
username="regular",
23+
email="regular@example.com",
24+
password="pass"
25+
)
26+
cls.other_user = User.objects.create_user(
27+
username="other",
28+
email="other@example.com",
29+
password="pass"
30+
)
1731

18-
self.competition = Competition.objects.create(
32+
cls.competition = Competition.objects.create(
1933
title="Test Competition",
20-
created_by=self.admin_user,
34+
created_by=cls.admin_user,
2135
published=False,
36+
forum_enabled=True
2237
)
23-
self.forum = Forum.objects.create(competition=self.competition)
24-
self.thread = Thread.objects.create(forum=self.forum, started_by=self.regular_user)
25-
self.post = Post.objects.create(thread=self.thread, posted_by=self.regular_user)
2638

27-
def test_forum_thread_list_view_returns_200(self):
28-
resp = self.client.get(reverse("forums:forum_detail", kwargs={'forum_pk': self.forum.pk}))
29-
self.assertEqual(resp.status_code, 200)
39+
cls.forum = Forum.objects.create(competition=cls.competition)
3040

31-
def test_forum_post_new_thread_non_logged_in_returns_302(self):
32-
resp = self.client.get(reverse("forums:forum_new_thread", kwargs={'forum_pk': self.forum.pk}))
33-
self.assertEqual(resp.status_code, 302)
41+
CompetitionParticipant.objects.create(
42+
competition=cls.competition,
43+
user=cls.regular_user,
44+
status=CompetitionParticipant.APPROVED
45+
)
3446

35-
def test_forum_post_new_thread_view_returns_200(self):
36-
self.client.login(username="regular", password="pass")
37-
resp = self.client.get(reverse("forums:forum_new_thread", kwargs={'forum_pk': self.forum.pk}))
47+
cls.thread = Thread.objects.create(
48+
forum=cls.forum,
49+
started_by=cls.regular_user
50+
)
51+
52+
cls.post = Post.objects.create(
53+
thread=cls.thread,
54+
posted_by=cls.regular_user,
55+
content="Initial post"
56+
)
57+
58+
def test_forum_detail_view_returns_200(self):
59+
resp = self.client.get(reverse("forums:forum_detail", kwargs={"forum_pk": self.forum.pk}))
3860
self.assertEqual(resp.status_code, 200)
3961

40-
def test_forum_view_thread_returns_200(self):
41-
resp = self.client.get(reverse("forums:forum_thread_detail", kwargs={'forum_pk': self.forum.pk, 'thread_pk': self.thread.pk}))
62+
def test_thread_detail_view_returns_200(self):
63+
resp = self.client.get(reverse(
64+
"forums:forum_thread_detail",
65+
kwargs={"forum_pk": self.forum.pk, "thread_pk": self.thread.pk}
66+
))
4267
self.assertEqual(resp.status_code, 200)
4368

44-
def test_forum_new_post_requires_login_returns_302(self):
45-
resp = self.client.get(reverse("forums:forum_new_post", kwargs={'forum_pk': self.forum.pk, 'thread_pk': self.thread.pk}))
69+
def test_create_thread_requires_login(self):
70+
resp = self.client.get(reverse("forums:forum_new_thread", kwargs={"forum_pk": self.forum.pk}))
4671
self.assertEqual(resp.status_code, 302)
4772

48-
def test_forum_new_post_returns_200(self):
73+
def test_create_thread_post(self):
4974
self.client.login(username="regular", password="pass")
50-
resp = self.client.get(reverse("forums:forum_new_post", kwargs={'forum_pk': self.forum.pk, 'thread_pk': self.thread.pk}))
51-
self.assertEqual(resp.status_code, 200)
75+
resp = self.client.post(reverse("forums:forum_new_thread", kwargs={"forum_pk": self.forum.pk}), {
76+
"title": "New thread",
77+
"content": "Hello world",
78+
})
79+
self.assertEqual(resp.status_code, 302)
80+
self.assertEqual(Thread.objects.count(), 2)
81+
82+
def test_create_post_requires_login(self):
83+
resp = self.client.get(reverse(
84+
"forums:forum_new_post",
85+
kwargs={"forum_pk": self.forum.pk, "thread_pk": self.thread.pk}
86+
))
87+
self.assertEqual(resp.status_code, 302)
5288

53-
def test_forum_delete_post_returns_200(self):
54-
self.client.login(username='admin', password='pass')
55-
resp = self.client.delete(reverse("forums:forum_delete_post", kwargs={'forum_pk': self.forum.pk, 'thread_pk': self.thread.pk, 'post_pk': self.post.pk}))
89+
def test_create_post(self):
90+
self.client.login(username="regular", password="pass")
91+
resp = self.client.post(reverse(
92+
"forums:forum_new_post",
93+
kwargs={"forum_pk": self.forum.pk, "thread_pk": self.thread.pk}
94+
), {"content": "Another message"})
5695
self.assertEqual(resp.status_code, 302)
96+
self.assertEqual(Post.objects.count(), 2)
5797

58-
def test_forum_delete_thread_returns_200(self):
59-
self.client.login(username='admin', password='pass')
60-
resp = self.client.delete(reverse("forums:forum_delete_thread", kwargs={'forum_pk': self.forum.pk, 'thread_pk': self.thread.pk}))
98+
def test_delete_post_by_admin(self):
99+
self.client.login(username="admin", password="pass")
100+
resp = self.client.post(reverse(
101+
"forums:forum_delete_post",
102+
kwargs={
103+
"forum_pk": self.forum.pk,
104+
"thread_pk": self.thread.pk,
105+
"post_pk": self.post.pk
106+
}
107+
))
61108
self.assertEqual(resp.status_code, 302)
109+
self.assertEqual(Post.objects.filter(pk=self.post.pk).count(), 0)
110+
111+
def test_delete_post_forbidden_for_other_user(self):
112+
p = Post.objects.create(thread=self.thread, posted_by=self.regular_user, content="temp-forb")
113+
114+
self.client.login(username="other", password="pass")
115+
resp = self.client.post(reverse(
116+
"forums:forum_delete_post",
117+
kwargs={
118+
"forum_pk": self.forum.pk,
119+
"thread_pk": self.thread.pk,
120+
"post_pk": p.pk
121+
}
122+
))
123+
124+
exists_after = Post.objects.filter(pk=p.pk).exists()
125+
self.assertIn(resp.status_code, (302, 403))
126+
if resp.status_code == 403:
127+
self.assertTrue(exists_after, "Post should remain when deletion is forbidden (403).")
128+
129+
def test_delete_thread_by_admin(self):
130+
t = Thread.objects.create(forum=self.forum, started_by=self.regular_user)
131+
Post.objects.create(thread=t, posted_by=self.regular_user, content="to be deleted")
132+
133+
self.client.login(username="admin", password="pass")
134+
resp = self.client.post(reverse(
135+
"forums:forum_delete_thread",
136+
kwargs={
137+
"forum_pk": self.forum.pk,
138+
"thread_pk": t.pk
139+
}
140+
))
141+
self.assertEqual(resp.status_code, 302)
142+
self.assertEqual(Thread.objects.filter(pk=t.pk).count(), 0)
143+
144+
def test_delete_thread_forbidden_for_other_user(self):
145+
t = Thread.objects.create(forum=self.forum, started_by=self.regular_user)
146+
Post.objects.create(thread=t, posted_by=self.regular_user, content="keep me")
147+
148+
self.client.login(username="other", password="pass")
149+
resp = self.client.post(reverse(
150+
"forums:forum_delete_thread",
151+
kwargs={
152+
"forum_pk": self.forum.pk,
153+
"thread_pk": t.pk
154+
}
155+
))
156+
157+
self.assertIn(resp.status_code, (302, 403))
158+
if resp.status_code == 403:
159+
self.assertEqual(Thread.objects.filter(pk=t.pk).count(), 1)

src/apps/forums/views.py

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,25 @@ class DeletePostView(ForumBaseMixin, LoginRequiredMixin, DeleteView):
115115
model = Post
116116
pk_url_kwarg = 'post_pk'
117117

118-
def delete(self, request, *args, **kwargs):
118+
def get_success_url(self):
119+
post = self.get_object()
120+
if post.thread:
121+
return post.thread.get_absolute_url() if post.thread.posts.count() > 1 else post.thread.forum.get_absolute_url()
122+
return '/'
123+
124+
def form_valid(self, form):
119125
self.object = self.get_object()
120126

121-
if self.object.posted_by == request.user or \
122-
request.user in self.object.thread.forum.competition.collaborators.all() or \
123-
self.object.thread.forum.competition.created_by == request.user:
127+
if self.object.posted_by == self.request.user or \
128+
self.request.user in self.object.thread.forum.competition.collaborators.all() or \
129+
self.object.thread.forum.competition.created_by == self.request.user:
124130
# If there are more posts in the thread, leave it around, otherwise delete it
131+
success_url = self.get_success_url()
125132
if self.object.thread.posts.count() == 1:
126-
success_url = self.object.thread.forum.get_absolute_url()
127133
self.object.thread.delete()
128-
else:
129-
success_url = self.object.thread.get_absolute_url()
130134
self.object.delete()
131135
return HttpResponseRedirect(success_url)
136+
132137
else:
133138
raise PermissionDenied("Cannot delete a post you don't own in a competition you aren't organizing!")
134139

@@ -165,11 +170,15 @@ class DeleteThreadView(ForumBaseMixin, LoginRequiredMixin, DeleteView):
165170
model = Thread
166171
pk_url_kwarg = 'thread_pk'
167172

168-
def delete(self, request, *args, **kwargs):
173+
def get_success_url(self):
174+
thread = self.get_object()
175+
return thread.forum.get_absolute_url()
176+
177+
def form_valid(self, form):
169178
self.object = self.get_object()
170179

171-
if self.object.forum.competition.created_by == request.user or \
172-
self.object.started_by == request.user:
180+
if self.object.forum.competition.created_by == self.request.user or \
181+
self.object.started_by == self.request.user:
173182

174183
success_url = self.object.forum.get_absolute_url()
175184
self.object.delete()

0 commit comments

Comments
 (0)