From d31801218cfd6ba11415af7206758cb0a923b604 Mon Sep 17 00:00:00 2001 From: shreeyash07 Date: Fri, 30 Jun 2023 15:46:22 +0545 Subject: [PATCH 1/6] Add screen and reference validations --- .../app/views/NewTutorial/index.tsx | 38 ++++++++++- .../app/views/NewTutorial/utils.ts | 67 +++++++++++++++++-- 2 files changed, 95 insertions(+), 10 deletions(-) diff --git a/manager-dashboard/app/views/NewTutorial/index.tsx b/manager-dashboard/app/views/NewTutorial/index.tsx index 07a11f0f3..a91513685 100644 --- a/manager-dashboard/app/views/NewTutorial/index.tsx +++ b/manager-dashboard/app/views/NewTutorial/index.tsx @@ -87,6 +87,9 @@ import CustomOptionInput from './CustomOptionInput'; import ScenarioPageInput from './ScenarioPageInput'; import InformationPageInput from './InformationPageInput'; import styles from './styles.css'; +import { TutorialTasksGeoJSON } from './utils'; +import { FootprintProperties } from './utils'; +import { BuildAreaProperties, ChangeDetectionProperties } from './utils'; type CustomScreen = Omit; function sanitizeScreens(scenarioPages: TutorialFormType['scenarioPages']) { @@ -351,8 +354,34 @@ function NewTutorial(props: Props) { const handleGeoJsonFile = React.useCallback(( geoProps: GeoJSON.GeoJSON | undefined, ) => { - const tutorialTasks = geoProps as PartialTutorialFormType['tutorialTasks']; + const tutorialTasks = geoProps as TutorialTasksGeoJSON; + if (!tutorialTasks) { + return; + } + + const everyTaskHasScreenAndReferenceProperty = tutorialTasks.features.every( + (feature) => isDefined(feature.properties.screen) && isDefined(feature.properties.reference), + ); + + if (!everyTaskHasScreenAndReferenceProperty) { + setError((prevValue) => ({ + ...getErrorObject(prevValue), + tutorialTasks: 'GeoJson does not contain property "screen" or "reference"', + })); + + return; + } + if (value?.projectType === PROJECT_TYPE_COMPLETENESS + || value?.projectType === PROJECT_TYPE_BUILD_AREA + || value?.projectType === PROJECT_TYPE_CHANGE_DETECTION) { + + tutorialTasks.features.every( + (feature: BuildAreaProperties | ChangeDetectionProperties) => isDefined(feature.properties.task_id), + ); + } + + setFieldValue(tutorialTasks, 'tutorialTasks'); // FIXME: we need to validate the geojson here @@ -369,9 +398,13 @@ function NewTutorial(props: Props) { success: {}, } )); + setFieldValue(tutorialTaskArray, 'scenarioPages'); + }, [setFieldValue]); + console.log(formError); + const handleAddInformationPage = React.useCallback( (template: InformationPageTemplateKey) => { setFieldValue( @@ -447,7 +480,6 @@ function NewTutorial(props: Props) { customOptions, informationPages, } = value; - return (
@@ -619,7 +651,7 @@ function NewTutorial(props: Props) { openByDefault={i === informationPages.length - 1} actions={( )}
diff --git a/manager-dashboard/app/views/NewTutorial/ScenarioPageInput/FootprintGeoJsonPreview/index.tsx b/manager-dashboard/app/views/NewTutorial/ScenarioPageInput/FootprintGeoJsonPreview/index.tsx index b81f83239..e5971e7dd 100644 --- a/manager-dashboard/app/views/NewTutorial/ScenarioPageInput/FootprintGeoJsonPreview/index.tsx +++ b/manager-dashboard/app/views/NewTutorial/ScenarioPageInput/FootprintGeoJsonPreview/index.tsx @@ -69,7 +69,7 @@ export default function FootprintGeoJsonPreview(props: Props) { {customOptions?.map((option) => { const Icon = option.icon ? iconMap[option.icon] - : iconMap.flagOutline; + : iconMap['flag-outline']; return (
[number]>['blocks']; -function checkDuplicate(array: Array) { - const valueCounts: Record = {}; - for (let index = 0; index < array.length; index += 1) { - const item = array[index]; - if (isNotDefined(item)) { - // eslint-disable-next-line - continue - } - if (valueCounts[item] !== undefined) { - valueCounts[item] += 1; - return true; - } - valueCounts[item] = 1; - } - return false; -} - export const MAX_OPTIONS = 6; export const MIN_OPTIONS = 2; export const MAX_SUB_OPTIONS = 6; @@ -635,11 +621,7 @@ export const tutorialFormSchema: TutorialFormSchema = { const allOptionValue = [...optionValue, ...subOptionValue]; - const hasDuplicateOptionValue = checkDuplicate( - options.map((val) => val.value).filter(isDefined), - ); - - const hasDuplicateAllOptionValue = checkDuplicate(allOptionValue); + const hasDuplicateAllOptionValue = getDuplicates(allOptionValue, (k) => k); if (options.length < MIN_OPTIONS) { return `There should be at least ${MIN_OPTIONS} options`; @@ -648,12 +630,10 @@ export const tutorialFormSchema: TutorialFormSchema = { if (options.length > MAX_OPTIONS) { return `There shouldn\`t be more than ${MAX_OPTIONS} options`; } - - if (hasDuplicateOptionValue) { - return 'Value cannot be same'; - } - if (hasDuplicateAllOptionValue) { - return 'Option and suboptions value cannot be same'; + if (hasDuplicateAllOptionValue.length > 0) { + return `The value for options/sub-options are duplicated : ${ + hasDuplicateAllOptionValue.map((duplicate) => duplicate) + }`; } return undefined; }, @@ -693,20 +673,12 @@ export const tutorialFormSchema: TutorialFormSchema = { if (!sub || sub.length === 0) { return undefined; } - - const hasDuplicateSubOptionValue = checkDuplicate( - sub.map((val) => val.value).filter(isDefined), - ); - if (sub.length < MIN_SUB_OPTIONS) { - return `There should be at least ${MIN_SUB_OPTIONS} sub options`; + return `There should be at least ${MIN_SUB_OPTIONS} sub-options`; } if (sub.length > MAX_SUB_OPTIONS) { - return `There shouldn\`t be more than ${MAX_SUB_OPTIONS} sub options`; - } - if (hasDuplicateSubOptionValue) { - return 'Value cannot be same'; + return `There shouldn\`t be more than ${MAX_SUB_OPTIONS} sub-options`; } return undefined; From 4922ad473d79d7264f23df1a590b9d6fa2233c77 Mon Sep 17 00:00:00 2001 From: tnagorra Date: Mon, 3 Jul 2023 18:51:38 +0545 Subject: [PATCH 6/6] Add validation for uploaded geojson --- .../app/components/AlertBanner/index.tsx | 13 +- .../app/components/AlertBanner/styles.css | 8 +- .../app/components/GeoJsonPreview/index.tsx | 2 + .../app/views/NewTutorial/index.tsx | 371 +++++++++++++----- .../app/views/NewTutorial/styles.css | 16 + .../app/views/NewTutorial/utils.ts | 30 +- 6 files changed, 314 insertions(+), 126 deletions(-) diff --git a/manager-dashboard/app/components/AlertBanner/index.tsx b/manager-dashboard/app/components/AlertBanner/index.tsx index ea495104b..08e439be3 100644 --- a/manager-dashboard/app/components/AlertBanner/index.tsx +++ b/manager-dashboard/app/components/AlertBanner/index.tsx @@ -2,10 +2,13 @@ import React from 'react'; import { IoAlertCircleOutline } from 'react-icons/io5'; import { _cs } from '@togglecorp/fujs'; +import Heading from '#components/Heading'; + import styles from './styles.css'; interface Props { className?: string; + title?: React.ReactNode; children: React.ReactNode; } @@ -13,6 +16,7 @@ function AlertBanner(props: Props) { const { className, children, + title, } = props; return ( @@ -20,7 +24,14 @@ function AlertBanner(props: Props) {
- {children} +
+ {title && ( + + {title} + + )} + {children} +
); } diff --git a/manager-dashboard/app/components/AlertBanner/styles.css b/manager-dashboard/app/components/AlertBanner/styles.css index 1175d9b3a..770bb8e83 100644 --- a/manager-dashboard/app/components/AlertBanner/styles.css +++ b/manager-dashboard/app/components/AlertBanner/styles.css @@ -8,4 +8,10 @@ .banner-icon { font-size: var(--font-size-super-large); } -} \ No newline at end of file + + .container { + display: flex; + flex-direction: column; + gap: var(--spacing-small); + } +} diff --git a/manager-dashboard/app/components/GeoJsonPreview/index.tsx b/manager-dashboard/app/components/GeoJsonPreview/index.tsx index ec944d54e..b881e96e9 100644 --- a/manager-dashboard/app/components/GeoJsonPreview/index.tsx +++ b/manager-dashboard/app/components/GeoJsonPreview/index.tsx @@ -89,6 +89,8 @@ function GeoJsonPreview(props: Props) { const layer = new Layer( finalUrl, { + // NOTE: we have a limit of 22 + maxZoom: 22, // attribution: '', // subdomains: ['a', 'b', 'c'], }, diff --git a/manager-dashboard/app/views/NewTutorial/index.tsx b/manager-dashboard/app/views/NewTutorial/index.tsx index e6fa3e520..433c9dca0 100644 --- a/manager-dashboard/app/views/NewTutorial/index.tsx +++ b/manager-dashboard/app/views/NewTutorial/index.tsx @@ -5,6 +5,8 @@ import { unique, isNotDefined, isTruthyString, + difference, + listToMap, } from '@togglecorp/fujs'; import { useForm, @@ -34,6 +36,9 @@ import { import { IoIosTrash, } from 'react-icons/io'; +import { + IoInformationCircleOutline, +} from 'react-icons/io5'; import { Link } from 'react-router-dom'; import UserContext from '#base/context/UserContext'; @@ -84,6 +89,8 @@ import { MAX_OPTIONS, deleteKey, TutorialTasksGeoJSON, + BuildAreaProperties, + ChangeDetectionProperties, } from './utils'; import CustomOptionPreview from './CustomOptionInput/CustomOptionPreview'; @@ -92,6 +99,219 @@ import ScenarioPageInput from './ScenarioPageInput'; import InformationPageInput from './InformationPageInput'; import styles from './styles.css'; +export function getDuplicates( + list: T[], + keySelector: (item: T) => K, + filter: (count: number) => boolean = (count) => count > 1, +) { + const counts = listToMap( + list, + keySelector, + (_, key, __, acc) => { + const value: number | undefined = acc[key]; + return isDefined(value) ? value + 1 : 1; + }, + ); + return Object.keys(counts).filter((key) => filter(counts[key as K])); +} + +type ValidType ='number' | 'string' | 'boolean'; +function checkSchema( + obj: T, + schema: Record, +) { + const schemaKeys = Object.keys(schema); + const errors = schemaKeys.map( + (key) => { + const expectedType = schema[key]; + + const keySafe = key as keyof T; + const currentValue: unknown = obj[keySafe]; + const valueType = typeof currentValue; + + if (Array.isArray(expectedType)) { + const indexOfType = expectedType.findIndex( + (type) => type === valueType, + ); + if (indexOfType === -1) { + return `type of ${key} expected to be one of type ${expectedType.join(', ')}`; + } + } else if (typeof currentValue !== expectedType) { + return `type of ${key} expected to be of ${expectedType}`; + } + + return undefined; + }, + ).filter(isDefined); + + return errors; +} + +function getGeoJSONError( + tutorialTasks: TutorialTasksGeoJSON, + projectType: ProjectType, +) { + if (isNotDefined(tutorialTasks.features) || !Array.isArray(tutorialTasks.features)) { + return 'GeoJSON does not contain iterable features'; + } + + // Check properties schema + const projectSchemas: { + [key in ProjectType]: Record; + } = { + [PROJECT_TYPE_FOOTPRINT]: { + id: ['string', 'number'], + reference: 'number', + screen: 'number', + }, + [PROJECT_TYPE_CHANGE_DETECTION]: { + reference: 'number', + screen: 'number', + task_id: 'string', + tile_x: 'number', + tile_y: 'number', + tile_z: 'number', + }, + [PROJECT_TYPE_BUILD_AREA]: { + reference: 'number', + screen: 'number', + task_id: 'string', + tile_x: 'number', + tile_y: 'number', + tile_z: 'number', + }, + [PROJECT_TYPE_COMPLETENESS]: { + reference: 'number', + screen: 'number', + task_id: 'string', + tile_x: 'number', + tile_y: 'number', + tile_z: 'number', + }, + }; + const schemaErrors = tutorialTasks.features.map( + (feature) => checkSchema( + feature.properties, + projectSchemas[projectType], + ).join(', '), + ).filter(isTruthyString); + if (schemaErrors.length > 0) { + return `Invalid GeoJSON for Footprint: ${schemaErrors[0]} (${schemaErrors.length} total errors)`; + } + + return undefined; +} + +function getGeoJSONWarning( + tutorialTasks: TutorialTasksGeoJSON | undefined, + projectType: ProjectType | undefined, + customOptions: number[] | undefined, + maxZoom: number | undefined, +) { + const errors = []; + + const screens = tutorialTasks?.features?.map( + (feature) => feature.properties.screen, + ) ?? []; + if (screens.length > 0) { + const minScreen = Math.min(...screens); + const maxScreen = Math.max(...screens); + const totalScreen = maxScreen - minScreen + 1; + + if (minScreen !== 1) { + errors.push(`Screen in GeoJSON should start from 1. The first screen is ${minScreen}`); + } + + const actualScreens = new Set(screens); + const expectedScreens = new Set(Array.from( + { length: totalScreen }, + (_, index) => minScreen + index, + )); + + const missingScreens = difference( + expectedScreens, + actualScreens, + ); + + if (missingScreens.size === 1) { + errors.push(`Screen in GeoJSON should be sequential. The missing screen is ${[...missingScreens].sort().join(', ')}`); + } else if (missingScreens.size > 1) { + errors.push(`Screen in GeoJSON should be sequential. The missing screens are ${[...missingScreens].sort().join(', ')}`); + } + + if ( + projectType === PROJECT_TYPE_BUILD_AREA + || projectType === PROJECT_TYPE_COMPLETENESS + ) { + const dups = getDuplicates( + screens, + (item) => item, + (count) => count !== 6, + ); + if (dups.length === 1) { + errors.push(`There should be exactly 6 squares with same screen in GeoJSON. The invalid screen is ${dups.join(', ')}`); + } else if (dups.length > 1) { + errors.push(`There should be exactly 6 squares with same screen in GeoJSON. The invalid screens are ${dups.join(', ')}`); + } + } else if ( + projectType === PROJECT_TYPE_CHANGE_DETECTION + || projectType === PROJECT_TYPE_FOOTPRINT + ) { + const dups = getDuplicates( + screens, + (item) => item, + (count) => count > 1, + ); + if (dups.length === 1) { + errors.push(`Screen in GeoJSON should not be duplicated. The duplicated screen is ${dups.join(', ')}`); + } else if (dups.length > 1) { + errors.push(`Screen in GeoJSON should not be duplicated. The duplicated screens are ${dups.join(', ')}`); + } + } + } + + if ( + projectType === PROJECT_TYPE_BUILD_AREA + || projectType === PROJECT_TYPE_CHANGE_DETECTION + || projectType === PROJECT_TYPE_COMPLETENESS + ) { + type LocalTutorialTasksGeoJSON = GeoJSON.FeatureCollection< + GeoJSON.Geometry, + BuildAreaProperties | ChangeDetectionProperties + >; + const zooms = (tutorialTasks as LocalTutorialTasksGeoJSON)?.features?.map( + (feature) => feature.properties.tile_z, + ) ?? []; + const uniqueZooms = new Set(zooms); + if (uniqueZooms.size > 1) { + errors.push(`Zoom in GeoJSON should be all be the same. Zoom should be either ${[...uniqueZooms].sort().join(' or ')}`); + } else if (isDefined(maxZoom) && uniqueZooms.size === 1) { + const zoom = [...uniqueZooms][0]; + if (zoom !== maxZoom) { + errors.push(`Zoom in GeoJSON does not match defined zoom level. ${zoom} != ${maxZoom}`); + } + } + } + + // Check if options are not available + const references = tutorialTasks?.features?.map( + (feature) => feature.properties.reference, + ) ?? []; + const selectedOptionsSet = new Set(references); + const availableOptionsSet = projectType === PROJECT_TYPE_FOOTPRINT + ? customOptions ?? [] + : [0, 1, 2, 3]; + + const invalidOptions = difference(new Set(selectedOptionsSet), new Set(availableOptionsSet)); + if (invalidOptions.size === 1) { + errors.push(`Reference in GeoJSON should be either ${availableOptionsSet.join(', ')}. The invalid reference is ${[...invalidOptions].join(', ')}`); + } else if (invalidOptions.size > 1) { + errors.push(`Reference in GeoJSON should be either ${availableOptionsSet.join(', ')}. The invalid references are ${[...invalidOptions].sort().join(', ')}`); + } + + return errors; +} + type CustomScreen = Omit; function sanitizeScreens(scenarioPages: TutorialFormType['scenarioPages']) { const screens = scenarioPages.reduce>( @@ -355,122 +575,16 @@ function NewTutorial(props: Props) { const handleGeoJsonFile = React.useCallback(( geoProps: GeoJSON.GeoJSON | undefined, ) => { + const projectType = value?.projectType; const tutorialTasks = geoProps as TutorialTasksGeoJSON; - function getGeoJSONError() { - if (isNotDefined(tutorialTasks.features) || !Array.isArray(tutorialTasks.features)) { - return 'GeoJson does not contain iterable properties'; - } - - type ValidType ='number' | 'string' | 'boolean'; - function checkSchema( - obj: T, - schema: Record, - ) { - const schemaKeys = Object.keys(schema); - const errors = schemaKeys.map( - (key) => { - const expectedType = schema[key]; - - const keySafe = key as keyof T; - const currentValue: unknown = obj[keySafe]; - const valueType = typeof currentValue; - - if (Array.isArray(expectedType)) { - const indexOfType = expectedType.findIndex( - (type) => type === valueType, - ); - if (indexOfType === -1) { - return `type of ${key} expected to be one of type ${expectedType.join(', ')}`; - } - } else if (typeof currentValue !== expectedType) { - return `type of ${key} expected to be of ${expectedType}`; - } - - return undefined; - }, - ).filter(isDefined); - - return errors; - } - - if (value?.projectType === PROJECT_TYPE_FOOTPRINT) { - const errors = tutorialTasks.features.map( - (feature) => checkSchema(feature.properties, { - id: ['string', 'number'], - reference: 'number', - screen: 'number', - }).join(', '), - ).filter(isTruthyString); - - if (errors.length > 0) { - return `Invalid GeoJson for Footprint: ${errors[0]} (${errors.length} total errors)`; - } - } - - if (value?.projectType === PROJECT_TYPE_CHANGE_DETECTION) { - const errors = tutorialTasks.features.map( - (feature) => checkSchema(feature.properties, { - reference: 'number', - screen: 'number', - task_id: 'string', - tile_x: 'number', - tile_y: 'number', - tile_z: 'number', - }).join(', '), - ).filter(isTruthyString); - - if (errors.length > 0) { - // NOTE: only showing errors first error - return `Invalid GeoJson for Change Detection: ${errors[0]} (${errors.length} total errors)`; - } - } - - if (value?.projectType === PROJECT_TYPE_BUILD_AREA) { - const errors = tutorialTasks.features.map( - (feature) => checkSchema(feature.properties, { - reference: 'number', - screen: 'number', - task_id: 'string', - tile_x: 'number', - tile_y: 'number', - tile_z: 'number', - }).join(', '), - ).filter(isTruthyString); - - if (errors.length > 0) { - // NOTE: only showing errors first error - return `Invalid GeoJson for Build Area: ${errors[0]} (${errors.length} total errors)`; - } - } - if (value?.projectType === PROJECT_TYPE_COMPLETENESS) { - const errors = tutorialTasks.features.map( - (feature) => checkSchema(feature.properties, { - reference: 'number', - screen: 'number', - task_id: 'string', - tile_x: 'number', - tile_y: 'number', - tile_z: 'number', - }).join(', '), - ).filter(isTruthyString); - - if (errors.length > 0) { - return `Invalid GeoJson for Completeness: ${errors[0]} (${errors.length} total errors)`; - } - } - // TODO: validate references - - return undefined; - } - - if (!tutorialTasks) { + if (!tutorialTasks || !projectType) { setFieldValue(undefined, 'tutorialTasks'); setFieldValue(undefined, 'scenarioPages'); return; } - const errors = getGeoJSONError(); + const errors = getGeoJSONError(tutorialTasks, projectType); if (errors) { setFieldValue(undefined, 'tutorialTasks'); setFieldValue(undefined, 'scenarioPages'); @@ -558,6 +672,26 @@ function NewTutorial(props: Props) { [error], ); + const warning = React.useMemo( + () => { + const options = value?.customOptions?.map((option) => option.value) ?? []; + const subOptions = value?.customOptions?.flatMap( + (option) => option.subOptions?.map((subOption) => subOption.value), + ) ?? []; + const selectedValues = [ + ...options, + ...subOptions, + ].filter(isDefined); + return getGeoJSONWarning( + value?.tutorialTasks, + value?.projectType, + selectedValues, + value?.zoomLevel, + ); + }, + [value?.tutorialTasks, value?.projectType, value?.customOptions, value?.zoomLevel], + ); + const getTileServerUrl = (val: PartialTutorialFormType['tileServer']) => { const tileServerName = val?.name; if (isNotDefined(tileServerName)) { @@ -873,6 +1007,25 @@ function NewTutorial(props: Props) { )} + {warning.length > 0 && ( + +
+ {warning.map((item) => ( +
+
+ +
+ {item} +
+ ))} +
+ + )} {hasErrors && (
Please correct all the errors above before submission! diff --git a/manager-dashboard/app/views/NewTutorial/styles.css b/manager-dashboard/app/views/NewTutorial/styles.css index a7bd9dd78..ffef8723b 100644 --- a/manager-dashboard/app/views/NewTutorial/styles.css +++ b/manager-dashboard/app/views/NewTutorial/styles.css @@ -62,6 +62,22 @@ } } + .warning-container { + display: flex; + flex-direction: column; + gap: var(--spacing-extra-small); + + .warning-item { + display: flex; + gap: var(--spacing-small); + align-items: flex-start; + + .icon-container { + font-size: var(--font-size-large); + } + } + } + .error-message { text-align: center; color: var(--color-danger); diff --git a/manager-dashboard/app/views/NewTutorial/utils.ts b/manager-dashboard/app/views/NewTutorial/utils.ts index bf1b76a02..0a3bc9349 100644 --- a/manager-dashboard/app/views/NewTutorial/utils.ts +++ b/manager-dashboard/app/views/NewTutorial/utils.ts @@ -608,31 +608,31 @@ export const tutorialFormSchema: TutorialFormSchema = { return undefined; } - const optionValue = options.flatMap( + if (options.length < MIN_OPTIONS) { + return `There should be at least ${MIN_OPTIONS} options`; + } + + if (options.length > MAX_OPTIONS) { + return `There shouldn\`t be more than ${MAX_OPTIONS} options`; + } + + const optionValues = options.flatMap( (val) => val.value, ).filter(isDefined); - const subOptionValue = options.flatMap((val) => { + const subOptionValues = options.flatMap((val) => { const subValue = val.subOptions?.map( (sub) => sub.value, ); return subValue; }).filter(isDefined); - const allOptionValue = [...optionValue, ...subOptionValue]; + const allOptionValues = [...optionValues, ...subOptionValues]; - const hasDuplicateAllOptionValue = getDuplicates(allOptionValue, (k) => k); - - if (options.length < MIN_OPTIONS) { - return `There should be at least ${MIN_OPTIONS} options`; - } - - if (options.length > MAX_OPTIONS) { - return `There shouldn\`t be more than ${MAX_OPTIONS} options`; - } - if (hasDuplicateAllOptionValue.length > 0) { - return `The value for options/sub-options are duplicated : ${ - hasDuplicateAllOptionValue.map((duplicate) => duplicate) + const hasDuplicateValue = getDuplicates(allOptionValues, (k) => k); + if (hasDuplicateValue.length > 0) { + return `The value for options and sub-options are duplicated: ${ + hasDuplicateValue.map((duplicate) => duplicate).join(', ') }`; } return undefined;