Skip to content

Commit

Permalink
Fix delete review view. Add test cases. (#90)
Browse files Browse the repository at this point in the history
  • Loading branch information
vitorfs authored Sep 18, 2021
1 parent 5c7e902 commit f6d268f
Show file tree
Hide file tree
Showing 9 changed files with 166 additions and 40 deletions.
15 changes: 15 additions & 0 deletions parsifal/apps/authentication/tests/test_user_model.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from django.test.testcases import TestCase
from django.urls import reverse

from parsifal.apps.authentication.tests.factories import UserFactory


class TestUserModel(TestCase):
@classmethod
def setUpTestData(cls):
cls.user = UserFactory(username="john.doe")

def test_get_absolute_url(self):
expected_url = reverse("reviews", args=(self.user.username,))
self.assertEqual(self.user.get_absolute_url(), expected_url)
self.assertEqual(expected_url, "/john.doe/")
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

{% load i18n static %}

{% block title %}{% trans "Review Settings" %} · {{ review.title }}{% endblock %}
{% block title %}{% translate "Review Settings" %} · {{ review.title }}{% endblock %}

{% block javascript %}
<script>
$(function () {

$("#id_user").change(function () {
if ($(this).val() !== "") {
$("#confirm-transfer").prop("disabled", false);
Expand All @@ -15,6 +16,7 @@
$("#confirm-transfer").prop("disabled", true);
}
});

$("#enable-confirm-deletion").click(function () {
if ($(this).is(":checked")) {
$("#confirm-deletion").prop("disabled", false);
Expand All @@ -23,6 +25,11 @@
$("#confirm-deletion").prop("disabled", true);
}
});

$("#formDeleteReview").submit(function () {
$("#formDeleteReview button[type='submit']").disable();
});

});
</script>
{% endblock javascript %}
Expand All @@ -40,31 +47,31 @@
{% csrf_token %}
<div class="panel panel-default">
<div class="panel-heading">
<h3 class="panel-title">{% trans "Review settings" %}</h3>
<h3 class="panel-title">{% translate "Review settings" %}</h3>
</div>
<div class="panel-body">
{% include 'form_vertical.html' with form=form %}
</div>
<div class="panel-footer">
<button type="submit" class="btn btn-success">{% trans "Save changes" %}</button>
<button type="submit" class="btn btn-success">{% translate "Save changes" %}</button>
</div>
</div>
</form>

<div class="panel panel-danger">
<div class="panel-heading">
<h3 class="panel-title">{% trans "Danger zone" %}</h3>
<h3 class="panel-title">{% translate "Danger zone" %}</h3>
</div>
<ul class="list-group">
<li class="list-group-item">
<button type="button" class="btn btn-danger pull-right" data-toggle="modal" data-target="#transfer-review">{% trans "Transfer" %}</button>
<p><strong>{% trans "Transfer ownership" %}</strong></p>
<p style="margin-bottom: 0;">{% trans "Transfer this review to another user." %}</p>
<button type="button" class="btn btn-danger pull-right" data-toggle="modal" data-target="#transfer-review">{% translate "Transfer" %}</button>
<p><strong>{% translate "Transfer ownership" %}</strong></p>
<p style="margin-bottom: 0;">{% translate "Transfer this review to another user." %}</p>
</li>
<li class="list-group-item">
<button type="button" class="btn btn-danger pull-right" data-toggle="modal" data-target="#delete-review">{% trans "Delete" %}</button>
<p><strong>{% trans "Delete this review" %}</strong></p>
<p style="margin-bottom: 0;">{% trans "Once you delete a review, there is no going back. Please be certain." %}</p>
<button type="button" class="btn btn-danger pull-right" data-toggle="modal" data-target="#delete-review">{% translate "Delete" %}</button>
<p><strong>{% translate "Delete this review" %}</strong></p>
<p style="margin-bottom: 0;">{% translate "Once you delete a review, there is no going back. Please be certain." %}</p>
</li>
</ul>
</div>
Expand All @@ -76,41 +83,44 @@ <h3 class="panel-title">{% trans "Danger zone" %}</h3>
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
<h4 class="modal-title" id="transfer-review-title">{% trans "Transfer ownership" %}</h4>
<button type="button" class="close" data-dismiss="modal" aria-label="{% translate "Close" %}"><span aria-hidden="true">&times;</span></button>
<h4 class="modal-title" id="transfer-review-title">{% translate "Transfer ownership" %}</h4>
</div>
<div class="modal-body">
<label for="id_user" class="control-label">{% trans "New owner's Parsifal username:" %}</label>
<label for="id_user" class="control-label">{% translate "New owner's Parsifal username:" %}</label>
<input type="text" id="id_user" name="transfer-user" class="form-control">
</div>
<div class="modal-footer">
<button type="submit" id="confirm-transfer" class="btn btn-danger btn-block">{% trans "Transfer ownership" %}</button>
<button type="submit" id="confirm-transfer" class="btn btn-danger btn-block">{% translate "Transfer ownership" %}</button>
</div>
</div>
</div>
</div>
</form>

<form method="post" action="{% url 'delete_review' %}">
<form method="post" action="{% url "delete_review" review.author.username review.name %}" id="formDeleteReview">
{% csrf_token %}
<input type="hidden" name="review-id" value="{{ review.pk }}">
<div class="modal fade" id="delete-review" tabindex="-1" role="dialog" aria-labelledby="delete-review-title" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
<h4 class="modal-title" id="delete-review-title">Are you sure?</h4>
<button type="button" class="close" data-dismiss="modal" aria-label="{% translate "Close" %}"><span aria-hidden="true">&times;</span></button>
<h4 class="modal-title" id="delete-review-title">{% translate "Are you sure?" %}</h4>
</div>
<div class="modal-body">
<p>This action <strong>CANNOT</strong> be undone. This will permanently delete the <strong>{{ review.title }}</strong> review and all associated data.</p>
<p>
{% blocktranslate trimmed with title=review.title %}
This action <strong>CANNOT</strong> be undone. This will permanently delete the <strong>{{ title }}</strong> review and all associated data.
{% endblocktranslate %}
</p>
<div class="checkbox">
<label>
<input type="checkbox" id="enable-confirm-deletion"> I understand the consequences.
<input type="checkbox" id="enable-confirm-deletion"> {% translate "I understand the consequences." %}
</label>
</div>
</div>
<div class="modal-footer">
<button type="submit" id="confirm-deletion" class="btn btn-danger btn-block" disabled>Delete this review</button>
<button type="submit" id="confirm-deletion" class="btn btn-danger btn-block" data-loading="{% translate "Deleting... please wait" %}" disabled>{% translate "Delete this review" %}</button>
</div>
</div>
</div>
Expand Down
Empty file.
65 changes: 65 additions & 0 deletions parsifal/apps/reviews/settings/tests/test_delete_review_view.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
from django.contrib.auth.models import User
from django.test.testcases import TestCase
from django.urls import reverse

from parsifal.apps.authentication.tests.factories import UserFactory
from parsifal.apps.reviews.models import Review, Source
from parsifal.apps.reviews.tests.factories import DefaultSourceFactory, ReviewFactory, SourceFactory
from parsifal.utils.test import login_redirect_url


class TestDeleteReviewView(TestCase):
@classmethod
def setUpTestData(cls):
cls.author = UserFactory()
cls.co_author = UserFactory()
cls.default_source = DefaultSourceFactory()
cls.review_source = SourceFactory()
cls.review = ReviewFactory(
author=cls.author, co_authors=[cls.co_author], sources=[cls.default_source, cls.review_source]
)
cls.url = reverse("delete_review", args=(cls.review.author.username, cls.review.name))

def test_setup(self):
self.assertEqual(2, User.objects.count())
self.assertEqual(2, Source.objects.count())
self.assertEqual(self.co_author, self.review.co_authors.first())

def test_get_not_allowed(self):
self.client.force_login(self.author)
response = self.client.get(self.url)
self.assertEqual(405, response.status_code)

def test_login_required(self):
response = self.client.post(self.url)
with self.subTest(msg="Test redirect"):
self.assertRedirects(response, login_redirect_url(self.url))
with self.subTest(msg="Test review not deleted"):
self.assertTrue(Review.objects.filter(pk=self.review.pk).exists())
self.assertEqual(2, Source.objects.count())

def test_main_author_required(self):
self.client.force_login(self.co_author)
response = self.client.post(self.url)
with self.subTest(msg="Test status code"):
self.assertEqual(403, response.status_code)
with self.subTest(msg="Test review not deleted"):
self.assertTrue(Review.objects.filter(pk=self.review.pk).exists())
self.assertEqual(2, Source.objects.count())

def test_delete_successful(self):
self.client.force_login(self.author)
response = self.client.post(self.url, follow=True)

with self.subTest(msg="Test post status code"):
self.assertEqual(302, response.redirect_chain[0][1])

with self.subTest(msg="Test post redirect status code"):
self.assertEqual(200, response.status_code)

with self.subTest(msg="Test review deleted"):
self.assertFalse(Review.objects.filter(pk=self.review.pk).exists())
self.assertEqual(1, Source.objects.count())

with self.subTest(msg="Test default source not deleted"):
self.assertTrue(Source.objects.filter(pk=self.default_source.pk).exists())
31 changes: 16 additions & 15 deletions parsifal/apps/reviews/settings/views.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.contrib.auth.mixins import LoginRequiredMixin
from django.contrib.auth.models import User
from django.db import transaction
from django.http import HttpResponseBadRequest
from django.shortcuts import redirect, render
from django.urls import reverse as r
from django.utils.text import slugify
from django.views.decorators.http import require_POST
from django.utils.translation import gettext
from django.views import View

from parsifal.apps.reviews.decorators import main_author_required
from parsifal.apps.reviews.mixins import MainAuthorRequiredMixin, ReviewMixin
from parsifal.apps.reviews.models import Review
from parsifal.apps.reviews.settings.forms import ReviewSettingsForm

Expand Down Expand Up @@ -64,17 +68,14 @@ def transfer(request):
return HttpResponseBadRequest("Something went wrong.")


@main_author_required
@login_required
@require_POST
def delete(request):
review_id = request.POST.get("review-id")
review = Review.objects.get(pk=review_id)
sources = review.sources.all()
for source in sources:
if not source.is_default:
review.sources.remove(source)
source.delete()
review.delete()
messages.success(request, "The review was deleted successfully.")
return redirect(r("reviews", args=(review.author.username,)))
class DeleteReviewView(LoginRequiredMixin, MainAuthorRequiredMixin, ReviewMixin, View):
@transaction.atomic()
def post(self, request, *args, **kwargs):
sources = self.review.sources.all()
for source in sources:
if not source.is_default:
self.review.sources.remove(source)
source.delete()
self.review.delete()
messages.success(request, gettext("The review was deleted with success."))
return redirect(request.user)
32 changes: 31 additions & 1 deletion parsifal/apps/reviews/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from factory.django import DjangoModelFactory

from parsifal.apps.authentication.tests.factories import UserFactory
from parsifal.apps.reviews.models import Review
from parsifal.apps.reviews.models import Review, Source


class ReviewFactory(DjangoModelFactory):
Expand All @@ -13,3 +13,33 @@ class ReviewFactory(DjangoModelFactory):
class Meta:
model = Review
django_get_or_create = ("name",)

@factory.post_generation
def co_authors(self, create, extracted, **kwargs):
if not create:
return

if extracted:
self.co_authors.add(*extracted)

@factory.post_generation
def sources(self, create, extracted, **kwargs):
if not create:
return

if extracted:
self.sources.add(*extracted)


class SourceFactory(DjangoModelFactory):
name = factory.Sequence(lambda n: f"Source #{n}")
url = factory.Sequence(lambda n: f"https://source-{n}.example.com")
is_default = False

class Meta:
model = Source
django_get_or_create = ("name",)


class DefaultSourceFactory(SourceFactory):
is_default = True
1 change: 1 addition & 0 deletions parsifal/newsfragments/89.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes error while trying to delete a literature review
4 changes: 2 additions & 2 deletions parsifal/static/js/parsifal.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@

$.fn.disable = function () {
$(this).prop("disabled", true);
$(this).attr("data-original", $(this).text());
$(this).text($(this).attr("data-loading"));
$(this).data("original", $(this).text());
$(this).text($(this).data("loading"));
};

$.fn.enable = function () {
Expand Down
6 changes: 5 additions & 1 deletion parsifal/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
path("library/", include("parsifal.apps.library.urls", namespace="library")),
path("settings/", include("parsifal.apps.accounts.urls", namespace="settings")),
path("review_settings/transfer/", reviews_settings_views.transfer, name="transfer_review"),
path("review_settings/delete/", reviews_settings_views.delete, name="delete_review"),
path("admin/", admin.site.urls),
path("sitemap.xml", sitemap, {"sitemaps": sitemaps}, name="sitemap"),
path("robots.txt", TemplateView.as_view(template_name="robots.txt", content_type="text/plain")),
Expand All @@ -55,6 +54,11 @@
"<str:username>/<str:review_name>/settings/invites/",
include("parsifal.apps.invites.urls", namespace="invites"),
),
path(
"<str:username>/<str:review_name>/settings/delete/",
reviews_settings_views.DeleteReviewView.as_view(),
name="delete_review",
),
# Planning Phase
path("<str:username>/<str:review_name>/planning/", reviews_planning_views.planning, name="planning"),
path("<str:username>/<str:review_name>/planning/protocol/", reviews_planning_views.protocol, name="protocol"),
Expand Down

0 comments on commit f6d268f

Please sign in to comment.