fix/cancel-order (#120)

* fix: adds ability to cancel order

* passing tests

* chore: clean up unused code
This commit is contained in:
Sebastian Rindom
2020-10-06 14:00:47 +02:00
committed by GitHub
parent 4bbb2a2367
commit 11bedf8c6f
29 changed files with 240 additions and 97 deletions
@@ -35,6 +35,10 @@ class ManualFulfillmentService extends FulfillmentService {
// No data is being sent anywhere
return Promise.resolve({})
}
cancelFulfillment() {
return Promise.resolve({})
}
}
export default ManualFulfillmentService
@@ -236,6 +236,25 @@ class WebshipperFulfillmentService extends FulfillmentService {
url: link,
})
}
/**
* Cancels a fulfillment. If the fulfillment has already been canceled this
* is idemptotent. Can only cancel pending orders.
* @param {object} data - the fulfilment data
* @return {Promise<object>} the result of the cancellation
*/
async cancelFulfillment(data) {
const order = await this.client_.orders.retrieve(data.id)
if (order.attributes.status !== "pending") {
if (order.attributes.status === "cancelled") {
return Promise.resolve(order)
}
throw new Error("Cannot cancel order")
}
return this.client_.orders.delete(data.id)
}
}
export default WebshipperFulfillmentService
@@ -83,6 +83,13 @@ class Webshipper {
},
}).then(({ data }) => data)
},
delete: async (id) => {
const path = `/v2/orders/${id}`
return this.client_({
method: "DELETE",
url: path,
}).then(({ data }) => data)
},
}
}
@@ -43,8 +43,8 @@ class StripeProviderService extends PaymentService {
status = "succeeded"
}
if (paymentIntent.status === "cancelled") {
status = "cancelled"
if (paymentIntent.status === "canceled") {
status = "canceled"
}
return status
@@ -228,8 +228,13 @@ class StripeProviderService extends PaymentService {
async cancelPayment(paymentData) {
const { id } = paymentData
try {
return this.stripe_.paymentIntents.cancel(id)
const result = await this.stripe_.paymentIntents.cancel(id)
return result
} catch (error) {
if (error.payment_intent.status === "canceled") {
return error.payment_intent
}
throw error
}
}
@@ -21,6 +21,9 @@ export default () => {
break
}
res.status(statusCode).json(err.message)
res.status(statusCode).json({
name: err.name,
message: err.message,
})
}
}
@@ -11,6 +11,14 @@ describe("POST /admin/orders/:id/fulfillment", () => {
"POST",
`/admin/orders/${IdMap.getId("test-order")}/fulfillment`,
{
payload: {
items: [
{
item_id: IdMap.getId("line1"),
quantity: 1,
},
],
},
adminSession: {
jwt: {
userId: IdMap.getId("admin_user"),
@@ -27,7 +35,14 @@ describe("POST /admin/orders/:id/fulfillment", () => {
it("calls OrderService createFulfillment", () => {
expect(OrderServiceMock.createFulfillment).toHaveBeenCalledTimes(1)
expect(OrderServiceMock.createFulfillment).toHaveBeenCalledWith(
IdMap.getId("test-order")
IdMap.getId("test-order"),
[
{
item_id: IdMap.getId("line1"),
quantity: 1,
},
],
undefined
)
})
@@ -41,7 +41,8 @@ describe("POST /admin/orders/:id/return", () => {
item_id: IdMap.getId("existingLine"),
quantity: 10,
},
]
],
undefined
)
})
})
@@ -3,7 +3,8 @@ export default async (req, res) => {
try {
const orderService = req.scope.resolve("orderService")
const order = await orderService.cancel(id)
let order = await orderService.cancel(id)
order = await orderService.decorate(order, [], ["region"])
res.json({ order })
} catch (error) {
throw error
@@ -14,17 +14,21 @@ describe("GET /store/carts", () => {
jest.clearAllMocks()
})
it("calls get product from productSerice", () => {
it("calls retrieve from CartService", () => {
expect(CartServiceMock.retrieve).toHaveBeenCalledTimes(1)
expect(CartServiceMock.retrieve).toHaveBeenCalledWith(
IdMap.getId("emptyCart")
)
})
it("returns products", () => {
it("returns cart", () => {
expect(subject.body.cart._id).toEqual(IdMap.getId("emptyCart"))
expect(subject.body.cart.decorated).toEqual(true)
})
it("returns 200 status", () => {
expect(subject.status).toEqual(200)
})
})
describe("returns 404 on undefined cart", () => {
@@ -42,6 +42,10 @@ describe("POST /store/customers/:id", () => {
expect(subject.body.customer.first_name).toEqual("LeBron")
expect(subject.body.customer.decorated).toEqual(true)
})
it("status code 200", () => {
expect(subject.status).toEqual(200)
})
})
describe("fails if not authenticated", () => {
+39 -41
View File
@@ -1,38 +1,14 @@
import { createContainer, asValue } from "awilix"
import express from "express"
import cookieParser from "cookie-parser"
import supertest from "supertest"
import jwt from "jsonwebtoken"
import sessions from "client-sessions"
import cookie from "cookie"
import session from "express-session"
import servicesLoader from "../loaders/services"
import expressLoader from "../loaders/express"
import apiLoader from "../loaders/api"
import passportLoader from "../loaders/passport"
import config from "../config"
const testApp = express()
const container = createContainer()
container.register({
logger: asValue({
error: () => {},
}),
})
servicesLoader({ container })
expressLoader({ app: testApp })
passportLoader({ app: testApp, container })
// Add the registered services to the request scope
testApp.use((req, res, next) => {
req.scope = container.createScope()
next()
})
apiLoader({ container, rootDirectory: ".", app: testApp })
const supertestRequest = supertest(testApp)
let adminSessionOpts = {
cookieName: "session",
secret: "test",
@@ -45,9 +21,44 @@ let clientSessionOpts = {
}
export { clientSessionOpts }
const testApp = express()
const container = createContainer()
container.register({
logger: asValue({
error: () => {},
}),
})
testApp.set("trust proxy", 1)
testApp.use((req, res, next) => {
req.session = {}
const data = req.get("Cookie")
if (data) {
req.session = {
...req.session,
...JSON.parse(data),
}
}
next()
})
servicesLoader({ container })
passportLoader({ app: testApp, container })
testApp.use((req, res, next) => {
req.scope = container.createScope()
next()
})
apiLoader({ container, rootDirectory: ".", app: testApp })
const supertestRequest = supertest(testApp)
export async function request(method, url, opts = {}) {
let { payload, headers } = opts
let req = supertestRequest[method.toLowerCase()](url)
headers = headers || {}
headers.Cookie = headers.Cookie || ""
if (opts.adminSession) {
@@ -60,13 +71,7 @@ export async function request(method, url, opts = {}) {
}
)
}
headers.Cookie +=
adminSessionOpts.cookieName +
"=" +
sessions.util.encode(adminSessionOpts, opts.adminSession) +
"; "
// console.log(sessions.util.decode(adminSessionOpts, opts.headers.Cookie))
headers.Cookie = JSON.stringify(opts.adminSession) || ""
}
if (opts.clientSession) {
if (opts.clientSession.jwt) {
@@ -79,16 +84,9 @@ export async function request(method, url, opts = {}) {
)
}
headers.Cookie +=
clientSessionOpts.cookieName +
"=" +
sessions.util.encode(clientSessionOpts, opts.clientSession) +
"; "
// console.log(sessions.util.decode(adminSessionOpts, opts.headers.Cookie))
headers.Cookie = JSON.stringify(opts.clientSession) || ""
}
let req = supertestRequest[method.toLowerCase()](url)
for (let name in headers) {
req.set(name, headers[name])
}
+4 -1
View File
@@ -10,7 +10,10 @@ import config from "../config"
export default async ({ app, configModule }) => {
let sameSite = false
let secure = false
if (process.env.NODE_ENV === "production" || process.env.NODE_ENV === "staging") {
if (
process.env.NODE_ENV === "production" ||
process.env.NODE_ENV === "staging"
) {
secure = true
sameSite = "none"
}
@@ -36,6 +36,7 @@ export const orders = {
},
quantity: 1,
},
fulfilled_quantity: 0,
quantity: 10,
},
],
@@ -56,6 +57,12 @@ export const orders = {
},
},
],
fulfillments: [
{
provider_id: "default_provider",
data: {},
},
],
fulfillment_status: "not_fulfilled",
payment_status: "awaiting",
status: "pending",
@@ -7,5 +7,6 @@ export default new mongoose.Schema({
data: { type: [mongoose.Schema.Types.Mixed], default: {} },
tracking_numbers: { type: [String], default: [] },
shipped_at: { type: String },
is_canceled: { type: Boolean, default: false },
metadata: { type: mongoose.Schema.Types.Mixed, default: {} },
})
@@ -1,4 +1,4 @@
import bcrypt from "bcrypt"
import Scrypt from "scrypt-kdf"
import { IdMap } from "medusa-test-utils"
export const CustomerServiceMock = {
@@ -43,9 +43,10 @@ export const CustomerServiceMock = {
})
}
if (email === "oliver@test.dk") {
return bcrypt
.hash("123456789", 10)
.then(hash => ({ email, password_hash: hash }))
return Scrypt.kdf("123456789", { logN: 1, r: 1, p: 1 }).then(hash => ({
email,
password_hash: hash.toString("base64"),
}))
}
return Promise.resolve(undefined)
}),
@@ -42,6 +42,9 @@ export const DiscountServiceMock = {
list: jest.fn().mockImplementation(data => {
return Promise.resolve([])
}),
decorate: jest.fn().mockImplementation(data => {
return Promise.resolve(data)
}),
addRegion: jest.fn().mockReturnValue(Promise.resolve()),
removeRegion: jest.fn().mockReturnValue(Promise.resolve()),
addValidVariant: jest.fn().mockReturnValue(Promise.resolve()),
@@ -13,6 +13,9 @@ export const DefaultProviderMock = {
return Promise.resolve(false)
}),
cancelFulfillment: jest.fn().mockImplementation(data => {
return {}
}),
calculatePrice: jest.fn().mockImplementation(data => {
return Promise.resolve()
}),
@@ -15,6 +15,7 @@ export const DefaultProviderMock = {
}),
capturePayment: jest.fn().mockReturnValue(Promise.resolve()),
refundPayment: jest.fn().mockReturnValue(Promise.resolve()),
cancelPayment: jest.fn().mockReturnValue(Promise.resolve({})),
}
export const PaymentProviderServiceMock = {
@@ -37,6 +37,7 @@ export const ProductServiceMock = {
return Promise.resolve({ ...data })
}),
count: jest.fn().mockReturnValue(4),
publish: jest.fn().mockImplementation(_ => {
return Promise.resolve({
_id: IdMap.getId("publish"),
@@ -126,6 +126,7 @@ export const ShippingProfileServiceMock = {
])
}
}),
decorate: jest.fn().mockImplementation(d => Promise.resolve(d)),
addShippingOption: jest.fn().mockImplementation(() => Promise.resolve()),
removeShippingOption: jest.fn().mockImplementation(() => Promise.resolve()),
addProduct: jest.fn().mockImplementation(() => Promise.resolve()),
@@ -1,4 +1,4 @@
import bcrypt from "bcrypt"
import Scrypt from "scrypt-kdf"
import { IdMap } from "medusa-test-utils"
import _ from "lodash"
@@ -92,9 +92,10 @@ export const UserServiceMock = {
})
}
if (email === "oliver@test.dk") {
return bcrypt
.hash("123456789", 10)
.then(hash => ({ email, password_hash: hash }))
return Scrypt.kdf("123456789", { logN: 1, r: 1, p: 1 }).then(hash => ({
email,
password_hash: hash.toString("base64"),
}))
}
return Promise.resolve(undefined)
}),
+21 -4
View File
@@ -228,12 +228,26 @@ describe("CartService", () => {
expect(CartModelMock.updateOne).toHaveBeenCalledWith(
{
_id: IdMap.getId("cartWithLine"),
"items._id": IdMap.getId("existingLine"),
},
{
$set: {
"items.$.quantity": 20,
"items.$.has_shipping": false,
$push: {
items: {
title: "merge line",
description: "This is a new line",
thumbnail: "test-img-yeah.com/thumb",
has_shipping: false,
content: {
unit_price: 123,
variant: {
_id: IdMap.getId("can-cover"),
},
product: {
_id: IdMap.getId("product"),
},
quantity: 1,
},
quantity: 10,
},
},
}
)
@@ -736,6 +750,9 @@ describe("CartService", () => {
$set: {
region_id: IdMap.getId("region-us"),
shipping_methods: [],
shipping_address: {
country_code: "US",
},
items: [
{
_id: IdMap.getId("line"),
@@ -141,9 +141,7 @@ describe("CustomerService", () => {
first_name: "Oliver",
last_name: "Juhl",
has_account: true,
password_hash: expect.stringMatching(
/^\$2[aby]?\$[\d]+\$[./A-Za-z0-9]{53}$/
),
password_hash: expect.stringMatching(/^.{128}$/),
})
})
@@ -259,9 +257,7 @@ describe("CustomerService", () => {
{
$set: {
has_account: true,
password_hash: expect.stringMatching(
/^\$2[aby]?\$[\d]+\$[./A-Za-z0-9]{53}$/
),
password_hash: expect.stringMatching(/^.{128}$/),
},
},
{ runValidators: true }
@@ -23,7 +23,9 @@ describe("EventBusService", () => {
it("creates bull queue", () => {
expect(Bull).toHaveBeenCalledTimes(2)
expect(Bull).toHaveBeenCalledWith("EventBusService:queue", "testhost")
expect(Bull).toHaveBeenCalledWith("EventBusService:queue", {
createClient: expect.any(Function),
})
})
})
@@ -27,6 +27,7 @@ describe("LineItemService", () => {
title: "test",
description: "EUR10US-12",
thumbnail: "test.1234",
should_merge: true,
content: {
unit_price: 10,
variant: {
@@ -41,6 +42,7 @@ describe("LineItemService", () => {
quantity: 1,
},
quantity: 2,
metadata: {},
})
})
})
@@ -88,6 +90,7 @@ describe("LineItemService", () => {
name: "Test Name",
},
quantity: 1,
should_merge: true,
})
})
})
@@ -47,7 +47,7 @@ describe("OrderService", () => {
discountService: DiscountServiceMock,
regionService: RegionServiceMock,
eventBusService: EventBusServiceMock,
counterService: CounterServiceMock
counterService: CounterServiceMock,
})
beforeEach(async () => {
@@ -135,6 +135,7 @@ describe("OrderService", () => {
tax_rate: 0.25,
email: "test",
giftcard: expect.any(Object),
line_item: expect.any(Object),
}
)
@@ -328,6 +329,8 @@ describe("OrderService", () => {
describe("cancel", () => {
const orderService = new OrderService({
fulfillmentProviderService: FulfillmentProviderServiceMock,
paymentProviderService: PaymentProviderServiceMock,
orderModel: OrderModelMock,
eventBusService: EventBusServiceMock,
})
@@ -342,7 +345,24 @@ describe("OrderService", () => {
expect(OrderModelMock.updateOne).toHaveBeenCalledTimes(1)
expect(OrderModelMock.updateOne).toHaveBeenCalledWith(
{ _id: IdMap.getId("not-fulfilled-order") },
{ $set: { status: "cancelled" } }
{
$set: {
status: "canceled",
fulfillment_status: "canceled",
payment_status: "canceled",
fulfillments: [
{
data: {},
is_canceled: true,
provider_id: "default_provider",
},
],
payment_method: {
data: {},
provider_id: "default_provider",
},
},
}
)
})
@@ -359,7 +379,7 @@ describe("OrderService", () => {
await orderService.cancel(IdMap.getId("payed-order"))
} catch (error) {
expect(error.message).toEqual(
"Can't cancel an order with payment processed"
"Can't cancel an order with a processed payment"
)
}
})
@@ -435,9 +455,11 @@ describe("OrderService", () => {
},
quantity: 1,
},
fulfilled_quantity: 0,
quantity: 10,
},
]
],
orders.testOrder
)
expect(OrderModelMock.updateOne).toHaveBeenCalledTimes(1)
@@ -467,6 +489,7 @@ describe("OrderService", () => {
},
quantity: 1,
},
fulfilled_quantity: 0,
quantity: 10,
},
],
@@ -504,15 +527,15 @@ describe("OrderService", () => {
)
})
it("throws if payment is already processed", async () => {
it("throws if too many items are requested fulfilled", async () => {
await expect(
orderService.createFulfillment(IdMap.getId("fulfilled-order"), [
orderService.createFulfillment(IdMap.getId("test-order"), [
{
item_id: IdMap.getId("existingLine"),
quantity: 10,
quantity: 11,
},
])
).rejects.toThrow("Order is already fulfilled")
).rejects.toThrow("Cannot fulfill more items than have been purchased")
})
})
@@ -53,9 +53,7 @@ describe("UserService", () => {
expect(UserModelMock.create).toHaveBeenCalledWith({
email: "oliver@test.dk",
name: "Oliver",
password_hash: expect.stringMatching(
/^\$2[aby]?\$[\d]+\$[./A-Za-z0-9]{53}$/
),
password_hash: expect.stringMatching(/.{128}$/),
})
})
})
@@ -134,9 +132,7 @@ describe("UserService", () => {
$set: {
// Since bcrypt hashing always varies, we are testing the password
// match by using a regular expression.
password_hash: expect.stringMatching(
/^\$2[aby]?\$[\d]+\$[./A-Za-z0-9]{53}$/
),
password_hash: expect.stringMatching(/^.{128}$/),
},
}
)
+35 -12
View File
@@ -12,7 +12,7 @@ class OrderService extends BaseService {
REFUND_CREATED: "order.refund_created",
PLACED: "order.placed",
UPDATED: "order.updated",
CANCELLED: "order.cancelled",
CANCELED: "order.canceled",
COMPLETED: "order.completed",
}
@@ -542,21 +542,35 @@ class OrderService extends BaseService {
async cancel(orderId) {
const order = await this.retrieve(orderId)
if (order.fulfillment_status !== "not_fulfilled") {
throw new MedusaError(
MedusaError.Types.NOT_ALLOWED,
"Can't cancel a fulfilled order"
)
}
if (order.payment_status !== "awaiting") {
throw new MedusaError(
MedusaError.Types.NOT_ALLOWED,
"Can't cancel an order with payment processed"
"Can't cancel an order with a processed payment"
)
}
// TODO: cancel payment method
const fulfillments = await Promise.all(
order.fulfillments.map(async fulfillment => {
const { provider_id, data } = fulfillment
const provider = await this.fulfillmentProviderService_.retrieveProvider(
provider_id
)
const newData = await provider.cancelFulfillment(data)
return {
...fulfillment,
is_canceled: true,
data: newData,
}
})
)
const { provider_id, data } = order.payment_method
const paymentProvider = await this.paymentProviderService_.retrieveProvider(
provider_id
)
// Cancel payment with payment provider
const payData = await paymentProvider.cancelPayment(data)
return this.orderModel_
.updateOne(
@@ -564,12 +578,21 @@ class OrderService extends BaseService {
_id: orderId,
},
{
$set: { status: "cancelled" },
$set: {
status: "canceled",
fulfillment_status: "canceled",
payment_status: "canceled",
fulfillments,
payment_method: {
...order.payment_method,
data: payData,
},
},
}
)
.then(result => {
// Notify subscribers
this.eventBus_.emit(OrderService.Events.CANCELLED, result)
this.eventBus_.emit(OrderService.Events.CANCELED, result)
return result
})
.catch(err => {
+1 -1
View File
@@ -231,7 +231,7 @@ class UserService extends BaseService {
async setPassword(userId, password) {
const user = await this.retrieve(userId)
const hashedPassword = await bcrypt.hash(password, 10)
const hashedPassword = await this.hashPassword_(password)
if (!hashedPassword) {
throw new MedusaError(
MedusaError.Types.DB_ERROR,