Skip to content

Commit

Permalink
Original url destinations like experiment detail page will not redire…
Browse files Browse the repository at this point in the history
…ct to home after login (#1143)

* ensure redirect url is set to the original url so post-login will carry you to your original destination when clicking on a shared link

* remove console.log

* remove console.log

* add logic to check login as destination, improve specs
  • Loading branch information
danoswaltCL authored Nov 21, 2023
1 parent d23c2df commit be88797
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 3 deletions.
15 changes: 15 additions & 0 deletions frontend/projects/upgrade/src/app/core/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,28 @@ export class AuthService {

initializeUserSession(): void {
const currentUser = this.getUserFromBrowserStorage();
this.determinePostLoginDestinationUrl();

if (currentUser) {
this.handleAutomaticLogin(currentUser);
} else {
this.authLogout();
}
}

/**
* determinePostLoginDestinationUrl
*
* - navigate to /home if user started from login or if no path is present
* - otherwise, navigate to the path they started from after google login does its thing
*/

determinePostLoginDestinationUrl(): void {
const originalDestinationUrl = window.location.pathname?.endsWith('login') ? 'home' : window.location.pathname;

this.setRedirectionUrl(originalDestinationUrl);
}

initializeGoogleSignInButton(btnRef: ElementRef): void {
this.initializeGoogleSignIn();
this.renderGoogleSignInButton(btnRef);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface UserPermission {
export interface AuthState {
isLoggedIn: boolean;
isAuthenticating: boolean;
redirectUrl?: string;
redirectUrl: string;
user: User;
googleCredential: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const initialState: AuthState = {
isAuthenticating: false,
user: null,
googleCredential: null,
redirectUrl: '/home',
};

const reducer = createReducer(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createReducer } from '@ngrx/store';
import { actionLogoutSuccess } from '../auth/store/auth.actions';
import { actionLogoutSuccess, actionLoginStart } from '../auth/store/auth.actions';
import { LocalStorageService } from '../local-storage/local-storage.service';
import { ThemeOptions } from '../settings/store/settings.model';
import { clearState } from './clear-state.reducer';
Expand All @@ -10,13 +10,16 @@ describe('clearState', () => {
settings: {
theme: ThemeOptions.DARK_THEME,
},
auth: {
redirectUrl: '/test',
},
};

beforeEach(() => {
LocalStorageService.prototype.removeItem = jest.fn();
});

it('should reset settings state and clear localStorage settings', () => {
it('should reset settings and auth state except for theme and redirectUrl on actionLogoutSuccess, and also clear localStorage settings', () => {
const reducer = createReducer(mockState);
const metaReducer = clearState(reducer);
const expectedResetState = {
Expand All @@ -25,6 +28,13 @@ describe('clearState', () => {
toCheckAuth: null,
toFilterMetric: null,
},
auth: {
isLoggedIn: false,
isAuthenticating: false,
user: null,
googleCredential: null,
redirectUrl: '/test',
},
};

const newState = metaReducer(mockState, actionLogoutSuccess());
Expand All @@ -34,4 +44,37 @@ describe('clearState', () => {
);
expect(newState).toEqual(expectedResetState);
});

it('should NOT reset anything or clear localStorage if action is anything but actionLogoutSuccess', () => {
const mockWithNonDefaultState = {
...mockState,
auth: {
...mockState.auth,
isLoggedIn: true,
},
};
const reducer = createReducer(mockState);
const metaReducer = clearState(reducer);
const expectedResetState = {
settings: {
theme: ThemeOptions.DARK_THEME,
toCheckAuth: null,
toFilterMetric: null,
},
auth: {
isLoggedIn: true,
isAuthenticating: false,
user: null,
googleCredential: null,
redirectUrl: '/test',
},
};

const newState = metaReducer(mockWithNonDefaultState, actionLoginStart());

expect(LocalStorageService.prototype.removeItem).not.toHaveBeenCalledTimes(
Object.keys(ExperimentLocalStorageKeys).length
);
expect(newState).not.toEqual(expectedResetState);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as AuthActions from '../auth/store/auth.actions';
import { ExperimentLocalStorageKeys } from '../experiments/store/experiments.model';
import { LocalStorageService } from '../local-storage/local-storage.service';
import { SettingsState } from '../settings/store/settings.model';
import { AuthState } from '../auth/store/auth.models';

export function clearState(reducer) {
return (state, action: Action) => {
Expand All @@ -12,10 +13,18 @@ export function clearState(reducer) {
toCheckAuth: null,
toFilterMetric: null,
};
const authState: AuthState = {
isLoggedIn: false,
isAuthenticating: false,
user: null,
googleCredential: null,
redirectUrl: state.auth.redirectUrl,
};
const localStorageService = new LocalStorageService();

state = {
settings: settingState, // Used to persist theme,
auth: authState, // Used to persist redirectUrl
};

Object.values(ExperimentLocalStorageKeys).forEach((key) => {
Expand Down

0 comments on commit be88797

Please sign in to comment.