Skip to content

Commit acacc2d

Browse files
MatthewPopat-NHSMatthewPopat-NHS
authored
Fix: [AEA-5965] - Speculative fixes for users being incorrectly told they have no access (#1609)
## Summary - ⚠️ Potential issues that might be caused by this change ### Details Stopped role selection from editing rolesWithAccess data Removed access related calculated fields from local storage Simplified rendering of role selection page
1 parent 3eab63f commit acacc2d

28 files changed

+179
-564
lines changed

‎.vscode/eps-prescription-tracker-ui.code-workspace‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
{
6868
"name": "packages/common/lambdaUtils",
6969
"path": "../packages/common/lambdaUtils"
70-
}
70+
},
7171
{
7272
"name": "packages/common/testing",
7373
"path": "../packages/common/testing"

‎packages/common/commonTypes/src/trackerUserInfo.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ export type TrackerUserInfo = {
3030
export type TrackerUserInfoResult = {
3131
rolesWithAccess: Array<RoleDetails>,
3232
rolesWithoutAccess: Array<RoleDetails>,
33-
hasNoAccess: boolean
3433
selectedRole: RoleDetails | undefined,
3534
userDetails: UserDetails | undefined,
36-
hasSingleRoleAccess: boolean,
3735
isConcurrentSession: boolean,
3836
invalidSessionCause: string | undefined,
3937
sessionId: string | undefined,

‎packages/cpt-ui/__tests__/AuthProvider.test.tsx‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,8 @@ const userDetails = {
4141
const mockUserInfo = {
4242
rolesWithAccess: rolesWithAccess,
4343
rolesWithoutAccess: [],
44-
hasNoAccess: false,
4544
selectedRole: currentlySelectedRole,
4645
userDetails: userDetails,
47-
hasSingleRoleAccess: true,
4846
error: undefined
4947
}
5048

@@ -142,10 +140,8 @@ const TestConsumer = () => {
142140
<div data-testid="isSigningIn">{auth.isSigningIn.toString()}</div>
143141
<div data-testid="rolesWithAccess">{JSON.stringify(auth.rolesWithAccess, null, 2)}</div>
144142
<div data-testid="rolesWithoutAccess">{JSON.stringify(auth.rolesWithoutAccess, null, 2)}</div>
145-
<div data-testid="noAccess">{auth.hasNoAccess.toString()}</div>
146143
<div data-testid="selectedRole">{JSON.stringify(auth.selectedRole, null, 2)}</div>
147144
<div data-testid="userDetails">{JSON.stringify(auth.userDetails, null, 2)}</div>
148-
<div data-testid="singleAccess">{auth.hasSingleRoleAccess.toString()}</div>
149145
</div>
150146
)
151147
}
@@ -265,10 +261,8 @@ describe("AuthProvider", () => {
265261
expect(screen.getByTestId("user").textContent).toBe("test_user")
266262
expect(screen.getByTestId("rolesWithAccess").textContent).toBe(JSON.stringify(rolesWithAccess, null, 2))
267263
expect(screen.getByTestId("rolesWithoutAccess").textContent).toBe("[]")
268-
expect(screen.getByTestId("noAccess").textContent).toBe("false")
269264
expect(screen.getByTestId("selectedRole").textContent).toBe(JSON.stringify(currentlySelectedRole, null, 2))
270265
expect(screen.getByTestId("userDetails").textContent).toBe(JSON.stringify(userDetails, null, 2))
271-
expect(screen.getByTestId("singleAccess").textContent).toBe("true")
272266
})
273267
})
274268

@@ -326,10 +320,8 @@ describe("AuthProvider", () => {
326320
expect(screen.getByTestId("error").textContent).toBe("")
327321
expect(screen.getByTestId("rolesWithAccess").textContent).toBe("[]")
328322
expect(screen.getByTestId("rolesWithoutAccess").textContent).toBe("[]")
329-
expect(screen.getByTestId("noAccess").textContent).toBe("true")
330323
expect(screen.getByTestId("selectedRole").textContent).toBe("")
331324
expect(screen.getByTestId("userDetails").textContent).toBe("")
332-
expect(screen.getByTestId("singleAccess").textContent).toBe("false")
333325
})
334326
})
335327

@@ -395,7 +387,7 @@ describe("AuthProvider", () => {
395387
};
396388

397389
(updateRemoteSelectedRole as jest.Mock).mockResolvedValue({
398-
rolesWithAccess: [newRole]
390+
currentlySelectedRole: newRole
399391
})
400392

401393
let contextValue: AuthContextType | null

‎packages/cpt-ui/__tests__/BasicDetailsSearch.test.tsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,14 @@ const signedInAuthState: AuthContextType = {
4545
error: null,
4646
rolesWithAccess: [],
4747
rolesWithoutAccess: [],
48-
hasNoAccess: false,
49-
hasSingleRoleAccess: false,
5048
selectedRole: undefined,
5149
userDetails: undefined,
5250
isConcurrentSession: false,
5351
sessionId: "test-session-id",
5452
cognitoSignIn: mockCognitoSignIn,
5553
cognitoSignOut: mockCognitoSignOut,
5654
clearAuthState: mockClearAuthState,
55+
hasSingleRoleAccess: jest.fn().mockReturnValue(false),
5756
updateSelectedRole: jest.fn(),
5857
updateTrackerUserInfo: jest.fn(),
5958
updateInvalidSessionCause: jest.fn(),

‎packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,14 @@ const mockAuthContext: AuthContextType = {
5050
invalidSessionCause: undefined,
5151
rolesWithAccess: [],
5252
rolesWithoutAccess: [],
53-
hasNoAccess: false,
54-
hasSingleRoleAccess: false,
5553
selectedRole: undefined,
5654
userDetails: undefined,
5755
isConcurrentSession: false,
5856
sessionId: undefined,
5957
cognitoSignIn: jest.fn(),
6058
cognitoSignOut: jest.fn(),
6159
clearAuthState: jest.fn(),
60+
hasSingleRoleAccess: jest.fn().mockReturnValue(false),
6261
updateSelectedRole: jest.fn(),
6362
updateTrackerUserInfo: jest.fn(),
6463
updateInvalidSessionCause: jest.fn(),

‎packages/cpt-ui/__tests__/EpsHeader.test.tsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,14 @@ const defaultAuthContext: AuthContextType = {
5555
invalidSessionCause: undefined,
5656
rolesWithAccess: [],
5757
rolesWithoutAccess: [],
58-
hasNoAccess: false,
59-
hasSingleRoleAccess: false,
6058
selectedRole: undefined,
6159
userDetails: undefined,
6260
isConcurrentSession: false,
6361
sessionId: undefined,
6462
cognitoSignIn: jest.fn(),
6563
cognitoSignOut: jest.fn(),
6664
clearAuthState: jest.fn(),
65+
hasSingleRoleAccess: jest.fn().mockReturnValue(false),
6766
updateSelectedRole: jest.fn(),
6867
updateTrackerUserInfo: jest.fn(),
6968
updateInvalidSessionCause: jest.fn(),

‎packages/cpt-ui/__tests__/EpsPrescriptionList.test.tsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,14 @@ const signedInAuthState: AuthContextType = {
137137
error: null,
138138
rolesWithAccess: [],
139139
rolesWithoutAccess: [],
140-
hasNoAccess: false,
141-
hasSingleRoleAccess: false,
142140
selectedRole: undefined,
143141
userDetails: undefined,
144142
isConcurrentSession: false,
145143
sessionId: undefined,
146144
cognitoSignIn: mockCognitoSignIn,
147145
cognitoSignOut: mockCognitoSignOut,
148146
clearAuthState: jest.fn(),
147+
hasSingleRoleAccess: jest.fn().mockReturnValue(false),
149148
updateSelectedRole: jest.fn(),
150149
updateTrackerUserInfo: jest.fn(),
151150
updateInvalidSessionCause: jest.fn(),

‎packages/cpt-ui/__tests__/EpsRoleSelectionPage.test.tsx‎

Lines changed: 48 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {FRONTEND_PATHS} from "@/constants/environment"
1313
import {getSearchParams} from "@/helpers/getSearchParams"
1414
import {handleRestartLogin} from "@/helpers/logout"
1515
import axios from "axios"
16+
import {RoleDetails} from "@cpt-ui-common/common-types"
1617

1718
jest.mock("@/context/AuthProvider")
1819
jest.mock("@/helpers/getSearchParams")
@@ -89,11 +90,11 @@ describe("RoleSelectionPage", () => {
8990
it("renders loading spinner if redirecting during sign in", () => {
9091
mockUseAuth.mockReturnValue({
9192
isSigningIn: true,
92-
hasNoAccess: false,
9393
rolesWithAccess: [],
9494
rolesWithoutAccess: [],
9595
error: null,
96-
clearAuthState: jest.fn()
96+
clearAuthState: jest.fn(),
97+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
9798
})
9899
mockGetSearchParams.mockReturnValue({
99100
codeParams: "foo",
@@ -107,24 +108,24 @@ describe("RoleSelectionPage", () => {
107108
it("renders error message if auth.error exists", () => {
108109
mockUseAuth.mockReturnValue({
109110
isSigningIn: false,
110-
hasNoAccess: false,
111111
rolesWithAccess: [],
112112
rolesWithoutAccess: [],
113-
error: "Something went wrong"
113+
error: "Something went wrong",
114+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
114115
})
115116

116117
render(<RoleSelectionPage contentText={defaultContentText} />)
117118
expect(screen.getByText("There was an error selecting your role")).toBeInTheDocument()
118119
expect(screen.getByText("Something went wrong")).toBeInTheDocument()
119120
})
120121

121-
it("renders titleNoAccess and captionNoAccess when hasNoAccess is true", () => {
122+
it("renders titleNoAccess and captionNoAccess when rolesWithAccess is empty", () => {
122123
mockUseAuth.mockReturnValue({
123124
isSigningIn: false,
124-
hasNoAccess: true,
125125
rolesWithAccess: [],
126126
rolesWithoutAccess: [],
127-
error: null
127+
error: null,
128+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
128129
})
129130

130131
render(<RoleSelectionPage contentText={defaultContentText} />)
@@ -139,13 +140,12 @@ describe("RoleSelectionPage", () => {
139140
mockUseAuth.mockReturnValue({
140141
isSigningIn: true,
141142
isSignedIn: false,
142-
hasSingleRoleAccess: true,
143-
hasNoAccess: false,
144143
rolesWithAccess: [],
145144
rolesWithoutAccess: [],
146145
selectedRole: null,
147146
error: null,
148-
clearAuthState: jest.fn()
147+
clearAuthState: jest.fn(),
148+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
149149
})
150150
mockGetSearchParams.mockReturnValue({
151151
codeParams: undefined,
@@ -157,19 +157,25 @@ describe("RoleSelectionPage", () => {
157157
expect(navigateMock).toHaveBeenCalledWith(FRONTEND_PATHS.LOGIN)
158158
})
159159

160-
it("redirects if user hasSingleRoleAccess", () => {
160+
it("redirects if user has single roleWithAccess", () => {
161161
const navigateMock = jest.fn()
162162
mockNavigate.mockReturnValue(navigateMock)
163163

164164
mockUseAuth.mockReturnValue({
165165
isSigningIn: false,
166166
isSignedIn: true,
167-
hasSingleRoleAccess: true,
168-
hasNoAccess: false,
169-
rolesWithAccess: [],
167+
rolesWithAccess: [
168+
{
169+
role_id: "1",
170+
role_name: "Pharmacist",
171+
org_code: "ABC",
172+
org_name: "Pharmacy Org"
173+
}
174+
],
170175
rolesWithoutAccess: [],
171176
selectedRole: null,
172-
error: null
177+
error: null,
178+
hasSingleRoleAccess: jest.fn().mockReturnValue(true)
173179
})
174180

175181
render(<RoleSelectionPage contentText={defaultContentText} />)
@@ -180,8 +186,6 @@ describe("RoleSelectionPage", () => {
180186
it("renders login info when selectedRole is present", () => {
181187
mockUseAuth.mockReturnValue({
182188
isSigningIn: false,
183-
hasSingleRoleAccess: false,
184-
hasNoAccess: false,
185189
selectedRole: {
186190
org_name: "Test Org",
187191
org_code: "TEST123",
@@ -190,7 +194,8 @@ describe("RoleSelectionPage", () => {
190194
},
191195
rolesWithAccess: [],
192196
rolesWithoutAccess: [],
193-
error: null
197+
error: null,
198+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
194199
})
195200

196201
render(<RoleSelectionPage contentText={defaultContentText} />)
@@ -202,7 +207,6 @@ describe("RoleSelectionPage", () => {
202207
it("renders roles without access in table", () => {
203208
mockUseAuth.mockReturnValue({
204209
isSigningIn: false,
205-
hasNoAccess: false,
206210
rolesWithAccess: [],
207211
rolesWithoutAccess: [
208212
{
@@ -211,7 +215,8 @@ describe("RoleSelectionPage", () => {
211215
org_code: "NO123"
212216
}
213217
],
214-
error: null
218+
error: null,
219+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
215220
})
216221

217222
render(<RoleSelectionPage contentText={defaultContentText} />)
@@ -224,7 +229,6 @@ describe("RoleSelectionPage", () => {
224229
it("renders EpsCard components for roles with access", () => {
225230
mockUseAuth.mockReturnValue({
226231
isSigningIn: false,
227-
hasNoAccess: false,
228232
selectedRole: {
229233
role_id: "1"
230234
},
@@ -249,7 +253,8 @@ describe("RoleSelectionPage", () => {
249253
}
250254
],
251255
rolesWithoutAccess: [],
252-
error: null
256+
error: null,
257+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
253258
})
254259

255260
render(<RoleSelectionPage contentText={defaultContentText} />)
@@ -265,7 +270,6 @@ describe("RoleSelectionPage", () => {
265270
it("navigates on confirm and continue button click", async () => {
266271
mockUseAuth.mockReturnValue({
267272
isSigningIn: false,
268-
hasNoAccess: false,
269273
selectedRole: {
270274
role_id: "1",
271275
org_name: "Pharmacy A",
@@ -274,7 +278,8 @@ describe("RoleSelectionPage", () => {
274278
},
275279
rolesWithAccess: [],
276280
rolesWithoutAccess: [],
277-
error: null
281+
error: null,
282+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
278283
})
279284

280285
render(<RoleSelectionPage contentText={defaultContentText} />)
@@ -284,7 +289,6 @@ describe("RoleSelectionPage", () => {
284289
expect(button).toBeEnabled()
285290
fireEvent.click(button)
286291

287-
expect(mockNavigate.mock.calls.length).toBe(5)
288292
expect(mockNavigate).toHaveBeenCalledWith("/continue")
289293
})
290294

@@ -293,13 +297,12 @@ describe("RoleSelectionPage", () => {
293297
const authState = {
294298
isSigningIn: true,
295299
isSignedIn: false,
296-
hasSingleRoleAccess: false,
297-
hasNoAccess: false,
298-
rolesWithAccess: [],
300+
rolesWithAccess: [] as Array<RoleDetails>,
299301
rolesWithoutAccess: [],
300-
selectedRole: null,
302+
selectedRole: undefined as RoleDetails | undefined,
301303
error: null,
302-
clearAuthState: jest.fn()
304+
clearAuthState: jest.fn(),
305+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
303306
}
304307

305308
mockUseAuth.mockReturnValue(authState)
@@ -317,10 +320,17 @@ describe("RoleSelectionPage", () => {
317320
// Step 2: Simulate login complete and role assignment
318321
act(() => {
319322
authState.isSigningIn = false
320-
authState.isSignedIn = true
321-
authState.hasSingleRoleAccess = true
323+
const role = {
324+
role_id: "2",
325+
role_name: "Pharmacist",
326+
org_code: "ABC",
327+
org_name: "Pharmacy Org"
328+
}
329+
authState.rolesWithAccess = [role]
330+
authState.selectedRole = role
322331
authState.isSignedIn = true
323332
mockUseAuth.mockReturnValue(authState)
333+
authState.hasSingleRoleAccess = jest.fn().mockReturnValue(true)
324334
})
325335

326336
rerender(<RoleSelectionPage contentText={defaultContentText} />)
@@ -346,7 +356,8 @@ describe("RoleSelectionPage", () => {
346356
rolesWithAccess: [roleWithAccess],
347357
rolesWithoutAccess: [],
348358
error: null,
349-
updateSelectedRole: mockUpdateSelectedRole
359+
updateSelectedRole: mockUpdateSelectedRole,
360+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
350361
})
351362
})
352363

@@ -506,7 +517,8 @@ describe("RoleSelectionPage", () => {
506517
rolesWithAccess: [incompleteRole],
507518
rolesWithoutAccess: [],
508519
error: null,
509-
updateSelectedRole: mockUpdateSelectedRole
520+
updateSelectedRole: mockUpdateSelectedRole,
521+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
510522
})
511523

512524
render(<RoleSelectionPage contentText={defaultContentText} />)
@@ -532,7 +544,8 @@ describe("RoleSelectionPage", () => {
532544
rolesWithAccess: [selectedRole, roleWithAccess],
533545
rolesWithoutAccess: [],
534546
error: null,
535-
updateSelectedRole: mockUpdateSelectedRole
547+
updateSelectedRole: mockUpdateSelectedRole,
548+
hasSingleRoleAccess: jest.fn().mockReturnValue(false)
536549
})
537550

538551
render(<RoleSelectionPage contentText={defaultContentText} />)

0 commit comments

Comments
 (0)