Skip to content

Commit

Permalink
Merge pull request #5477 from reactioncommerce/fix-aldeed-5098-publis…
Browse files Browse the repository at this point in the history
…h-carts-performance

Refactor after-publish cart updates for speed
  • Loading branch information
aldeed authored Aug 27, 2019
2 parents 3cf4aef + 276030b commit dc448b6
Show file tree
Hide file tree
Showing 58 changed files with 924 additions and 924 deletions.
8 changes: 8 additions & 0 deletions imports/collections/schemas/shipping.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,21 @@ export const ShipmentQuote = new SimpleSchema({
carrier: {
type: String
},
handlingPrice: {
type: Number,
optional: true
},
method: {
type: ShippingMethod
},
rate: {
type: Number,
defaultValue: 0.00
},
shippingPrice: {
type: Number,
optional: true
},
shopId: {
type: String,
optional: true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`throws if deleteOne fails 1`] = `"Unable to delete anonymous cart"`;

exports[`throws if insertOne fails 1`] = `"Unable to create account cart"`;
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`throws if deleteOne fails 1`] = `"Unable to delete anonymous cart"`;

exports[`throws if updateOne fails 1`] = `"Unable to update cart"`;
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`throws if deleteOne fails 1`] = `"Unable to delete anonymous cart"`;

exports[`throws if updateOne fails 1`] = `"Unable to update cart"`;

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import ReactionError from "@reactioncommerce/reaction-error";
import { Cart as CartSchema } from "/imports/collections/schemas";
import hashLoginToken from "/imports/node-app/core/util/hashLoginToken";
import addCartItemsUtil from "../util/addCartItems";

Expand All @@ -18,7 +17,7 @@ import addCartItemsUtil from "../util/addCartItems";
*/
export default async function addCartItems(context, input, options = {}) {
const { cartId, items, token } = input;
const { appEvents, collections, accountId = null, userId = null } = context;
const { collections, accountId = null } = context;
const { Cart } = collections;

let selector;
Expand All @@ -45,28 +44,13 @@ export default async function addCartItems(context, input, options = {}) {
updatedItemList
} = await addCartItemsUtil(context, cart.items, items, { skipPriceCheck: options.skipPriceCheck });

const updatedAt = new Date();

const modifier = {
$set: {
items: updatedItemList,
updatedAt
}
};
CartSchema.validate(modifier, { modifier: true });

const { matchedCount } = await Cart.updateOne({ _id: cart._id }, modifier);
if (matchedCount !== 1) throw new ReactionError("server-error", "Unable to update cart");

const updatedCart = {
...cart,
items: updatedItemList,
updatedAt
updatedAt: new Date()
};
await appEvents.emit("afterCartUpdate", {
cart: updatedCart,
updatedBy: userId
});

return { cart: updatedCart, incorrectPriceFailures, minOrderQuantityFailures };
const savedCart = await context.mutations.saveCart(context, updatedCart);

return { cart: savedCart, incorrectPriceFailures, minOrderQuantityFailures };
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ const items = [{
quantity: 1
}];

beforeAll(() => {
if (!mockContext.mutations.saveCart) {
mockContext.mutations.saveCart = jest.fn().mockName("context.mutations.saveCart").mockImplementation(async (_, cart) => cart);
}
});

test("add an item to an existing anonymous cart", async () => {
mockContext.collections.Cart.findOne.mockReturnValueOnce(Promise.resolve({
_id: "cartId",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,23 @@
import Random from "@reactioncommerce/random";
import ReactionError from "@reactioncommerce/reaction-error";
import { Cart as CartSchema } from "/imports/collections/schemas";
import appEvents from "/imports/node-app/core/util/appEvents";

/**
* @summary Copy items from an anonymous cart into a new account cart, and then delete the
* anonymous cart.
* @param {String} accountId The account ID to associate with the new account cart
* @param {Object} context App context
* @param {Object} anonymousCart The anonymous cart document
* @param {Object} anonymousCartSelector The MongoDB selector for the anonymous cart
* @param {MongoDB.Collection} Cart The Cart collection
* @param {String} shopId The shop ID to associate with the new account cart
* @param {String} userId The ID of the user
* @returns {Object} The new account cart
*/
export default async function convertAnonymousCartToNewAccountCart({
accountId,
export default async function convertAnonymousCartToNewAccountCart(context, {
anonymousCart,
anonymousCartSelector,
Cart,
shopId,
userId
anonymousCartSelector
}) {
const { accountId, collections: { Cart } } = context;

const createdAt = new Date();
const currencyCode = anonymousCart.currencyCode || "USD";
const { _id, referenceId } = anonymousCart;
const { _id, referenceId, shopId } = anonymousCart;

const newCart = {
_id: Random.id(),
Expand All @@ -47,15 +40,7 @@ export default async function convertAnonymousCartToNewAccountCart({
newCart.referenceId = referenceId;
}

CartSchema.validate(newCart);

const { result } = await Cart.insertOne(newCart);
if (result.ok !== 1) throw new ReactionError("server-error", "Unable to create account cart");

await appEvents.emit("afterCartCreate", {
cart: newCart,
createdBy: userId
});
const savedCart = await context.mutations.saveCart(context, newCart);

const { deletedCount } = await Cart.deleteOne(anonymousCartSelector);
if (deletedCount === 0) {
Expand All @@ -65,5 +50,5 @@ export default async function convertAnonymousCartToNewAccountCart({
throw new ReactionError("server-error", "Unable to delete anonymous cart");
}

return newCart;
return savedCart;
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,22 @@ const anonymousCartSelector = { _id: "123" };
const shopId = "shopId";
const items = [Factory.CartItem.makeOne()];

beforeAll(() => {
if (!mockContext.mutations.saveCart) {
mockContext.mutations.saveCart = jest.fn().mockName("context.mutations.saveCart").mockImplementation(async (_, cart) => cart);
}
});

test("inserts a cart with the existing cart's items and returns it", async () => {
Cart.insertOne.mockReturnValueOnce(Promise.resolve({ result: { ok: 1 } }));
mockContext.accountId = accountId;

const result = await convertAnonymousCartToNewAccountCart({
accountId,
const result = await convertAnonymousCartToNewAccountCart(mockContext, {
anonymousCart: {
currencyCode,
items
items,
shopId
},
anonymousCartSelector,
Cart,
shopId
anonymousCartSelector
});

const newCart = {
Expand All @@ -37,43 +41,24 @@ test("inserts a cart with the existing cart's items and returns it", async () =>
}
};

expect(Cart.insertOne).toHaveBeenCalledWith(newCart);

expect(Cart.deleteOne).toHaveBeenCalledWith(anonymousCartSelector);

expect(result).toEqual(newCart);
});

test("throws if insertOne fails", async () => {
Cart.insertOne.mockReturnValueOnce(Promise.resolve({ result: { ok: 0 } }));

const promise = convertAnonymousCartToNewAccountCart({
accountId,
anonymousCart: {
currencyCode,
items
},
anonymousCartSelector,
Cart,
shopId
});

return expect(promise).rejects.toThrowErrorMatchingSnapshot();
});

test("throws if deleteOne fails", async () => {
Cart.insertOne.mockReturnValueOnce(Promise.resolve({ result: { ok: 1 } }));
Cart.deleteOne.mockReturnValueOnce(Promise.resolve({ deletedCount: 0 }));

const promise = convertAnonymousCartToNewAccountCart({
accountId,
mockContext.accountId = accountId;

const promise = convertAnonymousCartToNewAccountCart(mockContext, {
anonymousCart: {
currencyCode,
items
items,
shopId
},
anonymousCartSelector,
Cart,
shopId
anonymousCartSelector
});

return expect(promise).rejects.toThrowErrorMatchingSnapshot();
Expand Down
16 changes: 3 additions & 13 deletions imports/plugins/core/cart/server/no-meteor/mutations/createCart.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import Random from "@reactioncommerce/random";
import ReactionError from "@reactioncommerce/reaction-error";
import Logger from "@reactioncommerce/logger";
import hashLoginToken from "/imports/node-app/core/util/hashLoginToken";
import { Cart as CartSchema } from "/imports/collections/schemas";
import addCartItems from "../util/addCartItems";

/**
Expand All @@ -23,7 +22,7 @@ import addCartItems from "../util/addCartItems";
*/
export default async function createCart(context, input) {
const { items, shopId, shouldCreateWithoutItems = false } = input;
const { appEvents, collections, accountId = null, userId = null, getFunctionsOfType } = context;
const { collections, accountId = null, getFunctionsOfType } = context;
const { Cart, Shops } = collections;

if (shouldCreateWithoutItems !== true && (!Array.isArray(items) || !items.length)) {
Expand Down Expand Up @@ -90,16 +89,7 @@ export default async function createCart(context, input) {

newCart.referenceId = referenceId;

CartSchema.validate(newCart);
const savedCart = await context.mutations.saveCart(context, newCart);

const { result } = await Cart.insertOne(newCart);

if (result.ok !== 1) throw new ReactionError("server-error", "Unable to create cart");

await appEvents.emit("afterCartCreate", {
cart: newCart,
createdBy: userId
});

return { cart: newCart, incorrectPriceFailures, minOrderQuantityFailures, token: anonymousAccessToken };
return { cart: savedCart, incorrectPriceFailures, minOrderQuantityFailures, token: anonymousAccessToken };
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ const items = [{
quantity: 1
}];

beforeAll(() => {
if (!mockContext.mutations.saveCart) {
mockContext.mutations.saveCart = jest.fn().mockName("context.mutations.saveCart").mockImplementation(async (_, cart) => cart);
}
});

test("creates an anonymous cart if no user is logged in", async () => {
const originalAccountId = mockContext.accountId;
mockContext.accountId = null;
Expand Down
6 changes: 6 additions & 0 deletions imports/plugins/core/cart/server/no-meteor/mutations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ import reconcileCartsKeepAccountCart from "./reconcileCartsKeepAccountCart";
import reconcileCartsKeepAnonymousCart from "./reconcileCartsKeepAnonymousCart";
import reconcileCartsMerge from "./reconcileCartsMerge";
import removeCartItems from "./removeCartItems";
import saveCart from "./saveCart";
import saveManyCarts from "./saveManyCarts";
import setEmailOnAnonymousCart from "./setEmailOnAnonymousCart";
import setShippingAddressOnCart from "./setShippingAddressOnCart";
import transformAndValidateCart from "./transformAndValidateCart";
import updateCartItemsQuantity from "./updateCartItemsQuantity";

export default {
Expand All @@ -19,7 +22,10 @@ export default {
reconcileCartsKeepAnonymousCart,
reconcileCartsMerge,
removeCartItems,
saveCart,
saveManyCarts,
setEmailOnAnonymousCart,
setShippingAddressOnCart,
transformAndValidateCart,
updateCartItemsQuantity
};
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import reconcileCartsMerge from "./reconcileCartsMerge";
* @returns {Promise<Object>} Object in which `cart` property is set to the updated account cart
*/
export default async function reconcileCarts(context, input) {
const { accountId, collections, user, userId = null } = context;
const { accountId, collections, user } = context;
const { Cart } = collections;
const { anonymousCartId, anonymousCartToken, mode = "merge" } = input;

Expand Down Expand Up @@ -57,17 +57,17 @@ export default async function reconcileCarts(context, input) {
switch (mode) {
case "keepAccountCart":
return {
cart: await reconcileCartsKeepAccountCart({ accountCart, anonymousCartSelector, Cart, userId })
cart: await reconcileCartsKeepAccountCart({ accountCart, anonymousCartSelector, Cart })
};

case "keepAnonymousCart":
return {
cart: await reconcileCartsKeepAnonymousCart({ accountCart, accountCartSelector, anonymousCart, anonymousCartSelector, Cart, userId })
cart: await reconcileCartsKeepAnonymousCart({ accountCart, anonymousCart, anonymousCartSelector, context })
};

case "merge":
return {
cart: await reconcileCartsMerge({ accountCart, accountCartSelector, anonymousCart, anonymousCartSelector, context, userId })
cart: await reconcileCartsMerge({ accountCart, anonymousCart, anonymousCartSelector, context })
};

default:
Expand All @@ -77,13 +77,9 @@ export default async function reconcileCarts(context, input) {

// We have only an anonymous cart, so convert it to an account cart
return {
cart: await convertAnonymousCartToNewAccountCart({
accountId,
cart: await convertAnonymousCartToNewAccountCart(context, {
anonymousCart,
anonymousCartSelector,
Cart,
shopId,
userId
anonymousCartSelector
})
};
}
Loading

0 comments on commit dc448b6

Please sign in to comment.