From 2ec08f8d3c6d4f0f33dbe14f02e7d57cae028e1e Mon Sep 17 00:00:00 2001 From: Kai Date: Fri, 8 Mar 2024 20:35:54 -0500 Subject: [PATCH 01/18] wip get registered date from sam --- src/libs/ajax/User.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libs/ajax/User.ts b/src/libs/ajax/User.ts index 4f271ecf12..8eb285b4c8 100644 --- a/src/libs/ajax/User.ts +++ b/src/libs/ajax/User.ts @@ -188,6 +188,14 @@ const getFirstTimeStamp = Utils.memoizeAsync( { keyFn: (...args) => JSON.stringify(args) } ) as (token: string) => Promise; +const getRegisteredAtDate = Utils.memoizeAsync( + async (token): Promise => { + const res = await fetchSam('api/users/v2/self', _.mergeAll([authOpts(token), { method: 'GET' }])); + return res.json(); + }, + { keyFn: (...args) => JSON.stringify(args) } +) as (token: string) => Promise; + export const User = (signal?: AbortSignal) => { return { getStatus: async (): Promise => { @@ -272,6 +280,10 @@ export const User = (signal?: AbortSignal) => { return getFirstTimeStamp(getTerraUser().token!); }, + registeredAtDate: (): Promise => { + return getRegisteredAtDate(getTerraUser().token!); + }, + getNihStatus: async (): Promise => { try { const res = await fetchOrchestration('api/nih/status', _.merge(authOpts(), { signal })); From 1b71e1e107af8796dfc48fe39dca1d8e0e394bf0 Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 12 Mar 2024 14:07:33 -0400 Subject: [PATCH 02/18] get registration date from sam --- src/auth/auth.ts | 3 +-- src/libs/ajax/User.ts | 26 +++++--------------------- 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index ee27c6518d..2c250d12d8 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -1,5 +1,4 @@ import { DEFAULT, switchCase } from '@terra-ui-packages/core-utils'; -import { parseJSON } from 'date-fns/fp'; import jwtDecode, { JwtPayload } from 'jwt-decode'; import _ from 'lodash/fp'; import { sessionTimedOutErrorMessage } from 'src/auth/auth-errors'; @@ -528,7 +527,7 @@ authStore.subscribe( if (!oldState.termsOfService.permitsSystemUsage && state.termsOfService.permitsSystemUsage) { if (window.Appcues) { window.Appcues.identify(userStore.get().terraUser.id!, { - dateJoined: parseJSON((await Ajax().User.firstTimestamp()).timestamp).getTime(), + dateJoined: await Ajax().User.registeredAtDate(), }); window.Appcues.on('all', captureAppcuesEvent); } diff --git a/src/libs/ajax/User.ts b/src/libs/ajax/User.ts index 8eb285b4c8..3a86b3989c 100644 --- a/src/libs/ajax/User.ts +++ b/src/libs/ajax/User.ts @@ -1,6 +1,6 @@ import _ from 'lodash/fp'; import * as qs from 'qs'; -import { authOpts, fetchBond, fetchOrchestration, fetchRex, fetchSam, jsonBody } from 'src/libs/ajax/ajax-common'; +import { authOpts, fetchBond, fetchOrchestration, fetchSam, jsonBody } from 'src/libs/ajax/ajax-common'; import { getTerraUser, TerraUserProfile } from 'src/libs/state'; import * as Utils from 'src/libs/utils'; @@ -154,10 +154,6 @@ export interface SamInviteUserResponse { userEmail: string; } -export interface RexFirstTimestampResponse { - timestamp: Date; -} - export interface SamUserResponse { id: string; googleSubjectId?: string; @@ -179,16 +175,7 @@ export type SamUserAttributesRequest = { export type OrchestrationUserRegistrationRequest = object; -// TODO: Remove this as a part of https://broadworkbench.atlassian.net/browse/ID-460 -const getFirstTimeStamp = Utils.memoizeAsync( - async (token): Promise => { - const res = await fetchRex('firstTimestamps/record', _.mergeAll([authOpts(token), { method: 'POST' }])); - return res.json(); - }, - { keyFn: (...args) => JSON.stringify(args) } -) as (token: string) => Promise; - -const getRegisteredAtDate = Utils.memoizeAsync( +const getSamUserResponse = Utils.memoizeAsync( async (token): Promise => { const res = await fetchSam('api/users/v2/self', _.mergeAll([authOpts(token), { method: 'GET' }])); return res.json(); @@ -276,12 +263,9 @@ export const User = (signal?: AbortSignal) => { return res.json(); }, - firstTimestamp: (): Promise => { - return getFirstTimeStamp(getTerraUser().token!); - }, - - registeredAtDate: (): Promise => { - return getRegisteredAtDate(getTerraUser().token!); + registeredAtDate: async (): Promise => { + const res = await getSamUserResponse(getTerraUser().token!); + return new Date(res.registeredAt!).getTime(); }, getNihStatus: async (): Promise => { From e720f8d59199f391383709265069a5f6b892d3f8 Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 12 Mar 2024 18:34:27 -0400 Subject: [PATCH 03/18] streamline and store samUser in state --- src/auth/auth.ts | 10 +++++++--- src/libs/ajax/User.ts | 17 ++++------------- src/libs/state.ts | 3 +++ 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index 2c250d12d8..974bd9d462 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -526,8 +526,9 @@ authStore.subscribe( withErrorIgnoring(async (state: AuthState, oldState: AuthState) => { if (!oldState.termsOfService.permitsSystemUsage && state.termsOfService.permitsSystemUsage) { if (window.Appcues) { - window.Appcues.identify(userStore.get().terraUser.id!, { - dateJoined: await Ajax().User.registeredAtDate(), + const { terraUser, samUser } = userStore.get(); + window.Appcues.identify(terraUser.id!, { + dateJoined: samUser!.registeredAt!.getTime(), }); window.Appcues.on('all', captureAppcuesEvent); } @@ -571,17 +572,20 @@ export const loadTerraUser = async (): Promise => { const getAllowances = Ajax().User.getUserAllowances(); const getAttributes = Ajax().User.getUserAttributes(); const getTermsOfService = Ajax().TermsOfService.getUserTermsOfServiceDetails(); - const [profile, terraUserAllowances, terraUserAttributes, termsOfService] = await Promise.all([ + const getSamUser = Ajax().User.getSamUserResponse(); + const [profile, terraUserAllowances, terraUserAttributes, termsOfService, samUser] = await Promise.all([ getProfile, getAllowances, getAttributes, getTermsOfService, + getSamUser, ]); clearNotification(sessionTimeoutProps.id); userStore.update((state: TerraUserState) => ({ ...state, profile, terraUserAttributes, + samUser, })); authStore.update((state: AuthState) => ({ ...state, diff --git a/src/libs/ajax/User.ts b/src/libs/ajax/User.ts index 3a86b3989c..b6278f4bdd 100644 --- a/src/libs/ajax/User.ts +++ b/src/libs/ajax/User.ts @@ -1,8 +1,7 @@ import _ from 'lodash/fp'; import * as qs from 'qs'; import { authOpts, fetchBond, fetchOrchestration, fetchSam, jsonBody } from 'src/libs/ajax/ajax-common'; -import { getTerraUser, TerraUserProfile } from 'src/libs/state'; -import * as Utils from 'src/libs/utils'; +import { TerraUserProfile } from 'src/libs/state'; export interface SamUserRegistrationStatusResponse { userSubjectId: string; @@ -175,14 +174,6 @@ export type SamUserAttributesRequest = { export type OrchestrationUserRegistrationRequest = object; -const getSamUserResponse = Utils.memoizeAsync( - async (token): Promise => { - const res = await fetchSam('api/users/v2/self', _.mergeAll([authOpts(token), { method: 'GET' }])); - return res.json(); - }, - { keyFn: (...args) => JSON.stringify(args) } -) as (token: string) => Promise; - export const User = (signal?: AbortSignal) => { return { getStatus: async (): Promise => { @@ -263,9 +254,9 @@ export const User = (signal?: AbortSignal) => { return res.json(); }, - registeredAtDate: async (): Promise => { - const res = await getSamUserResponse(getTerraUser().token!); - return new Date(res.registeredAt!).getTime(); + getSamUserResponse: async (): Promise => { + const res = await fetchSam('api/users/v2/self', _.mergeAll([authOpts(), { method: 'GET' }])); + return res.json(); }, getNihStatus: async (): Promise => { diff --git a/src/libs/state.ts b/src/libs/state.ts index 8806082c07..4cdb240fc0 100644 --- a/src/libs/state.ts +++ b/src/libs/state.ts @@ -11,6 +11,7 @@ import { NihDatasetPermission, SamUserAllowances, SamUserAttributes, + SamUserResponse, } from 'src/libs/ajax/User'; import { getLocalStorage, getSessionStorage, staticStorageSlot } from 'src/libs/browser-storage'; import type { WorkspaceWrapper } from 'src/workspaces/utils'; @@ -132,6 +133,7 @@ export interface TerraUserState { profile: TerraUserProfile; terraUser: TerraUser; terraUserAttributes: SamUserAttributes; + samUser: SamUserResponse | undefined; } /** @@ -165,6 +167,7 @@ export const userStore: Atom = atom({ terraUserAttributes: { marketingConsent: true, }, + samUser: undefined, }); export const getTerraUser = (): TerraUser => userStore.get().terraUser; From 1de9e4b0546dc9c28709ca2d78b1b60d0c97bd21 Mon Sep 17 00:00:00 2001 From: Kai Date: Wed, 13 Mar 2024 22:35:37 -0400 Subject: [PATCH 04/18] wip add unit tests --- src/auth/login.test.ts | 166 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 src/auth/login.test.ts diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts new file mode 100644 index 0000000000..ab26ff9dd9 --- /dev/null +++ b/src/auth/login.test.ts @@ -0,0 +1,166 @@ +import { expect } from '@storybook/test'; +import { DeepPartial } from '@terra-ui-packages/core-utils'; +import { asMockedFn } from '@terra-ui-packages/test-utils'; +import { act } from 'react-dom/test-utils'; +import { loadTerraUser } from 'src/auth/auth'; +import { Ajax } from 'src/libs/ajax'; +import { SamUserTermsOfServiceDetails } from 'src/libs/ajax/TermsOfService'; +import { SamUserResponse } from 'src/libs/ajax/User'; + +jest.mock('src/libs/ajax'); +jest.mock('react-notifications-component', () => { + return { + Store: { + addNotification: jest.fn(), + removeNotification: jest.fn(), + }, + }; +}); + +jest.mock('src/auth/oidc-broker.ts', () => ({ + ...jest.requireActual('src/auth/oidc-broker.ts'), + initializeOidcUserManager: () => { + return { + userManager: { + getUser: jest.fn().mockResolvedValue({}), + }, + }; + }, +})); + +const samUserDate = new Date('1970-01-01'); + +const mockSamUserResponse: SamUserResponse = { + id: 'string', + googleSubjectId: 'string', + email: 'string', + azureB2CId: 'string', + allowed: true, + createdAt: samUserDate, + registeredAt: samUserDate, + updatedAt: samUserDate, +}; + +const mockSamUserTermsOfServiceDetails: SamUserTermsOfServiceDetails = { + latestAcceptedVersion: '1234', + acceptedOn: samUserDate, + permitsSystemUsage: true, + isCurrentVersion: true, +}; + +describe('a request to load a terra user', () => { + describe('when successful', () => { + it('should include a samUserResponse', async () => { + // Arrange + const getProfileFunction = jest.fn().mockResolvedValue('testProfile'); + const getUserAllowancesFunction = jest.fn().mockResolvedValue('testAllowances'); + const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false }); + const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails); + const getSamUserResponseFunction = jest.fn().mockResolvedValue(mockSamUserResponse); + + type AjaxExports = typeof import('src/libs/ajax'); + type AjaxContract = ReturnType; + + // Act + asMockedFn(Ajax).mockImplementation( + () => + ({ + User: { + getProfile: getProfileFunction, + getUserAllowances: getUserAllowancesFunction, + getUserAttributes: getUserAttributesFunction, + getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction, + getSamUserResponse: getSamUserResponseFunction, + profile: { + get: jest.fn().mockReturnValue({}), + }, + }, + TermsOfService: { + getUserTermsOfServiceDetails: jest.fn().mockReturnValue({}), + }, + } as DeepPartial as AjaxContract) + ); + + await act(() => loadTerraUser()); + + // Assert + await expect(getSamUserResponseFunction).toHaveBeenCalled(); + }); + it('should update the samUser in state', async () => { + // Arrange + // type AjaxContract = ReturnType; + // type UserPartial = Partial; + // + // const samUserResponseFn = jest.fn().mockReturnValue(Promise.resolve()); + // + // asMockedFn(Ajax).mockImplementation( + // () => + // ({ + // User: { getUserAttributes: jest.fn().mockReturnValue(mockSamUserResponse) } as UserPartial, + // // User: { getSamUserResponse: jest.fn().mockReturnValue(mockSamUserResponse) } as UserPartial, + // } as AjaxContract) + // ); + // + // // Act + // await act(async () => { + // await User().getRegisteredAtDate(); + // }); + // + // // Arrange + // const getLinkStatusFn = jest.fn().mockResolvedValue(undefined); + // const getAuthorizationUrlFn = jest.fn().mockResolvedValue('https://foo.bar.com/oauth2/authorize'); + // asMockedFn(Ajax).mockImplementation( + // () => + // ({ + // ExternalCredentials: () => { + // return { + // getAccountLinkStatus: getLinkStatusFn, + // getAuthorizationUrl: getAuthorizationUrlFn, + // }; + // }, + // } as DeepPartial as AjaxContract) + // ); + // // Act + // const { container } = await act(() => render()); + // + // // Assert + // screen.getByText(`Link your ${testAccessTokenProvider.name} account`); + // expect(getLinkStatusFn).toHaveBeenCalled(); + // expect(getAuthorizationUrlFn).not.toHaveBeenCalled(); + // expect(await axe(container)).toHaveNoViolations(); + // Assert + // expect(samUserResponseFn).toBe(mockSamUserResponse); + }); + describe('when not successful', () => { + it('should fail with an error', async () => {}); + }); + }); +}); +// +// describe('a request to get the samUser', () => { +// it('should return a samUserResponse', async () => { +// // Arrange +// const getSamUserResponseFunction = jest.fn().mockResolvedValue(mockSamUserResponse); +// const getAuthTokenFromLocalStorageFunction = jest.fn().mockResolvedValue('mockToken'); +// +// type AjaxExports = typeof import('src/libs/ajax'); +// type AjaxContract = ReturnType; +// +// // Act +// asMockedFn(Ajax).mockImplementation( +// () => +// ({ +// User: { +// getSamUserResponse: getSamUserResponseFunction, +// }, +// getAuthTokenFromLocalStorage: getAuthTokenFromLocalStorageFunction, +// } as DeepPartial as AjaxContract) +// ); +// +// initializeOidcUserManager(); +// const samUserResponse = await User().getSamUserResponse(); +// +// // Assert +// await expect(samUserResponse).toBe(mockSamUserResponse); +// }); +// }); From 36e309646c0833a781e05f73771a16f742599d44 Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 18 Mar 2024 17:50:29 -0400 Subject: [PATCH 05/18] add test for failure case --- src/auth/login.test.ts | 200 ++++++++++++++++++----------------------- 1 file changed, 85 insertions(+), 115 deletions(-) diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index ab26ff9dd9..c23f67343b 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -6,6 +6,7 @@ import { loadTerraUser } from 'src/auth/auth'; import { Ajax } from 'src/libs/ajax'; import { SamUserTermsOfServiceDetails } from 'src/libs/ajax/TermsOfService'; import { SamUserResponse } from 'src/libs/ajax/User'; +import { TerraUserState, userStore } from 'src/libs/state'; jest.mock('src/libs/ajax'); jest.mock('react-notifications-component', () => { @@ -17,24 +18,13 @@ jest.mock('react-notifications-component', () => { }; }); -jest.mock('src/auth/oidc-broker.ts', () => ({ - ...jest.requireActual('src/auth/oidc-broker.ts'), - initializeOidcUserManager: () => { - return { - userManager: { - getUser: jest.fn().mockResolvedValue({}), - }, - }; - }, -})); - const samUserDate = new Date('1970-01-01'); const mockSamUserResponse: SamUserResponse = { - id: 'string', - googleSubjectId: 'string', - email: 'string', - azureB2CId: 'string', + id: 'testId', + googleSubjectId: 'testGoogleSubjectId', + email: 'testEmail', + azureB2CId: 'testAzureB2CId', allowed: true, createdAt: samUserDate, registeredAt: samUserDate, @@ -48,119 +38,99 @@ const mockSamUserTermsOfServiceDetails: SamUserTermsOfServiceDetails = { isCurrentVersion: true, }; +type AjaxExports = typeof import('src/libs/ajax'); +type AjaxContract = ReturnType; + describe('a request to load a terra user', () => { + // reset userStore before each test + beforeEach(() => { + userStore.reset; + }); describe('when successful', () => { - it('should include a samUserResponse', async () => { - // Arrange - const getProfileFunction = jest.fn().mockResolvedValue('testProfile'); - const getUserAllowancesFunction = jest.fn().mockResolvedValue('testAllowances'); - const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false }); - const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails); - const getSamUserResponseFunction = jest.fn().mockResolvedValue(mockSamUserResponse); - - type AjaxExports = typeof import('src/libs/ajax'); - type AjaxContract = ReturnType; + // Arrange (shared between tests for the success case) + const getProfileFunction = jest.fn().mockResolvedValue('testProfile'); + const getUserAllowancesFunction = jest.fn().mockResolvedValue('testAllowances'); + const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false }); + const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails); + const getSamUserResponseFunction = jest.fn().mockResolvedValue(mockSamUserResponse); - // Act - asMockedFn(Ajax).mockImplementation( - () => - ({ - User: { - getProfile: getProfileFunction, - getUserAllowances: getUserAllowancesFunction, - getUserAttributes: getUserAttributesFunction, - getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction, - getSamUserResponse: getSamUserResponseFunction, - profile: { - get: jest.fn().mockReturnValue({}), - }, + asMockedFn(Ajax).mockImplementation( + () => + ({ + User: { + getProfile: getProfileFunction, + getUserAllowances: getUserAllowancesFunction, + getUserAttributes: getUserAttributesFunction, + getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction, + getSamUserResponse: getSamUserResponseFunction, + profile: { + get: jest.fn().mockReturnValue({}), }, - TermsOfService: { - getUserTermsOfServiceDetails: jest.fn().mockReturnValue({}), - }, - } as DeepPartial as AjaxContract) - ); - + }, + TermsOfService: { + getUserTermsOfServiceDetails: jest.fn().mockReturnValue({}), + }, + } as DeepPartial as AjaxContract) + ); + it('should include a samUserResponse', async () => { + // Act await act(() => loadTerraUser()); // Assert await expect(getSamUserResponseFunction).toHaveBeenCalled(); }); it('should update the samUser in state', async () => { - // Arrange - // type AjaxContract = ReturnType; - // type UserPartial = Partial; - // - // const samUserResponseFn = jest.fn().mockReturnValue(Promise.resolve()); - // - // asMockedFn(Ajax).mockImplementation( - // () => - // ({ - // User: { getUserAttributes: jest.fn().mockReturnValue(mockSamUserResponse) } as UserPartial, - // // User: { getSamUserResponse: jest.fn().mockReturnValue(mockSamUserResponse) } as UserPartial, - // } as AjaxContract) - // ); - // - // // Act - // await act(async () => { - // await User().getRegisteredAtDate(); - // }); - // - // // Arrange - // const getLinkStatusFn = jest.fn().mockResolvedValue(undefined); - // const getAuthorizationUrlFn = jest.fn().mockResolvedValue('https://foo.bar.com/oauth2/authorize'); - // asMockedFn(Ajax).mockImplementation( - // () => - // ({ - // ExternalCredentials: () => { - // return { - // getAccountLinkStatus: getLinkStatusFn, - // getAuthorizationUrl: getAuthorizationUrlFn, - // }; - // }, - // } as DeepPartial as AjaxContract) - // ); - // // Act - // const { container } = await act(() => render()); - // - // // Assert - // screen.getByText(`Link your ${testAccessTokenProvider.name} account`); - // expect(getLinkStatusFn).toHaveBeenCalled(); - // expect(getAuthorizationUrlFn).not.toHaveBeenCalled(); - // expect(await axe(container)).toHaveNoViolations(); + // Act + await act(() => loadTerraUser()); + + let samUser; + await act(async () => { + samUser = await getSamUserResponseFunction.mock.results[0].value; + }); + userStore.update((state: TerraUserState) => ({ + ...state, + samUser, + })); // Assert - // expect(samUserResponseFn).toBe(mockSamUserResponse); + await expect(getSamUserResponseFunction).toHaveBeenCalled(); + await expect(userStore.get().samUser).toEqual(mockSamUserResponse); }); describe('when not successful', () => { - it('should fail with an error', async () => {}); + it('should fail with an error', async () => { + // Arrange + const getProfileFunction = jest.fn().mockResolvedValue('testProfile'); + const getUserAllowancesFunction = jest.fn().mockResolvedValue('testAllowances'); + const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false }); + const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails); + // mock a failure to get samUserResponse + const getSamUserResponseFunction = jest.fn().mockRejectedValue(new Error('unknown')); + + asMockedFn(Ajax).mockImplementation( + () => + ({ + User: { + getProfile: getProfileFunction, + getUserAllowances: getUserAllowancesFunction, + getUserAttributes: getUserAttributesFunction, + getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction, + getSamUserResponse: getSamUserResponseFunction, + profile: { + get: jest.fn().mockReturnValue({}), + }, + }, + TermsOfService: { + getUserTermsOfServiceDetails: jest.fn().mockReturnValue({}), + }, + } as DeepPartial as AjaxContract) + ); + // Act, Assert + await expect.assertions(1); + try { + await act(() => loadTerraUser()); + } catch (error) { + await expect(error).toEqual(new Error('unknown')); + } + }); }); }); }); -// -// describe('a request to get the samUser', () => { -// it('should return a samUserResponse', async () => { -// // Arrange -// const getSamUserResponseFunction = jest.fn().mockResolvedValue(mockSamUserResponse); -// const getAuthTokenFromLocalStorageFunction = jest.fn().mockResolvedValue('mockToken'); -// -// type AjaxExports = typeof import('src/libs/ajax'); -// type AjaxContract = ReturnType; -// -// // Act -// asMockedFn(Ajax).mockImplementation( -// () => -// ({ -// User: { -// getSamUserResponse: getSamUserResponseFunction, -// }, -// getAuthTokenFromLocalStorage: getAuthTokenFromLocalStorageFunction, -// } as DeepPartial as AjaxContract) -// ); -// -// initializeOidcUserManager(); -// const samUserResponse = await User().getSamUserResponse(); -// -// // Assert -// await expect(samUserResponse).toBe(mockSamUserResponse); -// }); -// }); From 24c91252da587a1dc031edfa4bf18b01f4755922 Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 18 Mar 2024 17:56:46 -0400 Subject: [PATCH 06/18] remove leftover rex code --- config/alpha.json | 1 - config/dev.json | 1 - config/prod.json | 1 - config/staging.json | 1 - src/libs/ajax/ajax-common.ts | 5 ----- 5 files changed, 9 deletions(-) diff --git a/config/alpha.json b/config/alpha.json index 65355ac867..8088016149 100644 --- a/config/alpha.json +++ b/config/alpha.json @@ -20,7 +20,6 @@ "leoUrlRoot": "https://leonardo.dsde-alpha.broadinstitute.org", "orchestrationUrlRoot": "https://firecloud-orchestration.dsde-alpha.broadinstitute.org", "rawlsUrlRoot": "https://rawls.dsde-alpha.broadinstitute.org", - "rexUrlRoot": "https://terra-rex-alpha.appspot.com", "samUrlRoot": "https://sam.dsde-alpha.broadinstitute.org", "shibbolethUrlRoot": "https://broad-shibboleth-prod.appspot.com/dev", "workspaceManagerUrlRoot": "https://workspace.dsde-alpha.broadinstitute.org", diff --git a/config/dev.json b/config/dev.json index 97cc4f02cc..3dc6c41a07 100644 --- a/config/dev.json +++ b/config/dev.json @@ -20,7 +20,6 @@ "leoUrlRoot": "https://leonardo.dsde-dev.broadinstitute.org", "orchestrationUrlRoot": "https://firecloud-orchestration.dsde-dev.broadinstitute.org", "rawlsUrlRoot": "https://rawls.dsde-dev.broadinstitute.org", - "rexUrlRoot": "https://terra-rex-dev.appspot.com", "samUrlRoot": "https://sam.dsde-dev.broadinstitute.org", "shibbolethUrlRoot": "https://broad-shibboleth-prod.appspot.com/dev", "workspaceManagerUrlRoot": "https://workspace.dsde-dev.broadinstitute.org", diff --git a/config/prod.json b/config/prod.json index 9c4c3710e9..3acb539bc1 100644 --- a/config/prod.json +++ b/config/prod.json @@ -19,7 +19,6 @@ "leoUrlRoot": "https://notebooks.firecloud.org", "orchestrationUrlRoot": "https://api.firecloud.org", "rawlsUrlRoot": "https://rawls.dsde-prod.broadinstitute.org", - "rexUrlRoot": "https://terra-rex-prod.appspot.com", "samUrlRoot": "https://sam.dsde-prod.broadinstitute.org", "shibbolethUrlRoot": "https://broad-shibboleth-prod.appspot.com", "workspaceManagerUrlRoot": "https://workspace.dsde-prod.broadinstitute.org", diff --git a/config/staging.json b/config/staging.json index 651227549b..15d185dbaa 100644 --- a/config/staging.json +++ b/config/staging.json @@ -20,7 +20,6 @@ "leoUrlRoot": "https://leonardo.dsde-staging.broadinstitute.org", "orchestrationUrlRoot": "https://firecloud-orchestration.dsde-staging.broadinstitute.org", "rawlsUrlRoot": "https://rawls.dsde-staging.broadinstitute.org", - "rexUrlRoot": "https://terra-rex-staging.appspot.com", "samUrlRoot": "https://sam.dsde-staging.broadinstitute.org", "shibbolethUrlRoot": "https://broad-shibboleth-prod.appspot.com/dev", "workspaceManagerUrlRoot": "https://workspace.dsde-staging.broadinstitute.org", diff --git a/src/libs/ajax/ajax-common.ts b/src/libs/ajax/ajax-common.ts index bc1d147207..fc8844ad2f 100644 --- a/src/libs/ajax/ajax-common.ts +++ b/src/libs/ajax/ajax-common.ts @@ -259,11 +259,6 @@ export const fetchOrchestration = _.flow( withRetryAfterReloadingExpiredAuthToken )(fetchOk); -export const fetchRex = _.flow( - withUrlPrefix(`${getConfig().rexUrlRoot}/api/`), - withRetryAfterReloadingExpiredAuthToken -)(fetchOk); - export const fetchBond = _.flow( withUrlPrefix(`${getConfig().bondUrlRoot}/`), withRetryAfterReloadingExpiredAuthToken From b4ddadc296d79348daea544316aa5208aeeeb1bb Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 18 Mar 2024 21:59:35 -0400 Subject: [PATCH 07/18] reorder const for consistency --- src/auth/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index 499809b2f8..7196da3db7 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -572,8 +572,8 @@ export const loadTerraUser = async (): Promise => { const getAllowances = Ajax().User.getUserAllowances(); const getAttributes = Ajax().User.getUserAttributes(); const getTermsOfService = Ajax().TermsOfService.getUserTermsOfServiceDetails(); - const getSamUser = Ajax().User.getSamUserResponse(); const getEnterpriseFeatures = Ajax().User.getEnterpriseFeatures(); + const getSamUser = Ajax().User.getSamUserResponse(); const [profile, terraUserAllowances, terraUserAttributes, termsOfService, enterpriseFeatures, samUser] = await Promise.all([ getProfile, From 6223873f93ece36728f131fe177c0f1e910a8fd1 Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 18 Mar 2024 22:12:56 -0400 Subject: [PATCH 08/18] fix tests after merge from dev --- src/auth/login.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index c23f67343b..8f0c585da0 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -52,6 +52,7 @@ describe('a request to load a terra user', () => { const getUserAllowancesFunction = jest.fn().mockResolvedValue('testAllowances'); const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false }); const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails); + const getEnterpriseFeaturesFunction = jest.fn().mockResolvedValue('testEnterpriseFeatures'); const getSamUserResponseFunction = jest.fn().mockResolvedValue(mockSamUserResponse); asMockedFn(Ajax).mockImplementation( @@ -62,6 +63,7 @@ describe('a request to load a terra user', () => { getUserAllowances: getUserAllowancesFunction, getUserAttributes: getUserAttributesFunction, getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction, + getEnterpriseFeatures: getEnterpriseFeaturesFunction, getSamUserResponse: getSamUserResponseFunction, profile: { get: jest.fn().mockReturnValue({}), @@ -101,6 +103,7 @@ describe('a request to load a terra user', () => { const getProfileFunction = jest.fn().mockResolvedValue('testProfile'); const getUserAllowancesFunction = jest.fn().mockResolvedValue('testAllowances'); const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false }); + const getEnterpriseFeaturesFunction = jest.fn().mockResolvedValue('testEnterpriseFeatures'); const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails); // mock a failure to get samUserResponse const getSamUserResponseFunction = jest.fn().mockRejectedValue(new Error('unknown')); @@ -113,6 +116,7 @@ describe('a request to load a terra user', () => { getUserAllowances: getUserAllowancesFunction, getUserAttributes: getUserAttributesFunction, getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction, + getEnterpriseFeatures: getEnterpriseFeaturesFunction, getSamUserResponse: getSamUserResponseFunction, profile: { get: jest.fn().mockReturnValue({}), From ef7e8807899402df4efdd6aecf4fbbbf3550bed1 Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 18 Mar 2024 23:23:05 -0400 Subject: [PATCH 09/18] fix wonky imports --- src/auth/login.test.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index 8f0c585da0..10dec7ea78 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -1,7 +1,6 @@ -import { expect } from '@storybook/test'; import { DeepPartial } from '@terra-ui-packages/core-utils'; import { asMockedFn } from '@terra-ui-packages/test-utils'; -import { act } from 'react-dom/test-utils'; +import { act } from '@testing-library/react'; import { loadTerraUser } from 'src/auth/auth'; import { Ajax } from 'src/libs/ajax'; import { SamUserTermsOfServiceDetails } from 'src/libs/ajax/TermsOfService'; @@ -79,7 +78,7 @@ describe('a request to load a terra user', () => { await act(() => loadTerraUser()); // Assert - await expect(getSamUserResponseFunction).toHaveBeenCalled(); + expect(getSamUserResponseFunction).toHaveBeenCalled(); }); it('should update the samUser in state', async () => { // Act @@ -94,8 +93,8 @@ describe('a request to load a terra user', () => { samUser, })); // Assert - await expect(getSamUserResponseFunction).toHaveBeenCalled(); - await expect(userStore.get().samUser).toEqual(mockSamUserResponse); + expect(getSamUserResponseFunction).toHaveBeenCalled(); + expect(userStore.get().samUser).toEqual(mockSamUserResponse); }); describe('when not successful', () => { it('should fail with an error', async () => { @@ -128,11 +127,11 @@ describe('a request to load a terra user', () => { } as DeepPartial as AjaxContract) ); // Act, Assert - await expect.assertions(1); + expect.assertions(1); try { await act(() => loadTerraUser()); } catch (error) { - await expect(error).toEqual(new Error('unknown')); + expect(error).toEqual(new Error('unknown')); } }); }); From b964235e818361d42987c622c3e306dd4d718d6a Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 18 Mar 2024 23:52:07 -0400 Subject: [PATCH 10/18] fix one more test --- .../terms-of-service/TermsOfServicePage.test.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/registration/terms-of-service/TermsOfServicePage.test.ts b/src/registration/terms-of-service/TermsOfServicePage.test.ts index c45877408f..d006a42423 100644 --- a/src/registration/terms-of-service/TermsOfServicePage.test.ts +++ b/src/registration/terms-of-service/TermsOfServicePage.test.ts @@ -3,7 +3,7 @@ import { act, fireEvent, screen } from '@testing-library/react'; import { h } from 'react-hyperscript-helpers'; import { Ajax } from 'src/libs/ajax'; import { SamUserTermsOfServiceDetails } from 'src/libs/ajax/TermsOfService'; -import { SamUserAllowances } from 'src/libs/ajax/User'; +import { SamUserAllowances, SamUserResponse } from 'src/libs/ajax/User'; import { AuthState, authStore } from 'src/libs/state'; import { TermsOfServicePage } from 'src/registration/terms-of-service/TermsOfServicePage'; import { asMockedFn, renderWithAppContexts as render } from 'src/testing/test-utils'; @@ -52,7 +52,16 @@ const setupMockAjax = async ( allowed: termsOfService.permitsSystemUsage, details: { enabled: true, termsOfService: termsOfService.permitsSystemUsage }, }; - + const mockSamUserResponse: SamUserResponse = { + id: 'testId', + googleSubjectId: 'testGoogleSubjectId', + email: 'testEmail', + azureB2CId: 'testAzureB2CId', + allowed: true, + createdAt: new Date('1970-01-01'), + registeredAt: new Date('1970-01-01'), + updatedAt: new Date('1970-01-01'), + }; const getTermsOfServiceText = jest.fn().mockResolvedValue('some text'); const getUserTermsOfServiceDetails = jest .fn() @@ -75,6 +84,7 @@ const setupMockAjax = async ( getUserAttributes: jest.fn().mockResolvedValue({ marketingConsent: true }), getUserAllowances: jest.fn().mockResolvedValue(terraUserAllowances), getEnterpriseFeatures: jest.fn().mockResolvedValue([]), + getSamUserResponse: jest.fn().mockResolvedValue(mockSamUserResponse), profile: { get: jest.fn().mockResolvedValue({ keyValuePairs: [] }), update: jest.fn().mockResolvedValue({ keyValuePairs: [] }), From ce2271115470ece8ff9aeca4a8b4329bfedbd909 Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 19 Mar 2024 12:50:13 -0400 Subject: [PATCH 11/18] make mock return values more realistic --- src/auth/login.test.ts | 75 +++++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 16 deletions(-) diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index 10dec7ea78..68c69cd34e 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -3,11 +3,16 @@ import { asMockedFn } from '@terra-ui-packages/test-utils'; import { act } from '@testing-library/react'; import { loadTerraUser } from 'src/auth/auth'; import { Ajax } from 'src/libs/ajax'; +import { GroupRole } from 'src/libs/ajax/Groups'; import { SamUserTermsOfServiceDetails } from 'src/libs/ajax/TermsOfService'; import { SamUserResponse } from 'src/libs/ajax/User'; import { TerraUserState, userStore } from 'src/libs/state'; jest.mock('src/libs/ajax'); + +type AjaxExports = typeof import('src/libs/ajax'); +type AjaxContract = ReturnType; + jest.mock('react-notifications-component', () => { return { Store: { @@ -37,40 +42,84 @@ const mockSamUserTermsOfServiceDetails: SamUserTermsOfServiceDetails = { isCurrentVersion: true, }; -type AjaxExports = typeof import('src/libs/ajax'); -type AjaxContract = ReturnType; +const mockTerraUserProfile = { + firstName: 'testFirstName', + lastName: 'testLastName', + institute: 'testInstitute', + contactEmail: 'testContactEmail', + title: 'testTitle', + department: 'testDepartment', + interestInTerra: 'testInterestInTerra', + programLocationCity: 'testProgramLocationCity', + programLocationState: 'testProgramLocationState', + programLocationCountry: 'testProgramLocationCountry', + researchArea: 'testResearchArea', + starredWorkspaces: 'testStarredWorkspaces', +}; + +const testSamUserAllowancesDetails = { + enabled: true, + termsOfService: true, +}; + +const testSamUserAllowances = { + allowed: true, + details: testSamUserAllowancesDetails, +}; + +const mockNihDatasetPermission = { + name: 'testNihDatasetPermissionName', + authorized: true, +}; + +const mockOrchestrationNihStatusResponse = { + linkedNihUsername: 'testLinkedNihUsername', + datasetPermissions: mockNihDatasetPermission, + linkExpireTime: 1234, +}; + +const mockCurrentUserGroupMembership = { + groupEmail: 'testGroupEmail', + groupName: 'testGroupName', + role: 'member' as GroupRole, +}; describe('a request to load a terra user', () => { - // reset userStore before each test + // reset userStore state before each test beforeEach(() => { userStore.reset; }); describe('when successful', () => { // Arrange (shared between tests for the success case) - const getProfileFunction = jest.fn().mockResolvedValue('testProfile'); - const getUserAllowancesFunction = jest.fn().mockResolvedValue('testAllowances'); + const getUserAllowancesFunction = jest.fn().mockResolvedValue(testSamUserAllowances); const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false }); const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails); - const getEnterpriseFeaturesFunction = jest.fn().mockResolvedValue('testEnterpriseFeatures'); + const getEnterpriseFeaturesFunction = jest.fn().mockResolvedValue([]); const getSamUserResponseFunction = jest.fn().mockResolvedValue(mockSamUserResponse); + const getNihStatusFunction = jest.fn().mockResolvedValue(mockOrchestrationNihStatusResponse); + const getFenceStatusFunction = jest.fn().mockResolvedValue({}); asMockedFn(Ajax).mockImplementation( () => ({ User: { - getProfile: getProfileFunction, getUserAllowances: getUserAllowancesFunction, getUserAttributes: getUserAttributesFunction, getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction, getEnterpriseFeatures: getEnterpriseFeaturesFunction, getSamUserResponse: getSamUserResponseFunction, + getNihStatus: getNihStatusFunction, + getFenceStatus: getFenceStatusFunction, profile: { - get: jest.fn().mockReturnValue({}), + get: jest.fn().mockReturnValue(mockTerraUserProfile), }, }, TermsOfService: { getUserTermsOfServiceDetails: jest.fn().mockReturnValue({}), }, + Groups: { + list: jest.fn().mockReturnValue([mockCurrentUserGroupMembership]), + }, } as DeepPartial as AjaxContract) ); it('should include a samUserResponse', async () => { @@ -98,12 +147,7 @@ describe('a request to load a terra user', () => { }); describe('when not successful', () => { it('should fail with an error', async () => { - // Arrange - const getProfileFunction = jest.fn().mockResolvedValue('testProfile'); - const getUserAllowancesFunction = jest.fn().mockResolvedValue('testAllowances'); - const getUserAttributesFunction = jest.fn().mockResolvedValue({ marketingConsent: false }); - const getEnterpriseFeaturesFunction = jest.fn().mockResolvedValue('testEnterpriseFeatures'); - const getUserTermsOfServiceDetailsFunction = jest.fn().mockResolvedValue(mockSamUserTermsOfServiceDetails); + // // Arrange // mock a failure to get samUserResponse const getSamUserResponseFunction = jest.fn().mockRejectedValue(new Error('unknown')); @@ -111,14 +155,13 @@ describe('a request to load a terra user', () => { () => ({ User: { - getProfile: getProfileFunction, getUserAllowances: getUserAllowancesFunction, getUserAttributes: getUserAttributesFunction, getUserTermsOfServiceDetails: getUserTermsOfServiceDetailsFunction, getEnterpriseFeatures: getEnterpriseFeaturesFunction, getSamUserResponse: getSamUserResponseFunction, profile: { - get: jest.fn().mockReturnValue({}), + get: jest.fn().mockReturnValue(mockTerraUserProfile), }, }, TermsOfService: { From 0218e8da2906e3f2cf1d5171cbbf2975cb0062f0 Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 19 Mar 2024 16:39:18 -0400 Subject: [PATCH 12/18] fully initialize samUserResponse in state --- src/auth/auth.ts | 2 +- src/auth/login.test.ts | 1 + src/libs/ajax/User.ts | 16 ++++++++-------- src/libs/state.ts | 13 +++++++++++-- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index 7196da3db7..4893181fc0 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -528,7 +528,7 @@ authStore.subscribe( if (window.Appcues) { const { terraUser, samUser } = userStore.get(); window.Appcues.identify(terraUser.id!, { - dateJoined: samUser!.registeredAt!.getTime(), + dateJoined: samUser.registeredAt!.getTime(), }); window.Appcues.on('all', captureAppcuesEvent); } diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index 68c69cd34e..fa729a53ae 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -84,6 +84,7 @@ const mockCurrentUserGroupMembership = { role: 'member' as GroupRole, }; +// TODO centralize Ajax mock setup so it can be reused across tests describe('a request to load a terra user', () => { // reset userStore state before each test beforeEach(() => { diff --git a/src/libs/ajax/User.ts b/src/libs/ajax/User.ts index 129740e144..37e5d22df7 100644 --- a/src/libs/ajax/User.ts +++ b/src/libs/ajax/User.ts @@ -154,14 +154,14 @@ export interface SamInviteUserResponse { } export interface SamUserResponse { - id: string; - googleSubjectId?: string; - email: string; - azureB2CId?: string; - allowed: boolean; - createdAt: Date; - registeredAt?: Date; - updatedAt: Date; + id: string | undefined; + googleSubjectId?: string | undefined; + email: string | undefined; + azureB2CId?: string | undefined; + allowed: boolean | undefined; + createdAt: Date | undefined; + registeredAt?: Date | undefined; + updatedAt: Date | undefined; } export type SamUserAttributes = { diff --git a/src/libs/state.ts b/src/libs/state.ts index a03a3ea56f..8b1cb584bd 100644 --- a/src/libs/state.ts +++ b/src/libs/state.ts @@ -134,7 +134,7 @@ export interface TerraUserState { terraUser: TerraUser; terraUserAttributes: SamUserAttributes; enterpriseFeatures: string[]; - samUser: SamUserResponse | undefined; + samUser: SamUserResponse; } /** @@ -169,7 +169,16 @@ export const userStore: Atom = atom({ marketingConsent: true, }, enterpriseFeatures: [], - samUser: undefined, + samUser: { + id: undefined, + googleSubjectId: undefined, + email: undefined, + azureB2CId: undefined, + allowed: undefined, + createdAt: undefined, + registeredAt: undefined, + updatedAt: undefined, + }, }); export const getTerraUser = (): TerraUser => userStore.get().terraUser; From a1bef943df7394bb964cc3f329dec618b741c489 Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 19 Mar 2024 16:42:44 -0400 Subject: [PATCH 13/18] add explanatory comment --- src/auth/login.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index fa729a53ae..02df2323f7 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -171,6 +171,7 @@ describe('a request to load a terra user', () => { } as DeepPartial as AjaxContract) ); // Act, Assert + // this expect.assertions is here to prevent the test from passing if the error is not thrown expect.assertions(1); try { await act(() => loadTerraUser()); From bb7bd0a475f247d847a38e8d0110976ef0ee56a3 Mon Sep 17 00:00:00 2001 From: Kai Date: Wed, 20 Mar 2024 13:03:22 -0400 Subject: [PATCH 14/18] handle if registeredAt is null in the sam db --- src/auth/auth.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index 4893181fc0..3f1193592a 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -527,8 +527,11 @@ authStore.subscribe( if (!oldState.termsOfService.permitsSystemUsage && state.termsOfService.permitsSystemUsage) { if (window.Appcues) { const { terraUser, samUser } = userStore.get(); + // for Sam users who registered before we started tracking registration dates in Sam, + // registeredAt may be null in the Sam db. In that case, use epoch instead + const dateJoined = samUser.registeredAt ? samUser.registeredAt.getTime() : new Date('1970-01-01').getTime(); window.Appcues.identify(terraUser.id!, { - dateJoined: samUser.registeredAt!.getTime(), + dateJoined, }); window.Appcues.on('all', captureAppcuesEvent); } From b7dd27f0ab1b40f205a88ada7ad23d2360d6f526 Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 25 Mar 2024 15:02:55 -0400 Subject: [PATCH 15/18] simplify after updates in Sam --- src/auth/auth.ts | 5 +---- src/libs/ajax/User.ts | 12 ++++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index 3f1193592a..d9c7b9fbe6 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -527,11 +527,8 @@ authStore.subscribe( if (!oldState.termsOfService.permitsSystemUsage && state.termsOfService.permitsSystemUsage) { if (window.Appcues) { const { terraUser, samUser } = userStore.get(); - // for Sam users who registered before we started tracking registration dates in Sam, - // registeredAt may be null in the Sam db. In that case, use epoch instead - const dateJoined = samUser.registeredAt ? samUser.registeredAt.getTime() : new Date('1970-01-01').getTime(); window.Appcues.identify(terraUser.id!, { - dateJoined, + dateJoined: samUser.registeredAt.getTime(), }); window.Appcues.on('all', captureAppcuesEvent); } diff --git a/src/libs/ajax/User.ts b/src/libs/ajax/User.ts index a1967caf5d..32d089b39f 100644 --- a/src/libs/ajax/User.ts +++ b/src/libs/ajax/User.ts @@ -154,14 +154,14 @@ export interface SamInviteUserResponse { } export interface SamUserResponse { - id: string | undefined; + id: string; googleSubjectId?: string | undefined; - email: string | undefined; + email: string; azureB2CId?: string | undefined; - allowed: boolean | undefined; - createdAt: Date | undefined; - registeredAt?: Date | undefined; - updatedAt: Date | undefined; + allowed: boolean; + createdAt: Date; + registeredAt: Date; + updatedAt: Date; } export type SamUserAttributes = { From 5dbf432cca0f982e2436c44235fdc2d1294e402b Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 26 Mar 2024 11:46:24 -0400 Subject: [PATCH 16/18] put back ui default to epoch --- src/auth/auth.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index d9c7b9fbe6..88e651eaa4 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -527,8 +527,13 @@ authStore.subscribe( if (!oldState.termsOfService.permitsSystemUsage && state.termsOfService.permitsSystemUsage) { if (window.Appcues) { const { terraUser, samUser } = userStore.get(); + // for Sam users who have been invited but not yet registered + // and for a set of users who didn't have registration dates to migrate into Sam + // registeredAt may be null in the Sam db. In that case, default to epoch (1970) instead + // so the survey won't be immediately displayed + const dateJoined = samUser.registeredAt ? samUser.registeredAt.getTime() : new Date('1970-01-01').getTime(); window.Appcues.identify(terraUser.id!, { - dateJoined: samUser.registeredAt.getTime(), + dateJoined, }); window.Appcues.on('all', captureAppcuesEvent); } From 6815a6438343d977335390d851d093ef1f3db22e Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 26 Mar 2024 12:07:24 -0400 Subject: [PATCH 17/18] allow initialization of samuser with undefined --- src/libs/ajax/User.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libs/ajax/User.ts b/src/libs/ajax/User.ts index 32d089b39f..543dae8efa 100644 --- a/src/libs/ajax/User.ts +++ b/src/libs/ajax/User.ts @@ -154,14 +154,14 @@ export interface SamInviteUserResponse { } export interface SamUserResponse { - id: string; + id: string | undefined; googleSubjectId?: string | undefined; - email: string; + email: string | undefined; azureB2CId?: string | undefined; - allowed: boolean; - createdAt: Date; - registeredAt: Date; - updatedAt: Date; + allowed: boolean | undefined; + createdAt: Date | undefined; + registeredAt: Date | undefined; + updatedAt: Date | undefined; } export type SamUserAttributes = { From 534a5a948af0059fe2b2c5d8e03f1570629e0802 Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 26 Mar 2024 12:33:05 -0400 Subject: [PATCH 18/18] nit remove extra undefined on optional fields --- src/libs/ajax/User.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/ajax/User.ts b/src/libs/ajax/User.ts index 543dae8efa..8c19a0627e 100644 --- a/src/libs/ajax/User.ts +++ b/src/libs/ajax/User.ts @@ -155,9 +155,9 @@ export interface SamInviteUserResponse { export interface SamUserResponse { id: string | undefined; - googleSubjectId?: string | undefined; + googleSubjectId?: string; email: string | undefined; - azureB2CId?: string | undefined; + azureB2CId?: string; allowed: boolean | undefined; createdAt: Date | undefined; registeredAt: Date | undefined;