Merge pull request #6605 from hotosm/fastapi-refactor

All messages, organisation retrieve and task activities fixes

backend/api/organisations/resources.py

@@ -78,7 +78,6 @@ async def retrieve_organisation(
         user_id = 0
     else:
         user_id = authenticated_user_id
-    # Validate abbreviated.
     organisation_dto = await OrganisationService.get_organisation_by_id_as_dto(
         organisation_id, user_id, omit_managers, db
     )

backend/models/dtos/message_dto.py

@@ -7,17 +7,18 @@
 class MessageDTO(BaseModel):
     """DTO used to define a message that will be sent to a user"""
 
-    message_id: int = Field(None, alias="message_id")
-    subject: str = Field(None, alias="subject")
-    message: str = Field(None, alias="message")
+    message_id: Optional[int] = Field(None, alias="messageId")
+    subject: str = Field(min_length=1, alias="subject")
+    message: str = Field(min_length=1, alias="message")
+    from_user_id: int = Field(alias="fromUserId")
     from_username: Optional[str] = Field("", alias="fromUsername")
     display_picture_url: Optional[str] = Field("", alias="displayPictureUrl")
     project_id: Optional[int] = Field(None, alias="projectId")
     project_title: Optional[str] = Field(None, alias="projectTitle")
     task_id: Optional[int] = Field(None, alias="taskId")
-    message_type: Optional[str] = Field(None, alias="message_type")
-    sent_date: datetime = Field(None, alias="sentDate")
-    read: bool = False
+    message_type: Optional[str] = Field(None, alias="messageType")
+    sent_date: Optional[datetime] = Field(None, alias="sentDate")
+    read: Optional[bool] = None
 
     class Config:
         populate_by_name = True

backend/models/postgis/organisation.py

@@ -92,80 +92,78 @@ async def create_from_dto(new_organisation_dto: NewOrganisationDTO, db: Database
         }
 
         try:
-            async with db.transaction():
-                organisation_id = await db.execute(query, values)
+            organisation_id = await db.execute(query, values)
 
-                for manager in new_organisation_dto.managers:
-                    user_query = "SELECT id FROM users WHERE username = :username"
-                    user = await db.fetch_one(user_query, {"username": manager})
+            for manager in new_organisation_dto.managers:
+                user_query = "SELECT id FROM users WHERE username = :username"
+                user = await db.fetch_one(user_query, {"username": manager})
 
-                    if not user:
-                        raise NotFound(sub_code="USER_NOT_FOUND", username=manager)
+                if not user:
+                    raise NotFound(sub_code="USER_NOT_FOUND", username=manager)
 
-                    manager_query = """
-                    INSERT INTO organisation_managers (organisation_id, user_id)
-                    VALUES (:organisation_id, :user_id)
-                    """
-                    await db.execute(
-                        manager_query,
-                        {"organisation_id": organisation_id, "user_id": user.id},
-                    )
+                manager_query = """
+                INSERT INTO organisation_managers (organisation_id, user_id)
+                VALUES (:organisation_id, :user_id)
+                """
+                await db.execute(
+                    manager_query,
+                    {"organisation_id": organisation_id, "user_id": user.id},
+                )
 
-                return organisation_id
+            return organisation_id
 
         except Exception as e:
             raise HTTPException(status_code=500, detail=str(e)) from e
 
     async def update(organisation_dto: UpdateOrganisationDTO, db: Database):
         """Updates Organisation from DTO"""
-        async with db.transaction():
-            try:
-                org_id = organisation_dto.organisation_id
-                org_dict = organisation_dto.dict(exclude_unset=True)
-                if "type" in org_dict and org_dict["type"] is not None:
-                    org_dict["type"] = OrganisationType[org_dict["type"].upper()].value
-
-                update_keys = {
-                    key: org_dict[key]
-                    for key in org_dict.keys()
-                    if key not in ["organisation_id", "managers"]
-                }
-                set_clause = ", ".join(f"{key} = :{key}" for key in update_keys.keys())
-                update_query = f"""
-                UPDATE organisations
-                SET {set_clause}
-                WHERE id = :id
+        try:
+            org_id = organisation_dto.organisation_id
+            org_dict = organisation_dto.dict(exclude_unset=True)
+            if "type" in org_dict and org_dict["type"] is not None:
+                org_dict["type"] = OrganisationType[org_dict["type"].upper()].value
+
+            update_keys = {
+                key: org_dict[key]
+                for key in org_dict.keys()
+                if key not in ["organisation_id", "managers"]
+            }
+            set_clause = ", ".join(f"{key} = :{key}" for key in update_keys.keys())
+            update_query = f"""
+            UPDATE organisations
+            SET {set_clause}
+            WHERE id = :id
+            """
+            await db.execute(update_query, values={**update_keys, "id": org_id})
+
+            if organisation_dto.managers:
+                clear_managers_query = """
+                DELETE FROM organisation_managers
+                WHERE organisation_id = :id
                 """
-                await db.execute(update_query, values={**update_keys, "id": org_id})
+                await db.execute(clear_managers_query, values={"id": org_id})
 
-                if organisation_dto.managers:
-                    clear_managers_query = """
-                    DELETE FROM organisation_managers
-                    WHERE organisation_id = :id
-                    """
-                    await db.execute(clear_managers_query, values={"id": org_id})
+                for manager_username in organisation_dto.managers:
+                    user_query = "SELECT id FROM users WHERE username = :username"
+                    user = await db.fetch_one(
+                        user_query, {"username": manager_username}
+                    )
 
-                    for manager_username in organisation_dto.managers:
-                        user_query = "SELECT id FROM users WHERE username = :username"
-                        user = await db.fetch_one(
-                            user_query, {"username": manager_username}
+                    if not user:
+                        raise NotFound(
+                            sub_code="USER_NOT_FOUND", username=manager_username
                         )
 
-                        if not user:
-                            raise NotFound(
-                                sub_code="USER_NOT_FOUND", username=manager_username
-                            )
-
-                        insert_manager_query = """
-                        INSERT INTO organisation_managers (organisation_id, user_id)
-                        VALUES (:organisation_id, :user_id)
-                        """
-                        await db.execute(
-                            insert_manager_query,
-                            {"organisation_id": org_id, "user_id": user.id},
-                        )
-            except Exception as e:
-                raise HTTPException(status_code=500, detail=str(e)) from e
+                    insert_manager_query = """
+                    INSERT INTO organisation_managers (organisation_id, user_id)
+                    VALUES (:organisation_id, :user_id)
+                    """
+                    await db.execute(
+                        insert_manager_query,
+                        {"organisation_id": org_id, "user_id": user.id},
+                    )
+        except Exception as e:
+            raise HTTPException(status_code=500, detail=str(e)) from e
 
     def delete(self):
         """Deletes the current model from the DB"""
@@ -305,23 +303,47 @@ async def fetch_managers(self, session):
         """Fetch managers asynchronously"""
         await session.refresh(self, ["managers"])
 
-    def as_dto(self, omit_managers=False):
+    # def as_dto(self, omit_managers=False):
+    #     """Returns a dto for an organisation"""
+    #     organisation_dto = OrganisationDTO()
+    #     organisation_dto.organisation_id = self.id
+    #     organisation_dto.name = self.name
+    #     organisation_dto.slug = self.slug
+    #     organisation_dto.logo = self.logo
+    #     organisation_dto.description = self.description
+    #     organisation_dto.url = self.url
+    #     organisation_dto.managers = []
+    #     organisation_dto.type = OrganisationType(self.type).name
+    #     organisation_dto.subscription_tier = self.subscription_tier
+
+    #     if omit_managers:
+    #         return organisation_dto
+
+    #     for manager in self.managers:
+    #         org_manager_dto = OrganisationManagerDTO()
+    #         org_manager_dto.username = manager.username
+    #         org_manager_dto.picture_url = manager.picture_url
+    #         organisation_dto.managers.append(org_manager_dto)
+
+    #     return organisation_dto
+
+    def as_dto(org, omit_managers=False):
         """Returns a dto for an organisation"""
         organisation_dto = OrganisationDTO()
-        organisation_dto.organisation_id = self.id
-        organisation_dto.name = self.name
-        organisation_dto.slug = self.slug
-        organisation_dto.logo = self.logo
-        organisation_dto.description = self.description
-        organisation_dto.url = self.url
+        organisation_dto.organisation_id = org.organisation_id
+        organisation_dto.name = org.name
+        organisation_dto.slug = org.slug
+        organisation_dto.logo = org.logo
+        organisation_dto.description = org.description
+        organisation_dto.url = org.url
         organisation_dto.managers = []
-        organisation_dto.type = OrganisationType(self.type).name
-        organisation_dto.subscription_tier = self.subscription_tier
+        organisation_dto.type = org.type
+        organisation_dto.subscription_tier = org.subscription_tier
 
         if omit_managers:
             return organisation_dto
 
-        for manager in self.managers:
+        for manager in org.managers:
             org_manager_dto = OrganisationManagerDTO()
             org_manager_dto.username = manager.username
             org_manager_dto.picture_url = manager.picture_url

backend/models/postgis/task.py

@@ -1,55 +1,60 @@
-import bleach
 import datetime
-from backend.models.dtos.task_annotation_dto import TaskAnnotationDTO
-import geojson
 import json
 from enum import Enum
+from typing import Any, Dict, List
 
-# # from flask import current_app
-from sqlalchemy import desc, func, distinct
-from sqlalchemy.orm.exc import MultipleResultsFound
+import bleach
+import geojson
 from geoalchemy2 import Geometry
-from typing import Any, Dict, List
 from shapely.geometry import shape
 
+# # from flask import current_app
 from sqlalchemy import (
-    Column,
-    Integer,
     BigInteger,
+    Boolean,
+    Column,
     DateTime,
-    String,
     ForeignKey,
-    Boolean,
-    Index,
     ForeignKeyConstraint,
+    Index,
+    Integer,
+    String,
     Unicode,
+    desc,
+    distinct,
+    func,
 )
 from sqlalchemy.orm import relationship
+from sqlalchemy.orm.exc import MultipleResultsFound
+
+from backend.db import Base, get_session
 from backend.exceptions import NotFound
 from backend.models.dtos.mapping_dto import TaskDTO, TaskHistoryDTO
-from backend.models.dtos.validator_dto import MappedTasksByUser, MappedTasks
+from backend.models.dtos.mapping_issues_dto import TaskMappingIssueDTO
 from backend.models.dtos.project_dto import (
+    LockedTasksForUser,
     ProjectComment,
     ProjectCommentsDTO,
-    LockedTasksForUser,
 )
-from backend.models.dtos.mapping_issues_dto import TaskMappingIssueDTO
-from backend.models.postgis.statuses import TaskStatus, MappingLevel
+from backend.models.dtos.task_annotation_dto import TaskAnnotationDTO
+from backend.models.dtos.validator_dto import MappedTasks, MappedTasksByUser
+from backend.models.postgis.statuses import MappingLevel, TaskStatus
+from backend.models.postgis.task_annotation import TaskAnnotation
 from backend.models.postgis.user import User
 from backend.models.postgis.utils import (
     InvalidData,
     InvalidGeoJson,
-    timestamp,
     parse_duration,
+    timestamp,
 )
-from backend.models.postgis.task_annotation import TaskAnnotation
-from backend.db import Base, get_session
 
 session = get_session()
-from backend.config import settings
-from sqlalchemy import select
 from typing import Optional
+
 from databases import Database
+from sqlalchemy import select
+
+from backend.config import settings
 
 
 class TaskAction(Enum):
@@ -1738,9 +1743,27 @@ async def as_dto_with_instructions(
             task_id, project_id, preferred_locale, db
         )
         if not per_task_instructions:
-            default_locale = rows[0]["default_locale"]
-            per_task_instructions = await Task.get_per_task_instructions(
-                task_id, project_id, default_locale, db
+            query_locale = """
+                SELECT
+                    p.default_locale
+                FROM
+                    projects p
+                WHERE
+                    p.id = :project_id
+            """
+            default_locale_row = await db.fetch_one(
+                query=query_locale, values={"project_id": project_id}
+            )
+            default_locale = (
+                default_locale_row["default_locale"] if default_locale_row else None
+            )
+
+            per_task_instructions = (
+                await Task.get_per_task_instructions(
+                    task_id, project_id, default_locale, db
+                )
+                if default_locale
+                else None
             )
 
         task_dto.per_task_instructions = per_task_instructions

backend/services/messaging/message_service.py

@@ -804,6 +804,7 @@ async def get_all_messages(
                 m.id AS message_id,
                 m.subject,
                 m.message,
+                m.from_user_id,
                 m.to_user_id,
                 m.task_id,
                 m.message_type,
@@ -862,7 +863,9 @@ async def get_all_messages(
                 message_dict["message_type"] = MessageType(
                     message_dict["message_type"]
                 ).name
-            msg_dto = MessageDTO(**message_dict)
+            msg_dto = MessageDTO(**message_dict).dict(
+                exclude={"from_user_id"}, by_alias=True
+            )
             messages_dto.user_messages.append(msg_dto)
 
         total_count_query = """