From 5dcd774d3c80216894419d9fe2b46ae3dedeaeeb Mon Sep 17 00:00:00 2001 From: Sam <78538841+spwoodcock@users.noreply.github.com> Date: Fri, 1 Mar 2024 17:41:02 +0100 Subject: [PATCH] refactor: reduce API calls on home page and project details page (#1315) * refactor: remove verbose logs from task pydantic model * fix(frontend): remove project-summaries call from project details page load * refactor: optimise homepage to use one endpoint (centroids in schema) * refactor: reduce number of endpoint calls on project details page --- src/backend/app/db/postgis_utils.py | 14 ++- src/backend/app/projects/project_crud.py | 2 +- src/backend/app/projects/project_routes.py | 3 +- src/backend/app/projects/project_schemas.py | 14 ++- .../app/submissions/submission_crud.py | 16 ++++ src/backend/app/tasks/tasks_routes.py | 51 +++++----- src/backend/tests/test_projects_routes.py | 4 +- src/frontend/src/api/HomeService.ts | 34 +------ src/frontend/src/api/Project.js | 22 +---- src/frontend/src/components/TasksLayer.jsx | 96 ------------------- .../src/components/home/ProjectListMap.tsx | 2 +- .../createproject/createProjectModel.ts | 2 +- .../src/store/types/ICreateProject.ts | 2 +- src/frontend/src/views/Home.tsx | 2 +- src/frontend/src/views/ProjectDetailsV2.tsx | 16 ++-- 15 files changed, 88 insertions(+), 192 deletions(-) delete mode 100755 src/frontend/src/components/TasksLayer.jsx diff --git a/src/backend/app/db/postgis_utils.py b/src/backend/app/db/postgis_utils.py index 69aa6794a6..9f682c443d 100644 --- a/src/backend/app/db/postgis_utils.py +++ b/src/backend/app/db/postgis_utils.py @@ -64,12 +64,20 @@ def geometry_to_geojson( def get_centroid( - geometry: WKBElement, properties: Optional[dict] = None, id: Optional[int] = None -): - """Convert SQLAlchemy geometry to Centroid GeoJSON.""" + geometry: WKBElement, + properties: Optional[dict] = None, + id: Optional[int] = None, +) -> Union[list[int], Feature]: + """Convert SQLAlchemy geometry to Centroid GeoJSON. + + If no id or properties fields are passed, returns the coordinate only. + Else returns a Feature GeoJSON. + """ if geometry: shape = to_shape(geometry) point = shape.centroid + if not properties and not id: + return point geojson = { "type": "Feature", "geometry": mapping(point), diff --git a/src/backend/app/projects/project_crud.py b/src/backend/app/projects/project_crud.py index a6c585818f..7e18b596c4 100644 --- a/src/backend/app/projects/project_crud.py +++ b/src/backend/app/projects/project_crud.py @@ -1514,7 +1514,7 @@ async def convert_to_app_project(db_project: db_models.DbProject): db_project.outline, {"id": db_project.id}, db_project.id ) - app_project.project_tasks = db_project.tasks + app_project.tasks = db_project.tasks return app_project diff --git a/src/backend/app/projects/project_routes.py b/src/backend/app/projects/project_routes.py index f9e0cc010e..5030aa7980 100644 --- a/src/backend/app/projects/project_routes.py +++ b/src/backend/app/projects/project_routes.py @@ -177,7 +177,7 @@ async def read_project_summaries( @router.get( - "/search_projects", response_model=project_schemas.PaginatedProjectSummaries + "/search-projects", response_model=project_schemas.PaginatedProjectSummaries ) async def search_project( search: str, @@ -1286,6 +1286,7 @@ async def project_dashboard( background_task_id = await project_crud.insert_background_task_into_database( db, "sync_submission", db_project.id ) + # Update submissions in S3 background_tasks.add_task( submission_crud.update_submission_in_s3, db, db_project.id, background_task_id ) diff --git a/src/backend/app/projects/project_schemas.py b/src/backend/app/projects/project_schemas.py index 0a13e15a0b..77b150b12f 100644 --- a/src/backend/app/projects/project_schemas.py +++ b/src/backend/app/projects/project_schemas.py @@ -234,6 +234,7 @@ class ProjectSummary(BaseModel): priority: ProjectPriority = ProjectPriority.MEDIUM priority_str: str = priority.name title: Optional[str] = None + centroid: list[float] location_str: Optional[str] = None description: Optional[str] = None total_tasks: Optional[int] = None @@ -252,11 +253,16 @@ def from_db_project( ) -> "ProjectSummary": """Generate model from database obj.""" priority = project.priority + centroid_point = read_wkb(project.centroid) + # NOTE format x,y (lon,lat) required for GeoJSON + centroid_coords = [centroid_point.x, centroid_point.y] + return cls( id=project.id, priority=priority, priority_str=priority.name, title=project.title, + centroid=centroid_coords, location_str=project.location_str, description=project.description, total_tasks=project.total_tasks, @@ -269,6 +275,12 @@ def from_db_project( organisation_logo=project.organisation_logo, ) + # @field_serializer("centroid") + # def get_coord_from_centroid(self, value): + # """Get the cetroid coordinates from WBKElement.""" + # if value is None: + # return None + class PaginationInfo(BaseModel): """Pagination JSON return.""" @@ -319,7 +331,7 @@ def outline_geojson(self) -> Optional[Feature]: class ProjectWithTasks(ProjectBase): """Project plus list of tasks objects.""" - project_tasks: Optional[List[tasks_schemas.Task]] + tasks: Optional[List[tasks_schemas.Task]] class ProjectOut(ProjectWithTasks): diff --git a/src/backend/app/submissions/submission_crud.py b/src/backend/app/submissions/submission_crud.py index 831038c5d5..2bfd123ea3 100644 --- a/src/backend/app/submissions/submission_crud.py +++ b/src/backend/app/submissions/submission_crud.py @@ -41,6 +41,7 @@ from app.central.central_crud import get_odk_form, get_odk_project, list_odk_xforms from app.config import settings from app.db import db_models +from app.models.enums import HTTPStatus from app.projects import project_crud, project_deps from app.s3 import add_obj_to_bucket, get_obj_from_bucket from app.tasks import tasks_crud @@ -315,6 +316,14 @@ def update_submission_in_s3( odk_credentials = odk_sync(db, project_id) odk_forms = list_odk_xforms(project.odkid, odk_credentials, True) + if not odk_forms: + msg = f"No odk forms returned for project ({project_id})" + log.warning(msg) + return HTTPException( + status_code=HTTPStatus.UNPROCESSABLE_ENTITY, + detail=msg, + ) + # Get latest submission date valid_datetimes = [ form["lastSubmission"] @@ -329,6 +338,13 @@ def update_submission_in_s3( if valid_datetimes else None ) + if not last_submission: + msg = f"Could not identify last submission for project ({project_id})" + log.warning(msg) + return HTTPException( + status_code=HTTPStatus.UNPROCESSABLE_ENTITY, + detail=msg, + ) # Check if the file already exists in s3 s3_project_path = f"/{project.organisation_id}/{project_id}" diff --git a/src/backend/app/tasks/tasks_routes.py b/src/backend/app/tasks/tasks_routes.py index 8534ef05b5..8285be1437 100644 --- a/src/backend/app/tasks/tasks_routes.py +++ b/src/backend/app/tasks/tasks_routes.py @@ -75,30 +75,33 @@ async def read_tasks( return tasks -@router.get("/point_on_surface") -async def get_point_on_surface(project_id: int, db: Session = Depends(database.get_db)): - """Get a point on the surface of the geometry for each task of the project. - - Parameters: - project_id (int): The ID of the project. - - Returns: - List[Tuple[int, str]]: A list of tuples containing the task ID - and the centroid as a string. - """ - query = text( - f""" - SELECT id, - ARRAY_AGG(ARRAY[ST_X(ST_PointOnSurface(outline)), - ST_Y(ST_PointOnSurface(outline))]) AS point - FROM tasks - WHERE project_id = {project_id} - GROUP BY id; """ - ) - - result = db.execute(query) - result_dict_list = [{"id": row[0], "point": row[1]} for row in result.fetchall()] - return result_dict_list +# TODO remove this? Not used anywhere +# @router.get("/point_on_surface") +# async def get_point_on_surface(project_id: int, +# db: Session = Depends(database.get_db)): +# """Get a point on the surface of the geometry for each task of the project. + +# Parameters: +# project_id (int): The ID of the project. + +# Returns: +# List[Tuple[int, str]]: A list of tuples containing the task ID +# and the centroid as a string. +# """ +# query = text( +# f""" +# SELECT id, +# ARRAY_AGG(ARRAY[ST_X(ST_PointOnSurface(outline)), +# ST_Y(ST_PointOnSurface(outline))]) AS point +# FROM tasks +# WHERE project_id = {project_id} +# GROUP BY id; """ +# ) + +# result = db.execute(query) +# result_dict_list = [ +# {"id": row[0], "point": row[1]} for row in result.fetchall()] +# return result_dict_list @router.post("/near_me", response_model=tasks_schemas.Task) diff --git a/src/backend/tests/test_projects_routes.py b/src/backend/tests/test_projects_routes.py index 44c9f36695..34d99dd4da 100644 --- a/src/backend/tests/test_projects_routes.py +++ b/src/backend/tests/test_projects_routes.py @@ -140,8 +140,8 @@ async def test_convert_to_app_project(): assert result.outline_geojson is not None - assert result.project_tasks is not None - assert isinstance(result.project_tasks, list) + assert result.tasks is not None + assert isinstance(result.tasks, list) async def test_create_project_with_project_info(db, project): diff --git a/src/frontend/src/api/HomeService.ts b/src/frontend/src/api/HomeService.ts index b15a519147..cb7b1741a1 100755 --- a/src/frontend/src/api/HomeService.ts +++ b/src/frontend/src/api/HomeService.ts @@ -1,7 +1,5 @@ import axios from 'axios'; import { HomeActions } from '@/store/slices/HomeSlice'; -import { HomeProjectCardModel } from '@/models/home/homeModel'; -import environment from '@/environment'; export const HomeSummaryService: Function = (url: string) => { return async (dispatch) => { @@ -10,19 +8,10 @@ export const HomeSummaryService: Function = (url: string) => { const fetchHomeSummaries = async (url) => { try { const fetchHomeData = await axios.get(url); - const resp: any = fetchHomeData.data.results; + const projectSummaries: any = fetchHomeData.data.results; const paginationResp = fetchHomeData.data.pagination; dispatch(HomeActions.SetHomeProjectPagination(paginationResp)); - const fetchProjectCentroid = await axios.get(`${import.meta.env.VITE_API_URL}/projects/centroid/`); - const projectCentroidResp: any = fetchProjectCentroid.data; - const addedProjectCentroidOnSummary = resp.map((project) => { - const findProjectId = projectCentroidResp.find((payload) => payload.id === project.id); - if (findProjectId) { - return { ...project, centroid: findProjectId.centroid }; - } - return project; - }); - dispatch(HomeActions.SetHomeProjectSummary(addedProjectCentroidOnSummary)); + dispatch(HomeActions.SetHomeProjectSummary(projectSummaries)); dispatch(HomeActions.HomeProjectLoading(false)); } catch (error) { dispatch(HomeActions.HomeProjectLoading(false)); @@ -32,22 +21,3 @@ export const HomeSummaryService: Function = (url: string) => { await fetchHomeSummaries(url); }; }; -export const ProjectCentroidService: Function = (url: string) => { - return async (dispatch) => { - dispatch(HomeActions.SetProjectCentroidLoading(true)); - - const fetchProjectCentroid = async (url) => { - try { - const fetchHomeData = await axios.get(url); - const resp: HomeProjectCardModel = fetchHomeData.data; - - dispatch(HomeActions.SetProjectCentroid(resp)); - dispatch(HomeActions.SetProjectCentroidLoading(false)); - } catch (error) { - dispatch(HomeActions.SetProjectCentroidLoading(false)); - } - }; - - await fetchProjectCentroid(url); - }; -}; diff --git a/src/frontend/src/api/Project.js b/src/frontend/src/api/Project.js index 769b03f5ec..1e3e738617 100755 --- a/src/frontend/src/api/Project.js +++ b/src/frontend/src/api/Project.js @@ -9,15 +9,8 @@ export const ProjectById = (existingProjectList, projectId) => { try { dispatch(ProjectActions.SetProjectDetialsLoading(true)); const project = await CoreModules.axios.get(`${import.meta.env.VITE_API_URL}/projects/${projectId}`); - const taskList = await CoreModules.axios.get( - `${import.meta.env.VITE_API_URL}/tasks/task-list?project_id=${projectId}`, - ); - const taskBbox = await CoreModules.axios.get( - `${import.meta.env.VITE_API_URL}/tasks/point_on_surface?project_id=${projectId}`, - ); const projectResp = project.data; - const taskListResp = taskList.data; - const persistingValues = taskListResp.map((data) => { + const persistingValues = projectResp.tasks.map((data) => { return { id: data.id, outline_geojson: data.outline_geojson, @@ -29,18 +22,9 @@ export const ProjectById = (existingProjectList, projectId) => { odk_token: data.odk_token, }; }); - // added centroid from another api to projecttaskboundries + // At top level id project id to object const projectTaskBoundries = [{ id: projectResp.id, taskBoundries: persistingValues }]; - const mergedBboxIntoTask = projectTaskBoundries[0].taskBoundries.map((projectTask) => { - const filteredTaskIdCentroid = taskBbox.data.find((task) => task.id === projectTask.id).point[0]; - return { - ...projectTask, - bbox: filteredTaskIdCentroid, - }; - }); - dispatch( - ProjectActions.SetProjectTaskBoundries([{ ...projectTaskBoundries[0], taskBoundries: mergedBboxIntoTask }]), - ); + dispatch(ProjectActions.SetProjectTaskBoundries([{ ...projectTaskBoundries[0] }])); dispatch( ProjectActions.SetProjectInfo({ id: projectResp.id, diff --git a/src/frontend/src/components/TasksLayer.jsx b/src/frontend/src/components/TasksLayer.jsx deleted file mode 100755 index 7dfab9bf63..0000000000 --- a/src/frontend/src/components/TasksLayer.jsx +++ /dev/null @@ -1,96 +0,0 @@ -import React, { useEffect } from 'react'; -import { Vector as VectorLayer } from 'ol/layer.js'; -import GeoJSON from 'ol/format/GeoJSON'; -import { Vector as VectorSource } from 'ol/source.js'; -import { geojsonObjectModel } from '@/models/geojsonObjectModel'; -import MapStyles from '@/hooks/MapStyles'; -import environment from '@/environment'; -import CoreModules from '@/shared/CoreModules'; -import { get } from 'ol/proj'; -let geojsonObject; -const TasksLayer = (map, view, feature) => { - const params = CoreModules.useParams(); - const state = CoreModules.useAppSelector((state) => state.project); - const geojsonStyles = MapStyles(feature); - - useEffect(() => { - if (state.projectTaskBoundries.length != 0 && map != undefined) { - if (state.projectTaskBoundries.findIndex((project) => project.id == environment.decode(params.id)) != -1) { - geojsonObject = null; - const index = state.projectTaskBoundries.findIndex((project) => project.id == environment.decode(params.id)); - - const styleFunction = function (feature) { - let id = feature.getId().toString().replace('_', ','); - geojsonStyles[id.split(',')[1]]; - return geojsonStyles[id.split(',')[1]]; - }; - - geojsonObject = { ...geojsonObjectModel }; - geojsonObject['features'] = []; - state.projectTaskBoundries[index].taskBoundries.forEach((task) => { - geojsonObject['features'].push({ - id: `${task.id}_${task.task_status}`, - type: task.outline_geojson.type, - geometry: task.outline_geojson.geometry, - properties: { centroid: task.bbox }, - }); - }); - const vectorSource = new VectorSource({ - features: new GeoJSON().readFeatures(geojsonObject, { - featureProjection: get('EPSG:3857'), - }), - }); - - const vectorLayer = new VectorLayer({ - source: vectorSource, - style: styleFunction, - zIndex: 10, - }); - vectorLayer.setProperties({ name: 'project-area' }); - - // Initialize variables to store the extent - var minX = Infinity; - var minY = Infinity; - var maxX = -Infinity; - var maxY = -Infinity; - - // Iterate through the features and calculate the extent - vectorSource.getFeatures().forEach(function (feature) { - var geometry = feature.getGeometry(); - var extent = geometry.getExtent(); - - minX = Math.min(minX, extent[0]); - minY = Math.min(minY, extent[1]); - maxX = Math.max(maxX, extent[2]); - maxY = Math.max(maxY, extent[3]); - }); - - // The extent of the vector layer - var extent = [minX, minY, maxX, maxY]; - - // Checking if Taskboundaries exist if doesn't exist dont throw error window - const checkForInfinity = extent.some((ext) => ext === Infinity); - if (!checkForInfinity) { - map.getView().fit(extent, { - duration: 2000, // Animation duration in milliseconds - padding: [50, 50, 50, 200], // Optional padding around the extent - }); - } - map.addLayer(vectorLayer); - map.on('loadend', function () { - map.getTargetElement().classList.remove('spinner'); - }); - } - } - }, [state.newProjectTrigger, map]); - - // useEffect(() => { - - // if (state.projectTaskBoundries.length != 0 && map != undefined) { - // if (state.projectTaskBoundries.findIndex(project => project.id == environment.decode(params.id)) != -1) { - // } - // } - // }, [map]) -}; - -export default TasksLayer; diff --git a/src/frontend/src/components/home/ProjectListMap.tsx b/src/frontend/src/components/home/ProjectListMap.tsx index 58d11c716d..1cd903d538 100644 --- a/src/frontend/src/components/home/ProjectListMap.tsx +++ b/src/frontend/src/components/home/ProjectListMap.tsx @@ -80,7 +80,7 @@ const ProjectListMap = () => { }, geometry: { type: 'Point', - coordinates: project.centroid[0], + coordinates: project.centroid, }, })), }; diff --git a/src/frontend/src/models/createproject/createProjectModel.ts b/src/frontend/src/models/createproject/createProjectModel.ts index 550ad96cf5..8035e0aa3b 100755 --- a/src/frontend/src/models/createproject/createProjectModel.ts +++ b/src/frontend/src/models/createproject/createProjectModel.ts @@ -23,7 +23,7 @@ export interface ProjectDetailsModel { id: string; bbox: [string, string, string, string]; }; - project_tasks: { + tasks: { id: number; project_id: number; project_task_index: number; diff --git a/src/frontend/src/store/types/ICreateProject.ts b/src/frontend/src/store/types/ICreateProject.ts index e54c874393..29439c5927 100644 --- a/src/frontend/src/store/types/ICreateProject.ts +++ b/src/frontend/src/store/types/ICreateProject.ts @@ -84,7 +84,7 @@ type EditProjectResponseTypes = { project_info: ProjectInfoTypes[]; status: number; outline_geojson: GeoJSONFeatureTypes; - project_tasks: ProjectTaskTypes[]; + tasks: ProjectTaskTypes[]; xform_title: string; hashtags: string[]; }; diff --git a/src/frontend/src/views/Home.tsx b/src/frontend/src/views/Home.tsx index 77b1bfac2d..cf38388a32 100755 --- a/src/frontend/src/views/Home.tsx +++ b/src/frontend/src/views/Home.tsx @@ -50,7 +50,7 @@ const Home = () => { HomeSummaryService( `${ import.meta.env.VITE_API_URL - }/projects/search_projects?page=${paginationPage}&results_per_page=12&search=${debouncedSearch}`, + }/projects/search-projects?page=${paginationPage}&results_per_page=12&search=${debouncedSearch}`, ), ); } diff --git a/src/frontend/src/views/ProjectDetailsV2.tsx b/src/frontend/src/views/ProjectDetailsV2.tsx index 5228f889aa..db67a2ff80 100644 --- a/src/frontend/src/views/ProjectDetailsV2.tsx +++ b/src/frontend/src/views/ProjectDetailsV2.tsx @@ -122,15 +122,17 @@ const Home = () => { useEffect(() => { if (!map) return; - const features = state.projectTaskBoundries[0]?.taskBoundries?.map((feature) => ({ + const features = state.projectTaskBoundries[0]?.taskBoundries?.map((taskObj) => ({ type: 'Feature', - geometry: { ...feature.outline_geojson.geometry }, + geometry: { ...taskObj.outline_geojson.geometry }, properties: { - ...feature.outline_geojson.properties, - centroid: feature.bbox, + ...taskObj.outline_geojson.properties, + centroid: taskObj.outline_centroid.geometry.coordinates, + // TODO add bbox field here too? }, - id: `${feature.id}_${feature.task_status}`, + id: `${taskObj.id}_${taskObj.task_status}`, })); + const taskBoundariesFeatcol = { ...geojsonObjectModel, features: features, @@ -138,10 +140,6 @@ const Home = () => { setTaskBoundariesLayer(taskBoundariesFeatcol); }, [state.projectTaskBoundries[0]?.taskBoundries?.length]); - useEffect(() => { - dispatch(GetProjectDashboard(`${import.meta.env.VITE_API_URL}/projects/project_dashboard/${decodedId}`)); - }, []); - const dataExtractDataPopup = (properties: dataExtractPropertyType) => { return (