From 6d1d078e9e30b549e646be0e9f9bdd17bee41772 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Tue, 14 May 2019 14:35:58 -0400 Subject: [PATCH 1/7] Use query parameters when loading checkout page --- static/js/containers/pages/CheckoutPage.js | 55 +++++-- .../js/containers/pages/CheckoutPage_test.js | 137 +++++++++++++++++- static/js/lib/ecommerce.js | 19 +++ 3 files changed, 192 insertions(+), 19 deletions(-) diff --git a/static/js/containers/pages/CheckoutPage.js b/static/js/containers/pages/CheckoutPage.js index cbd3aceaf..e52d69a14 100644 --- a/static/js/containers/pages/CheckoutPage.js +++ b/static/js/containers/pages/CheckoutPage.js @@ -2,19 +2,22 @@ import React from "react" import * as R from "ramda" import { connect } from "react-redux" -import { connectRequest, mutateAsync } from "redux-query" +import { mutateAsync, requestAsync } from "redux-query" import { compose } from "redux" +import queryString from "query-string" import queries from "../../lib/queries" import { calculateDiscount, calculatePrice, formatPrice, - formatRunTitle + formatRunTitle, + formatErrors } from "../../lib/ecommerce" import { createCyberSourceForm } from "../../lib/form" import type { Response } from "redux-query" +import type { Location } from "react-router" import type { BasketResponse, BasketPayload, @@ -50,6 +53,8 @@ export const calcSelectedRunIds = (item: BasketItem): { [number]: number } => { type Props = { basket: ?BasketResponse, checkout: () => Promise>, + fetchBasket: () => Promise<*>, + location: Location, updateBasket: (payload: BasketPayload) => Promise<*> } type State = { @@ -64,6 +69,30 @@ export class CheckoutPage extends React.Component { errors: null } + componentDidMount = async () => { + const { + fetchBasket, + location: { search } + } = this.props + const params = queryString.parse(search) + const productId = parseInt(params.product) + if (!productId) { + await fetchBasket() + return + } + + try { + await this.updateBasket({ items: [{ id: productId }] }) + + const couponCode = params.code + if (couponCode) { + await this.updateBasket({ coupons: [{ code: couponCode }] }) + } + } catch (_) { + // prevent complaints about unresolved promises + } + } + handleErrors = async (responsePromise: Promise<*>) => { const response = await responsePromise if (response.body.errors) { @@ -202,13 +231,14 @@ export class CheckoutPage extends React.Component { const { basket } = this.props const { couponCode, errors } = this.state - if (!basket) { - return null - } - - const item = basket.items[0] - if (!item) { - return
No item in basket
+ const item = basket && basket.items[0] + if (!basket || !item) { + return ( +
+ No item in basket + {formatErrors(errors)} +
+ ) } const coupon = basket.coupons.find(coupon => @@ -259,7 +289,7 @@ export class CheckoutPage extends React.Component { Apply - {errors ?
Error: {errors}
: null} + {formatErrors(errors)} @@ -302,15 +332,14 @@ const mapStateToProps = state => ({ }) const mapDispatchToProps = dispatch => ({ checkout: () => dispatch(mutateAsync(queries.ecommerce.checkoutMutation())), + fetchBasket: () => dispatch(requestAsync(queries.ecommerce.basketQuery())), updateBasket: payload => dispatch(mutateAsync(queries.ecommerce.basketMutation(payload))) }) -const mapPropsToConfigs = () => [queries.ecommerce.basketQuery()] export default compose( connect( mapStateToProps, mapDispatchToProps - ), - connectRequest(mapPropsToConfigs) + ) )(CheckoutPage) diff --git a/static/js/containers/pages/CheckoutPage_test.js b/static/js/containers/pages/CheckoutPage_test.js index 16c7549de..8c0c7fb5c 100644 --- a/static/js/containers/pages/CheckoutPage_test.js +++ b/static/js/containers/pages/CheckoutPage_test.js @@ -18,7 +18,7 @@ import { formatPrice, formatRunTitle } from "../../lib/ecommerce" -import { assertRaises } from "../../lib/util" +import { assertRaises, wait } from "../../lib/util" import { PRODUCT_TYPE_COURSERUN, PRODUCT_TYPE_PROGRAM } from "../../constants" describe("CheckoutPage", () => { @@ -36,7 +36,9 @@ describe("CheckoutPage", () => { basket } }, - {} + { + location: {} + } ) }) @@ -102,6 +104,132 @@ describe("CheckoutPage", () => { assert.equal(inner.find("img").prop("alt"), basketItem.description) assert.equal(inner.find(".item-row .title").text(), basketItem.description) }) + ;[true, false].forEach(hasError => { + it(`updates the basket with a product id from the query parameter${ + hasError ? ", but an error is returned" : "" + }`, async () => { + const productId = 4567 + if (hasError) { + helper.handleRequestStub.withArgs("/api/basket/", "PATCH").returns({ + status: 400, + body: { + errors: "error" + } + }) + } + const { inner } = await renderPage( + {}, + { + location: { + search: `product=${productId}` + } + } + ) + + sinon.assert.calledWith( + helper.handleRequestStub, + "/api/basket/", + "PATCH", + { + body: { items: [{ id: productId }] }, + credentials: undefined, + headers: { + "X-CSRFTOKEN": null + } + } + ) + assert.equal(inner.state().errors, hasError ? "error" : null) + }) + }) + + // + ;[[true, false], [true, true], [false, false], [false, true]].forEach( + ([hasValidProductId, hasValidCoupon]) => { + it(`updates the basket with a ${ + hasValidProductId ? "" : "in" + }valid product id and a ${ + hasValidCoupon ? "" : "in" + }valid coupon code from the query parameter`, async () => { + const couponCode = "codeforcoupon" + const productId = 12345 + const couponPayload = { + body: { coupons: [{ code: couponCode }] }, + credentials: undefined, + headers: { + "X-CSRFTOKEN": null + } + } + const productPayload = { + body: { items: [{ id: productId }] }, + credentials: undefined, + headers: { + "X-CSRFTOKEN": null + } + } + + if (!hasValidProductId) { + helper.handleRequestStub + .withArgs("/api/basket/", "PATCH", productPayload) + .returns({ + status: 400, + body: { + errors: "product error" + } + }) + } + if (!hasValidCoupon) { + helper.handleRequestStub + .withArgs("/api/basket/", "PATCH", couponPayload) + .returns({ + status: 400, + body: { + errors: "coupon error" + } + }) + } + const { inner } = await renderPage( + {}, + { + location: { + search: `product=${productId}&code=${couponCode}` + } + } + ) + // wait for componentDidMount to resolve + await wait(15) + sinon.assert.calledWith( + helper.handleRequestStub, + "/api/basket/", + "PATCH", + productPayload + ) + if (hasValidProductId) { + sinon.assert.calledWith( + helper.handleRequestStub, + "/api/basket/", + "PATCH", + couponPayload + ) + } else { + sinon.assert.neverCalledWith( + helper.handleRequestStub, + "/api/basket/", + "PATCH", + couponPayload + ) + } + + assert.equal( + inner.state().errors, + !hasValidProductId + ? "product error" + : !hasValidCoupon + ? "coupon error" + : null + ) + }) + } + ) it("displays the coupon code", async () => { const { inner } = await renderPage() @@ -167,10 +295,7 @@ describe("CheckoutPage", () => { }) assert.equal(inner.state().errors, errors) - assert.equal( - inner.find(".enrollment-input .error").text(), - "Error: Unknown error" - ) + assert.equal(inner.find(".enrollment-input .error").text(), "Unknown error") assert.isTrue(inner.find(".enrollment-input input.error-border").exists()) }) diff --git a/static/js/lib/ecommerce.js b/static/js/lib/ecommerce.js index 87c627147..3e330f91e 100644 --- a/static/js/lib/ecommerce.js +++ b/static/js/lib/ecommerce.js @@ -1,4 +1,5 @@ // @flow +import React from "react" import Decimal from "decimal.js-light" import * as R from "ramda" import { equals } from "ramda" @@ -57,6 +58,24 @@ const formatDateForRun = (dateString: ?string) => export const formatRunTitle = (run: CourseRun) => `${formatDateForRun(run.start_date)} - ${formatDateForRun(run.end_date)}` +export const formatErrors = (errors: string | Object) => { + if (!errors) { + return null + } + + let errorString + if (typeof errors === "object") { + if (errors.items) { + errorString = errors.items[0] + } else { + errorString = errors[0] + } + } else { + errorString = errors + } + return
{errorString}
+} + export const isPromo = equals(COUPON_TYPE_PROMO) export const createProductMap = ( From 8952a012f5983cbd1baa103710a5b1602ab992eb Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 20 May 2019 11:45:08 -0400 Subject: [PATCH 2/7] Refactor tests --- .../js/containers/pages/CheckoutPage_test.js | 64 ++++++++++--------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/static/js/containers/pages/CheckoutPage_test.js b/static/js/containers/pages/CheckoutPage_test.js index 8c0c7fb5c..82beb046c 100644 --- a/static/js/containers/pages/CheckoutPage_test.js +++ b/static/js/containers/pages/CheckoutPage_test.js @@ -142,31 +142,40 @@ describe("CheckoutPage", () => { }) }) - // - ;[[true, false], [true, true], [false, false], [false, true]].forEach( - ([hasValidProductId, hasValidCoupon]) => { + describe("mount", () => { + const productError = "product error", + couponError = "coupon error", + couponCode = "codeforcoupon", + productId = 12345 + let couponPayload, productPayload + + beforeEach(() => { + couponPayload = { + body: { coupons: [{ code: couponCode }] }, + credentials: undefined, + headers: { + "X-CSRFTOKEN": null + } + } + productPayload = { + body: { items: [{ id: productId }] }, + credentials: undefined, + headers: { + "X-CSRFTOKEN": null + } + } + }) + ;[ + [true, false, couponError], + [true, true, null], + [false, false, productError], + [false, true, productError] + ].forEach(([hasValidProductId, hasValidCoupon, expError]) => { it(`updates the basket with a ${ hasValidProductId ? "" : "in" }valid product id and a ${ hasValidCoupon ? "" : "in" }valid coupon code from the query parameter`, async () => { - const couponCode = "codeforcoupon" - const productId = 12345 - const couponPayload = { - body: { coupons: [{ code: couponCode }] }, - credentials: undefined, - headers: { - "X-CSRFTOKEN": null - } - } - const productPayload = { - body: { items: [{ id: productId }] }, - credentials: undefined, - headers: { - "X-CSRFTOKEN": null - } - } - if (!hasValidProductId) { helper.handleRequestStub .withArgs("/api/basket/", "PATCH", productPayload) @@ -196,7 +205,7 @@ describe("CheckoutPage", () => { } ) // wait for componentDidMount to resolve - await wait(15) + await wait(0) sinon.assert.calledWith( helper.handleRequestStub, "/api/basket/", @@ -219,17 +228,10 @@ describe("CheckoutPage", () => { ) } - assert.equal( - inner.state().errors, - !hasValidProductId - ? "product error" - : !hasValidCoupon - ? "coupon error" - : null - ) + assert.equal(inner.state().errors, expError) }) - } - ) + }) + }) it("displays the coupon code", async () => { const { inner } = await renderPage() From 459e403c1587e7760eb978c0532d39653651da20 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 20 May 2019 11:50:31 -0400 Subject: [PATCH 3/7] Replace wait with await Promise.resolve() --- static/js/containers/pages/CheckoutPage_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/static/js/containers/pages/CheckoutPage_test.js b/static/js/containers/pages/CheckoutPage_test.js index 82beb046c..9385eaecb 100644 --- a/static/js/containers/pages/CheckoutPage_test.js +++ b/static/js/containers/pages/CheckoutPage_test.js @@ -18,7 +18,7 @@ import { formatPrice, formatRunTitle } from "../../lib/ecommerce" -import { assertRaises, wait } from "../../lib/util" +import { assertRaises } from "../../lib/util" import { PRODUCT_TYPE_COURSERUN, PRODUCT_TYPE_PROGRAM } from "../../constants" describe("CheckoutPage", () => { @@ -205,7 +205,7 @@ describe("CheckoutPage", () => { } ) // wait for componentDidMount to resolve - await wait(0) + await Promise.resolve() sinon.assert.calledWith( helper.handleRequestStub, "/api/basket/", From 2854ab064a8bbcbd9f7294a1174dde3723b742ca Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 20 May 2019 12:02:34 -0400 Subject: [PATCH 4/7] Tests for formatErrors --- static/js/lib/ecommerce.js | 18 ------------------ static/js/lib/form.js | 21 +++++++++++++++++++++ static/js/lib/form_test.js | 26 +++++++++++++++++++++++++- 3 files changed, 46 insertions(+), 19 deletions(-) diff --git a/static/js/lib/ecommerce.js b/static/js/lib/ecommerce.js index 3e330f91e..55eab6fc7 100644 --- a/static/js/lib/ecommerce.js +++ b/static/js/lib/ecommerce.js @@ -58,24 +58,6 @@ const formatDateForRun = (dateString: ?string) => export const formatRunTitle = (run: CourseRun) => `${formatDateForRun(run.start_date)} - ${formatDateForRun(run.end_date)}` -export const formatErrors = (errors: string | Object) => { - if (!errors) { - return null - } - - let errorString - if (typeof errors === "object") { - if (errors.items) { - errorString = errors.items[0] - } else { - errorString = errors[0] - } - } else { - errorString = errors - } - return
{errorString}
-} - export const isPromo = equals(COUPON_TYPE_PROMO) export const createProductMap = ( diff --git a/static/js/lib/form.js b/static/js/lib/form.js index e37617bad..df8271422 100644 --- a/static/js/lib/form.js +++ b/static/js/lib/form.js @@ -1,5 +1,6 @@ // @flow import type { CheckoutPayload } from "../flow/ecommerceTypes" +import React from "react" /** * Creates a POST form with hidden input fields @@ -25,3 +26,23 @@ export function createCyberSourceForm( } return form } + +export const formatErrors = ( + errors: string | Object | null +): React$Element<*> | null => { + if (!errors) { + return null + } + + let errorString + if (typeof errors === "object") { + if (errors.items) { + errorString = errors.items[0] + } else { + errorString = errors[0] + } + } else { + errorString = errors + } + return
{errorString}
+} diff --git a/static/js/lib/form_test.js b/static/js/lib/form_test.js index 0b2551113..a68ff7ff2 100644 --- a/static/js/lib/form_test.js +++ b/static/js/lib/form_test.js @@ -1,8 +1,9 @@ // @flow import { assert } from "chai" +import { shallow } from "enzyme" import { CYBERSOURCE_CHECKOUT_RESPONSE } from "./test_constants" -import { createCyberSourceForm } from "./form" +import { createCyberSourceForm, formatErrors } from "./form" describe("form functions", () => { it("creates a form with hidden values corresponding to the payload", () => { @@ -23,4 +24,27 @@ describe("form functions", () => { assert.equal(form.getAttribute("action"), url) assert.equal(form.getAttribute("method"), "post") }) + + describe("formatErrors", () => { + it("should return null if there is no error", () => { + assert.isNull(formatErrors(null)) + }) + + it("should return a div with the error string if the error is a string", () => { + const wrapper = shallow(formatErrors("error")) + assert.equal(wrapper.find(".error").text(), "error") + }) + + it("should return the first item in the items in the error object if it has items", () => { + const error = { items: ["error"] } + const wrapper = shallow(formatErrors(error)) + assert.equal(wrapper.find(".error").text(), "error") + }) + + it("should return the first item in the error if there is no 'items'", () => { + const error = ["error"] + const wrapper = shallow(formatErrors(error)) + assert.equal(wrapper.find(".error").text(), "error") + }) + }) }) From c30caf6e1b312cb1da6847d0fc8fb64129400e76 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 20 May 2019 13:54:32 -0400 Subject: [PATCH 5/7] Fix import --- static/js/containers/pages/CheckoutPage.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/static/js/containers/pages/CheckoutPage.js b/static/js/containers/pages/CheckoutPage.js index e52d69a14..71c320a5c 100644 --- a/static/js/containers/pages/CheckoutPage.js +++ b/static/js/containers/pages/CheckoutPage.js @@ -11,10 +11,9 @@ import { calculateDiscount, calculatePrice, formatPrice, - formatRunTitle, - formatErrors + formatRunTitle } from "../../lib/ecommerce" -import { createCyberSourceForm } from "../../lib/form" +import { createCyberSourceForm, formatErrors } from "../../lib/form" import type { Response } from "redux-query" import type { Location } from "react-router" From 0986ed1519684cd0e3191cf0eae0eaf13365ffb0 Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 20 May 2019 13:58:54 -0400 Subject: [PATCH 6/7] Remove if/else --- static/js/containers/pages/CheckoutPage_test.js | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/static/js/containers/pages/CheckoutPage_test.js b/static/js/containers/pages/CheckoutPage_test.js index 9385eaecb..6c3ef7d72 100644 --- a/static/js/containers/pages/CheckoutPage_test.js +++ b/static/js/containers/pages/CheckoutPage_test.js @@ -212,21 +212,14 @@ describe("CheckoutPage", () => { "PATCH", productPayload ) - if (hasValidProductId) { - sinon.assert.calledWith( - helper.handleRequestStub, - "/api/basket/", - "PATCH", - couponPayload - ) - } else { - sinon.assert.neverCalledWith( - helper.handleRequestStub, + assert.equal( + helper.handleRequestStub.calledWith( "/api/basket/", "PATCH", couponPayload - ) - } + ), + hasValidProductId + ) assert.equal(inner.state().errors, expError) }) From 1437846c064ad7376b1d405aa940c7290b67484f Mon Sep 17 00:00:00 2001 From: George Schneeloch Date: Mon, 20 May 2019 14:19:31 -0400 Subject: [PATCH 7/7] Remove import --- static/js/lib/ecommerce.js | 1 - 1 file changed, 1 deletion(-) diff --git a/static/js/lib/ecommerce.js b/static/js/lib/ecommerce.js index 55eab6fc7..87c627147 100644 --- a/static/js/lib/ecommerce.js +++ b/static/js/lib/ecommerce.js @@ -1,5 +1,4 @@ // @flow -import React from "react" import Decimal from "decimal.js-light" import * as R from "ramda" import { equals } from "ramda"