Merge pull request #6646 from hotosm/fastapi-refactor

* Send message after comment, parse usernames and mentions. * Project mapping type filter fixed.

backend/api/comments/resources.py

@@ -1,6 +1,6 @@
 from backend.models.postgis.utils import timestamp
 from databases import Database
-from fastapi import APIRouter, Depends, Request
+from fastapi import APIRouter, Depends, Request, BackgroundTasks
 from loguru import logger
 from fastapi.responses import JSONResponse
 from backend.db import get_db, get_session
@@ -28,6 +28,7 @@
 async def post(
     project_id: int,
     request: Request,
+    background_tasks: BackgroundTasks,
     user: AuthUserDTO = Depends(login_required),
     db: Database = Depends(get_db),
 ):
@@ -86,7 +87,7 @@ async def post(
     try:
         async with db.transaction():
             project_messages = await ChatService.post_message(
-                chat_dto, project_id, user.id, db
+                chat_dto, project_id, user.id, db, background_tasks
             )
             return project_messages
     except ValueError as e:

backend/api/projects/resources.py

@@ -569,8 +569,8 @@ def setup_search_dto(request) -> ProjectSearchDTO:
 
     mapping_types_str = request.query_params.get("mappingTypes")
     if mapping_types_str:
-        search_dto.mapping_types = map(
-            str, mapping_types_str.split(",")
+        search_dto.mapping_types = list(
+            map(str, mapping_types_str.split(","))
         )  # Extract list from string
     search_dto.mapping_types_exact = strtobool(
         request.query_params.get("mappingTypesExact", "false")

backend/services/messaging/chat_service.py

@@ -1,6 +1,5 @@
-import threading
-
 from databases import Database
+from fastapi import BackgroundTasks
 
 from backend.exceptions import NotFound
 from backend.models.dtos.message_dto import ChatMessageDTO, ProjectChatDTO
@@ -12,6 +11,7 @@
 from backend.services.project_admin_service import ProjectAdminService
 from backend.services.project_service import ProjectService
 from backend.services.team_service import TeamService
+from backend.db import db_connection
 
 
 class ChatService:
@@ -21,6 +21,7 @@ async def post_message(
         project_id: int,
         authenticated_user_id: int,
         db: Database,
+        background_tasks: BackgroundTasks,
     ) -> ProjectChatDTO:
         project = await ProjectService.get_project_by_id(project_id, db)
         project_info_dto = await ProjectInfo.get_dto_for_locale(
@@ -66,18 +67,14 @@ async def post_message(
                     )
         if is_manager_permission or is_team_member or is_allowed_user:
             chat_message = await ProjectChat.create_from_dto(chat_dto, db)
-            # TODO: Refactor send_message_after_chat
-            threading.Thread(
-                target=MessageService.send_message_after_chat,
-                args=(
-                    chat_dto.user_id,
-                    chat_message.message,
-                    chat_dto.project_id,
-                    project_name,
-                    db,
-                ),
-            ).start()
-            # Ensure we return latest messages after post
+            background_tasks.add_task(
+                MessageService.send_message_after_chat,
+                chat_dto.user_id,
+                chat_message.message,
+                chat_dto.project_id,
+                project_name,
+                db_connection.database,
+            )
             return await ProjectChat.get_messages(chat_dto.project_id, db, 1, 5)
         else:
             raise ValueError("UserNotPermitted- User not permitted to post Comment")

backend/services/messaging/message_service.py

@@ -10,15 +10,15 @@
 from markdown import markdown
 from sqlalchemy import func, insert, text
 
-from backend import create_app, db
+from backend import db
 from backend.config import settings
 from backend.exceptions import NotFound
 from backend.models.dtos.message_dto import MessageDTO, MessagesDTO
 from backend.models.dtos.stats_dto import Pagination
 from backend.models.postgis.message import Message, MessageType
 from backend.models.postgis.notification import Notification
 from backend.models.postgis.project import Project, ProjectInfo
-from backend.models.postgis.task import TaskAction, TaskHistory, TaskStatus
+from backend.models.postgis.task import TaskAction, TaskStatus
 from backend.models.postgis.utils import timestamp
 from backend.services.messaging.smtp_service import SMTPService
 from backend.services.messaging.template_service import (
@@ -549,19 +549,17 @@ async def send_team_join_notification(
         await Message.save(message, db)
 
     @staticmethod
-    def send_message_after_chat(
-        chat_from: int, chat: str, project_id: int, project_name: str, db: Database
+    async def send_message_after_chat(
+        chat_from: int,
+        chat: str,
+        project_id: int,
+        project_name: str,
+        database: Database,
     ):
-        """Send alert to user if they were @'d in a chat message"""
-        app = (
-            create_app()
-        )  # Because message-all run on background thread it needs it's own app context
-        if (
-            app.config["ENVIRONMENT"] == "test"
-        ):  # Don't send in test mode as this will cause tests to fail.
-            return
-        with app.app_context():
-            usernames = MessageService._parse_message_for_username(chat, project_id)
+        async with database.connection() as db:
+            usernames = await MessageService._parse_message_for_username(
+                message=chat, project_id=project_id, db=db
+            )
             if len(usernames) != 0:
                 link = MessageService.get_project_link(
                     project_id, project_name, include_chat_section=True
@@ -570,7 +568,7 @@ def send_message_after_chat(
                 for username in usernames:
                     logger.debug(f"Searching for {username}")
                     try:
-                        user = UserService.get_user_by_username(username)
+                        user = await UserService.get_user_by_username(username, db)
                     except NotFound:
                         logger.error(f"Username {username} not found")
                         continue  # If we can't find the user, keep going no need to fail
@@ -588,37 +586,47 @@ def send_message_after_chat(
                         dict(message=message, user=user, project_name=project_name)
                     )
 
-                MessageService._push_messages(messages, db)
-
-            query = f""" select user_id from project_favorites where project_id ={project_id}"""
-            with db.engine.connect() as conn:
-                favorited_users_results = conn.execute(text(query))
-            favorited_users = [r[0] for r in favorited_users_results]
-
-            # Notify all contributors except the user that created the comment.
-            contributed_users_results = (
-                TaskHistory.query.with_entities(TaskHistory.user_id.distinct())
-                .filter(TaskHistory.project_id == project_id)
-                .filter(TaskHistory.user_id != chat_from)
-                .filter(TaskHistory.action == TaskAction.STATE_CHANGE.name)
-                .all()
+                await MessageService._push_messages(messages, db)
+            favorited_users_query = """ select user_id from project_favorites where project_id = :project_id"""
+            favorited_users_values = {
+                "project_id": project_id,
+            }
+            favorited_users_results = await db.fetch_all(
+                query=favorited_users_query, values=favorited_users_values
             )
-            contributed_users = [r[0] for r in contributed_users_results]
+            favorited_users = [r.user_id for r in favorited_users_results]
+            # Notify all contributors except the user that created the comment.
+            contributed_users_query = """
+            SELECT DISTINCT user_id
+            FROM task_history
+            WHERE project_id = :project_id
+            AND user_id != :chat_from
+            AND action = :state_change_action
+            """
 
+            values = {
+                "project_id": project_id,
+                "chat_from": chat_from,
+                "state_change_action": TaskAction.STATE_CHANGE.name,
+            }
+            contributed_users_results = await db.fetch_all(
+                query=contributed_users_query, values=values
+            )
+            contributed_users = [r.user_id for r in contributed_users_results]
             users_to_notify = list(set(contributed_users + favorited_users))
 
             if len(users_to_notify) != 0:
-                from_user = User.query.get(chat_from)
+                from_user = await UserService.get_user_by_id(chat_from, db)
                 from_user_link = MessageService.get_user_link(from_user.username)
                 project_link = MessageService.get_project_link(
                     project_id, project_name, include_chat_section=True
                 )
                 messages = []
                 for user_id in users_to_notify:
                     try:
-                        user = UserService.get_user_by_id(user_id)
+                        user = await UserService.get_user_by_id(user_id, db)
                     except NotFound:
-                        continue  # If we can't find the user, keep going no need to fail
+                        continue
                     message = Message()
                     message.message_type = MessageType.PROJECT_CHAT_NOTIFICATION.value
                     message.project_id = project_id
@@ -634,8 +642,7 @@ def send_message_after_chat(
                         dict(message=message, user=user, project_name=project_name)
                     )
 
-                # it's important to keep that line inside the if to avoid duplicated emails
-                MessageService._push_messages(messages, db)
+                await MessageService._push_messages(messages, db)
 
     @staticmethod
     async def send_favorite_project_activities(user_id: int):

backend/services/project_search_service.py

@@ -78,6 +78,7 @@ async def create_search_query(db, user=None):
             p.last_updated,
             p.due_date,
             p.country,
+            p.mapping_types,
             o.name AS organisation_name,
             o.logo AS organisation_logo
         FROM projects p
@@ -387,11 +388,14 @@ async def _filter_projects(search_dto: ProjectSearchDTO, user, db: Database):
 
         if search_dto.mapping_types:
             if search_dto.mapping_types_exact:
-                filters.append("p.mapping_types @> :mapping_types")
+                filters.append(
+                    "p.mapping_types @> :mapping_types AND array_length(p.mapping_types, 1) = :mapping_length"
+                )
                 params["mapping_types"] = tuple(
                     MappingTypes[mapping_type].value
                     for mapping_type in search_dto.mapping_types
                 )
+                params["mapping_length"] = len(search_dto.mapping_types)
             else:
                 filters.append("p.mapping_types && :mapping_types")
                 params["mapping_types"] = tuple(

backend/services/stats_service.py

@@ -1,42 +1,42 @@
 import datetime
-from cachetools import TTLCache, cached
 from datetime import date, timedelta
+
+from cachetools import TTLCache, cached
+from databases import Database
 from sqlalchemy import func, or_, select
 
+from backend.db import get_session
 from backend.exceptions import NotFound
+from backend.models.dtos.project_dto import ProjectSearchResultsDTO
 from backend.models.dtos.stats_dto import (
-    ProjectContributionsDTO,
-    UserContribution,
+    CampaignStatsDTO,
+    GenderStatsDTO,
+    HomePageStatsDTO,
+    OrganizationListStatsDTO,
     Pagination,
-    TaskHistoryDTO,
-    TaskStatusDTO,
     ProjectActivityDTO,
+    ProjectContributionsDTO,
     ProjectLastActivityDTO,
-    HomePageStatsDTO,
-    OrganizationListStatsDTO,
-    CampaignStatsDTO,
+    TaskHistoryDTO,
     TaskStats,
     TaskStatsDTO,
-    GenderStatsDTO,
+    TaskStatusDTO,
+    UserContribution,
     UserStatsDTO,
 )
-
-from backend.models.dtos.project_dto import ProjectSearchResultsDTO
 from backend.models.postgis.campaign import Campaign, campaign_projects
 from backend.models.postgis.organisation import Organisation
 from backend.models.postgis.project import Project
-from backend.models.postgis.statuses import TaskStatus, MappingLevel, UserGender
-from backend.models.postgis.task import TaskHistory, User, Task, TaskAction
+from backend.models.postgis.statuses import MappingLevel, TaskStatus, UserGender
+from backend.models.postgis.task import Task, TaskAction, TaskHistory, User
 from backend.models.postgis.utils import timestamp  # noqa: F401
-from backend.services.project_service import ProjectService
+from backend.services.campaign_service import CampaignService
+from backend.services.organisation_service import OrganisationService
 from backend.services.project_search_service import ProjectSearchService
+from backend.services.project_service import ProjectService
 from backend.services.users.user_service import UserService
-from backend.services.organisation_service import OrganisationService
-from backend.services.campaign_service import CampaignService
-from backend.db import get_session
 
 session = get_session()
-from databases import Database
 
 homepage_stats_cache = TTLCache(maxsize=4, ttl=30)
 
@@ -90,15 +90,25 @@ async def _update_tasks_stats(
         action="change",
     ):
         project_stats = dict(project)  # Mutable copy of the project dictionary
-
         if new_state == last_state:
             return project_stats, user
 
         # Increment counters for the new state
         if new_state == TaskStatus.MAPPED:
+            print(type(project_stats["tasks_mapped"]))
+            print(project_stats["tasks_mapped"], "Task mapped before...")
+
             project_stats["tasks_mapped"] += 1
+
+            print(project_stats["tasks_mapped"], "Task mapped after...")
+
         elif new_state == TaskStatus.VALIDATED:
+            print(project_stats["tasks_validated"], "Task validated before...")
+
             project_stats["tasks_validated"] += 1
+
+            print(project_stats["tasks_validated"], "Task validated after...")
+
         elif new_state == TaskStatus.BADIMAGERY:
             project_stats["tasks_bad_imagery"] += 1
 
@@ -113,12 +123,38 @@ async def _update_tasks_stats(
 
         # Decrement counters for the old state
         if last_state == TaskStatus.MAPPED:
+            print(
+                project_stats["tasks_mapped"], "Last state mapped decrement before..."
+            )
+
             project_stats["tasks_mapped"] -= 1
+
+            print(project_stats["tasks_mapped"], "Last state mapped decrement after...")
+
         elif last_state == TaskStatus.VALIDATED:
+            print(
+                project_stats["tasks_mapped"],
+                "Last state validation decrement before...",
+            )
+
             project_stats["tasks_validated"] -= 1
+
+            print(
+                project_stats["tasks_mapped"],
+                "Last state validation decrement after...",
+            )
+
         elif last_state == TaskStatus.BADIMAGERY:
+            print(
+                project_stats["tasks_mapped"], "Last state bad_img decrement before..."
+            )
+
             project_stats["tasks_bad_imagery"] -= 1
 
+            print(
+                project_stats["tasks_mapped"], "Last state bad_img decrement after..."
+            )
+
         # Undo user stats if action is "undo"
         if action == "undo":
             if last_state == TaskStatus.MAPPED: