Skip to content

Commit 84d59df

Browse files
committed
O3-4436: Navigation occasionally breaks
O3-4436: Navigation occasionally breaks O3-4436: Applied review changes
1 parent 55973fa commit 84d59df

File tree

3 files changed

+24
-17
lines changed

3 files changed

+24
-17
lines changed

packages/apps/esm-login-app/src/config-schema.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,14 @@ export const configSchema = {
77
_default: 'basic',
88
_description:
99
"Selects the login mechanism to use. Choices are 'basic' and 'oauth2'. " +
10-
"For 'oauth2' you'll also need to set the 'loginUrl' and 'logoutUrl'.",
10+
"For 'oauth2' you'll also need to set the 'loginUrl'",
1111
},
1212
loginUrl: {
1313
_type: Type.String,
1414
_default: '${openmrsSpaBase}/login',
1515
_description: 'The URL to use for an OAuth2 login.',
1616
_validators: [validators.isUrl],
1717
},
18-
logoutUrl: {
19-
_type: Type.String,
20-
_default: '${openmrsSpaBase}/logout',
21-
_description: 'The URL to use for an OAuth2 logout.',
22-
_validators: [validators.isUrl],
23-
},
2418
},
2519
chooseLocation: {
2620
enabled: {

packages/apps/esm-login-app/src/redirect-logout/redirect-logout.component.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ const RedirectLogout: React.FC = () => {
1212
useEffect(() => {
1313
clearHistory();
1414
if (!session.authenticated || !isLoginEnabled) {
15-
navigate({ to: '${openmrsSpaBase}/login' });
15+
if (config.provider.type !== 'oauth2') {
16+
navigate({ to: '${openmrsSpaBase}/login' });
17+
}
1618
} else {
1719
performLogout()
1820
.then(() => {
@@ -24,9 +26,7 @@ const RedirectLogout: React.FC = () => {
2426
sessionId: '',
2527
});
2628

27-
if (config.provider.type === 'oauth2') {
28-
navigate({ to: config.provider.logoutUrl });
29-
} else {
29+
if (config.provider.type !== 'oauth2') {
3030
navigate({ to: '${openmrsSpaBase}/login' });
3131
}
3232
})

packages/apps/esm-login-app/src/redirect-logout/redirect-logout.test.tsx

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,10 @@ describe('RedirectLogout', () => {
7272
expect(mockNavigate).toHaveBeenCalledWith({ to: '${openmrsSpaBase}/login' });
7373
});
7474

75-
it('should redirect to `provider.logoutUrl` if the configured provider is `oauth2`', async () => {
75+
it('should not redirect if the configured provider is `oauth2`', async () => {
7676
mockUseConfig.mockReturnValue({
7777
provider: {
7878
type: 'oauth2',
79-
logoutUrl: '/oauth/logout',
8079
},
8180
});
8281

@@ -95,7 +94,7 @@ describe('RedirectLogout', () => {
9594
authenticated: false,
9695
sessionId: '',
9796
});
98-
expect(mockNavigate).toHaveBeenCalledWith({ to: '/oauth/logout' });
97+
expect(mockNavigate).toHaveBeenCalledTimes(0);
9998
});
10099

101100
it('should redirect to login if the session is already unauthenticated', async () => {
@@ -153,15 +152,29 @@ describe('RedirectLogout', () => {
153152

154153
mockUseConfig.mockReturnValue({
155154
provider: {
156-
type: 'oauth2',
157-
logoutUrl: '/new/logout/url',
155+
type: 'testProvider',
158156
},
159157
});
160158

161159
rerender(<RedirectLogout />);
162160

163161
await waitFor(() => {
164-
expect(mockNavigate).toHaveBeenCalledWith({ to: '/new/logout/url' });
162+
expect(mockNavigate).toHaveBeenCalledWith({ to: '${openmrsSpaBase}/login' });
163+
});
164+
});
165+
166+
it('should not redirect to login if user is not authenticated and the provider is oauth2', async () => {
167+
mockUseSession.mockReturnValue({
168+
authenticated: false,
169+
} as Session);
170+
mockUseConfig.mockReturnValue({
171+
provider: {
172+
type: 'oauth2',
173+
},
165174
});
175+
176+
render(<RedirectLogout />);
177+
178+
expect(mockNavigate).toHaveBeenCalledTimes(0);
166179
});
167180
});

0 commit comments

Comments
 (0)