From 0143fe191da4410fab29a703a6c4b2d5bf418ee5 Mon Sep 17 00:00:00 2001 From: James Mullaney Date: Mon, 12 Nov 2018 01:49:00 -0800 Subject: [PATCH] fix(mergePersona): Do not error if the merge from does not exist (#117) * fix(mergePersona): Do not error if the merge from does not exist * fix(mergePersona): Ensure we catch NoModel on delete --- src/service/mergePersona.ts | 29 ++++++++++--------- .../mergeNonExistingPersona.test.ts | 6 ++-- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/service/mergePersona.ts b/src/service/mergePersona.ts index ea25e09c7..36a95e242 100644 --- a/src/service/mergePersona.ts +++ b/src/service/mergePersona.ts @@ -1,5 +1,5 @@ +import NoModel from 'jscommons/dist/errors/NoModel'; import DuplicateMergeId from '../errors/DuplicateMergeId'; -import MissingMergeFromPersona from '../errors/MissingMergeFromPersona'; import MissingMergeToPersona from '../errors/MissingMergeToPersona'; import NoModelWithId from '../errors/NoModelWithId'; import MergePersonaOptions from '../serviceFactory/options/MergePersonaOptions'; @@ -15,17 +15,9 @@ export default (config: Config) => async ({ throw new DuplicateMergeId(fromPersonaId); } try { - await Promise.all([ - config.repo.getPersona({ personaId: fromPersonaId, organisation }), - config.repo.getPersona({ personaId: toPersonaId, organisation }), - ]); + await config.repo.getPersona({ personaId: toPersonaId, organisation }); } catch (err) { - /* istanbul ignore else */ if (err instanceof NoModelWithId) { - if (err.id === fromPersonaId) { - throw new MissingMergeFromPersona(err.modelName, err.id); - } - /* istanbul ignore else */ if (err.id === toPersonaId) { throw new MissingMergeToPersona(err.modelName, err.id); } @@ -41,10 +33,19 @@ export default (config: Config) => async ({ toPersonaId, }); - await config.repo.deletePersona({ - organisation, - personaId: fromPersonaId, - }); + try { + await config.repo.deletePersona({ + organisation, + personaId: fromPersonaId, + }); + } catch (err) { + if (err instanceof NoModel) { + // potentially expected, if persona was removed by another process during this exchange + return { identifierIds }; + } + /* istanbul ignore next */ + throw err; + } return { identifierIds }; }; diff --git a/src/tests/mergePersona/mergeNonExistingPersona.test.ts b/src/tests/mergePersona/mergeNonExistingPersona.test.ts index 48846514a..48ecd4c82 100644 --- a/src/tests/mergePersona/mergeNonExistingPersona.test.ts +++ b/src/tests/mergePersona/mergeNonExistingPersona.test.ts @@ -1,5 +1,4 @@ import assertError from 'jscommons/dist/tests/utils/assertError'; -import MissingMergeFromPersona from '../../errors/MissingMergeFromPersona'; import MissingMergeToPersona from '../../errors/MissingMergeToPersona'; import createTestPersona from '../utils/createTestPersona'; import setup from '../utils/setup'; @@ -20,13 +19,12 @@ describe('mergePersona with non-existing personas', () => { await assertError(MissingMergeToPersona, promise); }); - it('Should throw error if fromModel does not exist', async () => { + it('Should not throw error if fromModel does not exist', async () => { const persona = await createTestPersona(); - const promise = service.mergePersona({ + await service.mergePersona({ fromPersonaId: MISSING_ID, organisation: TEST_ORGANISATION, toPersonaId: persona.id, }); - await assertError(MissingMergeFromPersona, promise); }); });