Merge pull request #1606 from IFRCGo/feature/optimistic-lock-op-update

Add optimistic lock in dref-op-update

Author: szabozoltan69

dref/models.py

@@ -689,7 +689,11 @@ def clone(self, user):
 
 class DrefOperationalUpdate(models.Model):
     created_at = models.DateTimeField(verbose_name=_('created at'), auto_now_add=True)
-    modified_at = models.DateTimeField(verbose_name=_('modified at'), auto_now=True)
+    modified_at = models.DateTimeField(
+        verbose_name=_('modified at'),
+        auto_now=True,
+        blank=True
+    )
     created_by = models.ForeignKey(
         settings.AUTH_USER_MODEL,
         verbose_name=_('created by'),

dref/serializers.py

@@ -3,6 +3,7 @@
 
 from django.utils.translation import gettext
 from django.db import models, transaction
+from django.conf import settings
 
 from rest_framework import serializers
 
@@ -184,7 +185,6 @@ class DrefSerializer(
     ALLOWED_ASSESSMENT_REPORT_EXTENSIONS = ["pdf", "docx", "pptx"]
     MAX_OPERATION_TIMEFRAME = 30
     ASSESSMENT_REPORT_MAX_OPERATION_TIMEFRAME = 2
-    DREF_UPDATE_ERROR_MESSAGE = "OBSOLETE_PAYLOAD"
     national_society_actions = NationalSocietyActionSerializer(many=True, required=False)
     needs_identified = IdentifiedNeedSerializer(many=True, required=False)
     planned_interventions = PlannedInterventionSerializer(many=True, required=False)
@@ -349,7 +349,7 @@ def update(self, instance, validated_data):
         is_assessment_report = validated_data.get('is_assessment_report')
         modified_at = validated_data.pop('modified_at', None)
         if modified_at is None:
-            raise serializers.ValidationError({ 'modified_at': 'Modified At is required!' })
+            raise serializers.ValidationError({'modified_at': 'Modified At is required!'})
         if is_assessment_report:
             # Previous Operations
             validated_data['lessons_learned'] = None
@@ -383,7 +383,7 @@ def update(self, instance, validated_data):
         else:
             to = None
         if modified_at and instance.modified_at and modified_at < instance.modified_at:
-            raise serializers.ValidationError({ 'modified_at': self.DREF_UPDATE_ERROR_MESSAGE })
+            raise serializers.ValidationError({'modified_at': settings.DREF_OP_UPDATE_UPDATE_ERROR_MESSAGE})
         dref = super().update(instance, validated_data)
         if to:
             transaction.on_commit(
@@ -414,6 +414,7 @@ class DrefOperationalUpdateSerializer(
     district_details = MiniDistrictSerializer(source='district', read_only=True, many=True)
     assessment_report_file = DrefFileSerializer(source='assessment_report', required=False, allow_null=True)
     risk_security = RiskSecuritySerializer(many=True, required=False)
+    modified_at = serializers.DateTimeField(required=False)
 
     class Meta:
         model = DrefOperationalUpdate
@@ -656,6 +657,9 @@ def update(self, instance, validated_data):
         dref_target_population_of_operation = validated_data.get('dref', instance.dref).total_targeted_population
         dref_amount_requested = validated_data.get('dref', instance.dref).amount_requested
         dref_district = validated_data.get('dref', instance.dref).district.all()
+        modified_at = validated_data.pop('modified_at', None)
+        if modified_at is None:
+            raise serializers.ValidationError({'modified_at': 'Modified At is required!'})
 
         if (not changing_timeframe_operation) and total_operation_timeframe and dref_operation_timeframe and\
         total_operation_timeframe != dref_operation_timeframe:
@@ -682,6 +686,8 @@ def update(self, instance, validated_data):
         if changing_geographic_location and district and dref_district and set(district) == set(dref_district):
             raise serializers.ValidationError('Found same district for dref and operational update with changing set to true')
 
+        if modified_at and instance.modified_at and modified_at < instance.modified_at:
+            raise serializers.ValidationError({'modified_at': settings.DREF_OP_UPDATE_UPDATE_ERROR_MESSAGE})
         return super().update(instance, validated_data)
 
 

dref/test_views.py

@@ -620,6 +620,7 @@ def test_dref_operational_update_patch(self):
             'new_operational_end_date': '2022-10-10',
             'reporting_timeframe': '2022-10-16',
             'is_timeframe_extension_required': True,
+            'modified_at': datetime.now() + timedelta(days=12),
         }
         url = f'/api/v2/dref-op-update/{response_id}/'
         self.authenticate(user=user1)
@@ -920,3 +921,42 @@ def test_dref_latest_update(self):
         self.assertEqual(response.status_code, 200)
         # Title should be latest since modified_at is greater than modified_at in database
         self.assertEqual(response.data['title'], "New title")
+
+    def test_dref_op_update_locking(self):
+        user1, _ = UserFactory.create_batch(2)
+        dref = DrefFactory.create(
+            title='Test Title', created_by=user1,
+            is_published=True,
+        )
+        dref.users.add(user1)
+        DrefOperationalUpdateFactory.create(
+            dref=dref,
+            is_published=True,
+            operational_update_number=1,
+            modified_at=datetime.now()
+        )
+        url = '/api/v2/dref-op-update/'
+        data = {
+            'dref': dref.id,
+        }
+        self.authenticate(user=user1)
+        response = self.client.post(url, data=data)
+        self.assert_201(response)
+        response_id = response.data['id']
+
+        # without `modified_at`
+        data = {
+            'title': 'New Operation title',
+            'new_operational_end_date': '2022-10-10',
+            'reporting_timeframe': '2022-10-16',
+            'is_timeframe_extension_required': True,
+        }
+        url = f'/api/v2/dref-op-update/{response_id}/'
+        self.authenticate(user=user1)
+        response = self.client.patch(url, data)
+        self.assert_400(response)
+
+        # with `modified_at` less than instance `modified_at`
+        data['modified_at'] = datetime.now() - timedelta(days=2)
+        response = self.client.patch(url, data=data)
+        self.assertEqual(response.status_code, 400)

main/settings.py

@@ -467,3 +467,6 @@
 
 # MISC
 FRONTEND_URL = env('FRONTEND_URL')
+
+
+DREF_OP_UPDATE_UPDATE_ERROR_MESSAGE = "OBSOLETE_PAYLOAD"