diff --git a/client/components/FeaturedProduct.js b/client/components/FeaturedProduct.js index 3a789e5..b47765e 100644 --- a/client/components/FeaturedProduct.js +++ b/client/components/FeaturedProduct.js @@ -13,6 +13,10 @@ class FeaturedProduct extends Component { loading: true } } + // OB/LM: you can do + /* + state = {products: [], loading: true} + */ async componentDidMount() { try { diff --git a/client/components/HomePage.js b/client/components/HomePage.js index 8ce6c56..0733dab 100644 --- a/client/components/HomePage.js +++ b/client/components/HomePage.js @@ -17,6 +17,7 @@ import { } from 'semantic-ui-react' import {NavLink} from 'react-router-dom' +// OB/LM: listen to linter const getWidth = () => { const isSSR = typeof window === 'undefined' diff --git a/client/components/ProductCard.js b/client/components/ProductCard.js index dc393ad..4b58f6e 100644 --- a/client/components/ProductCard.js +++ b/client/components/ProductCard.js @@ -6,6 +6,7 @@ import axios from 'axios' import {Button, Icon, Item, Label} from 'semantic-ui-react' import ReactHtmlParser from 'react-html-parser' +// OB/LM: could make for a good test (especially if you separate out the local storage stuff) const addToCart = async (product, setCartSize, user) => { const currentItems = JSON.parse(localStorage.getItem('cart')) diff --git a/client/components/Profile.js b/client/components/Profile.js index 73a48b1..5f07d40 100644 --- a/client/components/Profile.js +++ b/client/components/Profile.js @@ -49,6 +49,7 @@ class Profile extends React.Component { key: orderNumber, title: orderNumber, content: { + // Maybe could be content: orderTable(order.id, products, totalPrice) } } @@ -86,6 +87,7 @@ const orderTable = (id, products, totalPrice) => { + {/* OB/LM: maybe group by product id before mapping */} {products.map((product, idx) => { totalPrice += product.price return ( diff --git a/client/store/index.js b/client/store/index.js index bd98a32..8135aab 100644 --- a/client/store/index.js +++ b/client/store/index.js @@ -6,6 +6,7 @@ import user from './user' import cart from './cart' import checkout from './checkout' +// OB/LM: redux middleware for persisting cart to `localStorage` (could help reduce "multiple update" issues) const reducer = combineReducers({user, cart, checkout}) const middleware = composeWithDevTools( applyMiddleware(thunkMiddleware, createLogger({collapsed: true})) diff --git a/server/api/payment.js b/server/api/payment.js index bcb4216..a5781a7 100644 --- a/server/api/payment.js +++ b/server/api/payment.js @@ -17,6 +17,7 @@ const paymentApi = app => { }) app.post('/', (req, res) => { + // OB/LM: client can define charge amount (not good), sanitize the request body (recommend util method for this) stripe.charges.create(req.body, postStripeCharge(res)) }) diff --git a/server/api/users.js b/server/api/users.js index d24fb31..5f6d3eb 100644 --- a/server/api/users.js +++ b/server/api/users.js @@ -21,6 +21,7 @@ router.get('/', async (req, res, next) => { next(err) } } + // OB/LM: missing else }) // GET api/users/:id @@ -69,8 +70,10 @@ router.put('/:id/cart', checkUser, async (req, res, next) => { orderData: req.body.products, completed: !!req.body.completed }) + // OB/LM: consider moving the innards of the POST logic below to here (detect if a new cart needs to be made and make it) res.sendStatus(201) } catch (error) { + // OB/LM: firewood here, burn it console.log('PUT CART ERROR') next(error) } @@ -93,8 +96,10 @@ router.post('/:id/cart', checkUser, async (req, res, next) => { // POST /api/users/guestCheckout // saves a completed guest order in DB with userId null +// OB/LM: could be a POST /api/orders instead (more RESTful) router.post('/guestCheckout', async (req, res, next) => { try { + // OB/LM: web security issue, client can define the price of the order, maybe you want a method to calculate it await Order.create(req.body) res.sendStatus(201) } catch (error) { diff --git a/server/db/models/order.js b/server/db/models/order.js index 483b62c..fbf112c 100644 --- a/server/db/models/order.js +++ b/server/db/models/order.js @@ -26,6 +26,7 @@ const Order = db.define( }, { hooks: { + // OB/LM: recommend a setter method instead of a hook beforeValidate: function(order) { if (typeof order.orderData !== 'string') { order.orderData = JSON.stringify(order.orderData)