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

Minimal warning if student has never signed in #23625

Merged
merged 6 commits into from
Jul 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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