From 425ff5d9063d699d05a3f135d8d2aa30e0e3eb10 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Fri, 8 Sep 2017 15:15:01 +0800 Subject: [PATCH 01/19] Capture just the records for a particular shop --- server/methods/core/orders.js | 114 +++++++++++++++++----------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/server/methods/core/orders.js b/server/methods/core/orders.js index 486acb97b3b..11c554483c4 100644 --- a/server/methods/core/orders.js +++ b/server/methods/core/orders.js @@ -805,8 +805,6 @@ export const methods = { * orders/capturePayments * @summary Finalize any payment where mode is "authorize" * and status is "approved", reprocess as "capture" - * @todo: add tests working with new payment methods - * @todo: refactor to use non Meteor.namespace * @param {String} orderId - add tracking to orderId * @return {null} no return value */ @@ -814,13 +812,14 @@ export const methods = { check(orderId, String); // REVIEW: For marketplace implmentations who should be able to capture payments? - // Probably just the marketplace and not shops/vendors? if (!Reaction.hasPermission("orders")) { throw new Meteor.Error("access-denied", "Access Denied"); } - + const shopId = Reaction.getShopId(); // the shopId of the current user, e.g. merchant const order = Orders.findOne(orderId); - const itemIds = order.shipping[0].items.map((item) => { + // find the appropriate shipping record by shop + const shippingRecord = order.shipping.find((sRecord) => sRecord.shopId === shopId); + const itemIds = shippingRecord.items.map((item) => { return item._id; }); @@ -831,61 +830,62 @@ export const methods = { } // process order..payment.paymentMethod - _.each(order.billing, function (billing) { - const paymentMethod = billing.paymentMethod; - const transactionId = paymentMethod.transactionId; - - if (paymentMethod.mode === "capture" && paymentMethod.status === "approved" && paymentMethod.processor) { - // Grab the amount from the shipment, otherwise use the original amount - const processor = paymentMethod.processor.toLowerCase(); - - Meteor.call(`${processor}/payment/capture`, paymentMethod, (error, result) => { - if (result && result.saved === true) { - const metadata = Object.assign(billing.paymentMethod.metadata || {}, result.metadata || {}); - - Orders.update({ - "_id": orderId, - "billing.paymentMethod.transactionId": transactionId - }, { - $set: { - "billing.$.paymentMethod.mode": "capture", - "billing.$.paymentMethod.status": "completed", - "billing.$.paymentMethod.metadata": metadata - }, - $push: { - "billing.$.paymentMethod.transactions": result - } - }); - - // event onOrderPaymentCaptured used for confirmation hooks - // ie: confirmShippingMethodForOrder is triggered here - Hooks.Events.run("onOrderPaymentCaptured", orderId); - } else { - if (result && result.error) { - Logger.fatal("Failed to capture transaction.", order, paymentMethod.transactionId, result.error); - } else { - Logger.fatal("Failed to capture transaction.", order, paymentMethod.transactionId, error); + // find the billing record based on shopId + const bilingRecord = order.billing.find((bRecord) => bRecord.shopId === shopId); + + const paymentMethod = bilingRecord.paymentMethod; + const transactionId = paymentMethod.transactionId; + + if (paymentMethod.mode === "capture" && paymentMethod.status === "approved" && paymentMethod.processor) { + // Grab the amount from the shipment, otherwise use the original amount + const processor = paymentMethod.processor.toLowerCase(); + + Meteor.call(`${processor}/payment/capture`, paymentMethod, (error, result) => { + if (result && result.saved === true) { + const metadata = Object.assign(bilingRecord.paymentMethod.metadata || {}, result.metadata || {}); + + Orders.update({ + "_id": orderId, + "billing.paymentMethod.transactionId": transactionId + }, { + $set: { + "billing.$.paymentMethod.mode": "capture", + "billing.$.paymentMethod.status": "completed", + "billing.$.paymentMethod.metadata": metadata + }, + $push: { + "billing.$.paymentMethod.transactions": result } + }); - Orders.update({ - "_id": orderId, - "billing.paymentMethod.transactionId": transactionId - }, { - $set: { - "billing.$.paymentMethod.mode": "capture", - "billing.$.paymentMethod.status": "error" - }, - $push: { - "billing.$.paymentMethod.transactions": result - } - }); - - return { error: "orders/capturePayments: Failed to capture transaction" }; + // event onOrderPaymentCaptured used for confirmation hooks + // ie: confirmShippingMethodForOrder is triggered here + Hooks.Events.run("onOrderPaymentCaptured", orderId); + } else { + if (result && result.error) { + Logger.fatal("Failed to capture transaction.", order, paymentMethod.transactionId, result.error); + } else { + Logger.fatal("Failed to capture transaction.", order, paymentMethod.transactionId, error); } - return { error, result }; - }); - } - }); + + Orders.update({ + "_id": orderId, + "billing.paymentMethod.transactionId": transactionId + }, { + $set: { + "billing.$.paymentMethod.mode": "capture", + "billing.$.paymentMethod.status": "error" + }, + $push: { + "billing.$.paymentMethod.transactions": result + } + }); + + return { error: "orders/capturePayments: Failed to capture transaction" }; + } + return { error, result }; + }); + } }, /** From 2da0fb44dab08e8efe6acb3e42123cc3c6ea1bb5 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Fri, 8 Sep 2017 15:15:35 +0800 Subject: [PATCH 02/19] if refund results are empty do nothing --- .../payments-stripe/server/methods/stripe.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/imports/plugins/included/payments-stripe/server/methods/stripe.js b/imports/plugins/included/payments-stripe/server/methods/stripe.js index 15f8850d826..db0376d1428 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripe.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripe.js @@ -425,14 +425,16 @@ export const methods = { try { const refunds = StripeApi.methods.listRefunds.call({ transactionId: paymentMethod.transactionId }); result = []; - for (const refund of refunds.data) { - result.push({ - type: refund.object, - amount: refund.amount / 100, - created: refund.created * 1000, - currency: refund.currency, - raw: refund - }); + if (refunds) { + for (const refund of refunds.data) { + result.push({ + type: refund.object, + amount: refund.amount / 100, + created: refund.created * 1000, + currency: refund.currency, + raw: refund + }); + } } } catch (error) { Logger.error(error); From 55291036ea86dd785f1ebcac5bc9506295310f1d Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Fri, 8 Sep 2017 15:18:43 +0800 Subject: [PATCH 03/19] Catch and log errors with refunds --- .../payments-stripe/server/methods/stripeapi.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/imports/plugins/included/payments-stripe/server/methods/stripeapi.js b/imports/plugins/included/payments-stripe/server/methods/stripeapi.js index 61c7c2995ab..babc2f09456 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripeapi.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripeapi.js @@ -153,8 +153,12 @@ StripeApi.methods.listRefunds = new ValidatedMethod({ } else { stripe = require("stripe")(apiKey); } - const refundListPromise = stripe.refunds.list({ charge: transactionId }); - const refundListResults = Promise.await(refundListPromise); - return refundListResults; + try { + const refundListPromise = stripe.refunds.list({ charge: transactionId }); + const refundListResults = Promise.await(refundListPromise); + return refundListResults; + } catch (error) { + Logger.error("Encountered an error when trying to list refunds", error); + } } }); From fbca2c577b1183dc5a43c6e23fef6df0f859e99c Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Mon, 11 Sep 2017 10:10:08 +0800 Subject: [PATCH 04/19] Return all orders where the users shopId is attached to an item --- server/publications/collections/orders.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/server/publications/collections/orders.js b/server/publications/collections/orders.js index 818545cb433..e9373eb3a37 100644 --- a/server/publications/collections/orders.js +++ b/server/publications/collections/orders.js @@ -63,9 +63,15 @@ Meteor.publish("CustomPaginatedOrders", function (query, options) { if (!shopId) { return this.ready(); } + + // return any order for which the shopId is attached to an item + const selector = { + "items.shopId": shopId + }; + if (Roles.userIsInRole(this.userId, ["admin", "owner", "orders"], shopId)) { - Counts.publish(this, "order-count", Orders.find({ shopId: shopId }), { noReady: true }); - return Orders.find({ shopId: shopId }); + Counts.publish(this, "order-count", Orders.find(selector), { noReady: true }); + return Orders.find(selector); } return Orders.find({ shopId: shopId, From f9d5b2bfd55ad85de306941c15ec7232fedb0a59 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Mon, 11 Sep 2017 10:31:52 +0800 Subject: [PATCH 05/19] Return all orders where the users shopId is attached to an item --- server/methods/core/orders.js | 45 ++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/server/methods/core/orders.js b/server/methods/core/orders.js index 11c554483c4..8b670518a9f 100644 --- a/server/methods/core/orders.js +++ b/server/methods/core/orders.js @@ -56,6 +56,43 @@ export function ordersInventoryAdjust(orderId) { }); } +// TODO: Marketplace: Is there a reason to do this any other way? Can admins reduce for more +// than one shop +/** + * ordersInventoryAdjustByShop + * adjust inventory for a particular shop when an order is approved + * @param {String} orderId - orderId + * @param {String} shopId - the id of the shop approving the order + * @return {null} no return value + */ +export function ordersInventoryAdjustByShop(orderId, shopId) { + check(orderId, String); + check(shopId, String); + + if (!Reaction.hasPermission("orders")) { + throw new Meteor.Error("access-denied", "Access Denied"); + } + + const order = Orders.findOne(orderId); + order.items.forEach(item => { + if (item.shopId === shopId) { + Products.update({ + _id: item.variants._id + }, { + $inc: { + inventoryQuantity: -item.quantity + } + }, { + publish: true, + selector: { + type: "variant" + } + }); + } + }); +} + + export function orderQuantityAdjust(orderId, refundedItem) { check(orderId, String); @@ -233,6 +270,7 @@ export const methods = { "orders/approvePayment": function (order) { check(order, Object); const invoice = orderCreditMethod(order).invoice; + const shopId = Reaction.getShopId(); // the shop of the user who is currently logged on // REVIEW: Who should have access to do this for a marketplace? // Do we have/need a shopId on each order? @@ -252,11 +290,12 @@ export const methods = { const total = accounting.toFixed(Number(discountTotal) + Number(shipping) + Number(taxes), 2); // Updates flattened inventory count on variants in Products collection - ordersInventoryAdjust(order._id); + ordersInventoryAdjustByShop(order._id, shopId); - return Orders.update({ + Orders.update({ "_id": order._id, - "billing.paymentMethod.method": "credit" + "billing.paymentMethod.method": "credit", + "billing.shopId": shopId }, { $set: { "billing.$.paymentMethod.amount": total, From cb81e27f53e0aa87e80ac84293dc3b656c000a66 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Mon, 11 Sep 2017 13:02:58 +0800 Subject: [PATCH 06/19] Remove validated-method mess I created. Use plain functions --- .../payments-stripe/server/methods/stripe.js | 6 +- .../server/methods/stripeapi.js | 178 +++++++----------- 2 files changed, 71 insertions(+), 113 deletions(-) diff --git a/imports/plugins/included/payments-stripe/server/methods/stripe.js b/imports/plugins/included/payments-stripe/server/methods/stripe.js index db0376d1428..aa76b50c845 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripe.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripe.js @@ -35,7 +35,7 @@ function stripeCaptureCharge(paymentMethod) { }; try { - const captureResult = StripeApi.methods.captureCharge.call({ + const captureResult = StripeApi.methods.captureCharge({ transactionId: paymentMethod.transactionId, captureDetails: captureDetails }); @@ -388,7 +388,7 @@ export const methods = { let result; try { - const refundResult = StripeApi.methods.createRefund.call({ refundDetails }); + const refundResult = StripeApi.methods.createRefund({ refundDetails }); Logger.debug(refundResult); if (refundResult && refundResult.object === "refund") { result = { @@ -423,7 +423,7 @@ export const methods = { check(paymentMethod, Reaction.Schemas.PaymentMethod); let result; try { - const refunds = StripeApi.methods.listRefunds.call({ transactionId: paymentMethod.transactionId }); + const refunds = StripeApi.methods.listRefunds({ transactionId: paymentMethod.transactionId }); result = []; if (refunds) { for (const refund of refunds.data) { diff --git a/imports/plugins/included/payments-stripe/server/methods/stripeapi.js b/imports/plugins/included/payments-stripe/server/methods/stripeapi.js index babc2f09456..bb974a43001 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripeapi.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripeapi.js @@ -1,6 +1,6 @@ /* eslint camelcase: 0 */ import _ from "lodash"; -import { ValidatedMethod } from "meteor/mdg:validated-method"; +import { check } from "meteor/check"; import { Meteor } from "meteor/meteor"; import { SimpleSchema } from "meteor/aldeed:simple-schema"; import { Reaction, Logger } from "/server/api"; @@ -43,122 +43,80 @@ const expectedErrors = [ "incorrect_number" ]; -StripeApi.methods.getApiKey = new ValidatedMethod({ - name: "StripeApi.methods.getApiKey", - validate: null, - run() { - const stripePkg = Reaction.getPackageSettingsWithOptions({ - shopId: Reaction.getPrimaryShopId(), - name: "reaction-stripe" - }); - if (stripePkg || stripePkg.settings && stripePkg.settings.api_key) { - return stripePkg.settings.api_key; - } - throw new Meteor.Error("access-denied", "Invalid Stripe Credentials"); +StripeApi.methods.getApiKey = function () { + const stripePkg = Reaction.getPackageSettingsWithOptions({ + shopId: Reaction.getPrimaryShopId(), + name: "reaction-stripe" + }); + if (stripePkg || stripePkg.settings && stripePkg.settings.api_key) { + return stripePkg.settings.api_key; } -}); + throw new Meteor.Error("invalid-credentials", "Invalid Stripe Credentials"); +}; -StripeApi.methods.createCharge = new ValidatedMethod({ - name: "StripeApi.methods.createCharge", - validate: new SimpleSchema({ - chargeObj: { type: chargeObjectSchema }, - apiKey: { type: String, optional: true } - }).validator(), - run({ chargeObj, apiKey }) { - let stripe; - if (!apiKey) { - const dynamicApiKey = StripeApi.methods.getApiKey.call(); - stripe = require("stripe")(dynamicApiKey); - } else { - stripe = require("stripe")(apiKey); - } - try { - const chargePromise = stripe.charges.create(chargeObj); - const promiseResult = Promise.await(chargePromise); - return promiseResult; - } catch (e) { - // Handle "expected" errors differently - if (e.rawType === "card_error" && _.includes(expectedErrors, e.code)) { - Logger.debug("Error from Stripe is expected, not throwing"); - const normalizedError = { - details: e.message - }; - return { error: normalizedError, result: null }; - } - Logger.error("Received unexpected error type: " + e.rawType); - Logger.error(e); - - // send raw error to server log, but sanitized version to client - const sanitisedError = { - details: "An unexpected error has occurred" +StripeApi.methods.createCharge = function(chargeObj, apiKey) { + check(chargeObj, chargeObjectSchema); + + const stripeKey = apiKey || StripeApi.methods.getApiKey(); + const stripe = require("stripe")(stripeKey); + try { + const chargePromise = stripe.charges.create(chargeObj); + const promiseResult = Promise.await(chargePromise); + return promiseResult; + } catch (e) { + // Handle "expected" errors differently + if (e.rawType === "card_error" && _.includes(expectedErrors, e.code)) { + Logger.debug("Error from Stripe is expected, not throwing"); + const normalizedError = { + details: e.message }; - return { error: sanitisedError, result: null }; + return { error: normalizedError, result: null }; } - } -}); + Logger.error("Received unexpected error type: " + e.rawType); + Logger.error(e); -StripeApi.methods.captureCharge = new ValidatedMethod({ - name: "StripeApi.methods.captureCharge", - validate: new SimpleSchema({ - transactionId: { type: String }, - captureDetails: { type: captureDetailsSchema }, - apiKey: { type: String, optional: true } - }).validator(), - run({ transactionId, captureDetails, apiKey }) { - let stripe; - if (!apiKey) { - const dynamicApiKey = StripeApi.methods.getApiKey.call(); - stripe = require("stripe")(dynamicApiKey); - } else { - stripe = require("stripe")(apiKey); - } - const capturePromise = stripe.charges.capture(transactionId, captureDetails); - const captureResults = Promise.await(capturePromise); - return captureResults; + // send raw error to server log, but sanitized version to client + const sanitisedError = { + details: "An unexpected error has occurred" + }; + return { error: sanitisedError, result: null }; } -}); +}; -StripeApi.methods.createRefund = new ValidatedMethod({ - name: "StripeApi.methods.createRefund", - validate: new SimpleSchema({ - refundDetails: { type: refundDetailsSchema }, - apiKey: { type: String, optional: true } - }).validator(), - run({ refundDetails, apiKey }) { - let stripe; - if (!apiKey) { - const dynamicApiKey = StripeApi.methods.getApiKey.call(); - stripe = require("stripe")(dynamicApiKey); - } else { - stripe = require("stripe")(apiKey); - } - const refundPromise = stripe.refunds.create({ charge: refundDetails.charge, amount: refundDetails.amount }); - const refundResults = Promise.await(refundPromise); - return refundResults; - } -}); -StripeApi.methods.listRefunds = new ValidatedMethod({ - name: "StripeApi.methods.listRefunds", - validate: new SimpleSchema({ - transactionId: { type: String }, - apiKey: { type: String, optional: true } - }).validator(), - run({ transactionId, apiKey }) { - let stripe; - if (!apiKey) { - const dynamicApiKey = StripeApi.methods.getApiKey.call(); - stripe = require("stripe")(dynamicApiKey); - } else { - stripe = require("stripe")(apiKey); - } - try { - const refundListPromise = stripe.refunds.list({ charge: transactionId }); - const refundListResults = Promise.await(refundListPromise); - return refundListResults; - } catch (error) { - Logger.error("Encountered an error when trying to list refunds", error); - } +StripeApi.methods.captureCharge = function ({ transactionId, captureDetails, apiKey }) { + check(transactionId, String); + check(captureDetails, captureDetailsSchema); + + const stripeKey = apiKey || StripeApi.methods.getApiKey(); + const stripe = require("stripe")(stripeKey); + const capturePromise = stripe.charges.capture(transactionId, captureDetails); + const captureResults = Promise.await(capturePromise); + return captureResults; +}; + +StripeApi.methods.createRefund = function ({ refundDetails, apiKey }) { + check(refundDetails, refundDetailsSchema); + + const stripeKey = apiKey || StripeApi.methods.getApiKey(); + const stripe = require("stripe")(stripeKey); + const refundPromise = stripe.refunds.create({ charge: refundDetails.charge, amount: refundDetails.amount }); + const refundResults = Promise.await(refundPromise); + return refundResults; +}; + +StripeApi.methods.listRefunds = function ({ transactionId, apiKey }) { + check(transactionId, String); + + const stripeKey = apiKey || StripeApi.methods.getApiKey(); + const stripe = require("stripe")(stripeKey); + try { + const refundListPromise = stripe.refunds.list({ charge: transactionId }); + const refundListResults = Promise.await(refundListPromise); + return refundListResults; + } catch (error) { + // Logger.error("Encountered an error when trying to list refunds", error); + Logger.error("Encountered an error when trying to list refunds"); } -}); +}; From c05eec49790512c5a6c2ac255af8becdf72852c8 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Mon, 11 Sep 2017 15:40:57 +0800 Subject: [PATCH 07/19] Fix stripe tests --- .../server/methods/stripeapi-methods-capture.app-test.js | 4 ++-- .../server/methods/stripeapi-methods-refund.app-test.js | 4 ++-- .../server/methods/stripeapi-methods-refundlist.app-test.js | 4 ++-- .../included/payments-stripe/server/methods/stripeapi.js | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-capture.app-test.js b/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-capture.app-test.js index 9526ea1f054..d84d2cfc515 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-capture.app-test.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-capture.app-test.js @@ -90,7 +90,7 @@ describe("stripe/payment/capture", function () { mode: "capture", createdAt: new Date() }; - sandbox.stub(StripeApi.methods.captureCharge, "call", function () { + sandbox.stub(StripeApi.methods, "captureCharge", function () { return stripeCaptureResult; }); // spyOn(StripeApi.methods.captureCharge, "call").and.returnValue(stripeCaptureResult); @@ -103,7 +103,7 @@ describe("stripe/payment/capture", function () { expect(captureError).to.be.undefined; expect(captureResult).to.not.be.undefined; expect(captureResult.saved).to.be.true; - expect(StripeApi.methods.captureCharge.call).to.have.been.calledWith({ + expect(StripeApi.methods.captureCharge).to.have.been.calledWith({ transactionId: paymentMethod.transactionId, captureDetails: { amount: 1999 diff --git a/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-refund.app-test.js b/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-refund.app-test.js index 9556e17dc1a..ce8cc2ef9b2 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-refund.app-test.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-refund.app-test.js @@ -44,7 +44,7 @@ describe("stripe/refund/create", function () { receipt_number: null }; - sandbox.stub(StripeApi.methods.createRefund, "call", function () { + sandbox.stub(StripeApi.methods, "createRefund", function () { return stripeRefundResult; }); // spyOn(StripeApi.methods.createRefund, "call").and.returnValue(stripeRefundResult); @@ -57,7 +57,7 @@ describe("stripe/refund/create", function () { expect(refundError).to.be.undefined; expect(refundResult).to.not.be.undefined; expect(refundResult.saved).to.be.true; - expect(StripeApi.methods.createRefund.call).to.have.been.calledWith({ + expect(StripeApi.methods.createRefund).to.have.been.calledWith({ refundDetails: { charge: paymentMethod.transactionId, amount: 1999, diff --git a/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-refundlist.app-test.js b/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-refundlist.app-test.js index 47e3de3f1df..7aa6b91558c 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-refundlist.app-test.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-refundlist.app-test.js @@ -53,7 +53,7 @@ describe("stripe/refunds/list", function () { has_more: false, url: "/v1/refunds" }; - sandbox.stub(StripeApi.methods.listRefunds, "call", function () { + sandbox.stub(StripeApi.methods, "listRefunds", function () { return stripeRefundListResult; }); @@ -69,7 +69,7 @@ describe("stripe/refunds/list", function () { expect(refundListResult[0].amount).to.equal(19.99); expect(refundListResult[0].currency).to.equal("usd"); - expect(StripeApi.methods.listRefunds.call).to.have.been.calledWith({ + expect(StripeApi.methods.listRefunds).to.have.been.calledWith({ transactionId: paymentMethod.transactionId }); done(); diff --git a/imports/plugins/included/payments-stripe/server/methods/stripeapi.js b/imports/plugins/included/payments-stripe/server/methods/stripeapi.js index bb974a43001..ea6dfef45c4 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripeapi.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripeapi.js @@ -55,7 +55,7 @@ StripeApi.methods.getApiKey = function () { }; -StripeApi.methods.createCharge = function(chargeObj, apiKey) { +StripeApi.methods.createCharge = function({ chargeObj, apiKey }) { check(chargeObj, chargeObjectSchema); const stripeKey = apiKey || StripeApi.methods.getApiKey(); From 3743aa78ef48af4a4bd74dd7bdf6794de5d941c6 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Mon, 11 Sep 2017 15:49:50 +0800 Subject: [PATCH 08/19] Fix fixtures --- server/imports/fixtures/orders.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/imports/fixtures/orders.js b/server/imports/fixtures/orders.js index c6cda497776..1cc1eea8d12 100755 --- a/server/imports/fixtures/orders.js +++ b/server/imports/fixtures/orders.js @@ -129,18 +129,20 @@ export default function () { }, requiresShipping: true, shipping: [{ + shopId: getShopId(), + _id: Random.id(), items: [ { _id: itemIdOne, productId: Random.id(), - shopId: Random.id(), + shopId: getShopId(), variantId: Random.id(), packed: false }, { _id: itemIdTwo, productId: Random.id(), - shopId: Random.id(), + shopId: getShopId(), variantId: Random.id(), packed: false } @@ -148,6 +150,7 @@ export default function () { }], // Shipping Schema billing: [{ _id: Random.id(), + shopId: getShopId(), address: getAddress({ isBillingDefault: true }), paymentMethod: paymentMethod({ method: "credit", From c9436bd2fa6792ce902720c7d95ffb12a7c93e20 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Mon, 11 Sep 2017 17:25:33 +0800 Subject: [PATCH 09/19] Correctly store paymentPackageId and then use that to get right key --- .../payments-stripe/server/methods/stripe.js | 16 +++++++++++----- .../payments-stripe/server/methods/stripeapi.js | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/imports/plugins/included/payments-stripe/server/methods/stripe.js b/imports/plugins/included/payments-stripe/server/methods/stripe.js index aa76b50c845..a73ef989e66 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripe.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripe.js @@ -34,10 +34,14 @@ function stripeCaptureCharge(paymentMethod) { amount: formatForStripe(paymentMethod.amount) }; + const stripePackage = Packages.findOne(paymentMethod.paymentPackageId); + const stripeKey = stripePackage.settings.api_key || stripePackage.settings.connectAuth.access_token; + try { const captureResult = StripeApi.methods.captureCharge({ transactionId: paymentMethod.transactionId, - captureDetails: captureDetails + captureDetails: captureDetails, + apiKey: stripeKey }); if (captureResult.status === "succeeded") { result = { @@ -120,10 +124,7 @@ function buildPaymentMethods(options) { const shopIds = Object.keys(transactionsByShopId); const storedCard = cardData.type.charAt(0).toUpperCase() + cardData.type.slice(1) + " " + cardData.number.slice(-4); - const packageData = Packages.findOne({ - name: "reaction-stripe", - shopId: Reaction.getPrimaryShopId() - }); + const paymentMethods = []; @@ -140,6 +141,11 @@ function buildPaymentMethods(options) { }; }); + const packageData = Packages.findOne({ + name: "reaction-stripe", + shopId: shopId + }); + const paymentMethod = { processor: "Stripe", storedCard: storedCard, diff --git a/imports/plugins/included/payments-stripe/server/methods/stripeapi.js b/imports/plugins/included/payments-stripe/server/methods/stripeapi.js index ea6dfef45c4..04465176fc0 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripeapi.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripeapi.js @@ -55,7 +55,7 @@ StripeApi.methods.getApiKey = function () { }; -StripeApi.methods.createCharge = function({ chargeObj, apiKey }) { +StripeApi.methods.createCharge = function ({ chargeObj, apiKey }) { check(chargeObj, chargeObjectSchema); const stripeKey = apiKey || StripeApi.methods.getApiKey(); From 8e0ad77342118a14c550869e8208446b4c92af91 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Mon, 11 Sep 2017 18:13:04 +0800 Subject: [PATCH 10/19] Approve by shopId --- server/methods/core/orders.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/server/methods/core/orders.js b/server/methods/core/orders.js index 8b670518a9f..450bd0e890d 100644 --- a/server/methods/core/orders.js +++ b/server/methods/core/orders.js @@ -271,7 +271,6 @@ export const methods = { check(order, Object); const invoice = orderCreditMethod(order).invoice; const shopId = Reaction.getShopId(); // the shop of the user who is currently logged on - // REVIEW: Who should have access to do this for a marketplace? // Do we have/need a shopId on each order? if (!Reaction.hasPermission("orders")) { @@ -292,17 +291,19 @@ export const methods = { // Updates flattened inventory count on variants in Products collection ordersInventoryAdjustByShop(order._id, shopId); + const billing = order.billing; + const billingRecord = billing.find((bRecord) => bRecord.shopId === shopId); + billingRecord.paymentMethod.amount = total; + billingRecord.paymentMethod.status = "approved"; + billingRecord.paymentMethod.mode = "capture"; + billingRecord.invoice.discounts = discount; + billingRecord.invoice.total = Number(total); + Orders.update({ - "_id": order._id, - "billing.paymentMethod.method": "credit", - "billing.shopId": shopId + _id: order._id }, { $set: { - "billing.$.paymentMethod.amount": total, - "billing.$.paymentMethod.status": "approved", - "billing.$.paymentMethod.mode": "capture", - "billing.$.invoice.discounts": discount, - "billing.$.invoice.total": Number(total) + billing: billing, } }); }, From 980035196d9c516609ad5c28c3f7d7928db32ccd Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Mon, 11 Sep 2017 18:40:17 +0800 Subject: [PATCH 11/19] Don't rewrite values when approving order --- server/methods/core/orders.js | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/server/methods/core/orders.js b/server/methods/core/orders.js index 450bd0e890d..f61c188aba5 100644 --- a/server/methods/core/orders.js +++ b/server/methods/core/orders.js @@ -269,41 +269,24 @@ export const methods = { */ "orders/approvePayment": function (order) { check(order, Object); - const invoice = orderCreditMethod(order).invoice; const shopId = Reaction.getShopId(); // the shop of the user who is currently logged on - // REVIEW: Who should have access to do this for a marketplace? - // Do we have/need a shopId on each order? if (!Reaction.hasPermission("orders")) { throw new Meteor.Error("access-denied", "Access Denied"); } - this.unblock(); // REVIEW: why unblock here? - - // this is server side check to verify - // that the math all still adds up. - const subTotal = invoice.subtotal; - const shipping = invoice.shipping; - const taxes = invoice.taxes; - const discount = invoice.discounts; - const discountTotal = Math.max(0, subTotal - discount); // ensure no discounting below 0. - const total = accounting.toFixed(Number(discountTotal) + Number(shipping) + Number(taxes), 2); - // Updates flattened inventory count on variants in Products collection ordersInventoryAdjustByShop(order._id, shopId); const billing = order.billing; const billingRecord = billing.find((bRecord) => bRecord.shopId === shopId); - billingRecord.paymentMethod.amount = total; billingRecord.paymentMethod.status = "approved"; billingRecord.paymentMethod.mode = "capture"; - billingRecord.invoice.discounts = discount; - billingRecord.invoice.total = Number(total); Orders.update({ _id: order._id }, { $set: { - billing: billing, + billing: billing } }); }, @@ -850,7 +833,6 @@ export const methods = { */ "orders/capturePayments": (orderId) => { check(orderId, String); - // REVIEW: For marketplace implmentations who should be able to capture payments? if (!Reaction.hasPermission("orders")) { throw new Meteor.Error("access-denied", "Access Denied"); From 12f6d5902b55fb88a21ca04cab94a306d2c13b53 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Tue, 12 Sep 2017 15:52:35 +0800 Subject: [PATCH 12/19] Use an aggregate function to filter orders --- server/publications/collections/orders.js | 43 +++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/server/publications/collections/orders.js b/server/publications/collections/orders.js index e9373eb3a37..bdb18daa684 100644 --- a/server/publications/collections/orders.js +++ b/server/publications/collections/orders.js @@ -2,9 +2,11 @@ import { Meteor } from "meteor/meteor"; import { check, Match } from "meteor/check"; import { Roles } from "meteor/alanning:roles"; import { Counts } from "meteor/tmeasday:publish-counts"; +import { ReactiveAggregate } from "./reactiveAggregate"; import { Orders } from "/lib/collections"; import { Reaction } from "/server/api"; + /** * orders */ @@ -68,11 +70,46 @@ Meteor.publish("CustomPaginatedOrders", function (query, options) { const selector = { "items.shopId": shopId }; - if (Roles.userIsInRole(this.userId, ["admin", "owner", "orders"], shopId)) { - Counts.publish(this, "order-count", Orders.find(selector), { noReady: true }); - return Orders.find(selector); + ReactiveAggregate(this, Orders, [ + { + $project: { + items: { + $filter: { + input: "$items", + as: "item", + cond: { $eq: ["$$item.shopId", "J8Bhq3uTtdgwZx3rz"] } + } + }, + billing: { + $filter: { + input: "$billing", + as: "billing", + cond: { $eq: ["$$billing.shopId", "J8Bhq3uTtdgwZx3rz"] } + } + }, + shipping: { + $filter: { + input: "$shipping", + as: "shipping", + cond: { $eq: ["$$shipping.shopId", "J8Bhq3uTtdgwZx3rz"] } + } + }, + cartId: 1, + sessionId: 1, + shopId: 1, + workflow: 1, + discount: 1, + tax: 1, + email: 1, + createdAt: 1 + } + } + ], selector); } + + // TODO How to we return this order-count + // Counts.publish(this, "order-count", Orders.find(selector), { noReady: true }); return Orders.find({ shopId: shopId, userId: this.userId From 089c08eb10f53f1675bc9829a7a08325fef7738f Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Tue, 12 Sep 2017 15:53:02 +0800 Subject: [PATCH 13/19] Create a reactive-aggregate --- .../collections/reactiveAggregate.js | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 server/publications/collections/reactiveAggregate.js diff --git a/server/publications/collections/reactiveAggregate.js b/server/publications/collections/reactiveAggregate.js new file mode 100644 index 00000000000..1ff2496e315 --- /dev/null +++ b/server/publications/collections/reactiveAggregate.js @@ -0,0 +1,90 @@ +import _ from "lodash"; +import { Meteor } from "meteor/meteor"; +import { Mongo, MongoInternals } from "meteor/mongo"; + + +// Add the aggregate function available in tbe raw collection to normal collections +Mongo.Collection.prototype.aggregate = function (pipelines, options) { + const coll = this._getCollection(); + return Meteor.wrapAsync(coll.aggregate.bind(coll))(pipelines, options); +}; + +Mongo.Collection.prototype._getDb = function () { + if (typeof this._collection._getDb === "function") { + return this._collection._getDb(); + } + const mongoConn = MongoInternals.defaultRemoteCollectionDriver().mongo; + return wrapWithDb(mongoConn); +}; + +Mongo.Collection.prototype._getCollection = function () { + const db = this._getDb(); + return db.collection(this._name); +}; + +function wrapWithDb(mongoConn) { + if (mongoConn.db) { + return mongoConn.db; + } +} + + +export function ReactiveAggregate(sub, collection, pipeline, options) { + const defaultOptions = { + observeSelector: {}, + observeOptions: {}, + clientCollection: collection._name + }; + const subOptions = Object.assign({}, defaultOptions, options); + + let initializing = true; + sub._ids = {}; + sub._iteration = 1; + + function update() { + if (initializing) { + return; + } + + // add and update documents on the client + collection.aggregate(pipeline).forEach(function (doc) { + if (!sub._ids[doc._id]) { + sub.added(subOptions.clientCollection, doc._id, doc); + } else { + sub.changed(subOptions.clientCollection, doc._id, doc); + } + sub._ids[doc._id] = sub._iteration; + }); + // remove documents not in the result anymore + _.forEach(sub._ids, function (value, key) { + if (value !== sub._iteration) { + delete sub._ids[key]; + sub.removed(subOptions.clientCollection, key); + } + }); + sub._iteration++; + } + + // track any changes on the collection used for the aggregation + const query = collection.find(subOptions.observeSelector, subOptions.observeOptions); + const handle = query.observeChanges({ + added: update, + changed: update, + removed: update, + error: function (error) { + throw new Meteor.Error(`Encountered an error while observing ${collection._name}`, error); + } + }); + // observeChanges() will immediately fire an "added" event for each document in the query + // these are skipped using the initializing flag + initializing = false; + // send an initial result set to the client + update(); + // mark the subscription as ready + sub.ready(); + + // stop observing the cursor when the client unsubscribes + sub.onStop(function () { + handle.stop(); + }); +} From 47a747bd6a9167303b412dbf91dd2ecc7e98d37b Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Tue, 12 Sep 2017 16:32:07 +0800 Subject: [PATCH 14/19] Add JSDoc and check --- .../publications/collections/reactiveAggregate.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/server/publications/collections/reactiveAggregate.js b/server/publications/collections/reactiveAggregate.js index 1ff2496e315..58b615ade6a 100644 --- a/server/publications/collections/reactiveAggregate.js +++ b/server/publications/collections/reactiveAggregate.js @@ -1,5 +1,6 @@ import _ from "lodash"; import { Meteor } from "meteor/meteor"; +import { check, Match } from "meteor/check"; import { Mongo, MongoInternals } from "meteor/mongo"; @@ -28,8 +29,19 @@ function wrapWithDb(mongoConn) { } } - +/** + * Create a Reactive collection from a Mongo aggregate pipeline + * @param {Object} sub - The publication we are creating + * @param {Collection} collection - the collection we are operating on + * @param {Array} pipeline - The mongo aggregation pipeline to run + * @param {Object} options - an object of options + * @returns {Cursor} A mongo cursor for subscription + * @constructor + */ export function ReactiveAggregate(sub, collection, pipeline, options) { + check(pipeline, Array); + check(options, Match.Optional(Object)); + const defaultOptions = { observeSelector: {}, observeOptions: {}, From cd10fedd6bc8d90529f5eee1e3a2b920ab528704 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Tue, 12 Sep 2017 18:55:07 +0800 Subject: [PATCH 15/19] Fix Stripe tests --- .../included/payments-stripe/server/methods/stripe.js | 2 +- .../server/methods/stripeapi-methods-capture.app-test.js | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/imports/plugins/included/payments-stripe/server/methods/stripe.js b/imports/plugins/included/payments-stripe/server/methods/stripe.js index a73ef989e66..6bdec4059fe 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripe.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripe.js @@ -356,7 +356,7 @@ export const methods = { */ "stripe/payment/capture": function (paymentMethod) { check(paymentMethod, Reaction.Schemas.PaymentMethod); - // let result; + const captureDetails = { amount: formatForStripe(paymentMethod.amount) }; diff --git a/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-capture.app-test.js b/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-capture.app-test.js index d84d2cfc515..fa46ce0b4c7 100644 --- a/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-capture.app-test.js +++ b/imports/plugins/included/payments-stripe/server/methods/stripeapi-methods-capture.app-test.js @@ -2,6 +2,7 @@ import { Meteor } from "meteor/meteor"; import { expect } from "meteor/practicalmeteor:chai"; import { sinon } from "meteor/practicalmeteor:sinon"; +import { Packages } from "/lib/collections"; import { StripeApi } from "./stripeapi"; const stripeCaptureResult = { @@ -78,10 +79,12 @@ describe("stripe/payment/capture", function () { }); it("should call StripeApi.methods.captureCharge with the proper parameters and return saved = true", function (done) { + const stripePackage = Packages.findOne({ name: "reaction-stripe" }); + const apiKey = stripePackage.settings.api_key; const paymentMethod = { processor: "Stripe", storedCard: "Visa 4242", - paymentPackageId: "vrXutd72c2m7Lenqw", + paymentPackageId: stripePackage._id, paymentSettingsKey: "reaction-stripe", method: "credit", transactionId: "ch_17hZ4wBXXkbZQs3xL5JhlSgS", @@ -93,7 +96,6 @@ describe("stripe/payment/capture", function () { sandbox.stub(StripeApi.methods, "captureCharge", function () { return stripeCaptureResult; }); - // spyOn(StripeApi.methods.captureCharge, "call").and.returnValue(stripeCaptureResult); let captureResult = null; let captureError = null; @@ -107,7 +109,8 @@ describe("stripe/payment/capture", function () { transactionId: paymentMethod.transactionId, captureDetails: { amount: 1999 - } + }, + apiKey: apiKey }); done(); }); From 403a7a328f831915d0ebf778659e368e1204c65b Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Tue, 12 Sep 2017 19:50:17 +0800 Subject: [PATCH 16/19] Don't hardcode shopId --- server/publications/collections/orders.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/publications/collections/orders.js b/server/publications/collections/orders.js index bdb18daa684..af0a3c2206f 100644 --- a/server/publications/collections/orders.js +++ b/server/publications/collections/orders.js @@ -78,21 +78,21 @@ Meteor.publish("CustomPaginatedOrders", function (query, options) { $filter: { input: "$items", as: "item", - cond: { $eq: ["$$item.shopId", "J8Bhq3uTtdgwZx3rz"] } + cond: { $eq: ["$$item.shopId", shopId] } } }, billing: { $filter: { input: "$billing", as: "billing", - cond: { $eq: ["$$billing.shopId", "J8Bhq3uTtdgwZx3rz"] } + cond: { $eq: ["$$billing.shopId", shopId] } } }, shipping: { $filter: { input: "$shipping", as: "shipping", - cond: { $eq: ["$$shipping.shopId", "J8Bhq3uTtdgwZx3rz"] } + cond: { $eq: ["$$shipping.shopId", shopId] } } }, cartId: 1, From 961f94602b690ec3613a654f1e5ccf7a4a7bc47f Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Tue, 12 Sep 2017 20:45:52 +0800 Subject: [PATCH 17/19] Fix tests --- server/methods/core/orders.app-test.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/server/methods/core/orders.app-test.js b/server/methods/core/orders.app-test.js index 655f383ee99..90b19b6e7d2 100644 --- a/server/methods/core/orders.app-test.js +++ b/server/methods/core/orders.app-test.js @@ -275,22 +275,14 @@ describe("orders test", function () { it("should approve payment", function () { sandbox.stub(Reaction, "hasPermission", () => true); spyOnMethod("approvePayment", order.userId); - const invoice = orderCreditMethod(order).invoice; - const subTotal = invoice.subtotal; - const shipping = invoice.shipping; - const taxes = invoice.taxes; - const discount = invoice.discounts; - const discountTotal = Math.max(0, subTotal - discount); // ensure no discounting below 0. - const total = accounting.toFixed(discountTotal + shipping + taxes, 2); Meteor.call("orders/approvePayment", order); const orderBilling = Orders.findOne({ _id: order._id }).billing[0]; expect(orderBilling.paymentMethod.status).to.equal("approved"); expect(orderBilling.paymentMethod.mode).to.equal("capture"); - expect(orderBilling.invoice.discounts).to.equal(discount); - expect(orderBilling.invoice.total).to.equal(Number(total)); }); }); + describe("orders/shipmentShipped", function () { it("should throw an error if user does not have permission", function () { sandbox.stub(Reaction, "hasPermission", () => false); From b04bd8c31613f80e0c89b057cc22e6bded1ed7eb Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Tue, 12 Sep 2017 21:04:21 +0800 Subject: [PATCH 18/19] pass in orderCount to orderTable --- .../plugins/core/orders/client/components/orderDashboard.js | 3 +++ imports/plugins/core/orders/client/components/orderTable.js | 3 ++- .../orders/client/containers/orderDashboardContainer.js | 6 ++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/imports/plugins/core/orders/client/components/orderDashboard.js b/imports/plugins/core/orders/client/components/orderDashboard.js index cf8ad4ac28f..eea0fd339d6 100644 --- a/imports/plugins/core/orders/client/components/orderDashboard.js +++ b/imports/plugins/core/orders/client/components/orderDashboard.js @@ -16,6 +16,7 @@ class OrderDashboard extends Component { handleSelect: PropTypes.func, isLoading: PropTypes.object, multipleSelect: PropTypes.bool, + orderCount: PropTypes.number, orders: PropTypes.array, query: PropTypes.object, renderFlowList: PropTypes.bool, @@ -58,6 +59,7 @@ class OrderDashboard extends Component { } render() { + console.log("props", this.props); return (
{ if (mediaSubscription.ready() && ordersSubscription.ready()) { const orders = Orders.find().fetch(); - + const orderCount = Orders.find().count(); onData(null, { - orders + orders, + orderCount }); } }; From 3c1fdb4e18d3d7cded2bee67be6db1c1ef58f8b3 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Tue, 12 Sep 2017 21:33:14 +0800 Subject: [PATCH 19/19] pass in orderCount to orderTable --- .../plugins/core/orders/client/components/orderDashboard.js | 1 - imports/plugins/core/orders/client/components/orderTable.js | 2 +- .../core/orders/client/containers/orderDashboardContainer.js | 3 ++- server/methods/core/orders.app-test.js | 5 ----- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/imports/plugins/core/orders/client/components/orderDashboard.js b/imports/plugins/core/orders/client/components/orderDashboard.js index eea0fd339d6..7b81b125e03 100644 --- a/imports/plugins/core/orders/client/components/orderDashboard.js +++ b/imports/plugins/core/orders/client/components/orderDashboard.js @@ -59,7 +59,6 @@ class OrderDashboard extends Component { } render() { - console.log("props", this.props); return (
value.paymentMethod.method === "credit")[0]; - } - describe("orders/cancelOrder", function () { beforeEach(function () { sandbox.stub(Meteor.server.method_handlers, "orders/sendNotification", function () {