Skip to content

Commit

Permalink
(fix) Require fewer requests to the session endpoint (openmrs#923)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ibacher
ibacher authored Feb 21, 2024
1 parent e542382 commit ed0b94f
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 167 deletions.
2 changes: 1 addition & 1 deletion e2e/pages/login-page.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Page } from '@playwright/test';
import { type Page } from '@playwright/test';

export class LoginPage {
constructor(readonly page: Page) {}
Expand Down
2 changes: 1 addition & 1 deletion e2e/support/github/run-e2e-docker-env.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
# create a temporary working directory
working_dir=$(mktemp -d "${TMPDIR:-/tmp/}openmrs-e2e-frontends.XXXXXXXXXX")
# get a list of all the apps in this workspace
apps=$(yarn workspaces list --json | jq -r 'if ((.location == ".") or (.location | test("-app") | not)) then halt else .name end')
apps=$(yarn workspaces list --json | jq -r 'if .location | test("-app$") then .name else empty end')
# this array will hold all of the packed app names
app_names=()

Expand Down
26 changes: 7 additions & 19 deletions packages/apps/esm-login-app/src/login.resource.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,9 @@ import { useMemo } from 'react';
import { useTranslation } from 'react-i18next';
import useSwrInfinite from 'swr/infinite';
import useSwrImmutable from 'swr/immutable';
import type { FetchResponse, Session } from '@openmrs/esm-framework';
import { fhirBaseUrl, openmrsFetch, refetchCurrentUser, showNotification, useDebounce } from '@openmrs/esm-framework';
import { type FetchResponse, fhirBaseUrl, openmrsFetch, useDebounce, showNotification } from '@openmrs/esm-framework';
import type { LocationEntry, LocationResponse } from './types';

export async function performLogin(username: string, password: string): Promise<{ data: Session }> {
const abortController = new AbortController();
const token = window.btoa(`${username}:${password}`);
const url = `/ws/rest/v1/session`;

return openmrsFetch(url, {
headers: {
Authorization: `Basic ${token}`,
},
signal: abortController.signal,
}).then((res) => {
refetchCurrentUser();
return res;
});
}

interface LoginLocationData {
locations: Array<LocationEntry>;
isLoading: boolean;
Expand All @@ -38,6 +21,7 @@ export function useLoginLocations(
): LoginLocationData {
const { t } = useTranslation();
const debouncedSearchQuery = useDebounce(searchQuery);

function constructUrl(page: number, prevPageData: FetchResponse<LocationResponse>) {
if (prevPageData) {
const nextLink = prevPageData.data?.link?.find((link) => link.relation === 'next');
Expand Down Expand Up @@ -104,6 +88,7 @@ export function useLoginLocations(
hasMore: data?.length ? data?.[data.length - 1]?.data?.link?.some((link) => link.relation === 'next') : false,
loadingNewData: isValidating,
setPage: setSize,
error,
};
}, [isLoading, data, isValidating, setSize]);

Expand All @@ -117,17 +102,20 @@ export function useValidateLocationUuid(userPreferredLocationUuid: string) {
if (err?.response?.status) {
return err.response.status >= 500;
}

return false;
},
});

const results = useMemo(
() => ({
isLocationValid: data?.ok && data?.data?.total > 0,
isLocationValid: data?.ok,
defaultLocation: data?.data?.entry ?? [],
error,
isLoading,
}),
[data, isLoading, error],
);

return results;
}
177 changes: 89 additions & 88 deletions packages/apps/esm-login-app/src/login/login.component.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,43 +1,47 @@
import React, { useState, useRef, useEffect, useCallback } from 'react';
import { useLocation, useNavigate } from 'react-router-dom';
import { type To, useLocation, useNavigate } from 'react-router-dom';
import { useTranslation } from 'react-i18next';
import { Button, InlineLoading, InlineNotification, PasswordInput, TextInput, Tile } from '@carbon/react';
import { ArrowLeft, ArrowRight } from '@carbon/react/icons';
import { useTranslation } from 'react-i18next';
import type { Session } from '@openmrs/esm-framework';
import {
useConfig,
interpolateUrl,
useSession,
refetchCurrentUser,
clearCurrentUser,
getSessionStore,
useConnectivity,
navigate as openmrsNavigate,
} from '@openmrs/esm-framework';
import { performLogin } from '../login.resource';
import { type ConfigSchema } from '../config-schema';
import styles from './login.scss';

export interface LoginReferrer {
referrer?: string;
}

const hidden: React.CSSProperties = {
height: 0,
width: 0,
border: 0,
padding: 0,
};

export interface LoginReferrer {
referrer?: string;
}

export interface LoginProps extends LoginReferrer {}

const Login: React.FC<LoginProps> = () => {
const config = useConfig();
const { showPasswordOnSeparateScreen } = config;
const Login: React.FC = () => {
const { showPasswordOnSeparateScreen, provider: loginProvider, links: loginLinks } = useConfig<ConfigSchema>();
const isLoginEnabled = useConnectivity();
const { t } = useTranslation();
const { user } = useSession();
const location = useLocation();
const navigate = useNavigate();
const location = useLocation() as unknown as Omit<Location, 'state'> & {
state: LoginReferrer;
};

const rawNavigate = useNavigate();
const navigate = useCallback(
(to: To) => {
rawNavigate(to, { state: location.state });
},
[rawNavigate, location.state],
);

const [username, setUsername] = useState('');
const [password, setPassword] = useState('');
const [errorMessage, setErrorMessage] = useState('');
Expand All @@ -49,71 +53,37 @@ const Login: React.FC<LoginProps> = () => {
const showUsername = location.pathname === '/login';
const showPassword = !showPasswordOnSeparateScreen || location.pathname === '/login/confirm';

const handleLogin = useCallback(
(session: Session) => {
if (session.sessionLocation) {
openmrsNavigate({
to: location?.state?.referrer ? `\${openmrsSpaBase}${location.state.referrer}` : config?.links?.loginSuccess,
});
} else {
navigate('/login/location', { state: location.state });
}
},
[config?.links?.loginSuccess, location.state, navigate],
);

const handleUpdateSessionStore = useCallback(() => {
refetchCurrentUser().then(() => {
const authenticated = getSessionStore().getState().session.authenticated;
if (authenticated) {
handleLogin(getSessionStore().getState().session);
}
});
}, [handleLogin]);

useEffect(() => {
if (user) {
clearCurrentUser();
handleUpdateSessionStore();
} else if (!username && location.pathname === '/login/confirm') {
navigate('/login', { state: location.state });
if (!user) {
if (loginProvider.type === 'oauth2') {
openmrsNavigate({ to: loginProvider.loginUrl });
} else if (!username && location.pathname === '/login/confirm') {
navigate('/login');
}
}
}, [username, navigate, location, user, handleLogin, handleUpdateSessionStore]);
}, [username, navigate, location, user, loginProvider]);

useEffect(() => {
const fieldToFocus =
showPasswordOnSeparateScreen && showPassword ? passwordInputRef.current : usernameInputRef.current;

if (fieldToFocus) {
fieldToFocus.focus();
}
fieldToFocus?.focus();
}, [showPassword, showPasswordOnSeparateScreen]);

useEffect(() => {
if (!user && config.provider.type === 'oauth2') {
openmrsNavigate({ to: config.provider.loginUrl });
}
}, [config, user]);

const continueLogin = useCallback(() => {
const field = usernameInputRef.current;
const usernameField = usernameInputRef.current;

if (field.value.length > 0) {
navigate('/login/confirm', { state: location.state });
if (usernameField.value && usernameField.value.trim()) {
navigate('/login/confirm');
} else {
field.focus();
usernameField.focus();
}
}, [location.state, navigate]);

const changeUsername = useCallback((evt: React.ChangeEvent<HTMLInputElement>) => setUsername(evt.target.value), []);

const changePassword = useCallback((evt: React.ChangeEvent<HTMLInputElement>) => setPassword(evt.target.value), []);

const resetUserNameAndPassword = useCallback(() => {
setUsername('');
setPassword('');
}, []);

const handleSubmit = useCallback(
async (evt: React.FormEvent<HTMLFormElement>) => {
evt.preventDefault();
Expand All @@ -122,52 +92,69 @@ const Login: React.FC<LoginProps> = () => {
if (!showPassword) {
continueLogin();
return false;
} else if (!password || !password.trim()) {
passwordInputRef.current.focus();
return false;
}

try {
setIsLoggingIn(true);
const loginRes = await performLogin(username, password);
const authData = loginRes.data;
const valid = authData && authData.authenticated;

if (valid) {
handleUpdateSessionStore();
const sessionStore = await refetchCurrentUser(username, password);
const session = sessionStore.session;
const authenticated = sessionStore?.session?.authenticated;
if (authenticated) {
if (session.sessionLocation) {
let to = loginLinks?.loginSuccess || '/home';
if (location?.state?.referrer) {
if (location.state.referrer.startsWith('/')) {
to = `\${openmrsSpaBase}${location.state.referrer}`;
} else {
to = location.state.referrer;
}
}

openmrsNavigate({ to });
} else {
navigate('/login/location');
}
} else {
throw new Error('invalidCredentials');
setErrorMessage(t('invalidCredentials', 'Invalid username or password'));

setUsername('');
setPassword('');
navigate('/login');
}
} catch (error) {

return true;
} catch (error: unknown) {
if (error instanceof Error) {
setErrorMessage(error.message);
} else {
setErrorMessage(t('invalidCredentials', 'Invalid username or password'));
}

setUsername('');
setPassword('');
navigate('/login');
} finally {
setIsLoggingIn(false);
setErrorMessage(error.message);
resetUserNameAndPassword();
}

return false;
},

[showPassword, continueLogin, username, password, handleUpdateSessionStore, resetUserNameAndPassword],
[showPassword, username, password, navigate],
);

const logo = config.logo.src ? (
<img src={interpolateUrl(config.logo.src)} alt={config.logo.alt} className={styles['logo-img']} />
) : (
<svg role="img" className={styles['logo']}>
<title>OpenMRS logo</title>
<use xlinkHref="#omrs-logo-full-color"></use>
</svg>
);

if (config.provider.type === 'basic') {
if (!loginProvider || loginProvider.type === 'basic') {
return (
<div className={styles.container}>
<Tile className={styles['login-card']}>
{errorMessage && (
<div className={styles.errorMessage}>
<InlineNotification
kind="error"
/**
* This comment tells i18n to still keep the following translation keys (used as value for: errorMessage):
* t('invalidCredentials')
*/
subtitle={t(errorMessage)}
title={t('error', 'Error')}
onClick={() => setErrorMessage('')}
Expand All @@ -187,7 +174,9 @@ const Login: React.FC<LoginProps> = () => {
</Button>
</div>
) : null}
<div className={styles['center']}>{logo}</div>
<div className={styles['center']}>
<Logo />
</div>
<form onSubmit={handleSubmit} ref={formRef}>
{showUsername && (
<div className={styles['input-group']}>
Expand Down Expand Up @@ -284,4 +273,16 @@ const Login: React.FC<LoginProps> = () => {
return null;
};

const Logo = () => {
const { logo } = useConfig<ConfigSchema>();
return logo.src ? (
<img src={interpolateUrl(logo.src)} alt={logo.alt ?? 'OpenMRS Logo'} className={styles['logo-img']} />
) : (
<svg role="img" className={styles['logo']}>
<title>OpenMRS logo</title>
<use xlinkHref="#omrs-logo-full-color"></use>
</svg>
);
};

export default Login;
13 changes: 4 additions & 9 deletions packages/apps/esm-login-app/src/login/login.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { useState } from 'react';
import { waitFor, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { useConfig, useSession } from '@openmrs/esm-framework';
import { performLogin } from '../login.resource';
import { refetchCurrentUser, useConfig, useSession } from '@openmrs/esm-framework';
import { mockConfig } from '../../__mocks__/config.mock';
import renderWithRouter from '../test-helpers/render-with-router';
import Login from './login.component';

const mockedLogin = performLogin as jest.Mock;
const mockedLogin = refetchCurrentUser as jest.Mock;
const mockedUseConfig = useConfig as jest.Mock;
const mockedUseSession = useSession as jest.Mock;

Expand All @@ -32,10 +31,6 @@ jest.mock('@openmrs/esm-framework', () => {
};
});

jest.mock('../login.resource', () => ({
performLogin: jest.fn(),
}));

const loginLocations = [
{ uuid: '111', display: 'Earth' },
{ uuid: '222', display: 'Mars' },
Expand Down Expand Up @@ -121,7 +116,7 @@ describe('Login', () => {
await screen.findByLabelText(/password/i);
await user.type(screen.getByLabelText(/password/i), 'no-tax-fraud');
await user.click(loginButton);
await waitFor(() => expect(performLogin).toHaveBeenCalledWith('yoshi', 'no-tax-fraud'));
await waitFor(() => expect(refetchCurrentUser).toHaveBeenCalledWith('yoshi', 'no-tax-fraud'));
});

// TODO: Complete the test
Expand Down Expand Up @@ -230,7 +225,7 @@ describe('Login', () => {
await user.type(passwordInput, 'no-tax-fraud');
await user.click(loginButton);

await waitFor(() => expect(performLogin).toHaveBeenCalledWith('yoshi', 'no-tax-fraud'));
await waitFor(() => expect(refetchCurrentUser).toHaveBeenCalledWith('yoshi', 'no-tax-fraud'));
});

it('should focus the username input', async () => {
Expand Down
Loading

0 comments on commit ed0b94f

Please sign in to comment.