From d6beb030654ebd9b6a8feb80259692e324d9fa89 Mon Sep 17 00:00:00 2001 From: Maciej Ziarkowski Date: Sun, 22 Mar 2020 19:18:07 +0000 Subject: [PATCH] Verify correct land use group parameters --- app/src/api/dataAccess/landUse.ts | 11 +++++++ .../__tests__/domainLogic/landUse.test.ts | 29 +++++++++++++++++ app/src/api/services/domainLogic/landUse.ts | 21 ++++++++++-- .../domainLogic/processBuildingUpdate.ts | 32 ++++++++++++------- 4 files changed, 79 insertions(+), 14 deletions(-) diff --git a/app/src/api/dataAccess/landUse.ts b/app/src/api/dataAccess/landUse.ts index 02cdfcb8..fbd2bea4 100644 --- a/app/src/api/dataAccess/landUse.ts +++ b/app/src/api/dataAccess/landUse.ts @@ -36,3 +36,14 @@ export async function getLandUseOrderFromGroup(groups: string[]): Promise { + let groupResult = await db.oneOrNone(` + SELECT landuse_id + FROM reference_tables.buildings_landuse_group + WHERE description = $1 + `, [group] + ); + + return (groupResult != undefined); +} diff --git a/app/src/api/services/__tests__/domainLogic/landUse.test.ts b/app/src/api/services/__tests__/domainLogic/landUse.test.ts index 00793647..c7b5bf7a 100644 --- a/app/src/api/services/__tests__/domainLogic/landUse.test.ts +++ b/app/src/api/services/__tests__/domainLogic/landUse.test.ts @@ -1,5 +1,6 @@ import * as _ from 'lodash'; +import { ArgumentError } from '../../../errors/general'; import { LandUseState, updateLandUse } from '../../domainLogic/landUse'; const testGroupToOrder = { @@ -9,6 +10,8 @@ const testGroupToOrder = { 'Offices': 'Industry And Business' }; +const testAllowedGroups = Object.keys(testGroupToOrder); + jest.mock('../../../dataAccess/landUse', () => ({ getLandUseOrderFromGroup: jest.fn((groups: string[]) => { const orders = _.chain(groups).map(g => testGroupToOrder[g]).uniq().value(); @@ -19,6 +22,9 @@ jest.mock('../../../dataAccess/landUse', () => ({ else result = 'Mixed Use'; return Promise.resolve(result); + }), + isLandUseGroupAllowed: jest.fn((group: string) => { + return testAllowedGroups.includes(group); }) })); @@ -94,4 +100,27 @@ describe('updateLandUse()', () => { } ); + it.each([ + [['Blah'], "'Blah' is not a valid Land Use Group"], + [['Agriculture', 'Zonk'], "'Zonk' is not a valid Land Use Group"], + [['Zonk', 'Blah'], "'Zonk' is not a valid Land Use Group"] + ])('Should throw ArgumentError when invalid land use group supplied', async (groups: string[], message: string) => { + const resultPromise = updateLandUse({landUseGroup: [], landUseOrder: null}, { landUseGroup: groups}); + + await expect(resultPromise).rejects.toBeInstanceOf(ArgumentError); + await expect(resultPromise).rejects.toHaveProperty('argumentName', 'landUseUpdate'); + await expect(resultPromise).rejects.toHaveProperty('message', message); + }); + + it('Should throw ArgumentError when duplicate land use groups supplied', async () => { + const resultPromise = updateLandUse( + {landUseGroup: [], landUseOrder: null}, + { landUseGroup: ['Agriculture', 'Agriculture']} + ); + + await expect(resultPromise).rejects.toBeInstanceOf(ArgumentError); + await expect(resultPromise).rejects.toHaveProperty('argumentName', 'landUseUpdate'); + await expect(resultPromise).rejects.toHaveProperty('message', 'Cannot supply duplicate Land Use Groups'); + }); + }); diff --git a/app/src/api/services/domainLogic/landUse.ts b/app/src/api/services/domainLogic/landUse.ts index b4b0f4b9..8ca8dbb3 100644 --- a/app/src/api/services/domainLogic/landUse.ts +++ b/app/src/api/services/domainLogic/landUse.ts @@ -1,6 +1,7 @@ import _ from 'lodash'; -import { getLandUseOrderFromGroup } from '../../dataAccess/landUse'; +import * as landUseDataAccess from '../../dataAccess/landUse'; +import { ArgumentError } from '../../errors/general'; export interface LandUseState { landUseGroup: string[]; @@ -9,10 +10,26 @@ export interface LandUseState { export async function updateLandUse(landUse: LandUseState, landUseUpdate: Partial): Promise { const landUseGroupUpdate = landUseUpdate.landUseGroup; - const landUseOrderUpdate = await getLandUseOrderFromGroup(landUseGroupUpdate); + + for(let group of landUseGroupUpdate) { + const isGroupValid = await landUseDataAccess.isLandUseGroupAllowed(group); + if(!isGroupValid) { + throw new ArgumentError(`'${group}' is not a valid Land Use Group`, 'landUseUpdate'); + } + } + + if(hasDuplicates(landUseGroupUpdate)) { + throw new ArgumentError('Cannot supply duplicate Land Use Groups', 'landUseUpdate'); + } + + const landUseOrderUpdate = await landUseDataAccess.getLandUseOrderFromGroup(landUseGroupUpdate); return { landUseGroup: landUseGroupUpdate, landUseOrder: landUseOrderUpdate }; } + +function hasDuplicates(array: any[]) { + return (new Set(array).size !== array.length); +} diff --git a/app/src/api/services/domainLogic/processBuildingUpdate.ts b/app/src/api/services/domainLogic/processBuildingUpdate.ts index 46f5f7c7..2b2cc7ca 100644 --- a/app/src/api/services/domainLogic/processBuildingUpdate.ts +++ b/app/src/api/services/domainLogic/processBuildingUpdate.ts @@ -1,6 +1,7 @@ import * as _ from 'lodash'; import { hasAnyOwnProperty } from '../../../helpers'; +import { ArgumentError } from '../../errors/general'; import { getCurrentBuildingDataById } from '../building'; import { updateLandUse } from './landUse'; @@ -16,17 +17,24 @@ export async function processBuildingUpdate(buildingId: number, buildingUpdate: async function processCurrentLandUseClassifications(buildingId: number, buildingUpdate: any): Promise { const currentBuildingData = await getCurrentBuildingDataById(buildingId); - const currentLandUseUpdate = await updateLandUse( - { - landUseGroup: currentBuildingData.current_landuse_group, - landUseOrder: currentBuildingData.current_landuse_order - }, { - landUseGroup: buildingUpdate.current_landuse_group - } - ); + try { + const currentLandUseUpdate = await updateLandUse( + { + landUseGroup: currentBuildingData.current_landuse_group, + landUseOrder: currentBuildingData.current_landuse_order + }, { + landUseGroup: buildingUpdate.current_landuse_group + } + ); - return Object.assign({}, buildingUpdate, { - current_landuse_group: currentLandUseUpdate.landUseGroup, - current_landuse_order: currentLandUseUpdate.landUseOrder, - }); + return Object.assign({}, buildingUpdate, { + current_landuse_group: currentLandUseUpdate.landUseGroup, + current_landuse_order: currentLandUseUpdate.landUseOrder, + }); + } catch (error) { + if(error instanceof ArgumentError && error.argumentName === 'landUseUpdate') { + error.argumentName = 'buildingUpdate'; + } + throw error; + } }