Browse Source

fix(server): Correct handling of active order when logging in and out

Michael Bromley 7 years ago
parent
commit
0575a62638

+ 20 - 5
server/e2e/order.e2e-spec.ts

@@ -34,9 +34,9 @@ describe('Orders', () => {
             expect(client.getAuthToken()).toBe('');
         });
 
-        it('activeOrder() creates and returns a new order in the default state', async () => {
+        it('activeOrder() returns null before any items have been added', async () => {
             const result = await client.query(GET_ACTIVE_ORDER);
-            expect(result.activeOrder).toEqual({ id: 'T_1', state: 'AddingItems' });
+            expect(result.activeOrder).toBeNull();
         });
 
         it('activeOrder() creates an anonymous session', () => {
@@ -204,6 +204,9 @@ describe('Orders', () => {
 
     describe('as authenticated user', () => {
         let firstOrderItemId: string;
+        let activeOrderId: string;
+        let authenticatedUserEmailAddress: string;
+        const password = 'test';
 
         beforeAll(async () => {
             await client.asSuperAdmin();
@@ -216,12 +219,13 @@ describe('Orders', () => {
                 },
             );
             const customer = result.customers.items[0];
-            await client.asUserWithCredentials(customer.emailAddress, 'test');
+            authenticatedUserEmailAddress = customer.emailAddress;
+            await client.asUserWithCredentials(authenticatedUserEmailAddress, password);
         });
 
-        it('activeOrder() creates and returns a new order in the default state', async () => {
+        it('activeOrder() returns null before any items have been added', async () => {
             const result = await client.query(GET_ACTIVE_ORDER);
-            expect(result.activeOrder).toEqual({ id: 'T_2', state: 'AddingItems' });
+            expect(result.activeOrder).toBeNull();
         });
 
         it('addItemToOrder() creates a new Order with an item', async () => {
@@ -233,6 +237,7 @@ describe('Orders', () => {
             expect(result.addItemToOrder.lines.length).toBe(1);
             expect(result.addItemToOrder.lines[0].quantity).toBe(1);
             expect(result.addItemToOrder.lines[0].productVariant.id).toBe('T_1');
+            activeOrderId = result.addItemToOrder.id;
             firstOrderItemId = result.addItemToOrder.lines[0].id;
         });
 
@@ -280,6 +285,16 @@ describe('Orders', () => {
 
             expect(result.nextOrderStates).toEqual(['ArrangingShipping']);
         });
+
+        it('logging out and back in again resumes the last active order', async () => {
+            await client.asAnonymousUser();
+            const result1 = await client.query(GET_ACTIVE_ORDER);
+            expect(result1.activeOrder).toBeNull();
+
+            await client.asUserWithCredentials(authenticatedUserEmailAddress, password);
+            const result2 = await client.query(GET_ACTIVE_ORDER);
+            expect(result2.activeOrder.id).toBe(activeOrderId);
+        });
     });
 });
 

+ 13 - 2
server/src/api/common/request-context.ts

@@ -3,10 +3,9 @@ import { ID } from 'shared/shared-types';
 
 import { DEFAULT_LANGUAGE_CODE } from '../../common/constants';
 import { Channel } from '../../entity/channel/channel.entity';
+import { AuthenticatedSession } from '../../entity/session/authenticated-session.entity';
 import { Session } from '../../entity/session/session.entity';
-import { User } from '../../entity/user/user.entity';
 import { Zone } from '../../entity/zone/zone.entity';
-import { I18nError } from '../../i18n/i18n-error';
 
 /**
  * The RequestContext is intended to hold information relevant to the current request, which may be
@@ -51,6 +50,14 @@ export class RequestContext {
         return this._session;
     }
 
+    get activeUserId(): ID | undefined {
+        if (this.session) {
+            if (this.isAuthenticatedSession(this.session)) {
+                return this.session.user.id;
+            }
+        }
+    }
+
     get activeTaxZone(): Zone {
         // TODO: This will vary depending on Customer data available -
         // a customer with a billing address in another zone will alter the value etc.
@@ -71,4 +78,8 @@ export class RequestContext {
     get authorizedAsOwnerOnly(): boolean {
         return this._authorizedAsOwnerOnly;
     }
+
+    private isAuthenticatedSession(session: Session): session is AuthenticatedSession {
+        return session.hasOwnProperty('user');
+    }
 }

+ 6 - 4
server/src/api/resolvers/auth.resolver.ts

@@ -3,7 +3,6 @@ import { Request, Response } from 'express';
 import { LoginMutationArgs, LoginResult, Permission } from 'shared/generated-types';
 
 import { ConfigService } from '../../config/config.service';
-import { AuthenticatedSession } from '../../entity/session/authenticated-session.entity';
 import { User } from '../../entity/user/user.entity';
 import { AuthService } from '../../service/services/auth.service';
 import { ChannelService } from '../../service/services/channel.service';
@@ -26,12 +25,14 @@ export class AuthResolver {
      * the user data and returns the token either in a cookie or in the response body.
      */
     @Mutation()
+    @Allow(Permission.Public)
     async login(
         @Args() args: LoginMutationArgs,
+        @Ctx() ctx: RequestContext,
         @Context('req') req: Request,
         @Context('res') res: Response,
     ): Promise<LoginResult> {
-        const session = await this.authService.authenticate(args.username, args.password);
+        const session = await this.authService.authenticate(ctx, args.username, args.password);
         setAuthToken({
             req,
             res,
@@ -45,6 +46,7 @@ export class AuthResolver {
     }
 
     @Mutation()
+    @Allow(Permission.Public)
     async logout(@Context('req') req: Request, @Context('res') res: Response): Promise<boolean> {
         const token = extractAuthToken(req, this.configService.authOptions.tokenMethod);
         if (!token) {
@@ -67,8 +69,8 @@ export class AuthResolver {
     @Query()
     @Allow(Permission.Authenticated)
     async me(@Ctx() ctx: RequestContext) {
-        const sessionUser = (ctx.session as AuthenticatedSession).user;
-        const user = sessionUser && (await this.authService.getUserById(sessionUser.id));
+        const userId = ctx.activeUserId;
+        const user = userId && (await this.authService.getUserById(userId));
         return user ? this.publiclyAccessibleUser(user) : null;
     }
 

+ 22 - 8
server/src/api/resolvers/order.resolver.ts

@@ -62,7 +62,7 @@ export class OrderResolver {
     @Allow(Permission.Owner)
     async nextOrderStates(@Ctx() ctx: RequestContext): Promise<string[]> {
         if (ctx.authorizedAsOwnerOnly) {
-            const sessionOrder = await this.getOrderFromContext(ctx);
+            const sessionOrder = await this.getOrderFromContext(ctx, true);
             return this.orderService.getNextOrderStates(sessionOrder);
         }
         return [];
@@ -75,7 +75,7 @@ export class OrderResolver {
         @Args() args: TransitionOrderToStateMutationArgs,
     ): Promise<Order | undefined> {
         if (ctx.authorizedAsOwnerOnly) {
-            const sessionOrder = await this.getOrderFromContext(ctx);
+            const sessionOrder = await this.getOrderFromContext(ctx, true);
             return this.orderService.transitionToState(ctx, sessionOrder.id, args.state as OrderState);
         }
     }
@@ -87,7 +87,7 @@ export class OrderResolver {
         @Ctx() ctx: RequestContext,
         @Args() args: AddItemToOrderMutationArgs,
     ): Promise<Order> {
-        const order = await this.getOrderFromContext(ctx);
+        const order = await this.getOrderFromContext(ctx, true);
         return this.orderService.addItemToOrder(ctx, order.id, args.productVariantId, args.quantity);
     }
 
@@ -98,7 +98,7 @@ export class OrderResolver {
         @Ctx() ctx: RequestContext,
         @Args() args: AdjustItemQuantityMutationArgs,
     ): Promise<Order> {
-        const order = await this.getOrderFromContext(ctx);
+        const order = await this.getOrderFromContext(ctx, true);
         return this.orderService.adjustItemQuantity(ctx, order.id, args.orderItemId, args.quantity);
     }
 
@@ -109,18 +109,32 @@ export class OrderResolver {
         @Ctx() ctx: RequestContext,
         @Args() args: RemoveItemFromOrderMutationArgs,
     ): Promise<Order> {
-        const order = await this.getOrderFromContext(ctx);
+        const order = await this.getOrderFromContext(ctx, true);
         return this.orderService.removeItemFromOrder(ctx, order.id, args.orderItemId);
     }
 
-    private async getOrderFromContext(ctx: RequestContext): Promise<Order> {
+    private async getOrderFromContext(ctx: RequestContext): Promise<Order | undefined>;
+    private async getOrderFromContext(ctx: RequestContext, createIfNotExists: true): Promise<Order>;
+    private async getOrderFromContext(
+        ctx: RequestContext,
+        createIfNotExists = false,
+    ): Promise<Order | undefined> {
         if (!ctx.session) {
             throw new I18nError(`error.no-active-session`);
         }
         let order = ctx.session.activeOrder;
         if (!order) {
-            order = await this.orderService.create();
-            await this.authService.setActiveOrder(ctx.session, order);
+            if (ctx.activeUserId) {
+                order = await this.orderService.getActiveOrderForUser(ctx, ctx.activeUserId);
+            }
+
+            if (!order && createIfNotExists) {
+                order = await this.orderService.create(ctx.activeUserId);
+            }
+
+            if (order) {
+                await this.authService.setActiveOrder(ctx.session, order);
+            }
         }
         return order;
     }

+ 3 - 0
server/src/entity/order/order.entity.ts

@@ -19,6 +19,9 @@ export class Order extends VendureEntity {
 
     @Column('varchar') state: OrderState;
 
+    @Column({ default: true })
+    active: boolean;
+
     @ManyToOne(type => Customer)
     customer: Customer;
 

+ 12 - 3
server/src/service/services/auth.service.ts

@@ -5,15 +5,17 @@ import * as ms from 'ms';
 import { ID } from 'shared/shared-types';
 import { Connection } from 'typeorm';
 
+import { RequestContext } from '../../api/common/request-context';
 import { ConfigService } from '../../config/config.service';
 import { Order } from '../../entity/order/order.entity';
 import { AnonymousSession } from '../../entity/session/anonymous-session.entity';
 import { AuthenticatedSession } from '../../entity/session/authenticated-session.entity';
 import { Session } from '../../entity/session/session.entity';
 import { User } from '../../entity/user/user.entity';
-
 import { PasswordCiper } from '../helpers/password-cipher/password-ciper';
 
+import { OrderService } from './order.service';
+
 /**
  * The AuthService manages both authenticated and anonymous sessions.
  */
@@ -22,9 +24,10 @@ export class AuthService {
     private readonly sessionDurationInMs;
 
     constructor(
-        private passwordCipher: PasswordCiper,
         @InjectConnection() private connection: Connection,
+        private passwordCipher: PasswordCiper,
         private configService: ConfigService,
+        private orderService: OrderService,
     ) {
         this.sessionDurationInMs = ms(this.configService.authOptions.sessionDuration);
     }
@@ -32,16 +35,22 @@ export class AuthService {
     /**
      * Authenticates a user's credentials and if okay, creates a new session.
      */
-    async authenticate(identifier: string, password: string): Promise<AuthenticatedSession> {
+    async authenticate(
+        ctx: RequestContext,
+        identifier: string,
+        password: string,
+    ): Promise<AuthenticatedSession> {
         const user = await this.getUserFromIdentifier(identifier);
         const passwordMatches = await this.passwordCipher.check(password, user.passwordHash);
         if (!passwordMatches) {
             throw new UnauthorizedException();
         }
         const token = await this.generateSessionToken();
+        const activeOrder = await this.orderService.getActiveOrderForUser(ctx, user.id);
         const session = new AuthenticatedSession({
             token,
             user,
+            activeOrder,
             expires: this.getExpiryDate(this.sessionDurationInMs),
             invalidated: false,
         });

+ 24 - 1
server/src/service/services/order.service.ts

@@ -18,12 +18,14 @@ import { OrderState } from '../helpers/order-state-machine/order-state';
 import { OrderStateMachine } from '../helpers/order-state-machine/order-state-machine';
 import { translateDeep } from '../helpers/utils/translate-entity';
 
+import { CustomerService } from './customer.service';
 import { ProductVariantService } from './product-variant.service';
 
 export class OrderService {
     constructor(
         @InjectConnection() private connection: Connection,
         private productVariantService: ProductVariantService,
+        private customerService: CustomerService,
         private orderCalculator: OrderCalculator,
         private orderStateMachine: OrderStateMachine,
         private listQueryBuilder: ListQueryBuilder,
@@ -63,7 +65,22 @@ export class OrderService {
         }
     }
 
-    create(): Promise<Order> {
+    async getActiveOrderForUser(ctx: RequestContext, userId: ID): Promise<Order | undefined> {
+        const customer = await this.customerService.findOneByUserId(userId);
+        if (customer) {
+            const activeOrder = await this.connection.getRepository(Order).findOne({
+                where: {
+                    customer,
+                    active: true,
+                },
+            });
+            if (activeOrder) {
+                return this.findOne(ctx, activeOrder.id);
+            }
+        }
+    }
+
+    async create(userId?: ID): Promise<Order> {
         const newOrder = new Order({
             code: generatePublicId(),
             state: this.orderStateMachine.getInitialState(),
@@ -72,6 +89,12 @@ export class OrderService {
             subTotal: 0,
             subTotalBeforeTax: 0,
         });
+        if (userId) {
+            const customer = await this.customerService.findOneByUserId(userId);
+            if (customer) {
+                newOrder.customer = customer;
+            }
+        }
         return this.connection.getRepository(Order).save(newOrder);
     }