Skip to content

Commit

Permalink
Prevent the user from choosing an incorrect variable type
Browse files Browse the repository at this point in the history
  • Loading branch information
mthh committed Sep 17, 2024
1 parent d0d99a3 commit 33fb72a
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 6 deletions.
7 changes: 5 additions & 2 deletions src/components/Modals/FieldTypingModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,17 @@ export default function FieldTypingModal(
}

const hasDuplicates: Record<string, boolean> = {};
const hasOnlyOneModality: Record<string, boolean> = {};

dataset.fields.forEach((field) => {
const varName = field.name;
const values = key === 'layers'
? (dataset.data as GeoJSONFeatureCollection).features.map((f) => f.properties[varName])
: (dataset.data as Record<string, any>[]).map((f) => f[varName]);
const filteredValues = values.filter((v) => v !== null && v !== '' && v !== undefined);
hasDuplicates[varName] = filteredValues.length !== (new Set(filteredValues)).size;
const dedupValues = new Set(filteredValues);
hasDuplicates[varName] = filteredValues.length !== dedupValues.size;
hasOnlyOneModality[varName] = dedupValues.size === 1;
});

const descriptions = unproxify(dataset.fields as never) as Variable[];
Expand Down Expand Up @@ -143,7 +146,7 @@ export default function FieldTypingModal(
{LL().FieldsTyping.VariableTypes.stock()}
</option>
</Show>
<Show when={field.dataType === 'number'}>
<Show when={field.dataType === 'number' && !hasOnlyOneModality[field.name]}>
<option value="ratio" selected={field.type === VariableType.ratio}>
{LL().FieldsTyping.VariableTypes.ratio()}
</option>
Expand Down
60 changes: 56 additions & 4 deletions src/components/Modals/TableWindow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import {
import AgGridSolid, { AgGridSolidRef } from 'ag-grid-solid';
import 'ag-grid-community/styles/ag-grid.min.css'; // grid core CSS
import 'ag-grid-community/styles/ag-theme-quartz.min.css'; // theme

// Imports from other packages
import alasql from 'alasql';
import { type AllGeoJSON, area } from '@turf/turf';
import type { LocalizedString } from 'typesafe-i18n';
import toast from 'solid-toast';

// Helpers
import { useI18nContext } from '../../i18n/i18n-solid';
Expand All @@ -27,6 +27,7 @@ import d3 from '../../helpers/d3-custom';
import { isDarkMode } from '../../helpers/darkmode';
import { clickLinkFromBlob } from '../../helpers/exports';
import {
type DataType,
detectTypeField,
type Variable,
VariableType,
Expand Down Expand Up @@ -344,6 +345,40 @@ const getHandlerFunctions = (type: 'layer' | 'table'): DataHandlerFunctions => {
return res as DataHandlerFunctions;
};

/**
* Check that the variable type is compatible with the content of the column.
* If not, use the detected type.
*
*/
const checkVariableType = (
values: any[],
dataType: DataType,
detectedType: VariableType,
userType: VariableType,
): boolean => {
if (userType === VariableType.unknown) {
return true;
}
if (userType === detectedType) {
return true;
}
if (userType === VariableType.stock) {
return dataType === 'number';
}
const filteredValues = values.filter((v) => v !== null && v !== '' && v !== undefined);
const dedupValues = new Set(filteredValues);
if (userType === VariableType.ratio) {
if (dataType !== 'number') {
return false;
}
return dedupValues.size !== 1;
}
if (userType === VariableType.identifier) {
return filteredValues.length === dedupValues.size;
}
return true;
};

export default function TableWindow(): JSX.Element {
const { LL } = useI18nContext();
// Extract identifier and editable value from tableWindowStore
Expand Down Expand Up @@ -469,14 +504,31 @@ export default function TableWindow(): JSX.Element {
// Detect the type of the new variable and count the number of missing values
const t = detectTypeField(newColumn as never[], variableName);

// Check if the user has chosen a variable type that is compatible with the content of
// the column. If not, use the detected type.
const isVariableTypeCompatible = checkVariableType(
newColumn,
t.dataType,
t.variableType,
newColumnType,
);
const variableType = isVariableTypeCompatible
? newColumnType
: t.variableType;
// Also warn the user if the variable type has been changed
toast.error(
LL().DataTable.NewColumnModal.alertNotValidVariableType(
LL().FieldsTyping.VariableTypes[newColumnType as keyof typeof VariableType](),
LL().FieldsTyping.VariableTypes[t.variableType as keyof typeof VariableType](),
),
);

// Add the new variable to the list of new variables
// (will be used to update the layer description)
newVariables.push({
name: variableName,
hasMissingValues: t.hasMissingValues,
// TODO: check 'newColumnType' it is compatible with the content
// of the new column (and if not, use the one detected and stored in 't.variableType')
type: newColumnType,
type: variableType,
dataType: t.dataType,
} as Variable);

Expand Down
1 change: 1 addition & 0 deletions src/i18n/en/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,7 @@ const en = {
newColumnType: 'New column type',
compute: 'Compute',
formula: 'Formula',
alertNotValidVariableType: 'The variable type selected ({0}) is not valid for this field. Type "{1}" will be used instead - You can change the variable type in the field typing modal.',
},
},
FieldsTyping: {
Expand Down
1 change: 1 addition & 0 deletions src/i18n/fr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,7 @@ const fr = {
newColumnType: 'Type du nouveau champ',
compute: 'Calculer',
formula: 'Formule',
alertNotValidVariableType: 'Le type de variable choisie ({0}) n\'est pas valide pour ce champ. Le type "{1}" sera utilisé à la place - Vous pouvez modifier le type de variable dans la fenêtre de typage des champs.',
},
},
FieldsTyping: {
Expand Down
10 changes: 10 additions & 0 deletions src/i18n/i18n-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3745,6 +3745,12 @@ type RootTranslation = {
* F​o​r​m​u​l​a
*/
formula: string
/**
* T​h​e​ ​v​a​r​i​a​b​l​e​ ​t​y​p​e​ ​s​e​l​e​c​t​e​d​ ​(​{​0​}​)​ ​i​s​ ​n​o​t​ ​v​a​l​i​d​ ​f​o​r​ ​t​h​i​s​ ​f​i​e​l​d​.​ ​T​y​p​e​ ​"​{​1​}​"​ ​w​i​l​l​ ​b​e​ ​u​s​e​d​ ​i​n​s​t​e​a​d​ ​-​ ​Y​o​u​ ​c​a​n​ ​c​h​a​n​g​e​ ​t​h​e​ ​v​a​r​i​a​b​l​e​ ​t​y​p​e​ ​i​n​ ​t​h​e​ ​f​i​e​l​d​ ​t​y​p​i​n​g​ ​m​o​d​a​l​.
* @param {unknown} 0
* @param {unknown} 1
*/
alertNotValidVariableType: RequiredParams<'0' | '1'>
}
}
FieldsTyping: {
Expand Down Expand Up @@ -8067,6 +8073,10 @@ export type TranslationFunctions = {
* Formula
*/
formula: () => LocalizedString
/**
* The variable type selected ({0}) is not valid for this field. Type "{1}" will be used instead - You can change the variable type in the field typing modal.
*/
alertNotValidVariableType: (arg0: unknown, arg1: unknown) => LocalizedString
}
}
FieldsTyping: {
Expand Down

0 comments on commit 33fb72a

Please sign in to comment.