Skip to content

Commit

Permalink
fix(core): Fix emailAddress conflict when creating Customers
Browse files Browse the repository at this point in the history
Closes #300

BREAKING CHANGE: The `customer` and `user` tables have received some non-destructive modifications, requiring a DB migration.
  • Loading branch information
michaelbromley committed May 4, 2020
1 parent 8395c61 commit 0d4e31a
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 10 deletions.
36 changes: 35 additions & 1 deletion packages/core/e2e/customer.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import gql from 'graphql-tag';
import path from 'path';

import { initialData } from '../../../e2e-common/e2e-initial-data';
import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config';
import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';

import { CUSTOMER_FRAGMENT } from './graphql/fragments';
import {
Expand Down Expand Up @@ -400,6 +400,23 @@ describe('Customer resolver', () => {
expect(createCustomer.user!.verified).toBe(true);
expect(sendEmailFn).toHaveBeenCalledTimes(0);
});

it(
'throws when using an existing, non-deleted emailAddress',
assertThrowsWithMessage(async () => {
const { createCustomer } = await adminClient.query<
CreateCustomer.Mutation,
CreateCustomer.Variables
>(CREATE_CUSTOMER, {
input: {
emailAddress: '[email protected]',
firstName: 'New',
lastName: 'Customer',
},
password: 'test',
});
}, 'The email address must be unique'),
);
});

describe('deletion', () => {
Expand Down Expand Up @@ -456,6 +473,23 @@ describe('Customer resolver', () => {
`No Customer with the id '3' could be found`,
),
);

it('new Customer can be created with same emailAddress as a deleted Customer', async () => {
const { createCustomer } = await adminClient.query<
CreateCustomer.Mutation,
CreateCustomer.Variables
>(CREATE_CUSTOMER, {
input: {
emailAddress: thirdCustomer.emailAddress,
firstName: 'Reusing Email',
lastName: 'Customer',
},
});

expect(createCustomer.emailAddress).toBe(thirdCustomer.emailAddress);
expect(createCustomer.firstName).toBe('Reusing Email');
expect(createCustomer.user?.identifier).toBe(thirdCustomer.emailAddress);
});
});
});

Expand Down
12 changes: 9 additions & 3 deletions packages/core/src/entity/customer/customer.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,23 @@ export class Customer extends VendureEntity implements HasCustomFields, SoftDele
@Column({ nullable: true })
phoneNumber: string;

@Column({ unique: true })
@Column()
emailAddress: string;

@ManyToMany(type => CustomerGroup)
@JoinTable()
groups: CustomerGroup[];

@OneToMany(type => Address, address => address.customer)
@OneToMany(
type => Address,
address => address.customer,
)
addresses: Address[];

@OneToMany(type => Order, order => order.customer)
@OneToMany(
type => Order,
order => order.customer,
)
orders: Order[];

@OneToOne(type => User, { eager: true })
Expand Down
10 changes: 7 additions & 3 deletions packages/core/src/entity/user/user.entity.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { DeepPartial } from '@vendure/common/lib/shared-types';
import { Column, Entity, JoinTable, ManyToMany } from 'typeorm';

import { SoftDeletable } from '../../common/types/common-types';
import { HasCustomFields } from '../../config/custom-field/custom-field-types';
import { VendureEntity } from '../base/base.entity';
import { CustomUserFields } from '../custom-entity-fields';
Expand All @@ -14,12 +15,15 @@ import { Role } from '../role/role.entity';
* @docsCategory entities
*/
@Entity()
export class User extends VendureEntity implements HasCustomFields {
export class User extends VendureEntity implements HasCustomFields, SoftDeletable {
constructor(input?: DeepPartial<User>) {
super(input);
}

@Column({ unique: true })
@Column({ type: Date, nullable: true })
deletedAt: Date | null;

@Column()
identifier: string;

@Column({ select: false }) passwordHash: string;
Expand All @@ -44,7 +48,7 @@ export class User extends VendureEntity implements HasCustomFields {
/**
* @description
* When a request has been made to change the User's identifier, the new identifier
* will be stored here until it has been verfified, after which it will
* will be stored here until it has been verified, after which it will
* replace the current value of the `identifier` field.
*/
@Column({ type: 'varchar', nullable: true })
Expand Down
16 changes: 13 additions & 3 deletions packages/core/src/service/services/customer.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,20 @@ export class CustomerService {
input.emailAddress = normalizeEmailAddress(input.emailAddress);
const customer = new Customer(input);

const existing = await this.connection.getRepository(Customer).findOne({
const existingCustomer = await this.connection.getRepository(Customer).findOne({
where: {
emailAddress: input.emailAddress,
deletedAt: null,
},
});
const existingUser = await this.connection.getRepository(User).findOne({
where: {
identifier: input.emailAddress,
deletedAt: null,
},
});

if (existing) {
if (existingCustomer || existingUser) {
throw new UserInputError(`error.email-address-must-be-unique`);
}
customer.user = await this.userService.createCustomerUser(input.emailAddress, password);
Expand Down Expand Up @@ -243,6 +250,7 @@ export class CustomerService {
const existing = await this.connection.getRepository(Customer).findOne({
where: {
emailAddress: input.emailAddress,
deletedAt: null,
},
});
if (existing) {
Expand Down Expand Up @@ -305,8 +313,10 @@ export class CustomerService {
}

async softDelete(customerId: ID): Promise<DeletionResponse> {
await getEntityOrThrow(this.connection, Customer, customerId);
const customer = await getEntityOrThrow(this.connection, Customer, customerId);
await this.connection.getRepository(Customer).update({ id: customerId }, { deletedAt: new Date() });
// tslint:disable-next-line:no-non-null-assertion
await this.userService.softDelete(customer.user!.id);
return {
result: DeletionResult.DELETED,
};
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/service/services/user.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import { ConfigService } from '../../config/config.service';
import { User } from '../../entity/user/user.entity';
import { PasswordCiper } from '../helpers/password-cipher/password-ciper';
import { getEntityOrThrow } from '../helpers/utils/get-entity-or-throw';
import { VerificationTokenGenerator } from '../helpers/verification-token-generator/verification-token-generator';

import { RoleService } from './role.service';
Expand Down Expand Up @@ -71,6 +72,11 @@ export class UserService {
return this.connection.manager.save(user);
}

async softDelete(userId: ID) {
await getEntityOrThrow(this.connection, User, userId);
await this.connection.getRepository(User).update({ id: userId }, { deletedAt: new Date() });
}

async setVerificationToken(user: User): Promise<User> {
user.verificationToken = this.verificationTokenGenerator.generateVerificationToken();
user.verified = false;
Expand Down

0 comments on commit 0d4e31a

Please sign in to comment.