Merge pull request #185 from mapswipe/fix/validation-project

Author: tnagorra

apps/common/admin.py

@@ -195,6 +195,6 @@ def save_model(self, request, obj, form, change):  # type: ignore[reportMissingP
             previous_announcements = Announcement.objects.exclude(id=obj.id)
             previous_announcements.update(is_active=False)
 
-            FirebaseAnnouncementPush(obj).trigger()
+            FirebaseAnnouncementPush(obj).trigger(force_update=True)
         else:
             FirebaseAnnouncementPush(obj).trigger(delete=True)

apps/common/firebase/base.py

@@ -59,7 +59,12 @@ def handle_object_update_on_firebase(self, model_obj: T, fb_obj: K, fb_reference
     @abc.abstractmethod
     def get_firebase_path(self, firebase_id: str, model: type[T]) -> str: ...
 
-    def trigger(self, *, delete: bool | None = None) -> None:
+    def trigger(
+        self,
+        *,
+        delete: bool | None = None,
+        force_update: bool | None = None,
+    ) -> None:
         model_obj = self.obj
         model_obj.update_firebase_push_status(FirebasePushStatusEnum.PENDING)
 
@@ -73,7 +78,7 @@ def trigger(self, *, delete: bool | None = None) -> None:
                 return
             self._delete(model_obj)
         else:
-            self._push(model_obj)
+            self._push(model_obj, force_update=force_update)
 
     def _delete(self, model_obj: T) -> None:
         if model_obj.firebase_push_status_enum != FirebasePushStatusEnum.PENDING:
@@ -106,7 +111,12 @@ def _delete(self, model_obj: T) -> None:
             model_obj.firebase_push_status = None
             model_obj.save(update_fields=["firebase_last_pushed", "firebase_push_status"])
 
-    def _push(self, model_obj: T) -> None:
+    def _push(
+        self,
+        model_obj: T,
+        *,
+        force_update: bool | None = None,
+    ) -> None:
         if model_obj.firebase_push_status_enum != FirebasePushStatusEnum.PENDING:
             logger.warning(
                 "Firebase push error: push is not required for %s",
@@ -124,13 +134,14 @@ def _push(self, model_obj: T) -> None:
             fb_model: typing.Any = model_ref.get()
 
             if not model_obj.firebase_last_pushed:
-                if fb_model is not None:
+                if not force_update and fb_model is not None:
                     logger.error(
                         "Firebase create error: %s already exists in Firebase",
                         model_obj._meta.label,
                         extra={"id": model_obj.pk},
                     )
                     raise InvalidObjectPushException
+
                 self.handle_new_object_on_firebase(model_obj, model_ref)
             else:
                 if fb_model is None:

apps/common/tests/common_test.py

@@ -5,6 +5,7 @@
 
 from apps.common.models import Announcement
 from apps.user.models import User
+from main.config import Config
 from main.tests import TestCase
 
 
@@ -14,18 +15,26 @@ class MockRequest(typing.NamedTuple):
 
 class TestAnnouncement(TestCase):
     @typing.override
-    def setUp(self):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
         # Create a superuser for admin login
         User = get_user_model()
-        self.admin_user = User.objects.create_superuser(  # type: ignore[reportAttributeAccessIssue]
+        cls.admin_user = User.objects.create_superuser(  # type: ignore[reportAttributeAccessIssue]
             email="[email protected]",
             password="password123",  # noqa: S106
         )
-        self.client.login(email="[email protected]", password="password123")  # noqa: S106
 
     def test_create_announcement(self):
+        request = MockRequest(user=self.admin_user)
+        self.force_login(request.user)
+
         url = reverse("admin:common_announcement_add")
 
+        announcement_ref = self.firebase_helper.ref(
+            Config.FirebaseKeys.announcement(),
+        )
+
         data = {
             "client_id": "01K44YMVYTKY1R3906XW3QQK05",
             "text": "We have a new release v1.2.3",
@@ -43,6 +52,10 @@ def test_create_announcement(self):
         assert announcement_1.is_active
         assert announcement_1.url == "https://play.google.com/store/apps/details?id=org.missingmaps.mapswipe"
 
+        firebase_announcement: typing.Any = announcement_ref.get()
+        assert firebase_announcement is not None
+        assert firebase_announcement.get("url") == "https://play.google.com/store/apps/details?id=org.missingmaps.mapswipe"
+
         # test active only one announcement at once
         data = {
             "client_id": "01K44ZF3KMS6GV2AD93EG6WP9X",
@@ -61,9 +74,32 @@ def test_create_announcement(self):
         assert announcement_2.is_active
         assert announcement_2.url == "https://mapswipe.org/en/blogs/2025-04-03-papua-new-guinea-swiping-to-find-airstrips"
 
+        firebase_announcement: typing.Any = announcement_ref.get()
+        assert firebase_announcement is not None
+        assert (
+            firebase_announcement.get("url")
+            == "https://mapswipe.org/en/blogs/2025-04-03-papua-new-guinea-swiping-to-find-airstrips"
+        )
+
         # check only one active announcement
         assert Announcement.objects.filter(is_active=True).count() == 1
-        announcement_1.refresh_from_db()
 
-        # check if other announcements are inactive
+        # check if current announcement is active
+        announcement_1.refresh_from_db()
         assert not announcement_1.is_active
+
+        # test announcement de-activation
+        url = reverse("admin:common_announcement_change", args=[announcement_2.pk])
+        data = {
+            "client_id": "01K44ZF3KMS6GV2AD93EG6WP9X",
+            "text": "Checkout the latest blog post about airstrips",
+            "is_active": False,
+            "url": "https://mapswipe.org/en/blogs/2025-04-03-papua-new-guinea-swiping-to-find-airstrips",
+            "created_by": self.admin_user.id,
+            "modified_by": self.admin_user.id,
+        }
+        response = self.client.post(url, data, follow=True)
+        assert response.status_code == 200
+
+        firebase_announcement: typing.Any = announcement_ref.get()
+        assert firebase_announcement is None

apps/common/views.py

@@ -14,6 +14,7 @@
 from rest_framework.views import APIView
 
 from apps.contributor.models import ContributorUser
+from main.config import Config
 
 from .serializers import FirebaseAuthRequestSerializer
 
@@ -29,7 +30,7 @@
 
 # FIXME: Maybe a better approach then this?
 def _get_version_from_pyproject(base_path: Path) -> str:
-    data = toml.load(settings.BASE_DIR / base_path / "pyproject.toml")
+    data = toml.load(Config.BASE_DIR / base_path / "pyproject.toml")
     return data["project"]["version"]
 
 
@@ -46,7 +47,7 @@ def render_to_response_json(
             **json.loads(response.content),
             "app": {
                 "environment": settings.APP_ENVIRONMENT,
-                "version": _get_version_from_pyproject(settings.BASE_DIR),
+                "version": _get_version_from_pyproject(Config.BASE_DIR),
                 "git": {
                     "branch": git_helper.branch,
                     "commit": git_helper.commit_sha,

apps/contributor/management/commands/create_contributor_users.py

@@ -2,7 +2,6 @@
 import typing
 import uuid
 
-from django.conf import settings
 from django.core.management.base import BaseCommand
 from django.utils import timezone
 from pyfirebase_mapswipe import extended_models as firebase_extended_models
@@ -62,7 +61,7 @@ class Command(BaseCommand):
 
     @typing.override
     def handle(self, *args, **options):  # type: ignore[reportMissingParameterType]
-        if not settings.ENABLE_DANGER_MODE:
+        if not Config.ENABLE_DANGER_MODE:
             logger.warning("Dummy data generation is disabled")
             return
 

apps/contributor/tests/e2e_usergroup_test.py

@@ -3,11 +3,11 @@
 from pathlib import Path
 
 import json5
-from django.conf import settings
 
 from apps.common.utils import remove_object_keys
 from apps.contributor.factories import ContributorUserFactory
 from apps.user.factories import UserFactory
+from main.config import Config
 from main.tests import TestCase
 
 
@@ -83,7 +83,7 @@ def setUpClass(cls):
     def test_usergroup_e2e(self):
         self.force_login(self.user)
 
-        data_file = Path(settings.BASE_DIR, "assets/tests/usergroup/data.json5")
+        data_file = Path(Config.BASE_DIR, "assets/tests/usergroup/data.json5")
         with data_file.open("r", encoding="utf-8") as f:
             test_data_list = json5.load(f)
 

apps/project/exports/exports.py

@@ -6,18 +6,19 @@
 from django.db import transaction
 from ulid import ULID
 
-from apps.common.models import AssetTypeEnum
+from apps.common.models import AssetTypeEnum, FirebasePushStatusEnum
 from apps.project.custom_options import get_fallback_custom_options_for_export
 from apps.project.exports.geojson import gzipped_csv_to_gzipped_geojson
 from apps.project.models import Project, ProjectAsset, ProjectAssetExportTypeEnum, ProjectProgressStatusEnum, ProjectTypeEnum
-from apps.project.tasks import send_slack_message_for_project
+from apps.project.tasks import push_project_to_firebase, send_slack_message_for_project
 from apps.user.models import User
 from main.config import Config
 from main.logging import log_extra
 from project_types.store import get_project_type_handler
 from project_types.tile_map_service.compare.project import CompareProjectProperty
 from project_types.tile_map_service.completeness.project import CompletenessProjectProperty
 from project_types.tile_map_service.find.project import FindProjectProperty
+from utils.geo.raster_tile_server.config import RasterTileServerNameEnum
 
 from .mapping_results import generate_mapping_results
 from .mapping_results_aggregate.task import generate_mapping_results_aggregate_by_task
@@ -56,8 +57,32 @@ def _export_project_data(project: Project, tmp_directory: Path):
     # legacy system path: /api/hot_tm/hot_tm_{project.id}.geojson
     tmp_tasking_manager_hot_tm_geojson = tmp_directory / f"hot_tm_{project.id}.geojson"
 
-    # TODO: if maxar is used for tile_server_name, this should be true
-    add_metadata = False
+    # FIXME(tnagorra): move this to project handler
+    tile_servers = set[RasterTileServerNameEnum]()
+    if isinstance(
+        project_type_handler.project_type_specifics,
+        FindProjectProperty,
+    ):
+        tile_servers.add(project_type_handler.project_type_specifics.tile_server_property.name)
+    elif isinstance(
+        project_type_handler.project_type_specifics,
+        CompareProjectProperty,
+    ):
+        tile_servers.add(project_type_handler.project_type_specifics.tile_server_property.name)
+        tile_servers.add(project_type_handler.project_type_specifics.tile_server_b_property.name)
+    elif isinstance(
+        project_type_handler.project_type_specifics,
+        CompletenessProjectProperty,
+    ):
+        tile_servers.add(project_type_handler.project_type_specifics.tile_server_property.name)
+        if project_type_handler.project_type_specifics.overlay_tile_server_property.raster:
+            tile_servers.add(
+                project_type_handler.project_type_specifics.overlay_tile_server_property.raster.tile_server.name,
+            )
+
+    add_metadata = (
+        RasterTileServerNameEnum.MAXAR_STANDARD in tile_servers or RasterTileServerNameEnum.MAXAR_PREMIUM in tile_servers
+    )
 
     custom_options_raw = []
 
@@ -136,6 +161,7 @@ def _export_project_data(project: Project, tmp_directory: Path):
         tmp_project_stats_by_date_csv.name,
     )
 
+    # FIXME(tnagorra): move this to project handler
     generate_hot_tm_geometries = project.project_type_enum in [
         ProjectTypeEnum.COMPARE,
         ProjectTypeEnum.COMPLETENESS,
@@ -151,14 +177,17 @@ def _export_project_data(project: Project, tmp_directory: Path):
         )
 
     if not project_stats_by_date_df.empty:
-        project.progress = project_stats_by_date_df["cum_progress"].iloc[-1] * 100
-        if project.progress >= 100:
-            project.progress_status = ProjectProgressStatusEnum.COMPLETED
         project.number_of_contributor_users = project_stats_by_date_df["cum_number_of_users"].iloc[-1]
         project.number_of_results = project_stats_by_date_df["cum_number_of_results"].iloc[-1]
         project.number_of_results_for_progress = project_stats_by_date_df["cum_number_of_results_progress"].iloc[-1]
         project.last_contribution_date = project_stats_by_date_df.index[-1]
-        # TODO: Trigger slack notifications on progress change
+
+        previous_progress = project.progress
+        project.progress = project_stats_by_date_df["cum_progress"].iloc[-1] * 100
+
+        if project.progress >= 100:
+            project.progress_status = ProjectProgressStatusEnum.COMPLETED
+
         if project.progress >= 90 and project.slack_progress_notifications < 90:
             transaction.on_commit(
                 lambda: send_slack_message_for_project.delay(project_id=project.id, action="progress-change"),
@@ -169,6 +198,13 @@ def _export_project_data(project: Project, tmp_directory: Path):
                 lambda: send_slack_message_for_project.delay(project_id=project.id, action="progress-change"),
             )
 
+        if project.progress != previous_progress:
+            # FIXME(tnagorra): Do we only send updates for the 2 fields?
+            transaction.on_commit(
+                lambda: push_project_to_firebase.delay(project_id=project.id),
+            )
+            project.update_firebase_push_status(FirebasePushStatusEnum.PENDING, False)
+
     project.save(
         update_fields=(
             "progress",
@@ -177,6 +213,8 @@ def _export_project_data(project: Project, tmp_directory: Path):
             "number_of_results",
             "number_of_results_for_progress",
             "last_contribution_date",
+            "firebase_push_status",
+            "firebase_last_pushed",
         ),
     )