From 9722c173d86eb934fa260730d77e162078984875 Mon Sep 17 00:00:00 2001 From: Maciej Ziarkowski Date: Thu, 12 Aug 2021 01:29:37 +0100 Subject: [PATCH] Allow non-editable fields to be auto-derived Some fields shouldn't be editable through the API but can still be modified, because they are derived from another field which is editable. This change fixes a bug where a derived field wouldn't be updated, because it was not on the editable fields whitelist. --- app/src/api/config/dataFields.ts | 12 +++++++++++- app/src/api/dataAccess/building.ts | 2 +- app/src/api/errors/general.ts | 9 +++++++++ app/src/api/services/building/base.ts | 8 ++++++-- .../services/domainLogic/validateBuildingUpdate.ts | 9 ++++++++- 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/app/src/api/config/dataFields.ts b/app/src/api/config/dataFields.ts index e21680ff..523c3111 100644 --- a/app/src/api/config/dataFields.ts +++ b/app/src/api/config/dataFields.ts @@ -3,10 +3,19 @@ import { valueType } from '../../helpers'; /** Configuration for a single data field */ export interface DataFieldConfig { /** - * Allow editing the field? + * Allow editing the field through the API? */ edit: boolean; + /** + * Should the editing of the field be allowed - but only when + * the change is a result of an edit of another field, from which this field is derived. + * Example: editing Land Use Group modifies Land Use Order, too, because LU Order is automatically derived from LU Group. + * But Land Use Order itself cannot be modified directly by users. + * Default: false + */ + derivedEdit?: boolean; + /** * Allow verifying the field value? * Default: false; @@ -236,6 +245,7 @@ export const dataFieldsConfig = valueType()({ /* eslint-disable }, current_landuse_order: { edit: false, + derivedEdit: true, verify: false, }, diff --git a/app/src/api/dataAccess/building.ts b/app/src/api/dataAccess/building.ts index 9763df63..0d7115a0 100644 --- a/app/src/api/dataAccess/building.ts +++ b/app/src/api/dataAccess/building.ts @@ -55,7 +55,7 @@ export async function insertEditHistoryRevision( const columnConfigLookup = Object.assign( {}, - ...Object.entries(dataFieldsConfig).filter(([, config]) => config.edit).map(([key, { + ...Object.entries(dataFieldsConfig).filter(([, config]) => config.edit || config.derivedEdit).map(([key, { asJson = false, sqlCast }]) => ({ [key]: { diff --git a/app/src/api/errors/general.ts b/app/src/api/errors/general.ts index ae1f29fa..6dc99c2c 100644 --- a/app/src/api/errors/general.ts +++ b/app/src/api/errors/general.ts @@ -21,6 +21,15 @@ export class InvalidOperationError extends UserError { } } +export class InvalidFieldError extends UserError { + public fieldName: string; + constructor(message?: string, fieldName?: string) { + super(message); + this.name = 'InvalidFieldError'; + this.fieldName = fieldName; + } +} + export class FieldTypeError extends UserError { public fieldName: string; constructor(message?: string, fieldName?: string) { diff --git a/app/src/api/services/building/base.ts b/app/src/api/services/building/base.ts index d103aa25..ef55fea5 100644 --- a/app/src/api/services/building/base.ts +++ b/app/src/api/services/building/base.ts @@ -31,7 +31,11 @@ export async function getBuildingById(id: number) { } } -const FIELD_EDIT_WHITELIST = new Set(Object.entries(dataFieldsConfig).filter(([, value]) => value.edit).map(([key]) => key)); +/** + * List of fields for which modification is allowed + * (directly by the user, or for fields that are derived from others) + */ +const FINAL_FIELD_EDIT_ALLOWLIST = new Set(Object.entries(dataFieldsConfig).filter(([, value]) => value.edit || value.derivedEdit).map(([key]) => key)); export async function editBuilding(buildingId: number, building: any, userId: string): Promise { // TODO add proper building type return await updateBuildingData(buildingId, userId, async () => { @@ -44,7 +48,7 @@ export async function editBuilding(buildingId: number, building: any, userId: st delete processedBuilding.geometry_id; // return whitelisted fields to update - return pickFields(processedBuilding, FIELD_EDIT_WHITELIST); + return pickFields(processedBuilding, FINAL_FIELD_EDIT_ALLOWLIST); }); } diff --git a/app/src/api/services/domainLogic/validateBuildingUpdate.ts b/app/src/api/services/domainLogic/validateBuildingUpdate.ts index 12a5769f..8dcc2470 100644 --- a/app/src/api/services/domainLogic/validateBuildingUpdate.ts +++ b/app/src/api/services/domainLogic/validateBuildingUpdate.ts @@ -2,7 +2,8 @@ import Ajv from 'ajv'; import addFormats from 'ajv-formats'; import { mapObject } from '../../../helpers'; -import { FieldTypeError } from '../../errors/general'; +import { InvalidFieldError, FieldTypeError } from '../../errors/general'; +import { dataFieldsConfig } from '../../config/dataFields'; import { fieldSchemaConfig } from '../../config/fieldSchemaConfig'; const ajv = new Ajv(); @@ -10,8 +11,14 @@ addFormats(ajv); const compiledSchemas = mapObject(fieldSchemaConfig, ([, val]) => ajv.compile(val)) +const EXTERNAL_FIELD_EDIT_ALLOWLIST = new Set(Object.entries(dataFieldsConfig).filter(([, value]) => value.edit).map(([key]) => key)); + export function validateBuildingUpdate(buildingId: number, building: any) { for(const field of Object.keys(building)) { + if(!EXTERNAL_FIELD_EDIT_ALLOWLIST.has(field)) { + throw new InvalidFieldError('Field is not editable', field); + } + if(field in compiledSchemas) { if(!compiledSchemas[field](building[field])) { throw new FieldTypeError('Invalid format of data sent', field);