Skip to content

Commit aa16b33

Browse files
committed
Allow newlines between multiple email addresses in new enroll dialog
Users want to be able to paste lists. We also want to let them type spaces, which the previous impl. prevented. fixes: CNVS-34878 test plan - go to a course's people page (/courses/#/users) - click on +People - try entering text that includes spacee > expected result, you can - copy and paste auser@example.com buser@example.com cuser@example duser@example.com > expected result: - you are told that "duser@example,com" looks like an invalid address - you can click "Next" and all 4 adddresses are in the list of users canvas did not find Change-Id: Ia4eb40b1b1171f7734286c7e84b01ccb47a9e745 Reviewed-on: https://gerrit.instructure.com/102073 Tested-by: Jenkins Reviewed-by: Felix Milea-Ciobanu <fmileaciobanu@instructure.com> QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com> Product-Review: McCall Smith <mcsmith@instructure.com>
1 parent 07c9e14 commit aa16b33

8 files changed

Lines changed: 46 additions & 21 deletions

File tree

app/jsx/add_people/actions.jsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ define([
3939
dispatch(actions.validateUsersStart());
4040
const state = getState();
4141
const courseId = state.courseParams.courseId;
42-
const users = state.inputParams.nameList;
42+
// split on a newline or comma, stripping surrounding whitespace in the process
43+
const users = state.inputParams.nameList.trim().split(/\s*[\n,]\s*/);
4344
const searchType = state.inputParams.searchType;
4445
api.validateUsers({ courseId, users, searchType })
4546
.then((res) => {

app/jsx/add_people/components/people_search.jsx

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,24 @@ define([
1111

1212
static defaultProps = {
1313
searchType: 'cc_path',
14-
nameList: []
14+
nameList: ''
1515
};
1616

1717
constructor (props) {
1818
super(props);
1919

2020
this.namelistta = null;
21+
this.emailValidator = /.+@.+\..+/;
2122
}
22-
shouldComponentUpdate (nextProps /* , nextState */) {
23+
24+
shouldComponentUpdate (nextProps, /* nextState */) {
2325
return nextProps.searchType !== this.props.searchType
24-
|| nextProps.nameList.join(',') !== this.props.nameList.join(',')
26+
|| nextProps.nameList !== this.props.nameList
2527
|| nextProps.role !== this.props.role
2628
|| nextProps.section !== this.props.section
2729
|| nextProps.limitPrivilege !== this.props.limitPrivilege;
2830
}
2931

30-
3132
// event handlers ------------------------------------
3233
// inst-ui form elements are currently inconsistent in what args they send
3334
// to their onChange handler. Some send the event, others just the new value.
@@ -37,12 +38,9 @@ define([
3738
this.props.onChange({searchType: newValue});
3839
}
3940
onChangeNameList = (newValue) => {
40-
let nameList = newValue.trim();
41-
// split the user enteredd name list on commas,
42-
// then trim each result
43-
nameList = nameList.length ? nameList.split(/\s*,\s*/).map(n => n.trim()) : [];
44-
this.props.onChange({nameList});
41+
this.props.onChange({nameList: newValue});
4542
}
43+
4644
onChangeSection = (event) => {
4745
this.props.onChange({section: event.target.value});
4846
}
@@ -53,10 +51,26 @@ define([
5351
this.props.onChange({limitPrivilege: event.target.checked});
5452
}
5553

54+
// validate the user's input of names in the textbox
55+
// @returns: a message for <TextArea> or null
56+
getHint () {
57+
let message = ' '; // that's a copy/pasted en-space to trick TextArea into
58+
// reserving space for the message so the UI doesn't jump
59+
if (this.props.nameList.length > 0 && this.props.searchType === 'cc_path') { // search by email
60+
const badEmail = this.props.nameList.split(/[\n,]/).find(n => !this.emailValidator.test(n))
61+
if (badEmail) {
62+
message = I18n.t('It looks like you have an invalid email address: "%{addr}"', {addr: badEmail});
63+
}
64+
}
65+
return [{text: message, type: 'hint'}];
66+
}
67+
5668
// rendering ------------------------------------
5769
render () {
5870
let exampleText = '';
5971
let labelText = '';
72+
const message = this.getHint()
73+
6074
switch (this.props.searchType) {
6175
case 'sis_user_id':
6276
exampleText = 'student_2708, student_3693';
@@ -109,7 +123,8 @@ define([
109123
<TextArea
110124
label={<ScreenReaderContent>{labelText}</ScreenReaderContent>}
111125
autoGrow={false} resize="vertical" height="9em"
112-
value={this.props.nameList.join(',')} textareaRef={(ta) => { this.namelistta = ta; }}
126+
value={this.props.nameList} textareaRef={(ta) => { this.namelistta = ta; }}
127+
messages={message}
113128
onChange={this.onChangeNameList}
114129
/>
115130
</fieldset>
@@ -156,7 +171,7 @@ define([
156171
<i className="icon-user" />
157172
<Typography size="medium">
158173
{I18n.t('Add user by Email Address, Login ID, or SIS ID.')}<br />
159-
{I18n.t('Use "," between for adding multiple users.')}
174+
{I18n.t('Use a "," or new line between for adding multiple users.')}
160175
</Typography>
161176
</div>
162177
</div>

app/jsx/add_people/components/shapes.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ define([
1515

1616
const inputParamsShape = {
1717
searchType: PropTypes.oneOf(['cc_path', 'unique_id', 'sis_user_id']),
18-
nameList: PropTypes.arrayOf(PropTypes.string),
18+
nameList: PropTypes.string,
1919
role: PropTypes.string,
2020
section: PropTypes.string
2121
};

app/jsx/add_people/store.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ define([
1616
},
1717
inputParams: {
1818
searchType: 'cc_path', // cc_path=email, unique_id=login_id, sis_user_id=sis_user_id
19-
nameList: [], // user entered list of names to add to this course
19+
nameList: '', // user entered list of names to add to this course
2020
role: '', // the role to assign each of the added users
2121
section: '', // the section to assign each of the added users
2222
limitPrivilege: false, // user can interact with users in their section only

app/stylesheets/bundles/roster.scss

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
fieldset.peoplesearch__selections {
6363
margin-right: auto;
6464
margin-left: auto;
65+
margin-top: .5rem;
6566
text-align: center;
6667
.peoplesearch__selection {
6768
text-align: left;

spec/javascripts/jsx/add_people/components/peopleSearchSpec.jsx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ define([
1313
role: '2',
1414
limitPrivilege: true,
1515
searchType: 'unique_id',
16-
nameList: ['foo', 'bar', 'baz'],
16+
nameList: 'foo, bar, baz',
1717
canReadSIS: true
1818
};
1919

@@ -28,7 +28,7 @@ define([
2828
const loginRadio = peopleSearch.querySelector('input[type="radio"][value="unique_id"]');
2929
equal(loginRadio.checked, true, 'login id radio button is checked');
3030
const nameInput = peopleSearch.querySelector('textarea');
31-
equal(nameInput.value, 'foo,bar,baz', 'names are in the textarea');
31+
equal(nameInput.value, searchProps.nameList, 'names are in the textarea');
3232
const selects = peopleSearch.querySelectorAll('.peoplesearch__selections select');
3333
equal(selects[0].value, '2', 'role 2 is selected');
3434
equal(selects[1].value, '1', 'section 1 is selected');
@@ -38,11 +38,19 @@ define([
3838
equal(limitPrivilegeCheckbox.checked, true, 'limit privileges checkbox is checked');
3939
});
4040
test('removes search by SIS ID', () => {
41-
const newProps = Object.assign({}, searchProps);
42-
newProps.canReadSIS = false;
41+
const newProps = Object.assign({}, searchProps, {canReadSIS: false});
4342
const component = TestUtils.renderIntoDocument(<PeopleSearch {...newProps} />);
4443
const peopleSearch = TestUtils.findRenderedDOMComponentWithClass(component, 'addpeople__peoplesearch');
4544
const sisRadio = peopleSearch.querySelector('input[type="radio"][value="sis_user_id"]');
4645
equal(sisRadio, null, 'sis id radio button is not displayed');
4746
});
47+
test('shows hint with bad email address', () => {
48+
const badEmail = 'foobar@';
49+
const newProps = Object.assign({}, searchProps, {searchType: 'cc_path', nameList: badEmail});
50+
const component = TestUtils.renderIntoDocument(<PeopleSearch {...newProps} />);
51+
const peopleSearch = TestUtils.findRenderedDOMComponentWithClass(component, 'addpeople__peoplesearch');
52+
const nameInput = peopleSearch.querySelector('textarea');
53+
equal(nameInput.value, badEmail, 'email is in the textarea');
54+
ok(peopleSearch.innerHTML.includes('It looks like you have an invalid email address: "foobar@"'));
55+
});
4856
});

spec/javascripts/jsx/add_people/initialState.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ define([], () => {
88
},
99
inputParams: {
1010
searchType: 'unique_id',
11-
nameList: ['foo', 'bar', 'baz'],
11+
nameList: 'foo, bar, baz',
1212
role: '1',
1313
section: '1',
1414
limitPrivilege: false,

spec/javascripts/jsx/add_people/reducerSpec.jsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ define([
178178
},
179179
inputParams: {
180180
searchType: 'cc_path',
181-
nameList: [],
181+
nameList: '',
182182
role: '',
183183
section: '',
184184
limitPrivilege: false,
@@ -231,7 +231,7 @@ define([
231231
test('set input paramaters', () => {
232232
const newSearchParams = {
233233
fieldType: 'unique_id',
234-
nameList: ['foo', 'bar', 'baz'],
234+
nameList: 'foo, bar, baz',
235235
role: '2',
236236
secion: '2',
237237
limitPrivilege: true

0 commit comments

Comments
 (0)