Skip to content

Commit 8b46cbc

Browse files
authored
TECHDEBT/BUGFIX: Telemetry Import Fix + Critter Alias Check (#1532)
- Added validation check for duplicate aliases when creating and updating critters - Added additional logic for telemetry CSV import for deployments
1 parent b0b1b2a commit 8b46cbc

File tree

18 files changed

+749
-185
lines changed

18 files changed

+749
-185
lines changed

api/src/paths/project/{projectId}/survey/{surveyId}/critters/index.test.ts

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -194,47 +194,50 @@ describe('addCritterToSurvey', () => {
194194
sinon.restore();
195195
});
196196

197-
it('does not create a new critter', async () => {
197+
it('calls addCrittersToSurvey when critter_id included and returns critters from survey', async () => {
198198
const mockDBConnection = getMockDBConnection({ release: sinon.stub() });
199199
const getDBConnectionStub = sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
200200

201201
const mockSurveyCritter = { critter_id: 123, critterbase_critter_id: 'critterbase1' };
202202
const mockCBCritter = { critter_id: 'critterbase1' };
203203

204204
const mockAddCritterToSurvey = sinon
205-
.stub(SurveyCritterService.prototype, 'addCritterToSurvey')
206-
.resolves(mockSurveyCritter.critter_id);
207-
const mockCreateCritter = sinon.stub(CritterbaseService.prototype, 'createCritter').resolves();
205+
.stub(SurveyCritterService.prototype, 'addCrittersToSurvey')
206+
.resolves([mockSurveyCritter.critter_id]);
207+
const mockCreateCritter = sinon.stub(CritterbaseService.prototype, 'createCritter').resolves(mockCBCritter);
208208

209209
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
210210

211211
mockReq.params = {
212212
projectId: '1',
213213
surveyId: '2'
214214
};
215-
mockReq.body = mockCBCritter;
215+
216+
mockReq.body = {
217+
critter_id: 'critterbase1'
218+
};
216219

217220
const requestHandler = addCritterToSurvey();
218221

219222
await requestHandler(mockReq, mockRes, mockNext);
220223

221224
expect(getDBConnectionStub).to.have.been.calledOnce;
222225
expect(mockAddCritterToSurvey).to.have.been.calledOnce;
223-
expect(mockCreateCritter).not.to.have.been.called;
226+
expect(mockCreateCritter).to.not.have.been.calledOnce;
224227
expect(mockRes.status).to.have.been.calledWith(201);
225228
expect(mockRes.json).to.have.been.calledWith(mockSurveyCritter);
226229
});
227230

228-
it('returns critters from survey', async () => {
231+
it('calls createCritterAndAddToSurvey when critter_id not included and returns critters from survey', async () => {
229232
const mockDBConnection = getMockDBConnection({ release: sinon.stub() });
230233
const getDBConnectionStub = sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
231234

232235
const mockSurveyCritter = { critter_id: 123, critterbase_critter_id: 'critterbase1' };
233-
const mockCBCritter = { critter_id: 'critterbase1' };
236+
const mockCBCritter = { critter_id: undefined };
234237

235238
const mockAddCritterToSurvey = sinon
236-
.stub(SurveyCritterService.prototype, 'addCritterToSurvey')
237-
.resolves(mockSurveyCritter.critter_id);
239+
.stub(SurveyCritterService.prototype, 'createCritterAndAddToSurvey')
240+
.resolves({ critterbaseCritterId: 'critterbase1', surveyCritterId: 123 });
238241
const mockCreateCritter = sinon.stub(CritterbaseService.prototype, 'createCritter').resolves(mockCBCritter);
239242

240243
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
@@ -244,13 +247,17 @@ describe('addCritterToSurvey', () => {
244247
surveyId: '2'
245248
};
246249

250+
mockReq.body = {
251+
critter_id: undefined
252+
};
253+
247254
const requestHandler = addCritterToSurvey();
248255

249256
await requestHandler(mockReq, mockRes, mockNext);
250257

251258
expect(getDBConnectionStub).to.have.been.calledOnce;
252259
expect(mockAddCritterToSurvey).to.have.been.calledOnce;
253-
expect(mockCreateCritter).to.have.been.calledOnce;
260+
expect(mockCreateCritter).to.not.have.been.calledOnce;
254261
expect(mockRes.status).to.have.been.calledWith(201);
255262
expect(mockRes.json).to.have.been.calledWith(mockSurveyCritter);
256263
});
@@ -259,11 +266,8 @@ describe('addCritterToSurvey', () => {
259266
const mockDBConnection = getMockDBConnection({ release: sinon.stub() });
260267
const getDBConnectionStub = sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
261268

262-
const mockCBCritter = { critter_id: 'critterbase1' };
263-
264269
const mockError = new Error('a test error');
265-
const mockAddCritterToSurvey = sinon.stub(SurveyCritterService.prototype, 'addCritterToSurvey').rejects(mockError);
266-
const mockCreateCritter = sinon.stub(CritterbaseService.prototype, 'createCritter').resolves(mockCBCritter);
270+
const mockAddCritterToSurvey = sinon.stub(SurveyCritterService.prototype, 'addCrittersToSurvey').rejects(mockError);
267271

268272
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
269273

@@ -272,6 +276,10 @@ describe('addCritterToSurvey', () => {
272276
surveyId: '2'
273277
};
274278

279+
mockReq.body = {
280+
critter_id: 'critterbase1'
281+
};
282+
275283
const requestHandler = addCritterToSurvey();
276284

277285
try {
@@ -280,7 +288,6 @@ describe('addCritterToSurvey', () => {
280288
} catch (actualError) {
281289
expect(actualError).to.equal(mockError);
282290
expect(mockAddCritterToSurvey).to.have.been.calledOnce;
283-
expect(mockCreateCritter).to.have.been.calledOnce;
284291
expect(getDBConnectionStub).to.have.been.calledOnce;
285292
expect(mockDBConnection.release).to.have.been.called;
286293
}

api/src/paths/project/{projectId}/survey/{surveyId}/critters/index.ts

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { PROJECT_PERMISSION, SYSTEM_ROLE } from '../../../../../../constants/rol
44
import { getDBConnection } from '../../../../../../database/db';
55
import { critterSchema } from '../../../../../../openapi/schemas/critter';
66
import { authorizeRequestHandler } from '../../../../../../request-handlers/security/authorization';
7-
import { CritterbaseService, ICritterbaseUser } from '../../../../../../services/critterbase-service';
87
import { SurveyCritterService } from '../../../../../../services/survey-critter-service';
98
import { getLogger } from '../../../../../../utils/logger';
109

@@ -282,35 +281,46 @@ export function getCrittersFromSurvey(): RequestHandler {
282281
};
283282
}
284283

284+
/**
285+
* Adds a critter to a survey in SIMS. Will create a new critter in Critterbase if critter_id is not provided.
286+
*
287+
* @export
288+
* @returns {*} {RequestHandler}
289+
*/
285290
export function addCritterToSurvey(): RequestHandler {
286291
return async (req, res) => {
287292
const surveyId = Number(req.params.surveyId);
288-
let critterId = req.body.critter_id;
293+
let critterbaseCritterId: string = req.body.critter_id;
294+
let surveyCritterId: number | null = null;
289295

290296
const connection = getDBConnection(req.keycloak_token);
291297

292298
try {
293299
await connection.open();
294300

295-
const user: ICritterbaseUser = {
296-
keycloak_guid: connection.systemUserGUID(),
297-
username: connection.systemUserIdentifier()
298-
};
301+
const surveyCritterService = new SurveyCritterService(connection);
299302

300-
const critterbaseService = new CritterbaseService(user);
303+
if (critterbaseCritterId) {
304+
// If Critterbase critter_id is provided, add the critter to the survey
305+
const surveyCritterIds = await surveyCritterService.addCrittersToSurvey(surveyId, [critterbaseCritterId]);
301306

302-
// If request does not include critter ID, create a new critter and use its critter ID
303-
let result = null;
304-
if (!critterId) {
305-
result = await critterbaseService.createCritter(req.body);
306-
critterId = result.critter_id;
307-
}
307+
surveyCritterId = surveyCritterIds[0];
308+
} else {
309+
// If Critterbase critter_id is not provided, create a new critter in Critterbase and add it to the survey
310+
const critterIdentifiers = await surveyCritterService.createCritterAndAddToSurvey(surveyId, {
311+
wlh_id: req.body.wlh_id,
312+
animal_id: req.body.animal_id,
313+
sex_qualitative_option_id: req.body.sex_qualitative_option_id,
314+
itis_tsn: req.body.itis_tsn,
315+
critter_comment: req.body.critter_comment
316+
});
308317

309-
const surveyService = new SurveyCritterService(connection);
310-
const surveyCritterId = await surveyService.addCritterToSurvey(surveyId, critterId);
318+
critterbaseCritterId = critterIdentifiers.critterbaseCritterId;
319+
surveyCritterId = critterIdentifiers.surveyCritterId;
320+
}
311321

312322
await connection.commit();
313-
return res.status(201).json({ critterbase_critter_id: critterId, critter_id: surveyCritterId });
323+
return res.status(201).json({ critterbase_critter_id: critterbaseCritterId, critter_id: surveyCritterId });
314324
} catch (error) {
315325
defaultLog.error({ label: 'addCritterToSurvey', message: 'error', error });
316326
await connection.rollback();

api/src/paths/project/{projectId}/survey/{surveyId}/critters/{critterId}/index.test.ts

Lines changed: 44 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -13,24 +13,25 @@ describe('updateSurveyCritter', () => {
1313
sinon.restore();
1414
});
1515

16-
it('returns critters from survey', async () => {
17-
const mockDBConnection = getMockDBConnection({ release: sinon.stub() });
16+
it('returns status 204 when successfull', async () => {
17+
const mockDBConnection = getMockDBConnection({
18+
open: sinon.stub(),
19+
commit: sinon.stub(),
20+
rollback: sinon.stub(),
21+
release: sinon.stub()
22+
});
1823
const getDBConnectionStub = sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
1924

20-
const mockCBCritter = { critter_id: 'critterbase1' };
21-
2225
const mockSurveyUpdateCritter = sinon.stub(SurveyCritterService.prototype, 'updateCritter').resolves();
23-
const mockCritterbaseUpdateCritter = sinon
24-
.stub(CritterbaseService.prototype, 'updateCritter')
25-
.resolves(mockCBCritter);
26-
const mockCritterbaseCreateCritter = sinon
27-
.stub(CritterbaseService.prototype, 'createCritter')
28-
.resolves(mockCBCritter);
2926

3027
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
28+
3129
mockReq.body = {
32-
create: {},
33-
update: { critter_id: 'critterbase1' }
30+
critter_id: 'critterbase1',
31+
animal_id: 'animal1',
32+
wlh_id: 'wlh1',
33+
sex_qualitative_option_id: 'sex1',
34+
critter_comment: 'comments'
3435
};
3536

3637
const requestHandler = updateSurveyCritter();
@@ -39,61 +40,54 @@ describe('updateSurveyCritter', () => {
3940

4041
expect(getDBConnectionStub).to.have.been.calledOnce;
4142
expect(mockSurveyUpdateCritter).to.have.been.calledOnce;
42-
expect(mockCritterbaseUpdateCritter).to.have.been.calledOnce;
43-
expect(mockCritterbaseCreateCritter).to.have.been.calledOnce;
44-
expect(mockRes.status).to.have.been.calledWith(200);
45-
expect(mockRes.json).to.have.been.calledWith(mockCBCritter);
43+
expect(mockDBConnection.open).to.have.been.calledOnce;
44+
expect(mockRes.status).to.have.been.calledWith(204);
45+
expect(mockRes.send).to.have.been.calledOnce;
46+
expect(mockDBConnection.commit).to.have.been.calledOnce;
47+
expect(mockDBConnection.release).to.have.been.calledOnce;
48+
expect(mockDBConnection.rollback).to.not.have.been.called;
4649
});
4750

4851
it('catches and re-throws errors', async () => {
49-
const mockDBConnection = getMockDBConnection({ release: sinon.stub() });
50-
const getDBConnectionStub = sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
51-
52-
const mockError = new Error('a test error');
53-
const mockSurveyUpdateCritter = sinon.stub(SurveyCritterService.prototype, 'updateCritter').rejects(mockError);
54-
55-
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
56-
mockReq.body = {
57-
create: {},
58-
update: { critter_id: 'critterbase1' }
59-
};
60-
61-
const requestHandler = updateSurveyCritter();
62-
63-
try {
64-
await requestHandler(mockReq, mockRes, mockNext);
65-
expect.fail();
66-
} catch (actualError) {
67-
expect(actualError).to.equal(mockError);
68-
expect(mockSurveyUpdateCritter).to.have.been.calledOnce;
69-
expect(getDBConnectionStub).to.have.been.calledOnce;
70-
expect(mockDBConnection.release).to.have.been.called;
71-
}
72-
});
52+
const mockDBConnection = getMockDBConnection({
53+
open: sinon.stub(),
54+
commit: sinon.stub(),
55+
release: sinon.stub(),
56+
rollback: sinon.stub()
57+
});
7358

74-
it('catches and re-throws errors', async () => {
75-
const mockDBConnection = getMockDBConnection({ release: sinon.stub() });
7659
const getDBConnectionStub = sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
7760

78-
const errMsg = 'No external critter ID was found.';
79-
const mockSurveyUpdateCritter = sinon.stub(SurveyCritterService.prototype, 'updateCritter').resolves();
61+
const mockSurveyUpdateCritter = sinon
62+
.stub(SurveyCritterService.prototype, 'updateCritter')
63+
.throws(new Error('error'));
8064

8165
const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
66+
8267
mockReq.body = {
83-
update: {}
68+
critter_id: 'critterbase1',
69+
animal_id: 'animal1',
70+
wlh_id: 'wlh1',
71+
sex_qualitative_option_id: 'sex1',
72+
critter_comment: 'comments'
8473
};
8574

8675
const requestHandler = updateSurveyCritter();
8776

8877
try {
8978
await requestHandler(mockReq, mockRes, mockNext);
9079
expect.fail();
91-
} catch (actualError) {
92-
expect((actualError as HTTPError).message).to.equal(errMsg);
93-
expect((actualError as HTTPError).status).to.equal(400);
94-
expect(mockSurveyUpdateCritter).not.to.have.been.called;
80+
} catch (err: any) {
81+
expect(err.message).to.equal('error');
82+
9583
expect(getDBConnectionStub).to.have.been.calledOnce;
96-
expect(mockDBConnection.release).to.have.been.called;
84+
expect(mockSurveyUpdateCritter).to.have.been.calledOnce;
85+
expect(mockDBConnection.open).to.have.been.calledOnce;
86+
expect(mockRes.status).to.not.have.been.called;
87+
expect(mockRes.send).to.not.have.been.called;
88+
expect(mockDBConnection.commit).to.not.have.been.called;
89+
expect(mockDBConnection.release).to.have.been.calledOnce;
90+
expect(mockDBConnection.rollback).to.have.been.calledOnce;
9791
}
9892
});
9993
});

0 commit comments

Comments
 (0)