From 087e7e6651069fe76d88972b07090f9c15bf652f Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Tue, 5 Mar 2019 15:34:27 -0600 Subject: [PATCH 01/11] feat: create moveOrderItems internal mutation Signed-off-by: Eric Dobbertin --- .../__snapshots__/moveOrderItems.test.js.snap | 29 + .../server/no-meteor/mutations/index.js | 2 + .../no-meteor/mutations/moveOrderItems.js | 135 ++++ .../mutations/moveOrderItems.test.js | 598 ++++++++++++++++++ imports/test-utils/helpers/mockContext.js | 6 + 5 files changed, 770 insertions(+) create mode 100644 imports/plugins/core/orders/server/no-meteor/mutations/__snapshots__/moveOrderItems.test.js.snap create mode 100644 imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.js create mode 100644 imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js diff --git a/imports/plugins/core/orders/server/no-meteor/mutations/__snapshots__/moveOrderItems.test.js.snap b/imports/plugins/core/orders/server/no-meteor/mutations/__snapshots__/moveOrderItems.test.js.snap new file mode 100644 index 00000000000..45d94fd4fa1 --- /dev/null +++ b/imports/plugins/core/orders/server/no-meteor/mutations/__snapshots__/moveOrderItems.test.js.snap @@ -0,0 +1,29 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`throws if an order item doesn't exist 1`] = `"Some order items not found"`; + +exports[`throws if fromFulfillmentGroupId isn't supplied 1`] = `"From fulfillment group ID is required"`; + +exports[`throws if itemIds is empty 1`] = `"You must specify at least 1 values"`; + +exports[`throws if itemIds isn't supplied 1`] = `"Item ids is required"`; + +exports[`throws if orderId isn't supplied 1`] = `"Order ID is required"`; + +exports[`throws if permission check fails 1`] = `"Access Denied"`; + +exports[`throws if the database update fails 1`] = `"Unable to update order"`; + +exports[`throws if the from group would have no items remaining 1`] = `"You must specify at least 1 values"`; + +exports[`throws if the fromFulfillmentGroup doesn't exist 1`] = `"Order fulfillment group (from) not found"`; + +exports[`throws if the order doesn't exist 1`] = `"Order not found"`; + +exports[`throws if the toFulfillmentGroup doesn't exist 1`] = `"Order fulfillment group (to) not found"`; + +exports[`throws if toFulfillmentGroupId isn't supplied 1`] = `"To fulfillment group ID is required"`; + +exports[`throws if user who placed order tries to move item at invalid current item status 1`] = `"Item status (processing) is not one of: new"`; + +exports[`throws if user who placed order tries to move item at invalid current order status 1`] = `"Order status (processing) is not one of: new"`; diff --git a/imports/plugins/core/orders/server/no-meteor/mutations/index.js b/imports/plugins/core/orders/server/no-meteor/mutations/index.js index cd40e60d70b..6c8e87ecc46 100644 --- a/imports/plugins/core/orders/server/no-meteor/mutations/index.js +++ b/imports/plugins/core/orders/server/no-meteor/mutations/index.js @@ -1,5 +1,7 @@ +import moveOrderItems from "./moveOrderItems"; import placeOrder from "./placeOrder"; export default { + moveOrderItems, placeOrder }; diff --git a/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.js b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.js new file mode 100644 index 00000000000..c8155f0144b --- /dev/null +++ b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.js @@ -0,0 +1,135 @@ +import SimpleSchema from "simpl-schema"; +import ReactionError from "@reactioncommerce/reaction-error"; +import { Order as OrderSchema } from "/imports/collections/schemas"; + +// These should eventually be configurable in settings +const itemStatusesThatOrdererCanMove = ["new"]; +const orderStatusesThatOrdererCanMove = ["new"]; + +const inputSchema = new SimpleSchema({ + "fromFulfillmentGroupId": String, + "itemIds": { + type: Array, + minCount: 1 + }, + "itemIds.$": String, + "orderId": String, + "toFulfillmentGroupId": String +}); + +/** + * @method moveOrderItems + * @summary Use this mutation to move one or more items between existing order + * fulfillment groups. + * @param {Object} context - an object containing the per-request state + * @param {Object} input - Necessary input. See SimpleSchema + * @return {Promise} Object with `order` property containing the updated order + */ +export default async function moveOrderItems(context, input) { + inputSchema.validate(input); + + const { + fromFulfillmentGroupId, + itemIds, + orderId, + toFulfillmentGroupId + } = input; + + const { accountId, appEvents, collections, isInternalCall, userHasPermission, userId } = context; + const { Orders } = collections; + + // First verify that this order actually exists + const order = await Orders.findOne({ _id: orderId }); + if (!order) throw new ReactionError("not-found", "Order not found"); + + // Allow move if the account that placed the order is attempting to move + // or if the account has "orders" permission. When called internally by another + // plugin, context.isInternalCall can be set to `true` to disable this check. + if ( + !isInternalCall && + (!accountId || accountId !== order.accountId) && + !userHasPermission(["orders"], order.shopId) + ) { + throw new ReactionError("access-denied", "Access Denied"); + } + + // Is the account calling this mutation also the account that placed the order? + // We need this check in a couple places below, so we'll get it here. + const accountIsOrderer = (order.accountId && accountId === order.accountId); + + // The orderer may only move items while the order status is still "new" + if (accountIsOrderer && !orderStatusesThatOrdererCanMove.includes(order.workflow.status)) { + throw new ReactionError("invalid", `Order status (${order.workflow.status}) is not one of: ${orderStatusesThatOrdererCanMove.join(", ")}`); + } + + // Find the two fulfillment groups we're modifying + const fromGroup = order.shipping.find((group) => group._id === fromFulfillmentGroupId); + if (!fromGroup) throw new ReactionError("not-found", "Order fulfillment group (from) not found"); + + const toGroup = order.shipping.find((group) => group._id === toFulfillmentGroupId); + if (!toGroup) throw new ReactionError("not-found", "Order fulfillment group (to) not found"); + + // Pull out the item's we're moving + const foundItemIds = []; + const movedItems = fromGroup.items.reduce((list, item) => { + if (itemIds.includes(item._id)) { + // The orderer may only move while the order item status is still "new" + if (accountIsOrderer && !itemStatusesThatOrdererCanMove.includes(item.workflow.status)) { + throw new ReactionError("invalid", `Item status (${item.workflow.status}) is not one of: ${itemStatusesThatOrdererCanMove.join(", ")}`); + } + + list.push(item); + foundItemIds.push(item._id); + } + return list; + }, []); + + if (!itemIds.every((id) => foundItemIds.includes(id))) { + throw new ReactionError("not-found", "Some order items not found"); + } + + // Find and move the items + const updatedGroups = order.shipping.map((group) => { + if (group._id === fromFulfillmentGroupId) { + // Return group items with the moved items removed + return { + ...group, + items: group.items.filter((item) => !itemIds.includes(item._id)) + }; + } + + if (group._id === toFulfillmentGroupId) { + // Return group items with the moved items added + return { + ...group, + items: [...group.items, ...movedItems] + }; + } + + return group; + }); + + // We're now ready to actually update the database and emit events + const modifier = { + $set: { + shipping: updatedGroups, + updatedAt: new Date() + } + }; + + OrderSchema.validate(modifier, { modifier: true }); + + const { modifiedCount, value: updatedOrder } = await Orders.findOneAndUpdate( + { _id: orderId }, + modifier, + { returnOriginal: false } + ); + if (modifiedCount === 0 || !updatedOrder) throw new ReactionError("server-error", "Unable to update order"); + + await appEvents.emit("afterOrderUpdate", { + order: updatedOrder, + updatedBy: userId + }); + + return { order: updatedOrder }; +} diff --git a/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js new file mode 100644 index 00000000000..60a4060d50d --- /dev/null +++ b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js @@ -0,0 +1,598 @@ +import moveOrderItems from "./moveOrderItems"; +import Factory from "/imports/test-utils/helpers/factory"; +import mockContext from "/imports/test-utils/helpers/mockContext"; + +beforeEach(() => { + jest.resetAllMocks(); +}); + +test("throws if orderId isn't supplied", async () => { + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if itemIds isn't supplied", async () => { + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if itemIds is empty", async () => { + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: [], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if fromFulfillmentGroupId isn't supplied", async () => { + await expect(moveOrderItems(mockContext, { + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if toFulfillmentGroupId isn't supplied", async () => { + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if the order doesn't exist", async () => { + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve(null)); + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if the fromFulfillmentGroup doesn't exist", async () => { + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + shipping: [ + { + _id: "group200", + items: [ + { + _id: "xyz", + workflow: { + status: "new", + workflow: ["new"] + } + } + ], + workflow: { + status: "new", + workflow: ["new"] + } + } + ], + shopId: "SHOP_ID", + workflow: { + status: "new", + workflow: ["new"] + } + })); + + mockContext.userHasPermission.mockReturnValueOnce(true); + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if the toFulfillmentGroup doesn't exist", async () => { + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + shipping: [ + { + _id: "group1", + items: [ + { + _id: "xyz", + workflow: { + status: "new", + workflow: ["new"] + } + } + ], + workflow: { + status: "new", + workflow: ["new"] + } + } + ], + shopId: "SHOP_ID", + workflow: { + status: "new", + workflow: ["new"] + } + })); + + mockContext.userHasPermission.mockReturnValueOnce(true); + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if an order item doesn't exist", async () => { + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + shipping: [ + { + _id: "group1", + items: [ + { + _id: "xyz", + workflow: { + status: "new", + workflow: ["new"] + } + } + ], + workflow: { + status: "new", + workflow: ["new"] + } + }, + { + _id: "group2", + items: [ + { + _id: "xyz", + workflow: { + status: "new", + workflow: ["new"] + } + } + ], + workflow: { + status: "new", + workflow: ["new"] + } + } + ], + shopId: "SHOP_ID", + workflow: { + status: "new", + workflow: ["new"] + } + })); + + mockContext.userHasPermission.mockReturnValueOnce(true); + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if permission check fails", async () => { + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + shipping: [ + { + items: [] + } + ], + shopId: "SHOP_ID" + })); + + mockContext.userHasPermission.mockReturnValueOnce(false); + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); + + expect(mockContext.userHasPermission).toHaveBeenCalledWith(["orders"], "SHOP_ID"); +}); + +test("throws if user who placed order tries to move item at invalid current item status", async () => { + const accountId = "ACCOUNT_ID"; + + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + accountId, + shipping: [ + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group1", + items: [ + Factory.OrderItem.makeOne({ + _id: "item1", + quantity: 1, + workflow: { + status: "processing", + workflow: ["new", "processing"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }), + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group2", + items: [ + Factory.OrderItem.makeOne({ + _id: "item2", + quantity: 1, + workflow: { + status: "processing", + workflow: ["new", "processing"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + shopId: "SHOP_ID", + workflow: { + status: "new", + workflow: ["new"] + } + })); + + mockContext.accountId = accountId; + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); + + mockContext.accountId = null; +}); + +test("throws if user who placed order tries to move item at invalid current order status", async () => { + const accountId = "ACCOUNT_ID"; + + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + accountId, + shipping: [ + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group1", + items: [ + Factory.OrderItem.makeOne({ + _id: "item1", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }), + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group2", + items: [ + Factory.OrderItem.makeOne({ + _id: "item2", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + shopId: "SHOP_ID", + workflow: { + status: "processing", + workflow: ["new", "processing"] + } + })); + + mockContext.accountId = accountId; + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); + + mockContext.accountId = null; +}); + +test("throws if the from group would have no items remaining", async () => { + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + shipping: [ + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group1", + items: [ + Factory.OrderItem.makeOne({ + _id: "item1", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }), + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group2", + items: [ + Factory.OrderItem.makeOne({ + _id: "item2", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + shopId: "SHOP_ID", + workflow: { + status: "new", + workflow: ["new"] + } + })); + + mockContext.userHasPermission.mockReturnValueOnce(true); + + mockContext.collections.Orders.findOneAndUpdate.mockReturnValueOnce(Promise.resolve({ + modifiedCount: 0 + })); + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("throws if the database update fails", async () => { + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + shipping: [ + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group1", + items: [ + Factory.OrderItem.makeOne({ + _id: "item1", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }), + Factory.OrderItem.makeOne({ + _id: "item10", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }), + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group2", + items: [ + Factory.OrderItem.makeOne({ + _id: "item2", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + shopId: "SHOP_ID", + workflow: { + status: "new", + workflow: ["new"] + } + })); + + mockContext.userHasPermission.mockReturnValueOnce(true); + + mockContext.collections.Orders.findOneAndUpdate.mockReturnValueOnce(Promise.resolve({ + modifiedCount: 0 + })); + + await expect(moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + })).rejects.toThrowErrorMatchingSnapshot(); +}); + +test("skips permission check if context.isInternalCall", async () => { + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + shipping: [ + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group1", + items: [ + Factory.OrderItem.makeOne({ + _id: "item1", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }), + Factory.OrderItem.makeOne({ + _id: "item10", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }), + Factory.OrderFulfillmentGroup.makeOne({ + _id: "group2", + items: [ + Factory.OrderItem.makeOne({ + _id: "item2", + quantity: 1, + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + workflow: { + status: "new", + workflow: ["new"] + } + }) + ], + shopId: "SHOP_ID", + workflow: { + status: "new", + workflow: ["new"] + } + })); + + mockContext.collections.Orders.findOneAndUpdate.mockReturnValueOnce(Promise.resolve({ + modifiedCount: 1, + value: {} + })); + + mockContext.isInternalCall = true; + + await moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + }); + + delete mockContext.isInternalCall; + + expect(mockContext.userHasPermission).not.toHaveBeenCalled(); +}); + +test("moves items", async () => { + const group1Items = Factory.OrderItem.makeMany(3, { + _id: (index) => `item_1_${index}`, + workflow: { + status: "new", + workflow: ["new"] + } + }); + + const group2Items = Factory.OrderItem.makeMany(2, { + _id: (index) => `item_2_${index}`, + workflow: { + status: "new", + workflow: ["new"] + } + }); + + const group1 = Factory.OrderFulfillmentGroup.makeOne({ + _id: "group1", + items: group1Items + }); + + const group2 = Factory.OrderFulfillmentGroup.makeOne({ + _id: "group2", + items: group2Items + }); + + mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ + _id: "order1", + shipping: [group1, group2], + shopId: "SHOP_ID", + workflow: { + status: "new", + workflow: ["new"] + } + })); + + mockContext.userHasPermission.mockReturnValueOnce(true); + + mockContext.collections.Orders.findOneAndUpdate.mockReturnValueOnce(Promise.resolve({ + modifiedCount: 1, + value: {} + })); + + await moveOrderItems(mockContext, { + fromFulfillmentGroupId: "group1", + itemIds: ["item_1_0", "item_1_1"], + orderId: "order1", + toFulfillmentGroupId: "group2" + }); + + expect(mockContext.collections.Orders.findOneAndUpdate).toHaveBeenCalledWith( + { _id: "order1" }, + { + $set: { + shipping: [ + { + ...group1, + items: [group1.items[2]] + }, + { + ...group2, + items: [...group2.items, group1.items[0], group1.items[1]] + } + ], + updatedAt: jasmine.any(Date) + } + }, + { returnOriginal: false } + ); +}); diff --git a/imports/test-utils/helpers/mockContext.js b/imports/test-utils/helpers/mockContext.js index 212cccea5a2..b997e185af5 100644 --- a/imports/test-utils/helpers/mockContext.js +++ b/imports/test-utils/helpers/mockContext.js @@ -11,6 +11,11 @@ const mockContext = { userId: "FAKE_USER_ID" }; +/** + * @summary Returns a mock collection instance with the given name + * @param {String} collectionName The collection name + * @returns {Object} Mock collection instance + */ export function mockCollection(collectionName) { return { insert() { @@ -32,6 +37,7 @@ export function mockCollection(collectionName) { .mockReturnThis(), findOne: jest.fn().mockName(`${collectionName}.findOne`), findOneAndDelete: jest.fn().mockName(`${collectionName}.findOneAndDelete`), + findOneAndUpdate: jest.fn().mockName(`${collectionName}.findOneAndUpdate`), fetch: jest.fn().mockName(`${collectionName}.fetch`), insertOne: jest.fn().mockName(`${collectionName}.insertOne`), insertMany: jest.fn().mockName(`${collectionName}.insertMany`), From 6a556359344c40fa8d5c9d73da77e5042b411355 Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Tue, 5 Mar 2019 16:08:50 -0600 Subject: [PATCH 02/11] feat: create moveOrderItems GraphQL mutation Signed-off-by: Eric Dobbertin --- .../no-meteor/resolvers/Mutation/index.js | 2 + .../resolvers/Mutation/moveOrderItems.js | 44 ++++++++ .../server/no-meteor/schemas/schema.graphql | 33 ++++++ tests/order/MoveOrderItemsMutation.graphql | 19 ++++ tests/order/moveOrderItems.test.js | 104 ++++++++++++++++++ 5 files changed, 202 insertions(+) create mode 100644 imports/plugins/core/orders/server/no-meteor/resolvers/Mutation/moveOrderItems.js create mode 100644 tests/order/MoveOrderItemsMutation.graphql create mode 100644 tests/order/moveOrderItems.test.js diff --git a/imports/plugins/core/orders/server/no-meteor/resolvers/Mutation/index.js b/imports/plugins/core/orders/server/no-meteor/resolvers/Mutation/index.js index cd40e60d70b..6c8e87ecc46 100644 --- a/imports/plugins/core/orders/server/no-meteor/resolvers/Mutation/index.js +++ b/imports/plugins/core/orders/server/no-meteor/resolvers/Mutation/index.js @@ -1,5 +1,7 @@ +import moveOrderItems from "./moveOrderItems"; import placeOrder from "./placeOrder"; export default { + moveOrderItems, placeOrder }; diff --git a/imports/plugins/core/orders/server/no-meteor/resolvers/Mutation/moveOrderItems.js b/imports/plugins/core/orders/server/no-meteor/resolvers/Mutation/moveOrderItems.js new file mode 100644 index 00000000000..c62f8052492 --- /dev/null +++ b/imports/plugins/core/orders/server/no-meteor/resolvers/Mutation/moveOrderItems.js @@ -0,0 +1,44 @@ +import { + decodeOrderFulfillmentGroupOpaqueId, + decodeOrderItemOpaqueId, + decodeOrderOpaqueId +} from "@reactioncommerce/reaction-graphql-xforms/order"; + +/** + * @name Mutation/moveOrderItems + * @method + * @memberof Payments/GraphQL + * @summary resolver for the moveOrderItems GraphQL mutation + * @param {Object} parentResult - unused + * @param {Object} args.input - an object of all mutation arguments that were sent by the client + * @param {String} args.input.fromFulfillmentGroupId - The ID of the order fulfillment group from which all the items + * are to be moved. + * @param {String[]} args.input.itemIds - The list of item IDs to move. The full quantity must be moved. + * @param {String} args.input.orderId - The order ID + * @param {String} args.input.toFulfillmentGroupId - The ID of the order fulfillment group to which all the items + * are to be moved. + * @param {String} [args.input.clientMutationId] - An optional string identifying the mutation call + * @param {Object} context - an object containing the per-request state + * @return {Promise} MoveOrderItemsPayload + */ +export default async function moveOrderItems(parentResult, { input }, context) { + const { + clientMutationId = null, + fromFulfillmentGroupId, + itemIds, + orderId, + toFulfillmentGroupId + } = input; + + const { order } = await context.mutations.moveOrderItems(context, { + fromFulfillmentGroupId: decodeOrderFulfillmentGroupOpaqueId(fromFulfillmentGroupId), + itemIds: itemIds.map(decodeOrderItemOpaqueId), + orderId: decodeOrderOpaqueId(orderId), + toFulfillmentGroupId: decodeOrderFulfillmentGroupOpaqueId(toFulfillmentGroupId) + }); + + return { + clientMutationId, + order + }; +} diff --git a/imports/plugins/core/orders/server/no-meteor/schemas/schema.graphql b/imports/plugins/core/orders/server/no-meteor/schemas/schema.graphql index e26d993c64a..c4866e98eea 100644 --- a/imports/plugins/core/orders/server/no-meteor/schemas/schema.graphql +++ b/imports/plugins/core/orders/server/no-meteor/schemas/schema.graphql @@ -23,6 +23,11 @@ extend type Query { } extend type Mutation { + """ + Use this mutation to move one or more items between existing order fulfillment groups. + """ + moveOrderItems(input: MoveOrderItemsInput!): MoveOrderItemsPayload! + """ Use this mutation to place an order, providing information necessary to pay for it. The order will be placed only if authorization is successful for all submitted payments. @@ -401,6 +406,34 @@ input PlaceOrderInput { payments: [PaymentInput] } +"Input for the moveOrderItems mutation" +input MoveOrderItemsInput { + "The ID of the order fulfillment group from which all the items are to be moved." + fromFulfillmentGroupId: ID! + + "An optional string identifying the mutation call, which will be returned in the response payload" + clientMutationId: String + + "The list of item IDs to move. The full quantity must be moved." + itemIds: [ID]! + + "ID of the order that has the items you want to move" + orderId: ID! + + "The ID of the order fulfillment group to which all the items are to be moved." + toFulfillmentGroupId: ID! +} + +"Response payload for the moveOrderItems mutation" +type MoveOrderItemsPayload { + "The same string you sent with the mutation params, for matching mutation calls with their responses" + clientMutationId: String + + "The updated order" + order: Order! +} + +"Response payload for the placeOrder mutation" type PlaceOrderPayload { "The same string you sent with the mutation params, for matching mutation calls with their responses" clientMutationId: String diff --git a/tests/order/MoveOrderItemsMutation.graphql b/tests/order/MoveOrderItemsMutation.graphql new file mode 100644 index 00000000000..5737a6499a2 --- /dev/null +++ b/tests/order/MoveOrderItemsMutation.graphql @@ -0,0 +1,19 @@ +mutation ($fromFulfillmentGroupId: ID!, $itemIds: [ID]!, $orderId: ID!, $toFulfillmentGroupId: ID!) { + moveOrderItems(input: { + fromFulfillmentGroupId: $fromFulfillmentGroupId, + itemIds: $itemIds, + orderId: $orderId, + toFulfillmentGroupId: $toFulfillmentGroupId + }) { + order { + fulfillmentGroups { + _id + items { + nodes { + _id + } + } + } + } + } +} diff --git a/tests/order/moveOrderItems.test.js b/tests/order/moveOrderItems.test.js new file mode 100644 index 00000000000..fca61dc81f8 --- /dev/null +++ b/tests/order/moveOrderItems.test.js @@ -0,0 +1,104 @@ +import Factory from "/imports/test-utils/helpers/factory"; +import { + encodeOrderFulfillmentGroupOpaqueId, + encodeOrderItemOpaqueId, + encodeOrderOpaqueId +} from "@reactioncommerce/reaction-graphql-xforms/order"; +import TestApp from "../TestApp"; +import MoveOrderItemsMutation from "./MoveOrderItemsMutation.graphql"; + +jest.setTimeout(300000); + +let testApp; +let moveOrderItems; +let catalogItem; +beforeAll(async () => { + testApp = new TestApp(); + await testApp.start(); + await testApp.insertPrimaryShop(); + + catalogItem = Factory.Catalog.makeOne({ + isDeleted: false, + product: Factory.CatalogProduct.makeOne({ + isDeleted: false, + isVisible: true, + variants: Factory.CatalogVariantSchema.makeMany(1) + }) + }); + await testApp.collections.Catalog.insertOne(catalogItem); + + moveOrderItems = testApp.mutate(MoveOrderItemsMutation); +}); + +afterAll(async () => { + await testApp.collections.Catalog.remove({}); + await testApp.collections.Shops.remove({}); + testApp.stop(); +}); + +const accountInternalId = "123"; + +test("user who placed an order can move an order item", async () => { + await testApp.setLoggedInUser({ _id: accountInternalId }); + + const group1Items = Factory.OrderItem.makeMany(3, { + productId: catalogItem.product.productId, + variantId: catalogItem.product.variants[0].variantId, + workflow: { + status: "new", + workflow: ["new"] + } + }); + + const group2Items = Factory.OrderItem.makeMany(2, { + productId: catalogItem.product.productId, + variantId: catalogItem.product.variants[0].variantId, + workflow: { + status: "new", + workflow: ["new"] + } + }); + + const group1 = Factory.OrderFulfillmentGroup.makeOne({ + items: group1Items + }); + + const group2 = Factory.OrderFulfillmentGroup.makeOne({ + items: group2Items + }); + + const order = Factory.Order.makeOne({ + accountId: accountInternalId, + shipping: [group1, group2], + workflow: { + status: "new", + workflow: ["new"] + } + }); + await testApp.collections.Orders.insertOne(order); + + const group1OpaqueId = encodeOrderFulfillmentGroupOpaqueId(group1._id); + const group2OpaqueId = encodeOrderFulfillmentGroupOpaqueId(group2._id); + + let result; + try { + result = await moveOrderItems({ + fromFulfillmentGroupId: group1OpaqueId, + itemIds: [encodeOrderItemOpaqueId(group1Items[0]._id), encodeOrderItemOpaqueId(group1Items[1]._id)], + orderId: encodeOrderOpaqueId(order._id), + toFulfillmentGroupId: group2OpaqueId + }); + } catch (error) { + expect(error).toBeUndefined(); + return; + } + + const updatedGroup1 = result.moveOrderItems.order.fulfillmentGroups.find((group) => + group._id === group1OpaqueId); + + const updatedGroup2 = result.moveOrderItems.order.fulfillmentGroups.find((group) => + group._id === group2OpaqueId); + + expect(updatedGroup1.items.nodes.length).toBe(1); + expect(updatedGroup2.items.nodes.length).toBe(4); +}); From 18d7794dd247437e6c366ffee5fad3ecc063a478 Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Tue, 12 Mar 2019 15:33:34 -0500 Subject: [PATCH 03/11] docs: fix jsdoc comments Signed-off-by: Eric Dobbertin --- .../orders/server/no-meteor/util/addShipmentMethodToGroup.js | 4 ++-- .../core/orders/server/no-meteor/util/addTaxesToGroup.js | 4 ++-- .../no-meteor/util/buildOrderFulfillmentGroupFromInput.js | 4 ++-- .../orders/server/no-meteor/util/getSurchargesForGroup.js | 4 ++-- .../server/no-meteor/queries/getVariantPrice.js | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/imports/plugins/core/orders/server/no-meteor/util/addShipmentMethodToGroup.js b/imports/plugins/core/orders/server/no-meteor/util/addShipmentMethodToGroup.js index c33897132ce..d6d05c808c3 100644 --- a/imports/plugins/core/orders/server/no-meteor/util/addShipmentMethodToGroup.js +++ b/imports/plugins/core/orders/server/no-meteor/util/addShipmentMethodToGroup.js @@ -6,8 +6,8 @@ import xformOrderGroupToCommonOrder from "./xformOrderGroupToCommonOrder"; * @param {Object} context An object containing the per-request state * @param {Object} [billingAddress] The primary billing address for the order, if known * @param {String|null} [cartId] ID of the cart from which the order is being placed, if applicable - * @param {Object} currencyCode Currency code for all money values - * @param {String} discountTotal Calculated discount total + * @param {String} currencyCode Currency code for all money values + * @param {Number} discountTotal Calculated discount total * @param {Object} group The fulfillment group to be mutated * @param {String} orderId ID of existing or new order to which this group will belong * @param {String} selectedFulfillmentMethodId ID of the fulfillment method option chosen by the user diff --git a/imports/plugins/core/orders/server/no-meteor/util/addTaxesToGroup.js b/imports/plugins/core/orders/server/no-meteor/util/addTaxesToGroup.js index 8038612140c..6e265ca6235 100644 --- a/imports/plugins/core/orders/server/no-meteor/util/addTaxesToGroup.js +++ b/imports/plugins/core/orders/server/no-meteor/util/addTaxesToGroup.js @@ -5,8 +5,8 @@ import xformOrderGroupToCommonOrder from "./xformOrderGroupToCommonOrder"; * @param {Object} context An object containing the per-request state * @param {Object} [billingAddress] The primary billing address for the order, if known * @param {String|null} [cartId] ID of the cart from which the order is being placed, if applicable - * @param {Object} currencyCode Currency code for all money values - * @param {String} discountTotal Calculated discount total + * @param {String} currencyCode Currency code for all money values + * @param {Number} discountTotal Calculated discount total * @param {Object} group The fulfillment group to be mutated * @param {String} orderId ID of existing or new order to which this group will belong * @returns {Object} An object with `taxTotal` and `taxableAmount` numeric properties diff --git a/imports/plugins/core/orders/server/no-meteor/util/buildOrderFulfillmentGroupFromInput.js b/imports/plugins/core/orders/server/no-meteor/util/buildOrderFulfillmentGroupFromInput.js index 104b24bf2cb..d7ae52e739c 100644 --- a/imports/plugins/core/orders/server/no-meteor/util/buildOrderFulfillmentGroupFromInput.js +++ b/imports/plugins/core/orders/server/no-meteor/util/buildOrderFulfillmentGroupFromInput.js @@ -9,8 +9,8 @@ import updateGroupTotals from "./updateGroupTotals"; * items array before calculating shipping, tax, surcharges, and totals. * @param {Object} [billingAddress] The primary billing address for the order, if known * @param {String|null} [cartId] ID of the cart from which the order is being placed, if applicable - * @param {Object} currencyCode Currency code for all money values - * @param {String} discountTotal Calculated discount total + * @param {String} currencyCode Currency code for all money values + * @param {Number} discountTotal Calculated discount total * @param {Object} inputGroup Order fulfillment group input. See schema. * @param {String} orderId ID of existing or new order to which this group will belong * @returns {Promise} The fulfillment group diff --git a/imports/plugins/core/orders/server/no-meteor/util/getSurchargesForGroup.js b/imports/plugins/core/orders/server/no-meteor/util/getSurchargesForGroup.js index 2f037b1f701..d0130f02ecd 100644 --- a/imports/plugins/core/orders/server/no-meteor/util/getSurchargesForGroup.js +++ b/imports/plugins/core/orders/server/no-meteor/util/getSurchargesForGroup.js @@ -5,8 +5,8 @@ import xformOrderGroupToCommonOrder from "./xformOrderGroupToCommonOrder"; * @param {Object} context An object containing the per-request state * @param {Object} [billingAddress] The primary billing address for the order, if known * @param {String|null} [cartId] ID of the cart from which the order is being placed, if applicable - * @param {Object} currencyCode Currency code for all money values - * @param {String} discountTotal Calculated discount total + * @param {String} currencyCode Currency code for all money values + * @param {Number} discountTotal Calculated discount total * @param {Object} group The fulfillment group to be mutated * @param {String} orderId ID of existing or new order to which this group will belong * @returns {undefined} diff --git a/imports/plugins/included/simple-pricing/server/no-meteor/queries/getVariantPrice.js b/imports/plugins/included/simple-pricing/server/no-meteor/queries/getVariantPrice.js index e615cfe6365..9b9ef0352ee 100644 --- a/imports/plugins/included/simple-pricing/server/no-meteor/queries/getVariantPrice.js +++ b/imports/plugins/included/simple-pricing/server/no-meteor/queries/getVariantPrice.js @@ -3,7 +3,7 @@ * @summary This method returns the applicable price and currency code for a selected product. * @param {Object} context - App context * @param {Object} catalogVariant - A selected product variant. - * @param {Object} currencyCode - The currency code in which to get price + * @param {String} currencyCode - The currency code in which to get price * @return {Object} - A cart item price value. */ export default function getVariantPrice(context, catalogVariant, currencyCode) { From dd3790dde3e85ab91087b6bf7922dede54ec40d2 Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Tue, 12 Mar 2019 16:04:31 -0500 Subject: [PATCH 04/11] fix: properly update groups after move order items Signed-off-by: Eric Dobbertin --- .../__snapshots__/moveOrderItems.test.js.snap | 4 +- .../no-meteor/mutations/moveOrderItems.js | 59 ++++++++++++++----- .../mutations/moveOrderItems.test.js | 33 +++++++++-- tests/order/moveOrderItems.test.js | 45 ++++++++++++-- 4 files changed, 117 insertions(+), 24 deletions(-) diff --git a/imports/plugins/core/orders/server/no-meteor/mutations/__snapshots__/moveOrderItems.test.js.snap b/imports/plugins/core/orders/server/no-meteor/mutations/__snapshots__/moveOrderItems.test.js.snap index 45d94fd4fa1..77e17f23a52 100644 --- a/imports/plugins/core/orders/server/no-meteor/mutations/__snapshots__/moveOrderItems.test.js.snap +++ b/imports/plugins/core/orders/server/no-meteor/mutations/__snapshots__/moveOrderItems.test.js.snap @@ -12,9 +12,9 @@ exports[`throws if orderId isn't supplied 1`] = `"Order ID is required"`; exports[`throws if permission check fails 1`] = `"Access Denied"`; -exports[`throws if the database update fails 1`] = `"Unable to update order"`; +exports[`throws if the database update fails 1`] = `"queries.getFulfillmentMethodsWithQuotes is not a function"`; -exports[`throws if the from group would have no items remaining 1`] = `"You must specify at least 1 values"`; +exports[`throws if the from group would have no items remaining 1`] = `"move would result in group having no items"`; exports[`throws if the fromFulfillmentGroup doesn't exist 1`] = `"Order fulfillment group (from) not found"`; diff --git a/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.js b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.js index c8155f0144b..dc7949ca127 100644 --- a/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.js +++ b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.js @@ -1,6 +1,8 @@ import SimpleSchema from "simpl-schema"; import ReactionError from "@reactioncommerce/reaction-error"; import { Order as OrderSchema } from "/imports/collections/schemas"; +import updateGroupStatusFromItemStatus from "../util/updateGroupStatusFromItemStatus"; +import updateGroupTotals from "../util/updateGroupTotals"; // These should eventually be configurable in settings const itemStatusesThatOrdererCanMove = ["new"]; @@ -88,26 +90,55 @@ export default async function moveOrderItems(context, input) { throw new ReactionError("not-found", "Some order items not found"); } + const { billingAddress, cartId, currencyCode } = order; + // Find and move the items - const updatedGroups = order.shipping.map((group) => { + const orderSurcharges = []; + const updatedGroups = await Promise.all(order.shipping.map(async (group) => { + if (group._id !== fromFulfillmentGroupId && group._id !== toFulfillmentGroupId) return group; + + let updatedItems; if (group._id === fromFulfillmentGroupId) { - // Return group items with the moved items removed - return { - ...group, - items: group.items.filter((item) => !itemIds.includes(item._id)) - }; + // Remove the moved items + updatedItems = group.items.filter((item) => !itemIds.includes(item._id)); + } else { + // Add the moved items + updatedItems = [...group.items, ...movedItems]; } - if (group._id === toFulfillmentGroupId) { - // Return group items with the moved items added - return { - ...group, - items: [...group.items, ...movedItems] - }; + if (updatedItems.length === 0) { + throw new ReactionError("invalid-param", "move would result in group having no items"); } - return group; - }); + // Create an updated group + const updatedGroup = { + ...group, + // There is a convenience itemIds prop, so update that, too + itemIds: updatedItems.map((item) => item._id), + items: updatedItems, + totalItemQuantity: updatedItems.reduce((sum, item) => sum + item.quantity, 0) + }; + + // Update group shipping, tax, totals, etc. + const { groupSurcharges } = await updateGroupTotals(context, { + billingAddress, + cartId, + currencyCode, + discountTotal: updatedGroup.invoice.discounts, + group: updatedGroup, + orderId, + selectedFulfillmentMethodId: updatedGroup.shipmentMethod._id + }); + + // Push all group surcharges to overall order surcharge array. + // Currently, we do not save surcharges per group + orderSurcharges.push(...groupSurcharges); + + // Ensure proper group status + updateGroupStatusFromItemStatus(updatedGroup); + + return updatedGroup; + })); // We're now ready to actually update the database and emit events const modifier = { diff --git a/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js index 60a4060d50d..95aaa422bc0 100644 --- a/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js +++ b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js @@ -1,11 +1,19 @@ import moveOrderItems from "./moveOrderItems"; import Factory from "/imports/test-utils/helpers/factory"; import mockContext from "/imports/test-utils/helpers/mockContext"; +import { + restore as restore$updateGroupTotals, + rewire as rewire$updateGroupTotals +} from "../util/updateGroupTotals"; beforeEach(() => { jest.resetAllMocks(); }); +afterEach(() => { + restore$updateGroupTotals(); +}); + test("throws if orderId isn't supplied", async () => { await expect(moveOrderItems(mockContext, { fromFulfillmentGroupId: "group1", @@ -513,6 +521,9 @@ test("skips permission check if context.isInternalCall", async () => { mockContext.isInternalCall = true; + const mockUpdateGroupTotals = jest.fn().mockName("updateGroupTotals").mockReturnValue(Promise.resolve({ groupSurcharges: [] })); + rewire$updateGroupTotals(mockUpdateGroupTotals); + await moveOrderItems(mockContext, { fromFulfillmentGroupId: "group1", itemIds: ["item1"], @@ -528,6 +539,7 @@ test("skips permission check if context.isInternalCall", async () => { test("moves items", async () => { const group1Items = Factory.OrderItem.makeMany(3, { _id: (index) => `item_1_${index}`, + quantity: 1, workflow: { status: "new", workflow: ["new"] @@ -536,6 +548,7 @@ test("moves items", async () => { const group2Items = Factory.OrderItem.makeMany(2, { _id: (index) => `item_2_${index}`, + quantity: 1, workflow: { status: "new", workflow: ["new"] @@ -544,18 +557,21 @@ test("moves items", async () => { const group1 = Factory.OrderFulfillmentGroup.makeOne({ _id: "group1", - items: group1Items + items: group1Items, + totalItemQuantity: group1Items.reduce((sum, item) => sum + item.quantity, 0) }); const group2 = Factory.OrderFulfillmentGroup.makeOne({ _id: "group2", - items: group2Items + items: group2Items, + totalItemQuantity: group2Items.reduce((sum, item) => sum + item.quantity, 0) }); mockContext.collections.Orders.findOne.mockReturnValueOnce(Promise.resolve({ _id: "order1", shipping: [group1, group2], shopId: "SHOP_ID", + totalItemQuantity: [group1, group2].reduce((sum, group) => sum + group.totalItemQuantity, 0), workflow: { status: "new", workflow: ["new"] @@ -569,6 +585,9 @@ test("moves items", async () => { value: {} })); + const mockUpdateGroupTotals = jest.fn().mockName("updateGroupTotals").mockReturnValue(Promise.resolve({ groupSurcharges: [] })); + rewire$updateGroupTotals(mockUpdateGroupTotals); + await moveOrderItems(mockContext, { fromFulfillmentGroupId: "group1", itemIds: ["item_1_0", "item_1_1"], @@ -576,6 +595,8 @@ test("moves items", async () => { toFulfillmentGroupId: "group2" }); + const expectedGroup2Items = [...group2.items, group1.items[0], group1.items[1]]; + expect(mockContext.collections.Orders.findOneAndUpdate).toHaveBeenCalledWith( { _id: "order1" }, { @@ -583,11 +604,15 @@ test("moves items", async () => { shipping: [ { ...group1, - items: [group1.items[2]] + items: [group1.items[2]], + itemIds: [group1.items[2]._id], + totalItemQuantity: 1 }, { ...group2, - items: [...group2.items, group1.items[0], group1.items[1]] + items: expectedGroup2Items, + itemIds: expectedGroup2Items.map((item) => item._id), + totalItemQuantity: 4 } ], updatedAt: jasmine.any(Date) diff --git a/tests/order/moveOrderItems.test.js b/tests/order/moveOrderItems.test.js index fca61dc81f8..ed410595f32 100644 --- a/tests/order/moveOrderItems.test.js +++ b/tests/order/moveOrderItems.test.js @@ -12,10 +12,37 @@ jest.setTimeout(300000); let testApp; let moveOrderItems; let catalogItem; +let shopId; + +const fulfillmentMethodId = "METHOD_ID"; +const mockShipmentMethod = { + _id: fulfillmentMethodId, + handling: 0, + label: "mockLabel", + name: "mockName", + rate: 3.99 +}; + beforeAll(async () => { - testApp = new TestApp(); + const getFulfillmentMethodsWithQuotes = (context, commonOrderExtended, [rates]) => { + rates.push({ + carrier: "CARRIER", + handlingPrice: 0, + method: mockShipmentMethod, + rate: 3.99, + shippingPrice: 3.99, + shopId + }); + }; + + testApp = new TestApp({ + functionsByType: { + getFulfillmentMethodsWithQuotes: [getFulfillmentMethodsWithQuotes] + } + }); + await testApp.start(); - await testApp.insertPrimaryShop(); + shopId = await testApp.insertPrimaryShop(); catalogItem = Factory.Catalog.makeOne({ isDeleted: false, @@ -60,11 +87,21 @@ test("user who placed an order can move an order item", async () => { }); const group1 = Factory.OrderFulfillmentGroup.makeOne({ - items: group1Items + items: group1Items, + shipmentMethod: { + ...mockShipmentMethod, + currencyCode: "USD" + }, + shopId }); const group2 = Factory.OrderFulfillmentGroup.makeOne({ - items: group2Items + items: group2Items, + shipmentMethod: { + ...mockShipmentMethod, + currencyCode: "USD" + }, + shopId }); const order = Factory.Order.makeOne({ From 2f14d76d023487ecd6ff186009c18bd271a67629 Mon Sep 17 00:00:00 2001 From: Erik Kieckhafer Date: Tue, 12 Mar 2019 22:27:21 -0700 Subject: [PATCH 05/11] lint fix Signed-off-by: Erik Kieckhafer --- .../orders/server/no-meteor/mutations/moveOrderItems.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js index 95aaa422bc0..d939c4c2720 100644 --- a/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js +++ b/imports/plugins/core/orders/server/no-meteor/mutations/moveOrderItems.test.js @@ -1,10 +1,10 @@ -import moveOrderItems from "./moveOrderItems"; import Factory from "/imports/test-utils/helpers/factory"; import mockContext from "/imports/test-utils/helpers/mockContext"; import { restore as restore$updateGroupTotals, rewire as rewire$updateGroupTotals } from "../util/updateGroupTotals"; +import moveOrderItems from "./moveOrderItems"; beforeEach(() => { jest.resetAllMocks(); From fdc1cbfb16bc56588ec1de14d5a0e284cd6eae19 Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Thu, 14 Mar 2019 10:44:05 -0500 Subject: [PATCH 06/11] fix: naming issue caused taxes to not be calculated Signed-off-by: Eric Dobbertin --- .../core/orders/server/no-meteor/util/addTaxesToGroup.js | 6 +++--- .../no-meteor/mutations/setTaxesOnOrderFulfillmentGroup.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/imports/plugins/core/orders/server/no-meteor/util/addTaxesToGroup.js b/imports/plugins/core/orders/server/no-meteor/util/addTaxesToGroup.js index 6e265ca6235..13b060af51a 100644 --- a/imports/plugins/core/orders/server/no-meteor/util/addTaxesToGroup.js +++ b/imports/plugins/core/orders/server/no-meteor/util/addTaxesToGroup.js @@ -31,12 +31,12 @@ export default async function addTaxesToGroup(context, { discountTotal }); - // A taxes plugin is expected to add a mutation named `setTaxesOnFulfillmentGroup`. + // A taxes plugin is expected to add a mutation named `setTaxesOnOrderFulfillmentGroup`. // If this isn't done, assume 0 tax. - if (typeof mutations.setTaxesOnFulfillmentGroup !== "function") { + if (typeof mutations.setTaxesOnOrderFulfillmentGroup !== "function") { return { taxTotal: 0, taxableAmount: 0 }; } // This will mutate `group` to add whatever tax fields the `taxes` plugin has added to the schemas. - return mutations.setTaxesOnFulfillmentGroup(context, { group, commonOrder }); + return mutations.setTaxesOnOrderFulfillmentGroup(context, { group, commonOrder }); } diff --git a/imports/plugins/core/taxes/server/no-meteor/mutations/setTaxesOnOrderFulfillmentGroup.js b/imports/plugins/core/taxes/server/no-meteor/mutations/setTaxesOnOrderFulfillmentGroup.js index 628fdda8f12..cbfe28083e5 100644 --- a/imports/plugins/core/taxes/server/no-meteor/mutations/setTaxesOnOrderFulfillmentGroup.js +++ b/imports/plugins/core/taxes/server/no-meteor/mutations/setTaxesOnOrderFulfillmentGroup.js @@ -5,7 +5,7 @@ * @param {Object} commonOrder The group in CommonOrder schema * @returns {Object} An object with `taxableAmount` and `taxTotal` properties. Also mutates `group`. */ -export default async function setTaxesOnFulfillmentGroup(context, { group, commonOrder }) { +export default async function setTaxesOnOrderFulfillmentGroup(context, { group, commonOrder }) { const { itemTaxes, taxSummary } = await context.mutations.getFulfillmentGroupTaxes(context, { order: commonOrder, forceZeroes: true }); group.items = group.items.map((item) => { const itemTax = itemTaxes.find((entry) => entry.itemId === item._id) || {}; From 37f78c5e52765ecd141439f7a2a2195ec421be72 Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Thu, 14 Mar 2019 10:44:58 -0500 Subject: [PATCH 07/11] fix: ensure latest cart returned after select fulfillment option Signed-off-by: Eric Dobbertin --- .../mutations/selectFulfillmentOptionForGroup.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/imports/plugins/core/shipping/server/no-meteor/mutations/selectFulfillmentOptionForGroup.js b/imports/plugins/core/shipping/server/no-meteor/mutations/selectFulfillmentOptionForGroup.js index 3800f4b9cde..b33dca1d350 100644 --- a/imports/plugins/core/shipping/server/no-meteor/mutations/selectFulfillmentOptionForGroup.js +++ b/imports/plugins/core/shipping/server/no-meteor/mutations/selectFulfillmentOptionForGroup.js @@ -50,17 +50,12 @@ export default async function selectFulfillmentOptionForGroup(context, input) { }); if (matchedCount !== 1) throw new ReactionError("server-error", "Unable to update cart"); - // We can do the same update locally to avoid a db find - cart.shipping.forEach((group) => { - if (group._id === fulfillmentGroupId) { - group.shipmentMethod = option.method; - } - }); - await appEvents.emit("afterCartUpdate", { cart, updatedBy: userId }); - return { cart }; + const updatedCart = await Cart.findOne({ _id: cartId }); + + return { cart: updatedCart }; } From ca5f5098d5e352e332182d9e5c6f289c09e59b6f Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Thu, 14 Mar 2019 12:00:48 -0500 Subject: [PATCH 08/11] test: load tax schema extensions to fix integration test Signed-off-by: Eric Dobbertin --- imports/node-app/devserver/extendSchemas.js | 1 + imports/node-app/devserver/index.js | 1 + tests/TestApp.js | 1 + 3 files changed, 3 insertions(+) create mode 100644 imports/node-app/devserver/extendSchemas.js diff --git a/imports/node-app/devserver/extendSchemas.js b/imports/node-app/devserver/extendSchemas.js new file mode 100644 index 00000000000..5cb9784f846 --- /dev/null +++ b/imports/node-app/devserver/extendSchemas.js @@ -0,0 +1 @@ +import "/imports/plugins/core/taxes/lib/extendCoreSchemas"; diff --git a/imports/node-app/devserver/index.js b/imports/node-app/devserver/index.js index 6c72c894c35..4fbbc07ba4f 100644 --- a/imports/node-app/devserver/index.js +++ b/imports/node-app/devserver/index.js @@ -6,6 +6,7 @@ import queries from "./queries"; import resolvers from "./resolvers"; import schemas from "./schemas"; import filesStartup from "./filesStartup"; +import "./extendSchemas"; const { MONGO_URL, PORT = 3030, ROOT_URL } = process.env; if (!MONGO_URL) throw new Error("You must set MONGO_URL"); diff --git a/tests/TestApp.js b/tests/TestApp.js index bfe4517b760..2f998f8b0ca 100644 --- a/tests/TestApp.js +++ b/tests/TestApp.js @@ -14,6 +14,7 @@ import mutations from "../imports/node-app/devserver/mutations"; import queries from "../imports/node-app/devserver/queries"; import schemas from "../imports/node-app/devserver/schemas"; import resolvers from "../imports/node-app/devserver/resolvers"; +import "../imports/node-app/devserver/extendSchemas"; class TestApp { constructor(options = {}) { From 45bff61335f935421252e14f4549743184ae08da Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Thu, 14 Mar 2019 12:33:59 -0500 Subject: [PATCH 09/11] test: fix random int test failures Signed-off-by: Eric Dobbertin --- tests/order/moveOrderItems.test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/order/moveOrderItems.test.js b/tests/order/moveOrderItems.test.js index ed410595f32..a93d0d1894c 100644 --- a/tests/order/moveOrderItems.test.js +++ b/tests/order/moveOrderItems.test.js @@ -69,6 +69,10 @@ test("user who placed an order can move an order item", async () => { await testApp.setLoggedInUser({ _id: accountInternalId }); const group1Items = Factory.OrderItem.makeMany(3, { + price: { + amount: 4.99, + currencyCode: "USD" + }, productId: catalogItem.product.productId, variantId: catalogItem.product.variants[0].variantId, workflow: { @@ -78,6 +82,10 @@ test("user who placed an order can move an order item", async () => { }); const group2Items = Factory.OrderItem.makeMany(2, { + price: { + amount: 4.99, + currencyCode: "USD" + }, productId: catalogItem.product.productId, variantId: catalogItem.product.variants[0].variantId, workflow: { @@ -87,6 +95,11 @@ test("user who placed an order can move an order item", async () => { }); const group1 = Factory.OrderFulfillmentGroup.makeOne({ + invoice: Factory.Invoice.makeOne({ + currencyCode: "USD", + // Need to ensure 0 discount to avoid creating negative totals + discounts: 0 + }), items: group1Items, shipmentMethod: { ...mockShipmentMethod, @@ -96,6 +109,11 @@ test("user who placed an order can move an order item", async () => { }); const group2 = Factory.OrderFulfillmentGroup.makeOne({ + invoice: Factory.Invoice.makeOne({ + currencyCode: "USD", + // Need to ensure 0 discount to avoid creating negative totals + discounts: 0 + }), items: group2Items, shipmentMethod: { ...mockShipmentMethod, From 80f24f611090659509a3de5727bc23f36b98a419 Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Thu, 14 Mar 2019 15:10:33 -0500 Subject: [PATCH 10/11] test: fix unit test expectation Signed-off-by: Eric Dobbertin --- .../selectFulfillmentOptionForGroup.test.js | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/imports/plugins/core/shipping/server/no-meteor/mutations/selectFulfillmentOptionForGroup.test.js b/imports/plugins/core/shipping/server/no-meteor/mutations/selectFulfillmentOptionForGroup.test.js index fda13cca752..95b8f1a7a08 100644 --- a/imports/plugins/core/shipping/server/no-meteor/mutations/selectFulfillmentOptionForGroup.test.js +++ b/imports/plugins/core/shipping/server/no-meteor/mutations/selectFulfillmentOptionForGroup.test.js @@ -27,25 +27,7 @@ test("selects an existing shipping method", async () => { fulfillmentGroupId: "group1", fulfillmentMethodId: "valid-method" }); - expect(result).toEqual({ - cart: { - _id: "cartId", - shipping: [{ - _id: "group1", - itemIds: ["123"], - shipmentQuotes: [{ - rate: 0, - method: { - _id: "valid-method" - } - }], - shipmentMethod: { - _id: "valid-method" - }, - type: "shipping" - }] - } - }); + expect(result).toEqual({ cart: fakeCart }); expect(mockContext.collections.Cart.updateOne).toHaveBeenCalledWith({ "_id": "cartId", From 0e8227a50df11cc0e8f3b565f5906c1e80cba157 Mon Sep 17 00:00:00 2001 From: Eric Dobbertin Date: Thu, 14 Mar 2019 15:59:38 -0500 Subject: [PATCH 11/11] test: fix random test failures Signed-off-by: Eric Dobbertin --- tests/order/addOrderFulfillmentGroup.test.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/order/addOrderFulfillmentGroup.test.js b/tests/order/addOrderFulfillmentGroup.test.js index 54edf3491cf..b64763bedd7 100644 --- a/tests/order/addOrderFulfillmentGroup.test.js +++ b/tests/order/addOrderFulfillmentGroup.test.js @@ -25,6 +25,13 @@ const mockShipmentMethod = { rate: 3.99 }; +const mockInvoice = Factory.Invoice.makeOne({ + currencyCode: "USD", + // Need to ensure 0 discount to avoid creating negative totals + discounts: 0 +}); +delete mockInvoice._id; // bug in Factory pkg + beforeAll(async () => { const getFulfillmentMethodsWithQuotes = (context, commonOrderExtended, [rates]) => { rates.push({ @@ -116,6 +123,7 @@ test("user with orders role can add an order fulfillment group with new items", }); const group = Factory.OrderFulfillmentGroup.makeOne({ + invoice: mockInvoice, items: [orderItem], shopId }); @@ -238,6 +246,7 @@ test("user with orders role can add an order fulfillment group with moved items" }); const group = Factory.OrderFulfillmentGroup.makeOne({ + invoice: mockInvoice, items: [orderItemToStay, orderItemToMove], itemIds: [orderItemToStay._id, orderItemToMove._id], shipmentMethod: {