Skip to content

Commit 89f42d8

Browse files
ID-1174 Combine loadTerraUser sam api calls into one (#4844)
Co-authored-by: Nick Watts <[email protected]>
1 parent 8789275 commit 89f42d8

File tree

4 files changed

+82
-79
lines changed

4 files changed

+82
-79
lines changed

src/auth/auth.ts

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import { fetchOk } from 'src/libs/ajax/ajax-common';
1313
import { ExternalCredentials } from 'src/libs/ajax/ExternalCredentials';
1414
import { Groups } from 'src/libs/ajax/Groups';
1515
import { Metrics } from 'src/libs/ajax/Metrics';
16-
import { TermsOfService } from 'src/libs/ajax/TermsOfService';
1716
import { SamUserAttributes, User } from 'src/libs/ajax/User';
1817
import { withErrorIgnoring, withErrorReporting } from 'src/libs/error';
1918
import Events, { captureAppcuesEvent, MetricsEventName } from 'src/libs/events';
@@ -483,20 +482,10 @@ export const loadTerraUser = async (): Promise<void> => {
483482
try {
484483
const signInStatus = 'userLoaded';
485484
const getProfile = User().profile.get();
486-
const getAllowances = User().getUserAllowances();
487-
const getAttributes = User().getUserAttributes();
488-
const getTermsOfService = TermsOfService().getUserTermsOfServiceDetails();
489-
const getEnterpriseFeatures = User().getEnterpriseFeatures();
490-
const getSamUser = User().getSamUserResponse();
491-
const [profile, terraUserAllowances, terraUserAttributes, termsOfService, enterpriseFeatures, samUser] =
492-
await Promise.all([
493-
getProfile,
494-
getAllowances,
495-
getAttributes,
496-
getTermsOfService,
497-
getEnterpriseFeatures,
498-
getSamUser,
499-
]);
485+
const getCombinedState = User().getSamUserCombinedState();
486+
const [profile, terraUserCombinedState] = await Promise.all([getProfile, getCombinedState]);
487+
const { terraUserAttributes, enterpriseFeatures, samUser, terraUserAllowances, termsOfService } =
488+
terraUserCombinedState;
500489
clearNotification(sessionExpirationProps.id);
501490
userStore.update((state: TerraUserState) => ({
502491
...state,

src/auth/login.test.ts

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import { loadTerraUser } from 'src/auth/auth';
55
import { Groups, GroupsContract } from 'src/libs/ajax/Groups';
66
import { Metrics, MetricsContract } from 'src/libs/ajax/Metrics';
77
import { SamUserTermsOfServiceDetails, TermsOfService, TermsOfServiceContract } from 'src/libs/ajax/TermsOfService';
8-
import { SamUserResponse, User, UserContract } from 'src/libs/ajax/User';
9-
import { oidcStore, TerraUserState, userStore } from 'src/libs/state';
8+
import { SamUserCombinedStateResponse, SamUserResponse, User, UserContract } from 'src/libs/ajax/User';
9+
import { oidcStore, userStore } from 'src/libs/state';
1010

1111
jest.mock('src/libs/ajax/TermsOfService');
1212
jest.mock('src/libs/ajax/User');
@@ -73,6 +73,14 @@ const testSamUserAllowances = {
7373
details: testSamUserAllowancesDetails,
7474
};
7575

76+
const mockSamUserCombinedState: SamUserCombinedStateResponse = {
77+
samUser: mockSamUserResponse,
78+
terraUserAllowances: testSamUserAllowances,
79+
terraUserAttributes: { marketingConsent: false },
80+
termsOfService: mockSamUserTermsOfServiceDetails,
81+
enterpriseFeatures: [],
82+
};
83+
7684
const mockNihDatasetPermission = {
7785
name: 'testNihDatasetPermissionName',
7886
authorized: true,
@@ -87,11 +95,7 @@ const mockOrchestrationNihStatusResponse = {
8795
// TODO centralize Ajax mock setup so it can be reused across tests
8896
describe('a request to load a terra user', () => {
8997
// Arrange (shared between tests for the success case)
90-
const getUserAllowancesFunction = jest.fn().mockResolvedValue(testSamUserAllowances);
91-
const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false });
92-
const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails);
93-
const getEnterpriseFeaturesFunction = jest.fn().mockResolvedValue([]);
94-
const getSamUserResponseFunction = jest.fn().mockResolvedValue(mockSamUserResponse);
98+
const getSamUserCombinedStateFunction = jest.fn().mockResolvedValue(mockSamUserCombinedState);
9599
const getNihStatusFunction = jest.fn().mockResolvedValue(mockOrchestrationNihStatusResponse);
96100
const getFenceStatusFunction = jest.fn().mockResolvedValue({});
97101

@@ -105,11 +109,7 @@ describe('a request to load a terra user', () => {
105109
captureEvent: jest.fn(),
106110
} as Partial<MetricsContract> as MetricsContract);
107111
asMockedFn(User).mockReturnValue({
108-
getUserAllowances: getUserAllowancesFunction,
109-
getUserAttributes: getUserAttributesFunction,
110-
getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction,
111-
getEnterpriseFeatures: getEnterpriseFeaturesFunction,
112-
getSamUserResponse: getSamUserResponseFunction,
112+
getSamUserCombinedState: getSamUserCombinedStateFunction,
113113
getNihStatus: getNihStatusFunction,
114114
getFenceStatus: getFenceStatusFunction,
115115
profile: {
@@ -131,36 +131,24 @@ describe('a request to load a terra user', () => {
131131
await act(() => loadTerraUser());
132132

133133
// Assert
134-
expect(getSamUserResponseFunction).toHaveBeenCalled();
134+
expect(getSamUserCombinedStateFunction).toHaveBeenCalled();
135135
});
136136
it('should update the samUser in state', async () => {
137137
// Act
138138
await act(() => loadTerraUser());
139139

140-
let samUser;
141-
await act(async () => {
142-
samUser = await getSamUserResponseFunction.mock.results[0].value;
143-
});
144-
userStore.update((state: TerraUserState) => ({
145-
...state,
146-
samUser,
147-
}));
148140
// Assert
149-
expect(getSamUserResponseFunction).toHaveBeenCalled();
141+
expect(getSamUserCombinedStateFunction).toHaveBeenCalled();
150142
expect(userStore.get().samUser).toEqual(mockSamUserResponse);
151143
});
152144
describe('when not successful', () => {
153145
it('should fail with an error', async () => {
154146
// // Arrange
155147
// mock a failure to get samUserResponse
156-
const getSamUserResponseFunction = jest.fn().mockRejectedValue(new Error('unknown'));
148+
const getSamUserCombinedStateMockFailure = jest.fn().mockRejectedValue(new Error('unknown'));
157149

158150
asMockedFn(User).mockReturnValue({
159-
getUserAllowances: getUserAllowancesFunction,
160-
getUserAttributes: getUserAttributesFunction,
161-
getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction,
162-
getEnterpriseFeatures: getEnterpriseFeaturesFunction,
163-
getSamUserResponse: getSamUserResponseFunction,
151+
getSamUserCombinedState: getSamUserCombinedStateMockFailure,
164152
profile: {
165153
get: jest.fn().mockReturnValue(mockTerraUserProfile),
166154
},

src/libs/ajax/User.ts

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import _ from 'lodash/fp';
22
import { authOpts, fetchOrchestration, fetchSam, jsonBody } from 'src/libs/ajax/ajax-common';
3+
import { SamUserTermsOfServiceDetails } from 'src/libs/ajax/TermsOfService';
34
import { TerraUserProfile } from 'src/libs/state';
45

56
export interface SamUserRegistrationStatusResponse {
@@ -162,6 +163,14 @@ export type SamUserAttributesRequest = {
162163
marketingConsent: boolean | undefined;
163164
};
164165

166+
export type SamUserCombinedStateResponse = {
167+
samUser: SamUserResponse;
168+
terraUserAllowances: SamUserAllowances;
169+
terraUserAttributes: SamUserAttributes;
170+
termsOfService: SamUserTermsOfServiceDetails;
171+
enterpriseFeatures: string[];
172+
};
173+
165174
export type OrchestrationUserRegistrationRequest = object;
166175

167176
export const User = (signal?: AbortSignal) => {
@@ -171,11 +180,6 @@ export const User = (signal?: AbortSignal) => {
171180
return res.json();
172181
},
173182

174-
getUserAllowances: async (): Promise<SamUserAllowances> => {
175-
const res = await fetchSam('api/users/v2/self/allowed', _.mergeAll([authOpts(), { signal }]));
176-
return res.json();
177-
},
178-
179183
getUserAttributes: async (): Promise<SamUserAttributes> => {
180184
const res = await fetchSam('api/users/v2/self/attributes', _.mergeAll([authOpts(), { signal }]));
181185
return res.json().then((obj) => {
@@ -192,17 +196,44 @@ export const User = (signal?: AbortSignal) => {
192196
return res.json();
193197
},
194198

195-
getEnterpriseFeatures: async (): Promise<string[]> => {
196-
try {
197-
const res = await fetchSam(
198-
'/api/resources/v2?format=flat&resourceTypes=enterprise-feature&roles=user',
199-
_.mergeAll([authOpts(), { signal }])
200-
);
201-
const json = await res.json();
202-
return json.resources.map((resource) => resource.resourceId);
203-
} catch (error: unknown) {
204-
return [];
205-
}
199+
getSamUserCombinedState: async (): Promise<SamUserCombinedStateResponse> => {
200+
const res = await fetchSam('api/users/v2/self/combinedState', _.mergeAll([authOpts(), { signal }]));
201+
const responseJson = await res.json();
202+
const samUser: SamUserResponse = {
203+
id: responseJson.samUser.id,
204+
googleSubjectId: responseJson.samUser.googleSubjectId,
205+
email: responseJson.samUser.email,
206+
azureB2CId: responseJson.samUser.azureB2CId,
207+
allowed: responseJson.samUser.allowed,
208+
createdAt: responseJson.samUser.createdAt ? new Date(responseJson.samUser.createdAt) : undefined,
209+
registeredAt: responseJson.samUser.registeredAt ? new Date(responseJson.samUser.registeredAt) : undefined,
210+
updatedAt: responseJson.samUser.updatedAt ? new Date(responseJson.samUser.updatedAt) : undefined,
211+
};
212+
213+
const terraUserAllowances: SamUserAllowances = responseJson.allowances;
214+
215+
const terraUserAttributes: SamUserAttributes = { marketingConsent: responseJson.attributes.marketingConsent };
216+
217+
const termsOfService: SamUserTermsOfServiceDetails = {
218+
latestAcceptedVersion: responseJson.termsOfServiceDetails.latestAcceptedVersion,
219+
acceptedOn: responseJson.termsOfServiceDetails.acceptedOn
220+
? new Date(responseJson.termsOfServiceDetails.acceptedOn)
221+
: undefined,
222+
permitsSystemUsage: responseJson.termsOfServiceDetails.permitsSystemUsage,
223+
isCurrentVersion: responseJson.termsOfServiceDetails.isCurrentVersion,
224+
};
225+
226+
const enterpriseFeatures = responseJson.additionalDetails.enterpriseFeatures
227+
? responseJson.additionalDetails.enterpriseFeatures.resources.map((resource) => resource.resourceId)
228+
: [];
229+
230+
return {
231+
samUser,
232+
terraUserAllowances,
233+
terraUserAttributes,
234+
termsOfService,
235+
enterpriseFeatures,
236+
};
206237
},
207238

208239
registerWithProfile: async (
@@ -267,21 +298,6 @@ export const User = (signal?: AbortSignal) => {
267298
return res.json();
268299
},
269300

270-
getSamUserResponse: async (): Promise<SamUserResponse> => {
271-
const res = await fetchSam('api/users/v2/self', _.mergeAll([authOpts(), { method: 'GET' }]));
272-
const json = await res.json();
273-
return {
274-
id: json.id,
275-
googleSubjectId: json.googleSubjectId,
276-
email: json.email,
277-
azureB2CId: json.azureB2CId,
278-
allowed: json.allowed,
279-
createdAt: json.createdAt ? new Date(json.createdAt) : undefined,
280-
registeredAt: json.registeredAt ? new Date(json.registeredAt) : undefined,
281-
updatedAt: json.updatedAt ? new Date(json.updatedAt) : undefined,
282-
};
283-
},
284-
285301
getNihStatus: async (): Promise<OrchestrationNihStatusResponse | undefined> => {
286302
try {
287303
const res = await fetchOrchestration('api/nih/status', _.merge(authOpts(), { signal }));

src/registration/terms-of-service/TermsOfServicePage.test.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@ import { Ajax } from 'src/libs/ajax';
55
import { Groups, GroupsContract } from 'src/libs/ajax/Groups';
66
import { Metrics, MetricsContract } from 'src/libs/ajax/Metrics';
77
import { SamUserTermsOfServiceDetails, TermsOfService, TermsOfServiceContract } from 'src/libs/ajax/TermsOfService';
8-
import { SamUserAllowances, SamUserResponse, User, UserContract } from 'src/libs/ajax/User';
8+
import {
9+
SamUserAllowances,
10+
SamUserCombinedStateResponse,
11+
SamUserResponse,
12+
User,
13+
UserContract,
14+
} from 'src/libs/ajax/User';
915
import { AuthState, authStore } from 'src/libs/state';
1016
import { TermsOfServicePage } from 'src/registration/terms-of-service/TermsOfServicePage';
1117
import { asMockedFn, renderWithAppContexts as render } from 'src/testing/test-utils';
@@ -68,6 +74,13 @@ const setupMockAjax = async (
6874
registeredAt: new Date('1970-01-01'),
6975
updatedAt: new Date('1970-01-01'),
7076
};
77+
const mockSamUserCombinedState: SamUserCombinedStateResponse = {
78+
samUser: mockSamUserResponse,
79+
terraUserAllowances,
80+
terraUserAttributes: { marketingConsent: false },
81+
termsOfService,
82+
enterpriseFeatures: [],
83+
};
7184
const getTermsOfServiceText = jest.fn().mockResolvedValue('some text');
7285
const getUserTermsOfServiceDetails = jest
7386
.fn()
@@ -95,10 +108,7 @@ const setupMockAjax = async (
95108
} as Partial<MetricsContract> as MetricsContract);
96109

97110
asMockedFn(User).mockReturnValue({
98-
getUserAttributes: jest.fn().mockResolvedValue({ marketingConsent: true }),
99-
getUserAllowances: jest.fn().mockResolvedValue(terraUserAllowances),
100-
getEnterpriseFeatures: jest.fn().mockResolvedValue([]),
101-
getSamUserResponse: jest.fn().mockResolvedValue(mockSamUserResponse),
111+
getSamUserCombinedState: jest.fn().mockResolvedValue(mockSamUserCombinedState),
102112
profile: {
103113
get: jest.fn().mockResolvedValue({ keyValuePairs: [] }),
104114
update: jest.fn().mockResolvedValue({ keyValuePairs: [] }),

0 commit comments

Comments
 (0)