Skip to content

Commit

Permalink
Merge pull request #23625 from code-dot-org/remove-student-warning-types
Browse files Browse the repository at this point in the history
Minimal warning if student has never signed in
  • Loading branch information
islemaster committed Jul 11, 2018
2 parents 5843e9e + caedac2 commit 2dcf80e
Show file tree
Hide file tree
Showing 10 changed files with 227 additions and 40 deletions.
10 changes: 9 additions & 1 deletion apps/src/lib/ui/SystemDialog/Header.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,19 @@ import color from '@cdo/apps/util/color';
export default class Header extends React.Component {
static propTypes = {
text: PropTypes.string.isRequired,
hideBorder: PropTypes.bool,
};

render() {
const computedStyle = {
...style,
...(this.props.hideBorder && {
borderBottomWidth: 0,
paddingBottom: 5,
}),
};
return (
<h1 style={style}>
<h1 style={computedStyle}>
{this.props.text}
</h1>
);
Expand Down
28 changes: 16 additions & 12 deletions apps/src/templates/BaseDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export default class BaseDialog extends React.Component {
uncloseable: PropTypes.bool,
hideCloseButton: PropTypes.bool,
handleKeyDown: PropTypes.func,
// For use in react-storybook; allows rendering dialog inline in story tables.
hideBackdrop: PropTypes.bool,
fullWidth: PropTypes.bool,
fullHeight: PropTypes.bool,
Expand Down Expand Up @@ -62,21 +63,16 @@ export default class BaseDialog extends React.Component {

render() {
if (!this.props.isOpen && !this.props.hideBackdrop) {
return <div></div>;
return <div/>;
}

let bodyStyle, modalBodyStyle, xCloseStyle;
if (this.props.hideBackdrop) {
bodyStyle = {
position: 'initial',
marginLeft: 0,
};
}
if (this.props.fullWidth) {
bodyStyle = Object.assign({}, bodyStyle, {
bodyStyle = {
...bodyStyle,
width: '90%',
marginLeft: '-45%'
});
};
}
if (this.props.fullHeight) {
bodyStyle = {
Expand All @@ -100,10 +96,11 @@ export default class BaseDialog extends React.Component {
overflowY: (this.props.fixedHeight || this.props.fullHeight) ? 'hidden' : 'auto',
borderRadius: 4,
};
bodyStyle = Object.assign({}, bodyStyle, {
bodyStyle = {
...bodyStyle,
width: this.props.fixedWidth || 700,
marginLeft: (-this.props.fixedWidth / 2) || -350,
});
};
xCloseStyle = {
position: 'absolute',
top: 0,
Expand All @@ -117,7 +114,14 @@ export default class BaseDialog extends React.Component {
modalClassNames = "";
modalBodyClassNames = "";
}
bodyStyle = { ...bodyStyle, ...this.props.style };
bodyStyle = {
...bodyStyle,
...(this.props.hideBackdrop && {
position: 'initial',
marginLeft: 0,
}),
...this.props.style
};
let body = (
<div
style={bodyStyle}
Expand Down
76 changes: 51 additions & 25 deletions apps/src/templates/manageStudents/ConfirmRemoveStudentDialog.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'lodash';
import React, {PropTypes} from 'react';
import i18n from '@cdo/locale';
import {Header, ConfirmCancelFooter} from '../../lib/ui/SystemDialog/SystemDialog';
Expand All @@ -6,51 +7,76 @@ import Button from "../Button";
import color from "../../util/color";
import {ADD_A_PERSONAL_LOGIN_HELP_URL} from '../../lib/util/urlHelpers';

// A stub set of otherwise-required props for use in stories and unit tests.
export const MINIMUM_TEST_PROPS = {
isOpen: true,
onConfirm: () => {},
onCancel: () => {},
};

// This set of props will be 'inherited' from BaseDialog and automatically
// passed through to it.
const propsFromBaseDialog = ['isOpen', 'hideBackdrop'];

export default class ConfirmRemoveStudentDialog extends React.Component {
static propTypes = {
isOpen: BaseDialog.propTypes.isOpen,
..._.pick(BaseDialog.propTypes, propsFromBaseDialog),
disabled: PropTypes.bool,
hasEverSignedIn: PropTypes.bool,
onConfirm: PropTypes.func.isRequired,
onCancel: PropTypes.func.isRequired,
};

headerText() {
return this.props.hasEverSignedIn ?
i18n.removeStudentHeaderNew() :
i18n.removeStudentHeader();
}

render() {
const {isOpen, disabled, onConfirm, onCancel} = this.props;
const {disabled, hasEverSignedIn, onConfirm, onCancel} = this.props;
return (
<BaseDialog
{...(_.pick(this.props, propsFromBaseDialog))}
useUpdatedStyles
isOpen={isOpen}
handleClose={onCancel}
>
<div style={styles.container}>
<Header text={i18n.removeStudentHeaderNew()}/>
<p>
<strong>{i18n.removeStudentBody1()}</strong>
{' '}
{i18n.removeStudentBody2()}
</p>
<p>
{i18n.removeStudentBody3()}
</p>
<Button
text={i18n.removeStudentSendHomeInstructions()}
target="_blank"
href={ADD_A_PERSONAL_LOGIN_HELP_URL}
color={Button.ButtonColor.blue}
size={Button.ButtonSize.large}
style={styles.sendHomeInstructionsButton}
tabIndex="1"
<Header
text={this.headerText()}
hideBorder={!hasEverSignedIn}
/>
<p>
{i18n.removeStudentBody4()}
</p>
{hasEverSignedIn &&
<div>
<p>
<strong>{i18n.removeStudentBody1()}</strong>
{' '}
{i18n.removeStudentBody2()}
</p>
<p>
{i18n.removeStudentBody3()}
</p>
<Button
text={i18n.removeStudentSendHomeInstructions()}
target="_blank"
href={ADD_A_PERSONAL_LOGIN_HELP_URL}
color={Button.ButtonColor.blue}
size={Button.ButtonSize.large}
style={styles.sendHomeInstructionsButton}
tabIndex="1"
/>
<p>
{i18n.removeStudentBody4()}
</p>
</div>
}
<ConfirmCancelFooter
confirmText={i18n.removeStudent()}
confirmColor={Button.ButtonColor.red}
onConfirm={onConfirm}
onCancel={onCancel}
disableConfirm={disabled}
disableCancel={disabled}
disableConfirm={!!disabled}
disableCancel={!!disabled}
tabIndex="1"
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React from 'react';
import ConfirmRemoveStudentDialog, {MINIMUM_TEST_PROPS} from './ConfirmRemoveStudentDialog';

const STORY_PROPS = {
...MINIMUM_TEST_PROPS,
hideBackdrop: true,
};

export default storybook => storybook
.storiesOf('ConfirmRemoveStudentDialog', module)
.addStoryTable([
{
name:'For a student who has never signed in',
description: `
Removing a student who has never signed in incurs no risk
of losing access to data for the student or the school, so the
warning dialog can be minimal.
`,
story: () => (
<ConfirmRemoveStudentDialog
{...STORY_PROPS}
hasEverSignedIn={false}
/>
)
}, {
name:'For a student who has signed in',
description: `
Removing a student who has signed in may give the student the ability
to delete their account, which would result in destruction of school
records. Therefore, we show a scary warning.
`,
story: () => (
<ConfirmRemoveStudentDialog
{...STORY_PROPS}
hasEverSignedIn={true}
/>
)
}
]);
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ const styles = {

class ManageStudentActionsCell extends Component {
static propTypes = {
id: PropTypes.number.isRequired,
id: PropTypes.number.isRequired, // the student's user id
sectionId: PropTypes.number,
isEditing: PropTypes.bool,
isSaving: PropTypes.bool,
disableSaving: PropTypes.bool,
rowType: PropTypes.oneOf(Object.values(RowType)),
loginType: PropTypes.string,
hasEverSignedIn: PropTypes.string,
// Provided by redux
startEditingStudent: PropTypes.func,
cancelEditingStudent: PropTypes.func,
Expand Down Expand Up @@ -150,6 +151,7 @@ class ManageStudentActionsCell extends Component {
<ConfirmRemoveStudentDialog
isOpen={this.state.deleting}
disabled={this.state.requestInProgress}
hasEverSignedIn={this.props.hasEverSignedIn}
onConfirm={this.onConfirmDelete}
onCancel={this.onCancelDelete}
/>
Expand Down
2 changes: 2 additions & 0 deletions apps/src/templates/manageStudents/ManageStudentsTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export const studentSectionDataPropType = PropTypes.shape({
secretPicturePath: PropTypes.string,
sectionId: PropTypes.number,
loginType: PropTypes.string,
hasEverSignedIn: PropTypes.bool,
rowType: PropTypes.oneOf(Object.values(RowType)),
});

Expand Down Expand Up @@ -252,6 +253,7 @@ class ManageStudentsTable extends Component {
disableSaving={disableSaving}
rowType={rowData.rowType}
loginType={rowData.loginType}
hasEverSignedIn={rowData.hasEverSignedIn}
/>
);
};
Expand Down
1 change: 1 addition & 0 deletions apps/src/templates/manageStudents/manageStudentsRedux.js
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ export const convertStudentServerData = (studentData, loginType, sectionId) => {
loginType: loginType,
sectionId: sectionId,
sharingDisabled: student.sharing_disabled,
hasEverSignedIn: student.has_ever_signed_in,
isEditing: false,
isSaving: false,
rowType: RowType.STUDENT,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import React from 'react';
import {mount} from 'enzyme';
import i18n from '@cdo/locale';
import {expect} from '../../../util/configuredChai';
import ConfirmRemoveStudentDialog, {MINIMUM_TEST_PROPS} from '@cdo/apps/templates/manageStudents/ConfirmRemoveStudentDialog';
import Button from '@cdo/apps/templates/Button';
import {Header, ConfirmCancelFooter} from '@cdo/apps/lib/ui/SystemDialog/SystemDialog';
import {ADD_A_PERSONAL_LOGIN_HELP_URL} from '@cdo/apps/lib/util/urlHelpers';

describe('ConfirmRemoveStudentDialog', () => {
it('renders nothing if not open', () => {
const wrapper = mount(
<ConfirmRemoveStudentDialog
{...MINIMUM_TEST_PROPS}
isOpen={false}
/>
);
expect(wrapper).to.be.empty;
});

it('renders minimal content if student has never signed in', () => {
const wrapper = mount(
<ConfirmRemoveStudentDialog
{...MINIMUM_TEST_PROPS}
hasEverSignedIn={false}
/>
);
expect(wrapper).to.containMatchingElement(
<div>
<Header text={i18n.removeStudentHeader()}/>
<ConfirmCancelFooter
confirmText={i18n.removeStudent()}
confirmColor={Button.ButtonColor.red}
onConfirm={MINIMUM_TEST_PROPS.onConfirm}
onCancel={MINIMUM_TEST_PROPS.onCancel}
disableConfirm={false}
disableCancel={false}
tabIndex="1"
/>
</div>
);
});

it('renders full text if student has ever signed in', () => {
const wrapper = mount(
<ConfirmRemoveStudentDialog
{...MINIMUM_TEST_PROPS}
hasEverSignedIn={true}
/>
);
expect(wrapper).to.containMatchingElement(
<div>
<Header text={i18n.removeStudentHeaderNew()}/>
<div>
<p>
<strong>{i18n.removeStudentBody1()}</strong>
{' '}
{i18n.removeStudentBody2()}
</p>
<p>
{i18n.removeStudentBody3()}
</p>
<Button
text={i18n.removeStudentSendHomeInstructions()}
target="_blank"
href={ADD_A_PERSONAL_LOGIN_HELP_URL}
color={Button.ButtonColor.blue}
size={Button.ButtonSize.large}
tabIndex="1"
/>
<p>
{i18n.removeStudentBody4()}
</p>
</div>
<ConfirmCancelFooter
confirmText={i18n.removeStudent()}
confirmColor={Button.ButtonColor.red}
onConfirm={MINIMUM_TEST_PROPS.onConfirm}
onCancel={MINIMUM_TEST_PROPS.onCancel}
disableConfirm={false}
disableCancel={false}
tabIndex="1"
/>
</div>
);
});
});
5 changes: 5 additions & 0 deletions dashboard/app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1695,9 +1695,14 @@ def summarize
location: "/v2/users/#{id}",
age: age,
sharing_disabled: sharing_disabled?,
has_ever_signed_in: has_ever_signed_in?,
}
end

def has_ever_signed_in?
current_sign_in_at.present?
end

def self.progress_queue
AsyncProgressHandler.progress_queue
end
Expand Down

0 comments on commit 2dcf80e

Please sign in to comment.