fix: redis lock handling and update translation check method

Author: sudip-khanal

dref/test_views.py

@@ -11,6 +11,7 @@
 from rest_framework import status
 
 from api.models import Country, DisasterType, District, Region, RegionName
+from api.utils import get_model_name
 from deployments.factories.project import SectorFactory
 from deployments.factories.user import UserFactory
 from dref.factories.dref import (
@@ -999,9 +1000,9 @@ def test_dref_create_and_update_in_local_language(
         response = self.client.patch(url, data=data_en, format="json", HTTP_ACCEPT_LANGUAGE="en")
         self.assert_400(response)
 
-    @patch("dref.views.trigger_translation")
+    @patch("dref.views.translate_model_fields.delay")
     @patch("dref.views.is_translation_complete")
-    def test_update_and_finalize_dref(self, mock_is_translation_complete, mock_trigger_translation):
+    def test_update_and_finalize_dref(self, mock_is_translation_complete, mock_translation):
         dref = DrefFactory.create(
             title="Título original en español",
             type_of_dref=Dref.DrefType.IMMINENT,
@@ -1035,13 +1036,13 @@ def test_update_and_finalize_dref(self, mock_is_translation_complete, mock_trigg
         mock_is_translation_complete.return_value = False
         response = self.client.post(finalize_url)
         self.assert_400(response)
-        mock_trigger_translation.assert_called_once_with(dref)
+        mock_translation.assert_called_once_with(get_model_name(type(dref)), dref.pk)
         # Test finalize with Translation completion
-        mock_trigger_translation.reset_mock()
+        mock_translation.reset_mock()
         mock_is_translation_complete.return_value = True
         response = self.client.post(finalize_url)
         self.assert_200(response)
-        mock_trigger_translation.assert_not_called()
+        mock_translation.assert_not_called()
         self.assertEqual(response.data["status"], Dref.Status.FINALIZED)
         self.assertEqual(response.data["translation_module_original_language"], "en")
 
@@ -1186,9 +1187,9 @@ def test_create_and_update_operational_update(self):
         self.assert_200(response)
         self.assertEqual(response.data["title"], "Titre en français")
 
-    @patch("dref.views.trigger_translation")
+    @patch("dref.views.translate_model_fields.delay")
     @patch("dref.views.is_translation_complete")
-    def test_dref_operational_update_finalize(self, mock_is_translation_complete, mock_trigger_translation):
+    def test_dref_operational_update_finalize(self, mock_is_translation_complete, mock_translation):
         # Create users
         user1, user2 = UserFactory.create_batch(2)
         dref = DrefFactory.create(
@@ -1226,13 +1227,13 @@ def test_dref_operational_update_finalize(self, mock_is_translation_complete, mo
         mock_is_translation_complete.return_value = False
         response = self.client.post(finalize_url)
         self.assert_400(response)
-        mock_trigger_translation.assert_called_once_with(op_update)
+        mock_translation.assert_called_once_with(get_model_name(type(op_update)), op_update.pk)
         # Test Finalize with translation complete
-        mock_trigger_translation.reset_mock()
+        mock_translation.reset_mock()
         mock_is_translation_complete.return_value = True
         response = self.client.post(finalize_url)
         self.assert_200(response)
-        mock_trigger_translation.assert_not_called()
+        mock_translation.assert_not_called()
         self.assertEqual(response.data["status"], Dref.Status.FINALIZED)
         self.assertEqual(response.data["translation_module_original_language"], "en")
         # Update in English
@@ -2245,9 +2246,9 @@ def test_create_and_update_final_report(self):
         self.assertEqual(response.data["translation_module_original_language"], "es")
         self.assertEqual(response.data["title"], "Título en español")
 
-    @patch("dref.views.trigger_translation")
+    @patch("dref.views.translate_model_fields.delay")
     @patch("dref.views.is_translation_complete")
-    def test_dref_final_report_finalize(self, mock_is_translation_complete, mock_trigger_translation):
+    def test_dref_final_report_finalize(self, mock_is_translation_complete, mock_translation):
         region = Region.objects.create(name=RegionName.AFRICA)
         country = Country.objects.create(name="Test country12", region=region)
         # Create users
@@ -2288,13 +2289,13 @@ def test_dref_final_report_finalize(self, mock_is_translation_complete, mock_tri
         mock_is_translation_complete.return_value = False
         response = self.client.post(finalize_url)
         self.assert_400(response)
-        mock_trigger_translation.assert_called_once_with(final_report)
+        mock_translation.assert_called_once_with(get_model_name(type(final_report)), final_report.pk)
         # Test finalize with Translation completion
-        mock_trigger_translation.reset_mock()
+        mock_translation.reset_mock()
         mock_is_translation_complete.return_value = True
         response = self.client.post(finalize_url)
         self.assert_200(response)
-        mock_trigger_translation.assert_not_called()
+        mock_translation.assert_not_called()
         self.assertEqual(response.data["status"], Dref.Status.FINALIZED)
         self.assertEqual(response.data["translation_module_original_language"], "en")
 

dref/utils.py

@@ -7,8 +7,6 @@
 from modeltranslation.utils import build_localized_fieldname
 
 from dref.models import Dref, DrefFinalReport, DrefOperationalUpdate
-from lang.tasks import translate_model_fields
-from main.lock import RedisLockKey, redis_lock
 
 logger = logging.getLogger(__name__)
 
@@ -63,34 +61,92 @@ def get_dref_users():
     return dref_users_list
 
 
-def is_translation_complete(instance, target_lang="en"):
+def is_translation_complete(instance, target_lang="en", visited=None):
     """
-    Check all translatable fields of a instance have been
-    translated to the target language.
+    Checks if instance and all related translatable fields are complete.
     """
-    original_lang = getattr(instance, "translation_module_original_language", None)
-    if not original_lang:
+    if visited is None:
+        visited = set()
+
+    instance_id = id(instance)
+    if instance_id in visited:
+        return True
+
+    visited.add(instance_id)
+
+    if not _is_instance_translatable(instance):
+        return False
+
+    if not _check_instance_fields(instance, target_lang):
         return False
+    if not _check_related_fields(instance, target_lang, visited):
+        return False
+    return True
+
+
+def _is_instance_translatable(instance):
+    """Check if instance has translation capability."""
+    return bool(getattr(instance, "translation_module_original_language", None))
+
+
+def _check_instance_fields(instance, target_lang):
+    """Check translatable fields on the instance itself."""
     try:
         opts = translator.get_options_for_model(type(instance))
     except Exception as e:
-        logger.warning(f"Failed to get translation options {e}", exc_info=True)
+        logger.warning(f"Failed to get translation options: {e}")
         return False
-    for field in getattr(opts, "fields", []):
-        original_value = getattr(instance, build_localized_fieldname(field, original_lang), None)
-        translated_value = getattr(instance, build_localized_fieldname(field, target_lang), None)
+
+    original_lang = getattr(instance, "translation_module_original_language")
+    translatable_fields = getattr(opts, "fields", [])
+
+    for field in translatable_fields:
+        original_field = build_localized_fieldname(field, original_lang)
+        target_field = build_localized_fieldname(field, target_lang)
+
+        original_value = getattr(instance, original_field, None)
+        translated_value = getattr(instance, target_field, None)
+
         if original_value and not translated_value:
             return False
+
     return True
 
 
-def trigger_translation(instance):
-    """
-    Trigger translation task with Redis lock.
-    """
-    with redis_lock(key=RedisLockKey.DREF_TRANSLATION, id=instance.pk) as acquired:
-        if not acquired:
-            logger.warning(f"Translation already in progress for {instance._meta.label} (pk={instance.pk})")
-            return
-        translate_model_fields.delay(instance._meta.label, instance.pk)
-        logger.info(f"Triggered translation task for {instance._meta.label} (pk={instance.pk})")
+def _check_related_fields(instance, target_lang, visited):
+    """Check all related translatable fields."""
+    try:
+        opts = translator.get_options_for_model(type(instance))
+    except Exception as e:
+        logger.warning(f"Failed to get translation options for related fields: {e}")
+        return False
+
+    for related_field in getattr(opts, "related_fields", []):
+        if not _check_related_field_translation(instance, related_field, target_lang, visited):
+            return False
+
+    return True
+
+
+def _check_related_field_translation(instance, related_field, target_lang, visited):
+    try:
+        related_instance = getattr(instance, related_field.name, None)
+
+        if related_instance is None:
+            return True
+
+        if hasattr(related_instance, "translation_module_original_language"):
+            return is_translation_complete(related_instance, target_lang, visited)
+
+        elif hasattr(related_instance, "all"):
+            for related_obj in related_instance.all():
+                if hasattr(related_obj, "translation_module_original_language"):
+                    if not is_translation_complete(related_obj, target_lang, visited):
+                        return False
+            return True
+
+    except Exception as e:
+        logger.warning(f"Error checking related field {related_field.name}: {e}")
+        return False
+
+    return True

dref/views.py

@@ -17,6 +17,7 @@
 from rest_framework.exceptions import NotFound
 from reversion.views import RevisionMixin
 
+from api.utils import get_model_name
 from dref.filter_set import (
     ActiveDrefFilterSet,
     CompletedDrefOperationsFilterSet,
@@ -41,7 +42,8 @@
     DrefShareUserSerializer,
     MiniDrefSerializer,
 )
-from dref.utils import is_translation_complete, trigger_translation
+from dref.utils import is_translation_complete
+from lang.tasks import translate_model_fields
 from main.permissions import DenyGuestUserPermission, UseBySuperAdminOnly
 
 
@@ -110,7 +112,7 @@ def finalize(self, request, pk=None, version=None):
         if dref.status in [Dref.Status.FINALIZED, Dref.Status.APPROVED]:
             raise serializers.ValidationError(gettext("Cannot be finalized because it is already %s") % dref.get_status_display())
         if not is_translation_complete(dref):
-            trigger_translation(dref)
+            translate_model_fields.delay(get_model_name(type(dref)), dref.pk)
             raise serializers.ValidationError(
                 gettext("The translation is currently being processed. Please wait a little while before trying again.")
             )
@@ -205,7 +207,7 @@ def finalize(self, request, pk=None, version=None):
                 gettext("Cannot be finalized because it is already %s") % operational_update.get_status_display()
             )
         if not is_translation_complete(operational_update):
-            trigger_translation(operational_update)
+            translate_model_fields.delay(get_model_name(type(operational_update)), operational_update.pk)
             raise serializers.ValidationError(
                 gettext("The translation is currently being processed. Please wait a little while before trying again.")
             )
@@ -270,7 +272,7 @@ def finalize(self, request, pk=None, version=None):
                 gettext("Cannot be finalized because it is already %s") % field_report.get_status_display()
             )
         if not is_translation_complete(field_report):
-            trigger_translation(field_report)
+            translate_model_fields.delay(get_model_name(type(field_report)), field_report.pk)
             raise serializers.ValidationError(
                 gettext("The translation is currently being processed. Please wait a little while before trying again.")
             )

lang/tasks.py

@@ -13,6 +13,7 @@
 from modeltranslation.utils import build_localized_fieldname
 
 from main.celery import Queues
+from main.lock import RedisLockKey, redis_lock
 from main.translation import (
     TRANSLATOR_ORIGINAL_LANGUAGE_FIELD_NAME,
     TRANSLATOR_SKIP_FIELD_NAME,
@@ -203,7 +204,13 @@ def translate_remaining_models_fields():
 def translate_model_fields(model_name, pk):
     model = django_apps.get_model(model_name)
     obj = model.objects.get(pk=pk)
-    ModelTranslator().translate_model_fields(obj)
+
+    with redis_lock(key=RedisLockKey.MODEL_TRANSLATION, id=pk, model_name=model_name) as acquired:
+        if not acquired:
+            logger.warning(f"Translation is already in progress for {model_name} with pk={pk}.")
+            return
+        ModelTranslator().translate_model_fields(obj)
+        logger.info(f"Translation success for {model_name} with pk={pk}.")
 
 
 @shared_task(queue=Queues.HEAVY)

main/lock.py

@@ -19,19 +19,23 @@ class RedisLockKey(models.TextChoices):
 
     OPERATION_LEARNING_SUMMARY = _BASE + "-operation-learning-summary-{0}"
     OPERATION_LEARNING_SUMMARY_EXPORT = _BASE + "-operation-learning-summary-export-{0}"
-    DREF_TRANSLATION = _BASE + "-dref-translation-{0}"
+    MODEL_TRANSLATION = _BASE + "-{model_name}-translation-{id}"
 
 
 @contextmanager
 def redis_lock(
     key: RedisLockKey,
     id: typing.Union[int, str],
+    model_name: typing.Optional[str] = None,
     lock_expire: int = settings.REDIS_DEFAULT_LOCK_EXPIRE,
 ):
     """
     Locking mechanism using Redis
     """
-    lock_id = key.format(id)
+    if model_name and id:
+        lock_id = key.format(model_name=model_name, id=id)
+    else:
+        lock_id = key.format(id)
     timeout_at = time.monotonic() + lock_expire - 3
     # cache.add fails if the key already exists
     status = cache.add(lock_id, 1, lock_expire)