From fdbe6b042b79ddeff9413dce0ac806ff183d9b08 Mon Sep 17 00:00:00 2001 From: frozenhelium Date: Mon, 1 Jun 2026 23:34:07 +0545 Subject: [PATCH 1/3] fix(locate-features): improve tutorial GeoJSON import and preview rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Order preview sub-grid references by task partition index so colors stay aligned with their cells after reload - Reject uploads with duplicate screens - Warn when scenario screen numbers are not serial (1, 2, 3, …) - Show error when project's sub-grid size is not resolved before importing instead of silently creating a single task per feature - Default missing sub-grid references to 0 and clip extra references to the sub-grid size, warning in both cases - Add --size-tile-locate (2x --size-tile) and use it for the Locate preview --- .../LocateFeaturesScenarioPreview/index.tsx | 28 +++- .../styles.module.css | 4 +- app/index.css | 1 + app/views/EditTutorial/index.tsx | 146 +++++++++++++----- backend | 2 +- 5 files changed, 135 insertions(+), 46 deletions(-) diff --git a/app/components/domain/LocateFeaturesScenarioPreview/index.tsx b/app/components/domain/LocateFeaturesScenarioPreview/index.tsx index 372104c..14b37a9 100644 --- a/app/components/domain/LocateFeaturesScenarioPreview/index.tsx +++ b/app/components/domain/LocateFeaturesScenarioPreview/index.tsx @@ -1,6 +1,7 @@ import { useMemo } from 'react'; import { _cs, + isDefined, isNotDefined, } from '@togglecorp/fujs'; import { @@ -74,9 +75,26 @@ function LocateFeaturesScenarioPreview(props: Props) { } satisfies Omit; }, [customOptions]); - const references = useMemo(() => ( - scenario?.tasks?.map(({ reference }) => reference) - ), [scenario?.tasks]); + const references = useMemo(() => { + const tasks = scenario?.tasks; + if (isNotDefined(tasks)) { + return undefined; + } + + // createSubGridGeoJsonFromTile maps references positionally onto the + // row-major sub-grid cells, where cell i corresponds to + // taskPartitionIndex i. Task order from the server is not guaranteed to + // match partition order on reload, so we index by taskPartitionIndex + // rather than rely on the array position. + const orderedReferences: (number | undefined)[] = []; + tasks.forEach((task) => { + if (isDefined(task.taskPartitionIndex)) { + orderedReferences[task.taskPartitionIndex] = task.reference; + } + }); + + return orderedReferences; + }, [scenario?.tasks]); const firstTask = scenario?.tasks?.[0]; const [tileX, tileY, tileZ] = useMemo(() => { @@ -134,8 +152,8 @@ function LocateFeaturesScenarioPreview(props: Props) { contentClassName={styles.content} > { + // Each Locate feature is one parent tile rendered as a single scenario + // screen, so a screen must not appear on more than one feature. + const screens = features.map(({ properties }) => properties.screen); + const groupedScreens = listToGroupList( + screens, + (screen) => screen, + (screen) => screen, + ); + + const duplicateScreens = Object.values(groupedScreens) + .filter((group) => group.length > 1) + .map((group) => group[0]); + + if (duplicateScreens.length === 0) { + return true; + } + + ctx.error({ + problem: `expected each screen to appear only once (duplicated screen(s): ${duplicateScreens.join(', ')})`, + }); + + return false; + }), }); const CompletenessTutorialGeoJsonType = type({ @@ -856,49 +879,96 @@ function NewTutorial() { [nonFieldError]: result.summary, }, }); - } else { - const featuresByScreen = listToGroupList( - result.features, - (feature) => feature.properties.screen, - ); + return; + } - // eslint-disable-next-line no-underscore-dangle - const subgridSize = projectDetailResponse.project.projectTypeSpecifics?.__typename === 'LocateProjectPropertyType' - ? projectDetailResponse.project.projectTypeSpecifics.subGridSize - : undefined; + // eslint-disable-next-line no-underscore-dangle + const subgridSize = projectDetailResponse.project.projectTypeSpecifics?.__typename === 'LocateProjectPropertyType' + ? projectDetailResponse.project.projectTypeSpecifics.subGridSize + : undefined; - const subGridValue = isDefined(subgridSize) - ? subgridSizeToValueMap[subgridSize] - : 0; + // The number of sub-grid cells (and hence tasks) per feature is + // derived from the project's sub-grid size, so we cannot proceed + // without it -- otherwise we'd silently create a single task per + // feature and drop the rest of the references. + if (isNotDefined(subgridSize)) { + setError({ + scenarios: { + [nonFieldError]: 'Could not determine the sub-grid size for this project. Please configure the project before uploading scenarios.', + }, + }); + return; + } - const numSubGrids = (2 ** subGridValue) ** 2; + const numSubGrids = (2 ** subgridSizeToValueMap[subgridSize]) ** 2; - const scenarioPages: PartialScenarioPageInputFields[] = unique( - result.features, - (feature) => feature.properties.screen, - ).toSorted( - (a, b) => compareNumber(a.properties.screen, b.properties.screen), - ).map(({ properties }) => ({ - clientId: ulid(), - scenarioPageNumber: properties.screen, - tasks: featuresByScreen[properties.screen].flatMap((feature) => ( - Array.from(new Array(numSubGrids).keys()).map((index) => ({ - clientId: ulid(), - reference: feature.properties.references[index], - taskPartitionIndex: index, - projectTypeSpecifics: { - locate: { - tileX: feature.properties.tile_x, - tileY: feature.properties.tile_y, - tileZ: feature.properties.tile_z, - } satisfies LocateFeaturesPropertyInputFields, - }, - })) - )), - })); + // Each feature should carry one reference per sub-grid cell. If it + // provides more, we clip to the first numSubGrids (below) and warn; + // if it provides fewer, the missing cells default to 0. + result.features.forEach((feature) => { + if (feature.properties.references.length > numSubGrids) { + // eslint-disable-next-line no-console + console.warn( + `Feature on screen ${feature.properties.screen} has ${feature.properties.references.length} references but the sub-grid only has ${numSubGrids} cells; extra references were ignored.`, + ); + } - setFieldValue(scenarioPages, 'scenarios'); + if (feature.properties.references.length < numSubGrids) { + // eslint-disable-next-line no-console + console.warn( + `Feature on screen ${feature.properties.screen} has ${feature.properties.references.length} references but the sub-grid needs ${numSubGrids} cells; missing references were filled in.`, + ); + } + }); + + // Screens are unique (enforced by the schema above) and should be + // serial (1, 2, 3, …). Warn -- but don't block -- on gaps so the + // author can fix the scenario numbering. + const sortedScreens = result.features + .map((feature) => feature.properties.screen) + .toSorted((a, b) => compareNumber(a, b)); + + const screensAreSerial = sortedScreens.every( + (screen, index) => screen === index + 1, + ); + if (!screensAreSerial) { + // eslint-disable-next-line no-console + console.warn( + `Locate scenario screens are expected to be serial (1, 2, 3, …). Found: ${sortedScreens.join(', ')}.`, + ); } + + const featuresByScreen = listToGroupList( + result.features, + (feature) => feature.properties.screen, + ); + + const scenarioPages: PartialScenarioPageInputFields[] = unique( + result.features, + (feature) => feature.properties.screen, + ).toSorted( + (a, b) => compareNumber(a.properties.screen, b.properties.screen), + ).map(({ properties }) => ({ + clientId: ulid(), + scenarioPageNumber: properties.screen, + tasks: featuresByScreen[properties.screen].flatMap((feature) => ( + Array.from(new Array(numSubGrids).keys()).map((index) => ({ + clientId: ulid(), + // FIXME(frozenhelium): maybe the default value should be first option? + reference: feature.properties.references[index] ?? 0, + taskPartitionIndex: index, + projectTypeSpecifics: { + locate: { + tileX: feature.properties.tile_x, + tileY: feature.properties.tile_y, + tileZ: feature.properties.tile_z, + } satisfies LocateFeaturesPropertyInputFields, + }, + })) + )), + })); + + setFieldValue(scenarioPages, 'scenarios'); } }, [projectDetailResponse, setError, setFieldValue]); diff --git a/backend b/backend index c5bfe14..1c83e8f 160000 --- a/backend +++ b/backend @@ -1 +1 @@ -Subproject commit c5bfe14dc6f6b3417f09c3f2d4a19bb0f4ea4a70 +Subproject commit 1c83e8f39713984cba3a9a19e9f7565d5626608b From 2cccef89951446cac0c8773c0a23cc9bdb27aa59 Mon Sep 17 00:00:00 2001 From: frozenhelium Date: Fri, 5 Jun 2026 08:34:51 +0545 Subject: [PATCH 2/3] refactor(edit-tutorial): enforce scenario GeoJSON import validation - extract per-project-type GeoJSON transforms - enforce serial screens (all types), - unique screens via getDuplicates, - exact sub-grid reference counts, and reference values against - project custom options / tile options - add unit tests for the tutorial transforms - fix vitest config (node env) --- app/views/EditTutorial/index.tsx | 505 +++------------------- app/views/EditTutorial/utils.test.ts | 589 +++++++++++++++++++++++++ app/views/EditTutorial/utils.ts | 625 +++++++++++++++++++++++++-- vite.config.ts | 5 +- 4 files changed, 1225 insertions(+), 499 deletions(-) create mode 100644 app/views/EditTutorial/utils.test.ts diff --git a/app/views/EditTutorial/index.tsx b/app/views/EditTutorial/index.tsx index 15c2712..11daed0 100644 --- a/app/views/EditTutorial/index.tsx +++ b/app/views/EditTutorial/index.tsx @@ -12,12 +12,10 @@ import { } from 'react-icons/pi'; import { useParams } from 'react-router'; import { - compareNumber, isDefined, isNotDefined, listToGroupList, listToMap, - unique, } from '@togglecorp/fujs'; import { createSubmitHandler, @@ -49,10 +47,7 @@ import { ValidateImageTutorialTaskPropertyInput, } from '#generated/types/graphql'; import useAlert from '#hooks/useAlert'; -import { - readFileAsText, - subgridSizeToValueMap, -} from '#utils/common'; +import { readFileAsText } from '#utils/common'; import { alertCombinedError, checkAndAlertGraphQLResultError, @@ -61,13 +56,6 @@ import { import { CocoType } from '#utils/validation'; import { PartialInformationPageInputFields } from './InformationPageInput/schema'; -import { PartialScenarioPageInputFields } from './ScenarioPageInput/schema'; -import { ComparePropertyInputFields } from './ScenarioPageInput/TaskInput/ComparePropertyInput/schema'; -import { CompletenessPropertyInputFields } from './ScenarioPageInput/TaskInput/CompletenessPropertyInput/schema'; -import { FindPropertyInputFields } from './ScenarioPageInput/TaskInput/FindPropertyInput/schema'; -import { LocateFeaturesPropertyInputFields } from './ScenarioPageInput/TaskInput/LocateFeaturesPropertyInput/schema'; -import { StreetPropertyInputFields } from './ScenarioPageInput/TaskInput/StreetPropertyInput/schema'; -import { ValidatePropertyInputFields } from './ScenarioPageInput/TaskInput/ValidatePropertyInput/schema'; import InformationPageInput from './InformationPageInput'; import ScenarioPageInput from './ScenarioPageInput'; import tutorialUpdateSchema, { @@ -75,6 +63,16 @@ import tutorialUpdateSchema, { TutorialFormContext, } from './schema'; import TutorialActions from './TutorialActions'; +import { + getValidReferenceValues, + transformCompareGeoJson, + transformCompletenessGeoJson, + transformFindGeoJson, + transformLocateGeoJson, + transformStreetGeoJson, + transformValidateGeoJson, + TutorialGeoJsonTransformResult, +} from './utils'; import styles from './styles.module.css'; @@ -89,179 +87,8 @@ function stringifyId(value: number | undefined) { return String(value); } -const PositionType = type.number.array(); - -const PolygonType = type({ - type: "'Polygon'", - coordinates: PositionType.array().array(), -}); -const MultiPolygonType = type({ - type: "'MultiPolygon'", - coordinates: PositionType.array().array().array(), -}); - -const CommonFeaturePropertyType = type({ - screen: type.number, - reference: type.number, -}); -const TileFeaturePropertyType = type({ - tile_x: type.number, - tile_y: type.number, - tile_z: type.number, -}); - -const ValidateFeaturePropertyType = type.merge( - CommonFeaturePropertyType, - { - // This is not used anymore - // id: '"string" | "number"', - id: type.number, - }, -); - -const FindFeaturePropertyType = type.merge( - CommonFeaturePropertyType, - TileFeaturePropertyType, - { - // This is not used anymore - // task_id: 'string', - }, -); -const CompareFeaturePropertyType = type.merge( - CommonFeaturePropertyType, - TileFeaturePropertyType, - { - // This is not used anymore - // task_id: 'string', - }, -); -const LocateFeaturesPropertyType = type.merge( - TileFeaturePropertyType, - { - screen: type.number, - references: type.number.array(), - // This is not used anymore - // task_id: 'string', - }, -); - -const CompletenessFeaturePropertyType = type.merge( - CommonFeaturePropertyType, - TileFeaturePropertyType, - { - // This is not used anymore - // task_id: 'string', - }, -); - -const StreetFeaturePropertyType = type({ - '...': CommonFeaturePropertyType, - id: 'string', -}); - -const ValidateTutorialGeoJsonType = type({ - type: '"FeatureCollection"', - features: type({ - geometry: PolygonType.or(MultiPolygonType), - properties: ValidateFeaturePropertyType, - }).array(), -}); - -const FindTutorialGeoJsonType = type({ - type: '"FeatureCollection"', - features: type({ - geometry: PolygonType.or(MultiPolygonType), - properties: FindFeaturePropertyType, - }).array().narrow((features, ctx) => { - // FIXME: add similar validations to other types - const screens = features.map(({ properties }) => properties.screen); - const groupedScreens = listToGroupList( - screens, - (screen) => screen, - (screen) => screen, - ); - - const errors = Object.values(groupedScreens).map((group) => { - if (group.length === 6) { - return undefined; - } - - return { - screen: group[0], - numEntries: group.length, - }; - }).filter(isDefined); - - if (errors.length === 0) { - return true; - } - - const errorDescription = errors.map(({ screen, numEntries }) => `${numEntries} for screen ${screen}`).join(', '); - ctx.error({ - problem: `expected to have 6 instances of every screen(found ${errorDescription})`, - }); - - return false; - }), -}); - -const CompareTutorialGeoJsonType = type({ - type: '"FeatureCollection"', - features: type({ - geometry: PolygonType.or(MultiPolygonType), - properties: CompareFeaturePropertyType, - }).array(), -}); - -const LocateFeaturesTutorialGeoJsonType = type({ - type: '"FeatureCollection"', - features: type({ - geometry: PolygonType.or(MultiPolygonType), - properties: LocateFeaturesPropertyType, - }).array().narrow((features, ctx) => { - // Each Locate feature is one parent tile rendered as a single scenario - // screen, so a screen must not appear on more than one feature. - const screens = features.map(({ properties }) => properties.screen); - const groupedScreens = listToGroupList( - screens, - (screen) => screen, - (screen) => screen, - ); - - const duplicateScreens = Object.values(groupedScreens) - .filter((group) => group.length > 1) - .map((group) => group[0]); - - if (duplicateScreens.length === 0) { - return true; - } - - ctx.error({ - problem: `expected each screen to appear only once (duplicated screen(s): ${duplicateScreens.join(', ')})`, - }); - - return false; - }), -}); - -const CompletenessTutorialGeoJsonType = type({ - type: '"FeatureCollection"', - features: type({ - geometry: PolygonType.or(MultiPolygonType), - properties: CompletenessFeaturePropertyType, - }).array(), -}); - const ValidateImageJsonType = CocoType; -const StreetTutorialGeoJsonType = type({ - type: '"FeatureCollection"', - features: type({ - geometry: PolygonType.or(MultiPolygonType), - properties: StreetFeaturePropertyType, - }).array(), -}); - function createMapping(items: T[]) { return listToMap(items, ({ clientId }) => clientId); } @@ -688,288 +515,64 @@ function NewTutorial() { const { projectType, + projectTypeSpecifics, } = projectDetailResponse.project; - if (projectType === ProjectTypeEnum.Validate) { - const result = ValidateTutorialGeoJsonType(geoJson); - if (result instanceof type.errors) { - setError({ - scenarios: { - [nonFieldError]: result.summary, - }, - }); - } else { - const scenarioPages = result.features.map((feature, i) => ({ - clientId: ulid(), - scenarioPageNumber: isDefined(feature.properties.screen) - ? feature.properties.screen - : i + 1, - tasks: [ - { - clientId: ulid(), - reference: feature.properties.reference, - projectTypeSpecifics: { - // FIXME: Why objectGeometry is string? - validate: { - identifier: feature.properties.id, - objectGeometry: JSON.stringify(feature.geometry, null, 4), - } satisfies ValidatePropertyInputFields, - }, - }, - ], - })); - - setFieldValue( - scenarioPages.toSorted((a, b) => ( - compareNumber(a.scenarioPageNumber, b.scenarioPageNumber) - )), - 'scenarios', - ); - } - } else if (projectType === ProjectTypeEnum.Find) { - const result = FindTutorialGeoJsonType(geoJson); - if (result instanceof type.errors) { - setError({ - scenarios: { - [nonFieldError]: result.summary, - }, - }); - } else { - const featuresByScreen = listToGroupList( - result.features, - (feature) => feature.properties.screen, - ); - const scenarioPages: PartialScenarioPageInputFields[] = unique( - result.features, - (feature) => feature.properties.screen, - ).toSorted( - (a, b) => compareNumber(a.properties.screen, b.properties.screen), - ).map(({ properties }) => ({ - clientId: ulid(), - scenarioPageNumber: properties.screen, - tasks: featuresByScreen[properties.screen].map((feature) => ({ - clientId: ulid(), - reference: feature.properties.reference, - projectTypeSpecifics: { - find: { - tileX: feature.properties.tile_x, - tileY: feature.properties.tile_y, - tileZ: feature.properties.tile_z, - } satisfies FindPropertyInputFields, - }, - })), - })); + if (projectType === ProjectTypeEnum.ValidateImage) { + return; + } - setFieldValue(scenarioPages, 'scenarios'); - } - } else if (projectType === ProjectTypeEnum.Compare) { - const result = CompareTutorialGeoJsonType(geoJson); - if (result instanceof type.errors) { - setError({ - scenarios: { - [nonFieldError]: result.summary, - }, - }); - } else { - const featuresByScreen = listToGroupList( - result.features, - (feature) => feature.properties.screen, - ); + let result: TutorialGeoJsonTransformResult | undefined; - const scenarioPages = unique( - result.features, - (feature) => feature.properties.screen, - ).toSorted( - (a, b) => compareNumber(a.properties.screen, b.properties.screen), - ).map(({ properties }) => ({ - clientId: ulid(), - scenarioPageNumber: properties.screen, - tasks: featuresByScreen[properties.screen].map((feature) => ({ - clientId: ulid(), - reference: feature.properties.reference, - projectTypeSpecifics: { - compare: { - tileX: feature.properties.tile_x, - tileY: feature.properties.tile_y, - tileZ: feature.properties.tile_z, - } satisfies ComparePropertyInputFields, - }, - })), - })); + if (projectType === ProjectTypeEnum.Validate) { + // eslint-disable-next-line no-underscore-dangle + const validReferenceValues = projectTypeSpecifics?.__typename === 'ValidateProjectPropertyType' + ? getValidReferenceValues(projectTypeSpecifics.customOptions) + : undefined; - setFieldValue(scenarioPages, 'scenarios'); - } + result = transformValidateGeoJson(geoJson, validReferenceValues); + } else if (projectType === ProjectTypeEnum.Find) { + result = transformFindGeoJson(geoJson); + } else if (projectType === ProjectTypeEnum.Compare) { + result = transformCompareGeoJson(geoJson); } else if (projectType === ProjectTypeEnum.Completeness) { - const result = CompletenessTutorialGeoJsonType(geoJson); - if (result instanceof type.errors) { - setError({ - scenarios: { - [nonFieldError]: result.summary, - }, - }); - } else { - const featuresByScreen = listToGroupList( - result.features, - (feature) => feature.properties.screen, - ); - - const scenarioPages = unique( - result.features, - (feature) => feature.properties.screen, - ).toSorted( - (a, b) => compareNumber(a.properties.screen, b.properties.screen), - ).map(({ properties }) => ({ - clientId: ulid(), - scenarioPageNumber: properties.screen, - tasks: featuresByScreen[properties.screen].map((feature) => ({ - clientId: ulid(), - reference: feature.properties.reference, - projectTypeSpecifics: { - completeness: { - tileX: feature.properties.tile_x, - tileY: feature.properties.tile_y, - tileZ: feature.properties.tile_z, - } satisfies CompletenessPropertyInputFields, - }, - })), - })); - - setFieldValue(scenarioPages, 'scenarios'); - } + result = transformCompletenessGeoJson(geoJson); } else if (projectType === ProjectTypeEnum.Street) { - const result = StreetTutorialGeoJsonType(geoJson); - if (result instanceof type.errors) { - setError({ - scenarios: { - [nonFieldError]: result.summary, - }, - }); - } else { - const scenarioPages = result.features.map((feature, i) => ({ - clientId: ulid(), - scenarioPageNumber: isDefined(feature.properties.screen) - ? feature.properties.screen - : i + 1, - tasks: [ - { - clientId: ulid(), - reference: feature.properties.reference, - projectTypeSpecifics: { - street: { - mapillaryImageId: feature.properties.id, - geometry: JSON.stringify(feature.geometry, null, 4), - } satisfies StreetPropertyInputFields, - }, - }, - ], - })); - - setFieldValue( - scenarioPages.toSorted((a, b) => ( - compareNumber(a.scenarioPageNumber, b.scenarioPageNumber) - )), - 'scenarios', - ); - } - } else if (projectType === ProjectTypeEnum.Locate) { - const result = LocateFeaturesTutorialGeoJsonType(geoJson); - if (result instanceof type.errors) { - setError({ - scenarios: { - [nonFieldError]: result.summary, - }, - }); - return; - } - // eslint-disable-next-line no-underscore-dangle - const subgridSize = projectDetailResponse.project.projectTypeSpecifics?.__typename === 'LocateProjectPropertyType' - ? projectDetailResponse.project.projectTypeSpecifics.subGridSize + const validReferenceValues = projectTypeSpecifics?.__typename === 'StreetProjectPropertyType' + ? getValidReferenceValues(projectTypeSpecifics.customOptions) : undefined; - // The number of sub-grid cells (and hence tasks) per feature is - // derived from the project's sub-grid size, so we cannot proceed - // without it -- otherwise we'd silently create a single task per - // feature and drop the rest of the references. - if (isNotDefined(subgridSize)) { - setError({ - scenarios: { - [nonFieldError]: 'Could not determine the sub-grid size for this project. Please configure the project before uploading scenarios.', - }, - }); - return; - } + result = transformStreetGeoJson(geoJson, validReferenceValues); + } else if (projectType === ProjectTypeEnum.Locate) { + // eslint-disable-next-line no-underscore-dangle + const isLocateProperty = projectTypeSpecifics?.__typename === 'LocateProjectPropertyType'; + const subgridSize = isLocateProperty + ? projectTypeSpecifics.subGridSize + : undefined; + const validReferenceValues = isLocateProperty + ? getValidReferenceValues(projectTypeSpecifics.customOptions) + : undefined; - const numSubGrids = (2 ** subgridSizeToValueMap[subgridSize]) ** 2; + result = transformLocateGeoJson(geoJson, subgridSize, validReferenceValues); + } else { + projectType satisfies never; + } - // Each feature should carry one reference per sub-grid cell. If it - // provides more, we clip to the first numSubGrids (below) and warn; - // if it provides fewer, the missing cells default to 0. - result.features.forEach((feature) => { - if (feature.properties.references.length > numSubGrids) { - // eslint-disable-next-line no-console - console.warn( - `Feature on screen ${feature.properties.screen} has ${feature.properties.references.length} references but the sub-grid only has ${numSubGrids} cells; extra references were ignored.`, - ); - } + if (isNotDefined(result)) { + return; + } - if (feature.properties.references.length < numSubGrids) { - // eslint-disable-next-line no-console - console.warn( - `Feature on screen ${feature.properties.screen} has ${feature.properties.references.length} references but the sub-grid needs ${numSubGrids} cells; missing references were filled in.`, - ); - } + if (!result.ok) { + setError({ + scenarios: { + [nonFieldError]: result.error, + }, }); - - // Screens are unique (enforced by the schema above) and should be - // serial (1, 2, 3, …). Warn -- but don't block -- on gaps so the - // author can fix the scenario numbering. - const sortedScreens = result.features - .map((feature) => feature.properties.screen) - .toSorted((a, b) => compareNumber(a, b)); - - const screensAreSerial = sortedScreens.every( - (screen, index) => screen === index + 1, - ); - if (!screensAreSerial) { - // eslint-disable-next-line no-console - console.warn( - `Locate scenario screens are expected to be serial (1, 2, 3, …). Found: ${sortedScreens.join(', ')}.`, - ); - } - - const featuresByScreen = listToGroupList( - result.features, - (feature) => feature.properties.screen, - ); - - const scenarioPages: PartialScenarioPageInputFields[] = unique( - result.features, - (feature) => feature.properties.screen, - ).toSorted( - (a, b) => compareNumber(a.properties.screen, b.properties.screen), - ).map(({ properties }) => ({ - clientId: ulid(), - scenarioPageNumber: properties.screen, - tasks: featuresByScreen[properties.screen].flatMap((feature) => ( - Array.from(new Array(numSubGrids).keys()).map((index) => ({ - clientId: ulid(), - // FIXME(frozenhelium): maybe the default value should be first option? - reference: feature.properties.references[index] ?? 0, - taskPartitionIndex: index, - projectTypeSpecifics: { - locate: { - tileX: feature.properties.tile_x, - tileY: feature.properties.tile_y, - tileZ: feature.properties.tile_z, - } satisfies LocateFeaturesPropertyInputFields, - }, - })) - )), - })); - - setFieldValue(scenarioPages, 'scenarios'); + return; } + + setFieldValue(result.scenarioPages, 'scenarios'); }, [projectDetailResponse, setError, setFieldValue]); const handleDatasetFileSelect = useCallback(async (file: File | undefined) => { diff --git a/app/views/EditTutorial/utils.test.ts b/app/views/EditTutorial/utils.test.ts new file mode 100644 index 0000000..2e3b903 --- /dev/null +++ b/app/views/EditTutorial/utils.test.ts @@ -0,0 +1,589 @@ +import { + describe, + expect, + it, +} from 'vitest'; + +import { SubGridSizeEnum } from '#generated/types/graphql'; + +import { + getValidReferenceValues, + transformCompareGeoJson, + transformCompletenessGeoJson, + transformFindGeoJson, + transformLocateGeoJson, + transformStreetGeoJson, + transformValidateGeoJson, +} from './utils'; + +const polygon: GeoJSON.Polygon = { + type: 'Polygon', + coordinates: [[[0, 0], [1, 0], [1, 1], [0, 0]]], +}; + +function createFeatureCollection( + propertiesList: GeoJSON.GeoJsonProperties[], +): GeoJSON.GeoJSON { + return { + type: 'FeatureCollection', + features: propertiesList.map((properties) => ({ + type: 'Feature', + geometry: polygon, + properties, + })), + }; +} + +describe('getValidReferenceValues', () => { + it('flattens option values and sub-option values', () => { + expect(getValidReferenceValues([ + { value: 1, subOptions: [{ value: 11 }, { value: 12 }] }, + { value: 2 }, + ])).toEqual([1, 11, 12, 2]); + }); + + it('keeps the falsy option value 0 and drops valueless sub-options', () => { + expect(getValidReferenceValues([ + { value: 0, subOptions: [{ value: 11 }, {}] }, + { value: 2 }, + ])).toEqual([0, 11, 2]); + }); + + it('skips options without a value along with their sub-options', () => { + expect(getValidReferenceValues([ + { subOptions: [{ value: 5 }] }, + ])).toBeUndefined(); + }); + + it('returns undefined for missing or empty options', () => { + expect(getValidReferenceValues(undefined)).toBeUndefined(); + expect(getValidReferenceValues(null)).toBeUndefined(); + expect(getValidReferenceValues([])).toBeUndefined(); + }); +}); + +describe('transformValidateGeoJson', () => { + const validOptions = [0, 1]; + + it('maps each feature to its own scenario page, sorted by screen', () => { + const result = transformValidateGeoJson(createFeatureCollection([ + { screen: 2, reference: 1, id: 200 }, + { screen: 1, reference: 0, id: 100 }, + ]), validOptions); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.scenarioPages.map( + (page) => page.scenarioPageNumber, + )).toEqual([1, 2]); + + const [firstPage] = result.scenarioPages; + expect(firstPage.tasks).toHaveLength(1); + + const [task] = firstPage.tasks ?? []; + expect(task.reference).toBe(0); + expect(task.projectTypeSpecifics?.validate?.identifier).toBe(100); + expect(JSON.parse( + task.projectTypeSpecifics?.validate?.objectGeometry ?? '', + )).toEqual(polygon); + }); + + it('reports a validation error for malformed properties', () => { + const result = transformValidateGeoJson(createFeatureCollection([ + { screen: 1 }, + ]), validOptions); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error.length).toBeGreaterThan(0); + }); + + it('errors when the project has no custom options', () => { + const result = transformValidateGeoJson(createFeatureCollection([ + { screen: 1, reference: 0, id: 100 }, + ]), undefined); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('custom options'); + }); + + it('rejects reference values outside the custom options', () => { + const result = transformValidateGeoJson(createFeatureCollection([ + { screen: 1, reference: 9, id: 100 }, + ]), validOptions); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('invalid reference value(s): 9 for screen 1'); + expect(result.error).toContain('valid values are 0, 1'); + }); + + it('rejects duplicated screens', () => { + const result = transformValidateGeoJson(createFeatureCollection([ + { screen: 1, reference: 0, id: 100 }, + { screen: 1, reference: 1, id: 101 }, + { screen: 2, reference: 0, id: 102 }, + ]), validOptions); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('duplicated screen(s): 1'); + }); + + it('rejects non-serial screens', () => { + const result = transformValidateGeoJson(createFeatureCollection([ + { screen: 1, reference: 0, id: 100 }, + { screen: 3, reference: 0, id: 101 }, + ]), validOptions); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('screens must be serial'); + expect(result.error).toContain('found: 1, 3'); + }); + + it('aggregates multiple problems into one error', () => { + const result = transformValidateGeoJson(createFeatureCollection([ + { screen: 2, reference: 9, id: 100 }, + { screen: 2, reference: 0, id: 101 }, + ]), validOptions); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('duplicated screen(s): 2'); + expect(result.error).toContain('screens must be serial'); + expect(result.error).toContain('invalid reference value(s): 9 for screen 2'); + }); +}); + +describe('transformFindGeoJson', () => { + // tile_x encodes the screen so mis-grouped tasks are detectable + const makeScreen = (screen: number) => Array.from( + new Array(6).keys(), + ).map((i) => ({ + screen, + reference: i % 2, + tile_x: screen * 100 + i, + tile_y: 20 + i, + tile_z: 18, + })); + + it('groups features by screen into sorted scenario pages', () => { + const result = transformFindGeoJson(createFeatureCollection([ + ...makeScreen(2), + ...makeScreen(1), + ])); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.scenarioPages.map( + (page) => page.scenarioPageNumber, + )).toEqual([1, 2]); + + expect(result.scenarioPages[0].tasks?.map( + (task) => task.projectTypeSpecifics?.find?.tileX, + )).toEqual([100, 101, 102, 103, 104, 105]); + expect(result.scenarioPages[1].tasks?.map( + (task) => task.projectTypeSpecifics?.find?.tileX, + )).toEqual([200, 201, 202, 203, 204, 205]); + + const [task] = result.scenarioPages[0].tasks ?? []; + expect(task.projectTypeSpecifics?.find).toEqual({ + tileX: 100, + tileY: 20, + tileZ: 18, + }); + }); + + it('rejects screens without exactly 6 features', () => { + const result = transformFindGeoJson(createFeatureCollection([ + { + screen: 1, + reference: 0, + tile_x: 1, + tile_y: 2, + tile_z: 18, + }, + ])); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('expected to have 6 instances'); + }); + + it('rejects non-serial screens', () => { + const result = transformFindGeoJson(createFeatureCollection([ + ...makeScreen(2), + ...makeScreen(3), + ])); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('screens must be serial'); + expect(result.error).toContain('found: 2, 3'); + }); + + it('rejects reference values outside the tile options', () => { + const [first, ...others] = makeScreen(1); + const result = transformFindGeoJson(createFeatureCollection([ + { ...first, reference: 9 }, + ...others, + ])); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('invalid reference value(s): 9 for screen 1'); + expect(result.error).toContain('valid values are 0, 1, 2, 3'); + }); +}); + +describe('transformStreetGeoJson', () => { + const validOptions = [0, 1]; + + it('maps the string id to mapillaryImageId and serializes geometry', () => { + const result = transformStreetGeoJson(createFeatureCollection([ + { screen: 1, reference: 0, id: 'mapillary-123' }, + ]), validOptions); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + const [task] = result.scenarioPages[0].tasks ?? []; + expect(task.projectTypeSpecifics?.street?.mapillaryImageId).toBe('mapillary-123'); + expect(JSON.parse( + task.projectTypeSpecifics?.street?.geometry ?? '', + )).toEqual(polygon); + }); + + it('rejects numeric ids', () => { + const result = transformStreetGeoJson(createFeatureCollection([ + { screen: 1, reference: 0, id: 123 }, + ]), validOptions); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('id'); + }); +}); + +describe('transformCompareGeoJson', () => { + it('accepts repeated screens and maps compare specifics', () => { + const result = transformCompareGeoJson(createFeatureCollection([ + { + screen: 1, + reference: 0, + tile_x: 1, + tile_y: 2, + tile_z: 18, + }, + { + screen: 1, + reference: 1, + tile_x: 3, + tile_y: 4, + tile_z: 18, + }, + ])); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.scenarioPages).toHaveLength(1); + expect(result.scenarioPages[0].tasks).toHaveLength(2); + + const [task] = result.scenarioPages[0].tasks ?? []; + expect(task.projectTypeSpecifics?.compare).toEqual({ + tileX: 1, + tileY: 2, + tileZ: 18, + }); + }); + + it('rejects non-serial screens', () => { + const result = transformCompareGeoJson(createFeatureCollection([ + { + screen: 2, + reference: 0, + tile_x: 1, + tile_y: 2, + tile_z: 18, + }, + ])); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('screens must be serial'); + }); +}); + +describe('transformCompletenessGeoJson', () => { + it('accepts repeated screens and maps completeness specifics', () => { + const result = transformCompletenessGeoJson(createFeatureCollection([ + { + screen: 1, + reference: 0, + tile_x: 1, + tile_y: 2, + tile_z: 18, + }, + { + screen: 1, + reference: 1, + tile_x: 3, + tile_y: 4, + tile_z: 18, + }, + ])); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.scenarioPages).toHaveLength(1); + expect(result.scenarioPages[0].tasks).toHaveLength(2); + + const [task] = result.scenarioPages[0].tasks ?? []; + expect(task.projectTypeSpecifics?.completeness).toEqual({ + tileX: 1, + tileY: 2, + tileZ: 18, + }); + }); + + it('rejects reference values outside the tile options', () => { + const result = transformCompletenessGeoJson(createFeatureCollection([ + { + screen: 1, + reference: 9, + tile_x: 1, + tile_y: 2, + tile_z: 18, + }, + ])); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('invalid reference value(s): 9 for screen 1'); + }); +}); + +describe('transformLocateGeoJson', () => { + const validOptions = [0, 1, 2, 3, 4]; + + const locateProperties = ( + screen: number, + references: number[], + ) => ({ + screen, + references, + tile_x: 5, + tile_y: 6, + tile_z: 14, + }); + + it('expands each feature into one task per sub-grid cell', () => { + const result = transformLocateGeoJson( + createFeatureCollection([locateProperties(1, [1, 2, 3, 4])]), + SubGridSizeEnum.Size_2X2, + validOptions, + ); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.scenarioPages).toHaveLength(1); + + const tasks = result.scenarioPages[0].tasks ?? []; + expect(tasks.map((task) => task.reference)).toEqual([1, 2, 3, 4]); + expect(tasks.map((task) => task.taskPartitionIndex)).toEqual([0, 1, 2, 3]); + tasks.forEach((task) => { + expect(task.projectTypeSpecifics?.locate).toEqual({ + tileX: 5, + tileY: 6, + tileZ: 14, + }); + }); + }); + + it('derives the sub-grid cell count from the sub-grid size', () => { + const references = Array.from(new Array(16).keys()); + const result = transformLocateGeoJson( + createFeatureCollection([locateProperties(1, references)]), + SubGridSizeEnum.Size_4X4, + references, + ); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + const tasks = result.scenarioPages[0].tasks ?? []; + expect(tasks).toHaveLength(16); + expect(tasks.map((task) => task.taskPartitionIndex)).toEqual(references); + expect(tasks.map((task) => task.reference)).toEqual(references); + }); + + it('rejects features with more references than sub-grid cells', () => { + const result = transformLocateGeoJson( + createFeatureCollection([locateProperties(1, [0, 1, 2, 3, 4])]), + SubGridSizeEnum.Size_2X2, + validOptions, + ); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('has 5 references but the sub-grid has 4 cells'); + }); + + it('rejects features with fewer references than sub-grid cells', () => { + const result = transformLocateGeoJson( + createFeatureCollection([locateProperties(1, [4])]), + SubGridSizeEnum.Size_2X2, + validOptions, + ); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('has 1 references but the sub-grid has 4 cells'); + }); + + it('rejects non-serial screens', () => { + const result = transformLocateGeoJson( + createFeatureCollection([ + locateProperties(1, [1, 2, 3, 4]), + locateProperties(3, [1, 2, 3, 4]), + ]), + SubGridSizeEnum.Size_2X2, + validOptions, + ); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('screens must be serial'); + expect(result.error).toContain('found: 1, 3'); + }); + + it('rejects duplicated screens', () => { + const result = transformLocateGeoJson( + createFeatureCollection([ + locateProperties(1, [1, 2, 3, 4]), + locateProperties(1, [1, 2, 3, 4]), + ]), + SubGridSizeEnum.Size_2X2, + validOptions, + ); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('duplicated screen(s): 1'); + }); + + it('rejects reference values outside the custom options', () => { + const result = transformLocateGeoJson( + createFeatureCollection([locateProperties(1, [1, 2, 3, 9])]), + SubGridSizeEnum.Size_2X2, + validOptions, + ); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('invalid reference value(s): 9 for screen 1'); + }); + + it('errors when the sub-grid size is unknown', () => { + const result = transformLocateGeoJson( + createFeatureCollection([locateProperties(1, [1, 2, 3, 4])]), + undefined, + validOptions, + ); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('Could not determine the sub-grid size'); + }); + + it('aggregates missing sub-grid size and missing custom options', () => { + const result = transformLocateGeoJson( + createFeatureCollection([locateProperties(1, [1, 2, 3, 4])]), + undefined, + undefined, + ); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + + expect(result.error).toContain('Could not determine the sub-grid size'); + expect(result.error).toContain('custom options'); + }); +}); diff --git a/app/views/EditTutorial/utils.ts b/app/views/EditTutorial/utils.ts index 4ac167e..220cec2 100644 --- a/app/views/EditTutorial/utils.ts +++ b/app/views/EditTutorial/utils.ts @@ -1,65 +1,596 @@ -import { isNotDefined } from '@togglecorp/fujs'; +import { + compareNumber, + getDuplicates, + isDefined, + isNotDefined, + listToGroupList, + unique, +} from '@togglecorp/fujs'; +import { type } from 'arktype'; +import { ulid } from 'ulid'; -export interface FindTutorialProperties { - group_id: number; - reference: number; +import { SubGridSizeEnum } from '#generated/types/graphql'; +import { + defaultTileOptions, + subgridSizeToValueMap, +} from '#utils/common'; + +import { PartialScenarioPageInputFields } from './ScenarioPageInput/schema'; +import { ComparePropertyInputFields } from './ScenarioPageInput/TaskInput/ComparePropertyInput/schema'; +import { CompletenessPropertyInputFields } from './ScenarioPageInput/TaskInput/CompletenessPropertyInput/schema'; +import { FindPropertyInputFields } from './ScenarioPageInput/TaskInput/FindPropertyInput/schema'; +import { LocateFeaturesPropertyInputFields } from './ScenarioPageInput/TaskInput/LocateFeaturesPropertyInput/schema'; +import { PartialProjectTypeSpecifics } from './ScenarioPageInput/TaskInput/schema'; +import { StreetPropertyInputFields } from './ScenarioPageInput/TaskInput/StreetPropertyInput/schema'; +import { ValidatePropertyInputFields } from './ScenarioPageInput/TaskInput/ValidatePropertyInput/schema'; + +const PositionType = type.number.array(); + +const PolygonType = type({ + type: "'Polygon'", + coordinates: PositionType.array().array(), +}); +const MultiPolygonType = type({ + type: "'MultiPolygon'", + coordinates: PositionType.array().array().array(), +}); + +const CommonFeaturePropertyType = type({ + screen: type.number, + reference: type.number, +}); +const TileFeaturePropertyType = type({ + tile_x: type.number, + tile_y: type.number, + tile_z: type.number, +}); + +const ValidateFeaturePropertyType = type.merge( + CommonFeaturePropertyType, + { + // This is not used anymore + // id: '"string" | "number"', + id: type.number, + }, +); + +const FindFeaturePropertyType = type.merge( + CommonFeaturePropertyType, + TileFeaturePropertyType, + { + // This is not used anymore + // task_id: 'string', + }, +); +const CompareFeaturePropertyType = type.merge( + CommonFeaturePropertyType, + TileFeaturePropertyType, + { + // This is not used anymore + // task_id: 'string', + }, +); +const LocateFeaturesPropertyType = type.merge( + TileFeaturePropertyType, + { + screen: type.number, + references: type.number.array(), + // This is not used anymore + // task_id: 'string', + }, +); + +const CompletenessFeaturePropertyType = type.merge( + CommonFeaturePropertyType, + TileFeaturePropertyType, + { + // This is not used anymore + // task_id: 'string', + }, +); + +const StreetFeaturePropertyType = type({ + '...': CommonFeaturePropertyType, + id: 'string', +}); + +const ValidateTutorialGeoJsonType = type({ + type: '"FeatureCollection"', + features: type({ + geometry: PolygonType.or(MultiPolygonType), + properties: ValidateFeaturePropertyType, + }).array(), +}); + +const FindTutorialGeoJsonType = type({ + type: '"FeatureCollection"', + features: type({ + geometry: PolygonType.or(MultiPolygonType), + properties: FindFeaturePropertyType, + }).array(), +}); + +const CompareTutorialGeoJsonType = type({ + type: '"FeatureCollection"', + features: type({ + geometry: PolygonType.or(MultiPolygonType), + properties: CompareFeaturePropertyType, + }).array(), +}); + +const LocateFeaturesTutorialGeoJsonType = type({ + type: '"FeatureCollection"', + features: type({ + geometry: PolygonType.or(MultiPolygonType), + properties: LocateFeaturesPropertyType, + }).array(), +}); + +const CompletenessTutorialGeoJsonType = type({ + type: '"FeatureCollection"', + features: type({ + geometry: PolygonType.or(MultiPolygonType), + properties: CompletenessFeaturePropertyType, + }).array(), +}); + +const StreetTutorialGeoJsonType = type({ + type: '"FeatureCollection"', + features: type({ + geometry: PolygonType.or(MultiPolygonType), + properties: StreetFeaturePropertyType, + }).array(), +}); + +export type TutorialGeoJsonTransformResult = { + ok: true, + scenarioPages: PartialScenarioPageInputFields[], +} | { + ok: false, + error: string, +}; + +const noOptionsError = 'Could not determine the valid reference values for this project. Please configure the custom options for the project before uploading scenarios.'; + +const tileOptionValues = defaultTileOptions.map((option) => option.value); + +interface ReferenceCustomOption { + value?: number | null | undefined; + subOptions?: ({ value?: number | null | undefined } | null | undefined)[] | null | undefined; +} + +// NOTE: mirrors the option flattening in CustomOptionSelectInput: both the +// option values and the sub-option values are selectable as a reference, but +// an option without a value is skipped along with its sub-options. +export function getValidReferenceValues( + customOptions: readonly (ReferenceCustomOption | null | undefined)[] | null | undefined, +): number[] | undefined { + if (isNotDefined(customOptions)) { + return undefined; + } + + const values = customOptions.flatMap((option) => { + if (isNotDefined(option) || isNotDefined(option.value)) { + return []; + } + + const subOptionValues = option.subOptions + ?.map((subOption) => subOption?.value) + .filter(isDefined) ?? []; + + return [option.value, ...subOptionValues]; + }); + + return values.length > 0 ? values : undefined; +} + +function checkScreensUnique(screens: number[]): string[] { + const duplicateScreens = getDuplicates(screens, (screen) => screen); + + if (duplicateScreens.length === 0) { + return []; + } + + return [`expected each screen to appear only once (duplicated screen(s): ${duplicateScreens.join(', ')})`]; +} + +function checkScreensSerial(screens: number[]): string[] { + const sortedScreens = unique(screens, (screen) => screen) + .toSorted((a, b) => compareNumber(a, b)); + + const screensAreSerial = sortedScreens.every( + (screen, index) => screen === index + 1, + ); + + if (screensAreSerial) { + return []; + } + + return [`screens must be serial (1, 2, 3, …); found: ${sortedScreens.join(', ')}`]; +} + +function checkScreenFeatureCount(screens: number[], expectedCount: number): string[] { + const groupedScreens = listToGroupList(screens, (screen) => screen); + + const mismatches = Object.values(groupedScreens).map((group) => { + if (group.length === expectedCount) { + return undefined; + } + + return { + screen: group[0], + numEntries: group.length, + }; + }).filter(isDefined); + + if (mismatches.length === 0) { + return []; + } + + const description = mismatches.map( + ({ screen, numEntries }) => `${numEntries} for screen ${screen}`, + ).join(', '); + + return [`expected to have ${expectedCount} instances of every screen (found ${description})`]; +} + +function checkReferenceValues( + references: { screen: number, reference: number }[], + validReferenceValues: number[], +): string[] { + const invalidReferences = references.filter( + ({ reference }) => !validReferenceValues.includes(reference), + ); + + if (invalidReferences.length === 0) { + return []; + } + + const description = invalidReferences.map( + ({ screen, reference }) => `${reference} for screen ${screen}`, + ).join(', '); + + return [`invalid reference value(s): ${description}; valid values are ${validReferenceValues.join(', ')}`]; +} + +// Validation common to the tile based project types, where multiple features +// share a screen and the reference values come from the default tile options. +function checkTileGroupedFeatures( + features: { properties: { screen: number, reference: number } }[], +): string[] { + const screens = features.map((feature) => feature.properties.screen); + + return [ + ...checkScreensSerial(screens), + ...checkReferenceValues( + features.map((feature) => ({ + screen: feature.properties.screen, + reference: feature.properties.reference, + })), + tileOptionValues, + ), + ]; +} + +// Validation common to the project types where each feature is its own +// scenario screen and the reference values come from the project's custom +// options. +function checkPerFeatureScenarios( + features: { properties: { screen: number, reference: number } }[], + validReferenceValues: number[] | undefined, +): string[] { + const screens = features.map((feature) => feature.properties.screen); + + const problems = [ + ...checkScreensUnique(screens), + ...checkScreensSerial(screens), + ]; + + if (isNotDefined(validReferenceValues) || validReferenceValues.length === 0) { + problems.push(noOptionsError); + return problems; + } + + problems.push(...checkReferenceValues( + features.map((feature) => ({ + screen: feature.properties.screen, + reference: feature.properties.reference, + })), + validReferenceValues, + )); + + return problems; +} + +interface TileGroupedFeatureProperties { screen: number; - task_id: number; + reference: number; tile_x: number; tile_y: number; tile_z: number; } -export type FindTutorialGeoJson = GeoJSON.FeatureCollection< - GeoJSON.Geometry, - FindTutorialProperties ->; +// Groups features by screen into one scenario page per screen, with one task +// per feature on that screen (used by the tile based project types). +function buildTileGroupedScenarioPages( + features: { properties: TileGroupedFeatureProperties }[], + buildSpecifics: (properties: TileGroupedFeatureProperties) => PartialProjectTypeSpecifics, +): PartialScenarioPageInputFields[] { + const featuresByScreen = listToGroupList( + features, + (feature) => feature.properties.screen, + ); + + return unique( + features, + (feature) => feature.properties.screen, + ).toSorted( + (a, b) => compareNumber(a.properties.screen, b.properties.screen), + ).map(({ properties }) => ({ + clientId: ulid(), + scenarioPageNumber: properties.screen, + tasks: featuresByScreen[properties.screen].map((feature) => ({ + clientId: ulid(), + reference: feature.properties.reference, + projectTypeSpecifics: buildSpecifics(feature.properties), + })), + })); +} -export function validateFindTutorialGeoJson( - geoJson: unknown, -): geoJson is FindTutorialGeoJson { - if (typeof geoJson !== 'object' || isNotDefined(geoJson)) { - return false; +// Maps each feature to its own scenario page with a single task (used by the +// project types where one feature represents one screen). +function buildPerFeatureScenarioPages< + FEATURE extends { properties: { screen: number, reference: number } }, +>( + features: FEATURE[], + buildSpecifics: (feature: FEATURE) => PartialProjectTypeSpecifics, +): PartialScenarioPageInputFields[] { + const scenarioPages = features.map((feature) => ({ + clientId: ulid(), + scenarioPageNumber: feature.properties.screen, + tasks: [ + { + clientId: ulid(), + reference: feature.properties.reference, + projectTypeSpecifics: buildSpecifics(feature), + }, + ], + })); + + return scenarioPages.toSorted((a, b) => ( + compareNumber(a.scenarioPageNumber, b.scenarioPageNumber) + )); +} + +export function transformValidateGeoJson( + geoJson: GeoJSON.GeoJSON, + validReferenceValues: number[] | undefined, +): TutorialGeoJsonTransformResult { + const result = ValidateTutorialGeoJsonType(geoJson); + if (result instanceof type.errors) { + return { ok: false, error: result.summary }; } - if (!('features' in geoJson) || !Array.isArray(geoJson.features)) { - return false; + const problems = checkPerFeatureScenarios(result.features, validReferenceValues); + if (problems.length > 0) { + return { ok: false, error: problems.join('\n') }; } - const hasInvalidFeature = geoJson.features.some((feature) => { - if ( - !('type' in feature) - || feature.type !== 'Feature' - || !('geometry' in feature) - || !('properties' in feature) - || !Array.isArray(feature.properties) - ) { - return false; - } + return { + ok: true, + scenarioPages: buildPerFeatureScenarioPages( + result.features, + (feature) => ({ + // FIXME: Why objectGeometry is string? + validate: { + identifier: feature.properties.id, + objectGeometry: JSON.stringify(feature.geometry, null, 4), + } satisfies ValidatePropertyInputFields, + }), + ), + }; +} + +export function transformFindGeoJson( + geoJson: GeoJSON.GeoJSON, +): TutorialGeoJsonTransformResult { + const result = FindTutorialGeoJsonType(geoJson); + if (result instanceof type.errors) { + return { ok: false, error: result.summary }; + } + + const problems = [ + ...checkScreenFeatureCount( + result.features.map((feature) => feature.properties.screen), + 6, + ), + ...checkTileGroupedFeatures(result.features), + ]; + if (problems.length > 0) { + return { ok: false, error: problems.join('\n') }; + } + + return { + ok: true, + scenarioPages: buildTileGroupedScenarioPages( + result.features, + (properties) => ({ + find: { + tileX: properties.tile_x, + tileY: properties.tile_y, + tileZ: properties.tile_z, + } satisfies FindPropertyInputFields, + }), + ), + }; +} + +export function transformCompareGeoJson( + geoJson: GeoJSON.GeoJSON, +): TutorialGeoJsonTransformResult { + const result = CompareTutorialGeoJsonType(geoJson); + if (result instanceof type.errors) { + return { ok: false, error: result.summary }; + } - return feature.properties.some((property: unknown) => ( - typeof property !== 'object' - || isNotDefined(property) - || !('group_id' in property) - || typeof property.group_id !== 'number' - || !('reference' in property) - || typeof property.reference !== 'number' - || !('screen' in property) - || typeof property.screen !== 'number' - || !('task_id' in property) - || typeof property.task_id !== 'number' - || !('tile_x' in property) - || typeof property.tile_x !== 'number' - || !('tile_y' in property) - || typeof property.tile_y !== 'number' - || !('tile_z' in property) - || typeof property.tile_z !== 'number' + const problems = checkTileGroupedFeatures(result.features); + if (problems.length > 0) { + return { ok: false, error: problems.join('\n') }; + } + + return { + ok: true, + scenarioPages: buildTileGroupedScenarioPages( + result.features, + (properties) => ({ + compare: { + tileX: properties.tile_x, + tileY: properties.tile_y, + tileZ: properties.tile_z, + } satisfies ComparePropertyInputFields, + }), + ), + }; +} + +export function transformCompletenessGeoJson( + geoJson: GeoJSON.GeoJSON, +): TutorialGeoJsonTransformResult { + const result = CompletenessTutorialGeoJsonType(geoJson); + if (result instanceof type.errors) { + return { ok: false, error: result.summary }; + } + + const problems = checkTileGroupedFeatures(result.features); + if (problems.length > 0) { + return { ok: false, error: problems.join('\n') }; + } + + return { + ok: true, + scenarioPages: buildTileGroupedScenarioPages( + result.features, + (properties) => ({ + completeness: { + tileX: properties.tile_x, + tileY: properties.tile_y, + tileZ: properties.tile_z, + } satisfies CompletenessPropertyInputFields, + }), + ), + }; +} + +export function transformStreetGeoJson( + geoJson: GeoJSON.GeoJSON, + validReferenceValues: number[] | undefined, +): TutorialGeoJsonTransformResult { + const result = StreetTutorialGeoJsonType(geoJson); + if (result instanceof type.errors) { + return { ok: false, error: result.summary }; + } + + const problems = checkPerFeatureScenarios(result.features, validReferenceValues); + if (problems.length > 0) { + return { ok: false, error: problems.join('\n') }; + } + + return { + ok: true, + scenarioPages: buildPerFeatureScenarioPages( + result.features, + (feature) => ({ + street: { + mapillaryImageId: feature.properties.id, + geometry: JSON.stringify(feature.geometry, null, 4), + } satisfies StreetPropertyInputFields, + }), + ), + }; +} + +export function transformLocateGeoJson( + geoJson: GeoJSON.GeoJSON, + subgridSize: SubGridSizeEnum | undefined, + validReferenceValues: number[] | undefined, +): TutorialGeoJsonTransformResult { + const result = LocateFeaturesTutorialGeoJsonType(geoJson); + if (result instanceof type.errors) { + return { ok: false, error: result.summary }; + } + + const screens = result.features.map((feature) => feature.properties.screen); + + // Each Locate feature is one parent tile rendered as a single scenario + // screen, so a screen must not appear on more than one feature. + const problems = [ + ...checkScreensUnique(screens), + ...checkScreensSerial(screens), + ]; + + const numSubGrids = isDefined(subgridSize) + ? (2 ** subgridSizeToValueMap[subgridSize]) ** 2 + : undefined; + + if (isNotDefined(numSubGrids)) { + problems.push('Could not determine the sub-grid size for this project. Please configure the project before uploading scenarios.'); + } else { + // Each feature should carry exactly one reference per sub-grid cell. + result.features.forEach((feature) => { + if (feature.properties.references.length !== numSubGrids) { + problems.push( + `Feature on screen ${feature.properties.screen} has ${feature.properties.references.length} references but the sub-grid has ${numSubGrids} cells.`, + ); + } + }); + } + + if (isNotDefined(validReferenceValues) || validReferenceValues.length === 0) { + problems.push(noOptionsError); + } else { + problems.push(...checkReferenceValues( + result.features.flatMap((feature) => ( + feature.properties.references.map((reference) => ({ + screen: feature.properties.screen, + reference, + })) + )), + validReferenceValues, )); - }); + } - if (hasInvalidFeature) { - return false; + if (isNotDefined(numSubGrids) || problems.length > 0) { + return { ok: false, error: problems.join('\n') }; } - return true; + const featuresByScreen = listToGroupList( + result.features, + (feature) => feature.properties.screen, + ); + + const scenarioPages: PartialScenarioPageInputFields[] = unique( + result.features, + (feature) => feature.properties.screen, + ).toSorted( + (a, b) => compareNumber(a.properties.screen, b.properties.screen), + ).map(({ properties }) => ({ + clientId: ulid(), + scenarioPageNumber: properties.screen, + tasks: featuresByScreen[properties.screen].flatMap((feature) => ( + Array.from(new Array(numSubGrids).keys()).map((index) => ({ + clientId: ulid(), + reference: feature.properties.references[index], + taskPartitionIndex: index, + projectTypeSpecifics: { + locate: { + tileX: feature.properties.tile_x, + tileY: feature.properties.tile_y, + tileZ: feature.properties.tile_z, + } satisfies LocateFeaturesPropertyInputFields, + }, + })) + )), + })); + + return { ok: true, scenarioPages }; } diff --git a/vite.config.ts b/vite.config.ts index 360cba1..7a300f7 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -83,7 +83,10 @@ export default defineConfig(({ mode }) => { }, test: { - environment: 'happy-dom', + // NOTE: happy-dom is configured nowhere in dependencies; use node + // until DOM-based tests are actually added + environment: 'node', + dir: 'app', }, }; }); From 17b5db428501eefab8a557536583122716c3ad13 Mon Sep 17 00:00:00 2001 From: tnagorra Date: Fri, 5 Jun 2026 13:58:04 +0545 Subject: [PATCH 3/3] refactor: add notes and re-organization --- app/views/EditTutorial/index.tsx | 6 ++-- app/views/EditTutorial/utils.ts | 50 +++++++++++++++++--------------- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/app/views/EditTutorial/index.tsx b/app/views/EditTutorial/index.tsx index 11daed0..36f17eb 100644 --- a/app/views/EditTutorial/index.tsx +++ b/app/views/EditTutorial/index.tsx @@ -528,7 +528,7 @@ function NewTutorial() { // eslint-disable-next-line no-underscore-dangle const validReferenceValues = projectTypeSpecifics?.__typename === 'ValidateProjectPropertyType' ? getValidReferenceValues(projectTypeSpecifics.customOptions) - : undefined; + : []; result = transformValidateGeoJson(geoJson, validReferenceValues); } else if (projectType === ProjectTypeEnum.Find) { @@ -541,7 +541,7 @@ function NewTutorial() { // eslint-disable-next-line no-underscore-dangle const validReferenceValues = projectTypeSpecifics?.__typename === 'StreetProjectPropertyType' ? getValidReferenceValues(projectTypeSpecifics.customOptions) - : undefined; + : []; result = transformStreetGeoJson(geoJson, validReferenceValues); } else if (projectType === ProjectTypeEnum.Locate) { @@ -552,7 +552,7 @@ function NewTutorial() { : undefined; const validReferenceValues = isLocateProperty ? getValidReferenceValues(projectTypeSpecifics.customOptions) - : undefined; + : []; result = transformLocateGeoJson(geoJson, subgridSize, validReferenceValues); } else { diff --git a/app/views/EditTutorial/utils.ts b/app/views/EditTutorial/utils.ts index 220cec2..fd0a1a9 100644 --- a/app/views/EditTutorial/utils.ts +++ b/app/views/EditTutorial/utils.ts @@ -4,6 +4,7 @@ import { isDefined, isNotDefined, listToGroupList, + Maybe, unique, } from '@togglecorp/fujs'; import { type } from 'arktype'; @@ -152,21 +153,22 @@ export type TutorialGeoJsonTransformResult = { const noOptionsError = 'Could not determine the valid reference values for this project. Please configure the custom options for the project before uploading scenarios.'; +// FIXME: Get this from the server const tileOptionValues = defaultTileOptions.map((option) => option.value); interface ReferenceCustomOption { - value?: number | null | undefined; - subOptions?: ({ value?: number | null | undefined } | null | undefined)[] | null | undefined; + value?: Maybe; + subOptions?: Maybe }>[]>; } // NOTE: mirrors the option flattening in CustomOptionSelectInput: both the // option values and the sub-option values are selectable as a reference, but // an option without a value is skipped along with its sub-options. export function getValidReferenceValues( - customOptions: readonly (ReferenceCustomOption | null | undefined)[] | null | undefined, -): number[] | undefined { + customOptions: Maybe[]>, +): number[] { if (isNotDefined(customOptions)) { - return undefined; + return []; } const values = customOptions.flatMap((option) => { @@ -181,7 +183,7 @@ export function getValidReferenceValues( return [option.value, ...subOptionValues]; }); - return values.length > 0 ? values : undefined; + return values; } function checkScreensUnique(screens: number[]): string[] { @@ -196,7 +198,7 @@ function checkScreensUnique(screens: number[]): string[] { function checkScreensSerial(screens: number[]): string[] { const sortedScreens = unique(screens, (screen) => screen) - .toSorted((a, b) => compareNumber(a, b)); + .toSorted(compareNumber); const screensAreSerial = sortedScreens.every( (screen, index) => screen === index + 1, @@ -210,7 +212,11 @@ function checkScreensSerial(screens: number[]): string[] { } function checkScreenFeatureCount(screens: number[], expectedCount: number): string[] { - const groupedScreens = listToGroupList(screens, (screen) => screen); + // FIXME: We can identify mismatches using listToGroupList transformer + const groupedScreens = listToGroupList( + screens, + (screen) => screen, + ); const mismatches = Object.values(groupedScreens).map((group) => { if (group.length === expectedCount) { @@ -231,7 +237,7 @@ function checkScreenFeatureCount(screens: number[], expectedCount: number): stri ({ screen, numEntries }) => `${numEntries} for screen ${screen}`, ).join(', '); - return [`expected to have ${expectedCount} instances of every screen (found ${description})`]; + return [`expected to have ${expectedCount} instance(s) of every screen (found ${description})`]; } function checkReferenceValues( @@ -261,6 +267,7 @@ function checkTileGroupedFeatures( const screens = features.map((feature) => feature.properties.screen); return [ + ...checkScreenFeatureCount(screens, 6), ...checkScreensSerial(screens), ...checkReferenceValues( features.map((feature) => ({ @@ -275,7 +282,7 @@ function checkTileGroupedFeatures( // Validation common to the project types where each feature is its own // scenario screen and the reference values come from the project's custom // options. -function checkPerFeatureScenarios( +function checkSingletonFeatures( features: { properties: { screen: number, reference: number } }[], validReferenceValues: number[] | undefined, ): string[] { @@ -290,7 +297,6 @@ function checkPerFeatureScenarios( problems.push(noOptionsError); return problems; } - problems.push(...checkReferenceValues( features.map((feature) => ({ screen: feature.properties.screen, @@ -316,6 +322,7 @@ function buildTileGroupedScenarioPages( features: { properties: TileGroupedFeatureProperties }[], buildSpecifics: (properties: TileGroupedFeatureProperties) => PartialProjectTypeSpecifics, ): PartialScenarioPageInputFields[] { + // FIXME: We can easily process using listToGroupList transformer const featuresByScreen = listToGroupList( features, (feature) => feature.properties.screen, @@ -339,7 +346,7 @@ function buildTileGroupedScenarioPages( // Maps each feature to its own scenario page with a single task (used by the // project types where one feature represents one screen). -function buildPerFeatureScenarioPages< +function buildSingletonScenarioPages< FEATURE extends { properties: { screen: number, reference: number } }, >( features: FEATURE[], @@ -371,14 +378,14 @@ export function transformValidateGeoJson( return { ok: false, error: result.summary }; } - const problems = checkPerFeatureScenarios(result.features, validReferenceValues); + const problems = checkSingletonFeatures(result.features, validReferenceValues); if (problems.length > 0) { return { ok: false, error: problems.join('\n') }; } return { ok: true, - scenarioPages: buildPerFeatureScenarioPages( + scenarioPages: buildSingletonScenarioPages( result.features, (feature) => ({ // FIXME: Why objectGeometry is string? @@ -399,13 +406,7 @@ export function transformFindGeoJson( return { ok: false, error: result.summary }; } - const problems = [ - ...checkScreenFeatureCount( - result.features.map((feature) => feature.properties.screen), - 6, - ), - ...checkTileGroupedFeatures(result.features), - ]; + const problems = checkTileGroupedFeatures(result.features); if (problems.length > 0) { return { ok: false, error: problems.join('\n') }; } @@ -490,14 +491,14 @@ export function transformStreetGeoJson( return { ok: false, error: result.summary }; } - const problems = checkPerFeatureScenarios(result.features, validReferenceValues); + const problems = checkSingletonFeatures(result.features, validReferenceValues); if (problems.length > 0) { return { ok: false, error: problems.join('\n') }; } return { ok: true, - scenarioPages: buildPerFeatureScenarioPages( + scenarioPages: buildSingletonScenarioPages( result.features, (feature) => ({ street: { @@ -559,10 +560,11 @@ export function transformLocateGeoJson( )); } - if (isNotDefined(numSubGrids) || problems.length > 0) { + if (problems.length > 0) { return { ok: false, error: problems.join('\n') }; } + // FIXME: We can easily process using listToGroupList transformer const featuresByScreen = listToGroupList( result.features, (feature) => feature.properties.screen,