add dry permissions for facility cover_image (#1003)

* add dry permissions for facility cover_image

* fix read_cover_image_url returning incomplete url

* refactor facility cover image endpoint

care/facility/api/serializers/facility.py

@@ -1,17 +1,18 @@
-from distutils import extension
 import boto3
-
-from django.contrib.auth import get_user_model
 from django.conf import settings
+from django.contrib.auth import get_user_model
 from rest_framework import serializers
 
-from care.facility.api.serializers.facility_capacity import FacilityCapacitySerializer
 from care.facility.models import FACILITY_TYPES, Facility, FacilityLocalGovtBody
 from care.facility.models.facility import FEATURE_CHOICES
-from care.facility.models.patient_base import reverse_choices
-from care.users.api.serializers.lsg import DistrictSerializer, LocalBodySerializer, StateSerializer, WardSerializer
-from config.serializers import ChoiceField
+from care.users.api.serializers.lsg import (
+    DistrictSerializer,
+    LocalBodySerializer,
+    StateSerializer,
+    WardSerializer,
+)
 from care.utils.csp import config as cs_provider
+from config.serializers import ChoiceField
 
 User = get_user_model()
 
@@ -47,7 +48,10 @@ class FacilityBasicInfoSerializer(serializers.ModelSerializer):
     features = serializers.MultipleChoiceField(choices=FEATURE_CHOICES)
 
     def get_facility_type(self, facility):
-        return {"id": facility.facility_type, "name": facility.get_facility_type_display()}
+        return {
+            "id": facility.facility_type,
+            "name": facility.get_facility_type_display(),
+        }
 
     class Meta:
         model = Facility
@@ -75,7 +79,7 @@ class FacilitySerializer(FacilityBasicInfoSerializer):
     #     "latitude": 49.8782482189424,
     #     "longitude": 24.452545489
     # }
-    read_cover_image_url = serializers.CharField(read_only=True)
+    read_cover_image_url = serializers.URLField(read_only=True)
     # location = PointField(required=False)
     features = serializers.MultipleChoiceField(choices=FEATURE_CHOICES)
 
@@ -121,25 +125,26 @@ def create(self, validated_data):
 
 class FacilityImageUploadSerializer(serializers.ModelSerializer):
     cover_image = serializers.ImageField(required=True, write_only=True)
+    read_cover_image_url = serializers.URLField(read_only=True)
 
     class Meta:
         model = Facility
-        fields = ("cover_image",)
+        # Check DRYpermissions before updating
+        fields = ("cover_image", "read_cover_image_url")
 
     def save(self, **kwargs):
         facility = self.instance
         image = self.validated_data["cover_image"]
-        image_extension = image.name.split(".")[-1]
+        image_extension = image.name.rsplit(".", 1)[-1]
         s3 = boto3.client(
-            "s3",
-            **cs_provider.get_client_config(cs_provider.BucketType.FACILITY.value)
+            "s3", **cs_provider.get_client_config(cs_provider.BucketType.FACILITY.value)
         )
-        upload_response = s3.put_object(
+        image_location = f"cover_images/{facility.external_id}_cover.{image_extension}"
+        s3.put_object(
             Bucket=settings.FACILITY_S3_BUCKET,
-            Key=f"cover_images/{facility.external_id}_cover.{image_extension}",
+            Key=image_location,
             Body=image.file,
         )
-        print(upload_response['ResponseMetadata']['HTTPHeaders']['location'])
-        facility.cover_image_url = f"cover_images/{facility.external_id}_cover.{image_extension}"
+        facility.cover_image_url = image_location
         facility.save()
         return facility

care/facility/api/viewsets/facility.py

@@ -5,21 +5,21 @@
 from rest_framework import filters as drf_filters
 from rest_framework import mixins, status, viewsets
 from rest_framework.decorators import action
+from rest_framework.parsers import MultiPartParser
 from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
 
 from care.facility.api.serializers.facility import (
     FacilityBasicInfoSerializer,
     FacilityImageUploadSerializer,
     FacilitySerializer,
-    FacilityImageUploadSerializer
 )
 from care.facility.models import (
     Facility,
     FacilityCapacity,
     FacilityPatientStatsHistory,
     HospitalDoctors,
-    PatientRegistration
+    PatientRegistration,
 )
 from care.users.models import User
 
@@ -85,9 +85,21 @@ class FacilityViewSet(
     FACILITY_DOCTORS_CSV_KEY = "doctors"
     FACILITY_TRIAGE_CSV_KEY = "triage"
 
+    def initialize_request(self, request, *args, **kwargs):
+        self.action = self.action_map.get(request.method.lower())
+        return super().initialize_request(request, *args, **kwargs)
+
+    def get_parsers(self):
+        if self.action == "cover_image":
+            return [MultiPartParser()]
+        return super().get_parsers()
+
     def get_serializer_class(self):
         if self.request.query_params.get("all") == "true":
             return FacilityBasicInfoSerializer
+        if self.action == "cover_image":
+            # Check DRYpermissions before updating
+            return FacilityImageUploadSerializer
         else:
             return FacilitySerializer
 
@@ -129,24 +141,20 @@ def list(self, request, *args, **kwargs):
 
         return super(FacilityViewSet, self).list(request, *args, **kwargs)
 
-    @action(methods=["POST", "DELETE"], detail=True, permission_classes=[IsAuthenticated, DRYPermissions])
+    @action(methods=["POST"], detail=True)
     def cover_image(self, request, external_id):
         facility = self.get_object()
-        if not facility:
-            return Response({"facility": "does not exist"}, status=status.HTTP_404_NOT_FOUND)
-
-        if request.method == "POST":
-            serialized_data = FacilityImageUploadSerializer(facility, data=request.data)
-            if serialized_data.is_valid():
-                serialized_data.save()
-                return Response(serialized_data.data)
-
-            return Response(serialized_data.errors, status=status.HTTP_400_BAD_REQUEST)
-
-        if request.method == "DELETE":
-            facility.cover_image_url = None
-            facility.save()
-            return Response(status=status.HTTP_204_NO_CONTENT)
+        serializer = FacilityImageUploadSerializer(facility, data=request.data)
+        serializer.is_valid(raise_exception=True)
+        serializer.save()
+        return Response(serializer.data)
+
+    @cover_image.mapping.delete
+    def cover_image_delete(self, *args, **kwargs):
+        facility = self.get_object()
+        facility.cover_image_url = None
+        facility.save()
+        return Response(status=status.HTTP_204_NO_CONTENT)
 
 
 class AllFacilityViewSet(

care/facility/models/facility.py

@@ -168,7 +168,9 @@ class Meta:
         verbose_name_plural = "Facilities"
 
     def read_cover_image_url(self):
-        return settings.FACILITY_S3_STATIC_PREFIX + (self.cover_image_url or "")
+        if self.cover_image_url:
+            return settings.FACILITY_S3_STATIC_PREFIX + self.cover_image_url
+        return None
 
     def __str__(self):
         return f"{self.name}"

care/facility/models/mixins/permissions/facility.py

@@ -9,23 +9,27 @@ def has_bulk_upsert_permission(request):
 
     @staticmethod
     def has_write_permission(request):
-        from care.users.models import State, District, LocalBody
+        from care.users.models import District, LocalBody, State
+
         try:
             state = State.objects.get(id=request.data["state"])
-            district = District.objects.get(id=request.data["district"]) 
+            district = District.objects.get(id=request.data["district"])
             local_body = LocalBody.objects.get(id=request.data["local_body"])
             return (
-                request.user.is_superuser or (
+                request.user.is_superuser
+                or (
                     request.user.user_type <= User.TYPE_VALUE_MAP["LocalBodyAdmin"]
                     and state == request.user.state
                     and district == request.user.district
                     and local_body == request.user.local_body
-                ) or (
+                )
+                or (
                     request.user.user_type > User.TYPE_VALUE_MAP["LocalBodyAdmin"]
                     and request.user.user_type <= User.TYPE_VALUE_MAP["DistrictAdmin"]
                     and state == request.user.state
                     and district == request.user.district
-                ) or (
+                )
+                or (
                     request.user.user_type > User.TYPE_VALUE_MAP["DistrictAdmin"]
                     and request.user.user_type <= User.TYPE_VALUE_MAP["StateAdmin"]
                     and state == request.user.state
@@ -34,6 +38,11 @@ def has_write_permission(request):
         except Exception:
             return False
 
+    @staticmethod
+    def has_cover_image_permission(request):
+        # Returning true here as the permission is validated at object level for this action
+        return True
+
     def has_object_read_permission(self, request):
         return (
             (request.user.is_superuser)
@@ -62,11 +71,16 @@ def has_object_write_permission(self, request):
         return self.has_object_read_permission(request)
 
     def has_object_update_permission(self, request):
-        return super().has_object_update_permission(request) or self.has_object_write_permission(request)
+        return super().has_object_update_permission(
+            request
+        ) or self.has_object_write_permission(request)
 
     def has_object_destroy_permission(self, request):
         return self.has_object_read_permission(request)
 
+    def has_object_cover_image_permission(self, request):
+        return self.has_object_update_permission(request)
+
 
 class FacilityRelatedPermissionMixin(BasePermissionMixin):
     @staticmethod
@@ -82,7 +96,9 @@ def has_write_permission(request):
 
         facility = False
         try:
-            facility = Facility.objects.get(external_id=request.parser_context["kwargs"]["facility_external_id"])
+            facility = Facility.objects.get(
+                external_id=request.parser_context["kwargs"]["facility_external_id"]
+            )
         except Facility.DoesNotExist:
             return False
         return (request.user.is_superuser or request.user.verified) and (