diff --git a/api/src/paths/project/{projectId}/survey/{surveyId}/critters/markings/import.ts b/api/src/paths/project/{projectId}/survey/{surveyId}/critters/markings/import.ts index 8c5ac9fed7..c2a1c7dc5d 100644 --- a/api/src/paths/project/{projectId}/survey/{surveyId}/critters/markings/import.ts +++ b/api/src/paths/project/{projectId}/survey/{surveyId}/critters/markings/import.ts @@ -2,6 +2,7 @@ import { RequestHandler } from 'express'; import { Operation } from 'express-openapi'; import { PROJECT_PERMISSION, SYSTEM_ROLE } from '../../../../../../../constants/roles'; import { getDBConnection } from '../../../../../../../database/db'; +import { ApiGeneralError } from '../../../../../../../errors/api-error'; import { csvFileSchema } from '../../../../../../../openapi/schemas/file'; import { authorizeRequestHandler } from '../../../../../../../request-handlers/security/authorization'; import { ImportMarkingsService } from '../../../../../../../services/import-services/marking/import-markings-service'; @@ -108,6 +109,11 @@ POST.apiDoc = { /** * Imports a `Critterbase Marking CSV` which bulk adds markings to Critterbase. * + * TODO: Decide what to do with validation errors. For now, just throw an error. + * In future, potentially return the validation errors in the response. + * Why? The frontend should be able to easily determine if the extra errors + * in the response are CSV validation errors or just regular errors. + * * @return {*} {RequestHandler} */ export function importCsv(): RequestHandler { @@ -125,7 +131,11 @@ export function importCsv(): RequestHandler { const importMarkings = new ImportMarkingsService(connection, worksheet, surveyId); - await importMarkings.importCSVWorksheet(); + const validationErrors = await importMarkings.importCSVWorksheet(); + + if (validationErrors.length) { + throw new ApiGeneralError('Failed to validate CSV', validationErrors); + } await connection.commit(); diff --git a/api/src/services/import-services/marking/import-markings-service.test.ts b/api/src/services/import-services/marking/import-markings-service.test.ts new file mode 100644 index 0000000000..e5486b3e2f --- /dev/null +++ b/api/src/services/import-services/marking/import-markings-service.test.ts @@ -0,0 +1,157 @@ +import chai, { expect } from 'chai'; +import sinon from 'sinon'; +import sinonChai from 'sinon-chai'; +import { WorkSheet } from 'xlsx'; +import * as csv from '../../../utils/csv-utils/csv-config-validation'; +import { CSVConfig } from '../../../utils/csv-utils/csv-config-validation.interface'; +import { NestedRecord } from '../../../utils/nested-record'; +import { getMockDBConnection } from '../../../__mocks__/db'; +import { IAsSelectLookup } from '../../critterbase-service'; +import { ImportMarkingsService } from './import-markings-service'; + +chai.use(sinonChai); + +describe('import-markings-service', () => { + beforeEach(() => { + sinon.restore(); + }); + + describe('constructor', () => { + it('should create an instance of ImportMarkingsService', () => { + const mockConnection = getMockDBConnection(); + const worksheet = {} as WorkSheet; + const surveyId = 1; + + const service = new ImportMarkingsService(mockConnection, worksheet, surveyId); + + expect(service).to.be.instanceof(ImportMarkingsService); + }); + }); + + describe('importCSVWorksheet', () => { + it('should import the CSV worksheet', async () => { + const mockConnection = getMockDBConnection(); + const worksheet = {} as WorkSheet; + const surveyId = 1; + + const service = new ImportMarkingsService(mockConnection, worksheet, surveyId); + + const mockCSVConfig = {} as CSVConfig; + const mockGetConfig = sinon.stub(service, 'getCSVConfig').resolves(mockCSVConfig); + const bulkCreateStub = sinon.stub(service.surveyCritterService.critterbaseService, 'bulkCreate').resolves(); + + const mockValidate = sinon.stub(csv, 'validateCSVWorksheet').returns({ + errors: [], + rows: [ + { + ALIAS: 'uuid', + CAPTURE_DATE: 'uuid2', + BODY_LOCATION: 'ear', + MARKING_TYPE: 'tag', + IDENTIFIER: 'id', + PRIMARY_COLOUR: 'red', + SECONDARY_COLOUR: 'blue', + DESCRIPTION: 'comments' + } + ] + }); + + const errors = await service.importCSVWorksheet(); + + expect(mockGetConfig).to.have.been.called; + expect(mockValidate).to.have.been.calledOnceWithExactly(worksheet, mockCSVConfig); + expect(bulkCreateStub).to.have.been.calledOnceWithExactly({ + markings: [ + { + critter_id: 'uuid', + capture_id: 'uuid2', + body_location: 'ear', + marking_type: 'tag', + identifier: 'id', + primary_colour: 'red', + secondary_colour: 'blue', + comment: 'comments' + } + ] + }); + + expect(errors).to.be.an('array').that.is.empty; + }); + + it('should return the errors early', async () => { + const mockConnection = getMockDBConnection(); + const worksheet = {} as WorkSheet; + const surveyId = 1; + + const service = new ImportMarkingsService(mockConnection, worksheet, surveyId); + + const mockCSVConfig = {} as CSVConfig; + const mockGetConfig = sinon.stub(service, 'getCSVConfig').resolves(mockCSVConfig); + + const mockValidate = sinon.stub(csv, 'validateCSVWorksheet').returns({ + errors: [{ error: 'error', solution: 'solution', values: [] }], + rows: [] + }); + + const errors = await service.importCSVWorksheet(); + + expect(mockGetConfig).to.have.been.called; + expect(mockValidate).to.have.been.calledOnceWithExactly(worksheet, mockCSVConfig); + + expect(errors).to.be.an('array').that.is.not.empty; + }); + }); + + describe('getCSVConfig', () => { + it('should return a CSVConfig object', async () => { + const mockConnection = getMockDBConnection(); + const worksheet = {} as WorkSheet; + const surveyId = 1; + + const service = new ImportMarkingsService(mockConnection, worksheet, surveyId); + + const mockAliasMap = new Map(); + const mockDictionary = new NestedRecord(); + const mockMarkingTypes = [{ value: 'type1' }, { value: 'type2' }] as IAsSelectLookup[]; + const mockColours = [{ value: 'colour1' }, { value: 'colour2' }] as IAsSelectLookup[]; + + const surveyAliasMapStub = sinon + .stub(service.surveyCritterService, 'getSurveyCritterAliasMap') + .resolves(mockAliasMap); + + const bodyLocationDictionaryStub = sinon.stub(service, '_getBodyLocationDictionary').resolves(mockDictionary); + + const markingTypesStub = sinon + .stub(service.surveyCritterService.critterbaseService, 'getMarkingTypes') + .resolves(mockMarkingTypes); + + const coloursStub = sinon + .stub(service.surveyCritterService.critterbaseService, 'getColours') + .resolves(mockColours); + + expect(surveyAliasMapStub).to.not.have.been.calledOnceWithExactly(surveyId); + expect(bodyLocationDictionaryStub).to.not.have.been.calledOnceWithExactly(mockAliasMap); + expect(markingTypesStub).to.not.have.been.calledOnceWithExactly(); + expect(coloursStub).to.not.have.been.calledOnceWithExactly(); + + try { + const config = await service.getCSVConfig(); + + expect(config.dynamicHeadersConfig).to.be.equal(undefined); + expect(config.staticHeadersConfig).to.have.keys([ + 'ALIAS', + 'CAPTURE_DATE', + 'CAPTURE_TIME', + 'BODY_LOCATION', + 'MARKING_TYPE', + 'IDENTIFIER', + 'PRIMARY_COLOUR', + 'SECONDARY_COLOUR', + 'DESCRIPTION' + ]); + } catch (error) { + expect.fail(); + } + }); + }); +}); diff --git a/api/src/services/import-services/marking/import-markings-service.ts b/api/src/services/import-services/marking/import-markings-service.ts index 8579508484..1a174581c1 100644 --- a/api/src/services/import-services/marking/import-markings-service.ts +++ b/api/src/services/import-services/marking/import-markings-service.ts @@ -1,9 +1,8 @@ import { WorkSheet } from 'xlsx'; import { IDBConnection } from '../../../database/db'; -import { ApiGeneralError } from '../../../errors/api-error'; import { CSVConfigUtils } from '../../../utils/csv-utils/csv-config-utils'; import { validateCSVWorksheet } from '../../../utils/csv-utils/csv-config-validation'; -import { CSVConfig } from '../../../utils/csv-utils/csv-config-validation.interface'; +import { CSVConfig, CSVError } from '../../../utils/csv-utils/csv-config-validation.interface'; import { getDescriptionCellValidator, getTimeCellSetter, @@ -86,15 +85,15 @@ export class ImportMarkingsService extends DBService { * * @async * @throws {ApiGeneralError} - If unable to fully insert records into Critterbase - * @returns {*} {Promise} + * @returns {*} {Promise} */ - async importCSVWorksheet(): Promise { + async importCSVWorksheet(): Promise { const config = await this.getCSVConfig(); const { errors, rows } = validateCSVWorksheet(this.worksheet, config); if (errors.length) { - throw new ApiGeneralError('Failed to validate CSV', errors); + return errors; } const markings = rows.map((row) => ({ @@ -111,8 +110,15 @@ export class ImportMarkingsService extends DBService { defaultLog.debug({ label: 'import markings', markings }); await this.surveyCritterService.critterbaseService.bulkCreate({ markings }); + + return []; } + /** + * Get the CSV configuration for Markings. + * + * @returns {Promise>} The CSV configuration + */ async getCSVConfig(): Promise> { const surveyAliasMap = await this.surveyCritterService.getSurveyCritterAliasMap(this.surveyId); const bodyLocationDictionary = await this._getBodyLocationDictionary(surveyAliasMap); @@ -158,6 +164,12 @@ export class ImportMarkingsService extends DBService { return this.utils.getConfig(); } + /** + * Get a dictionary of critter alias -> body location -> body_location_id. + * + * @param {Map} surveyAliasMap - The survey alias map + * @returns {Promise>} The body location dictionary + */ async _getBodyLocationDictionary(surveyAliasMap: Map): Promise> { const dictionary = new NestedRecord(); const uniqueTsns = new Set(); diff --git a/api/src/services/import-services/marking/marking-header-configs.test.ts b/api/src/services/import-services/marking/marking-header-configs.test.ts index c5d4297372..8065ff13ea 100644 --- a/api/src/services/import-services/marking/marking-header-configs.test.ts +++ b/api/src/services/import-services/marking/marking-header-configs.test.ts @@ -117,25 +117,6 @@ describe('marking-header-configs', () => { expect(result).to.deep.equal([]); }); - it('should update the mutateCell value to the body_location_id', () => { - const dictionary = new NestedRecord({ alias: { location: 'uuid' } }); - const mockConfig: CSVConfig = { staticHeadersConfig: { ALIAS: { aliases: [] } }, ignoreDynamicHeaders: true }; - const utils = new CSVConfigUtils({}, mockConfig); - - const params = { - mutateCell: 'body_location_id', - cell: 'location', - row: { ALIAS: 'alias' }, - header: '', - rowIndex: 0 - } as CSVParams; - - const result = getMarkingBodyLocationCellValidator(dictionary, utils)(params); - - expect(params.mutateCell).to.deep.equal('uuid'); - expect(result.length).to.deep.equal(0); - }); - it('should return a single error when alias has no body locations', () => { const dictionary = new NestedRecord({ alias: { location: 'uuid' } }); const mockConfig: CSVConfig = { staticHeadersConfig: { ALIAS: { aliases: [] } }, ignoreDynamicHeaders: true }; diff --git a/api/src/services/import-services/marking/marking-header-configs.ts b/api/src/services/import-services/marking/marking-header-configs.ts index d85acc2f6b..e15b92a2d2 100644 --- a/api/src/services/import-services/marking/marking-header-configs.ts +++ b/api/src/services/import-services/marking/marking-header-configs.ts @@ -120,8 +120,6 @@ export const getMarkingColourCellValidator = (colours: Set): CSVCellVali /** * Get the marking body location cell validator. * - * Note: Modifies the mutateCell value to the `body_location_id` - * * Rules: * 1. The cell must be a valid body location for the critter ie: exists in the rowDictionary * @@ -164,9 +162,6 @@ export const getMarkingBodyLocationCellValidator = ( ]; } - // Set the body location id in the state for the setter - params.mutateCell = rowDictionaryBodyLocation; - return []; }; };