From 68ca69c3af4431930e8c77d5eae2f88f8337f7e6 Mon Sep 17 00:00:00 2001 From: prabinoid Date: Tue, 3 Sep 2024 10:18:29 +0545 Subject: [PATCH] Project detail summary and user touched projects api refactored --- backend/api/projects/resources.py | 14 ++-- backend/models/dtos/user_dto.py | 23 ++--- backend/models/postgis/project.py | 27 +++--- backend/models/postgis/project_info.py | 6 +- backend/models/postgis/user.py | 112 +++++++++++-------------- backend/services/project_service.py | 67 ++++++++++++--- backend/services/users/user_service.py | 11 ++- 7 files changed, 145 insertions(+), 115 deletions(-) diff --git a/backend/api/projects/resources.py b/backend/api/projects/resources.py index e52c7652fd..9c07b4b788 100644 --- a/backend/api/projects/resources.py +++ b/backend/api/projects/resources.py @@ -843,12 +843,10 @@ async def get(request: Request, db: Database = Depends(get_db), user: AuthUserDT search_dto, db ) - # return admin_projects.model_dump(by_alias=True), 200 return admin_projects -# class ProjectsQueriesTouchedAPI(): @router.get("/queries/{username}/touched/") -async def get(request: Request, username): +async def get(request: Request, username, db: Database = Depends(get_db)): """ Gets projects user has mapped --- @@ -882,12 +880,12 @@ async def get(request: Request, username): if request.headers.get("accept-language") else "en" ) - user_dto = UserService.get_mapped_projects(username, locale) - return user_dto.model_dump(by_alias=True), 200 + user_dto = await UserService.get_mapped_projects(username, locale, db) + return user_dto @router.get("/{project_id}/queries/summary/") -async def get(request: Request, project_id: int): +async def get(request: Request, project_id: int, db: Database = Depends(get_db)): """ Gets project summary --- @@ -917,8 +915,8 @@ async def get(request: Request, project_id: int): description: Internal Server Error """ preferred_locale = request.headers.get("accept-language") - summary = ProjectService.get_project_summary(project_id, preferred_locale) - return summary.model_dump(by_alias=True), 200 + summary = await ProjectService.get_project_summary(project_id, db, preferred_locale) + return summary @router.get("/{project_id}/queries/nogeometries/") diff --git a/backend/models/dtos/user_dto.py b/backend/models/dtos/user_dto.py index 1c062cb7be..24117eb762 100644 --- a/backend/models/dtos/user_dto.py +++ b/backend/models/dtos/user_dto.py @@ -142,26 +142,27 @@ class UserOSMDTO(BaseModel): account_created: Optional[str] = Field(None, alias="accountCreated") changeset_count: Optional[int] = Field(None, alias="changesetCount") - class MappedProject(BaseModel): """Describes a single project a user has mapped""" - project_id: int = Field(alias="projectId") - name: str - tasks_mapped: int = Field(alias="tasksMapped") - tasks_validated: int = Field(alias="tasksValidated") - status: str - centroid: str + project_id: Optional[int] = Field(None, alias="projectId") + name: Optional[str] = None + tasks_mapped: Optional[int] = Field(None, alias="tasksMapped") + tasks_validated: Optional[int] = Field(None, alias="tasksValidated") + status: Optional[str] = None + centroid: Optional[str] = None + + class Config: + populate_by_name = True class UserMappedProjectsDTO(BaseModel): """DTO for projects a user has mapped""" - def __init__(self): - super().__init__() - self.mapped_projects = [] + mapped_projects: Optional[List[MappedProject]] = Field(default_factory=list, alias="mappedProjects") - mapped_projects: List[MappedProject] = Field(alias="mappedProjects") + class Config: + populate_by_name = True class UserSearchQuery(BaseModel): diff --git a/backend/models/postgis/project.py b/backend/models/postgis/project.py index 4f6ddb4a52..a5c976f180 100644 --- a/backend/models/postgis/project.py +++ b/backend/models/postgis/project.py @@ -600,6 +600,7 @@ async def get_projects_for_admin( admin_id: int, preferred_locale: str, search_dto: ProjectSearchDTO, db: Database ) -> PMDashboardDTO: """Get all projects for provided admin.""" + query = """ SELECT p.id AS id, @@ -1010,20 +1011,22 @@ async def get_project_summary( summary.project_info = project_info return summary + - @staticmethod - async def calculate_tasks_percent(status: str, project_id: int, db: Database) -> float: - """Calculate the percentage of tasks with a given status for a project.""" - query = f""" - SELECT COUNT(*) - FROM tasks - WHERE project_id = :project_id AND status = :status - """ - total_tasks_query = "SELECT COUNT(*) FROM tasks WHERE project_id = :project_id" + #TODO Remove if not used. + # @staticmethod + # async def calculate_tasks_percent(status: str, project_id: int, db: Database) -> float: + # """Calculate the percentage of tasks with a given status for a project.""" + # query = f""" + # SELECT COUNT(*) + # FROM tasks + # WHERE project_id = :project_id AND status = :status + # """ + # total_tasks_query = "SELECT COUNT(*) FROM tasks WHERE project_id = :project_id" - total_tasks = await db.fetch_val(total_tasks_query, {"project_id": project_id}) - status_tasks = await db.fetch_val(query, {"project_id": project_id, "status": status}) - return (status_tasks / total_tasks) * 100 if total_tasks > 0 else 0.0 + # total_tasks = await db.fetch_val(total_tasks_query, {"project_id": project_id}) + # status_tasks = await db.fetch_val(query, {"project_id": project_id, "status": status}) + # return (status_tasks / total_tasks) * 100 if total_tasks > 0 else 0.0 @staticmethod diff --git a/backend/models/postgis/project_info.py b/backend/models/postgis/project_info.py index 2f61957032..1b4791984f 100644 --- a/backend/models/postgis/project_info.py +++ b/backend/models/postgis/project_info.py @@ -116,9 +116,9 @@ async def get_dto_for_locale( if default_locale_info is None: error_message = f"BAD DATA: no info for project {project_id}, locale: {locale}, default {default_locale}" raise ValueError(error_message) - - # Pass through default_locale in case of partial translation - return ProjectInfoDTO(**project_info, **default_locale_info) + + combined_info = {**default_locale_info, **project_info} + return ProjectInfoDTO(**combined_info) # def get_dto(self, default_locale=ProjectInfoDTO()) -> ProjectInfoDTO: # """ diff --git a/backend/models/postgis/user.py b/backend/models/postgis/user.py index e42ed3a294..4294a9fa96 100644 --- a/backend/models/postgis/user.py +++ b/backend/models/postgis/user.py @@ -25,6 +25,7 @@ UserRole, UserGender, ) + from backend.models.postgis.utils import timestamp from backend.models.postgis.interests import Interest, user_interests from backend.db import Base, get_session @@ -264,88 +265,69 @@ def upsert_mapped_projects(user_id: int, project_id: int): user.projects_mapped.append(project_id) session.commit() + #TODO Optimization: Get only project name instead of all the locale attributes. @staticmethod - def get_mapped_projects( - user_id: int, preferred_locale: str + async def get_mapped_projects( + user_id: int, preferred_locale: str, db: Database ) -> UserMappedProjectsDTO: """Get all projects a user has mapped on""" - from backend.models.postgis.task import Task - from backend.models.postgis.project import Project + # Subquery for validated tasks + query_validated = """ + SELECT project_id, COUNT(validated_by) AS validated + FROM tasks + WHERE project_id IN ( + SELECT unnest(projects_mapped) FROM users WHERE id = :user_id + ) AND validated_by = :user_id + GROUP BY project_id, validated_by + """ - query = session.query(func.unnest(User.projects_mapped)).filter_by( - id=user_id - ) - query_validated = ( - session.query( - Task.project_id.label("project_id"), - func.count(Task.validated_by).label("validated"), - ) - .filter(Task.project_id.in_(query)) - .filter_by(validated_by=user_id) - .group_by(Task.project_id, Task.validated_by) - .subquery() - ) + # Subquery for mapped tasks + query_mapped = """ + SELECT project_id, COUNT(mapped_by) AS mapped + FROM tasks + WHERE project_id IN ( + SELECT unnest(projects_mapped) FROM users WHERE id = :user_id + ) AND mapped_by = :user_id + GROUP BY project_id, mapped_by + """ - query_mapped = ( - session.query( - Task.project_id.label("project_id"), - func.count(Task.mapped_by).label("mapped"), - ) - .filter(Task.project_id.in_(query)) - .filter_by(mapped_by=user_id) - .group_by(Task.project_id, Task.mapped_by) - .subquery() - ) + # Union of validated and mapped tasks + query_union = f""" + SELECT COALESCE(v.project_id, m.project_id) AS project_id, + COALESCE(v.validated, 0) AS validated, + COALESCE(m.mapped, 0) AS mapped + FROM ({query_validated}) v + FULL OUTER JOIN ({query_mapped}) m + ON v.project_id = m.project_id + """ - query_union = ( - session.query( - func.coalesce( - query_validated.c.project_id, query_mapped.c.project_id - ).label("project_id"), - func.coalesce(query_validated.c.validated, 0).label("validated"), - func.coalesce(query_mapped.c.mapped, 0).label("mapped"), - ) - .join( - query_mapped, - query_validated.c.project_id == query_mapped.c.project_id, - full=True, - ) - .subquery() - ) + # Main query to get project details + query_projects = f""" + SELECT p.id, p.status, p.default_locale, u.mapped, u.validated, ST_AsGeoJSON(p.centroid) AS centroid + FROM projects p + JOIN ({query_union}) u ON p.id = u.project_id + ORDER BY p.id DESC + """ - results = ( - session.query( - Project.id, - Project.status, - Project.default_locale, - query_union.c.mapped, - query_union.c.validated, - functions.ST_AsGeoJSON(Project.centroid), - ) - .filter(Project.id == query_union.c.project_id) - .order_by(desc(Project.id)) - .all() - ) + results = await db.fetch_all(query_projects, {"user_id": user_id}) mapped_projects_dto = UserMappedProjectsDTO() for row in results: mapped_project = MappedProject() - mapped_project.project_id = row[0] - mapped_project.status = ProjectStatus(row[1]).name - mapped_project.tasks_mapped = row[3] - mapped_project.tasks_validated = row[4] - mapped_project.centroid = geojson.loads(row[5]) - - project_info = ProjectInfo.get_dto_for_locale( - row[0], preferred_locale, row[2] + mapped_project.project_id = row["id"] + mapped_project.status = ProjectStatus(row["status"]).name + mapped_project.tasks_mapped = row["mapped"] + mapped_project.tasks_validated = row["validated"] + mapped_project.centroid = geojson.loads(row["centroid"]) + project_info = await ProjectInfo.get_dto_for_locale( + db, row["id"], preferred_locale, row["default_locale"] ) mapped_project.name = project_info.name - mapped_projects_dto.mapped_projects.append(mapped_project) - return mapped_projects_dto + def set_user_role(self, role: UserRole): """Sets the supplied role on the user""" self.role = role.value diff --git a/backend/services/project_service.py b/backend/services/project_service.py index 31fc265873..6544a4e502 100644 --- a/backend/services/project_service.py +++ b/backend/services/project_service.py @@ -500,6 +500,8 @@ def is_user_permitted_to_validate(project_id, user_id): return True, "User allowed to validate" + + #TODO: Implement Caching. @staticmethod @cached(summary_cache) def get_cached_project_summary( @@ -510,27 +512,72 @@ def get_cached_project_summary( # We don't want to cache the project stats, so we set calculate_completion to False return project.get_project_summary(preferred_locale, calculate_completion=False) + @staticmethod - def get_project_summary( - project_id: int, preferred_locale: str = "en" + async def get_project_summary( + project_id: int, db: Database, preferred_locale: str = "en" ) -> ProjectSummary: + query = """ + SELECT + p.id AS id, + p.difficulty, + p.priority, + p.default_locale, + ST_AsGeoJSON(p.centroid) AS centroid, + p.organisation_id, + p.tasks_bad_imagery, + p.tasks_mapped, + p.tasks_validated, + p.status, + p.mapping_types, + p.total_tasks, + p.last_updated, + p.due_date, + p.country, + p.changeset_comment, + p.created, + p.osmcha_filter_id, + p.mapping_permission, + p.validation_permission, + p.enforce_random_task_selection, + p.private, + p.license_id, + p.id_presets, + p.extra_id_params, + p.rapid_power_user, + p.imagery, + p.mapping_editors, + p.validation_editors, + u.username AS author, + o.name AS organisation_name, + o.slug AS organisation_slug, + o.logo AS organisation_logo, + ARRAY(SELECT user_id FROM project_allowed_users WHERE project_id = p.id) AS allowed_users + FROM projects p + LEFT JOIN organisations o ON o.id = p.organisation_id + LEFT JOIN users u ON u.id = p.author_id + WHERE p.id = :id + """ + params = {'id': project_id} + # Execute query + project = await db.fetch_one(query, params) + """Gets the project summary DTO""" - project = ProjectService.get_project_by_id(project_id) - summary = ProjectService.get_cached_project_summary( - project_id, preferred_locale - ) - # Since we don't want to cache the project stats, we need to update them - summary.percent_mapped = project.calculate_tasks_percent("mapped") - summary.percent_validated = project.calculate_tasks_percent("validated") - summary.percent_bad_imagery = project.calculate_tasks_percent("bad_imagery") + + summary = await Project.get_project_summary(project, preferred_locale, db, calculate_completion=False) + summary.percent_mapped = Project.calculate_tasks_percent("mapped", project.tasks_mapped, project.tasks_validated, project.total_tasks, project.tasks_bad_imagery) + summary.percent_validated = Project.calculate_tasks_percent("validated", project.tasks_validated, project.tasks_validated, project.total_tasks, project.tasks_bad_imagery) + summary.percent_bad_imagery = Project.calculate_tasks_percent("bad_imagery", project.tasks_mapped, project.tasks_validated, project.total_tasks, project.tasks_bad_imagery) return summary + @staticmethod def set_project_as_featured(project_id: int): """Sets project as featured""" project = ProjectService.get_project_by_id(project_id) project.set_as_featured() + @staticmethod def unset_project_as_featured(project_id: int): """Sets project as featured""" diff --git a/backend/services/users/user_service.py b/backend/services/users/user_service.py index 62db7bb224..9637479f61 100644 --- a/backend/services/users/user_service.py +++ b/backend/services/users/user_service.py @@ -60,8 +60,8 @@ async def get_user_by_id(user_id: int, db: Database) -> User: return user @staticmethod - def get_user_by_username(username: str, db) -> User: - user = User.get_by_username(username, db) + async def get_user_by_username(username: str, db) -> User: + user = await User.get_by_username(username, db) if user is None: raise NotFound(sub_code="USER_NOT_FOUND", username=username) @@ -602,7 +602,6 @@ def get_countries_contributed(user_id: int): countries_dto = UserCountriesContributed() countries_dto.countries_contributed = result countries_dto.total = len(result) - return countries_dto @staticmethod @@ -611,10 +610,10 @@ def upsert_mapped_projects(user_id: int, project_id: int): User.upsert_mapped_projects(user_id, project_id) @staticmethod - def get_mapped_projects(user_name: str, preferred_locale: str): + async def get_mapped_projects(user_name: str, preferred_locale: str, db: Database): """Gets all projects a user has mapped or validated on""" - user = UserService.get_user_by_username(user_name) - return User.get_mapped_projects(user.id, preferred_locale) + user = await UserService.get_user_by_username(user_name, db) + return await User.get_mapped_projects(user.id, preferred_locale, db) @staticmethod def get_recommended_projects(user_name: str, preferred_locale: str):