Merge pull request #2543 from IFRCGo/project/localunit-feedbacks

szabozoltan69 · web-flow · commit a56093d3a75b · 2025-08-28T13:40:09.000+02:00
Project/localunit feedbacks
diff --git a/deploy/helm/ifrcgo-helm/values.yaml b/deploy/helm/ifrcgo-helm/values.yaml
@@ -272,9 +272,8 @@ cronjobs:
     schedule: '0 0 * * 0'
   - command: 'ingest_icrc'
     schedule: '0 3 * * 0'
-  # NOTE: Disabling local unit email notification for now 
-  # - command: 'notify_validators'
-  #   schedule: '0 0 * * *'
+  - command: 'notify_validators'
+    schedule: '0 0 * * *'
   # https://github.com/jazzband/django-oauth-toolkit/blob/master/docs/management_commands.rst#cleartokens
   - command: 'oauth_cleartokens'
     schedule: '0 1 * * *'
diff --git a/local_units/bulk_upload.py b/local_units/bulk_upload.py
@@ -6,6 +6,7 @@
 
 from django.core.files.base import ContentFile
 from django.db import transaction
+from rest_framework.exceptions import ErrorDetail
 from rest_framework.serializers import Serializer
 
 from local_units.models import HealthData, LocalUnit, LocalUnitBulkUpload, LocalUnitType
@@ -27,16 +28,69 @@ class BulkUploadError(Exception):
 class ErrorWriter:
     def __init__(self, fieldnames: list[str]):
         self._fieldnames = ["upload_status"] + fieldnames
+        self._rows: list[dict[str, str]] = []
         self._output = io.StringIO()
         self._writer = csv.DictWriter(self._output, fieldnames=self._fieldnames)
         self._writer.writeheader()
+        self._has_errors = False
+
+    def _format_errors(self, errors: dict) -> dict[str, list[str]]:
+        """Format serializer errors into field_name and list of messages."""
+        error = {}
+        for key, value in errors.items():
+            if isinstance(value, dict):
+                for inner_key, inner_value in self._format_errors(value).items():
+                    error[inner_key] = inner_value
+            elif isinstance(value, list):
+                error[key] = [self._clean_message(v) for v in value]
+            else:
+                error[key] = [self._clean_message(value)]
+        return error
+
+    def _clean_message(self, msg: Any) -> str:
+        """Convert ErrorDetail or other objects into normal text."""
+        if isinstance(msg, ErrorDetail):
+            return str(msg)
+        return str(msg)
+
+    def _update_csv_header_with_errors(self):
+        """Update the CSV with updated headers when new error columns are introduced."""
+        self._output.seek(0)
+        self._output.truncate()
+        self._writer = csv.DictWriter(self._output, fieldnames=self._fieldnames)
+        self._writer.writeheader()
+        for row in self._rows:
+            self._writer.writerow(row)
 
     def write(
-        self, row: dict[str, str], status: Literal[LocalUnitBulkUpload.Status.SUCCESS, LocalUnitBulkUpload.Status.FAILED]
+        self,
+        row: dict[str, str],
+        status: Literal[LocalUnitBulkUpload.Status.SUCCESS, LocalUnitBulkUpload.Status.FAILED],
+        error_detail: dict | None = None,
     ) -> None:
-        self._writer.writerow({"upload_status": status.name, **row})
-        if status == LocalUnitBulkUpload.Status.FAILED:
-            self._has_errors = True
+        row_copy = {col: row.get(col, "") for col in self._fieldnames}
+        row_copy["upload_status"] = status.name
+        added_error_column = False
+
+        if status == LocalUnitBulkUpload.Status.FAILED and error_detail:
+            formatted_errors = self._format_errors(error_detail)
+            for field, messages in formatted_errors.items():
+                error_col = f"{field}_error"
+
+                if error_col not in self._fieldnames:
+                    if field in self._fieldnames:
+                        col_idx = self._fieldnames.index(field)
+                        self._fieldnames.insert(col_idx + 1, error_col)
+                    else:
+                        self._fieldnames.append(error_col)
+
+                    added_error_column = True
+                row_copy[error_col] = "; ".join(messages)
+        self._rows.append(row_copy)
+        if added_error_column:
+            self._update_csv_header_with_errors()
+        else:
+            self._writer.writerow(row_copy)
 
     def to_content_file(self) -> ContentFile:
         return ContentFile(self._output.getvalue().encode("utf-8"))
@@ -70,17 +124,17 @@ def process_row(self, data: Dict[str, Any]) -> bool:
         if serializer.is_valid():
             self.bulk_manager.add(LocalUnit(**serializer.validated_data))
             return True
+        self.error_detail = serializer.errors
         return False
 
     def run(self) -> None:
         with self.bulk_upload.file.open("rb") as csv_file:
             file = io.TextIOWrapper(csv_file, encoding="utf-8")
             csv_reader = csv.DictReader(file)
             fieldnames = csv_reader.fieldnames or []
-
             try:
                 self._validate_csv(fieldnames)
-            except ValueError as e:
+            except BulkUploadError as e:
                 self.bulk_upload.status = LocalUnitBulkUpload.Status.FAILED
                 self.bulk_upload.error_message = str(e)
                 self.bulk_upload.save(update_fields=["status", "error_message"])
@@ -99,7 +153,9 @@ def run(self) -> None:
                             self.error_writer.write(row_data, status=LocalUnitBulkUpload.Status.SUCCESS)
                         else:
                             self.failed_count += 1
-                            self.error_writer.write(row_data, status=LocalUnitBulkUpload.Status.FAILED)
+                            self.error_writer.write(
+                                row_data, status=LocalUnitBulkUpload.Status.FAILED, error_detail=self.error_detail
+                            )
                             logger.warning(f"[BulkUpload:{self.bulk_upload.pk}] Row '{row_index}' failed")
 
                     if self.failed_count > 0:
@@ -175,7 +231,7 @@ def _validate_csv(self, fieldnames) -> None:
             raise ValueError("Invalid local unit type")
 
         if present_health_fields and local_unit_type.name.strip().lower() != "health care":
-            raise ValueError(f"Health data are not allowed for this type: {local_unit_type.name}.")
+            raise BulkUploadError(f"Health data are not allowed for this type: {local_unit_type.name}.")
 
 
 class BulkUploadHealthData(BaseBulkUpload[LocalUnitUploadContext]):
diff --git a/local_units/dev_views.py b/local_units/dev_views.py
@@ -9,19 +9,65 @@ class LocalUnitsEmailPreview(APIView):
 
     def get(self, request):
         type_param = request.GET.get("type")
-        param_types = {"new", "update", "validate", "revert", "deprecate", "regional_validator", "global_validator"}
+        param_types = {
+            "new",
+            "update",
+            "validate",
+            "revert",
+            "deprecate",
+            "regional_validator",
+            "global_validator",
+        }
 
         if type_param not in param_types:
-            return HttpResponse(f"Invalid type parameter. Please use one of the following values:  {', '.join(param_types)}.")
+            return HttpResponse(f"Invalid 'type' parameter. Please use one of the following values:  {', '.join(param_types)}.")
 
         context_mapping = {
-            "new": {"new_local_unit": True, "validator_email": "Test Validator", "full_name": "Test User"},
-            "update": {"update_local_unit": True, "validator_email": "Test Validator", "full_name": "Test User"},
-            "validate": {"validate_success": True, "full_name": "Test User"},
-            "revert": {"revert_reason": "Test Reason", "full_name": "Test User"},
-            "deprecate": {"deprecate_local_unit": True, "deprecate_reason": "Test Deprecate Reason", "full_name": "Test User"},
-            "regional_validator": {"is_validator_regional_admin": True, "full_name": "Regional User"},
-            "global_validator": {"is_validator_global_admin": True, "full_name": "Global User"},
+            "new": {
+                "new_local_unit": True,
+                "validator_email": "Test Validator",
+                "full_name": "Test User",
+                "country": "Test Country",
+            },
+            "update": {
+                "update_local_unit": True,
+                "update_reason_overview": "Test update reason",
+                "validator_email": "Test Validator",
+                "full_name": "Test User",
+                "country": "Test Country",
+                "country_id": 1,
+            },
+            "validate": {
+                "validate_success": True,
+                "full_name": "Test User",
+                "country": "Test Country",
+                "country_id": 1,
+            },
+            "revert": {
+                "revert_reason": "Test Reason",
+                "full_name": "Test User",
+                "country": "Test Country",
+                "country_id": 1,
+            },
+            "deprecate": {
+                "deprecate_local_unit": True,
+                "deprecate_reason": "Test Deprecate Reason",
+                "full_name": "Test User",
+                "country": "Test Country",
+                "country_id": 1,
+            },
+            "regional_validator": {
+                "is_validator_regional_admin": True,
+                "full_name": "Regional User",
+                "country": "Test Country",
+                "country_id": 1,
+            },
+            "global_validator": {
+                "is_validator_global_admin": True,
+                "full_name": "Global User",
+                "country": "Test Country",
+                "country_id": 1,
+            },
         }
 
         context = context_mapping.get(type_param)
diff --git a/local_units/management/commands/notify_validators.py b/local_units/management/commands/notify_validators.py
@@ -1,5 +1,6 @@
 from datetime import timedelta
 
+from django.conf import settings
 from django.core.management.base import BaseCommand
 from django.template.loader import render_to_string
 from django.utils import timezone
@@ -21,21 +22,28 @@ class Command(BaseCommand):
     @monitor(monitor_slug=SentryMonitor.NOTIFY_VALIDATORS)
     def handle(self, *args, **options):
         self.stdout.write(self.style.NOTICE("Notifying the validators..."))
-
+        # NOTE: In production use standard email notification time(7days/14days),shorter delays(1day/2days) elsewhere for testing
+        production = settings.GO_ENVIRONMENT == "production"
+        if production:
+            regional_email_notification_days = 7
+            global_email_notification_days = 14
+        else:
+            regional_email_notification_days = 1
+            global_email_notification_days = 2
         # Regional Validators: 14 days
         queryset_for_regional_validators = LocalUnit.objects.filter(
             status__in=[LocalUnit.Status.UNVALIDATED, LocalUnit.Status.PENDING_EDIT_VALIDATION],
             is_deprecated=False,
             last_sent_validator_type=Validator.LOCAL,
-            created_at__lte=timezone.now() - timedelta(days=7),
+            created_at__lte=timezone.now() - timedelta(days=regional_email_notification_days),
         )
 
         # Global Validators: 28 days
         queryset_for_global_validators = LocalUnit.objects.filter(
             status__in=[LocalUnit.Status.UNVALIDATED, LocalUnit.Status.PENDING_EDIT_VALIDATION],
             is_deprecated=False,
             last_sent_validator_type=Validator.REGIONAL,
-            created_at__lte=timezone.now() - timedelta(days=14),
+            created_at__lte=timezone.now() - timedelta(days=global_email_notification_days),
         )
 
         for local_unit in queryset_for_regional_validators:
diff --git a/local_units/serializers.py b/local_units/serializers.py
@@ -446,6 +446,7 @@ class PrivateLocalUnitSerializer(serializers.ModelSerializer):
     status_details = serializers.CharField(source="get_status_display", read_only=True)
     modified_by_details = LocalUnitMiniUserSerializer(source="modified_by", read_only=True)
     is_locked = serializers.BooleanField(read_only=True)
+    update_reason_overview = serializers.CharField(read_only=True)
 
     class Meta:
         model = LocalUnit
@@ -473,6 +474,7 @@ class Meta:
             "is_locked",
             "is_new_local_unit",
             "bulk_upload",
+            "update_reason_overview",
         )
 
     def get_location_geojson(self, unit) -> dict:
@@ -697,6 +699,8 @@ class Meta:
     def validate_file(self, file):
         if not file.name.endswith(".csv"):
             raise serializers.ValidationError(gettext("File must be a CSV file."))
+        if file.size > 10 * 1024 * 1024:
+            raise serializers.ValidationError(gettext("File must be less than 10 MB."))
         return file
 
     def get_file_name(self, obj):
@@ -903,7 +907,7 @@ def validate_level(self, value):
         level_name = value.strip().lower()
         level_id = self.level_map.get(level_name)
         if not level_id:
-            raise serializers.ValidationError({"Level": gettext("Level '{value}' is not valid")})
+            raise serializers.ValidationError(gettext("Level is not valid"))
         return level_id
 
     def validate(self, validated_data):
diff --git a/local_units/tasks.py b/local_units/tasks.py
@@ -45,6 +45,9 @@ def send_local_unit_email(local_unit_id: int, new: bool = True):
 
     for user in users:
         # NOTE: Adding the validator email to the context
+        if not user.email:
+            logger.warning(f"Email address not found for validator: {user.get_full_name()}.")
+            return None
         email_context["validator_email"] = user.email
         email_context["full_name"] = user.get_full_name()
         email_body = render_to_string("email/local_units/local_unit.html", email_context)
@@ -58,6 +61,14 @@ def send_validate_success_email(local_unit_id: int, message: str = ""):
 
     instance = LocalUnit.objects.get(id=local_unit_id)
     user = instance.created_by
+    if not user:
+        logger.warning(f"Email not sent for Local Unit:{local_unit_id} because creator is unknown.")
+        return None
+    elif not user.email:
+        logger.warning(
+            f"Email not sent for Local Unit:{local_unit_id} because creator: {user.get_full_name()} has no email address."
+        )
+        return None
     email_context = get_email_context(instance)
     email_context["full_name"] = user.get_full_name()
     email_context["validate_success"] = True
@@ -76,6 +87,14 @@ def send_revert_email(local_unit_id: int, change_request_id: int):
     instance = LocalUnit.objects.get(id=local_unit_id)
     change_request_instance = LocalUnitChangeRequest.objects.get(id=change_request_id)
     user = instance.created_by
+    if not user:
+        logger.warning(f"Email not sent for Local Unit:{local_unit_id} because creator is unknown.")
+        return None
+    elif not user.email:
+        logger.warning(
+            f"Email not sent for Local Unit:{local_unit_id} because creator: {user.get_full_name()} has no email address."
+        )
+        return None
     email_context = get_email_context(instance)
     email_context["full_name"] = user.get_full_name()
     email_context["revert_reason"] = change_request_instance.rejected_reason
@@ -94,6 +113,15 @@ def send_deprecate_email(local_unit_id: int):
 
     instance = LocalUnit.objects.get(id=local_unit_id)
     user = instance.created_by
+    if not user:
+        logger.warning(f"Email not sent for Local Unit:{local_unit_id} because creator is unknown.")
+        return None
+    elif not user.email:
+        logger.warning(
+            f"Email not sent for Local Unit:{local_unit_id} because creator: {user.get_full_name()} has no email address."
+        )
+        return None
+
     email_context = get_email_context(instance)
     email_context["full_name"] = user.get_full_name()
     email_context["deprecate_local_unit"] = True
@@ -110,7 +138,7 @@ def process_bulk_upload_local_unit(bulk_upload_id: int) -> None:
     bulk_upload: LocalUnitBulkUpload | None = LocalUnitBulkUpload.objects.filter(id=bulk_upload_id).first()
 
     if not bulk_upload:
-        logger.error(f"BulkUploadLocalUnit:'{bulk_upload_id}' Not found.", exc_info=True)
+        logger.warning(f"BulkUploadLocalUnit:'{bulk_upload_id}' Not found.", exc_info=True)
         return
     try:
         if (
@@ -121,6 +149,6 @@ def process_bulk_upload_local_unit(bulk_upload_id: int) -> None:
             BaseBulkUploadLocalUnit(bulk_upload).run()
 
     except Exception as exc:
-        logger.error(f"BulkUploadLocalUnit:'{bulk_upload_id}' Failed with exception: {exc}", exc_info=True)
+        logger.warning(f"BulkUploadLocalUnit:'{bulk_upload_id}' Failed with exception: {exc}", exc_info=True)
         bulk_upload.update_status(LocalUnitBulkUpload.Status.FAILED)
         raise exc
diff --git a/local_units/utils.py b/local_units/utils.py
@@ -14,7 +14,10 @@ def get_email_context(instance):
     email_context = {
         "id": local_unit_data["id"],
         "local_branch_name": local_unit_data["local_branch_name"],
-        "frontend_url": settings.FRONTEND_URL,
+        "country": local_unit_data["country_details"]["name"],
+        "country_id": local_unit_data["country_details"]["id"],
+        "update_reason_overview": local_unit_data["update_reason_overview"],
+        "frontend_url": settings.GO_WEB_URL,
     }
     return email_context
 
diff --git a/local_units/views.py b/local_units/views.py
@@ -401,6 +401,7 @@ class ExternallyManagedLocalUnitViewSet(
 class LocalUnitBulkUploadViewSet(
     mixins.CreateModelMixin,
     mixins.ListModelMixin,
+    mixins.RetrieveModelMixin,
     viewsets.GenericViewSet,
 ):
     queryset = LocalUnitBulkUpload.objects.select_related("country", "local_unit_type", "triggered_by").order_by("-triggered_at")
diff --git a/main/sentry.py b/main/sentry.py
@@ -128,8 +128,7 @@ class SentryMonitor(models.TextChoices):
     INGEST_NS_DOCUMENT = "ingest_ns_document", "0 0 * * 0"
     INGEST_NS_INITIATIVES = "ingest_ns_initiatives", "0 0 * * 0"
     INGEST_ICRC = "ingest_icrc", "0 3 * * 0"
-    # NOTE: Disabling local unit email notification for now
-    # NOTIFY_VALIDATORS = "notify_validators", "0 0 * * *"
+    NOTIFY_VALIDATORS = "notify_validators", "0 0 * * *"
     OAUTH_CLEARTOKENS = "oauth_cleartokens", "0 1 * * *"
 
     @staticmethod
diff --git a/notifications/templates/design/foot1.html b/notifications/templates/design/foot1.html
@@ -8,11 +8,11 @@
                       <table width="100%" border="0" cellspacing="0" cellpadding="0">
                         <tr>
                           <td class="text-default td-smaller-font-style text-right pl-10 pr-10" style="color: #000000;font-family:'Lato', Arial, sans-serif;border-right:1px solid #000;font-size: 16px;text-align: right;padding-left: 10px;padding-right: 10px;">
-                            <a href="https://go.ifrc.org/login" target="_blank" class="link" style="font-family: 'Lato', Arial, sans-serif;text-decoration: underline;color: #000;">Login to IFRC GO</a>
+                            <a href="{{ frontend_url }}/login" target="_blank" class="link" style="font-family: 'Lato', Arial, sans-serif;text-decoration: underline;color: #000;">Login to IFRC GO</a>
                           </td>
 {% if not hide_preferences %}
                           <td class="text-default td-smaller-font-style pl-10 pr-10" style="color: #000000;font-family:'Lato', Arial, sans-serif;font-size: 16px;padding-left: 10px;padding-right: 10px;">
-                            <a href="https://go.ifrc.org/account#notifications" target="_blank" class="link" style="font-family: 'Lato', Arial, sans-serif;text-decoration: underline;color: #000;">Unsubscribe</a>
+                            <a href="{{ frontend_url }}/account#notifications" target="_blank" class="link" style="font-family: 'Lato', Arial, sans-serif;text-decoration: underline;color: #000;">Unsubscribe</a>
                           </td>
 {% endif %}
                         </tr>
diff --git a/notifications/templates/design/foot2.html b/notifications/templates/design/foot2.html
diff --git a/notifications/templates/email/local_units/local_unit.html b/notifications/templates/email/local_units/local_unit.html
