From afa976066ae7e401c924ba883067767fde89c5c6 Mon Sep 17 00:00:00 2001 From: Talha Date: Thu, 10 Sep 2020 09:33:57 +0530 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20table=20constraint=20err?= =?UTF-8?q?or=20when=20updating=20member's=20email=20with=20an=20already?= =?UTF-8?q?=20existing=20email=20(#12178)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #12045 - When member's email is updated to an already existing email of different member it caused table's unique constraint error, which was not handled properly. - Added handling for this error similar to one in members `add` method. --- core/server/api/canary/members.js | 43 +++++++++++++------ .../server/services/members/importer/index.js | 4 +- core/server/translations/en.json | 2 +- .../api/canary/admin/members_spec.js | 2 +- 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/core/server/api/canary/members.js b/core/server/api/canary/members.js index 80e2a43e8b..0c7f981ea1 100644 --- a/core/server/api/canary/members.js +++ b/core/server/api/canary/members.js @@ -242,7 +242,9 @@ module.exports = { if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) { throw new errors.ValidationError({ message: i18n.t('errors.models.member.memberAlreadyExists.message'), - context: i18n.t('errors.models.member.memberAlreadyExists.context') + context: i18n.t('errors.models.member.memberAlreadyExists.context', { + action: 'add' + }) }); } @@ -282,24 +284,37 @@ module.exports = { }, permissions: true, async query(frame) { - frame.options.withRelated = ['stripeSubscriptions']; - const member = await membersService.api.members.update(frame.data.members[0], frame.options); + try { + frame.options.withRelated = ['stripeSubscriptions']; + const member = await membersService.api.members.update(frame.data.members[0], frame.options); - const hasCompedSubscription = !!member.related('stripeSubscriptions').find(subscription => subscription.get('plan_nickname') === 'Complimentary'); + const hasCompedSubscription = !!member.related('stripeSubscriptions').find(subscription => subscription.get('plan_nickname') === 'Complimentary'); - if (typeof frame.data.members[0].comped === 'boolean') { - if (frame.data.members[0].comped && !hasCompedSubscription) { - await membersService.api.members.setComplimentarySubscription(member); - } else if (!(frame.data.members[0].comped) && hasCompedSubscription) { - await membersService.api.members.cancelComplimentarySubscription(member); + if (typeof frame.data.members[0].comped === 'boolean') { + if (frame.data.members[0].comped && !hasCompedSubscription) { + await membersService.api.members.setComplimentarySubscription(member); + } else if (!(frame.data.members[0].comped) && hasCompedSubscription) { + await membersService.api.members.cancelComplimentarySubscription(member); + } + + await member.load(['stripeSubscriptions']); } - await member.load(['stripeSubscriptions']); + await member.load(['stripeSubscriptions.customer']); + + return member.toJSON(frame.options); + } catch (error) { + if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) { + throw new errors.ValidationError({ + message: i18n.t('errors.models.member.memberAlreadyExists.message'), + context: i18n.t('errors.models.member.memberAlreadyExists.context', { + action: 'edit' + }) + }); + } + + throw error; } - - await member.load(['stripeSubscriptions.customer']); - - return member.toJSON(frame.options); } }, diff --git a/core/server/services/members/importer/index.js b/core/server/services/members/importer/index.js index 51f2e0a08a..59071d1cdb 100644 --- a/core/server/services/members/importer/index.js +++ b/core/server/services/members/importer/index.js @@ -55,7 +55,9 @@ const doImport = async ({members, labels, importSetLabels, createdBy}) => { if (error.code === 'ER_DUP_ENTRY') { return new errors.ValidationError({ message: i18n.t('errors.models.member.memberAlreadyExists.message'), - context: i18n.t('errors.models.member.memberAlreadyExists.context'), + context: i18n.t('errors.models.member.memberAlreadyExists.context', { + action: 'add' + }), err: error }); } else { diff --git a/core/server/translations/en.json b/core/server/translations/en.json index e3bf84bef5..5d5ce8200b 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -250,7 +250,7 @@ "member": { "memberAlreadyExists": { "message": "Member already exists", - "context": "Attempting to add member with existing email address." + "context": "Attempting to {action} member with existing email address." } }, "member_stripe_customer": { diff --git a/test/regression/api/canary/admin/members_spec.js b/test/regression/api/canary/admin/members_spec.js index 781baecefa..538f9b3773 100644 --- a/test/regression/api/canary/admin/members_spec.js +++ b/test/regression/api/canary/admin/members_spec.js @@ -494,7 +494,7 @@ describe('Members API', function () { }); }); - it('Fails to import memmber duplicate emails', function () { + it('Fails to import member duplicate emails', function () { return request .post(localUtils.API.getApiQuery(`members/upload/`)) .attach('membersfile', path.join(__dirname, '/../../../../utils/fixtures/csv/members-duplicate-emails.csv'))