Skip to content

Commit b145085

Browse files
eschiebeleschiebel
committed
Forward user's real name to missing person list in new enroll
The user can enter "Real Name" <email@exampple.com> (and variations) into the search box. This captures the real name of users that don't exist and pre-populates the name text box if they choose to create this user on the fly. This way they can copy/paste large lists from a spreadsheet and not have to enter anything. Well it turns out that some schools (I'm looking at you Boston College) want to paste in a list of emails only and let us create and invite those people w/o a real name, which they can add later. It also turns out that we need a Select All checkbox so the user doesn't have to check each and every row. I'm sorry that this commit got large, but it kind of grew organically fixes CNVS-35149 fixes CNVS-35137 test plan: 1. users names are automatically forwarded - go to course people page (/courses/#/users) - click on +People - enter (or copy/paste) into the textbox "A User" <auser@example.com> "B User" buser@example.com cuser@example.com C User "User, D" <duser@example.com> euser@example.com - click Next > expected: the modal shows all 5 users as missing - check the "select all" checkbox in the header row of the table > expected: A, B C and D User's names are pre-populated in the name text box. euser's text box prompts for a "New user's name" the next button is enabled 2. the checkboxes work as expected - the select all (and all row checkboxes) are currently checked - uncheck one of the checkboxes next to a user > Expected: the select all checkbox unchecks. everything else remains the same - check the checkbox > Expected: the select all checkbox checks. - uncheck the select all checkbox > expecte: all the row checkboxes uncheck. 3. users are created and invited correctly - check the auser@example.com and euser@exaple.com checkboxes - click next > expected: A User is ready to enroll and has her name and email euser is ready to enroll and her name and email are euser@example.com Change-Id: I5dd944488bff89279d3d8e405b900c4aa307d919 Reviewed-on: https://gerrit.instructure.com/103075 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 c1d33aa commit b145085

9 files changed

Lines changed: 238 additions & 27 deletions

File tree

‎app/jsx/add_people/actions.jsx‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
define([
22
'redux-actions',
33
'./api_client',
4-
'./resolveValidationIssues'
5-
], ({ createActions }, api, resolveValidationIssues) => {
4+
'./resolveValidationIssues',
5+
'./helpers'
6+
], ({ createActions }, api, resolveValidationIssues, {parseNameList, findEmailInEntry}) => {
67
const actionDefs = [
78
'SET_INPUT_PARAMS',
89

@@ -28,6 +29,7 @@ define([
2829
'RESET' // reset([array of state subsections to reset]) undefined or empty = reset everything
2930
];
3031

32+
3133
const actionTypes = actionDefs.reduce((types, action) => {
3234
types[action] = action; // eslint-disable-line no-param-reassign
3335
return types;
@@ -39,8 +41,21 @@ define([
3941
dispatch(actions.validateUsersStart());
4042
const state = getState();
4143
const courseId = state.courseParams.courseId;
42-
// split on a newline or comma, stripping surrounding whitespace in the process
43-
const users = state.inputParams.nameList.trim().split(/\s*[\n,]\s*/);
44+
let users = parseNameList(state.inputParams.nameList);
45+
if (state.inputParams.searchType === 'cc_path') {
46+
// normalize the input to be "User Name <email address>"
47+
// 1. include the email address w/in < ... >
48+
// 2. if the user includes a name and email, be sure the name is first
49+
users = users.map((u) => {
50+
let email = findEmailInEntry(u);
51+
let user = u.replace(email, '');
52+
if (!/<.+>/.test(email)) {
53+
email = `<${email}>`;
54+
}
55+
user = `${user.trim()} ${email}`;
56+
return user;
57+
});
58+
}
4459
const searchType = state.inputParams.searchType;
4560
api.validateUsers({ courseId, users, searchType })
4661
.then((res) => {
@@ -71,7 +86,12 @@ define([
7186
// the list of users to be enrolled
7287
let usersToBeEnrolled = state.userValidationResult.validUsers.concat(newUsers.usersToBeEnrolled);
7388
// and the list of users to be created
74-
const usersToBeCreated = newUsers.usersToBeCreated;
89+
const usersToBeCreated = newUsers.usersToBeCreated.map((u) => {
90+
if (!u.name) {
91+
return Object.assign(u, {name: u.email});
92+
}
93+
return u;
94+
});
7595

7696
api.createUsers({ courseId, users: usersToBeCreated, inviteUsersURL })
7797
.then((res) => {

‎app/jsx/add_people/components/add_people.jsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ define([
2626
function arePeopleValidationIssuesResolved (props) {
2727
function isReadyToCreate (candidate) {
2828
return !!(candidate.createNew && candidate.newUserInfo
29-
&& candidate.newUserInfo.email && candidate.newUserInfo.name);
29+
&& candidate.newUserInfo.email); // newUserInfo.name is now optional
3030
}
3131

3232
let found = Object.keys(props.userValidationResult.duplicates).find((address) => {

‎app/jsx/add_people/components/missing_people_section.jsx‎

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,35 @@ define([
2525
inviteUsersURL: undefined
2626
}
2727

28+
constructor (props) {
29+
super(props);
30+
31+
this.state = {
32+
selectAll: false
33+
};
34+
}
35+
36+
componentWillReceiveProps (nextProps) {
37+
const all = Object.keys(nextProps.missing).every(m => nextProps.missing[m].createNew)
38+
this.setState({selectAll: all});
39+
}
40+
2841
// event handlers ------------------------------------
2942
// user has chosen to create a new user for this group of duplicates
3043
onSelectNewForMissing = (event) => {
3144
eatEvent(event);
45+
46+
// user may have clicked on the link. if so, put focus on the adjacent checkbox
47+
if (!(event.target.tagName === 'input' && event.target.getAttribute('type') === 'checkbox')) {
48+
const checkbox = event.target.parentElement.parentElement.querySelector('input[type="checkbox"]');
49+
checkbox.focus();
50+
}
51+
3252
const address = event.target.value || event.target.getAttribute('data-address');
53+
this.onSelectNewForMissingByAddress(address);
54+
}
55+
56+
onSelectNewForMissingByAddress (address) {
3357
const missingUser = this.props.missing[address];
3458
let defaultEmail = '';
3559
if (this.props.searchType === 'cc_path') {
@@ -40,16 +64,21 @@ define([
4064
email: (missingUser.newUserInfo && missingUser.newUserInfo.email) || defaultEmail
4165
};
4266

43-
// user may have clicked on the link. if so, put focus on the adjacent checkbox
44-
if (!(event.target.tagName === 'input' && event.target.getAttribute('type') === 'checkbox')) {
45-
const checkbox = event.target.parentElement.parentElement.querySelector('input[type="checkbox"]');
46-
checkbox.focus();
47-
}
48-
4967
if (typeof this.props.onChange === 'function') {
5068
this.props.onChange(address, newUserInfo);
5169
}
5270
}
71+
72+
// check or uncheck all the missing users' checkboxes
73+
onSelectNewForMissingAll = (event) => {
74+
this.setState({selectAll: event.target.checked});
75+
if (event.target.checked) {
76+
Object.keys(this.props.missing).forEach(address => this.onSelectNewForMissingByAddress(address));
77+
} else {
78+
Object.keys(this.props.missing).forEach(address => this.onUncheckUserByAddress(address));
79+
}
80+
}
81+
5382
// when either of the TextInputs for creating a new user for a missing person
5483
// changes, we come here collect the input
5584
// @param event: the event that triggered the change
@@ -60,13 +89,17 @@ define([
6089
newUserInfo[field] = event.target.value;
6190
this.props.onChange(address, newUserInfo);
6291
}
92+
6393
// when user unchecks a checked new user
64-
// @param address: the address field of this user
6594
// @param event: the click event
6695
onUncheckUser = (event) => {
67-
const address = event.target.value;
96+
this.onUncheckUserByAddress(event.target.value);
97+
this.setState({selectAll: false});
98+
}
99+
onUncheckUserByAddress = (address) => {
68100
this.props.onChange(address, false);
69101
}
102+
70103
// send the current list of users on up
71104
onChangeUsers () {
72105
const userList = this.state.candidateUsers.filter(u => (
@@ -184,6 +217,7 @@ define([
184217
label={<ScreenReaderContent>{nameLabel}</ScreenReaderContent>}
185218
data-address={missing.address}
186219
onChange={this.onNewForMissingChange}
220+
value={missing.newUserInfo.name || null}
187221
/>
188222
</td>
189223
<td>{missing.address}</td>
@@ -226,6 +260,13 @@ define([
226260
<tr>
227261
<th scope="col">
228262
<ScreenReaderContent>{I18n.t('User Selection')}</ScreenReaderContent>
263+
<Checkbox
264+
id="missing_users_select_all"
265+
value="__ALL__"
266+
checked={this.state.selectAll}
267+
onChange={this.onSelectNewForMissingAll}
268+
label={<ScreenReaderContent>{I18n.t('Check to select all')}</ScreenReaderContent>}
269+
/>
229270
</th>
230271
<th scope="col">{I18n.t('Name')}</th>
231272
<th scope="col">{I18n.t('Email Address')}</th>

‎app/jsx/add_people/components/people_search.jsx‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ define([
22
'i18n!roster',
33
'react',
44
'instructure-ui',
5-
'./shapes'
5+
'./shapes',
6+
'../helpers'
67
], (I18n, React, {Button, Typography, RadioInputGroup,
78
RadioInput, Select, TextArea, ScreenReaderContent,
8-
Checkbox, Alert}, {courseParamsShape, inputParamsShape}) => {
9+
Checkbox, Alert}, {courseParamsShape, inputParamsShape},
10+
{parseNameList, findEmailInEntry, emailValidator}) => {
911
class PeopleSearch extends React.Component {
1012
static propTypes = Object.assign({}, inputParamsShape, courseParamsShape);
1113

@@ -18,7 +20,6 @@ define([
1820
super(props);
1921

2022
this.namelistta = null;
21-
this.emailValidator = /.+@.+\..+/;
2223
}
2324

2425
shouldComponentUpdate (nextProps, /* nextState */) {
@@ -57,7 +58,11 @@ define([
5758
let message = ' '; // that's a copy/pasted en-space to trick TextArea into
5859
// reserving space for the message so the UI doesn't jump
5960
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+
const users = parseNameList(this.props.nameList);
62+
const badEmail = users.find((u) => {
63+
const email = findEmailInEntry(u);
64+
return !emailValidator.test(email);
65+
});
6166
if (badEmail) {
6267
message = I18n.t('It looks like you have an invalid email address: "%{addr}"', {addr: badEmail});
6368
}

‎app/jsx/add_people/helpers.jsx‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
define([], () => {
2+
// parse the list of names entered by our user into an array
3+
// separates entries on , or \n
4+
// deals with entries like '"Last, First" email' where there's a common w/in quotes
5+
function parseNameList (nameList) {
6+
const names = [];
7+
let iStart = 0;
8+
let inQuote = false;
9+
for (let i = 0; i < nameList.length; ++i) {
10+
const c = nameList.charAt(i);
11+
if (c === '"') {
12+
inQuote = !inQuote;
13+
} else if ((c === ',' && !inQuote) || c === '\n') {
14+
const n = nameList.slice(iStart, i).trim();
15+
if (n.length) names.push(n);
16+
iStart = i + 1;
17+
}
18+
}
19+
const n = nameList.slice(iStart).trim();
20+
if (n.length) names.push(n);
21+
return names;
22+
}
23+
24+
function findEmailInEntry (entry) {
25+
const tokens = entry.split(/\s+/);
26+
const emailIndex = tokens.findIndex(t => t.indexOf('@') >= 0);
27+
return tokens[emailIndex];
28+
}
29+
30+
const emailValidator = /.+@.+\..+/;
31+
32+
return {parseNameList, findEmailInEntry, emailValidator};
33+
});

‎app/jsx/add_people/reducers/userValidationResult_reducer.jsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ define([
4242
address: d.address,
4343
type: d.type,
4444
createNew: false,
45-
newUserInfo: {name: '', email: d.type === 'email' ? d.address : ''}
45+
newUserInfo: {
46+
name: d.user_name || '', // if the user entered a name as part of the email, it comes back in user_name
47+
email: d.type === 'email' ? d.address : ''
48+
}
4649
};
4750
});
4851
}

‎app/jsx/add_people/resolveValidationIssues.jsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ define([], () => {
55

66
Object.keys(duplicates).forEach((addr) => {
77
const dupeSet = duplicates[addr];
8-
if (dupeSet.createNew && dupeSet.newUserInfo.name && dupeSet.newUserInfo.email) {
8+
if (dupeSet.createNew && dupeSet.newUserInfo.email) { // newUserInfo.name now optional
99
usersToBeCreated.push(dupeSet.newUserInfo);
1010
} else if (dupeSet.selectedUserId >= 0) {
1111
const selectedUser = dupeSet.userList.find(u => u.user_id === dupeSet.selectedUserId);
@@ -14,7 +14,7 @@ define([], () => {
1414
});
1515
Object.keys(missings).forEach((addr) => {
1616
const missing = missings[addr];
17-
if (missing.createNew && missing.newUserInfo.name && missing.newUserInfo.email) {
17+
if (missing.createNew && missing.newUserInfo.email) { // newUserInfo.name now optional
1818
usersToBeCreated.push(missing.newUserInfo);
1919
}
2020
});

‎spec/javascripts/jsx/add_people/components/missingPeopleSectionSpec.jsx‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ define([
66
], (React, ReactDOM, TestUtils, MissingPeopleSection) => {
77
QUnit.module('MissingPeopleSection')
88

9-
const missing = {
9+
const missingLogins = {
1010
addr1: {address: 'addr1', type: 'unique_id', createNew: false, newUserInfo: undefined},
1111
addr2: {address: 'addr2', type: 'unique_id', createNew: true, newUserInfo: {name: 'the name2', email: 'email2'}}
1212
}
13+
const missingEmails = {
14+
addr1: {address: 'addr1', type: 'email', createNew: true, newUserInfo: {name: 'Searched Name1', email: 'addr1'}}
15+
}
1316
const noop = function () {};
1417
const inviteUsersURL = '/courses/#/invite_users';
1518

@@ -18,7 +21,7 @@ define([
1821
<MissingPeopleSection
1922
searchType="unique_id"
2023
inviteUsersURL={inviteUsersURL}
21-
missing={missing}
24+
missing={missingLogins}
2225
onChange={noop}
2326
/>)
2427
const missingPeopleSection = TestUtils.findRenderedDOMComponentWithClass(component, 'namelist')
@@ -30,7 +33,7 @@ define([
3033
<MissingPeopleSection
3134
searchType="unique_id"
3235
inviteUsersURL={inviteUsersURL}
33-
missing={missing}
36+
missing={missingLogins}
3437
onChange={noop}
3538
/>)
3639
const missingPeopleSection = TestUtils.findRenderedDOMComponentWithClass(component, 'namelist')
@@ -46,17 +49,32 @@ define([
4649
const emailInput = rows[2].querySelector('input[type="email"]');
4750
ok(emailInput, 'email input');
4851
});
49-
test('cannot create users', () => {
52+
test('cannot create users because we don\'t have the URL', () => {
5053
const component = TestUtils.renderIntoDocument(
5154
<MissingPeopleSection
5255
searchType="unique_id"
5356
inviteUsersURL={undefined}
54-
missing={missing}
57+
missing={missingLogins}
5558
onChange={noop}
5659
/>)
5760
const missingPeopleSection = TestUtils.findRenderedDOMComponentWithClass(component, 'namelist')
5861

5962
const createUserBtn = missingPeopleSection.querySelector('button');
6063
equal(createUserBtn, null, 'create new user button');
6164
})
65+
test('renders real names with email addresses', () => {
66+
const component = TestUtils.renderIntoDocument(
67+
<MissingPeopleSection
68+
searchType="cc_path"
69+
inviteUsersURL={inviteUsersURL}
70+
missing={missingEmails}
71+
onChange={noop}
72+
/>)
73+
const missingPeopleSection = TestUtils.findRenderedDOMComponentWithClass(component, 'namelist')
74+
75+
const rows = missingPeopleSection.querySelectorAll('tbody tr');
76+
equal(rows.length, 1, 'two rows')
77+
const nameInput = rows[0].querySelector('input[type="text"]');
78+
equal(nameInput.value, 'Searched Name1', 'name input');
79+
});
6280
})

0 commit comments

Comments
 (0)