Skip to content

Commit

Permalink
♻️ [#390] Refactor endpoint to use new permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
SilviaAmAm committed Oct 1, 2024
1 parent ba0f3b7 commit 29c6885
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 28 deletions.
33 changes: 30 additions & 3 deletions backend/src/openarchiefbeheer/accounts/api/serializers.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,49 @@
from django.utils.translation import gettext_lazy as _

from drf_spectacular.utils import extend_schema_field
from rest_framework import serializers

from ..models import User


class RoleSerializer(serializers.Serializer):
can_start_destruction = serializers.BooleanField()
can_review_destruction = serializers.BooleanField()
can_review_final_list = serializers.BooleanField()

class Meta:
fields = (
"name",
"can_start_destruction",
"can_review_destruction",
"can_review_final_list",
"can_view_case_details",
)


class UserSerializer(serializers.ModelSerializer):
role = RoleSerializer()
role = serializers.SerializerMethodField(
help_text=_("The role of the user within the application logic."),
allow_null=True,
)

class Meta:
model = User
fields = ("pk", "username", "first_name", "last_name", "email", "role")

@extend_schema_field(RoleSerializer)
def get_role(self, user: User) -> dict | None:
serializer = RoleSerializer(
data={
"can_review_destruction": user.has_perm(
"accounts.can_review_destruction"
),
"can_start_destruction": user.has_perm(
"accounts.can_start_destruction"
),
"can_review_final_list": user.has_perm(
"accounts.can_review_final_list"
),
}
)
serializer.is_valid()

return serializer.data
21 changes: 20 additions & 1 deletion backend/src/openarchiefbeheer/accounts/managers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
from django.contrib.auth.models import BaseUserManager
from typing import TYPE_CHECKING

from django.contrib.auth.models import BaseUserManager, Permission
from django.db.models import Q, QuerySet

if TYPE_CHECKING:
from .models import User


class UserManager(BaseUserManager):
Expand Down Expand Up @@ -32,3 +38,16 @@ def create_superuser(self, username, email, password, **extra_fields):
raise ValueError("Superuser must have is_superuser=True.")

return self._create_user(username, email, password, **extra_fields)

def _users_with_permission(self, permission: Permission) -> QuerySet["User"]:
return self.filter(
Q(groups__permissions=permission) | Q(user_permissions=permission)
).distinct()

def reviewers(self) -> QuerySet["User"]:
permission = Permission.objects.get(codename="can_review_destruction")
return self._users_with_permission(permission)

def archivists(self) -> QuerySet["User"]:
permission = Permission.objects.get(codename="can_review_final_list")
return self._users_with_permission(permission)
21 changes: 10 additions & 11 deletions backend/src/openarchiefbeheer/destruction/api/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,16 @@ class CanStartDestructionPermission(permissions.BasePermission):
message = _("You are not allowed to create a destruction list.")

def has_permission(self, request, view):
return request.user.role and request.user.role.can_start_destruction
return request.user.has_perm("accounts.can_start_destruction")


class CanReviewPermission(permissions.BasePermission):
message = _("You are not allowed to review a destruction list.")

def has_permission(self, request, view):
return request.user.role and (
request.user.role.can_review_destruction
or request.user.role.can_review_final_list
)
return request.user.has_perm(
"accounts.can_review_destruction"
) or request.user.has_perm("accounts.can_review_final_list")


class CanUpdateDestructionList(permissions.BasePermission):
Expand All @@ -29,7 +28,7 @@ class CanUpdateDestructionList(permissions.BasePermission):
)

def has_permission(self, request, view):
return request.user.role and request.user.role.can_start_destruction
return request.user.has_perm("accounts.can_start_destruction")

def has_object_permission(self, request, view, destruction_list):
return (
Expand All @@ -45,7 +44,7 @@ class CanMarkListAsFinal(permissions.BasePermission):
)

def has_permission(self, request, view):
return request.user.role and request.user.role.can_start_destruction
return request.user.has_perm("accounts.can_start_destruction")

def has_object_permission(self, request, view, destruction_list):
return (
Expand All @@ -61,7 +60,7 @@ class CanTriggerDeletion(permissions.BasePermission):
)

def has_permission(self, request, view):
return request.user.role and request.user.role.can_start_destruction
return request.user.has_perm("accounts.can_start_destruction")

def has_object_permission(self, request, view, destruction_list):
return (
Expand All @@ -74,7 +73,7 @@ class CanReassignDestructionList(permissions.BasePermission):
message = _("You are not allowed to reassign the destruction list.")

def has_permission(self, request, view):
return request.user.role and request.user.role.can_start_destruction
return request.user.has_perm("accounts.can_start_destruction")

def has_object_permission(self, request, view, destruction_list):
return request.user == destruction_list.author and destruction_list.status in [
Expand All @@ -87,7 +86,7 @@ class CanMarkAsReadyToReview(permissions.BasePermission):
message = _("You are not allowed to mark this destruction list as ready to review.")

def has_permission(self, request, view):
return request.user.role and request.user.role.can_start_destruction
return request.user.has_perm("accounts.can_start_destruction")

def has_object_permission(self, request, view, destruction_list):
return (
Expand All @@ -100,7 +99,7 @@ class CanAbortDestruction(permissions.BasePermission):
message = _("You are not allowed to stop the planned destruction of the list.")

def has_permission(self, request, view):
return request.user.role and request.user.role.can_start_destruction
return request.user.has_perm("accounts.can_start_destruction")

def has_object_permission(self, request, view, destruction_list):
return (
Expand Down
14 changes: 7 additions & 7 deletions backend/src/openarchiefbeheer/destruction/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Meta:
fields = ("user",)

def validate_user(self, user: User) -> User:
if not user.role.can_review_destruction:
if not user.has_perm("accounts.can_review_destruction"):
raise ValidationError(
_(
"The chosen user does not have the permission of reviewing a destruction list."
Expand All @@ -68,18 +68,18 @@ def validate(self, attrs: dict) -> dict:
class MarkAsFinalSerializer(serializers.Serializer):
comment = serializers.CharField(allow_blank=True)
user = serializers.PrimaryKeyRelatedField(
queryset=User.objects.all().select_related("role")
queryset=User.objects.all().prefetch_related("user_permissions")
)

def validate_user(self, value: User) -> User:
if not value.role.can_review_final_list:
def validate_user(self, user: User) -> User:
if not user.has_perm("accounts.can_review_final_list"):
raise ValidationError(
_(
"The chosen user does not have the permission to review a final list."
)
)

return value
return user


class DestructionListAssigneeReadSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -352,11 +352,11 @@ def validate(self, attrs: dict) -> dict:
if (
(
destruction_list.status == ListStatus.ready_to_review
and not user.role.can_review_destruction
and not user.has_perm("accounts.can_review_destruction")
)
or (
destruction_list.status == ListStatus.ready_for_archivist
and not user.role.can_review_final_list
and not user.has_perm("accounts.can_review_final_list")
)
or destruction_list.status
not in [ListStatus.ready_to_review, ListStatus.ready_for_archivist]
Expand Down
6 changes: 3 additions & 3 deletions backend/src/openarchiefbeheer/destruction/api/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,12 @@ class DestructionListViewSet(
serializer_class = DestructionListWriteSerializer
queryset = (
DestructionList.objects.all()
.select_related("author", "author__role", "assignee", "assignee__role")
.select_related("author", "assignee")
.prefetch_related(
Prefetch(
"assignees",
queryset=DestructionListAssignee.objects.select_related(
"user", "user__role"
queryset=DestructionListAssignee.objects.prefetch_related(
"user__user_permissions"
).order_by("pk"),
)
)
Expand Down
3 changes: 0 additions & 3 deletions frontend/src/lib/format/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,5 @@ export function formatUser(
? `${user.firstName} ${user.lastName}${userNameSuffix}`
: user.username;

if (showRole && user.role.name) {
return `${displayName} (${user.role.name})`;
}
return displayName;
}

0 comments on commit 29c6885

Please sign in to comment.