Skip to content

Commit

Permalink
O3-1242 App crashes on logout (#423)
Browse files Browse the repository at this point in the history
  • Loading branch information
brandones authored May 6, 2022
1 parent 5ec4bc5 commit 1a4a071
Show file tree
Hide file tree
Showing 35 changed files with 607 additions and 405 deletions.
54 changes: 0 additions & 54 deletions packages/apps/esm-login-app/src/CurrentUserContext.tsx

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import {
navigate,
useConfig,
setSessionLocation,
useSession,
} from "@openmrs/esm-framework";
import { useLoginLocations } from "./choose-location.resource";
import { useCurrentUser } from "../CurrentUserContext";
import type { StaticContext } from "react-router";

export interface LoginReferrer {
Expand All @@ -27,7 +27,7 @@ export const ChooseLocation: React.FC<ChooseLocationProps> = ({
const returnToUrl = new URLSearchParams(location?.search).get("returnToUrl");
const referrer = location?.state?.referrer;
const config = useConfig();
const user = useCurrentUser();
const { user } = useSession();
const { locationData, isLoading } = useLoginLocations(
config.chooseLocation.useLoginLocationTag
);
Expand All @@ -39,8 +39,10 @@ export const ChooseLocation: React.FC<ChooseLocationProps> = ({
: Promise.resolve();

sessionDefined.then(() => {
// console.log("referrer: ", referrer);
if (referrer && referrer !== "/") {
if (
referrer &&
!["/", "/login", "/login/location"].includes(referrer)
) {
navigate({ to: "${openmrsSpaBase}" + referrer });
return;
}
Expand All @@ -55,6 +57,8 @@ export const ChooseLocation: React.FC<ChooseLocationProps> = ({
[referrer, config.links.loginSuccess, returnToUrl]
);

// Handle cases where the location picker is disabled, there is only one
// location, or there are no locations.
useEffect(() => {
if (!isLoading) {
if (!config.chooseLocation.enabled || locationData?.length === 1) {
Expand All @@ -72,6 +76,11 @@ export const ChooseLocation: React.FC<ChooseLocationProps> = ({
isLoading,
]);

if (!isLoading && !user) {
navigate({ to: "${openmrsSpaBase}/login" });
return null;
}

if (!isLoading || !isLoginEnabled) {
return (
<LocationPicker
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
openmrsFetch,
fhirBaseUrl,
FetchResponse,
showNotification,
reportError,
} from "@openmrs/esm-framework";
import useSwrInfinite from "swr/infinite";
import { LocationEntry, LocationResponse } from "../types";
Expand Down Expand Up @@ -61,11 +61,8 @@ export function useLoginLocations(
>(getUrl, openmrsFetch);

if (error) {
showNotification({
title: error.name,
description: error.message,
kind: "error",
});
console.error(error);
reportError(error);
}

const memoizedLocationData = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { waitFor } from "@testing-library/react";
import renderWithRouter from "../test-helpers/render-with-router";
import { navigate, openmrsFetch, useConfig } from "@openmrs/esm-framework";
import {
navigate,
openmrsFetch,
useConfig,
useSession,
} from "@openmrs/esm-framework";
import {
mockSetSessionLocation,
mockSoleLoginLocation,
Expand All @@ -12,13 +17,13 @@ const mockedNavigate = navigate as jest.Mock;
const mockedOpenmrsFetch = openmrsFetch as jest.Mock;
const mockedUseConfig = useConfig as jest.Mock;

jest.mock("../CurrentUserContext", () => ({
useCurrentUser() {
return {
display: "Testy McTesterface",
};
const mockUseSession = useSession as jest.Mock;

mockUseSession.mockReturnValue({
user: {
display: "Testy McTesterface",
},
}));
});

describe("ChooseLocation: ", () => {
beforeEach(() => {
Expand Down
5 changes: 2 additions & 3 deletions packages/apps/esm-login-app/src/login/login.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ import ArrowRight24 from "@carbon/icons-react/es/arrow--right/24";
import { Button, InlineNotification, TextInput } from "carbon-components-react";
import { RouteComponentProps } from "react-router-dom";
import { useTranslation } from "react-i18next";
import { useConfig, interpolateUrl } from "@openmrs/esm-framework";
import { useConfig, interpolateUrl, useSession } from "@openmrs/esm-framework";
import { performLogin } from "./login.resource";
import { useCurrentUser } from "../CurrentUserContext";
import type { StaticContext } from "react-router";

const hidden: React.CSSProperties = {
Expand All @@ -27,7 +26,7 @@ export interface LoginProps

const Login: React.FC<LoginProps> = ({ history, location, isLoginEnabled }) => {
const config = useConfig();
const user = useCurrentUser();
const { user } = useSession();
const [username, setUsername] = useState("");
const [password, setPassword] = useState("");
const [errorMessage, setErrorMessage] = useState("");
Expand Down
20 changes: 10 additions & 10 deletions packages/apps/esm-login-app/src/login/login.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { useState } from "react";
import { waitFor, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { setSessionLocation, useConfig } from "@openmrs/esm-framework";
import {
setSessionLocation,
useConfig,
useSession,
} from "@openmrs/esm-framework";
import { performLogin } from "./login.resource";
import { useCurrentUser } from "../CurrentUserContext";
import { mockConfig } from "../../__mocks__/config.mock";
import renderWithRouter from "../test-helpers/render-with-router";
import Login from "./login.component";
Expand All @@ -15,12 +18,8 @@ jest.mock("./login.resource", () => ({
}));

const mockedSetSessionLocation = setSessionLocation as jest.Mock;
const mockedUseCurrentUser = useCurrentUser as jest.Mock;
const mockedUseConfig = useConfig as jest.Mock;

jest.mock("../CurrentUserContext", () => ({
useCurrentUser: jest.fn(),
}));
const mockedUseSession = useSession as jest.Mock;

const loginLocations = [
{ uuid: "111", display: "Earth" },
Expand All @@ -31,7 +30,8 @@ describe(`<Login />`, () => {
beforeEach(() => {
mockedLogin.mockReset();
mockedSetSessionLocation.mockReset();
mockedUseCurrentUser.mockReset();
mockedUseSession.mockReset();
mockedUseSession.mockReturnValue({ authenticated: false });
mockedUseConfig.mockReturnValue(mockConfig);
});

Expand Down Expand Up @@ -106,10 +106,10 @@ describe(`<Login />`, () => {
});
return Promise.resolve({ data: { authenticated: true } });
});
mockedUseCurrentUser.mockImplementation(() => {
mockedUseSession.mockImplementation(() => {
const [user, setUser] = useState();
refreshUser = setUser;
return user;
return { user, authenticated: !!user };
});

const wrapper = renderWithRouter(
Expand Down
14 changes: 10 additions & 4 deletions packages/apps/esm-login-app/src/redirect-logout/logout.resource.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { openmrsFetch, refetchCurrentUser } from "@openmrs/esm-framework";
import {
clearCurrentUser,
openmrsFetch,
refetchCurrentUser,
} from "@openmrs/esm-framework";

export function performLogout() {
return openmrsFetch("/ws/rest/v1/session", {
export async function performLogout() {
await openmrsFetch("/ws/rest/v1/session", {
method: "DELETE",
}).then(refetchCurrentUser);
});
clearCurrentUser();
await refetchCurrentUser();
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,29 @@
import React, { useEffect } from "react";
import { RouteComponentProps } from "react-router-dom";
import { navigate, useConfig } from "@openmrs/esm-framework";
import { navigate, useConfig, useSession } from "@openmrs/esm-framework";
import { performLogout } from "./logout.resource";
import { useCurrentUser } from "../CurrentUserContext";

export interface RedirectLogoutProps extends RouteComponentProps<{}> {
isLoginEnabled: boolean;
}

const RedirectLogout: React.FC<RedirectLogoutProps> = ({ isLoginEnabled }) => {
const config = useConfig();
const user = useCurrentUser();
const session = useSession();

useEffect(() => {
if (!user || !isLoginEnabled) {
if (!session.authenticated || !isLoginEnabled) {
navigate({ to: "${openmrsSpaBase}/login" });
} else {
performLogout().then(() => {
if (config.provider.type === "oauth2") {
location.href = config.provider.logoutUrl;
} else {
location.href = window.spaBase;
navigate({ to: "${openmrsSpaBase}/login" });
}
});
}
}, [isLoginEnabled, user, config]);
}, [isLoginEnabled, session, config]);

return null;
};
Expand Down
47 changes: 21 additions & 26 deletions packages/apps/esm-login-app/src/root.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,34 @@ import Login from "./login/login.component";
import ChooseLocation from "./choose-location/choose-location.component";
import RedirectLogout from "./redirect-logout/redirect-logout.component";
import { BrowserRouter, Route } from "react-router-dom";
import { CurrentUserContext } from "./CurrentUserContext";

export interface RootProps {
isLoginEnabled: boolean;
}

const Root: React.FC<RootProps> = ({ isLoginEnabled }) => {
return (
<CurrentUserContext>
<BrowserRouter basename={window.spaBase}>
<Route
exact
path="/login(/confirm)?"
render={(props) => (
<Login {...props} isLoginEnabled={isLoginEnabled} />
)}
/>
<Route
exact
path="/login/location"
render={(props) => (
<ChooseLocation {...props} isLoginEnabled={isLoginEnabled} />
)}
/>
<Route
exact
path="/logout"
render={(props) => (
<RedirectLogout {...props} isLoginEnabled={isLoginEnabled} />
)}
/>
</BrowserRouter>
</CurrentUserContext>
<BrowserRouter basename={window.spaBase}>
<Route
exact
path="/login(/confirm)?"
render={(props) => <Login {...props} isLoginEnabled={isLoginEnabled} />}
/>
<Route
exact
path="/login/location"
render={(props) => (
<ChooseLocation {...props} isLoginEnabled={isLoginEnabled} />
)}
/>
<Route
exact
path="/logout"
render={(props) => (
<RedirectLogout {...props} isLoginEnabled={isLoginEnabled} />
)}
/>
</BrowserRouter>
);
};

Expand Down
22 changes: 11 additions & 11 deletions packages/apps/esm-primary-navigation-app/src/root.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ const Root: React.FC<RootProps> = () => {
const openmrsSpaBase = window["getOpenmrsSpaBase"]();

useEffect(() => {
const currentUserSub = getSynchronizedCurrentUser({
includeAuthStatus: true,
}).subscribe((response) => {
setAllowedLocales(response.allowedLocales);
const currentUserSub = getSynchronizedCurrentUser().subscribe(
(response) => {
setAllowedLocales(response.allowedLocales);

if (response.authenticated) {
setUser(response.user);
} else {
setUser(false);
}
if (response.authenticated) {
setUser(response.user);
} else {
setUser(false);
}

createErrorHandler();
});
createErrorHandler();
}
);

const currentSessionSub = getCurrentSession().subscribe(({ data }) =>
setUserSession(data)
Expand Down
Loading

0 comments on commit 1a4a071

Please sign in to comment.