From c64f0b1a180897feae71c47ae52df237d4124427 Mon Sep 17 00:00:00 2001 From: dcadams Date: Mon, 29 Apr 2013 09:38:10 -0700 Subject: [PATCH 1/9] Modifications to instructor dashboard - enrollment tab: - Remove single enrollment/unenrollment - Make unenrollment work with multi-email listbox. - Provide checkbox for auto-enroll on activate if user not yet signed up. - Enroll student when activating if auto-enroll flag set. --- ...eld_courseenrollmentallowed_auto_enroll.py | 173 ++++++++++++++++++ common/djangoapps/student/models.py | 1 + common/djangoapps/student/views.py | 11 +- .../instructor/tests/test_enrollment.py | 160 ++++++++++++++++ lms/djangoapps/instructor/views.py | 114 ++++++++---- .../courseware/instructor_dashboard.html | 11 +- 6 files changed, 424 insertions(+), 46 deletions(-) create mode 100644 common/djangoapps/student/migrations/0025_auto__add_field_courseenrollmentallowed_auto_enroll.py create mode 100644 lms/djangoapps/instructor/tests/test_enrollment.py diff --git a/common/djangoapps/student/migrations/0025_auto__add_field_courseenrollmentallowed_auto_enroll.py b/common/djangoapps/student/migrations/0025_auto__add_field_courseenrollmentallowed_auto_enroll.py new file mode 100644 index 0000000000..8ce1d0cda1 --- /dev/null +++ b/common/djangoapps/student/migrations/0025_auto__add_field_courseenrollmentallowed_auto_enroll.py @@ -0,0 +1,173 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding field 'CourseEnrollmentAllowed.auto_enroll' + db.add_column('student_courseenrollmentallowed', 'auto_enroll', + self.gf('django.db.models.fields.BooleanField')(default=False), + keep_default=False) + + + def backwards(self, orm): + # Deleting field 'CourseEnrollmentAllowed.auto_enroll' + db.delete_column('student_courseenrollmentallowed', 'auto_enroll') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'student.courseenrollment': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'CourseEnrollment'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.courseenrollmentallowed': { + 'Meta': {'unique_together': "(('email', 'course_id'),)", 'object_name': 'CourseEnrollmentAllowed'}, + 'auto_enroll': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'email': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'student.pendingemailchange': { + 'Meta': {'object_name': 'PendingEmailChange'}, + 'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'new_email': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.pendingnamechange': { + 'Meta': {'object_name': 'PendingNameChange'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'new_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'rationale': ('django.db.models.fields.CharField', [], {'max_length': '1024', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.registration': { + 'Meta': {'object_name': 'Registration', 'db_table': "'auth_registration'"}, + 'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.testcenterregistration': { + 'Meta': {'object_name': 'TestCenterRegistration'}, + 'accommodation_code': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}), + 'accommodation_request': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '1024', 'blank': 'True'}), + 'authorization_id': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'db_index': 'True'}), + 'client_authorization_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '20', 'db_index': 'True'}), + 'confirmed_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '128', 'db_index': 'True'}), + 'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'eligibility_appointment_date_first': ('django.db.models.fields.DateField', [], {'db_index': 'True'}), + 'eligibility_appointment_date_last': ('django.db.models.fields.DateField', [], {'db_index': 'True'}), + 'exam_series_code': ('django.db.models.fields.CharField', [], {'max_length': '15', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'processed_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'testcenter_user': ('django.db.models.fields.related.ForeignKey', [], {'default': 'None', 'to': "orm['student.TestCenterUser']"}), + 'updated_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'upload_error_message': ('django.db.models.fields.CharField', [], {'max_length': '512', 'blank': 'True'}), + 'upload_status': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '20', 'blank': 'True'}), + 'uploaded_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'user_updated_at': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}) + }, + 'student.testcenteruser': { + 'Meta': {'object_name': 'TestCenterUser'}, + 'address_1': ('django.db.models.fields.CharField', [], {'max_length': '40'}), + 'address_2': ('django.db.models.fields.CharField', [], {'max_length': '40', 'blank': 'True'}), + 'address_3': ('django.db.models.fields.CharField', [], {'max_length': '40', 'blank': 'True'}), + 'candidate_id': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'db_index': 'True'}), + 'city': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}), + 'client_candidate_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '50', 'db_index': 'True'}), + 'company_name': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '50', 'blank': 'True'}), + 'confirmed_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'country': ('django.db.models.fields.CharField', [], {'max_length': '3', 'db_index': 'True'}), + 'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'extension': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '8', 'blank': 'True'}), + 'fax': ('django.db.models.fields.CharField', [], {'max_length': '35', 'blank': 'True'}), + 'fax_country_code': ('django.db.models.fields.CharField', [], {'max_length': '3', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '50', 'db_index': 'True'}), + 'middle_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'phone': ('django.db.models.fields.CharField', [], {'max_length': '35'}), + 'phone_country_code': ('django.db.models.fields.CharField', [], {'max_length': '3', 'db_index': 'True'}), + 'postal_code': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '16', 'blank': 'True'}), + 'processed_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'salutation': ('django.db.models.fields.CharField', [], {'max_length': '50', 'blank': 'True'}), + 'state': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '20', 'blank': 'True'}), + 'suffix': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'updated_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'upload_error_message': ('django.db.models.fields.CharField', [], {'max_length': '512', 'blank': 'True'}), + 'upload_status': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '20', 'blank': 'True'}), + 'uploaded_at': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'default': 'None', 'to': "orm['auth.User']", 'unique': 'True'}), + 'user_updated_at': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}) + }, + 'student.userprofile': { + 'Meta': {'object_name': 'UserProfile', 'db_table': "'auth_userprofile'"}, + 'allow_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'courseware': ('django.db.models.fields.CharField', [], {'default': "'course.xml'", 'max_length': '255', 'blank': 'True'}), + 'gender': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}), + 'goals': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'language': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'level_of_education': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}), + 'location': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'mailing_address': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'meta': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'profile'", 'unique': 'True', 'to': "orm['auth.User']"}), + 'year_of_birth': ('django.db.models.fields.IntegerField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'}) + }, + 'student.usertestgroup': { + 'Meta': {'object_name': 'UserTestGroup'}, + 'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}), + 'users': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.User']", 'db_index': 'True', 'symmetrical': 'False'}) + } + } + + complete_apps = ['student'] \ No newline at end of file diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 56b1293c2d..ab68b05f4b 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -662,6 +662,7 @@ class CourseEnrollmentAllowed(models.Model): """ email = models.CharField(max_length=255, db_index=True) course_id = models.CharField(max_length=255, db_index=True) + auto_enroll = models.BooleanField(default=0) created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 98b5265d5f..2b359acc6a 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -31,7 +31,7 @@ from student.models import (Registration, UserProfile, TestCenterUser, TestCente TestCenterRegistration, TestCenterRegistrationForm, PendingNameChange, PendingEmailChange, CourseEnrollment, unique_id_for_user, - get_testcenter_registration) + get_testcenter_registration, CourseEnrollmentAllowed) from certificates.models import CertificateStatuses, certificate_status_for_student @@ -825,6 +825,15 @@ def activate_account(request, key): if not r[0].user.is_active: r[0].activate() already_active = False + + #Enroll student in any pending courses he/she may have if auto_enroll flag is set + student = request.user + ceas = CourseEnrollmentAllowed.objects.filter(email=student.email) + for cea in ceas: + if cea.auto_enroll: + course_id = cea.course_id + enrollment, created = CourseEnrollment.objects.get_or_create(user_id=student.id, course_id=course_id) + resp = render_to_response("registration/activation_complete.html", {'user_logged_in': user_logged_in, 'already_active': already_active}) return resp if len(r) == 0: diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py new file mode 100644 index 0000000000..877f46da32 --- /dev/null +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -0,0 +1,160 @@ +""" Unit tests for enrollment methods in views.py """ + +from django.test.utils import override_settings +from django.contrib.auth.models import Group, User +from django.core.urlresolvers import reverse +from courseware.access import _course_staff_group_name +from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULESTORE, get_user +from xmodule.modulestore.django import modulestore +import xmodule.modulestore.django +from student.models import CourseEnrollment, CourseEnrollmentAllowed + + +@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) +class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): + + def setUp(self): + xmodule.modulestore.django._MODULESTORES = {} + + self.full = modulestore().get_course("edX/full/6.002_Spring_2012") + self.toy = modulestore().get_course("edX/toy/2012_Fall") + + #Create instructor and student accounts + self.instructor = 'instructor1@test.com' + self.student1 = 'student1@test.com' + self.student2 = 'student2@test.com' + self.password = 'foo' + self.create_account('it1', self.instructor, self.password) + self.create_account('st1', self.student1, self.password) + self.create_account('st2', self.student2, self.password) + self.activate_user(self.instructor) + self.activate_user(self.student1) + self.activate_user(self.student2) + + def make_instructor(course): + group_name = _course_staff_group_name(course.location) + g = Group.objects.create(name=group_name) + g.user_set.add(get_user(self.instructor)) + + make_instructor(self.toy) + + #Enroll Students + self.logout() + self.login(self.student1, self.password) + self.enroll(self.toy) + + self.logout() + self.login(self.student2, self.password) + self.enroll(self.toy) + + #Enroll Instructor + self.logout() + self.login(self.instructor, self.password) + self.enroll(self.toy) + + + def test_unenrollment(self): + #Do un-enrollment + course = self.toy + url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) + response = self.client.post(url, {'action': 'Unenroll multiple students', 'multiple_students': 'student1@test.com, student2@test.com'}) + + #Check the page output + self.assertContains(response, 'student1@test.com') + self.assertContains(response, 'student1@test.com') + self.assertContains(response, 'un-enrolled') + + #Check the enrollment table + user = User.objects.get(email='student1@test.com') + ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) + self.assertEqual(0, len(ce)) + + user = User.objects.get(email='student2@test.com') + ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) + self.assertEqual(0, len(ce)) + + + def test_enrollmemt_new_student_autoenroll_on(self): + + #Run the Enroll students command + course = self.toy + url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) + response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'test1_1@student.com, test1_2@student.com', 'auto_enroll': 'on'}) + + #Check the page output + self.assertContains(response, 'test1_1@student.com') + self.assertContains(response, 'test1_2@student.com') + self.assertContains(response, 'user does not exist, enrollment allowed, pending with auto enrollment on') + + #Check the enrollmentallowed db entries + cea = CourseEnrollmentAllowed.objects.filter(email='test1_1@student.com', course_id=course.id) + self.assertEqual(1, cea[0].auto_enroll) + cea = CourseEnrollmentAllowed.objects.filter(email='test1_2@student.com', course_id=course.id) + self.assertEqual(1, cea[0].auto_enroll) + + #Check there is no enrollment db entry other than for the setup instructor and students + ce = CourseEnrollment.objects.filter(course_id=course.id) + self.assertEqual(3, len(ce)) + + #Create and activate student accounts with same email + self.student1 = 'test1_1@student.com' + self.password = 'bar' + self.create_account('s1_1', self.student1, self.password) + self.activate_user(self.student1) + + self.student2 = 'test1_2@student.com' + self.password = 'bar' + self.create_account('s1_2', self.student2, self.password) + self.activate_user(self.student2) + + #Check students are enrolled + user = User.objects.get(email='test1_1@student.com') + ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) + self.assertEqual(1, len(ce)) + + user = User.objects.get(email='test1_2@student.com') + ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) + self.assertEqual(1, len(ce)) + + + def test_enrollmemt_new_student_autoenroll_off(self): + + #Run the Enroll students command + course = self.toy + url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) + response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'test2_1@student.com, test2_2@student.com'}) + + #Check the page output + self.assertContains(response, 'test2_1@student.com') + self.assertContains(response, 'test2_2@student.com') + self.assertContains(response, 'user does not exist, enrollment allowed, pending with auto enrollment off') + + #Check the enrollmentallowed db entries + cea = CourseEnrollmentAllowed.objects.filter(email='test2_1@student.com', course_id=course.id) + self.assertEqual(0, cea[0].auto_enroll) + cea = CourseEnrollmentAllowed.objects.filter(email='test2_2@student.com', course_id=course.id) + self.assertEqual(0, cea[0].auto_enroll) + + #Check there is no enrollment db entry other than for the setup instructor and students + ce = CourseEnrollment.objects.filter(course_id=course.id) + self.assertEqual(3, len(ce)) + + #Create and activate student accounts with same email + self.student = 'test2_1@student.com' + self.password = 'bar' + self.create_account('s2_1', self.student, self.password) + self.activate_user(self.student) + + self.student = 'test2_2@student.com' + self.password = 'bar' + self.create_account('s2_2', self.student, self.password) + self.activate_user(self.student) + + #Check students are not enrolled + user = User.objects.get(email='test2_1@student.com') + ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) + self.assertEqual(0, len(ce)) + user = User.objects.get(email='test2_2@student.com') + ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) + self.assertEqual(0, len(ce)) + \ No newline at end of file diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index dd6748e691..e36fb3a3ac 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -539,35 +539,17 @@ def instructor_dashboard(request, course_id): datatable['data'] = [[x.email] for x in ceaset] datatable['title'] = action - elif action == 'Enroll student': - - student = request.POST.get('enstudent', '') - ret = _do_enroll_students(course, course_id, student) - datatable = ret['datatable'] - - elif action == 'Un-enroll student': - - student = request.POST.get('enstudent', '') - datatable = {} - isok = False - cea = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=student) - if cea: - cea.delete() - msg += "Un-enrolled student with email '%s'" % student - isok = True - try: - nce = CourseEnrollment.objects.get(user=User.objects.get(email=student), course_id=course_id) - nce.delete() - msg += "Un-enrolled student with email '%s'" % student - except Exception as err: - if not isok: - msg += "Error! Failed to un-enroll student with email '%s'\n" % student - msg += str(err) + '\n' - elif action == 'Enroll multiple students': - students = request.POST.get('enroll_multiple', '') - ret = _do_enroll_students(course, course_id, students) + students = request.POST.get('multiple_students', '') + auto_enroll = request.POST.get('auto_enroll', False) != False + ret = _do_enroll_students(course_id, students, auto_enroll=auto_enroll) + datatable = ret['datatable'] + + elif action == 'Unenroll multiple students': + + students = request.POST.get('multiple_students', '') + ret = _do_unenroll_students(course_id, students) datatable = ret['datatable'] elif action == 'List sections available in remote gradebook': @@ -985,17 +967,11 @@ def grade_summary(request, course_id): #----------------------------------------------------------------------------- # enrollment -def _do_enroll_students(course, course_id, students, overload=False): +def _do_enroll_students(course_id, students, overload=False, auto_enroll=False): """Do the actual work of enrolling multiple students, presented as a string of emails separated by commas or returns""" - new_students = split_by_comma_and_whitespace(students) - new_students = [str(s.strip()) for s in new_students] - new_students_lc = [x.lower() for x in new_students] - - if '' in new_students: - new_students.remove('') - + new_students = get_and_clean_student_list(students) status = dict([x, 'unprocessed'] for x in new_students) if overload: # delete all but staff @@ -1015,13 +991,20 @@ def _do_enroll_students(course, course_id, students, overload=False): try: user = User.objects.get(email=student) except User.DoesNotExist: - # user not signed up yet, put in pending enrollment allowed table - if CourseEnrollmentAllowed.objects.filter(email=student, course_id=course_id): - status[student] = 'user does not exist, enrollment already allowed, pending' + + #User not signed up yet, put in pending enrollment allowed table + cea = CourseEnrollmentAllowed.objects.filter(email=student, course_id=course_id) + + #If enrollmentallowed already exists, update auto_enroll flag to however it was set in UI + #Will be 0 or 1 records as there is a unique key on email + course_id + if cea: + cea[0].auto_enroll = auto_enroll + cea[0].save() + status[student] = 'user does not exist, enrollment already allowed, pending with auto enrollment ' + ("off", "on")[auto_enroll] continue - cea = CourseEnrollmentAllowed(email=student, course_id=course_id) + cea = CourseEnrollmentAllowed(email=student, course_id=course_id, auto_enroll=auto_enroll) cea.save() - status[student] = 'user does not exist, enrollment allowed, pending' + status[student] = 'user does not exist, enrollment allowed, pending with auto enrollment ' + ("off", "on")[auto_enroll] continue if CourseEnrollment.objects.filter(user=user, course_id=course_id): @@ -1077,6 +1060,57 @@ def enroll_students(request, course_id): 'debug': new_students}) +#Unenrollment +def _do_unenroll_students(course_id, students): + """Do the actual work of un-enrolling multiple students, presented as a string + of emails separated by commas or returns""" + + old_students = get_and_clean_student_list(students) + status = dict([x, 'unprocessed'] for x in old_students) + + for student in old_students: + + isok = False + cea = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=student) + #Safe to get the first entry as there is a unique key on email + course_id + if cea: + cea[0].delete() + status[student] = "un-enrolled" + isok = True + + try: + user = User.objects.get(email=student) + except User.DoesNotExist: + continue + + nce = CourseEnrollment.objects.filter(user=user, course_id=course_id) + #Safe to get the first entry as there is a unique key on user + course_id + if nce: + try: + nce[0].delete() + status[student] = "un-enrolled" + except Exception as err: + if not isok: + status[student] = "Error! Failed to un-enroll" + + datatable = {'header': ['StudentEmail', 'action']} + datatable['data'] = [[x, status[x]] for x in status] + datatable['title'] = 'Un-enrollment of students' + + data = dict(datatable=datatable) + return data + +def get_and_clean_student_list(students): + + students = split_by_comma_and_whitespace(students) + students = [str(s.strip()) for s in students] + students_lc = [x.lower() for x in students] + + if '' in students: + students.remove('') + + return students + #----------------------------------------------------------------------------- # answer distribution diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 4898914d01..4b37004181 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -296,9 +296,6 @@ function goto( mode)

-

- Student Email: -

%if settings.MITX_FEATURES.get('REMOTE_GRADEBOOK_URL','') and instructor_access: @@ -320,9 +317,13 @@ function goto( mode) %endif -

Add students: enter emails, separated by new lines or commas;

- +

Enroll or un-enroll one or many students: enter emails, separated by new lines or commas;

+ +

+ Auto-enroll students when they activate +

+ %endif From aa4b61180c6f5aa36a62eab6c356ab45ec9fadb2 Mon Sep 17 00:00:00 2001 From: dcadams Date: Mon, 29 Apr 2013 10:03:11 -0700 Subject: [PATCH 2/9] Some cleanup. --- .../instructor/tests/test_enrollment.py | 2 -- lms/djangoapps/instructor/views.py | 34 ++----------------- lms/urls.py | 2 -- 3 files changed, 2 insertions(+), 36 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 877f46da32..a33e224e5e 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -103,7 +103,6 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): self.activate_user(self.student1) self.student2 = 'test1_2@student.com' - self.password = 'bar' self.create_account('s1_2', self.student2, self.password) self.activate_user(self.student2) @@ -146,7 +145,6 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): self.activate_user(self.student) self.student = 'test2_2@student.com' - self.password = 'bar' self.create_account('s2_2', self.student, self.password) self.activate_user(self.student) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index e36fb3a3ac..606f875eb8 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -1030,36 +1030,6 @@ def _do_enroll_students(course_id, students, overload=False, auto_enroll=False): return data -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -def enroll_students(request, course_id): - """Allows a staff member to enroll students in a course. - - This is a short-term hack for Berkeley courses launching fall - 2012. In the long term, we would like functionality like this, but - we would like both the instructor and the student to agree. Right - now, this allows any instructor to add students to their course, - which we do not want. - - It is poorly written and poorly tested, but it's designed to be - stripped out. - """ - - course = get_course_with_access(request.user, course_id, 'staff') - existing_students = [ce.user.email for ce in CourseEnrollment.objects.filter(course_id=course_id)] - - new_students = request.POST.get('new_students') - ret = _do_enroll_students(course, course_id, new_students) - added_students = ret['added'] - rejected_students = ret['rejected'] - - return render_to_response("enroll_students.html", {'course': course_id, - 'existing_students': existing_students, - 'added_students': added_students, - 'rejected_students': rejected_students, - 'debug': new_students}) - - #Unenrollment def _do_unenroll_students(course_id, students): """Do the actual work of un-enrolling multiple students, presented as a string @@ -1072,7 +1042,7 @@ def _do_unenroll_students(course_id, students): isok = False cea = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=student) - #Safe to get the first entry as there is a unique key on email + course_id + #Will be 0 or 1 records as there is a unique key on email + course_id if cea: cea[0].delete() status[student] = "un-enrolled" @@ -1084,7 +1054,7 @@ def _do_unenroll_students(course_id, students): continue nce = CourseEnrollment.objects.filter(user=user, course_id=course_id) - #Safe to get the first entry as there is a unique key on user + course_id + #Will be 0 or 1 records as there is a unique key on user + course_id if nce: try: nce[0].delete() diff --git a/lms/urls.py b/lms/urls.py index 082004c1be..375335e3ea 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -236,8 +236,6 @@ if settings.COURSEWARE_ENABLED: 'instructor.views.gradebook', name='gradebook'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/grade_summary$', 'instructor.views.grade_summary', name='grade_summary'), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/enroll_students$', - 'instructor.views.enroll_students', name='enroll_students'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading$', 'open_ended_grading.views.staff_grading', name='staff_grading'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/staff_grading/get_next$', From 6a1a907339ece6231dd681f4f96fac144eef098a Mon Sep 17 00:00:00 2001 From: dcadams Date: Tue, 30 Apr 2013 11:48:23 -0700 Subject: [PATCH 3/9] Fix pep8 violations. --- .../instructor/tests/test_enrollment.py | 69 +++++++++++-------- lms/djangoapps/instructor/views.py | 26 +++---- 2 files changed, 52 insertions(+), 43 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index a33e224e5e..d2e31d3eb6 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -1,4 +1,6 @@ -""" Unit tests for enrollment methods in views.py """ +''' +Unit tests for enrollment methods in views.py +''' from django.test.utils import override_settings from django.contrib.auth.models import Group, User @@ -12,9 +14,11 @@ from student.models import CourseEnrollment, CourseEnrollmentAllowed @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): + ''' + Check Enrollment/Unenrollment with/without auto-enrollment on activation + ''' def setUp(self): - xmodule.modulestore.django._MODULESTORES = {} self.full = modulestore().get_course("edX/full/6.002_Spring_2012") self.toy = modulestore().get_course("edX/toy/2012_Fall") @@ -42,112 +46,118 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): self.logout() self.login(self.student1, self.password) self.enroll(self.toy) - + self.logout() self.login(self.student2, self.password) self.enroll(self.toy) - + #Enroll Instructor self.logout() self.login(self.instructor, self.password) self.enroll(self.toy) - def test_unenrollment(self): - #Do un-enrollment + ''' + Do un-enrollment test + ''' + course = self.toy url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) response = self.client.post(url, {'action': 'Unenroll multiple students', 'multiple_students': 'student1@test.com, student2@test.com'}) - + #Check the page output self.assertContains(response, 'student1@test.com') self.assertContains(response, 'student1@test.com') self.assertContains(response, 'un-enrolled') - + #Check the enrollment table user = User.objects.get(email='student1@test.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) self.assertEqual(0, len(ce)) - + user = User.objects.get(email='student2@test.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) - self.assertEqual(0, len(ce)) - + self.assertEqual(0, len(ce)) def test_enrollmemt_new_student_autoenroll_on(self): - + ''' + Do auto-enroll on test + ''' + #Run the Enroll students command course = self.toy url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'test1_1@student.com, test1_2@student.com', 'auto_enroll': 'on'}) - + #Check the page output self.assertContains(response, 'test1_1@student.com') self.assertContains(response, 'test1_2@student.com') self.assertContains(response, 'user does not exist, enrollment allowed, pending with auto enrollment on') - + #Check the enrollmentallowed db entries cea = CourseEnrollmentAllowed.objects.filter(email='test1_1@student.com', course_id=course.id) self.assertEqual(1, cea[0].auto_enroll) cea = CourseEnrollmentAllowed.objects.filter(email='test1_2@student.com', course_id=course.id) self.assertEqual(1, cea[0].auto_enroll) - + #Check there is no enrollment db entry other than for the setup instructor and students ce = CourseEnrollment.objects.filter(course_id=course.id) self.assertEqual(3, len(ce)) - + #Create and activate student accounts with same email self.student1 = 'test1_1@student.com' self.password = 'bar' self.create_account('s1_1', self.student1, self.password) self.activate_user(self.student1) - + self.student2 = 'test1_2@student.com' self.create_account('s1_2', self.student2, self.password) self.activate_user(self.student2) - + #Check students are enrolled user = User.objects.get(email='test1_1@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) self.assertEqual(1, len(ce)) - + user = User.objects.get(email='test1_2@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) - self.assertEqual(1, len(ce)) - + self.assertEqual(1, len(ce)) def test_enrollmemt_new_student_autoenroll_off(self): - + ''' + Do auto-enroll off test + ''' + #Run the Enroll students command course = self.toy url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'test2_1@student.com, test2_2@student.com'}) - + #Check the page output self.assertContains(response, 'test2_1@student.com') self.assertContains(response, 'test2_2@student.com') self.assertContains(response, 'user does not exist, enrollment allowed, pending with auto enrollment off') - + #Check the enrollmentallowed db entries cea = CourseEnrollmentAllowed.objects.filter(email='test2_1@student.com', course_id=course.id) self.assertEqual(0, cea[0].auto_enroll) cea = CourseEnrollmentAllowed.objects.filter(email='test2_2@student.com', course_id=course.id) self.assertEqual(0, cea[0].auto_enroll) - + #Check there is no enrollment db entry other than for the setup instructor and students ce = CourseEnrollment.objects.filter(course_id=course.id) self.assertEqual(3, len(ce)) - + #Create and activate student accounts with same email self.student = 'test2_1@student.com' self.password = 'bar' self.create_account('s2_1', self.student, self.password) self.activate_user(self.student) - + self.student = 'test2_2@student.com' self.create_account('s2_2', self.student, self.password) - self.activate_user(self.student) - + self.activate_user(self.student) + #Check students are not enrolled user = User.objects.get(email='test2_1@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) @@ -155,4 +165,3 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): user = User.objects.get(email='test2_2@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) self.assertEqual(0, len(ce)) - \ No newline at end of file diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 606f875eb8..756d1a6204 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -542,12 +542,12 @@ def instructor_dashboard(request, course_id): elif action == 'Enroll multiple students': students = request.POST.get('multiple_students', '') - auto_enroll = request.POST.get('auto_enroll', False) != False - ret = _do_enroll_students(course_id, students, auto_enroll=auto_enroll) + auto_enroll = request.POST.get('auto_enroll', False) is not False + ret = _do_enroll_students(course, course_id, students, auto_enroll=auto_enroll) datatable = ret['datatable'] elif action == 'Unenroll multiple students': - + students = request.POST.get('multiple_students', '') ret = _do_unenroll_students(course_id, students) datatable = ret['datatable'] @@ -967,11 +967,11 @@ def grade_summary(request, course_id): #----------------------------------------------------------------------------- # enrollment -def _do_enroll_students(course_id, students, overload=False, auto_enroll=False): +def _do_enroll_students(course, course_id, students, overload=False, auto_enroll=False): """Do the actual work of enrolling multiple students, presented as a string of emails separated by commas or returns""" - new_students = get_and_clean_student_list(students) + new_students, new_students_lc = get_and_clean_student_list(students) status = dict([x, 'unprocessed'] for x in new_students) if overload: # delete all but staff @@ -991,10 +991,10 @@ def _do_enroll_students(course_id, students, overload=False, auto_enroll=False): try: user = User.objects.get(email=student) except User.DoesNotExist: - + #User not signed up yet, put in pending enrollment allowed table cea = CourseEnrollmentAllowed.objects.filter(email=student, course_id=course_id) - + #If enrollmentallowed already exists, update auto_enroll flag to however it was set in UI #Will be 0 or 1 records as there is a unique key on email + course_id if cea: @@ -1035,11 +1035,11 @@ def _do_unenroll_students(course_id, students): """Do the actual work of un-enrolling multiple students, presented as a string of emails separated by commas or returns""" - old_students = get_and_clean_student_list(students) + old_students, old_students_lc = get_and_clean_student_list(students) status = dict([x, 'unprocessed'] for x in old_students) - + for student in old_students: - + isok = False cea = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=student) #Will be 0 or 1 records as there is a unique key on email + course_id @@ -1047,7 +1047,7 @@ def _do_unenroll_students(course_id, students): cea[0].delete() status[student] = "un-enrolled" isok = True - + try: user = User.objects.get(email=student) except User.DoesNotExist: @@ -1066,7 +1066,7 @@ def _do_unenroll_students(course_id, students): datatable = {'header': ['StudentEmail', 'action']} datatable['data'] = [[x, status[x]] for x in status] datatable['title'] = 'Un-enrollment of students' - + data = dict(datatable=datatable) return data @@ -1079,7 +1079,7 @@ def get_and_clean_student_list(students): if '' in students: students.remove('') - return students + return students, students_lc #----------------------------------------------------------------------------- # answer distribution From 2ef3bcc92dd599125cc5fe34566953717e9ac841 Mon Sep 17 00:00:00 2001 From: dcadams Date: Wed, 8 May 2013 14:52:18 -0700 Subject: [PATCH 4/9] Commit just to get Jenkins to rerun. --- lms/djangoapps/instructor/tests/test_enrollment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index d2e31d3eb6..775f39f442 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -1,5 +1,6 @@ ''' Unit tests for enrollment methods in views.py + ''' from django.test.utils import override_settings From 5e75e9132204ce086cc6f454e2cd855320231f67 Mon Sep 17 00:00:00 2001 From: dcadams Date: Mon, 13 May 2013 13:20:37 -0700 Subject: [PATCH 5/9] Takes care of Victor's comments on the PR. Note, the students_lc list is used in _do_enroll_students when overload = True so it needs to be left in. --- .../instructor/tests/test_enrollment.py | 13 +++++++-- lms/djangoapps/instructor/views.py | 29 ++++++++++++------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 775f39f442..5c4cbd4ec5 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -1,6 +1,5 @@ ''' Unit tests for enrollment methods in views.py - ''' from django.test.utils import override_settings @@ -11,6 +10,7 @@ from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULE from xmodule.modulestore.django import modulestore import xmodule.modulestore.django from student.models import CourseEnrollment, CourseEnrollmentAllowed +from instructor.views import get_and_clean_student_list @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) @@ -68,7 +68,7 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): #Check the page output self.assertContains(response, 'student1@test.com') - self.assertContains(response, 'student1@test.com') + self.assertContains(response, 'student2@test.com') self.assertContains(response, 'un-enrolled') #Check the enrollment table @@ -166,3 +166,12 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): user = User.objects.get(email='test2_2@student.com') ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) self.assertEqual(0, len(ce)) + + def test_get_and_clean_student_list(self): + ''' + Clean user input test + ''' + + string = "abc@test.com, def@test.com ghi@test.com \n \n jkl@test.com " + cleaned_string, cleaned_string_lc = get_and_clean_student_list(string) + self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com']) \ No newline at end of file diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 756d1a6204..a16e5e84c4 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -542,7 +542,7 @@ def instructor_dashboard(request, course_id): elif action == 'Enroll multiple students': students = request.POST.get('multiple_students', '') - auto_enroll = request.POST.get('auto_enroll', False) is not False + auto_enroll = bool(request.POST.get('auto_enroll')) ret = _do_enroll_students(course, course_id, students, auto_enroll=auto_enroll) datatable = ret['datatable'] @@ -1000,11 +1000,12 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll if cea: cea[0].auto_enroll = auto_enroll cea[0].save() - status[student] = 'user does not exist, enrollment already allowed, pending with auto enrollment ' + ("off", "on")[auto_enroll] + status[student] = 'user does not exist, enrollment already allowed, pending with auto enrollment ' \ + + ('on' if auto_enroll else 'off') continue cea = CourseEnrollmentAllowed(email=student, course_id=course_id, auto_enroll=auto_enroll) cea.save() - status[student] = 'user does not exist, enrollment allowed, pending with auto enrollment ' + ("off", "on")[auto_enroll] + status[student] = 'user does not exist, enrollment allowed, pending with auto enrollment ' + ('on' if auto_enroll else 'off') continue if CourseEnrollment.objects.filter(user=user, course_id=course_id): @@ -1018,7 +1019,7 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll status[student] = 'rejected' datatable = {'header': ['StudentEmail', 'action']} - datatable['data'] = [[x, status[x]] for x in status] + datatable['data'] = [[x, status[x]] for x in sorted(status)] datatable['title'] = 'Enrollment of students' def sf(stat): @@ -1050,7 +1051,7 @@ def _do_unenroll_students(course_id, students): try: user = User.objects.get(email=student) - except User.DoesNotExist: + except User.DoesNotExist: continue nce = CourseEnrollment.objects.filter(user=user, course_id=course_id) @@ -1064,21 +1065,29 @@ def _do_unenroll_students(course_id, students): status[student] = "Error! Failed to un-enroll" datatable = {'header': ['StudentEmail', 'action']} - datatable['data'] = [[x, status[x]] for x in status] + datatable['data'] = [[x, status[x]] for x in sorted(status)] datatable['title'] = 'Un-enrollment of students' data = dict(datatable=datatable) return data + def get_and_clean_student_list(students): - + """ + Separate out individual student email from the comma, or space separated string. + + In: + students: string coming from the input text area + Return: + students: list of cleaned student emails + students_lc: list of lower case cleaned student emails + """ + students = split_by_comma_and_whitespace(students) students = [str(s.strip()) for s in students] + students = [s for s in students if s != ''] students_lc = [x.lower() for x in students] - if '' in students: - students.remove('') - return students, students_lc #----------------------------------------------------------------------------- From 67356cdcc816f2cec579c69bd44446e0dba6eb25 Mon Sep 17 00:00:00 2001 From: dcadams Date: Wed, 15 May 2013 09:54:53 -0700 Subject: [PATCH 6/9] Merge of master to get Jenkins running. --- lms/djangoapps/instructor/tests/test_enrollment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 5c4cbd4ec5..1136b7cd04 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -1,5 +1,6 @@ ''' Unit tests for enrollment methods in views.py + ''' from django.test.utils import override_settings From 1ea65455565f2369c220c0a9ce4dfa5376740411 Mon Sep 17 00:00:00 2001 From: dcadams Date: Tue, 4 Jun 2013 14:13:13 -0700 Subject: [PATCH 7/9] Worked on reducing pep8 violations. --- common/djangoapps/student/views.py | 28 ++++----- .../instructor/tests/test_enrollment.py | 4 +- lms/djangoapps/instructor/views.py | 61 +++++++++---------- 3 files changed, 42 insertions(+), 51 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index c74650b7f4..fd0f643af0 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -264,7 +264,6 @@ def dashboard(request): if not user.is_active: message = render_to_string('registration/activate_account_notice.html', {'email': user.email}) - # Global staff can see what courses errored on their dashboard staff_access = False errored_courses = {} @@ -355,7 +354,7 @@ def change_enrollment(request): course = course_from_id(course_id) except ItemNotFoundError: log.warning("User {0} tried to enroll in non-existent course {1}" - .format(user.username, course_id)) + .format(user.username, course_id)) return HttpResponseBadRequest("Course id is invalid") if not has_access(user, course, 'enroll'): @@ -363,9 +362,9 @@ def change_enrollment(request): org, course_num, run = course_id.split("/") statsd.increment("common.student.enrollment", - tags=["org:{0}".format(org), - "course:{0}".format(course_num), - "run:{0}".format(run)]) + tags=["org:{0}".format(org), + "course:{0}".format(course_num), + "run:{0}".format(run)]) try: enrollment, created = CourseEnrollment.objects.get_or_create(user=user, course_id=course.id) @@ -382,9 +381,9 @@ def change_enrollment(request): org, course_num, run = course_id.split("/") statsd.increment("common.student.unenrollment", - tags=["org:{0}".format(org), - "course:{0}".format(course_num), - "run:{0}".format(run)]) + tags=["org:{0}".format(org), + "course:{0}".format(course_num), + "run:{0}".format(run)]) return HttpResponse() except CourseEnrollment.DoesNotExist: @@ -454,7 +453,6 @@ def login_user(request, error=""): expires_time = time.time() + max_age expires = cookie_date(expires_time) - response.set_cookie(settings.EDXMKTG_COOKIE_NAME, 'true', max_age=max_age, expires=expires, domain=settings.SESSION_COOKIE_DOMAIN, @@ -515,8 +513,8 @@ def _do_create_account(post_vars): Note: this function is also used for creating test users. """ user = User(username=post_vars['username'], - email=post_vars['email'], - is_active=False) + email=post_vars['email'], + is_active=False) user.set_password(post_vars['password']) registration = Registration() # TODO: Rearrange so that if part of the process fails, the whole process fails. @@ -698,7 +696,6 @@ def create_account(request, post_override=None): expires_time = time.time() + max_age expires = cookie_date(expires_time) - response.set_cookie(settings.EDXMKTG_COOKIE_NAME, 'true', max_age=max_age, expires=expires, domain=settings.SESSION_COOKIE_DOMAIN, @@ -708,7 +705,6 @@ def create_account(request, post_override=None): return response - def exam_registration_info(user, course): """ Returns a Registration object if the user is currently registered for a current exam of the course. Returns None if the user is not registered, or if there is no @@ -849,7 +845,6 @@ def create_exam_registration(request, post_override=None): response_data['non_field_errors'] = form.non_field_errors() return HttpResponse(json.dumps(response_data), mimetype="application/json") - # only do the following if there is accommodation text to send, # and a destination to which to send it. # TODO: still need to create the accommodation email templates @@ -872,7 +867,6 @@ def create_exam_registration(request, post_override=None): # response_data['non_field_errors'] = [ 'Could not send accommodation e-mail.', ] # return HttpResponse(json.dumps(response_data), mimetype="application/json") - js = {'success': True} return HttpResponse(json.dumps(js), mimetype="application/json") @@ -916,7 +910,7 @@ def activate_account(request, key): if not r[0].user.is_active: r[0].activate() already_active = False - + #Enroll student in any pending courses he/she may have if auto_enroll flag is set student = request.user ceas = CourseEnrollmentAllowed.objects.filter(email=student.email) @@ -924,7 +918,7 @@ def activate_account(request, key): if cea.auto_enroll: course_id = cea.course_id enrollment, created = CourseEnrollment.objects.get_or_create(user_id=student.id, course_id=course_id) - + resp = render_to_response("registration/activation_complete.html", {'user_logged_in': user_logged_in, 'already_active': already_active}) return resp if len(r) == 0: diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 1136b7cd04..fcbe1381e6 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -172,7 +172,7 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): ''' Clean user input test ''' - + string = "abc@test.com, def@test.com ghi@test.com \n \n jkl@test.com " cleaned_string, cleaned_string_lc = get_and_clean_student_list(string) - self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com']) \ No newline at end of file + self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com']) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 795865efe2..d46af2269d 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -9,7 +9,6 @@ import os import re import requests from requests.status_codes import codes -import urllib from collections import OrderedDict from StringIO import StringIO @@ -230,13 +229,13 @@ def instructor_dashboard(request, course_id): if student_to_reset is not None: # find the module in question if '/' not in problem_to_reset: # allow state of modules other than problem to be reset - problem_to_reset = "problem/" + problem_to_reset # but problem is the default + problem_to_reset = "problem/" + problem_to_reset # but problem is the default try: (org, course_name, _) = course_id.split("/") module_state_key = "i4x://" + org + "/" + course_name + "/" + problem_to_reset module_to_reset = StudentModule.objects.get(student_id=student_to_reset.id, - course_id=course_id, - module_state_key=module_state_key) + course_id=course_id, + module_state_key=module_state_key) msg += "Found module to reset. " except Exception: msg += "Couldn't find module with that urlname. " @@ -260,19 +259,18 @@ def instructor_dashboard(request, course_id): module_to_reset.state = json.dumps(problem_state) module_to_reset.save() track.views.server_track(request, - '{instructor} reset attempts from {old_attempts} to 0 for {student} on problem {problem} in {course}'.format( - old_attempts=old_number_of_attempts, - student=student_to_reset, - problem=module_to_reset.module_state_key, - instructor=request.user, - course=course_id), - {}, - page='idashboard') + '{instructor} reset attempts from {old_attempts} to 0 for {student} on problem {problem} in {course}'.format( + old_attempts=old_number_of_attempts, + student=student_to_reset, + problem=module_to_reset.module_state_key, + instructor=request.user, + course=course_id), + {}, + page='idashboard') msg += "Module state successfully reset!" except: msg += "Couldn't reset module state. " - elif "Get link to student's progress page" in action: unique_student_identifier = request.POST.get('unique_student_identifier', '') try: @@ -282,12 +280,12 @@ def instructor_dashboard(request, course_id): student_to_reset = User.objects.get(username=unique_student_identifier) progress_url = reverse('student_progress', kwargs={'course_id': course_id, 'student_id': student_to_reset.id}) track.views.server_track(request, - '{instructor} requested progress page for {student} in {course}'.format( - student=student_to_reset, - instructor=request.user, - course=course_id), - {}, - page='idashboard') + '{instructor} requested progress page for {student} in {course}'.format( + student=student_to_reset, + instructor=request.user, + course=course_id), + {}, + page='idashboard') msg += " Progress page for username: {1} with email address: {2}.".format(progress_url, student_to_reset.username, student_to_reset.email) except: msg += "Couldn't find student with that username. " @@ -315,6 +313,7 @@ def instructor_dashboard(request, course_id): msg2, rg_stud_data = _do_remote_gradebook(request.user, course, 'get-membership') datatable = {'header': ['Student email', 'Match?']} rg_students = [x['email'] for x in rg_stud_data['retdata']] + def domatch(x): return 'yes' if x.email in rg_students else 'No' datatable['data'] = [[x.email, domatch(x)] for x in stud_data['students']] @@ -350,7 +349,6 @@ def instructor_dashboard(request, course_id): msg2, _ = _do_remote_gradebook(request.user, course, 'post-grades', files=files) msg += msg2 - #---------------------------------------- # Admin @@ -416,6 +414,7 @@ def instructor_dashboard(request, course_id): profkeys = ['name', 'language', 'location', 'year_of_birth', 'gender', 'level_of_education', 'mailing_address', 'goals'] datatable = {'header': ['username', 'email'] + profkeys} + def getdat(u): p = u.profile return [u.username, u.email] + [getattr(p, x, '') for x in profkeys] @@ -424,9 +423,8 @@ def instructor_dashboard(request, course_id): datatable['title'] = 'Student profile data for course %s' % course_id return return_csv('profiledata_%s.csv' % course_id, datatable) - elif 'Download CSV of all responses to problem' in action: - problem_to_dump = request.POST.get('problem_to_dump','') + problem_to_dump = request.POST.get('problem_to_dump', '') if problem_to_dump[-4:] == ".xml": problem_to_dump = problem_to_dump[:-4] @@ -444,7 +442,7 @@ def instructor_dashboard(request, course_id): if smdat: datatable = {'header': ['username', 'state']} - datatable['data'] = [ [x.student.username, x.state] for x in smdat ] + datatable['data'] = [[x.student.username, x.state] for x in smdat] datatable['title'] = 'Student state for problem %s' % problem_to_dump return return_csv('student_state_from_%s.csv' % problem_to_dump, datatable) @@ -481,7 +479,6 @@ def instructor_dashboard(request, course_id): msg += _list_course_forum_members(course_id, rolename, datatable) track.views.server_track(request, 'list-{0}'.format(rolename), {}, page='idashboard') - elif action == 'Remove forum admin': uname = request.POST['forumadmin'] msg += _update_forum_role_membership(uname, course, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_REMOVE) @@ -571,7 +568,6 @@ def instructor_dashboard(request, course_id): ret = _do_enroll_students(course, course_id, students, overload=overload) datatable = ret['datatable'] - #---------------------------------------- # psychometrics @@ -591,9 +587,9 @@ def instructor_dashboard(request, course_id): logs and swallows errors. """ url = settings.ANALYTICS_SERVER_URL + \ - "get?aname={}&course_id={}&apikey={}".format(analytics_name, - course_id, - settings.ANALYTICS_API_KEY) + "get?aname={}&course_id={}&apikey={}".format(analytics_name, + course_id, + settings.ANALYTICS_API_KEY) try: res = requests.get(url) except Exception: @@ -652,7 +648,7 @@ def instructor_dashboard(request, course_id): 'cohorts_ajax_url': reverse('cohorts', kwargs={'course_id': course_id}), 'analytics_results': analytics_results, - } + } return render_to_response('courseware/instructor_dashboard.html', context) @@ -815,7 +811,7 @@ def _add_or_remove_user_group(request, username_or_email, group, group_title, ev action = "Added" if do_add else "Removed" prep = "to" if do_add else "from" msg = '{action} {0} {prep} {1} group = {2}'.format(user, group_title, group.name, - action=action, prep=prep) + action=action, prep=prep) if do_add: user.groups.add(group) else: @@ -941,7 +937,7 @@ def gradebook(request, course_id): 'grade_summary': student_grades(student, request, course), 'realname': student.profile.name, } - for student in enrolled_students] + for student in enrolled_students] return render_to_response('courseware/gradebook.html', { 'students': student_info, @@ -1093,6 +1089,7 @@ def get_and_clean_student_list(students): #----------------------------------------------------------------------------- # answer distribution + def get_answers_distribution(request, course_id): """ Get the distribution of answers for all graded problems in the course. @@ -1184,5 +1181,5 @@ def dump_grading_context(course): msg += " %s (format=%s, Assignment=%s%s)\n" % (s.display_name, format, aname, notes) msg += "all descriptors:\n" msg += "length=%d\n" % len(gc['all_descriptors']) - msg = '

%s
' % msg.replace('<','<') + msg = '
%s
' % msg.replace('<', '<') return msg From a8ec1ca9d5804823772a6634cbf2cbee6bb8f426 Mon Sep 17 00:00:00 2001 From: dcadams Date: Tue, 4 Jun 2013 17:08:34 -0700 Subject: [PATCH 8/9] Modified code such that non-logged in student activates correctly. --- common/djangoapps/student/views.py | 13 +++++++------ lms/djangoapps/instructor/tests/test_enrollment.py | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index fd0f643af0..661b1da26b 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -912,12 +912,13 @@ def activate_account(request, key): already_active = False #Enroll student in any pending courses he/she may have if auto_enroll flag is set - student = request.user - ceas = CourseEnrollmentAllowed.objects.filter(email=student.email) - for cea in ceas: - if cea.auto_enroll: - course_id = cea.course_id - enrollment, created = CourseEnrollment.objects.get_or_create(user_id=student.id, course_id=course_id) + student = User.objects.filter(id=r[0].user_id) + if student: + ceas = CourseEnrollmentAllowed.objects.filter(email=student[0].email) + for cea in ceas: + if cea.auto_enroll: + course_id = cea.course_id + enrollment, created = CourseEnrollment.objects.get_or_create(user_id=student[0].id, course_id=course_id) resp = render_to_response("registration/activation_complete.html", {'user_logged_in': user_logged_in, 'already_active': already_active}) return resp diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index fcbe1381e6..ce5f2d2e50 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -81,7 +81,7 @@ class TestInstructorEnrollsStudent(LoginEnrollmentTestCase): ce = CourseEnrollment.objects.filter(course_id=course.id, user=user) self.assertEqual(0, len(ce)) - def test_enrollmemt_new_student_autoenroll_on(self): + def test_enrollment_new_student_autoenroll_on(self): ''' Do auto-enroll on test ''' From 7d961bae548727d27b6fb27d5eb8c31136d21c21 Mon Sep 17 00:00:00 2001 From: dcadams Date: Thu, 6 Jun 2013 10:26:30 -0700 Subject: [PATCH 9/9] undid last commit where some coffee files were inadvertantly added. --- lms/djangoapps/instructor/views.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index d46af2269d..d8f9085f62 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -9,6 +9,7 @@ import os import re import requests from requests.status_codes import codes +import urllib from collections import OrderedDict from StringIO import StringIO @@ -1008,8 +1009,8 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll status[student] = 'already enrolled' continue try: - nce = CourseEnrollment(user=user, course_id=course_id) - nce.save() + ce = CourseEnrollment(user=user, course_id=course_id) + ce.save() status[student] = 'added' except: status[student] = 'rejected' @@ -1050,11 +1051,11 @@ def _do_unenroll_students(course_id, students): except User.DoesNotExist: continue - nce = CourseEnrollment.objects.filter(user=user, course_id=course_id) + ce = CourseEnrollment.objects.filter(user=user, course_id=course_id) #Will be 0 or 1 records as there is a unique key on user + course_id - if nce: + if ce: try: - nce[0].delete() + ce[0].delete() status[student] = "un-enrolled" except Exception as err: if not isok: