diff --git a/apps/project/graphql/types/types.py b/apps/project/graphql/types/types.py index a20a0790..a1ee2ed9 100644 --- a/apps/project/graphql/types/types.py +++ b/apps/project/graphql/types/types.py @@ -187,13 +187,6 @@ class ProjectType(UserResourceTypeMixin, ProjectExportAssetTypeMixin, FirebasePu def progress(self, project: strawberry.Parent[Project]) -> float: return project.progress / 100 - @strawberry_django.field( - description="No. of unique contributors in this project", - ) - def contributors_count(self) -> int: - # TODO(tnagorra): We need to implement this - return 0 - @strawberry_django.field( only=["topic", "region", "project_number", "requesting_organization__name", "project_type"], annotate={"generated_name": Project.generate_name_query()}, diff --git a/apps/tutorial/tests/mutation_test.py b/apps/tutorial/tests/mutation_test.py index 268f1d55..78e93662 100644 --- a/apps/tutorial/tests/mutation_test.py +++ b/apps/tutorial/tests/mutation_test.py @@ -4,7 +4,6 @@ from django.core.files.temp import NamedTemporaryFile from PIL import Image -from ulid import ULID from apps.project.factories import OrganizationFactory, ProjectFactory from apps.project.models import ( @@ -346,7 +345,7 @@ def _update_tutorial_status_mutation(self, pk: str, tutorial_data: dict, **kwarg def test_tutorial_create(self): tutorial_data = { - "clientId": str(ULID()), + "clientId": "01K748SHD9W5BTQA8EB9RCZGB7", "name": "My Tutorial", "project": self.project.pk, } @@ -378,7 +377,7 @@ def test_tutorial_create(self): # Creating Project Image Asset tutorial_asset_data = { "tutorial": latest_tutorial.pk, - "clientId": str(ULID()), + "clientId": "01K748TDZPSDKPX1QVC5J5H558", } content = self._create_tutorial_image_asset(tutorial_asset_data) resp_data = content["data"]["createTutorialAsset"] @@ -386,13 +385,14 @@ def test_tutorial_create(self): image_asset = resp_data["result"] # Update Tutorial + tutorial_data.pop("project") tutorial_data = { **tutorial_data, "scenarios": [ { "create": { - "clientId": str(ULID()), + "clientId": "01K748TDZPADVS0XX5FV59Q8JC", "scenarioPageNumber": 1, "instructionsDescription": "Anything that is not naturally occurring", "instructionsIcon": "STAR_OUTLINE", @@ -405,32 +405,32 @@ def test_tutorial_create(self): "successTitle": "Well done!", "tasks": [ { - "clientId": str(ULID()), + "clientId": "01K748TDZQAX1242QFT3R8V3H1", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193196, "tileY": 110087}}, "reference": 0, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZQDAHN2RAEJ6KKQZBS", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193196, "tileY": 110088}}, "reference": 1, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZQN9KR7PPAWY282B8V", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193196, "tileY": 110089}}, "reference": 0, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZQCYF1Q88MVW3B5J8A", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193197, "tileY": 110087}}, "reference": 0, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZR861B76GJAYS3WCY6", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193197, "tileY": 110088}}, "reference": 1, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZRT35XEZFT0SRR1EF0", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193197, "tileY": 110089}}, "reference": 0, }, @@ -439,7 +439,7 @@ def test_tutorial_create(self): }, { "create": { - "clientId": str(ULID()), + "clientId": "01K748TDZSP64X7DY58NQZQ87E", "scenarioPageNumber": 2, "instructionsDescription": "Anything that is not naturally occurring", "instructionsIcon": "STAR_OUTLINE", @@ -452,32 +452,32 @@ def test_tutorial_create(self): "successTitle": "Well done!", "tasks": [ { - "clientId": str(ULID()), + "clientId": "01K748TDZSBB9R1D0B0JYWQVHS", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193204, "tileY": 110087}}, "reference": 1, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZS4875A6W77FFKCB39", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193204, "tileY": 110088}}, "reference": 1, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZSDZJ7VPY35KQRMZ8E", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193204, "tileY": 110089}}, "reference": 1, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZS86H58VNJ876JTB7R", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193205, "tileY": 110087}}, "reference": 1, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZS5DGYD7RGHT2EEEV6", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193205, "tileY": 110088}}, "reference": 1, }, { - "clientId": str(ULID()), + "clientId": "01K748TDZSC0NVNAMZKBQRRYQY", "projectTypeSpecifics": {"find": {"tileZ": 18, "tileX": 193205, "tileY": 110089}}, "reference": 1, }, @@ -488,26 +488,26 @@ def test_tutorial_create(self): "informationPages": [ { "create": { - "clientId": str(ULID()), + "clientId": "01K748TDZTPW7Y19MEMPH0332S", "title": "Man-made structures", "pageNumber": 1, "blocks": [ { - "clientId": str(ULID()), + "clientId": "01K748TDZTPVP7HF5VX03HTW9Z", "blockNumber": 1, "blockType": "TEXT", "text": "Man-made structures are physical constructions created by humans, typically " "using tools, materials, and engineering principles.", }, { - "clientId": str(ULID()), + "clientId": "01K748TDZT5T9D33D78VV5NFKS", "blockNumber": 2, "blockType": "TEXT", "text": "These structures are built to serve specific purposes, such as housing, " "transportation, defense, communication, or recreation.", }, { - "clientId": str(ULID()), + "clientId": "01K748TDZT7PV91ZX9W5H4BS47", "blockNumber": 3, "blockType": "IMAGE", "image": image_asset["id"], @@ -517,12 +517,12 @@ def test_tutorial_create(self): }, { "create": { - "clientId": str(ULID()), + "clientId": "01K748TDZTZ2M1FJWEKHGDVSV9", "title": "Natural structures", "pageNumber": 2, "blocks": [ { - "clientId": str(ULID()), + "clientId": "01K748TDZTS0MK857GPZHKYDWW", "blockNumber": 1, "blockType": "TEXT", "text": "Natural structures are physical formations that are created by nature " @@ -533,7 +533,6 @@ def test_tutorial_create(self): }, ], } - tutorial_data.pop("project") # Updating Tutorial: Without Authentication self.logout() @@ -547,8 +546,56 @@ def test_tutorial_create(self): }, ], content - # Updating Tutorial: With Authentication + # Updating Tutorial: Check deep nested validation issues self.force_login(self.user) + tutorial_data["scenarios"][1]["create"]["tasks"][1]["reference"] = -9 # type: ignore[reportArgumentType, reportIndexIssue] + content = self._update_tutorial_mutation(str(latest_tutorial.pk), tutorial_data) + tutorial_data["scenarios"][1]["create"]["tasks"][1]["reference"] = 1 # type: ignore[reportArgumentType, reportIndexIssue] + + resp_data = content["data"]["updateTutorial"] + expected_error_message = [ + { + "field": "scenarios", + "client_id": "01K748SHD9W5BTQA8EB9RCZGB7", + "messages": None, + "object_errors": None, + "array_errors": [ + { + "client_id": "01K748TDZSP64X7DY58NQZQ87E", + "messages": None, + "object_errors": [ + { + "field": "tasks", + "client_id": "01K748TDZSP64X7DY58NQZQ87E", + "messages": None, + "object_errors": None, + "array_errors": [ + { + "client_id": "01K748TDZS4875A6W77FFKCB39", + "messages": None, + "object_errors": [ + { + "field": "reference", + "client_id": "01K748TDZS4875A6W77FFKCB39", + "messages": "Ensure this value is greater than or equal to 0.", + "object_errors": None, + "array_errors": None, + "pydantic_errors": None, + }, + ], + }, + ], + "pydantic_errors": None, + }, + ], + }, + ], + "pydantic_errors": None, + }, + ] + assert resp_data["errors"] == expected_error_message, content + + # Updating Tutorial: With Authentication and correct data content = self._update_tutorial_mutation(str(latest_tutorial.pk), tutorial_data) resp_data = content["data"]["updateTutorial"] assert resp_data["errors"] is None, content @@ -618,7 +665,7 @@ def get_update_for_task(tut: dict): # type: ignore[reportMissingTypeArgument] }, { "create": { - "clientId": str(ULID()), + "clientId": "01K748TDZTRDF6Z3X2VTNKJEGF", # NOTE: blockNumber 2 is previously deleted so should not error on unique constraint "blockNumber": 2, "blockType": "TEXT", @@ -744,7 +791,7 @@ def get_update_for_task(tut: dict): # type: ignore[reportMissingTypeArgument] def test_tutorial_state_transitions(self): # Create a draft tutorial tutorial = TutorialFactory.create( - client_id=str(ULID()), + client_id="01K748TDZT84RRP5GACAYB71PP", name="Status Tutorial", project=self.project, created_by=self.user, diff --git a/assets b/assets index 555b47f6..69dae411 160000 --- a/assets +++ b/assets @@ -1 +1 @@ -Subproject commit 555b47f659791a5b873f55bf9771be2bdf94ef9a +Subproject commit 69dae411dde7df1b20df2d1ec0e35fc8ccee877b diff --git a/firebase b/firebase index ddabcd2e..b7337240 160000 --- a/firebase +++ b/firebase @@ -1 +1 @@ -Subproject commit ddabcd2e5184cac2ca330b8befd3e86a41c0217c +Subproject commit b733724018baea143e7fad6a79714a7fd2fca68b diff --git a/project_types/base/project.py b/project_types/base/project.py index cf62cfa0..27f91d44 100644 --- a/project_types/base/project.py +++ b/project_types/base/project.py @@ -4,6 +4,7 @@ from abc import ABC, abstractmethod from collections import defaultdict +from celery.exceptions import SoftTimeLimitExceeded from django.contrib.gis.db.models import GeometryField from django.contrib.gis.db.models.functions import Area from django.core.files.base import ContentFile @@ -18,7 +19,6 @@ from apps.common.models import FirebasePushStatusEnum from apps.common.utils import get_absolute_uri -from apps.mapping.models import MappingSession from apps.project.models import ( Project, ProjectAsset, @@ -226,6 +226,13 @@ def process_project(self): exc_info=True, ) self.project.status_message = str(ex) + elif isinstance(ex, SoftTimeLimitExceeded): + logger.error( + "process_project failed due to SoftTimeLimitExceeded", + extra=log_extra({"project": self.project.pk}), + exc_info=True, + ) + self.project.status_message = "Project processing timed out before completion" else: logger.error( "process_project failed", @@ -421,17 +428,6 @@ def update_project_on_firebase(self, project_ref: FbReference, fb_project: fireb assert self.project.tutorial_id is not None, "Tutorial is required before project can be pushed to firebase" assert self.project.tutorial is not None, "Tutorial is required before project can be pushed to firebase" - unique_contributors_count = ( - MappingSession.objects.filter( - project_task_group__in=ProjectTaskGroup.objects.filter(project=self.project), - ) - .values( - "contributor_user_id", - ) - .distinct() - .count() - ) - project_ref.update( value=firebase_utils.serialize( firebase_models.FbProjectUpdateInput( @@ -450,7 +446,7 @@ def update_project_on_firebase(self, project_ref: FbReference, fb_project: fireb tutorialId=self.project.tutorial.firebase_id, status=BaseProject.get_firebase_status(self.project.status_enum, not self.project.team_id), teamId=self.project.team.firebase_id if self.project.team else None, - contributorCount=unique_contributors_count, + contributorCount=self.project.number_of_contributor_users, progress=self.project.progress, # FIXME(tnagorra): Need to check how we get this? language="en-us", @@ -514,6 +510,13 @@ def push_project_on_firebase(self): exc_info=True, ) self.project.status_message = str(ex) + elif isinstance(ex, SoftTimeLimitExceeded): + logger.error( + "push_to_firebase for project failed due to SoftTimeLimitExceeded", + extra=log_extra({"project": self.project.pk}), + exc_info=True, + ) + self.project.status_message = "Project sync to firebase timed out before completion" else: logger.error( "push_to_firebase for project failed", diff --git a/project_types/base/tutorial.py b/project_types/base/tutorial.py index ae19a363..b92ae9f6 100644 --- a/project_types/base/tutorial.py +++ b/project_types/base/tutorial.py @@ -3,6 +3,7 @@ import typing from abc import ABC, abstractmethod +from celery.exceptions import SoftTimeLimitExceeded from django.core.files.base import ContentFile from firebase_admin.db import Reference as FbReference # type: ignore[reportMissingTypeStubs] from pydantic import BaseModel, ConfigDict @@ -367,12 +368,30 @@ def push_tutorial_on_firebase(self): try: self._push_tutorial_on_firebase() except Exception as ex: - logger.error( - "push_to_firebase for tutorial failed", - extra=log_extra({"tutorial": self.tutorial.pk}), - exc_info=True, - ) - self.tutorial.status_message = str(ex) if isinstance(ex, TutorialValidationException) else None + if isinstance(ex, TutorialValidationException): + logger.warning( + "push_to_firebase for tutorial failed", + extra=log_extra({"tutorial": self.tutorial.pk}), + exc_info=True, + ) + self.tutorial.status_message = str(ex) + elif isinstance(ex, SoftTimeLimitExceeded): + logger.error( + "push_to_firebase for tutorial failed due to SoftTimeLimitExceeded", + extra=log_extra({"tutorial": self.tutorial.pk}), + exc_info=True, + ) + self.tutorial.status_message = "Tutorial sync to firebase timed out before completion" + else: + logger.error( + "push_to_firebase for tutorial failed", + extra=log_extra({"tutorial": self.tutorial.pk}), + exc_info=True, + ) + self.tutorial.status_message = ( + "Something unexpected happened! Please reach out on the MapSwipe slack for any further assistance." + ) + self.tutorial.update_firebase_push_status(FirebasePushStatusEnum.FAILED, False) # TODO(tnagorra): We also need to clear any intermediate values for groups, tasks and tutorial in firebase # NOTE: If tutorial has already been published, we cannot update it's status diff --git a/schema.graphql b/schema.graphql index 411a0b94..a890a3c1 100644 --- a/schema.graphql +++ b/schema.graphql @@ -1737,9 +1737,6 @@ type ProjectType implements UserResourceTypeMixin & ProjectExportAssetTypeMixin aoiGeometry: GeometryType aoiGeometryInputAsset: ProjectAssetType clientId: String! - - """No. of unique contributors in this project""" - contributorsCount: Int! createdAt: DateTime! createdBy: UserType! description: String diff --git a/utils/graphql/drf.py b/utils/graphql/drf.py index 43af9d82..fc9d6f4f 100644 --- a/utils/graphql/drf.py +++ b/utils/graphql/drf.py @@ -11,6 +11,22 @@ ARRAY_NON_MEMBER_ERRORS = "nonMemberErrors" +# NOTE: Recursively iterate over dict and list and convert strawberry type object to dict +def recursive_dict(data: typing.Any): + if isinstance(data, dict): + new_obj = {} + for key, value in data.items(): + new_obj[key] = recursive_dict(value) + return new_obj + if isinstance(data, list): + return [recursive_dict(item) for item in data] + if isinstance(data, ArrayNestedErrorType): + return dict(data) + if isinstance(data, MutationCustomErrorType): + return dict(data) + return data + + @strawberry.type class ArrayNestedErrorType: client_id: str @@ -22,9 +38,10 @@ def keys(self): def __getitem__(self, key: str): key = to_snake_case(key) - if key in ("object_errors",) and getattr(self, key): - return [dict(each) for each in getattr(self, key)] - return getattr(self, key) + attr_value = getattr(self, key) + if key == "object_errors" and attr_value: + return [recursive_dict(each) for each in attr_value] + return attr_value @strawberry.type @@ -56,12 +73,12 @@ def keys(self): def __getitem__(self, key: str): key = to_snake_case(key) - if key in ("object_errors", "array_errors") and getattr(self, key): - # TODO(thenav56): Confirm if using str() is enough - if key == "object_errors": - return {each_key: str(each_data) for each_key, each_data in getattr(self, key).items()} - return [str(each) for each in getattr(self, key)] - return getattr(self, key) + attr_value = getattr(self, key) + if key == "object_errs" and attr_value: + return {each_key: recursive_dict(each_data) for each_key, each_data in attr_value.items()} + if key == "array_errors" and attr_value: + return [recursive_dict(each) for each in attr_value] + return attr_value # TODO(thenav56): Check again on the error structure @@ -86,59 +103,55 @@ def _serializer_error_to_error_types( initial_data = initial_data or {} node_client_id = initial_data.get("client_id") error_types: list[MutationCustomErrorType] = [] + for field, value in errors.items(): if field.endswith("-pydantic"): - error_types.append( - MutationCustomErrorType( - client_id=node_client_id, - field=to_camel_case(field.replace("-pydantic", "")), - object_errors=None, - array_errors=None, - messages=None, - # NOTE: Error from handle_pydantic_validation_error - pydantic_errors=value, - ), + err = MutationCustomErrorType( + client_id=node_client_id, + field=to_camel_case(field.replace("-pydantic", "")), + object_errors=None, + array_errors=None, + messages=None, + # NOTE: Error from handle_pydantic_validation_error + pydantic_errors=value, ) + error_types.append(err) continue if isinstance(value, dict): - error_types.append( - MutationCustomErrorType( - client_id=node_client_id, - field=to_camel_case(field), - object_errors=value, # type: ignore[reportArgumentType] - array_errors=None, - messages=None, - ), + err = MutationCustomErrorType( + client_id=node_client_id, + field=to_camel_case(field), + object_errors=value, # type: ignore[reportArgumentType] + array_errors=None, + messages=None, ) + error_types.append(err) elif isinstance(value, list): if isinstance(value[0], str): # type: ignore[reportUnnecessaryIsInstance] if isinstance(initial_data.get(field), list): # we have found an array input with top level error - error_types.append( - MutationCustomErrorType( - client_id=node_client_id, - field=to_camel_case(field), - array_errors=[ - ArrayNestedErrorType( - client_id=ARRAY_NON_MEMBER_ERRORS, - messages="".join(str(msg) for msg in value), - object_errors=None, - ), - ], - messages=None, - object_errors=None, - ), + err = MutationCustomErrorType( + client_id=node_client_id, + field=to_camel_case(field), + array_errors=[ + ArrayNestedErrorType( + client_id=ARRAY_NON_MEMBER_ERRORS, + messages="".join(str(msg) for msg in value), + object_errors=None, + ), + ], + messages=None, + object_errors=None, ) else: - error_types.append( - MutationCustomErrorType( - client_id=node_client_id, - field=to_camel_case(field), - messages=", ".join(str(msg) for msg in value), - object_errors=None, - array_errors=None, - ), + err = MutationCustomErrorType( + client_id=node_client_id, + field=to_camel_case(field), + messages=", ".join(str(msg) for msg in value), + object_errors=None, + array_errors=None, ) + error_types.append(err) elif isinstance(value[0], dict): # type: ignore[reportUnnecessaryIsInstance] array_errors = [] for pos, array_item in enumerate(value): @@ -154,25 +167,23 @@ def _serializer_error_to_error_types( messages=None, ), ) - error_types.append( - MutationCustomErrorType( - client_id=node_client_id, - field=to_camel_case(field), - array_errors=array_errors, - object_errors=None, - messages=None, - ), + err = MutationCustomErrorType( + client_id=node_client_id, + field=to_camel_case(field), + array_errors=array_errors, + object_errors=None, + messages=None, ) + error_types.append(err) else: # fallback - error_types.append( - MutationCustomErrorType( - field=to_camel_case(field), - messages=" ".join(str(msg) for msg in value or []), - array_errors=None, - object_errors=None, - ), + err = MutationCustomErrorType( + field=to_camel_case(field), + messages=" ".join(str(msg) for msg in value or []), + array_errors=None, + object_errors=None, ) + error_types.append(err) return error_types