Skip to content

Commit

Permalink
Merge pull request #436 from sharetribe/fix-private-marketplace-login
Browse files Browse the repository at this point in the history
auth.duck.js: login should wait currentUser entity to be loaded
  • Loading branch information
Gnito authored Aug 20, 2024
2 parents e5154d5 + 0b8c826 commit 7c97566
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/ducks/auth.duck.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ export const login = (username, password) => (dispatch, getState, sdk) => {
// just dispatches the login error action.
return sdk
.login({ username, password })
.then(() => dispatch(fetchCurrentUser({ afterLogin: true })))
.then(() => dispatch(loginSuccess()))
.then(() => dispatch(fetchCurrentUser()))
.catch(e => dispatch(loginError(storableError(e))));
};

Expand Down
50 changes: 36 additions & 14 deletions src/ducks/auth.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import * as log from '../util/log';
import { storableError } from '../util/errors';
import { clearCurrentUser, currentUserShowRequest, currentUserShowSuccess } from './user.duck';
import { createCurrentUser } from '../util/testData';
import {
clearCurrentUser,
currentUserShowRequest,
currentUserShowSuccess,
fetchCurrentUserNotificationsRequest,
fetchCurrentUserNotificationsSuccess,
} from './user.duck';
import reducer, {
authenticationInProgress,
authInfoRequest,
authInfoSuccess,
login,
loginRequest,
Expand Down Expand Up @@ -188,7 +196,15 @@ describe('auth duck', () => {
it('should dispatch success and fetch current user', () => {
const initialState = reducer();
const getState = () => ({ auth: initialState });
const sdk = { login: jest.fn(() => Promise.resolve({})) };
const fakeCurrentUser = createCurrentUser({ id: 'test-user' });
const fakeCurrentUserResponse = { data: { data: fakeCurrentUser, include: [] } };
const fakeTransactionsResponse = { data: { data: [], include: [] } };
const sdk = {
login: jest.fn(() => Promise.resolve({})),
authInfo: jest.fn(() => Promise.resolve({})),
currentUser: { show: jest.fn(() => Promise.resolve(fakeCurrentUserResponse)) },
transactions: { query: jest.fn(() => Promise.resolve(fakeTransactionsResponse)) },
};
const dispatch = createFakeDispatch(getState, sdk);
const username = '[email protected]';
const password = 'pass';
Expand All @@ -197,13 +213,13 @@ describe('auth duck', () => {
expect(sdk.login.mock.calls).toEqual([[{ username, password }]]);
expect(dispatchedActions(dispatch)).toEqual([
loginRequest(),
loginSuccess(),
currentUserShowRequest(),

// Test restriction: Since the getState mock always returns
// the initial state, user isn't marked as logged in and
// current user is still null.
currentUserShowSuccess(null),
currentUserShowSuccess(fakeCurrentUser),
fetchCurrentUserNotificationsRequest(),
authInfoRequest(),
fetchCurrentUserNotificationsSuccess([]),
authInfoSuccess({}),
loginSuccess(),
]);
});
});
Expand Down Expand Up @@ -335,11 +351,17 @@ describe('auth duck', () => {

describe('signup thunk', () => {
it('should dispatch success and login', () => {
const fakeCurrentUser = createCurrentUser({ id: 'test-user' });
const fakeCurrentUserResponse = { data: { data: fakeCurrentUser, include: [] } };
const fakeTransactionsResponse = { data: { data: [], include: [] } };
const sdk = {
currentUser: {
create: jest.fn(() => Promise.resolve({})),
show: jest.fn(() => Promise.resolve(fakeCurrentUserResponse)),
},
login: jest.fn(() => Promise.resolve({})),
authInfo: jest.fn(() => Promise.resolve({})),
transactions: { query: jest.fn(() => Promise.resolve(fakeTransactionsResponse)) },
};
const getState = () => ({ auth: state });
const dispatch = createFakeDispatch(getState, sdk);
Expand All @@ -363,13 +385,13 @@ describe('auth duck', () => {
signupRequest(),
signupSuccess(),
loginRequest(),
loginSuccess(),
currentUserShowRequest(),

// Test restriction: Since the getState mock always returns
// the initial state, user isn't marked as logged in and
// current user is still null.
currentUserShowSuccess(null),
currentUserShowSuccess(fakeCurrentUser),
fetchCurrentUserNotificationsRequest(),
authInfoRequest(),
fetchCurrentUserNotificationsSuccess([]),
authInfoSuccess({}),
loginSuccess(),
]);
});
});
Expand Down
7 changes: 4 additions & 3 deletions src/ducks/user.duck.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ const fetchCurrentUserHasListingsError = e => ({
payload: e,
});

const fetchCurrentUserNotificationsRequest = () => ({
export const fetchCurrentUserNotificationsRequest = () => ({
type: FETCH_CURRENT_USER_NOTIFICATIONS_REQUEST,
});

Expand Down Expand Up @@ -323,7 +323,8 @@ export const fetchCurrentUser = options => (dispatch, getState, sdk) => {
const state = getState();
const { currentUserHasListings, currentUserShowTimestamp } = state.user || {};
const { isAuthenticated } = state.auth;
const { callParams = null, updateHasListings = true, updateNotifications = true } = options || {};
const { callParams = null, updateHasListings = true, updateNotifications = true, afterLogin } =
options || {};

// Double fetch might happen when e.g. profile page is making a full page load
const aSecondAgo = new Date().getTime() - 1000;
Expand All @@ -333,7 +334,7 @@ export const fetchCurrentUser = options => (dispatch, getState, sdk) => {
// Set in-progress, no errors
dispatch(currentUserShowRequest());

if (!isAuthenticated) {
if (!isAuthenticated && !afterLogin) {
// Make sure current user is null
dispatch(currentUserShowSuccess(null));
return Promise.resolve({});
Expand Down

0 comments on commit 7c97566

Please sign in to comment.