From c64f0b1a180897feae71c47ae52df237d4124427 Mon Sep 17 00:00:00 2001 From: dcadams Date: Mon, 29 Apr 2013 09:38:10 -0700 Subject: [PATCH 001/254] 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 002/254] 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 003/254] 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 004/254] 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 005/254] 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 006/254] 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 62b74008e6ad161be5cfda1daec4d476cba4ebfd Mon Sep 17 00:00:00 2001 From: JonahStanley Date: Thu, 30 May 2013 14:22:37 -0400 Subject: [PATCH 007/254] Added tests for Limited Attempt Problems and Showing the Answer --- AUTHORS | 1 + .../contentstore/features/courses.py | 6 - .../features/studio-overview-togglesection.py | 2 +- .../courseware/features/problems.feature | 55 +++ .../courseware/features/problems.py | 376 +++--------------- 5 files changed, 114 insertions(+), 326 deletions(-) diff --git a/AUTHORS b/AUTHORS index cdfdd4c2fe..67279f9053 100644 --- a/AUTHORS +++ b/AUTHORS @@ -72,3 +72,4 @@ Giulio Gratta David Baumgold Jason Bau Frances Botsford +Jonah Stanley diff --git a/cms/djangoapps/contentstore/features/courses.py b/cms/djangoapps/contentstore/features/courses.py index aa2e9d68f8..a3e838a9d1 100644 --- a/cms/djangoapps/contentstore/features/courses.py +++ b/cms/djangoapps/contentstore/features/courses.py @@ -47,12 +47,6 @@ def i_see_the_course_in_my_courses(step): assert world.css_has_text(course_css, 'Robot Super Course') -@step('the course is loaded$') -def course_is_loaded(step): - class_css = 'a.class-name' - assert world.css_has_text(course_css, 'Robot Super Cousre') - - @step('I am on the "([^"]*)" tab$') def i_am_on_tab(step, tab_name): header_css = 'div.inner-wrapper h1' diff --git a/cms/djangoapps/contentstore/features/studio-overview-togglesection.py b/cms/djangoapps/contentstore/features/studio-overview-togglesection.py index 7f717b731c..3a39f3cc15 100644 --- a/cms/djangoapps/contentstore/features/studio-overview-togglesection.py +++ b/cms/djangoapps/contentstore/features/studio-overview-togglesection.py @@ -112,7 +112,7 @@ def all_sections_are_expanded(step): @step(u'all sections are collapsed$') -def all_sections_are_expanded(step): +def all_sections_are_collapsed(step): subsection_locator = 'div.subsection-list' subsections = world.css_find(subsection_locator) for s in subsections: diff --git a/lms/djangoapps/courseware/features/problems.feature b/lms/djangoapps/courseware/features/problems.feature index 266ffa3680..2a464922ee 100644 --- a/lms/djangoapps/courseware/features/problems.feature +++ b/lms/djangoapps/courseware/features/problems.feature @@ -84,3 +84,58 @@ Feature: Answer problems | formula | incorrect | | script | correct | | script | incorrect | + + + Scenario: I can answer a problem with one attempt correctly + Given I am viewing a "multiple choice" problem with "1" attempt + Then I should see "You have used 0 of 1 submissions" somewhere in the page + And The "Final Check" button does appear + When I answer a "multiple choice" problem "correctly" + Then My "multiple choice" answer is marked "correct" + And The "multiple choice" problem displays a "correct" answer + And The "Reset" button does not appear + + Scenario: I can answer a problem with one attempt incorrectly + Given I am viewing a "multiple choice" problem with "1" attempt + When I answer a "multiple choice" problem "incorrectly" + Then My "multiple choice" answer is marked "incorrect" + And The "multiple choice" problem displays a "incorrect" answer + And The "Reset" button does not appear + + Scenario: I can answer a problem with multiple attempts correctly + Given I am viewing a "multiple choice" problem with "3" attempts + Then I should see "You have used 0 of 3 submissions" somewhere in the page + When I answer a "multiple choice" problem "correctly" + Then My "multiple choice" answer is marked "correct" + And The "multiple choice" problem displays a "correct" answer + And The "Reset" button does appear + + Scenario: I can answer a problem with multiple attempts correctly on final guess + Given I am viewing a "multiple choice" problem with "3" attempts + Then I should see "You have used 0 of 3 submissions" somewhere in the page + When I answer a "multiple choice" problem "incorrectly" + Then My "multiple choice" answer is marked "incorrect" + And The "multiple choice" problem displays a "incorrect" answer + When I reset the problem + Then I should see "You have used 1 of 3 submissions" somewhere in the page + When I answer a "multiple choice" problem "incorrectly" + Then My "multiple choice" answer is marked "incorrect" + And The "multiple choice" problem displays a "incorrect" answer + When I reset the problem + Then I should see "You have used 2 of 3 submissions" somewhere in the page + And The "Final Check" button does appear + When I answer a "multiple choice" problem "correctly" + Then My "multiple choice" answer is marked "correct" + And The "multiple choice" problem displays a "correct" answer + And The "Reset" button does not appear + + Scenario: I can view and hide the answer if the problem has it: + Given I am viewing a "numerical" that shows the answer "always" + Then The "Show Answer" button does appear + When I press the "Show Answer" button + Then The "Hide Answer" button does appear + And The "Show Answer" button does not appear + And I should see "4.14159" somewhere in the page + When I press the "Hide Answer" button + Then The "Show Answer" button does appear + And I do not see "4.14159" anywhere on the page diff --git a/lms/djangoapps/courseware/features/problems.py b/lms/djangoapps/courseware/features/problems.py index 3d538d7ae1..af301493ae 100644 --- a/lms/djangoapps/courseware/features/problems.py +++ b/lms/djangoapps/courseware/features/problems.py @@ -7,119 +7,42 @@ Steps for problem.feature lettuce tests from lettuce import world, step from lettuce.django import django_url -import random -import textwrap -from common import i_am_registered_for_the_course, \ - TEST_SECTION_NAME, section_location -from capa.tests.response_xml_factory import OptionResponseXMLFactory, \ - ChoiceResponseXMLFactory, MultipleChoiceResponseXMLFactory, \ - StringResponseXMLFactory, NumericalResponseXMLFactory, \ - FormulaResponseXMLFactory, CustomResponseXMLFactory, \ - CodeResponseXMLFactory - -# Factories from capa.tests.response_xml_factory that we will use -# to generate the problem XML, with the keyword args used to configure -# the output. -PROBLEM_FACTORY_DICT = { - 'drop down': { - 'factory': OptionResponseXMLFactory(), - 'kwargs': { - 'question_text': 'The correct answer is Option 2', - 'options': ['Option 1', 'Option 2', 'Option 3', 'Option 4'], - 'correct_option': 'Option 2'}}, - - 'multiple choice': { - 'factory': MultipleChoiceResponseXMLFactory(), - 'kwargs': { - 'question_text': 'The correct answer is Choice 3', - 'choices': [False, False, True, False], - 'choice_names': ['choice_0', 'choice_1', 'choice_2', 'choice_3']}}, - - 'checkbox': { - 'factory': ChoiceResponseXMLFactory(), - 'kwargs': { - 'question_text': 'The correct answer is Choices 1 and 3', - 'choice_type': 'checkbox', - 'choices': [True, False, True, False, False], - 'choice_names': ['Choice 1', 'Choice 2', 'Choice 3', 'Choice 4']}}, - 'radio': { - 'factory': ChoiceResponseXMLFactory(), - 'kwargs': { - 'question_text': 'The correct answer is Choice 3', - 'choice_type': 'radio', - 'choices': [False, False, True, False], - 'choice_names': ['Choice 1', 'Choice 2', 'Choice 3', 'Choice 4']}}, - 'string': { - 'factory': StringResponseXMLFactory(), - 'kwargs': { - 'question_text': 'The answer is "correct string"', - 'case_sensitive': False, - 'answer': 'correct string'}}, - - 'numerical': { - 'factory': NumericalResponseXMLFactory(), - 'kwargs': { - 'question_text': 'The answer is pi + 1', - 'answer': '4.14159', - 'tolerance': '0.00001', - 'math_display': True}}, - - 'formula': { - 'factory': FormulaResponseXMLFactory(), - 'kwargs': { - 'question_text': 'The solution is [mathjax]x^2+2x+y[/mathjax]', - 'sample_dict': {'x': (-100, 100), 'y': (-100, 100)}, - 'num_samples': 10, - 'tolerance': 0.00001, - 'math_display': True, - 'answer': 'x^2+2*x+y'}}, - - 'script': { - 'factory': CustomResponseXMLFactory(), - 'kwargs': { - 'question_text': 'Enter two integers that sum to 10.', - 'cfn': 'test_add_to_ten', - 'expect': '10', - 'num_inputs': 2, - 'script': textwrap.dedent(""" - def test_add_to_ten(expect,ans): - try: - a1=int(ans[0]) - a2=int(ans[1]) - except ValueError: - a1=0 - a2=0 - return (a1+a2)==int(expect) - """)}}, - 'code': { - 'factory': CodeResponseXMLFactory(), - 'kwargs': { - 'question_text': 'Submit code to an external grader', - 'initial_display': 'print "Hello world!"', - 'grader_payload': '{"grader": "ps1/Spring2013/test_grader.py"}', }}, - } +from common import i_am_registered_for_the_course, TEST_SECTION_NAME +from problems_setup import * -def add_problem_to_course(course, problem_type): - ''' - Add a problem to the course we have created using factories. - ''' +@step(u'I am viewing a "([^"]*)" problem with "([^"]*)" attempt') +def view_problem_with_attempts(step, problem_type, attempts): + i_am_registered_for_the_course(step, 'model_course') - assert(problem_type in PROBLEM_FACTORY_DICT) + # Ensure that the course has this problem type + add_problem_to_course('model_course', problem_type, {'attempts': attempts}) - # Generate the problem XML using capa.tests.response_xml_factory - factory_dict = PROBLEM_FACTORY_DICT[problem_type] - problem_xml = factory_dict['factory'].build_xml(**factory_dict['kwargs']) + # Go to the one section in the factory-created course + # which should be loaded with the correct problem + chapter_name = TEST_SECTION_NAME.replace(" ", "_") + section_name = chapter_name + url = django_url('/courses/edx/model_course/Test_Course/courseware/%s/%s' % + (chapter_name, section_name)) - # Create a problem item using our generated XML - # We set rerandomize=always in the metadata so that the "Reset" button - # will appear. - template_name = "i4x://edx/templates/problem/Blank_Common_Problem" - world.ItemFactory.create(parent_location=section_location(course), - template=template_name, - display_name=str(problem_type), - data=problem_xml, - metadata={'rerandomize': 'always'}) + world.browser.visit(url) + + +@step(u'I am viewing a "([^"]*)" that shows the answer "([^"]*)"') +def view_problem_with_show_answer(step, problem_type, answer): + i_am_registered_for_the_course(step, 'model_course') + + # Ensure that the course has this problem type + add_problem_to_course('model_course', problem_type, {'show_answer': answer}) + + # Go to the one section in the factory-created course + # which should be loaded with the correct problem + chapter_name = TEST_SECTION_NAME.replace(" ", "_") + section_name = chapter_name + url = django_url('/courses/edx/model_course/Test_Course/courseware/%s/%s' % + (chapter_name, section_name)) + + world.browser.visit(url) @step(u'I am viewing a "([^"]*)" problem') @@ -153,7 +76,7 @@ def set_external_grader_response(step, correctness): @step(u'I answer a "([^"]*)" problem "([^"]*)ly"') -def answer_problem(step, problem_type, correctness): +def answer_problem_step(step, problem_type, correctness): """ Mark a given problem type correct or incorrect, then submit it. *problem_type* is a string representing the type of problem (e.g. 'drop down') @@ -161,73 +84,18 @@ def answer_problem(step, problem_type, correctness): """ assert(correctness in ['correct', 'incorrect']) - - if problem_type == "drop down": - select_name = "input_i4x-edx-model_course-problem-drop_down_2_1" - option_text = 'Option 2' if correctness == 'correct' else 'Option 3' - world.browser.select(select_name, option_text) - - elif problem_type == "multiple choice": - if correctness == 'correct': - inputfield('multiple choice', choice='choice_2').check() - else: - inputfield('multiple choice', choice='choice_1').check() - - elif problem_type == "checkbox": - if correctness == 'correct': - inputfield('checkbox', choice='choice_0').check() - inputfield('checkbox', choice='choice_2').check() - else: - inputfield('checkbox', choice='choice_3').check() - - elif problem_type == 'radio': - if correctness == 'correct': - inputfield('radio', choice='choice_2').check() - else: - inputfield('radio', choice='choice_1').check() - - elif problem_type == 'string': - textvalue = 'correct string' if correctness == 'correct' \ - else 'incorrect' - inputfield('string').fill(textvalue) - - elif problem_type == 'numerical': - textvalue = "pi + 1" if correctness == 'correct' \ - else str(random.randint(-2, 2)) - inputfield('numerical').fill(textvalue) - - elif problem_type == 'formula': - textvalue = "x^2+2*x+y" if correctness == 'correct' else 'x^2' - inputfield('formula').fill(textvalue) - - elif problem_type == 'script': - # Correct answer is any two integers that sum to 10 - first_addend = random.randint(-100, 100) - second_addend = 10 - first_addend - - # If we want an incorrect answer, then change - # the second addend so they no longer sum to 10 - if correctness == 'incorrect': - second_addend += random.randint(1, 10) - - inputfield('script', input_num=1).fill(str(first_addend)) - inputfield('script', input_num=2).fill(str(second_addend)) - - elif problem_type == 'code': - # The fake xqueue server is configured to respond - # correct / incorrect no matter what we submit. - # Furthermore, since the inline code response uses - # JavaScript to make the code display nicely, it's difficult - # to programatically input text - # (there's not @@ -221,7 +221,14 @@

- + <% + ## TODO: provide a better way to override these links + if self.stanford_theme_enabled(): + honor_code_path = marketing_link('TOS') + "#honor" + else: + honor_code_path = marketing_link('HONOR') + %> +
@@ -252,23 +259,33 @@

-
-

Welcome to edX

-

Registering with edX gives you access to all of our current and future free courses. Not ready to take a course just yet? Registering puts you on our mailing list – we will update you as courses are added.

-
+ ## TODO: Use a %block tag or something to allow themes to + ## override in a more generalizable fashion. + % if not self.stanford_theme_enabled(): +
+

Welcome to ${settings.PLATFORM_NAME}

+

Registering with ${settings.PLATFORM_NAME} gives you access to all of our current and future free courses. Not ready to take a course just yet? Registering puts you on our mailing list – we will update you as courses are added.

+
+ % endif

Next Steps

-

As part of joining edX, you will receive an activation email. You must click on the activation link to complete the process. Don’t see the email? Check your spam folder and mark edX emails as ‘not spam’. At edX, we communicate mostly through email.

+ % if self.stanford_theme_enabled(): +

You will receive an activation email. You must click on the activation link to complete the process. Don’t see the email? Check your spam folder and mark emails from class.stanford.edu as ‘not spam’, since you'll want to be able to receive email from your courses.

+ % else: +

As part of joining ${settings.PLATFORM_NAME}, you will receive an activation email. You must click on the activation link to complete the process. Don’t see the email? Check your spam folder and mark ${settings.PLATFORM_NAME} emails as ‘not spam’. At ${settings.PLATFORM_NAME}, we communicate mostly through email.

+ % endif
-
-

Need Help?

-

Need help in registering with edX? - - View our FAQs for answers to commonly asked questions. - - Once registered, most questions can be answered in the course specific discussion forums or through the FAQs.

-
+ % if settings.MKTG_URL_LINK_MAP.get('FAQ', None) is not None: +
+

Need Help?

+

Need help in registering with ${settings.PLATFORM_NAME}? + + View our FAQs for answers to commonly asked questions. + + Once registered, most questions can be answered in the course specific discussion forums or through the FAQs.

+
+ % endif From 90ebb4c836742db3881b991d41985be938197658 Mon Sep 17 00:00:00 2001 From: Nate Hardison Date: Fri, 31 May 2013 19:06:06 -0700 Subject: [PATCH 092/254] Theme the login page As with the registration page, the bulk of the theming work here is replacing instances of "edX" with the `PLATFORM_NAME` setting. There is also a change to the "help" section, disabling it if the FAQ marketing link isn't set. --- lms/templates/login.html | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lms/templates/login.html b/lms/templates/login.html index 91210e7bb7..e2558651f6 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -5,7 +5,7 @@ <%! from django.core.urlresolvers import reverse %> <%! from django.utils.translation import ugettext as _ %> -<%block name="title">Log into your edX Account +<%block name="title">Log into your ${settings.PLATFORM_NAME} Account <%block name="js_extra"> + +% else: + + + +% endif From 6526146b954f08897424fff3fc5f3073d60168c2 Mon Sep 17 00:00:00 2001 From: Frances Botsford Date: Wed, 5 Jun 2013 15:08:38 -0400 Subject: [PATCH 145/254] updated misused sass body-bg to container-bg variableon account and course views --- lms/static/sass/course/base/_base.scss | 4 ++-- lms/static/sass/multicourse/_account.scss | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/static/sass/course/base/_base.scss b/lms/static/sass/course/base/_base.scss index 584412ca22..6d87b7f554 100644 --- a/lms/static/sass/course/base/_base.scss +++ b/lms/static/sass/course/base/_base.scss @@ -35,7 +35,7 @@ a { width: 100%; border-radius: 3px; border: 1px solid $outer-border-color; - background: $body-bg; + background: $container-bg; @include box-shadow(0 1px 2px rgba(0, 0, 0, 0.05)); } } @@ -50,7 +50,7 @@ textarea, input[type="text"], input[type="email"], input[type="password"] { - background: $body-bg; + background: $white; border: 1px solid $border-color-2; @include border-radius(0); @include box-shadow(0 1px 0 0 rgba(255,255,255, 0.6), inset 0 0 3px 0 rgba(0,0,0, 0.1)); diff --git a/lms/static/sass/multicourse/_account.scss b/lms/static/sass/multicourse/_account.scss index dfd803a22d..e531754aac 100644 --- a/lms/static/sass/multicourse/_account.scss +++ b/lms/static/sass/multicourse/_account.scss @@ -6,7 +6,7 @@ // page-level .view-register, .view-login, .view-passwordreset { - background: $body-bg; + background: $container-bg; @@ -331,7 +331,7 @@ } textarea, input { - background: $body-bg; + background: $container-bg; color: rgba(0,0,0,.25); } } From 86a7a255c316fa81c951201b1c394187bd159898 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Thu, 6 Jun 2013 10:35:44 -0400 Subject: [PATCH 146/254] resolves background color syncing for particular page/container wrappers --- lms/static/sass/base/_variables.scss | 3 ++- lms/static/sass/multicourse/_account.scss | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/static/sass/base/_variables.scss b/lms/static/sass/base/_variables.scss index 5c8c8d18d9..2bbb34a03f 100644 --- a/lms/static/sass/base/_variables.scss +++ b/lms/static/sass/base/_variables.scss @@ -88,9 +88,10 @@ $dashboard-profile-header-color: transparent; $dashboard-profile-color: rgb(252,252,252); $dot-color: $light-gray; -$content-wrapper-bg: shade($body-bg, 2%); +$content-wrapper-bg: $white; $course-bg-color: #d6d6d6; $course-bg-image: url(../images/bg-texture.png); +$account-content-wrapper-bg: shade($body-bg, 2%); $course-profile-bg: rgb(245,245,245); $course-header-bg: rgba(255,255,255, 0.93); diff --git a/lms/static/sass/multicourse/_account.scss b/lms/static/sass/multicourse/_account.scss index e531754aac..7528368986 100644 --- a/lms/static/sass/multicourse/_account.scss +++ b/lms/static/sass/multicourse/_account.scss @@ -98,7 +98,7 @@ // layout .content-wrapper { - background: $content-wrapper-bg; + background: $account-content-wrapper-bg; padding-bottom: 0; } From 9220a4f45940484a31edf61fb599f75d062ac9e4 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 6 Jun 2013 10:29:10 -0400 Subject: [PATCH 147/254] Bullet-proofing for MKTG_URLS. --- .../contentstore/tests/test_utils.py | 6 ++++++ cms/djangoapps/contentstore/utils.py | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index c4b0f4bb51..0c33c2dcca 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -24,6 +24,12 @@ class LMSLinksTestCase(TestCase): with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': False}): self.assertEquals(self.get_about_page_link(), "//localhost:8000/courses/mitX/101/test/about") + @override_settings(MKTG_URLS={}) + def about_page_marketing_urls_not_set_test(self): + """ Error case. ENABLE_MKTG_SITE is True, but there is either no MKTG_URLS, or no MKTG_URLS Root property. """ + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': True}): + self.assertEquals(self.get_about_page_link(), None) + @override_settings(LMS_BASE=None) def about_page_no_lms_base_test(self): """ No LMS_BASE, nor is ENABLE_MKTG_SITE True """ diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 703e9a266a..83c413ac72 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -4,6 +4,9 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from django.core.urlresolvers import reverse import copy +import logging + +log = logging.getLogger(__name__) DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info'] @@ -108,9 +111,18 @@ def get_lms_link_for_about_page(location): Returns the url to the course about page from the location tuple. """ if settings.MITX_FEATURES.get('ENABLE_MKTG_SITE', False): - # Root will be "www.edx.org". The complete URL will still not be exactly correct, - # but redirects exist from www.edx.org to get to the drupal course about page URL. - about_base = settings.MKTG_URLS.get('ROOT') + if not hasattr(settings, 'MKTG_URLS'): + log.exception("ENABLE_MKTG_SITE is True, but MKTG_URLS is not defined.") + about_base = None + else: + marketing_urls = settings.MKTG_URLS + if marketing_urls.get('ROOT', None) is None: + log.exception('There is no ROOT defined in MKTG_URLS') + about_base = None + else: + # Root will be "www.edx.org". The complete URL will still not be exactly correct, + # but redirects exist from www.edx.org to get to the drupal course about page URL. + about_base = marketing_urls.get('ROOT') elif settings.LMS_BASE is not None: about_base = settings.LMS_BASE else: From e5efdde76a7c7eda45cd64c1cd3ed45e38774314 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 6 Jun 2013 10:55:41 -0400 Subject: [PATCH 148/254] Strip off https:// --- .../contentstore/tests/test_utils.py | 18 ++++++++++++++++++ cms/djangoapps/contentstore/utils.py | 7 +++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 0c33c2dcca..fec82db1bb 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -24,6 +24,24 @@ class LMSLinksTestCase(TestCase): with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': False}): self.assertEquals(self.get_about_page_link(), "//localhost:8000/courses/mitX/101/test/about") + @override_settings(MKTG_URLS={'ROOT': 'http://www.dummy'}) + def about_page_marketing_site_remove_http_test(self): + """ Get URL for about page, marketing root present, remove http://. """ + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': True}): + self.assertEquals(self.get_about_page_link(), "//www.dummy/courses/mitX/101/test/about") + + @override_settings(MKTG_URLS={'ROOT': 'https://www.dummy'}) + def about_page_marketing_site_remove_https_test(self): + """ Get URL for about page, marketing root present, remove https://. """ + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': True}): + self.assertEquals(self.get_about_page_link(), "//www.dummy/courses/mitX/101/test/about") + + @override_settings(MKTG_URLS={'ROOT': 'www.dummyhttps://x'}) + def about_page_marketing_site_https__edge_test(self): + """ Get URL for about page, only remove https:// at the beginning of the string. """ + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': True}): + self.assertEquals(self.get_about_page_link(), "//www.dummyhttps://x/courses/mitX/101/test/about") + @override_settings(MKTG_URLS={}) def about_page_marketing_urls_not_set_test(self): """ Error case. ENABLE_MKTG_SITE is True, but there is either no MKTG_URLS, or no MKTG_URLS Root property. """ diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 83c413ac72..e9b62331ef 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -5,6 +5,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from django.core.urlresolvers import reverse import copy import logging +import re log = logging.getLogger(__name__) @@ -120,9 +121,11 @@ def get_lms_link_for_about_page(location): log.exception('There is no ROOT defined in MKTG_URLS') about_base = None else: - # Root will be "www.edx.org". The complete URL will still not be exactly correct, - # but redirects exist from www.edx.org to get to the drupal course about page URL. + # Root will be "https://www.edx.org". The complete URL will still not be exactly correct, + # but redirects exist from www.edx.org to get to the Drupal course about page URL. about_base = marketing_urls.get('ROOT') + # Strip off https:// (or http://) to be consistent with the formatting of LMS_BASE. + about_base = re.sub(r"^https?://", "", about_base) elif settings.LMS_BASE is not None: about_base = settings.LMS_BASE else: From 7d961bae548727d27b6fb27d5eb8c31136d21c21 Mon Sep 17 00:00:00 2001 From: dcadams Date: Thu, 6 Jun 2013 10:26:30 -0700 Subject: [PATCH 149/254] 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: From 9a631fe47654e6c220d02fa7ac9b7dcdace9c48b Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 29 May 2013 17:36:40 -0400 Subject: [PATCH 150/254] All uses of safe_exec need to get the correct random seed. --- common/lib/capa/capa/responsetypes.py | 32 +++++++- .../lib/capa/capa/tests/test_responsetypes.py | 77 ++++++++++++++++--- 2 files changed, 96 insertions(+), 13 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 0fa50079de..a13ed3ca11 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -288,7 +288,13 @@ class LoncapaResponse(object): } try: - safe_exec.safe_exec(code, globals_dict, python_path=self.context['python_path'], slug=self.id) + safe_exec.safe_exec( + code, + globals_dict, + python_path=self.context['python_path'], + slug=self.id, + random_seed=self.context['seed'], + ) except Exception as err: msg = 'Error %s in evaluating hint function %s' % (err, hintfn) msg += "\nSee XML source line %s" % getattr( @@ -973,7 +979,13 @@ class CustomResponse(LoncapaResponse): 'ans': ans, } globals_dict.update(kwargs) - safe_exec.safe_exec(code, globals_dict, python_path=self.context['python_path'], slug=self.id) + safe_exec.safe_exec( + code, + globals_dict, + python_path=self.context['python_path'], + slug=self.id, + random_seed=self.context['seed'], + ) return globals_dict['cfn_return'] return check_function @@ -1090,7 +1102,13 @@ class CustomResponse(LoncapaResponse): # exec the check function if isinstance(self.code, basestring): try: - safe_exec.safe_exec(self.code, self.context, cache=self.system.cache, slug=self.id) + safe_exec.safe_exec( + self.code, + self.context, + cache=self.system.cache, + slug=self.id, + random_seed=self.context['seed'], + ) except Exception as err: self._handle_exec_exception(err) @@ -1814,7 +1832,13 @@ class SchematicResponse(LoncapaResponse): ] self.context.update({'submission': submission}) try: - safe_exec.safe_exec(self.code, self.context, cache=self.system.cache, slug=self.id) + safe_exec.safe_exec( + self.code, + self.context, + cache=self.system.cache, + slug=self.id, + random_seed=self.context['seed'], + ) except Exception as err: msg = 'Error %s in evaluating SchematicResponse' % err raise ResponseError(msg) diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 780c475b09..20de19f567 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -640,6 +640,23 @@ class StringResponseTest(ResponseTest): correct_map = problem.grade_answers(input_dict) self.assertEquals(correct_map.get_hint('1_2_1'), "Hello??") + def test_hint_function_randomization(self): + # The hint function should get the seed from the problem. + problem = self.build_problem( + answer="1", + hintfn="gimme_a_random_hint", + script=textwrap.dedent(""" + def gimme_a_random_hint(answer_ids, student_answers, new_cmap, old_cmap): + answer = str(random.randint(0, 1e9)) + new_cmap.set_hint_and_mode(answer_ids[0], answer, "always") + + """) + ) + correct_map = problem.grade_answers({'1_2_1': '2'}) + hint = correct_map.get_hint('1_2_1') + r = random.Random(problem.seed) + self.assertEqual(hint, str(r.randint(0, 1e9))) + class CodeResponseTest(ResponseTest): from response_xml_factory import CodeResponseXMLFactory @@ -948,7 +965,6 @@ class CustomResponseTest(ResponseTest): xml_factory_class = CustomResponseXMLFactory def test_inline_code(self): - # For inline code, we directly modify global context variables # 'answers' is a list of answers provided to us # 'correct' is a list we fill in with True/False @@ -961,15 +977,14 @@ class CustomResponseTest(ResponseTest): self.assert_grade(problem, '0', 'incorrect') def test_inline_message(self): - # Inline code can update the global messages list # to pass messages to the CorrectMap for a particular input # The code can also set the global overall_message (str) # to pass a message that applies to the whole response inline_script = textwrap.dedent(""" - messages[0] = "Test Message" - overall_message = "Overall message" - """) + messages[0] = "Test Message" + overall_message = "Overall message" + """) problem = self.build_problem(answer=inline_script) input_dict = {'1_2_1': '0'} @@ -983,8 +998,19 @@ class CustomResponseTest(ResponseTest): overall_msg = correctmap.get_overall_message() self.assertEqual(overall_msg, "Overall message") - def test_function_code_single_input(self): + def test_inline_randomization(self): + # Make sure the seed from the problem gets fed into the script execution. + inline_script = """messages[0] = str(random.randint(0, 1e9))""" + problem = self.build_problem(answer=inline_script) + input_dict = {'1_2_1': '0'} + correctmap = problem.grade_answers(input_dict) + + input_msg = correctmap.get_msg('1_2_1') + r = random.Random(problem.seed) + self.assertEqual(input_msg, str(r.randint(0, 1e9))) + + def test_function_code_single_input(self): # For function code, we pass in these arguments: # # 'expect' is the expect attribute of the @@ -1212,6 +1238,29 @@ class CustomResponseTest(ResponseTest): with self.assertRaises(ResponseError): problem.grade_answers({'1_2_1': '42'}) + def test_setup_randomization(self): + # Ensure that the problem setup script gets the random seed from the problem. + script = textwrap.dedent(""" + num = random.randint(0, 1e9) + """) + problem = self.build_problem(script=script) + r = random.Random(problem.seed) + self.assertEqual(r.randint(0, 1e9), problem.context['num']) + + def test_check_function_randomization(self): + # The check function should get random-seeded from the problem. + script = textwrap.dedent(""" + def check_func(expect, answer_given): + return {'ok': True, 'msg': str(random.randint(0, 1e9))} + """) + + problem = self.build_problem(script=script, cfn="check_func", expect="42") + input_dict = {'1_2_1': '42'} + correct_map = problem.grade_answers(input_dict) + msg = correct_map.get_msg('1_2_1') + r = random.Random(problem.seed) + self.assertEqual(msg, str(r.randint(0, 1e9))) + def test_module_imports_inline(self): ''' Check that the correct modules are available to custom @@ -1275,7 +1324,6 @@ class SchematicResponseTest(ResponseTest): xml_factory_class = SchematicResponseXMLFactory def test_grade(self): - # Most of the schematic-specific work is handled elsewhere # (in client-side JavaScript) # The is responsible only for executing the @@ -1290,7 +1338,7 @@ class SchematicResponseTest(ResponseTest): # The actual dictionary would contain schematic information # sent from the JavaScript simulation - submission_dict = {'test': 'test'} + submission_dict = {'test': 'the_answer'} input_dict = {'1_2_1': json.dumps(submission_dict)} correct_map = problem.grade_answers(input_dict) @@ -1299,8 +1347,19 @@ class SchematicResponseTest(ResponseTest): # is what we expect) self.assertEqual(correct_map.get_correctness('1_2_1'), 'correct') - def test_script_exception(self): + def test_check_function_randomization(self): + # The check function should get a random seed from the problem. + script = "correct = ['correct' if (submission[0]['num'] == random.randint(0, 1e9)) else 'incorrect']" + problem = self.build_problem(answer=script) + r = random.Random(problem.seed) + submission_dict = {'num': r.randint(0, 1e9)} + input_dict = {'1_2_1': json.dumps(submission_dict)} + correct_map = problem.grade_answers(input_dict) + + self.assertEqual(correct_map.get_correctness('1_2_1'), 'correct') + + def test_script_exception(self): # Construct a script that will raise an exception script = "raise Exception('test')" problem = self.build_problem(answer=script) From cab49716b56bd1d23e57ffb98805b60cdbfe65f3 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Thu, 6 Jun 2013 14:14:30 -0400 Subject: [PATCH 151/254] Whitelisted courses now run Python code outside the sandbox. --- common/lib/capa/capa/capa_problem.py | 1 + common/lib/capa/capa/responsetypes.py | 4 ++++ common/lib/capa/capa/safe_exec/safe_exec.py | 13 +++++++++-- .../capa/safe_exec/tests/test_safe_exec.py | 22 +++++++++++++++++++ requirements/edx/github.txt | 2 +- 5 files changed, 39 insertions(+), 3 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 150b3b3c9b..7dcd7b925e 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -470,6 +470,7 @@ class LoncapaProblem(object): python_path=python_path, cache=self.system.cache, slug=self.problem_id, + unsafely=self.system.can_execute_unsafe_code(), ) except Exception as err: log.exception("Error while execing script code: " + all_code) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index a13ed3ca11..6183ca2ade 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -294,6 +294,7 @@ class LoncapaResponse(object): python_path=self.context['python_path'], slug=self.id, random_seed=self.context['seed'], + unsafely=self.system.can_execute_unsafe_code(), ) except Exception as err: msg = 'Error %s in evaluating hint function %s' % (err, hintfn) @@ -985,6 +986,7 @@ class CustomResponse(LoncapaResponse): python_path=self.context['python_path'], slug=self.id, random_seed=self.context['seed'], + unsafely=self.system.can_execute_unsafe_code(), ) return globals_dict['cfn_return'] return check_function @@ -1108,6 +1110,7 @@ class CustomResponse(LoncapaResponse): cache=self.system.cache, slug=self.id, random_seed=self.context['seed'], + unsafely=self.system.can_execute_unsafe_code(), ) except Exception as err: self._handle_exec_exception(err) @@ -1838,6 +1841,7 @@ class SchematicResponse(LoncapaResponse): cache=self.system.cache, slug=self.id, random_seed=self.context['seed'], + unsafely=self.system.can_execute_unsafe_code(), ) except Exception as err: msg = 'Error %s in evaluating SchematicResponse' % err diff --git a/common/lib/capa/capa/safe_exec/safe_exec.py b/common/lib/capa/capa/safe_exec/safe_exec.py index 67e93be46f..3ab8f0bf9e 100644 --- a/common/lib/capa/capa/safe_exec/safe_exec.py +++ b/common/lib/capa/capa/safe_exec/safe_exec.py @@ -1,6 +1,7 @@ """Capa's specialized use of codejail.safe_exec.""" from codejail.safe_exec import safe_exec as codejail_safe_exec +from codejail.safe_exec import not_safe_exec as codejail_not_safe_exec from codejail.safe_exec import json_safe, SafeExecException from . import lazymod from statsd import statsd @@ -71,7 +72,7 @@ def update_hash(hasher, obj): @statsd.timed('capa.safe_exec.time') -def safe_exec(code, globals_dict, random_seed=None, python_path=None, cache=None, slug=None): +def safe_exec(code, globals_dict, random_seed=None, python_path=None, cache=None, slug=None, unsafely=False): """ Execute python code safely. @@ -90,6 +91,8 @@ def safe_exec(code, globals_dict, random_seed=None, python_path=None, cache=None `slug` is an arbitrary string, a description that's meaningful to the caller, that will be used in log messages. + If `unsafely` is true, then the code will actually be executed without sandboxing. + """ # Check the cache for a previous result. if cache: @@ -111,9 +114,15 @@ def safe_exec(code, globals_dict, random_seed=None, python_path=None, cache=None # Create the complete code we'll run. code_prolog = CODE_PROLOG % random_seed + # Decide which code executor to use. + if unsafely: + exec_fn = codejail_not_safe_exec + else: + exec_fn = codejail_safe_exec + # Run the code! Results are side effects in globals_dict. try: - codejail_safe_exec( + exec_fn( code_prolog + LAZY_IMPORTS + code, globals_dict, python_path=python_path, slug=slug, ) diff --git a/common/lib/capa/capa/safe_exec/tests/test_safe_exec.py b/common/lib/capa/capa/safe_exec/tests/test_safe_exec.py index 4592af8305..f8a8a32297 100644 --- a/common/lib/capa/capa/safe_exec/tests/test_safe_exec.py +++ b/common/lib/capa/capa/safe_exec/tests/test_safe_exec.py @@ -1,13 +1,17 @@ """Test safe_exec.py""" import hashlib +import os import os.path import random import textwrap import unittest +from nose.plugins.skip import SkipTest + from capa.safe_exec import safe_exec, update_hash from codejail.safe_exec import SafeExecException +from codejail.jail_code import is_configured class TestSafeExec(unittest.TestCase): @@ -68,6 +72,24 @@ class TestSafeExec(unittest.TestCase): self.assertIn("ZeroDivisionError", cm.exception.message) +class TestSafeOrNot(unittest.TestCase): + def test_cant_do_something_forbidden(self): + # Can't test for forbiddenness if CodeJail isn't configured for python. + if not is_configured("python"): + raise SkipTest + + g = {} + with self.assertRaises(SafeExecException) as cm: + safe_exec("import os; files = os.listdir('/')", g) + self.assertIn("OSError", cm.exception.message) + self.assertIn("Permission denied", cm.exception.message) + + def test_can_do_something_forbidden_if_run_unsafely(self): + g = {} + safe_exec("import os; files = os.listdir('/')", g, unsafely=True) + self.assertEqual(g['files'], os.listdir('/')) + + class DictCache(object): """A cache implementation over a simple dict, for testing.""" diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index fc9070bba3..8b5ab8df48 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -9,5 +9,5 @@ # Our libraries: -e git+https://github.com/edx/XBlock.git@2144a25d#egg=XBlock --e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail +-e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover From d3965cb3ee30499bef12435cedb8dc625788ef34 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Thu, 6 Jun 2013 11:27:09 -0400 Subject: [PATCH 152/254] removes redundant h2 heading and syncs up h1 value (with broken mak/template syntax) for register view --- lms/templates/login.html | 4 ---- lms/templates/register.html | 6 +----- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/lms/templates/login.html b/lms/templates/login.html index b185b02c84..d3c3384847 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -86,10 +86,6 @@