Skip to content

Commit

Permalink
auth.duck.js: login should wait currentUser entity to be loaded befor…
Browse files Browse the repository at this point in the history
…e calling loginSuccess

Private marketplace affected routes should be able to load data after auth redirection.
  • Loading branch information
Gnito committed Aug 19, 2024
1 parent e5154d5 commit 0b8c826
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 0b8c826

Please sign in to comment.