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-996] Add auth domain selection to create full view snapshot #1748

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
3 changes: 3 additions & 0 deletions cypress/integration/queryBuilder.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ testPlatforms.forEach((testPlatform) => {
cy.intercept('GET', 'api/repository/v1/datasets/**').as('getDataset');
cy.intercept('GET', 'api/repository/v1/datasets/**/policies').as('getDatasetPolicies');
cy.intercept('GET', 'api/resources/v1/profiles/**').as('getBillingProfileById');
cy.intercept('GET', 'https://sam.dsde-dev.broadinstitute.org/api/groups/v1').as(
'getUserGroups',
);

cy.visit('/login/e2e');
cy.get('#tokenInput').type(Cypress.env('GOOGLE_TOKEN'), {
Expand Down
8 changes: 6 additions & 2 deletions src/actions/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,20 +352,24 @@ export const { openSnapshotDialog } = createActions({
});

export const { snapshotCreateDetails } = createActions({
[ActionTypes.SNAPSHOT_CREATE_DETAILS]: (
[ActionTypes.SNAPSHOT_CREATE_DETAILS]: ({
name,
description,
mode,
dataset,
assetName,
filterData,
) => ({
authDomain,
billingProfileId,
}) => ({
name,
description,
mode,
assetName,
filterData,
dataset,
authDomain,
billingProfileId,
}),
});

Expand Down
6 changes: 6 additions & 0 deletions src/actions/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,9 @@ export const getFeatures = createAction<void>(ActionTypes.GET_FEATURES);
export const getFeaturesSuccess = createAction<ManagedGroupMembershipEntry[]>(
ActionTypes.GET_FEATURES_SUCCESS,
);

export const getUserGroups = createAction<void>(ActionTypes.GET_USER_GROUPS);
export const getUserGroupsSuccess = createAction<ManagedGroupMembershipEntry[]>(
ActionTypes.GET_USER_GROUPS_SUCCESS,
);
export const getUserGroupsFailure = createAction<void>(ActionTypes.GET_USER_GROUPS_FAILURE);
28 changes: 25 additions & 3 deletions src/components/common/FullViewSnapshotButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { BillingProfileModel, DatasetModel } from 'generated/tdr';
import _ from 'lodash';
import { initialUserState } from 'reducers/user';
import { initialQueryState } from 'reducers/query';
import { ManagedGroupMembershipEntry } from 'models/group';
import history from '../../modules/hist';
import globalTheme from '../../modules/theme';
import FullViewSnapshotButton from './FullViewSnapshotButton';
Expand All @@ -27,13 +28,20 @@ const initialState = {
const mountFullViewSnapshotButton = (
dataset: DatasetModel,
billingProfiles: Array<BillingProfileModel>,
groups: Array<ManagedGroupMembershipEntry>,
) => {
const mockStore = createMockStore([]);
const store = mockStore({ ...initialState, profiles: { profiles: billingProfiles } });
const store = mockStore({
...initialState,
profiles: { profiles: billingProfiles },
user: { groups },
});

// Intercept the getBillingProfiles API call onMount
cy.intercept('GET', '/api/resources/v1/profiles?offset=0&limit=1000').as('getBillingProfiles');

cy.intercept('GET', 'https://sam.dsde-dev.broadinstitute.org/api/groups/v1').as('getUserGroups');

mount(
<Router history={history}>
<Provider store={store}>
Expand All @@ -53,7 +61,11 @@ describe('FullViewSnapshotButton', () => {
{ id: 'profile1', profileName: 'profile1' },
{ id: 'profile2', profileName: 'profile2' },
];
mountFullViewSnapshotButton(dataset, profiles);
const groups = [
{ groupEmail: 'group1', groupName: 'group1', role: 'READER' },
{ groupEmail: 'group2', groupName: 'group2', role: 'READER' },
];
mountFullViewSnapshotButton(dataset, profiles, groups);
});

it('Displays the button with correct text', () => {
Expand All @@ -80,6 +92,13 @@ describe('FullViewSnapshotButton', () => {
cy.get('#billing-profile-select').should('have.value', 'profile2');
});

it('allows selecting an auth domain', () => {
cy.get('button').click();
cy.get('#authorization-domain-select').parent().click();
cy.get('[data-cy=menuItem-group2]').click();
cy.get('#authorization-domain-select').should('have.value', 'group2');
});

it('allows changing the name and description', () => {
cy.get('button').click();
cy.get('#snapshot-name').clear().type('New Name');
Expand All @@ -93,7 +112,10 @@ describe('FullViewSnapshotButton', () => {
it('Button is disabled and has tooltip with the no access message', () => {
const dataset = { defaultProfileId: 'profile2' };
const profiles: BillingProfileModel[] = [];
mountFullViewSnapshotButton(dataset, profiles);
const groups: ManagedGroupMembershipEntry[] = [
{ groupEmail: 'group1', groupName: 'group1', role: 'READER' },
];
mountFullViewSnapshotButton(dataset, profiles, groups);
cy.get('button').should('be.disabled');
cy.get('button').trigger('mouseover', { force: true });
cy.contains('You do not have access to any billing profiles to create a snapshot').should(
Expand Down
73 changes: 63 additions & 10 deletions src/components/common/FullViewSnapshotButton.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { createSnapshot, getBillingProfiles, snapshotCreateDetails } from 'actions/index';
import {
createSnapshot,
getBillingProfiles,
getUserGroups,
snapshotCreateDetails,
} from 'actions/index';
import { TdrState } from 'reducers';
import {
Button,
Expand All @@ -7,7 +12,9 @@ import {
DialogTitle,
Typography,
FormLabel,
Link,
TextField,
CustomTheme,
} from '@mui/material';
import React, { Dispatch } from 'react';
import {
Expand All @@ -18,23 +25,36 @@ import {
import { Action } from 'redux';
import { connect } from 'react-redux';
import { isEmpty, now, uniq } from 'lodash';
import { createStyles, withStyles, WithStyles } from '@mui/styles';
import { ManagedGroupMembershipEntry } from 'models/group';
import TerraTooltip from './TerraTooltip';
import JadeDropdown from '../dataset/data/JadeDropdown';
import { useOnMount } from '../../libs/utils';

interface FullViewSnapshotButtonProps {
const styles = (theme: CustomTheme) =>
createStyles({
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 would love to do this in the new style mechanism, so that is tbd

jadeLink: {
...theme.mixins.jadeLink,
},
});

interface FullViewSnapshotButtonProps extends WithStyles<typeof styles> {
dispatch: Dispatch<Action>;
dataset: DatasetModel;
billingProfiles: Array<BillingProfileModel>;
groups: Array<ManagedGroupMembershipEntry>;
}

function FullViewSnapshotButton({
classes,
dataset,
dispatch,
billingProfiles,
groups,
}: Readonly<FullViewSnapshotButtonProps>) {
useOnMount(() => {
dispatch(getBillingProfiles());
dispatch(getUserGroups());
});

const defaultBillingProfile = billingProfiles.find(
Expand All @@ -43,6 +63,7 @@ function FullViewSnapshotButton({

const [modalOpen, setModalOpen] = React.useState(false);
const [selectedBillingProfile, setSelectedBillingProfile] = React.useState(defaultBillingProfile);
const [selectedAuthDomain, setSelectedAuthDomain] = React.useState<string | undefined>(undefined);
const [snapshotName, setSnapshotName] = React.useState(
`Full_View_Snapshot_of_${dataset.name}_${now()}`,
);
Expand All @@ -59,14 +80,16 @@ function FullViewSnapshotButton({

const handleCreateFullViewSnapshot = () => {
dispatch(
snapshotCreateDetails(
snapshotName,
snapshotDescription,
SnapshotRequestContentsModelModeEnum.ByFullView,
snapshotCreateDetails({
name: snapshotName,
description: snapshotDescription,
mode: SnapshotRequestContentsModelModeEnum.ByFullView,
dataset,
),
authDomain: selectedAuthDomain,
billingProfileId: selectedBillingProfile?.id,
}),
);
dispatch(createSnapshot(selectedBillingProfile?.id));
dispatch(createSnapshot());
};

const onDismiss = () => setModalOpen(false);
Expand Down Expand Up @@ -155,7 +178,36 @@ function FullViewSnapshotButton({
}
value={selectedBillingProfile?.profileName || ''}
/>
<Typography>If this is the correct billing project - just click select</Typography>
<div style={{ marginTop: 8 }}>
<FormLabel
sx={{ fontWeight: 600, color: 'black' }}
htmlFor="authorization-domain-select"
>
Authorization Domain
<span style={{ fontWeight: 400, color: 'black' }}> - (optional)</span>
</FormLabel>
Copy link
Member

Choose a reason for hiding this comment

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

Is the use of (optional) consistent with other optional vs required inputs in TDR UI? I didn't see it used elsewhere in the code.

</div>
<JadeDropdown
sx={{ height: '2.5rem' }}
disabled={groups ? groups.length <= 1 : true}
options={groups ? groups.map((group) => group.groupName) : []}
name="authorization-domain"
Copy link
Member

Choose a reason for hiding this comment

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

Minor style question, does TDR UI prefer ?: over || and &&? For example

            disabled={!(groups?.length > 1)}
            options={groups?.map((group) => group.groupName) || []}

onSelectedItem={(event) => setSelectedAuthDomain(event.target.value)}
value={selectedAuthDomain || ''}
/>
<div>
Authorization Domains restrict data access to only specified individuals in a group and
are intended to fulfill requirements you may have for data governed by a compliance
Copy link
Member

Choose a reason for hiding this comment

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

The swagger API has a warning in bold: *** WARNING - Once this is set it cannot be modified ***

Should this have a warning too?

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 was mainly following the UX set out in the ticket, maybe we can bring this up in standup?

standard, such as federal controlled-access data or HIPAA protected data. They follow
all snapshot copies and cannot be removed. For more details, see{' '}
<Link
href="https://support.terra.bio/hc/en-us/articles/360026775691-Overview-Managing-access-to-controlled-data-with-Authorization-Domains#h_01J94P49XS1NE5KE4B3NA3A413"
target="_blank"
>
<span className={classes.jadeLink}>When to use an Authorization Domain</span>
</Link>
.
</div>
<DialogActions sx={{ display: 'flex', justifyContent: 'flex-end' }}>
<Button onClick={onDismiss} variant="outlined">
Cancel
Expand Down Expand Up @@ -183,7 +235,8 @@ function mapStateToProps(state: TdrState) {
return {
billingProfiles: state.profiles.profiles,
snapshot: state.snapshots.snapshot,
groups: state.user.groups,
};
}

export default connect(mapStateToProps)(FullViewSnapshotButton);
export default connect(mapStateToProps)(withStyles(styles)(FullViewSnapshotButton));
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@ export class CreateSnapshotPanel extends React.PureComponent {
const { dispatch, switchPanels, filterData, dataset } = this.props;
const { name, description, assetName } = this.state;
dispatch(
snapshotCreateDetails(
snapshotCreateDetails({
name,
description,
SnapshotRequestContentsModelModeEnum.ByQuery,
mode: SnapshotRequestContentsModelModeEnum.ByQuery,
dataset,
assetName,
filterData,
),
}),
);
switchPanels(ShareSnapshot);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ const initialState = {
profiles: [],
},
snapshots: {},
user: {
groups: [],
},
query: {
columns: [],
filterData: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ const initialState = {
snapshots: {
snapshots: [],
},
user: {
groups: [],
},
query: {
orderDirection: 'asc',
},
Expand Down
3 changes: 3 additions & 0 deletions src/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ export enum ActionTypes {
CHANGE_POLICY_USERS_TO_SNAPSHOT_REQUEST = 'CHANGE_POLICY_USERS_TO_SNAPSHOT_REQUEST',
GET_FEATURES = 'GET_FEATURES',
GET_FEATURES_SUCCESS = 'GET_FEATURES_SUCCESS',
GET_USER_GROUPS = 'GET_USER_GROUPS',
GET_USER_GROUPS_SUCCESS = 'GET_USER_GROUPS_SUCCESS',
GET_USER_GROUPS_FAILURE = 'GET_USER_GROUPS_FAILURE',
GET_SERVER_STATUS_SUCCESS = 'GET_SERVER_STATUS_SUCCESS',
GET_SERVER_STATUS_FAILURE = 'GET_SERVER_STATUS_FAILURE',
GET_SERVER_STATUS_DOWN = 'GET_SERVER_STATUS_DOWN',
Expand Down
12 changes: 11 additions & 1 deletion src/reducers/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,22 @@ export default {
});
},
[ActionTypes.SNAPSHOT_CREATE_DETAILS]: (state, action: any) => {
const { name, description, mode, assetName, filterData, dataset } = action.payload;
const {
name,
description,
mode,
assetName,
filterData,
dataset,
authDomain,
billingProfileId,
} = action.payload;
const snapshotRequest = {
...state.snapshotRequest,
name,
description,
mode,
dataAccessControlGroups: [authDomain],
};
if (mode === SnapshotRequestContentsModelModeEnum.ByQuery) {
snapshotRequest.assetName = assetName;
Expand Down
9 changes: 9 additions & 0 deletions src/reducers/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface UserState {
delegateToken: string;
tokenExpiration?: number;
features: { [key: string]: boolean };
groups: ManagedGroupMembershipEntry[];
isTimeoutEnabled: boolean;
id: string;
isTest: boolean;
Expand All @@ -37,6 +38,7 @@ export const initialUserState: UserState = {
delegateToken: '',
tokenExpiration: 0,
features: {},
groups: [],
isTimeoutEnabled: false,
id: '',
isTest: false,
Expand Down Expand Up @@ -139,6 +141,13 @@ export default {
isTimeoutEnabled: { $set: isTimeoutEnabled },
});
},
[ActionTypes.GET_USER_GROUPS_SUCCESS]: (
state,
action: { groups: ManagedGroupMembershipEntry[] },
) =>
immutable(state, {
groups: { $set: action.groups },
}),
},
initialUserState,
),
Expand Down
Loading
Loading