Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use query parameters when loading checkout page #283

Merged
merged 7 commits into from May 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 41 additions & 13 deletions static/js/containers/pages/CheckoutPage.js
Expand Up @@ -2,8 +2,9 @@
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 {
Expand All @@ -12,9 +13,10 @@ import {
formatPrice,
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"
import type {
BasketResponse,
BasketPayload,
Expand Down Expand Up @@ -50,6 +52,8 @@ export const calcSelectedRunIds = (item: BasketItem): { [number]: number } => {
type Props = {
basket: ?BasketResponse,
checkout: () => Promise<Response<CheckoutResponse>>,
fetchBasket: () => Promise<*>,
location: Location,
updateBasket: (payload: BasketPayload) => Promise<*>
}
type State = {
Expand All @@ -64,6 +68,30 @@ export class CheckoutPage extends React.Component<Props, State> {
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 }] })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this isn't already possible, can this query/endpoint be rewritten to accept items and coupons in the same call? Executing 2 requests seems unnecessary here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little tricky because the behavior we want is to update items then update coupons. If the coupons update fails the items update should still succeed. If both were sent as the same PATCH request and the coupons had a validation error but the items succeeded, I think the proper thing is to not make any changes and return a 400 error with an error message in the payload. What do you think?

}
} catch (_) {
// prevent complaints about unresolved promises
}
}

handleErrors = async (responsePromise: Promise<*>) => {
const response = await responsePromise
if (response.body.errors) {
Expand Down Expand Up @@ -202,13 +230,14 @@ export class CheckoutPage extends React.Component<Props, State> {
const { basket } = this.props
const { couponCode, errors } = this.state

if (!basket) {
return null
}

const item = basket.items[0]
if (!item) {
return <div>No item in basket</div>
const item = basket && basket.items[0]
if (!basket || !item) {
return (
<div className="checkout-page">
No item in basket
{formatErrors(errors)}
</div>
)
}

const coupon = basket.coupons.find(coupon =>
Expand Down Expand Up @@ -259,7 +288,7 @@ export class CheckoutPage extends React.Component<Props, State> {
Apply
</button>
</div>
{errors ? <div className="error">Error: {errors}</div> : null}
{formatErrors(errors)}
</form>
</div>
</div>
Expand Down Expand Up @@ -302,15 +331,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)
130 changes: 125 additions & 5 deletions static/js/containers/pages/CheckoutPage_test.js
Expand Up @@ -36,7 +36,9 @@ describe("CheckoutPage", () => {
basket
}
},
{}
{
location: {}
}
)
})

Expand Down Expand Up @@ -102,6 +104,127 @@ 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)
})
})

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 () => {
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 Promise.resolve()
sinon.assert.calledWith(
helper.handleRequestStub,
"/api/basket/",
"PATCH",
productPayload
)
assert.equal(
helper.handleRequestStub.calledWith(
"/api/basket/",
"PATCH",
couponPayload
),
hasValidProductId
)

assert.equal(inner.state().errors, expError)
})
})
})

it("displays the coupon code", async () => {
const { inner } = await renderPage()
Expand Down Expand Up @@ -167,10 +290,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())
})

Expand Down
21 changes: 21 additions & 0 deletions 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
Expand All @@ -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 <div className="error">{errorString}</div>
}
26 changes: 25 additions & 1 deletion 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", () => {
Expand All @@ -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")
})
})
})