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

[#898] Added 'delete' operation to menu in Action list #344

Merged
merged 8 commits into from
Nov 30, 2022
43 changes: 39 additions & 4 deletions src/open_inwoner/accounts/admin.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from django.contrib import admin
from django.contrib import admin, messages
from django.contrib.auth.admin import UserAdmin
from django.urls import reverse, reverse_lazy
from django.utils.html import format_html
from django.utils.translation import ugettext_lazy as _
from django.utils.translation import ngettext, ugettext_lazy as _

from privates.admin import PrivateMediaMixin

Expand Down Expand Up @@ -88,9 +88,44 @@ class _UserAdmin(UserAdmin):
@admin.register(Action)
class ActionAdmin(UUIDAdminFirstInOrder, PrivateMediaMixin, admin.ModelAdmin):
readonly_fields = ("uuid",)
list_display = ("name", "created_on", "created_by")
list_filter = ("created_by",)
list_display = ("name", "created_on", "created_by", "is_deleted")
list_filter = (
"is_deleted",
"created_by",
)
private_media_fields = ("file",)
actions = [
"mark_not_deleted",
"mark_deleted",
]

@admin.action(description=_("Mark selected actions as soft-deleted by user."))
def mark_deleted(self, request, queryset):
updated = queryset.update(is_deleted=True)
self.message_user(
request,
ngettext(
"%d actions was successfully marked as deleted.",
"%d stories were successfully marked as deleted.",
updated,
)
% updated,
messages.SUCCESS,
)

@admin.action(description=_("Restore selected soft-deleted actions"))
def mark_not_deleted(self, request, queryset):
updated = queryset.update(is_deleted=False)
self.message_user(
request,
ngettext(
"%d actions was successfully restored.",
"%d stories were successfully restored.",
updated,
)
% updated,
messages.SUCCESS,
)


@admin.register(Contact)
Expand Down
3 changes: 3 additions & 0 deletions src/open_inwoner/accounts/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,8 @@ def get_extended_contact_users(self, me):


class ActionQueryset(QuerySet):
def visible(self):
return self.filter(is_deleted=False)

def connected(self, user):
return self.filter(Q(created_by=user) | Q(is_for=user)).distinct()
22 changes: 22 additions & 0 deletions src/open_inwoner/accounts/migrations/0047_action_is_deleted.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Generated by Django 3.2.15 on 2022-11-21 14:02

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("accounts", "0046_alter_user_oidc_id"),
]

operations = [
migrations.AddField(
model_name="action",
name="is_deleted",
field=models.BooleanField(
default=False,
verbose_name="Is soft-deleted",
help_text="This indicates a soft-delete where an admin can restore the record.",
),
),
]
7 changes: 7 additions & 0 deletions src/open_inwoner/accounts/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,13 @@ class Action(models.Model):
related_name="actions",
help_text=_("The plan that the action belongs to. This can be left empty."),
)
is_deleted = models.BooleanField(
verbose_name=_("Is soft-deleted"),
default=False,
Bartvaderkin marked this conversation as resolved.
Show resolved Hide resolved
help_text=_(
"This indicates a soft-delete where an admin can restore the record."
),
)
logs = GenericRelation(TimelineLog)

objects = ActionQueryset.as_manager()
Expand Down
77 changes: 72 additions & 5 deletions src/open_inwoner/accounts/tests/test_action_views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from datetime import date

from django.contrib.messages import get_messages
from django.core.files.uploadedfile import SimpleUploadedFile
from django.urls import reverse
from django.utils.translation import gettext as _

from django_webtest import WebTest
from privates.test import temp_private_root
Expand All @@ -20,12 +22,20 @@ def setUp(self) -> None:
created_by=self.user,
file=SimpleUploadedFile("file.txt", b"test content"),
)
self.action_deleted = ActionFactory(
name="action_that_was_deleted",
created_by=self.user,
is_deleted=True,
)

self.login_url = reverse("login")
self.list_url = reverse("accounts:action_list")
self.edit_url = reverse(
"accounts:action_edit", kwargs={"uuid": self.action.uuid}
)
self.delete_url = reverse(
"accounts:action_delete", kwargs={"uuid": self.action.uuid}
)
self.create_url = reverse("accounts:action_create")
self.export_url = reverse(
"accounts:action_export", kwargs={"uuid": self.action.uuid}
Expand All @@ -38,6 +48,9 @@ def setUp(self) -> None:
"accounts:action_history", kwargs={"uuid": self.action.uuid}
)

def test_queryset_visible(self):
self.assertEqual(list(Action.objects.visible()), [self.action])

def test_action_list_login_required(self):
response = self.app.get(self.list_url)
self.assertRedirects(response, f"{self.login_url}?next={self.list_url}")
Expand All @@ -46,6 +59,7 @@ def test_action_list_filled(self):
response = self.app.get(self.list_url, user=self.user)
self.assertEqual(response.status_code, 200)
self.assertContains(response, self.action.name)
self.assertEqual(list(response.context["actions"]), [self.action])

def test_action_list_filter_is_for(self):
user = UserFactory()
Expand All @@ -72,7 +86,6 @@ def test_action_list_filter_status(self):
action2 = ActionFactory(
end_date="2021-04-02", status=StatusChoices.open, is_for=self.user
)
self.assertEqual(Action.objects.count(), 3)
response = self.app.get(
f"{self.list_url}?status={StatusChoices.open}", user=self.user
)
Expand All @@ -95,6 +108,7 @@ def test_action_list_only_show_personal_actions(self):
response = self.app.get(self.list_url, user=other_user)
self.assertEqual(response.status_code, 200)
self.assertNotContains(response, self.action.name)
self.assertEqual(list(response.context["actions"]), [])

def test_action_edit_login_required(self):
response = self.app.get(self.edit_url)
Expand All @@ -107,7 +121,41 @@ def test_action_edit(self):

def test_action_edit_not_your_action(self):
other_user = UserFactory()
response = self.app.get(self.edit_url, user=other_user, status=404)
self.app.get(self.edit_url, user=other_user, status=404)

def test_action_edit_deleted_action(self):
url = reverse("accounts:action_edit", kwargs={"uuid": self.action_deleted.uuid})
self.app.get(url, user=self.user, status=404)

def test_action_delete_login_required(self):
response = self.client.post(self.delete_url)
self.assertEquals(response.status_code, 403)

def test_action_delete_http_get_is_not_allowed(self):
self.client.force_login(self.user)
response = self.client.get(self.delete_url)
self.assertEqual(response.status_code, 405)

def test_action_delete(self):
self.client.force_login(self.user)
response = self.client.post(self.delete_url)
self.assertEqual(response.status_code, 302)
self.assertRedirects(response, self.list_url)

# Action is now marked as .is_deleted (and not actually deleted)
action = Action.objects.get(id=self.action.id)
self.assertTrue(action.is_deleted)

messages = list(get_messages(response.wsgi_request))
self.assertEqual(len(messages), 1)
expected = _("Actie '{action}' is verwijdered.").format(action=self.action)
self.assertEqual(str(messages[0]), expected)

def test_action_delete_not_your_action(self):
other_user = UserFactory()
self.client.force_login(other_user)
response = self.client.post(self.delete_url)
self.assertEqual(response.status_code, 404)

def test_action_create_login_required(self):
response = self.app.get(self.create_url)
Expand All @@ -128,7 +176,13 @@ def test_action_export_login_required(self):

def test_action_export_not_your_action(self):
other_user = UserFactory()
response = self.app.get(self.export_url, user=other_user, status=404)
self.app.get(self.export_url, user=other_user, status=404)

def test_action_export_deleted_action(self):
url = reverse(
"accounts:action_export", kwargs={"uuid": self.action_deleted.uuid}
)
self.app.get(url, user=self.user, status=404)

def test_action_list_export(self):
response = self.app.get(self.export_list_url, user=self.user)
Expand All @@ -137,6 +191,7 @@ def test_action_list_export(self):
self.assertEqual(
response["Content-Disposition"], f'attachment; filename="actions.pdf"'
)
self.assertEqual(list(response.context["actions"]), [self.action])

def test_action_list_export_login_required(self):
response = self.app.get(self.export_list_url)
Expand All @@ -154,7 +209,13 @@ def test_action_download_login_required(self):

def test_action_download_not_your_action(self):
other_user = UserFactory()
response = self.app.get(self.download_url, user=other_user, status=403)
self.app.get(self.download_url, user=other_user, status=403)

def test_action_download_deleted_action(self):
url = reverse(
"accounts:action_download", kwargs={"uuid": self.action_deleted.uuid}
)
self.app.get(url, user=self.user, status=404)

def test_action_history(self):
response = self.app.get(self.history_url, user=self.user)
Expand All @@ -163,8 +224,14 @@ def test_action_history(self):

def test_action_history_not_your_action(self):
other_user = UserFactory()
response = self.app.get(self.history_url, user=other_user, status=404)
self.app.get(self.history_url, user=other_user, status=404)

def test_action_history_login_required(self):
response = self.app.get(self.history_url)
self.assertRedirects(response, f"{self.login_url}?next={self.history_url}")

def test_action_history_deleted_action(self):
url = reverse(
"accounts:action_history", kwargs={"uuid": self.action_deleted.uuid}
)
self.app.get(url, user=self.user, status=404)
7 changes: 7 additions & 0 deletions src/open_inwoner/accounts/tests/test_profile_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ def setUp(self):
self.return_url = reverse("logout")
self.user = UserFactory()

self.action_deleted = ActionFactory(
name="deleted action, should not show up",
created_by=self.user,
is_deleted=True,
status=StatusChoices.open,
)

def test_login_required(self):
login_url = reverse("login")
response = self.app.get(self.url)
Expand Down
3 changes: 2 additions & 1 deletion src/open_inwoner/accounts/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
MyProfileView,
NecessaryFieldsUserView,
)
from .views.actions import ActionDeleteView

app_name = "accounts"
urlpatterns = [
Expand Down Expand Up @@ -57,7 +58,7 @@
path("actions/<str:uuid>/edit/", ActionUpdateView.as_view(), name="action_edit"),
path(
"actions/<str:uuid>/delete/",
ActionUpdateView.as_view(),
ActionDeleteView.as_view(),
name="action_delete",
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird: there already existed an action_delete that wasn't used/referred.

path(
Expand Down
Loading