From d087a3e1faa0090a7dd5f4da64abf0d34d9bc742 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 9 May 2017 09:32:12 +0300 Subject: [PATCH 1/6] Remove currentUserHasListings call from Inbox duck --- src/containers/InboxPage/InboxPage.duck.js | 1 - src/containers/InboxPage/InboxPage.test.js | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/containers/InboxPage/InboxPage.duck.js b/src/containers/InboxPage/InboxPage.duck.js index 666c1cd8..855ed09b 100644 --- a/src/containers/InboxPage/InboxPage.duck.js +++ b/src/containers/InboxPage/InboxPage.duck.js @@ -138,7 +138,6 @@ export const loadData = (params, search) => } dispatch(fetchOrdersOrSalesRequest()); - dispatch(fetchCurrentUserHasListings()); const { page = 1 } = parse(search); diff --git a/src/containers/InboxPage/InboxPage.test.js b/src/containers/InboxPage/InboxPage.test.js index 966cc17e..e593f659 100644 --- a/src/containers/InboxPage/InboxPage.test.js +++ b/src/containers/InboxPage/InboxPage.test.js @@ -33,7 +33,6 @@ describe('InboxPage', () => { lastTransitionedAt: new Date(Date.UTC(2016, 0, 15)), }), ], - currentUserHasListings: true, intl: fakeIntl, }; @@ -67,7 +66,6 @@ describe('InboxPage', () => { lastTransitionedAt: new Date(Date.UTC(2016, 0, 15)), }), ], - currentUserHasListings: true, intl: fakeIntl, }; From 02a478f76dffae0fd3c0737d4a1ec999876cfd2f Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 9 May 2017 09:35:26 +0300 Subject: [PATCH 2/6] Move fetchCurrentUserHasListings to the user duck --- src/containers/InboxPage/InboxPage.duck.js | 53 ---------------------- src/ducks/user.duck.js | 52 +++++++++++++++++++++ 2 files changed, 52 insertions(+), 53 deletions(-) diff --git a/src/containers/InboxPage/InboxPage.duck.js b/src/containers/InboxPage/InboxPage.duck.js index 855ed09b..f27f5e85 100644 --- a/src/containers/InboxPage/InboxPage.duck.js +++ b/src/containers/InboxPage/InboxPage.duck.js @@ -1,7 +1,6 @@ import { reverse, sortBy } from 'lodash'; import { parse } from '../../util/urlHelpers'; import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck'; -import { fetchCurrentUser } from '../../ducks/user.duck'; const sortedTransactions = txs => reverse( @@ -16,10 +15,6 @@ export const FETCH_ORDERS_OR_SALES_REQUEST = 'app/InboxPage/FETCH_ORDERS_OR_SALE export const FETCH_ORDERS_OR_SALES_SUCCESS = 'app/InboxPage/FETCH_ORDERS_OR_SALES_SUCCESS'; export const FETCH_ORDERS_OR_SALES_ERROR = 'app/InboxPage/FETCH_ORDERS_OR_SALES_ERROR'; -export const FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST'; -export const FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS'; -export const FETCH_CURRENT_USER_HAS_LISTINGS_ERROR = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_ERROR'; - // ================ Reducer ================ // const entityRefs = entities => @@ -33,8 +28,6 @@ const initialState = { fetchOrdersOrSalesError: null, pagination: null, transactionRefs: [], - currentUserHasListingsError: null, - currentUserHasListings: false, }; export default function checkoutPageReducer(state = initialState, action = {}) { @@ -55,14 +48,6 @@ export default function checkoutPageReducer(state = initialState, action = {}) { console.error(payload); // eslint-disable-line return { ...state, fetchInProgress: false, fetchOrdersOrSalesError: payload }; - case FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST: - return { ...state, currentUserHasListingsError: null }; - case FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS: - return { ...state, currentUserHasListings: payload.hasListings }; - case FETCH_CURRENT_USER_HAS_LISTINGS_ERROR: - console.error(payload); // eslint-disable-line - return { ...state, currentUserHasListingsError: payload }; - default: return state; } @@ -81,46 +66,8 @@ const fetchOrdersOrSalesError = e => ({ payload: e, }); -const fetchCurrentUserHasListingsRequest = () => ({ - type: FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST, -}); - -const fetchCurrentUserHasListingsSuccess = hasListings => ({ - type: FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS, - payload: { hasListings }, -}); - -const fetchCurrentUserHasListingsError = e => ({ - type: FETCH_CURRENT_USER_HAS_LISTINGS_ERROR, - error: true, - payload: e, -}); - // ================ Thunks ================ // -const fetchCurrentUserHasListings = () => - (dispatch, getState, sdk) => { - dispatch(fetchCurrentUserHasListingsRequest()); - dispatch(fetchCurrentUser()) - .then(() => { - const currentUserId = getState().user.currentUser.id; - const params = { - author_id: currentUserId, - - // Since we are only interested in if the user has - // listings, we only need at most one result. - page: 1, - per_page: 1, - }; - return sdk.listings.query(params); - }) - .then(response => { - const hasListings = response.data.data && response.data.data.length > 0; - dispatch(fetchCurrentUserHasListingsSuccess(hasListings)); - }) - .catch(e => dispatch(fetchCurrentUserHasListingsError(e))); - }; - const INBOX_PAGE_SIZE = 10; export const loadData = (params, search) => diff --git a/src/ducks/user.duck.js b/src/ducks/user.duck.js index 9827e11e..79e46df3 100644 --- a/src/ducks/user.duck.js +++ b/src/ducks/user.duck.js @@ -10,11 +10,17 @@ export const STRIPE_ACCOUNT_CREATE_REQUEST = 'app/user/STRIPE_ACCOUNT_CREATE_REQ export const STRIPE_ACCOUNT_CREATE_SUCCESS = 'app/user/STRIPE_ACCOUNT_CREATE_SUCCESS'; export const STRIPE_ACCOUNT_CREATE_ERROR = 'app/user/STRIPE_ACCOUNT_CREATE_ERROR'; +export const FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST'; +export const FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS'; +export const FETCH_CURRENT_USER_HAS_LISTINGS_ERROR = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_ERROR'; + // ================ Reducer ================ // const initialState = { currentUser: null, usersMeError: null, + currentUserHasListingsError: null, + currentUserHasListings: false, }; export default function reducer(state = initialState, action = {}) { @@ -32,6 +38,14 @@ export default function reducer(state = initialState, action = {}) { case CLEAR_CURRENT_USER: return { ...state, currentUser: null, usersMeError: null }; + case FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST: + return { ...state, currentUserHasListingsError: null }; + case FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS: + return { ...state, currentUserHasListings: payload.hasListings }; + case FETCH_CURRENT_USER_HAS_LISTINGS_ERROR: + console.error(payload); // eslint-disable-line + return { ...state, currentUserHasListingsError: payload }; + default: return state; } @@ -67,6 +81,21 @@ export const stripeAccountCreateError = e => ({ error: true, }); +const fetchCurrentUserHasListingsRequest = () => ({ + type: FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST, +}); + +const fetchCurrentUserHasListingsSuccess = hasListings => ({ + type: FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS, + payload: { hasListings }, +}); + +const fetchCurrentUserHasListingsError = e => ({ + type: FETCH_CURRENT_USER_HAS_LISTINGS_ERROR, + error: true, + payload: e, +}); + // ================ Thunks ================ // export const fetchCurrentUser = () => @@ -90,6 +119,29 @@ export const fetchCurrentUser = () => }); }; +export const fetchCurrentUserHasListings = () => + (dispatch, getState, sdk) => { + dispatch(fetchCurrentUserHasListingsRequest()); + dispatch(fetchCurrentUser()) + .then(() => { + const currentUserId = getState().user.currentUser.id; + const params = { + author_id: currentUserId, + + // Since we are only interested in if the user has + // listings, we only need at most one result. + page: 1, + per_page: 1, + }; + return sdk.listings.query(params); + }) + .then(response => { + const hasListings = response.data.data && response.data.data.length > 0; + dispatch(fetchCurrentUserHasListingsSuccess(hasListings)); + }) + .catch(e => dispatch(fetchCurrentUserHasListingsError(e))); + }; + export const createStripeAccount = (bankAccountToken, address) => (dispatch, getState, sdk) => { dispatch(stripeAccountCreateRequest()); From 841c00a4aa1beddf0244c6a9d2841a5b6b34741e Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 9 May 2017 10:08:48 +0300 Subject: [PATCH 3/6] Keep currentUserHasListings in sync - Update when current user is fetched - Update when a new listing is created --- .../EditListingPage/EditListingPage.duck.js | 9 ++- src/ducks/user.duck.js | 74 ++++++++++++------- 2 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/containers/EditListingPage/EditListingPage.duck.js b/src/containers/EditListingPage/EditListingPage.duck.js index 877317cb..9201a032 100644 --- a/src/containers/EditListingPage/EditListingPage.duck.js +++ b/src/containers/EditListingPage/EditListingPage.duck.js @@ -1,5 +1,5 @@ import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck'; -import { createStripeAccount } from '../../ducks/user.duck'; +import { createStripeAccount, fetchCurrentUser } from '../../ducks/user.duck'; const requestAction = actionType => params => ({ type: actionType, payload: { params } }); @@ -162,6 +162,13 @@ export function requestCreateListing(data) { dispatch(requestShowListing({ id, include: ['author', 'images'] })); return response; }) + .then(response => { + // We must update the user duck since this might be the first + // listing for the user, therefore changing the + // currentUserHasListingsFlag in the store. + dispatch(fetchCurrentUser()); + return response; + }) .catch(e => dispatch(createListingError(e))); }; } diff --git a/src/ducks/user.duck.js b/src/ducks/user.duck.js index 79e46df3..5a3fb1e4 100644 --- a/src/ducks/user.duck.js +++ b/src/ducks/user.duck.js @@ -10,17 +10,17 @@ export const STRIPE_ACCOUNT_CREATE_REQUEST = 'app/user/STRIPE_ACCOUNT_CREATE_REQ export const STRIPE_ACCOUNT_CREATE_SUCCESS = 'app/user/STRIPE_ACCOUNT_CREATE_SUCCESS'; export const STRIPE_ACCOUNT_CREATE_ERROR = 'app/user/STRIPE_ACCOUNT_CREATE_ERROR'; -export const FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST'; -export const FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS'; -export const FETCH_CURRENT_USER_HAS_LISTINGS_ERROR = 'app/InboxPage/FETCH_CURRENT_USER_HAS_LISTINGS_ERROR'; +export const FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST = 'app/user/FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST'; +export const FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS = 'app/user/FETCH_CURRENT_USER_HAS_LISTINGS_SUCCESS'; +export const FETCH_CURRENT_USER_HAS_LISTINGS_ERROR = 'app/user/FETCH_CURRENT_USER_HAS_LISTINGS_ERROR'; // ================ Reducer ================ // const initialState = { currentUser: null, usersMeError: null, - currentUserHasListingsError: null, currentUserHasListings: false, + currentUserHasListingsError: null, }; export default function reducer(state = initialState, action = {}) { @@ -36,7 +36,13 @@ export default function reducer(state = initialState, action = {}) { return { ...state, usersMeError: payload }; case CLEAR_CURRENT_USER: - return { ...state, currentUser: null, usersMeError: null }; + return { + ...state, + currentUser: null, + usersMeError: null, + currentUserHasListings: false, + currentUserHasListingsError: null, + }; case FETCH_CURRENT_USER_HAS_LISTINGS_REQUEST: return { ...state, currentUserHasListingsError: null }; @@ -98,6 +104,38 @@ const fetchCurrentUserHasListingsError = e => ({ // ================ Thunks ================ // +export const fetchCurrentUserHasListings = () => + (dispatch, getState, sdk) => { + dispatch(fetchCurrentUserHasListingsRequest()); + const { currentUser } = getState().user; + + if (!currentUser) { + dispatch(fetchCurrentUserHasListingsSuccess(false)); + return Promise.resolve(null); + } + + const currentUserId = currentUser.id; + const params = { + author_id: currentUserId, + + // Since we are only interested in if the user has + // listings, we only need at most one result. + page: 1, + per_page: 1, + }; + + return sdk.listings + .query(params) + .then(response => { + const hasListings = response.data.data && response.data.data.length > 0; + dispatch(fetchCurrentUserHasListingsSuccess(!!hasListings)); + }) + .catch(e => { + // TODO: dispatch flash message + dispatch(fetchCurrentUserHasListingsError(e)); + }); + }; + export const fetchCurrentUser = () => (dispatch, getState, sdk) => { dispatch(usersMeRequest()); @@ -113,35 +151,15 @@ export const fetchCurrentUser = () => .then(response => { dispatch(usersMeSuccess(response.data.data)); }) + .then(() => { + dispatch(fetchCurrentUserHasListings()); + }) .catch(e => { // TODO: dispatch flash message dispatch(usersMeError(e)); }); }; -export const fetchCurrentUserHasListings = () => - (dispatch, getState, sdk) => { - dispatch(fetchCurrentUserHasListingsRequest()); - dispatch(fetchCurrentUser()) - .then(() => { - const currentUserId = getState().user.currentUser.id; - const params = { - author_id: currentUserId, - - // Since we are only interested in if the user has - // listings, we only need at most one result. - page: 1, - per_page: 1, - }; - return sdk.listings.query(params); - }) - .then(response => { - const hasListings = response.data.data && response.data.data.length > 0; - dispatch(fetchCurrentUserHasListingsSuccess(hasListings)); - }) - .catch(e => dispatch(fetchCurrentUserHasListingsError(e))); - }; - export const createStripeAccount = (bankAccountToken, address) => (dispatch, getState, sdk) => { dispatch(stripeAccountCreateRequest()); From 4337493cad22caea0e491e6435a623af86164678 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 9 May 2017 10:17:32 +0300 Subject: [PATCH 4/6] Link to sales/orders in Topbar modal based on users listings If the current user has created listings, always link to sales, otherwise to orders. --- src/components/MobileMenu/MobileMenu.js | 13 +++++++++---- src/containers/Topbar/Topbar.js | 12 ++++++++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/components/MobileMenu/MobileMenu.js b/src/components/MobileMenu/MobileMenu.js index 88c5d894..1d729a7c 100644 --- a/src/components/MobileMenu/MobileMenu.js +++ b/src/components/MobileMenu/MobileMenu.js @@ -9,7 +9,7 @@ import { Avatar, InlineButton, NamedLink } from '../../components'; import css from './MobileMenu.css'; const MobileMenu = props => { - const { isAuthenticated, name, onLogout } = props; + const { isAuthenticated, currentUserHasListings, name, onLogout } = props; if (!isAuthenticated) { return ( @@ -31,6 +31,12 @@ const MobileMenu = props => { ); } + const inboxLink = ( + + + + ); + return (
@@ -43,9 +49,7 @@ const MobileMenu = props => {
- - - + {inboxLink}
@@ -62,6 +66,7 @@ const { bool, func, string } = PropTypes; MobileMenu.propTypes = { isAuthenticated: bool.isRequired, + currentUserHasListings: bool.isRequired, name: string, onLogout: func.isRequired, }; diff --git a/src/containers/Topbar/Topbar.js b/src/containers/Topbar/Topbar.js index 050e18df..ba0c0c25 100644 --- a/src/containers/Topbar/Topbar.js +++ b/src/containers/Topbar/Topbar.js @@ -84,6 +84,7 @@ class TopbarComponent extends Component { isAuthenticated, authInProgress, currentUser, + currentUserHasListings, intl, location, togglePageClassNames, @@ -100,7 +101,12 @@ class TopbarComponent extends Component { const isMobileMenuOpen = mobilemenu === 'open'; const isMobileSearchOpen = mobilesearch === 'open'; const mobileMenu = ( - + ); // Only render current search if full place object is available in the URL params @@ -161,6 +167,7 @@ TopbarComponent.propTypes = { isAuthenticated: bool.isRequired, authInProgress: bool.isRequired, currentUser: propTypes.currentUser, + currentUserHasListings: bool.isRequired, onLogout: func.isRequired, togglePageClassNames: func.isRequired, @@ -181,11 +188,12 @@ TopbarComponent.propTypes = { const mapStateToProps = state => { const { isAuthenticated } = state.Auth; - const { currentUser } = state.user; + const { currentUser, currentUserHasListings } = state.user; return { isAuthenticated, authInProgress: authenticationInProgress(state), currentUser, + currentUserHasListings, }; }; From a9e8ecf05265cb7500e7f236679579e73cd10232 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 9 May 2017 10:26:16 +0300 Subject: [PATCH 5/6] Make inbox tab names configurable in routesConfiguration --- src/components/MobileMenu/MobileMenu.js | 2 +- src/containers/OrderPage/OrderPage.js | 2 +- src/containers/SalePage/SalePage.js | 12 ++++++++++-- src/routesConfiguration.js | 16 ++++++++++++---- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/components/MobileMenu/MobileMenu.js b/src/components/MobileMenu/MobileMenu.js index 1d729a7c..ddac8ba8 100644 --- a/src/components/MobileMenu/MobileMenu.js +++ b/src/components/MobileMenu/MobileMenu.js @@ -32,7 +32,7 @@ const MobileMenu = props => { } const inboxLink = ( - + ); diff --git a/src/containers/OrderPage/OrderPage.js b/src/containers/OrderPage/OrderPage.js index 6df2a3e5..51ad52d3 100644 --- a/src/containers/OrderPage/OrderPage.js +++ b/src/containers/OrderPage/OrderPage.js @@ -28,7 +28,7 @@ export const OrderPageComponent = props => { if (isDataAvailable && !isOwnSale) { // eslint-disable-next-line no-console console.error('Tried to access an order that was not owned by the current user'); - return ; + return ; } const detailsProps = { diff --git a/src/containers/SalePage/SalePage.js b/src/containers/SalePage/SalePage.js index 87632ab5..7b054555 100644 --- a/src/containers/SalePage/SalePage.js +++ b/src/containers/SalePage/SalePage.js @@ -14,7 +14,15 @@ import css from './SalePage.css'; // SalePage handles data loading // It show loading data text or SaleDetailsPanel (and later also another panel for messages). export const SalePageComponent = props => { - const { currentUser, fetchSaleError, intl, onAcceptSale, onRejectSale, params, transaction } = props; + const { + currentUser, + fetchSaleError, + intl, + onAcceptSale, + onRejectSale, + params, + transaction, + } = props; const currentTransaction = ensureTransaction(transaction); const currentListing = ensureListing(currentTransaction.listing); const title = currentListing.attributes.title; @@ -29,7 +37,7 @@ export const SalePageComponent = props => { if (isDataAvailable && !isOwnSale) { // eslint-disable-next-line no-console console.error('Tried to access a sale that was not owned by the current user'); - return ; + return ; } const detailsProps = { diff --git a/src/routesConfiguration.js b/src/routesConfiguration.js index 4892fb0a..c60fb02b 100644 --- a/src/routesConfiguration.js +++ b/src/routesConfiguration.js @@ -158,14 +158,22 @@ const routesConfiguration = [ auth: true, exact: true, name: 'InboxBasePage', - component: () => , + component: () => , }, { - path: '/inbox/:tab', + path: '/inbox/sales', auth: true, exact: true, - name: 'InboxPage', - component: props => , + name: 'InboxSalesPage', + component: props => , + loadData: (params, search) => InboxPage.loadData(params, search), + }, + { + path: '/inbox/orders', + auth: true, + exact: true, + name: 'InboxOrdersPage', + component: props => , loadData: (params, search) => InboxPage.loadData(params, search), }, { From 3e59adfc9a63748ef6dcc6b9c5fa40aba703dfe7 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 9 May 2017 15:02:54 +0300 Subject: [PATCH 6/6] Avoid extra request and set hasListings flag directly --- src/containers/EditListingPage/EditListingPage.duck.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/containers/EditListingPage/EditListingPage.duck.js b/src/containers/EditListingPage/EditListingPage.duck.js index 9201a032..60c39e84 100644 --- a/src/containers/EditListingPage/EditListingPage.duck.js +++ b/src/containers/EditListingPage/EditListingPage.duck.js @@ -1,5 +1,5 @@ import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck'; -import { createStripeAccount, fetchCurrentUser } from '../../ducks/user.duck'; +import { createStripeAccount, fetchCurrentUserHasListingsSuccess } from '../../ducks/user.duck'; const requestAction = actionType => params => ({ type: actionType, payload: { params } }); @@ -165,8 +165,8 @@ export function requestCreateListing(data) { .then(response => { // We must update the user duck since this might be the first // listing for the user, therefore changing the - // currentUserHasListingsFlag in the store. - dispatch(fetchCurrentUser()); + // currentUserHasListings flag in the store. + dispatch(fetchCurrentUserHasListingsSuccess({ hasListings: true })); return response; }) .catch(e => dispatch(createListingError(e)));