Restrict the processing-batch API to ops/admin
Any authenticated user could PATCH a batch, and the write serializer accepted status, so a buyer could move a qc_fail batch to delivered. The buyer portal reads batches through the ORM, not this API, so the whole viewset is closed to ops/admin and the write serializer refuses a hand-set QC status and any transition out of qc_fail.
This commit is contained in:
parent
4b4477d359
commit
88eede5d7a
3 changed files with 129 additions and 9 deletions
|
|
@ -49,8 +49,37 @@ class ProcessingBatchDetailSerializer(serializers.ModelSerializer):
|
||||||
return DocumentSerializer(docs, many=True).data
|
return DocumentSerializer(docs, many=True).data
|
||||||
|
|
||||||
|
|
||||||
|
# Mirrors apps/ops/forms.py: a QC status is set by recording a QC result, not by
|
||||||
|
# hand, and a failed batch is sticky — only a new result moves it. The portal
|
||||||
|
# enforces this in BatchForm; without the same rule here the write serializer is
|
||||||
|
# a way around the screen.
|
||||||
|
QC_SET_STATUSES = {
|
||||||
|
ProcessingBatch.Status.QC_PASS,
|
||||||
|
ProcessingBatch.Status.QC_FAIL,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
class ProcessingBatchWriteSerializer(serializers.ModelSerializer):
|
class ProcessingBatchWriteSerializer(serializers.ModelSerializer):
|
||||||
class Meta:
|
class Meta:
|
||||||
model = ProcessingBatch
|
model = ProcessingBatch
|
||||||
fields = ["id", "batch_reference", "harvest_date", "weight_kg", "grade", "origin_facility", "notes", "status"]
|
fields = ["id", "batch_reference", "harvest_date", "weight_kg", "grade", "origin_facility", "notes", "status"]
|
||||||
read_only_fields = ["id"]
|
read_only_fields = ["id"]
|
||||||
|
|
||||||
|
def validate_status(self, status):
|
||||||
|
instance = self.instance
|
||||||
|
unchanged = instance is not None and status == instance.status
|
||||||
|
if status in QC_SET_STATUSES and not unchanged:
|
||||||
|
raise serializers.ValidationError(
|
||||||
|
f"Batch status {ProcessingBatch.Status(status).label} is set by the "
|
||||||
|
"QC result, not by hand."
|
||||||
|
)
|
||||||
|
if (
|
||||||
|
instance is not None
|
||||||
|
and instance.status == ProcessingBatch.Status.QC_FAIL
|
||||||
|
and status != ProcessingBatch.Status.QC_FAIL
|
||||||
|
):
|
||||||
|
raise serializers.ValidationError(
|
||||||
|
"A batch that has failed QC cannot be moved by hand; a new QC "
|
||||||
|
"result is what moves it."
|
||||||
|
)
|
||||||
|
return status
|
||||||
|
|
|
||||||
92
apps/batches/tests.py
Normal file
92
apps/batches/tests.py
Normal file
|
|
@ -0,0 +1,92 @@
|
||||||
|
"""The processing-batch API.
|
||||||
|
|
||||||
|
The portal screens have their own tests; these pin the API boundary that used to
|
||||||
|
be missing. ``ProcessingBatchViewSet`` was open to any authenticated user and the
|
||||||
|
write serializer accepted ``status``, so a buyer could PATCH a failed batch to
|
||||||
|
``delivered``. The write rules mirror ``apps/ops/forms.py``: a QC status is set by
|
||||||
|
a QC result, and a failed batch is sticky.
|
||||||
|
"""
|
||||||
|
from django.contrib.auth import get_user_model
|
||||||
|
from django.test import TestCase
|
||||||
|
from rest_framework.test import APIClient
|
||||||
|
|
||||||
|
from .models import ProcessingBatch
|
||||||
|
|
||||||
|
User = get_user_model()
|
||||||
|
|
||||||
|
|
||||||
|
class ProcessingBatchAPITests(TestCase):
|
||||||
|
def setUp(self):
|
||||||
|
self.ops = User.objects.create_user(
|
||||||
|
email="batch-api-ops@ixg.local", password="pass1234", role="ops",
|
||||||
|
full_name="Batch API Ops", is_staff=True,
|
||||||
|
)
|
||||||
|
self.buyer = User.objects.create_user(
|
||||||
|
email="batch-api-buyer@ixg.local", password="pass1234", role="buyer",
|
||||||
|
full_name="Batch API Buyer", is_staff=True,
|
||||||
|
)
|
||||||
|
self.failed = ProcessingBatch.objects.create(
|
||||||
|
batch_reference="IXG-BATCH-API-001",
|
||||||
|
status=ProcessingBatch.Status.QC_FAIL,
|
||||||
|
)
|
||||||
|
self.client = APIClient()
|
||||||
|
|
||||||
|
def test_a_buyer_cannot_patch_a_batch(self):
|
||||||
|
self.client.force_authenticate(user=self.buyer)
|
||||||
|
response = self.client.patch(
|
||||||
|
f"/api/batches/{self.failed.id}/", {"status": "delivered"}, format="json"
|
||||||
|
)
|
||||||
|
self.assertEqual(response.status_code, 403)
|
||||||
|
self.failed.refresh_from_db()
|
||||||
|
self.assertEqual(self.failed.status, ProcessingBatch.Status.QC_FAIL)
|
||||||
|
|
||||||
|
def test_a_buyer_cannot_read_the_batch_api(self):
|
||||||
|
"""The buyer portal reads batches through the ORM, not this API, so the
|
||||||
|
API needs no buyer read and is closed to one."""
|
||||||
|
self.client.force_authenticate(user=self.buyer)
|
||||||
|
self.assertEqual(self.client.get("/api/batches/").status_code, 403)
|
||||||
|
|
||||||
|
def test_ops_can_read_the_batch_api(self):
|
||||||
|
self.client.force_authenticate(user=self.ops)
|
||||||
|
self.assertEqual(self.client.get("/api/batches/").status_code, 200)
|
||||||
|
|
||||||
|
def test_a_qc_status_cannot_be_hand_set(self):
|
||||||
|
self.client.force_authenticate(user=self.ops)
|
||||||
|
journey = ProcessingBatch.objects.create(
|
||||||
|
batch_reference="IXG-BATCH-API-002",
|
||||||
|
status=ProcessingBatch.Status.HARVESTED,
|
||||||
|
)
|
||||||
|
response = self.client.patch(
|
||||||
|
f"/api/batches/{journey.id}/", {"status": "qc_pass"}, format="json"
|
||||||
|
)
|
||||||
|
self.assertEqual(response.status_code, 400)
|
||||||
|
journey.refresh_from_db()
|
||||||
|
self.assertEqual(journey.status, ProcessingBatch.Status.HARVESTED)
|
||||||
|
|
||||||
|
def test_a_batch_cannot_be_created_with_a_qc_status(self):
|
||||||
|
self.client.force_authenticate(user=self.ops)
|
||||||
|
response = self.client.post(
|
||||||
|
"/api/batches/",
|
||||||
|
{"batch_reference": "IXG-BATCH-API-003", "status": "qc_pass"},
|
||||||
|
format="json",
|
||||||
|
)
|
||||||
|
self.assertEqual(response.status_code, 400)
|
||||||
|
self.assertFalse(
|
||||||
|
ProcessingBatch.objects.filter(batch_reference="IXG-BATCH-API-003").exists()
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_a_failed_batch_cannot_be_moved_out_of_qc_fail(self):
|
||||||
|
self.client.force_authenticate(user=self.ops)
|
||||||
|
response = self.client.patch(
|
||||||
|
f"/api/batches/{self.failed.id}/", {"status": "delivered"}, format="json"
|
||||||
|
)
|
||||||
|
self.assertEqual(response.status_code, 400)
|
||||||
|
self.failed.refresh_from_db()
|
||||||
|
self.assertEqual(self.failed.status, ProcessingBatch.Status.QC_FAIL)
|
||||||
|
|
||||||
|
def test_a_failed_batch_can_be_saved_with_its_own_status(self):
|
||||||
|
self.client.force_authenticate(user=self.ops)
|
||||||
|
response = self.client.patch(
|
||||||
|
f"/api/batches/{self.failed.id}/", {"status": "qc_fail"}, format="json"
|
||||||
|
)
|
||||||
|
self.assertEqual(response.status_code, 200)
|
||||||
|
|
@ -1,12 +1,18 @@
|
||||||
from rest_framework import viewsets, permissions, filters
|
from rest_framework import viewsets, filters
|
||||||
from django_filters.rest_framework import DjangoFilterBackend
|
from django_filters.rest_framework import DjangoFilterBackend
|
||||||
|
|
||||||
|
from apps.compliance.permissions import IsOpsOrAdmin
|
||||||
|
|
||||||
from .models import ProcessingBatch
|
from .models import ProcessingBatch
|
||||||
from .serializers import ProcessingBatchListSerializer, ProcessingBatchDetailSerializer, ProcessingBatchWriteSerializer
|
from .serializers import ProcessingBatchListSerializer, ProcessingBatchDetailSerializer, ProcessingBatchWriteSerializer
|
||||||
|
|
||||||
|
|
||||||
class ProcessingBatchViewSet(viewsets.ModelViewSet):
|
class ProcessingBatchViewSet(viewsets.ModelViewSet):
|
||||||
queryset = ProcessingBatch.objects.prefetch_related("qc_results", "shipments", "documents").all()
|
queryset = ProcessingBatch.objects.prefetch_related("qc_results", "shipments", "documents").all()
|
||||||
permission_classes = [permissions.IsAuthenticated]
|
# The buyer portal reads batches through the ORM (apps/buyer/views.py), not
|
||||||
|
# this API, so nothing outside ops/admin needs it — including for reads. That
|
||||||
|
# closes the hole where a buyer could PATCH a batch's status.
|
||||||
|
permission_classes = [IsOpsOrAdmin]
|
||||||
filter_backends = [DjangoFilterBackend, filters.SearchFilter, filters.OrderingFilter]
|
filter_backends = [DjangoFilterBackend, filters.SearchFilter, filters.OrderingFilter]
|
||||||
filterset_fields = ["status", "grade"]
|
filterset_fields = ["status", "grade"]
|
||||||
search_fields = ["batch_reference"]
|
search_fields = ["batch_reference"]
|
||||||
|
|
@ -22,10 +28,3 @@ class ProcessingBatchViewSet(viewsets.ModelViewSet):
|
||||||
|
|
||||||
def perform_create(self, serializer):
|
def perform_create(self, serializer):
|
||||||
serializer.save(created_by=self.request.user)
|
serializer.save(created_by=self.request.user)
|
||||||
|
|
||||||
def get_queryset(self):
|
|
||||||
user = self.request.user
|
|
||||||
qs = super().get_queryset()
|
|
||||||
if user.role == "buyer":
|
|
||||||
return qs # Buyers can read all batches per spec
|
|
||||||
return qs
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue