Skip to content

Commit

Permalink
[RW-4159][risk=no] Terms of Service UI and cleanup of create-account …
Browse files Browse the repository at this point in the history
…flow. (#3075)
  • Loading branch information
gjuggler authored Feb 7, 2020
1 parent 187e417 commit c00ffc5
Show file tree
Hide file tree
Showing 22 changed files with 873 additions and 390 deletions.
2 changes: 1 addition & 1 deletion ui/app.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ threadsafe: true
default_expiration: "2m"

handlers:
- url: /(.*\.(css|eot|gz|html|ico|jpg|jpeg|js|map|mp4|png|svg|ttf|woff|woff2))
- url: /(.*\.(css|eot|gz|html|ico|jpg|jpeg|js|map|mp4|pdf|png|svg|ttf|woff|woff2))
static_files: dist/\1
upload: dist/(.*)
secure: always
Expand Down
3 changes: 2 additions & 1 deletion ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@
"tslib": "^1.9.0",
"validate.js": "^0.12.0",
"yarn": "^1.21.1",
"zone.js": "0.8.26"
"zone.js": "0.8.26",
"react-pdf": "4.1.0"
},
"devDependencies": {
"@angular-devkit/build-angular": "~0.11.0",
Expand Down
9 changes: 6 additions & 3 deletions ui/src/app/components/flex.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ export const flexStyle = reactStyles({


export const FlexRow = (props) => {
return <div style={{...flexStyle.row, ...props.style}}>
const {style, ...other} = props;
return <div {...other} style={{...flexStyle.row, ...style}}>
{props.children}
</div>;
};

FlexRow.displayName = 'FlexRow';

export const FlexColumn = (props) => {
return <div style={{...flexStyle.column, ...props.style}}>
const {style, ...other} = props;
return <div {...other} style={{...flexStyle.column, ...style}}>
{props.children}
</div>;
};
FlexColumn.displayName = 'FlexColumn';

export const FlexRowWrap = (props) => {
return <FlexRow style={{flexWrap: 'wrap', ...props.style}}>
Expand Down
4 changes: 4 additions & 0 deletions ui/src/app/pages/app/component.css
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,7 @@
button.btn:focus {
outline: none;
}

.create-account__degree-select .p-multiselect-header {
display: none;
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ export const AccountCreationOptions = {
{label: 'B.A.', value: Degree.BA},
{label: 'B.S.', value: Degree.BS},
{label: 'B.S.N.', value: Degree.BSN},
// This label can have a space in it when the following issue is resolved:
// https://github.com/primefaces/primereact/issues/1137
{label: 'None(blank)', value: Degree.NONE},
],
roles: [
{label: `Undergraduate (Bachelor level) student`, value: AcademicRole.UNDERGRADUATE},
Expand Down
85 changes: 32 additions & 53 deletions ui/src/app/pages/login/account-creation/account-creation-survey.tsx
Original file line number Diff line number Diff line change
@@ -1,30 +1,22 @@
import {ListPageHeader} from 'app/components/headers';
import * as fp from 'lodash/fp';
import {Dropdown} from 'primereact/dropdown';
import * as React from 'react';
import * as validate from 'validate.js';

import {Button} from 'app/components/buttons';
import {FlexColumn, FlexRow} from 'app/components/flex';
import {FormSection} from 'app/components/forms';
import {ListPageHeader} from 'app/components/headers';
import {CheckBox, RadioButton} from 'app/components/inputs';
import colors from 'app/styles/colors';
import {
DataAccessLevel,
Profile,
Race,
SexAtBirth,
} from 'generated/fetch';
import {Section, TextInputWithLabel} from './account-creation';

import {TooltipTrigger} from 'app/components/popups';
import {signedOutImages} from 'app/pages/login/signed-out-images';
import {
profileApi
} from 'app/services/swagger-fetch-clients';
import {SpinnerOverlay} from 'app/components/spinners';
import {profileApi} from 'app/services/swagger-fetch-clients';
import colors from 'app/styles/colors';
import {toggleIncludes} from 'app/utils';
import * as validate from 'validate.js';
import {Profile} from 'generated/fetch';
import {Section, TextInputWithLabel} from './account-creation';
import {AccountCreationOptions} from './account-creation-options';


const styles = {
checkbox: {height: 17, width: 17, marginTop: '0.15rem'},
checkboxWrapper: {display: 'flex', alignItems: 'flex-start', width: '13rem', marginBottom: '0.5rem'},
Expand Down Expand Up @@ -54,8 +46,10 @@ export const DropDownSection = (props) => {

export interface AccountCreationSurveyProps {
invitationKey: string;
termsOfServiceVersion?: number;
profile: Profile;
setProfile: Function;
onComplete: (profile: Profile) => void;
onPreviousClick: (profile: Profile) => void;
}

export interface AccountCreationState {
Expand All @@ -68,45 +62,29 @@ export class AccountCreationSurvey extends React.Component<AccountCreationSurvey
super(props);
this.state = {
creatingAccount: false,
profile: {
username: '',
dataAccessLevel: DataAccessLevel.Protected,
demographicSurvey: {
disability: false,
education: undefined,
ethnicity: undefined,
identifiesAsLgbtq: false,
lgbtqIdentity: '',
race: [] as Race[],
sexAtBirth: [] as SexAtBirth[],
yearOfBirth: 0
}
}
};
}

componentDidMount() {
this.setState({profile: this.profileObj});
}

get profileObj() {
const demographicSurvey = this.state.profile.demographicSurvey;
return {
...this.props.profile,
demographicSurvey
profile: this.props.profile,
};
}

// TODO: we should probably bump this logic out of the survey component and either into its own
// component or into the top-level SignIn component. The fact that we're awkwardly passing the
// invitation key and tos version into this component (for the sole purpose of relaying this data
// to the backend) is a telltale sign that this should be refactored.
createAccount(): void {
const {invitationKey, setProfile} = this.props;
const {invitationKey, termsOfServiceVersion, onComplete} = this.props;
this.setState({creatingAccount: true});
profileApi().createAccount({profile: this.profileObj, invitationKey: invitationKey})
profileApi().createAccount({
profile: this.state.profile,
invitationKey: invitationKey,
termsOfServiceVersion: termsOfServiceVersion
})
.then((savedProfile) => {
this.setState({profile: savedProfile, creatingAccount: false});
setProfile(savedProfile, {stepName: 'accountCreationSuccess', backgroundImages: signedOutImages.login});
onComplete(savedProfile);
}).catch(error => {
console.log(error);
this.setState({creatingAccount: false});
// TODO: we need to show some user-facing error message when this fails.
});
}

Expand All @@ -129,7 +107,7 @@ export class AccountCreationSurvey extends React.Component<AccountCreationSurvey
}

render() {
const {profile: {demographicSurvey}} = this.state;
const {profile: {demographicSurvey}, creatingAccount} = this.state;
const validationCheck = {
lgbtqIdentity: {
length: {
Expand All @@ -139,7 +117,7 @@ export class AccountCreationSurvey extends React.Component<AccountCreationSurvey
},
};
const errors = validate(demographicSurvey, validationCheck);
console.log(errors);

return <div style={{marginTop: '1rem', paddingLeft: '3rem', width: '26rem'}}>
<label style={{color: colors.primary, fontSize: 16}}>
Please complete Step 2 of 2
Expand Down Expand Up @@ -236,9 +214,9 @@ or another sexual and/or gender minority?'>
value={demographicSurvey.education}
onChange={
(e) => this.updateDemographicAttribute('education', e)}/>
<div style={{display: 'flex', paddingTop: '2rem'}}>
<Button type='secondary' style={{marginRight: '1rem'}} disabled={this.state.creatingAccount}
onClick={() => this.props.setProfile(this.profileObj, {stepName: 'accountCreation'})}>
<FormSection style={{paddingBottom: '1rem'}}>
<Button type='secondary' style={{marginRight: '1rem'}} disabled={creatingAccount}
onClick={() => this.props.onPreviousClick(this.state.profile)}>
Previous
</Button>
<TooltipTrigger content={errors && <React.Fragment>
Expand All @@ -247,12 +225,13 @@ or another sexual and/or gender minority?'>
{Object.keys(errors).map((key) => <li key={errors[key][0]}>{errors[key][0]}</li>)}
</ul>
</React.Fragment>}>
<Button type='primary' disabled={this.state.creatingAccount || this.state.creatingAccount || errors}
<Button type='primary' disabled={creatingAccount || creatingAccount || errors}
onClick={() => this.createAccount()}>
Submit
</Button>
</TooltipTrigger>
</div>
</FormSection>
{creatingAccount && <SpinnerOverlay />}
</div>;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import {mount, ReactWrapper, shallow, ShallowWrapper} from 'enzyme';
import * as React from 'react';
import {Document, Page} from 'react-pdf';
import AccountCreationTos, {AccountCreationTosProps} from './account-creation-tos';

type AnyWrapper = (ShallowWrapper|ReactWrapper);
const getPrivacyCheckbox = (wrapper: AnyWrapper): AnyWrapper => {
return wrapper.find('CheckBox[data-test-id="privacy-statement-check"]');
};
const getTosCheckbox = (wrapper: AnyWrapper): AnyWrapper => {
return wrapper.find('CheckBox[data-test-id="terms-of-service-check"]');
};
const getNextButton = (wrapper: AnyWrapper ): AnyWrapper => {
return wrapper.find('[data-test-id="next-button"]');
};

let props: AccountCreationTosProps;
const onCompleteSpy = jest.fn();

beforeEach(() => {
jest.clearAllMocks();
props = {
windowSize: {width: 1700, height: 0},
onComplete: onCompleteSpy,
pdfPath: '/assets/documents/fake-document-path.pdf'
};
});

it('should render', async() => {
const wrapper = mount(<AccountCreationTos {...props} />);
expect(wrapper.exists()).toBeTruthy();
});

// TODO: this test is really about the PDF rendering, which should be bumped out when we encapsulate
// PDF rendering into its own component.
it('should load PDF pages', async() => {
const wrapper = shallow(<AccountCreationTos {...props} />).shallow();

// Initially we should have a document and no pages.
expect(wrapper.find(Document).length).toEqual(1);
expect(wrapper.find(Page).length).toEqual(0);

// Simulate the PDF document loading and calling the 'onLoadSuccess' prop to indicate we have
// 10 pages in the PDF.
const pdfDocument = wrapper.find(Document);
const onSuccess = pdfDocument.prop('onLoadSuccess') as (data: object) => {};
onSuccess({numPages: 10});

// We should now be rendering a <Page> component for each of the pages.
expect(wrapper.find(Page).length).toEqual(10);
});

it('should enable checkboxes and next button with user input', async() => {
// Note: since AccountCreationTos is exported as withWindowSize(...), we need to shallow-render
// an extra time to get to the real AccountCreationTos component.
const wrapper = shallow(<AccountCreationTos {...props} />).shallow();

expect(getPrivacyCheckbox(wrapper).prop('disabled')).toBeTruthy();
expect(getTosCheckbox(wrapper).prop('disabled')).toBeTruthy();
expect(getNextButton(wrapper).prop('disabled')).toBeTruthy();

// In the real world, the user can either click "Scroll to bottom" or scroll until the last page
// element is visible, at which point this state variable will be set to true. This behavior
// was extremely difficult to simulate in Jest, and will be better-suited to an end-to-end test.
//
// As a workaround, we manually set the state here to allow us to test the input enablement.
wrapper.setState({hasReadEntireTos: true});

expect(getPrivacyCheckbox(wrapper).prop('disabled')).toBeFalsy();
expect(getTosCheckbox(wrapper).prop('disabled')).toBeFalsy();
expect(getNextButton(wrapper).prop('disabled')).toBeTruthy();

// Now, simulate checking both boxes, which should enable the "next" button.
getPrivacyCheckbox(wrapper).simulate('change', {target: {checked: true}});
getTosCheckbox(wrapper).simulate('change', {target: {checked: true}});

expect(getNextButton(wrapper).prop('disabled')).toBeFalsy();
});

it('should call onComplete when next button is pressed', async() => {
const wrapper = shallow(<AccountCreationTos {...props} />).shallow();

wrapper.setState({hasReadEntireTos: true});
getPrivacyCheckbox(wrapper).simulate('change', {target: {checked: true}});
getTosCheckbox(wrapper).simulate('change', {target: {checked: true}});

getNextButton(wrapper).simulate('click');

expect(onCompleteSpy).toHaveBeenCalled();
});
Loading

0 comments on commit c00ffc5

Please sign in to comment.