Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DT-979: Add Appcues Support #2722

Merged
merged 10 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions DEVNOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ docker build . -t duos
docker compose up -d
```

Visit https://local.dsde-dev.broadinstitute.org/ to see the instance running under docker.

# Testing

## Cypress Tests
Expand Down
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ ENV PATH /usr/src/app/node_modules/.bin:$PATH

# install and cache app dependencies
COPY src /usr/src/app/src
COPY types /usr/src/app/types
COPY public /usr/src/app/public
COPY package.json /usr/src/app/package.json
COPY package-lock.json /usr/src/app/package-lock.json
Expand Down
42 changes: 42 additions & 0 deletions cypress/component/utils/metrics.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/* eslint-disable no-undef */
import {Metrics} from '../../../src/libs/ajax/Metrics';
import eventList from '../../../src/libs/events';

describe('Metrics Tests', function () {

// Intercept configuration calls
beforeEach(() => {
cy.intercept({
method: 'GET',
url: '/config.json',
hostname: 'localhost',
}, {'env': 'ci'});
});

Cypress._.each(Object.keys(eventList), (eventType) => {
it(`Captures ${eventType} Event`, function () {
cy.intercept('**/event').as('event');
Metrics.captureEvent(eventType);
cy.wait('@event').then(interception => {
expect(interception).to.exist;
});
});
});

it(`Sync Profile`, function () {
cy.intercept('**/syncProfile').as('sync');
Metrics.syncProfile();
cy.wait('@sync').then(interception => {
expect(interception).to.exist;
});
});

it(`Identify`, function () {
cy.intercept('**/identify').as('identify');
Metrics.identify('anonymousId');
cy.wait('@identify').then(interception => {
expect(interception).to.exist;
});
});

});
5 changes: 4 additions & 1 deletion cypress/support/component-index.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width,initial-scale=1.0">
<script src="https://accounts.google.com/gsi/client" async defer id="gsiClient"></script>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another drive by fix - we removed "google sign in" in #2667

<title>Components App</title>
<!-- AppCues Requirements -->
<script type="text/javascript">
window.AppcuesSettings = { enableURLDetection: true };
</script>
Comment on lines +9 to +12
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to testing suggestions here. I was unable to figure out a way to unit test the global Appcues window object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at some point it is worth trusting the library. We could add tests that verify we're setting the right values, but as far as appcues itself, I think that's probably out of our purview to test? But happy to have that contested

</head>

<body>
Expand Down
10 changes: 9 additions & 1 deletion public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
<link rel="icon" type="image/png" href="%PUBLIC_URL%/images/favicon/favicon-32x32.png" sizes="32x32" />
<link rel="icon" type="image/png" href="%PUBLIC_URL%/images/favicon/favicon-16x16.png" sizes="16x16" />
<link rel="mask-icon" href="%PUBLIC_URL%/images/favicon/safari-pinned-tab.svg">
<script src="https://accounts.google.com/gsi/client" async defer id="gsiClient"></script>
Copy link
Contributor Author

@rushtong rushtong Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive by fix - we removed "google sign in" in #2667

<!--
Notice the use of %PUBLIC_URL% in the tags above.
It will be replaced with the URL of the `public` folder during the build.
Expand All @@ -32,6 +31,15 @@
work correctly both with client-side routing and a non-root public URL.
Learn how to configure a non-root public URL by running `npm run build`.
-->

<!-- AppCues Requirements -->
<script type="text/javascript">
window.AppcuesSettings = { enableURLDetection: true };
</script>
<script src="https://fast.appcues.com/49767.js">
</script>
<!-- End AppCues Requirements -->

<title>Broad Data Use Oversight System</title>
</head>

Expand Down
18 changes: 9 additions & 9 deletions src/components/SignInButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {Storage} from '../libs/storage';
import {Navigation, setUserRoleStatuses} from '../libs/utils';
import loadingIndicator from '../images/loading-indicator.svg';
import ReactTooltip from 'react-tooltip';
import eventList from '../libs/events';
import eventList, {MetricsEventName} from '../libs/events';
import {StackdriverReporter} from '../libs/stackdriverReporter';
import {History} from 'history';
import {OidcUser} from '../libs/auth/oidcBroker';
Expand Down Expand Up @@ -107,9 +107,9 @@ export const SignInButton = (props: SignInButtonProps) => {
history.push(`/tos_acceptance${shouldRedirect ? `?redirectTo=${redirectTo}` : ''}`);
};

const syncSignInOrRegistrationEvent = async (event: String) => {
const syncSignInOrRegistrationEvent = async (event: MetricsEventName) => {
Storage.setAnonymousId();
await Metrics.identify(Storage.getAnonymousId());
await Metrics.identify(`${Storage.getAnonymousId()}`);
rushtong marked this conversation as resolved.
Show resolved Hide resolved
await Metrics.syncProfile();
await Metrics.captureEvent(event);
};
Expand Down Expand Up @@ -195,10 +195,10 @@ export const SignInButton = (props: SignInButtonProps) => {
className='navbar-duos-icon-help'
style={{color: 'white', height: 16, width: 16, marginLeft: 5}}
href='https://support.terra.bio/hc/en-us/articles/28504837523995-How-to-Register-for-DUOS'
data-for="tip_google-help"
data-tip="Need account help? Click here!"
data-for='tip_google-help'
data-tip='Need account help? Click here!'
/>
<ReactTooltip id="tip_google-help" place="top" effect="solid" multiline={true} className="tooltip-wrapper"/>
<ReactTooltip id='tip_google-help' place='top' effect='solid' multiline={true} className='tooltip-wrapper'/>
</div>
);
};
Expand All @@ -209,10 +209,10 @@ export const SignInButton = (props: SignInButtonProps) => {
? <div>
{signInElement()}
</div>
: <div className="dialog-alert">
: <div className='dialog-alert'>
<Alert
id="dialog"
type="danger"
id='dialog'
type='danger'
title={(errorDisplay as ErrorInfo).title || 'Error'}
description={(errorDisplay as ErrorInfo).description || ''}
onClose={() => setErrorDisplay({})}
Expand Down
50 changes: 40 additions & 10 deletions src/libs/ajax/Metrics.js → src/libs/ajax/Metrics.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
import axios from 'axios';
import axios, {AxiosRequestConfig} from 'axios';
import {getDefaultProperties} from '@databiosphere/bard-client';

import {Storage} from '../storage';
import {getBardApiUrl} from '../ajax';
import {Token} from '../config';
import {MetricsEventName} from 'src/libs/events';

// Set default timeout for all metrics calls to 30 seconds
const defaultSignal: AbortSignal = AbortSignal.timeout(30000);

Comment on lines +9 to +10
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this was necessary for typescriptification to make all the axios calls compile. I'm open to alternative suggestions here but this seems like a safe timeout for bard & appcues calls.

export const Metrics = {
captureEvent: (event, details, signal) => captureEventFn(event, details, signal).catch(() => {
captureEvent: (
event: MetricsEventName,
details: Record<string, any> = {},
signal: AbortSignal = defaultSignal,
refreshAppcues: boolean = true
) => captureEventFn(event, details, signal, refreshAppcues).catch(() => {
}),
syncProfile: (signal) => syncProfile(signal),
identify: (anonId, signal) => identify(anonId, signal),
syncProfile: (signal: AbortSignal = defaultSignal) => syncProfile(signal),
identify: (anonId: String, signal: AbortSignal = defaultSignal) => identify(anonId, signal),
};

/**
Expand All @@ -18,12 +27,19 @@ export const Metrics = {
* @param {string} event - The event name.
* @param {Object} [details={}] - The event details.
* @param {AbortSignal} [signal] - The abort signal.
* @param refreshAppcues - The refresh Appcues flag.
* @returns {Promise} - A Promise that resolves when the event is captured.
*/
const captureEventFn = async (event, details = {}, signal) => {
const captureEventFn = async (event: MetricsEventName, details: object = {}, signal: AbortSignal, refreshAppcues: boolean): Promise<any> => {
const isSignedIn = Storage.userIsLogged();
rushtong marked this conversation as resolved.
Show resolved Hide resolved
const isRegistered = isSignedIn && Storage.getCurrentUser();

// Send event to Appcues and refresh Appcues state
window.Appcues?.track(event);
if (refreshAppcues) {
window.Appcues?.page();
}

if (!isRegistered && !Storage.getAnonymousId()) {
Storage.setAnonymousId();
}
Expand All @@ -40,7 +56,7 @@ const captureEventFn = async (event, details = {}, signal) => {
},
};

const config = {
const config: AxiosRequestConfig = {
method: 'POST',
url: `${await getBardApiUrl()}/api/event`,
data: body,
Expand All @@ -57,8 +73,8 @@ const captureEventFn = async (event, details = {}, signal) => {
* @param {AbortSignal} [signal] - The abort signal.
* @returns {Promise} - A Promise that resolves when the profile is synced.
*/
const syncProfile = async (signal) => {
const config = {
const syncProfile = async (signal: AbortSignal): Promise<any> => {
const config: AxiosRequestConfig = {
method: 'POST',
url: `${await getBardApiUrl()}/api/syncProfile`,
headers: {Authorization: `Bearer ${Token.getToken()}`},
Expand All @@ -76,10 +92,24 @@ const syncProfile = async (signal) => {
* @param {AbortSignal} [signal] - The abort signal.
* @returns {Promise} - A Promise that resolves when the user is identified.
*/
const identify = async (anonId, signal) => {
const identify = async (anonId: String, signal: AbortSignal): Promise<any> => {
const body = {anonId};

const config = {
if (window.Appcues) {
const user = Storage.getCurrentUser();
const createDate = user.createDate ? user.createDate : new Date().getTime();
const appcuesProps = {
dateJoined: createDate,
app: 'DUOS'
};
if (user.userStatusInfo?.userSubjectId) {
window.Appcues.identify(user.userStatusInfo.userSubjectId, appcuesProps);
} else {
window.Appcues.identify(`${user.userId}`, appcuesProps);
}
}

const config: AxiosRequestConfig = {
method: 'POST',
url: `${await getBardApiUrl()}/api/identify`,
data: body,
Expand Down
23 changes: 23 additions & 0 deletions src/libs/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import {OidcBroker, OidcUser} from './oidcBroker';
import {Storage} from './../storage';
import {UserManager} from 'oidc-client-ts';
import {MetricsEventName} from '../events';

export const Auth = {
signInError: () => {
Expand Down Expand Up @@ -42,3 +43,25 @@ export const Auth = {
await OidcBroker.signOut();
},
};

// extending Window interface to access Appcues
declare global {
interface Window {
Appcues?: {
/** Identifies the current user with an ID and an optional set of properties. */
identify: (userId: string, properties?: any) => void;
/** Notifies the SDK that the state of the application has changed. */
page: () => void;
/** Forces specific Appcues content to appear for the current user by passing in the ID. */
show: (contentId: string) => void;
/** Fire the callback function when the given event is triggered by the SDK */
on: ((eventName: Exclude<string, 'all'>, callbackFn: (event: any) => void | Promise<void>) => void) &
((eventName: 'all', callbackFn: (eventName: string, event: any) => void | Promise<void>) => void);
/** Clears all known information about the current user in this session */
reset: () => void;
/** Tracks a custom event (by name) taken by the current user. */
track: (eventName: MetricsEventName) => void;
};
forceSignIn: any;
}
}
14 changes: 0 additions & 14 deletions src/libs/events.js

This file was deleted.

26 changes: 26 additions & 0 deletions src/libs/events.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* NOTE: See the Mixpanel guide in the terra-ui GitHub Wiki for more details:
* https://github.com/DataBiosphere/terra-ui/wiki/Mixpanel
*/
const eventList = {
userRegister: 'user:register',
userSignIn: 'user:signin',

pageView: 'page:view',
dataLibrary: 'page:view:data-library',
dar: 'page:view:dar'
};

export default eventList;

// Helper type to create BaseMetricsEventName.
type MetricsEventsMap<EventName> = { [key: string]: EventName | MetricsEventsMap<EventName> };
// Union type of all event names configured in eventsList.
type BaseMetricsEventName = typeof eventList extends MetricsEventsMap<infer EventName> ? EventName : never;
// Each route has its own page view event, where the event name includes the name of the route.
type PageViewMetricsEventName = `${typeof eventList.pageView}:${string}`;

/**
* Union type of all metrics event names.
*/
export type MetricsEventName = BaseMetricsEventName | PageViewMetricsEventName;
3 changes: 2 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
}
},
"include": [
"src"
"src",
"types/index.d.ts"
],
"plugins": ["@typescript-eslint", "import"]
}
3 changes: 3 additions & 0 deletions types/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
declare module "@databiosphere/bard-client" {
export function getDefaultProperties(): any
}
Loading