Unable to add same student to two different courses

[Sriram Sami]

Re: the email sent on 14th August about changing capitalized email names to lowercase emails.

Expected Behavior

We are able to add the same student to two different courses.

Current Behavior

Archi system rejects addition of the same student to a second course.

Minimal Reproduce

Preconditions: at least 1 instructor account is created and approved.

1. Create two courses, call them anything.
2. Add a student to course 1 with an email starting with capital E
3. Add the exact same student with identical details to course 2 (after testing, changing the name is irrelevant)
4. Error displayed by red ngToast on top right: 11000 duplicate key error collection: archipelago-dev.students index: username already exists

Leads

I'll be taking a look at what the issue is. My suspicion is that the validation for StudentSchema in modules/students/server/models/student.server.model.js has a hidden bug:

/**
 * Student Schema
 */
var StudentSchema = new Schema({
  created: {
    type: Date,
    default: Date.now
  },
  updated: {
    type: Date
  },
  name: {
    type: String,
    trim: true,
    required: 'Name cannot be blank',
    validate: [validateNameLength, 'Name is too long!']
  },
  username: {
    type: String,
    lowercase: true,
    trim: true,
    required: 'Email cannot be blank',
    unique: 'Email already exists',
    validate: [validateEmail, 'Email is not valid']
  },
});

specifically in the username section, the lowercase + unique combination

  username: {
    type: String,
    lowercase: true,
    trim: true,
    required: 'Email cannot be blank',
    unique: 'Email already exists',
    validate: [validateEmail, 'Email is not valid']
  },

[Sriram Sami]

As specified in the email to users: if in Step 3 the email is changed to a lowercase version, the student is added without issue.

[Sriram Sami]

Somewhat unexpected behavior (?) if we try to add the same student to the same course twice (with lowercase email the second time) - the student is said to be successfully added even the second time. Seems suspicious in this context.

[Sriram Sami]

Analysis of the error code: 11000 duplicate key error collection: archipelago-dev.students index: username already exists - the error seems to be on the students collection, not the combined coursestudents collection. Looks like Mongoose sees this as an attempt to actually add another student to the overall table of students.

When attempting to POST the second student, the console reports an error:

(node:12837) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 21): WriteError({"code":11000,"index":0,"errmsg":"E11000 duplicate key error collection: archipelago-dev.students index: username_1 dup key: { : \"[e-mail]\" }","op":{"__v":0,"_id":"5b7398a3548a0b3225f78ec9","updated":"2018-08-15T03:06:11.217Z","username":"[e-mail]","name":"SriramSami CapsEntry2","created":"2018-08-15T03:06:11.218Z"}})

Relevant view: modules/students/client/instructor/views/course-students/control-panel.client.view.html.

Add Student button at: line 25: <form name="courseForm" class="form-horizontal" ng-submit="addStudent()">.

Add Students (multiple student addition, next tab) on line 69: <a ng-click="addMultipleStudents()" class="btn btn-primary btn-flat pull-right"><i class="fa fa-plus-circle"></i>&nbsp;&nbsp;Add Students</a>

Controller for this view: CourseStudentsControlPanelController at modules/students/client/instructor/controllers/course-students/control-panel.client.controller.js.

Controller code for adding students:

    //Add student
    $scope.addStudent = function(){
      var studentMultiple = [$scope.student];
      $http.post('/api/courses/'+ $scope.courseId +'/course-students/multiple/create', {studentsData: studentMultiple, addToSetup: $scope.students.addToSetup})

Route /course-students/multiple/create is handled in modules/students/server/routes/course-students.server.routes.js:

  app.route('/api/courses/:courseId/course-students/multiple/create').all(courseStudentsPolicy.isAllowed)
     .post(courseStudents.createMultiple);

courseStudents.createMultiple is present in modules/students/server/controllers/course-students.server.controller.js:

exports.createMultiple = function (req, res) {
  var course = req.course;
  var studentsData = req.body.studentsData;

  addMultipleStudentsToCourseWithStudent(course, studentsData, function(err, foundCourseStudents, createdCourseStudents){ .....

addMultipleStudentsToCourseWithStudent has an async waterfall that calls studentController.findOrCreateMultiple(authorisedStudentsData, callback); where authorizedStudentsData is any student submitted for creation that has both email and name.

The studentController specified is present in modules/students/server/controllers/students.server.controller.js.

Working Hypothesis: findOrCreateMultiple function cannot find the incoming student, so it attempts to create which is why there is an error.

[Sriram Sami]

Convert all student usernames to lowercase on add

Fixes #12

When adding students to a course, this commit ensures their usernames are converted to lowercase first before trying to search for the existence of that user.

E.g. previously: add user "[e-mail]" becomes: find user with "[e-mail]" --> doesn't exist because Mongoose converts all usernames to lowercase when adding --> try to create user with username "[e-mail]" --> now Mongoose tries to lowercase and add --> error.

We now lowercase so that the process fails at the "find" step and creation is not attempted.

→ <<cset 46d41180cc3d>>

[Sriram Sami]

Automatically closed by PR - re-opening until merged.