From 054a85c963c5f45d3b94f2e402d6b1c05753fd02 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Mon, 23 Sep 2024 20:46:57 +0100 Subject: [PATCH 1/7] docs: add brief bit of info about broken prod and what to do! --- docs/dev/Production.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/docs/dev/Production.md b/docs/dev/Production.md index ea603fd723..1e9c0deab9 100644 --- a/docs/dev/Production.md +++ b/docs/dev/Production.md @@ -209,3 +209,36 @@ pg_restore --verbose -U fmtm -d fmtm # Run the entire docker compose stack docker compose -f docker-compose.$GIT_BRANCH.yml up -d ``` + +## Help! FMTM Prod Is Broken 😨 + +### Debugging + +- Log into the production server, fmtm.hotosm.org and view the container logs: + + ```bash + docker logs fmtm-main-api-1 + docker logs fmtm-main-api-2 + docker logs fmtm-main-api-3 + docker logs fmtm-main-api-4 + ``` + + They often provide useful traceback information, including timestamps. + +- View error reports on Sentry: + You can get very detailed tracebacks here, even with the SQL executed on + the database amongst other things. + +- Reproduce the error on your local machine! + +### Making a hotfix + +- Sometimes fixes just can't wait to go through the development --> + staging --> production cycle. We need the fix now! + +- In this case, a `hotfix` can be made directly to the `main` branch: + - Create a branch `hotfix/something-i-fixed`, add your code and test + it works locally. + - Push your branch, then create a PR against the `main` branch in Github. + - Merge in the PR and wait for the deployment. + - Later the code can be pulled back into develop / staging. From 3fbd840b0a322cdfcc9771d5f30971c650c29e47 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Mon, 23 Sep 2024 20:47:52 +0100 Subject: [PATCH 2/7] docs: update info about production debugging --- docs/dev/Production.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/dev/Production.md b/docs/dev/Production.md index 1e9c0deab9..655c09a0b6 100644 --- a/docs/dev/Production.md +++ b/docs/dev/Production.md @@ -223,6 +223,9 @@ docker compose -f docker-compose.$GIT_BRANCH.yml up -d docker logs fmtm-main-api-4 ``` + > Note there are four replica containers running, and any one of them + > could have handled the request. You should check them all. + They often provide useful traceback information, including timestamps. - View error reports on Sentry: From f2ae207934cddd3f2b6dd6ce0e84a7d34571cef1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 23 Sep 2024 22:57:31 +0100 Subject: [PATCH 3/7] [pre-commit.ci] pre-commit autoupdate (#1803) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [pre-commit.ci] pre-commit autoupdate updates: - [github.com/astral-sh/ruff-pre-commit: v0.6.5 → v0.6.7](https://github.com/astral-sh/ruff-pre-commit/compare/v0.6.5...v0.6.7) - [github.com/sqlfluff/sqlfluff: 3.1.1 → 3.2.0](https://github.com/sqlfluff/sqlfluff/compare/3.1.1...3.2.0) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 4 ++-- .../archived/2024-02-24/003-project-roles.sql | 4 ++-- .../archived/2024-02-24/revert/003-project-roles.sql | 4 ++-- src/backend/migrations/init/fmtm_base_schema.sql | 12 ++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d8e672d123..7e8bcf3d4a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,7 +9,7 @@ repos: # Lint / autoformat: Python code - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: "v0.6.5" + rev: "v0.6.7" hooks: # Run the linter - id: ruff @@ -71,7 +71,7 @@ repos: # Lint & Autoformat: SQL - repo: https://github.com/sqlfluff/sqlfluff - rev: 3.1.1 + rev: 3.2.0 hooks: - id: sqlfluff-lint files: ^src/backend/migrations/(?:.*/)*.*$ diff --git a/src/backend/migrations/archived/2024-02-24/003-project-roles.sql b/src/backend/migrations/archived/2024-02-24/003-project-roles.sql index 75313f553b..213bb07128 100644 --- a/src/backend/migrations/archived/2024-02-24/003-project-roles.sql +++ b/src/backend/migrations/archived/2024-02-24/003-project-roles.sql @@ -21,10 +21,10 @@ END $$; ALTER TYPE public.projectrole OWNER TO fmtm; ALTER TABLE public.user_roles -ALTER COLUMN "role" TYPE VARCHAR(24); +ALTER COLUMN role TYPE VARCHAR(24); ALTER TABLE public.user_roles -ALTER COLUMN "role" TYPE public.projectrole USING role::public.projectrole; +ALTER COLUMN role TYPE public.projectrole USING role::public.projectrole; -- Commit the transaction COMMIT; diff --git a/src/backend/migrations/archived/2024-02-24/revert/003-project-roles.sql b/src/backend/migrations/archived/2024-02-24/revert/003-project-roles.sql index 72e6ad5d84..930bc17dbb 100644 --- a/src/backend/migrations/archived/2024-02-24/revert/003-project-roles.sql +++ b/src/backend/migrations/archived/2024-02-24/revert/003-project-roles.sql @@ -2,9 +2,9 @@ BEGIN; -- Revert user_roles table changes -ALTER TABLE public.user_roles ALTER COLUMN "role" TYPE VARCHAR(24); +ALTER TABLE public.user_roles ALTER COLUMN role TYPE VARCHAR(24); -ALTER TABLE public.user_roles ALTER COLUMN "role" TYPE public.userrole +ALTER TABLE public.user_roles ALTER COLUMN role TYPE public.userrole USING role::public.userrole; -- Drop the public.projectrole enum diff --git a/src/backend/migrations/init/fmtm_base_schema.sql b/src/backend/migrations/init/fmtm_base_schema.sql index c556d1f5b4..17f20b4d39 100644 --- a/src/backend/migrations/init/fmtm_base_schema.sql +++ b/src/backend/migrations/init/fmtm_base_schema.sql @@ -155,11 +155,11 @@ SET default_table_access_method = heap; -- Tables -CREATE TABLE IF NOT EXISTS public."_migrations" ( +CREATE TABLE IF NOT EXISTS public._migrations ( date_executed TIMESTAMP, script_name TEXT ); -ALTER TABLE public."_migrations" OWNER TO fmtm; +ALTER TABLE public._migrations OWNER TO fmtm; CREATE TABLE public.background_tasks ( @@ -193,11 +193,11 @@ ALTER TABLE public.mbtiles_path_id_seq OWNER TO fmtm; ALTER SEQUENCE public.mbtiles_path_id_seq OWNED BY public.mbtiles_path.id; -CREATE TABLE public."_migrations" ( +CREATE TABLE public._migrations ( script_name text, date_executed timestamp without time zone ); -ALTER TABLE public."_migrations" OWNER TO fmtm; +ALTER TABLE public._migrations OWNER TO fmtm; CREATE TABLE public.organisation_managers ( @@ -436,8 +436,8 @@ ALTER TABLE ONLY public.submission_photos ALTER COLUMN id SET DEFAULT nextval( -- Constraints for primary keys -ALTER TABLE public."_migrations" -ADD CONSTRAINT "_migrations_pkey" PRIMARY KEY (script_name); +ALTER TABLE public._migrations +ADD CONSTRAINT _migrations_pkey PRIMARY KEY (script_name); ALTER TABLE ONLY public.background_tasks ADD CONSTRAINT background_tasks_pkey PRIMARY KEY (id); From d368f03079b9eca3b67479bf3dc9514b4369d36c Mon Sep 17 00:00:00 2001 From: Sam <78538841+spwoodcock@users.noreply.github.com> Date: Mon, 23 Sep 2024 22:59:46 +0100 Subject: [PATCH 4/7] refactor(backend): remove task_id and task_filter params from XLSForm (#1805) * refactor: backend do not require adding task_count to xlsform modification * refactor(frontend): no longer add task_filter param to odk form intent * refactor: frontend remove task_filter logic as no longer in xlsform * refactor(frontend): renamed new_feature_point --> new_feature in xlsform * build: update osm-fieldwork --> 0.16.5 for latest xlsforms --- src/backend/app/central/central_crud.py | 6 ------ src/backend/app/projects/project_routes.py | 5 ----- src/backend/pdm.lock | 8 ++++---- src/backend/pyproject.toml | 2 +- src/frontend/src/components/DialogTaskActions.tsx | 2 +- src/frontend/src/views/SubmissionDetails.tsx | 10 +++------- 6 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/backend/app/central/central_crud.py b/src/backend/app/central/central_crud.py index 0896509835..22945aa9ed 100644 --- a/src/backend/app/central/central_crud.py +++ b/src/backend/app/central/central_crud.py @@ -321,7 +321,6 @@ async def append_fields_to_user_xlsform( xlsform: BytesIO, form_category: str = "buildings", additional_entities: list[str] = None, - task_count: int = None, existing_id: str = None, ) -> tuple[str, BytesIO]: """Helper to return the intermediate XLSForm prior to convert.""" @@ -330,7 +329,6 @@ async def append_fields_to_user_xlsform( xlsform, form_category=form_category, additional_entities=additional_entities, - task_count=task_count, existing_id=existing_id, ) @@ -339,7 +337,6 @@ async def validate_and_update_user_xlsform( xlsform: BytesIO, form_category: str = "buildings", additional_entities: list[str] = None, - task_count: int = None, existing_id: str = None, ) -> BytesIO: """Wrapper to append mandatory fields and validate user uploaded XLSForm.""" @@ -347,7 +344,6 @@ async def validate_and_update_user_xlsform( xlsform, form_category=form_category, additional_entities=additional_entities, - task_count=task_count, existing_id=existing_id, ) @@ -361,7 +357,6 @@ async def update_project_xform( odk_id: int, xlsform: BytesIO, category: str, - task_count: int, odk_credentials: project_schemas.ODKCentralDecrypted, ) -> None: """Update and publish the XForm for a project. @@ -371,7 +366,6 @@ async def update_project_xform( odk_id (int): ODK Central form ID. xlsform (UploadFile): XForm data. category (str): Category of the XForm. - task_count (int): The number of tasks in a project. odk_credentials (project_schemas.ODKCentralDecrypted): ODK Central creds. Returns: None diff --git a/src/backend/app/projects/project_routes.py b/src/backend/app/projects/project_routes.py index a16dcf86f8..1618119393 100644 --- a/src/backend/app/projects/project_routes.py +++ b/src/backend/app/projects/project_routes.py @@ -675,7 +675,6 @@ async def validate_form( if debug: xform_id, updated_form = await central_crud.append_fields_to_user_xlsform( xlsform, - task_count=1, # NOTE this must be included to append task_filter choices ) return StreamingResponse( updated_form, @@ -687,7 +686,6 @@ async def validate_form( else: await central_crud.validate_and_update_user_xlsform( xlsform, - task_count=1, # NOTE this must be included to append task_filter choices ) return JSONResponse( status_code=HTTPStatus.OK, @@ -735,7 +733,6 @@ async def generate_files( project = project_user_dict.get("project") project_id = project.id form_category = project.xform_category - task_count = len(project.tasks) log.debug(f"Generating additional files for project: {project.id}") @@ -746,7 +743,6 @@ async def generate_files( await central_crud.validate_and_update_user_xlsform( xlsform=xlsform_upload, form_category=form_category, - task_count=task_count, additional_entities=additional_entities, ) xlsform = xlsform_upload @@ -762,7 +758,6 @@ async def generate_files( xform_id, project_xlsform = await central_crud.append_fields_to_user_xlsform( xlsform=xlsform, form_category=form_category, - task_count=task_count, additional_entities=additional_entities, ) # Write XLS form content to db diff --git a/src/backend/pdm.lock b/src/backend/pdm.lock index 2215fb9fe8..5b5c11ca78 100644 --- a/src/backend/pdm.lock +++ b/src/backend/pdm.lock @@ -5,7 +5,7 @@ groups = ["default", "debug", "dev", "docs", "test", "monitoring"] strategy = ["cross_platform"] lock_version = "4.4.1" -content_hash = "sha256:d3b1cf0233422641fe8c1e49bb518a57c17fccc8c4d74f2b35cfa55eebbeca7d" +content_hash = "sha256:714741fe6a8df3b2625f3bdda0fa0be99d488d1a1d814eadb6bfbc9c6fbdf17e" [[package]] name = "aiohttp" @@ -1579,7 +1579,7 @@ files = [ [[package]] name = "osm-fieldwork" -version = "0.16.5rc0" +version = "0.16.5" requires_python = ">=3.10" summary = "Processing field data from ODK to OpenStreetMap format." dependencies = [ @@ -1605,8 +1605,8 @@ dependencies = [ "xmltodict>=0.13.0", ] files = [ - {file = "osm-fieldwork-0.16.5rc0.tar.gz", hash = "sha256:34efa14be5bfb111f8227809867d8c73bbdf3893e472d5c239643a727fe1c769"}, - {file = "osm_fieldwork-0.16.5rc0-py3-none-any.whl", hash = "sha256:a879a8b0dce9273d7c72d03ec6d704152305ccc8be72bc9404b95877737eec67"}, + {file = "osm-fieldwork-0.16.5.tar.gz", hash = "sha256:db608ba8f71459673882f7f0baece64e0963aa9b715d10d0f87ca8c71c955cc1"}, + {file = "osm_fieldwork-0.16.5-py3-none-any.whl", hash = "sha256:b24906bd28646a3766d52a8eaa4e515ef9195f24be3757eebbe240c7018a4299"}, ] [[package]] diff --git a/src/backend/pyproject.toml b/src/backend/pyproject.toml index f0dd0d3347..30e7d87c5c 100644 --- a/src/backend/pyproject.toml +++ b/src/backend/pyproject.toml @@ -44,7 +44,7 @@ dependencies = [ "cryptography>=42.0.8", "pyjwt>=2.8.0", "async-lru>=2.0.4", - "osm-fieldwork>=0.16.5rc0", + "osm-fieldwork>=0.16.5", "osm-login-python==2.0.0", "osm-rawdata==0.3.2", "fmtm-splitter==1.3.1", diff --git a/src/frontend/src/components/DialogTaskActions.tsx b/src/frontend/src/components/DialogTaskActions.tsx index cac25bb9d2..a9692f986d 100755 --- a/src/frontend/src/components/DialogTaskActions.tsx +++ b/src/frontend/src/components/DialogTaskActions.tsx @@ -232,7 +232,7 @@ export default function Dialog({ taskId, feature }: dialogPropType) { ); if (isMobile) { - document.location.href = `odkcollect://form/${projectInfo.xform_id}?task_filter=${taskId}`; + document.location.href = `odkcollect://form/${projectInfo.xform_id}`; } else { dispatch( CommonActions.SetSnackBar({ diff --git a/src/frontend/src/views/SubmissionDetails.tsx b/src/frontend/src/views/SubmissionDetails.tsx index 680ddb2aa9..0e4a233d0e 100644 --- a/src/frontend/src/views/SubmissionDetails.tsx +++ b/src/frontend/src/views/SubmissionDetails.tsx @@ -94,11 +94,7 @@ const SubmissionDetails = () => { const projectDashboardLoading = useAppSelector((state) => state.project.projectDashboardLoading); const submissionDetails = useAppSelector((state) => state.submission.submissionDetails); const submissionDetailsLoading = useAppSelector((state) => state.submission.submissionDetailsLoading); - const taskId = submissionDetails?.task_id - ? submissionDetails?.task_id - : submissionDetails?.task_filter - ? submissionDetails?.task_filter - : '-'; + const taskId = submissionDetails?.task_id ? submissionDetails?.task_id : '-'; const { start, end, today, deviceid, ...restSubmissionDetails } = submissionDetails || {}; const dateDeviceDetails = { start, end, today, deviceid }; @@ -158,7 +154,7 @@ const SubmissionDetails = () => { const newFeaturePoint = { type: 'Feature', geometry: { - ...restSubmissionDetails?.new_feature_point, + ...restSubmissionDetails?.new_feature, }, properties: {}, }; @@ -250,7 +246,7 @@ const SubmissionDetails = () => { featureGeojson={ submissionDetailsLoading ? {} - : restSubmissionDetails?.new_feature === 'yes' + : restSubmissionDetails?.new_feature ? newFeaturePoint : coordinatesArray ? geojsonFeature From 4556cf2db1e40678a94a3d432e5eb6786889f141 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Mon, 23 Sep 2024 23:11:13 +0100 Subject: [PATCH 5/7] fix(backend): error handling if submission download fails --- src/backend/app/submissions/submission_crud.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/backend/app/submissions/submission_crud.py b/src/backend/app/submissions/submission_crud.py index 9032ca7429..7d59807545 100644 --- a/src/backend/app/submissions/submission_crud.py +++ b/src/backend/app/submissions/submission_crud.py @@ -465,9 +465,18 @@ async def get_submission_detail( """ odk_credentials = await project_deps.get_odk_credentials(db, project.id) odk_form = get_odk_form(odk_credentials) - submission = json.loads( - odk_form.getSubmissions(project.odkid, project.odk_form_id, submission_id) + + project_submissions = odk_form.getSubmissions( + project.odkid, project.odk_form_id, submission_id ) + if not project_submissions: + log.warning("Failed to download submissions due to unknown error") + raise HTTPException( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + detail="Failed to download submissions", + ) + + submission = json.loads(project_submissions) return submission.get("value", [])[0] From 6df0d719b2508c125c0b9a07f2048d9346d21364 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Mon, 23 Sep 2024 23:36:18 +0100 Subject: [PATCH 6/7] ci: exclude sqlfluff RF02 for old migration file --- src/backend/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/pyproject.toml b/src/backend/pyproject.toml index 30e7d87c5c..c874cc6d33 100644 --- a/src/backend/pyproject.toml +++ b/src/backend/pyproject.toml @@ -147,7 +147,7 @@ update_changelog_on_bump = true [tool.sqlfluff.core] dialect = "postgres" large_file_skip_byte_limit = 30000 # Required to process fmtm_base_schema.sql -exclude_rules = "CP05" # Avoid capitalisation of enums +exclude_rules = "CP05, RF02" # Avoid capitalisation of enums [tool.codespell] skip = "contrib/*.py,*languages_and_countries.py,*pnpm-lock.yaml,*CHANGELOG.md" From 407b2def3c54a8df1874172a3da93fa16a444b18 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Mon, 23 Sep 2024 23:37:25 +0100 Subject: [PATCH 7/7] fix(backend): submission route ordering causing 500 error --- .../app/submissions/submission_routes.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/backend/app/submissions/submission_routes.py b/src/backend/app/submissions/submission_routes.py index 460fb0450e..4cb2ea5c01 100644 --- a/src/backend/app/submissions/submission_routes.py +++ b/src/backend/app/submissions/submission_routes.py @@ -412,20 +412,6 @@ async def submission_table( return response -@router.get("/{submission_id}") -async def submission_detail( - submission_id: str, - db: Session = Depends(database.get_db), - project_user: ProjectUserDict = Depends(mapper), -) -> dict: - """This api returns the submission detail of individual submission.""" - project = project_user.get("project") - submission_detail = await submission_crud.get_submission_detail( - submission_id, project, db - ) - return submission_detail - - @router.post("/update_review_state") async def update_review_state( instance_id: str, @@ -527,6 +513,20 @@ async def conflate_geojson( ) from e +@router.get("/{submission_id}") +async def submission_detail( + submission_id: str, + db: Session = Depends(database.get_db), + project_user: ProjectUserDict = Depends(mapper), +) -> dict: + """This api returns the submission detail of individual submission.""" + project = project_user.get("project") + submission_detail = await submission_crud.get_submission_detail( + submission_id, project, db + ) + return submission_detail + + @router.get("/{submission_id}/photos") async def submission_photo( submission_id: str,