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

IA-3732 Handle bulk reviews of change requests (backend) #1957

Open
wants to merge 7 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
35 changes: 35 additions & 0 deletions iaso/api/org_unit_change_requests/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,41 @@ def validate(self, validated_data):
return validated_data


class OrgUnitChangeRequestBulkReviewSerializer(serializers.Serializer):
"""
Bulk-approve or bulk-reject `OrgUnitChangeRequest`s.
"""

# Selection.
select_all = serializers.BooleanField(default=False)
selected_ids = serializers.ListField(child=serializers.IntegerField(min_value=1))
unselected_ids = serializers.ListField(child=serializers.IntegerField(min_value=1))
# Review data.
status = serializers.ChoiceField(choices=OrgUnitChangeRequest.Statuses, default=None)
approved_fields = serializers.MultipleChoiceField(choices=OrgUnitChangeRequest.get_new_fields(), default=None)
rejection_comment = serializers.CharField(required=False, default="")

def validate_status(self, value):
approved = OrgUnitChangeRequest.Statuses.APPROVED
rejected = OrgUnitChangeRequest.Statuses.REJECTED
if value not in [approved, rejected]:
raise serializers.ValidationError(f"Must be `{approved}` or `{rejected}`.")
return value

def validate(self, validated_data):
status = validated_data["status"]
rejection_comment = validated_data["rejection_comment"]
approved_fields = validated_data["approved_fields"]

if status == OrgUnitChangeRequest.Statuses.REJECTED and not rejection_comment:
raise serializers.ValidationError("A `rejection_comment` must be provided.")

if status == OrgUnitChangeRequest.Statuses.APPROVED and not approved_fields:
raise serializers.ValidationError("At least one `approved_fields` must be provided.")

return validated_data


class AuditOrgUnitChangeRequestSerializer(serializers.ModelSerializer):
class Meta:
model = OrgUnitChangeRequest
Expand Down
48 changes: 47 additions & 1 deletion iaso/api/org_unit_change_requests/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,15 @@
OrgUnitChangeRequestRetrieveSerializer,
OrgUnitChangeRequestReviewSerializer,
OrgUnitChangeRequestWriteSerializer,
OrgUnitChangeRequestBulkReviewSerializer,
)
from iaso.api.serializers import AppIdSerializer
from iaso.api.tasks.serializers import TaskSerializer
from iaso.models import OrgUnit, OrgUnitChangeRequest, Instance
from iaso.tasks.org_unit_change_requests_bulk_review import (
org_unit_change_requests_bulk_approve,
org_unit_change_requests_bulk_reject,
)
from iaso.utils.models.common import get_creator_name


Expand All @@ -47,7 +53,7 @@ class OrgUnitChangeRequestViewSet(viewsets.ModelViewSet):
pagination_class = OrgUnitChangeRequestPagination

def get_permissions(self):
if self.action == "partial_update":
if self.action in ["partial_update", "bulk_review"]:
permission_classes = [HasOrgUnitsChangeRequestReviewPermission]
else:
permission_classes = [HasOrgUnitsChangeRequestPermission]
Expand Down Expand Up @@ -102,6 +108,14 @@ def has_org_unit_permission(self, org_unit_to_change: OrgUnit) -> None:
if not org_units_for_user.filter(id=org_unit_to_change.pk).exists():
raise PermissionDenied("The user is trying to create a change request for an unauthorized OrgUnit.")

def list(self, request, *args, **kwargs):
response = super().list(request, *args, **kwargs)
# Allow the front-end to know the total number of change requests with the status "new" that can be used in bulk review.
response.data["select_all_count"] = (
self.filter_queryset(self.get_queryset()).filter(status=OrgUnitChangeRequest.Statuses.NEW).count()
)
return response

def perform_create(self, serializer):
"""
POST to create an `OrgUnitChangeRequest`.
Expand Down Expand Up @@ -145,6 +159,38 @@ def partial_update(self, request, *args, **kwargs):
response_serializer = OrgUnitChangeRequestRetrieveSerializer(change_request)
return Response(response_serializer.data)

@action(detail=False, methods=["patch"])
def bulk_review(self, request):
serializer = OrgUnitChangeRequestBulkReviewSerializer(data=request.data)
serializer.is_valid(raise_exception=True)

selected_ids = serializer.validated_data["selected_ids"]
unselected_ids = serializer.validated_data["unselected_ids"]
status = serializer.validated_data["status"]
approved_fields = serializer.validated_data["approved_fields"]
rejection_comment = serializer.validated_data["rejection_comment"]

queryset = self.filter_queryset(self.get_queryset()).filter(status=OrgUnitChangeRequest.Statuses.NEW)

if selected_ids:
queryset = queryset.filter(pk__in=selected_ids)

if unselected_ids:
queryset = queryset.exclude(pk__in=unselected_ids)

ids = list(queryset.values_list("pk", flat=True))

if status == OrgUnitChangeRequest.Statuses.APPROVED:
task = org_unit_change_requests_bulk_approve(
change_requests_ids=ids, approved_fields=list(approved_fields), user=self.request.user
)
else:
task = org_unit_change_requests_bulk_reject(
change_requests_ids=ids, rejection_comment=rejection_comment, user=self.request.user
)

return Response({"task": TaskSerializer(instance=task).data})

@staticmethod
def org_unit_change_request_csv_columns():
return [
Expand Down
43 changes: 43 additions & 0 deletions iaso/tasks/org_unit_change_requests_bulk_review.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
from django.db import transaction

from beanstalk_worker import task_decorator

from iaso.models import Task, OrgUnitChangeRequest


@task_decorator(task_name="org_unit_change_requests_bulk_approve")
def org_unit_change_requests_bulk_approve(
change_requests_ids: list[int],
approved_fields: list[str],
task: Task,
):
task.report_progress_and_stop_if_killed(progress_message="Synchronizing source versions…")

user = task.launcher

change_requests = OrgUnitChangeRequest.objects.filter(id__in=change_requests_ids)

with transaction.atomic():
for change_request in change_requests:
change_request.approve(user, approved_fields)

task.report_success(message=f"Bulk approved {len(change_requests_ids)} change requests.")


@task_decorator(task_name="org_unit_change_requests_bulk_reject")
def org_unit_change_requests_bulk_reject(
change_requests_ids: list[int],
rejection_comment: str,
task: Task,
):
task.report_progress_and_stop_if_killed(progress_message="Synchronizing source versions…")

user = task.launcher

change_requests = OrgUnitChangeRequest.objects.filter(id__in=change_requests_ids)

with transaction.atomic():
for change_request in change_requests:
change_request.reject(user, rejection_comment)

task.report_success(message=f"Bulk rejected {len(change_requests_ids)} change requests.")
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
import datetime
import io

from iaso.api.org_unit_change_requests.views import OrgUnitChangeRequestViewSet
from iaso.utils.models.common import get_creator_name
import time_machine

from iaso.test import APITestCase
from iaso import models as m
from iaso.api.org_unit_change_requests.views import OrgUnitChangeRequestViewSet
from iaso.tests.tasks.task_api_test_case import TaskAPITestCase
from iaso.utils.models.common import get_creator_name


class OrgUnitChangeRequestAPITestCase(APITestCase):
class OrgUnitChangeRequestAPITestCase(TaskAPITestCase):
"""
Test actions on the ViewSet.
"""
Expand Down Expand Up @@ -74,7 +74,7 @@ def test_list_ok(self):

self.client.force_authenticate(self.user)

with self.assertNumQueries(10):
with self.assertNumQueries(12):
# filter_for_user_and_app_id
# 1. SELECT OrgUnit
# get_queryset
Expand All @@ -88,9 +88,15 @@ def test_list_ok(self):
# 8. PREFETCH OrgUnitChangeRequest.new_reference_instances__form
# 9. PREFETCH OrgUnitChangeRequest.old_reference_instances__form
# 10. PREFETCH OrgUnitChangeRequest.org_unit_type.projects
# extra field `select_all_count` at the same level as `count` for pagination
# 11. COUNT(*) -> `self.get_queryset()` is called 2 times…
# 12. COUNT(status=new)
response = self.client.get("/api/orgunits/changes/")
self.assertJSONResponse(response, 200)
self.assertEqual(2, len(response.data["results"]))

self.assertEqual(2, len(response.data["results"]))
self.assertEqual(2, response.data["count"])
self.assertEqual(2, response.data["select_all_count"])

def test_list_without_auth(self):
response = self.client.get("/api/orgunits/changes/")
Expand Down Expand Up @@ -366,6 +372,101 @@ def test_delete_should_be_forbidden(self):
response = self.client.delete(f"/api/orgunits/changes/{change_request.pk}/", format="json")
self.assertEqual(response.status_code, 405)

def test_bulk_review_without_perm(self):
self.client.force_authenticate(self.user)
response = self.client.patch(f"/api/orgunits/changes/bulk_review/", data={}, format="json")
self.assertEqual(response.status_code, 403)

@time_machine.travel(DT, tick=False)
def test_bulk_review_approve(self):
self.client.force_authenticate(self.user_with_review_perm)

change_request_1 = m.OrgUnitChangeRequest.objects.create(
status=m.OrgUnitChangeRequest.Statuses.NEW, org_unit=self.org_unit, created_by=self.user, new_name="foo"
)
change_request_2 = m.OrgUnitChangeRequest.objects.create(
status=m.OrgUnitChangeRequest.Statuses.NEW, org_unit=self.org_unit, created_by=self.user, new_name="bar"
)

data = {
"select_all": 1,
"selected_ids": [],
"unselected_ids": [],
"status": m.OrgUnitChangeRequest.Statuses.APPROVED,
"approved_fields": ["new_name"],
}
response = self.client.patch(f"/api/orgunits/changes/bulk_review/", data=data, format="json")
self.assertEqual(response.status_code, 200)
data = response.json()

task = self.assertValidTaskAndInDB(data["task"], status="QUEUED", name="org_unit_change_requests_bulk_approve")

self.assertEqual(task.launcher, self.user_with_review_perm)
self.assertCountEqual(task.params["kwargs"]["change_requests_ids"], [change_request_1.pk, change_request_2.pk])
self.assertCountEqual(task.params["kwargs"]["approved_fields"], ["new_name"])

self.runAndValidateTask(task, "SUCCESS")

task.refresh_from_db()
self.assertEqual(task.progress_message, "Bulk approved 2 change requests.")

change_request_1.refresh_from_db()
self.assertEqual(change_request_1.status, m.OrgUnitChangeRequest.Statuses.APPROVED)
self.assertEqual(change_request_1.updated_by, self.user_with_review_perm)

change_request_2.refresh_from_db()
self.assertEqual(change_request_2.status, m.OrgUnitChangeRequest.Statuses.APPROVED)
self.assertEqual(change_request_2.updated_by, self.user_with_review_perm)

@time_machine.travel(DT, tick=False)
def test_bulk_review_reject(self):
self.client.force_authenticate(self.user_with_review_perm)

change_request_1 = m.OrgUnitChangeRequest.objects.create(
status=m.OrgUnitChangeRequest.Statuses.NEW, org_unit=self.org_unit, created_by=self.user, new_name="foo"
)
change_request_2 = m.OrgUnitChangeRequest.objects.create(
status=m.OrgUnitChangeRequest.Statuses.NEW, org_unit=self.org_unit, created_by=self.user, new_name="bar"
)
change_request_3 = m.OrgUnitChangeRequest.objects.create(
status=m.OrgUnitChangeRequest.Statuses.NEW, org_unit=self.org_unit, created_by=self.user, new_name="baz"
)

data = {
"select_all": 0,
"selected_ids": [change_request_1.pk, change_request_2.pk],
"unselected_ids": [change_request_3.pk],
"status": m.OrgUnitChangeRequest.Statuses.REJECTED,
"approved_fields": [],
"rejection_comment": "No way.",
}
response = self.client.patch(f"/api/orgunits/changes/bulk_review/", data=data, format="json")
self.assertEqual(response.status_code, 200)
data = response.json()

task = self.assertValidTaskAndInDB(data["task"], status="QUEUED", name="org_unit_change_requests_bulk_reject")

self.assertEqual(task.launcher, self.user_with_review_perm)
self.assertCountEqual(task.params["kwargs"]["change_requests_ids"], [change_request_1.pk, change_request_2.pk])
self.assertCountEqual(task.params["kwargs"]["rejection_comment"], "No way.")

self.runAndValidateTask(task, "SUCCESS")

task.refresh_from_db()
self.assertEqual(task.progress_message, "Bulk rejected 2 change requests.")

change_request_1.refresh_from_db()
self.assertEqual(change_request_1.status, m.OrgUnitChangeRequest.Statuses.REJECTED)
self.assertEqual(change_request_1.updated_by, self.user_with_review_perm)

change_request_2.refresh_from_db()
self.assertEqual(change_request_2.status, m.OrgUnitChangeRequest.Statuses.REJECTED)
self.assertEqual(change_request_2.updated_by, self.user_with_review_perm)

change_request_3.refresh_from_db()
self.assertEqual(change_request_3.status, m.OrgUnitChangeRequest.Statuses.NEW)
self.assertEqual(change_request_3.updated_by, None)

def test_export_to_csv(self):
"""
It tests the CSV export for the org change requests list.
Expand Down
61 changes: 61 additions & 0 deletions iaso/tests/api/org_unit_change_requests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
OrgUnitForChangeRequestSerializer,
OrgUnitChangeRequestReviewSerializer,
OrgUnitChangeRequestRetrieveSerializer,
OrgUnitChangeRequestBulkReviewSerializer,
)
from iaso.models import OrgUnitChangeRequest
from iaso.models.payments import PaymentStatuses
Expand Down Expand Up @@ -716,3 +717,63 @@ def test_validate(self):
self.assertEqual(
error.exception.detail["non_field_errors"][0], "At least one `approved_fields` must be provided."
)


class OrgUnitChangeRequestBulkReviewSerializerTestCase(TestCase):
"""
Test bulk review serializer.
"""

@classmethod
def setUpTestData(cls):
cls.org_unit = m.OrgUnit.objects.create()
cls.change_request = m.OrgUnitChangeRequest.objects.create(org_unit=cls.org_unit, new_name="Foo")

def test_serialize_ok(self):
data = {
"select_all": 0,
"selected_ids": [1, 2, 315646465465465465464],
"unselected_ids": [],
"status": self.change_request.Statuses.APPROVED,
"approved_fields": ["new_name"],
}
serializer = OrgUnitChangeRequestBulkReviewSerializer(data=data)
self.assertTrue(serializer.is_valid())
self.assertEqual(serializer.validated_data["select_all"], False)
self.assertEqual(serializer.validated_data["selected_ids"], [1, 2, 315646465465465465464])
self.assertEqual(serializer.validated_data["unselected_ids"], [])
self.assertEqual(serializer.validated_data["status"], self.change_request.Statuses.APPROVED)
self.assertEqual(serializer.validated_data["rejection_comment"], "")
self.assertEqual(serializer.validated_data["approved_fields"], {"new_name"})

def test_validate_status(self):
data = {
"select_all": 0,
"status": OrgUnitChangeRequest.Statuses.NEW,
}
serializer = OrgUnitChangeRequestBulkReviewSerializer(data=data)
with self.assertRaises(ValidationError) as error:
serializer.is_valid(raise_exception=True)
self.assertEqual(error.exception.detail["status"][0], "Must be `approved` or `rejected`.")

def test_validate_approve(self):
data = {
"status": OrgUnitChangeRequest.Statuses.APPROVED,
"approved_fields": [],
}
serializer = OrgUnitChangeRequestReviewSerializer(data=data)
with self.assertRaises(ValidationError) as error:
serializer.is_valid(raise_exception=True)
self.assertEqual(
error.exception.detail["non_field_errors"][0], "At least one `approved_fields` must be provided."
)

def test_validate_reject(self):
data = {
"status": OrgUnitChangeRequest.Statuses.REJECTED,
"rejection_comment": " ",
}
serializer = OrgUnitChangeRequestReviewSerializer(data=data)
with self.assertRaises(ValidationError) as error:
serializer.is_valid(raise_exception=True)
self.assertEqual(error.exception.detail["non_field_errors"][0], "A `rejection_comment` must be provided.")
Loading