From 9dd182800b46f781ef56a0a15ca620021f0f385d Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Fri, 8 Nov 2013 11:56:12 -0500 Subject: [PATCH 01/87] Set empty aws credentials to None. --- cms/envs/aws.py | 7 ++++++- lms/envs/aws.py | 6 ++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 2e53a555a5cc..9cdfd79b343e 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -156,9 +156,14 @@ if SEGMENT_IO_KEY: MITX_FEATURES['SEGMENT_IO'] = ENV_TOKENS.get('SEGMENT_IO', False) - AWS_ACCESS_KEY_ID = AUTH_TOKENS["AWS_ACCESS_KEY_ID"] +if AWS_ACCESS_KEY_ID == "": + AWS_ACCESS_KEY_ID = None + AWS_SECRET_ACCESS_KEY = AUTH_TOKENS["AWS_SECRET_ACCESS_KEY"] +if AWS_SECRET_ACCESS_KEY == "": + AWS_SECRET_ACCESS_KEY = None + DATABASES = AUTH_TOKENS['DATABASES'] MODULESTORE = AUTH_TOKENS['MODULESTORE'] CONTENTSTORE = AUTH_TOKENS['CONTENTSTORE'] diff --git a/lms/envs/aws.py b/lms/envs/aws.py index d524474d5b33..0262c9a5d491 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -243,7 +243,13 @@ SECRET_KEY = AUTH_TOKENS['SECRET_KEY'] AWS_ACCESS_KEY_ID = AUTH_TOKENS["AWS_ACCESS_KEY_ID"] +if AWS_ACCESS_KEY_ID == "": + AWS_ACCESS_KEY_ID = None + AWS_SECRET_ACCESS_KEY = AUTH_TOKENS["AWS_SECRET_ACCESS_KEY"] +if AWS_SECRET_ACCESS_KEY == "": + AWS_SECRET_ACCESS_KEY = None + AWS_STORAGE_BUCKET_NAME = AUTH_TOKENS.get('AWS_STORAGE_BUCKET_NAME', 'edxuploads') DATABASES = AUTH_TOKENS['DATABASES'] From 2a31e3567e623feffd74cb861d40c792d5909344 Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Wed, 6 Nov 2013 17:07:02 -0500 Subject: [PATCH 02/87] sending template pdf with certificate request --- .../management/commands/ungenerated_certs.py | 1 + .../0015_adding_mode_for_verified_certs.py | 86 +++++++++++++++++++ lms/djangoapps/certificates/models.py | 5 ++ lms/djangoapps/certificates/queue.py | 73 +++++++++------- 4 files changed, 135 insertions(+), 30 deletions(-) create mode 100644 lms/djangoapps/certificates/migrations/0015_adding_mode_for_verified_certs.py diff --git a/lms/djangoapps/certificates/management/commands/ungenerated_certs.py b/lms/djangoapps/certificates/management/commands/ungenerated_certs.py index 5fb9c53718c3..5aa223acabb5 100644 --- a/lms/djangoapps/certificates/management/commands/ungenerated_certs.py +++ b/lms/djangoapps/certificates/management/commands/ungenerated_certs.py @@ -93,6 +93,7 @@ def handle(self, *args, **options): total = enrolled_students.count() count = 0 start = datetime.datetime.now(UTC) + for student in enrolled_students: count += 1 if count % STATUS_INTERVAL == 0: diff --git a/lms/djangoapps/certificates/migrations/0015_adding_mode_for_verified_certs.py b/lms/djangoapps/certificates/migrations/0015_adding_mode_for_verified_certs.py new file mode 100644 index 000000000000..c16d51b8ee4e --- /dev/null +++ b/lms/djangoapps/certificates/migrations/0015_adding_mode_for_verified_certs.py @@ -0,0 +1,86 @@ +# -*- 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 'GeneratedCertificate.mode' + db.add_column('certificates_generatedcertificate', 'mode', + self.gf('django.db.models.fields.CharField')(default='honor', max_length=32), + keep_default=False) + + + def backwards(self, orm): + # Deleting field 'GeneratedCertificate.mode' + db.delete_column('certificates_generatedcertificate', 'mode') + + + 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'}) + }, + 'certificates.certificatewhitelist': { + 'Meta': {'object_name': 'CertificateWhitelist'}, + 'course_id': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}), + 'whitelist': ('django.db.models.fields.BooleanField', [], {'default': 'False'}) + }, + 'certificates.generatedcertificate': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'GeneratedCertificate'}, + 'course_id': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255', 'blank': 'True'}), + 'created_date': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'auto_now_add': 'True', 'blank': 'True'}), + 'distinction': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'download_url': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '128', 'blank': 'True'}), + 'download_uuid': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '32', 'blank': 'True'}), + 'error_reason': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '512', 'blank': 'True'}), + 'grade': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '5', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'key': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '32', 'blank': 'True'}), + 'mode': ('django.db.models.fields.CharField', [], {'default': "'honor'", 'max_length': '32'}), + 'modified_date': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'auto_now': 'True', 'blank': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'status': ('django.db.models.fields.CharField', [], {'default': "'unavailable'", 'max_length': '32'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}), + 'verify_uuid': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '32', 'blank': 'True'}) + }, + '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'}) + } + } + + complete_apps = ['certificates'] \ No newline at end of file diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 8cd1a292c4b8..36ff18618eee 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -62,6 +62,10 @@ class CertificateStatuses(object): restricted = 'restricted' unavailable = 'unavailable' +class CertificateModes(object): + verified = 'verified' + honor = 'honor' + audit = 'audit' class CertificateWhitelist(models.Model): """ @@ -86,6 +90,7 @@ class GeneratedCertificate(models.Model): key = models.CharField(max_length=32, blank=True, default='') distinction = models.BooleanField(default=False) status = models.CharField(max_length=32, default='unavailable') + mode = models.CharField(max_length=32, default=CertificateModes.honor) name = models.CharField(blank=True, max_length=255) created_date = models.DateTimeField( auto_now_add=True, default=datetime.now) diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 5f63bbf1e2fc..33db940c44c7 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -9,7 +9,7 @@ from capa.xqueue_interface import make_xheader, make_hashkey from django.conf import settings from requests.auth import HTTPBasicAuth -from student.models import UserProfile +from student.models import UserProfile, CourseEnrollment import json import random @@ -57,7 +57,7 @@ def __init__(self, request=None): if settings.XQUEUE_INTERFACE.get('basic_auth') is not None: requests_auth = HTTPBasicAuth( - *settings.XQUEUE_INTERFACE['basic_auth']) + *settings.XQUEUE_INTERFACE['basic_auth']) else: requests_auth = None @@ -68,10 +68,10 @@ def __init__(self, request=None): self.request = request self.xqueue_interface = XQueueInterface( - settings.XQUEUE_INTERFACE['url'], - settings.XQUEUE_INTERFACE['django_auth'], - requests_auth, - ) + settings.XQUEUE_INTERFACE['url'], + settings.XQUEUE_INTERFACE['django_auth'], + requests_auth, + ) self.whitelist = CertificateWhitelist.objects.all() self.restricted = UserProfile.objects.filter(allow_certificate=False) self.use_https = True @@ -84,7 +84,7 @@ def regen_cert(self, student, course_id, course=None): course_id - courseenrollment.course_id (string) WARNING: this command will leave the old certificate, if one exists, - laying around in AWS taking up space. If this is a problem, + laying around in AWS taking up space. If this is a problem, take pains to clean up storage before running this command. Change the certificate status to unavailable (if it exists) and request @@ -92,7 +92,7 @@ def regen_cert(self, student, course_id, course=None): Return the status object. """ - # TODO: when del_cert is implemented and plumbed through certificates + # TODO: when del_cert is implemented and plumbed through certificates # repo also, do a deletion followed by a creation r/t a simple # recreation. XXX: this leaves orphan cert files laying around in # AWS. See note in the docstring too. @@ -149,13 +149,15 @@ def add_cert(self, student, course_id, course=None): """ VALID_STATUSES = [status.generating, - status.unavailable, - status.deleted, + status.unavailable, + status.deleted, status.error, status.notpassing] cert_status = certificate_status_for_student(student, course_id)['status'] + new_status = cert_status + if cert_status in VALID_STATUSES: # grade the student @@ -165,9 +167,6 @@ def add_cert(self, student, course_id, course=None): course = courses.get_course_by_id(course_id) profile = UserProfile.objects.get(user=student) - cert, created = GeneratedCertificate.objects.get_or_create( - user=student, course_id=course_id) - # Needed self.request.user = student self.request.session = {} @@ -175,45 +174,59 @@ def add_cert(self, student, course_id, course=None): grade = grades.grade(student, self.request, course) is_whitelisted = self.whitelist.filter( user=student, course_id=course_id, whitelist=True).exists() + enrollment = CourseEnrollment.objects.get(user=student) + org = course_id.split('/')[0] + course_num = course_id.split('/')[1] + if enrolment.mode == CertificateModes.verified: + template_pdf = "certificate-template-{0}-{1}-verified.pdf".format( + org, course_num) + else: + # honor code and audit students + template_pdf = "certificate-template-{0}-{1}.pdf".format( + org, course_num) - if is_whitelisted or grade['grade'] is not None: + cert, created = GeneratedCertificate.objects.get_or_create( + user=student, course_id=course_id) + + cert.mode = enrollment.mode - key = make_hashkey(random.random()) + cert.user = student + cert.grade = grade['percent'] + cert.course_id = course_id + cert.name = profile.name - cert.grade = grade['percent'] - cert.user = student - cert.course_id = course_id - cert.key = key - cert.name = profile.name + if is_whitelisted or grade['grade'] is not None: # check to see whether the student is on the # the embargoed country restricted list # otherwise, put a new certificate request # on the queue + if self.restricted.filter(user=student).exists(): - cert.status = status.restricted + new_status = status.restricted + cert.status = new_status cert.save() else: + key = make_hashkey(random.random()) + cert.key = key contents = { 'action': 'create', 'username': student.username, 'course_id': course_id, 'name': profile.name, 'grade': grade['grade'], + 'template_pdf': template_pdf, } - cert.status = status.generating + new_status = status.generating + cert.status = new_status cert.save() self._send_to_xqueue(contents, key) else: - cert_status = status.notpassing - cert.grade = grade['percent'] - cert.user = student - cert.course_id = course_id - cert.name = profile.name - cert.status = cert_status + new_status = status.notpassing + cert.status = new_status cert.save() - return cert_status + return new_status def _send_to_xqueue(self, contents, key): @@ -227,7 +240,7 @@ def _send_to_xqueue(self, contents, key): proto, settings.SITE_NAME, key), key, settings.CERT_QUEUE) (error, msg) = self.xqueue_interface.send_to_queue( - header=xheader, body=json.dumps(contents)) + header=xheader, body=json.dumps(contents)) if error: logger.critical('Unable to add a request to the queue: {} {}'.format(error, msg)) raise Exception('Unable to send queue message') From 2fed61814a4ee63e2753933e816b8680647608b9 Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Wed, 6 Nov 2013 17:32:57 -0500 Subject: [PATCH 03/87] fixing some typos --- lms/djangoapps/certificates/queue.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 33db940c44c7..8dffa7ee247d 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -1,7 +1,7 @@ from certificates.models import GeneratedCertificate from certificates.models import certificate_status_for_student from certificates.models import CertificateStatuses as status -from certificates.models import CertificateWhitelist +from certificates.models import CertificateWhitelist, CertificateModes from courseware import grades, courses from django.test.client import RequestFactory @@ -177,12 +177,12 @@ def add_cert(self, student, course_id, course=None): enrollment = CourseEnrollment.objects.get(user=student) org = course_id.split('/')[0] course_num = course_id.split('/')[1] - if enrolment.mode == CertificateModes.verified: - template_pdf = "certificate-template-{0}-{1}-verified.pdf".format( + if enrollment.mode == CertificateModes.verified: + template_pdf = "certificate-template-{0}-{1}-verified.pdf".format( org, course_num) else: # honor code and audit students - template_pdf = "certificate-template-{0}-{1}.pdf".format( + template_pdf = "certificate-template-{0}-{1}.pdf".format( org, course_num) cert, created = GeneratedCertificate.objects.get_or_create( From 1b7a871926848beecd88c86db55c7e2d36296fe9 Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Fri, 15 Nov 2013 22:18:31 +0000 Subject: [PATCH 04/87] Fixed password reset message LMS-1507 --- common/djangoapps/student/views.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index f92ffe9d3ea8..d4a03dca377b 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1229,11 +1229,8 @@ def password_reset(request): from_email=settings.DEFAULT_FROM_EMAIL, request=request, domain_override=request.get_host()) - return HttpResponse(json.dumps({'success': True, + return HttpResponse(json.dumps({'success': True, 'value': render_to_string('registration/password_reset_done.html', {})})) - else: - return HttpResponse(json.dumps({'success': False, - 'error': _('Invalid e-mail or user')})) def password_reset_confirm_wrapper( @@ -1515,4 +1512,4 @@ def change_email_settings(request): log.info(u"User {0} ({1}) opted out of receiving emails from course {2}".format(user.username, user.email, course_id)) track.views.server_track(request, "change-email-settings", {"receive_emails": "no", "course": course_id}, page='dashboard') - return HttpResponse(json.dumps({'success': True})) \ No newline at end of file + return HttpResponse(json.dumps({'success': True})) \ No newline at end of file From c8a98051dd99b68da27d4e276b964b1c85beee04 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Fri, 15 Nov 2013 21:28:03 -0800 Subject: [PATCH 05/87] CSV Reporting of Shopping Cart Purchases, with tests squashing to one commit to make cherry-picking by feature possible --- lms/djangoapps/shoppingcart/admin.py | 7 + ...nannotation__add_field_orderitem_report.py | 132 +++++++++++++++ lms/djangoapps/shoppingcart/models.py | 90 ++++++++++- .../shoppingcart/tests/test_models.py | 85 +++++++++- .../shoppingcart/tests/test_views.py | 150 +++++++++++++++++- lms/djangoapps/shoppingcart/urls.py | 1 + lms/djangoapps/shoppingcart/views.py | 74 +++++++++ lms/envs/aws.py | 4 + lms/envs/common.py | 8 + lms/static/sass/views/_shoppingcart.scss | 8 + .../shoppingcart/download_report.html | 29 ++++ requirements/edx/base.txt | 1 + 12 files changed, 582 insertions(+), 7 deletions(-) create mode 100644 lms/djangoapps/shoppingcart/admin.py create mode 100644 lms/djangoapps/shoppingcart/migrations/0005_auto__add_paidcourseregistrationannotation__add_field_orderitem_report.py create mode 100644 lms/templates/shoppingcart/download_report.html diff --git a/lms/djangoapps/shoppingcart/admin.py b/lms/djangoapps/shoppingcart/admin.py new file mode 100644 index 000000000000..199a39d7c03d --- /dev/null +++ b/lms/djangoapps/shoppingcart/admin.py @@ -0,0 +1,7 @@ +""" +Allows django admin site to add PaidCourseRegistrationAnnotations +""" +from ratelimitbackend import admin +from shoppingcart.models import PaidCourseRegistrationAnnotation + +admin.site.register(PaidCourseRegistrationAnnotation) diff --git a/lms/djangoapps/shoppingcart/migrations/0005_auto__add_paidcourseregistrationannotation__add_field_orderitem_report.py b/lms/djangoapps/shoppingcart/migrations/0005_auto__add_paidcourseregistrationannotation__add_field_orderitem_report.py new file mode 100644 index 000000000000..04d37c730a30 --- /dev/null +++ b/lms/djangoapps/shoppingcart/migrations/0005_auto__add_paidcourseregistrationannotation__add_field_orderitem_report.py @@ -0,0 +1,132 @@ +# -*- 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 model 'PaidCourseRegistrationAnnotation' + db.create_table('shoppingcart_paidcourseregistrationannotation', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('course_id', self.gf('django.db.models.fields.CharField')(unique=True, max_length=128, db_index=True)), + ('annotation', self.gf('django.db.models.fields.TextField')(null=True)), + )) + db.send_create_signal('shoppingcart', ['PaidCourseRegistrationAnnotation']) + + # Adding field 'OrderItem.report_comments' + db.add_column('shoppingcart_orderitem', 'report_comments', + self.gf('django.db.models.fields.TextField')(default=''), + keep_default=False) + + + def backwards(self, orm): + # Deleting model 'PaidCourseRegistrationAnnotation' + db.delete_table('shoppingcart_paidcourseregistrationannotation') + + # Deleting field 'OrderItem.report_comments' + db.delete_column('shoppingcart_orderitem', 'report_comments') + + + 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'}) + }, + 'shoppingcart.certificateitem': { + 'Meta': {'object_name': 'CertificateItem', '_ormbases': ['shoppingcart.OrderItem']}, + 'course_enrollment': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['student.CourseEnrollment']"}), + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '128', 'db_index': 'True'}), + 'mode': ('django.db.models.fields.SlugField', [], {'max_length': '50'}), + 'orderitem_ptr': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['shoppingcart.OrderItem']", 'unique': 'True', 'primary_key': 'True'}) + }, + 'shoppingcart.order': { + 'Meta': {'object_name': 'Order'}, + 'bill_to_cardtype': ('django.db.models.fields.CharField', [], {'max_length': '32', 'blank': 'True'}), + 'bill_to_ccnum': ('django.db.models.fields.CharField', [], {'max_length': '8', 'blank': 'True'}), + 'bill_to_city': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}), + 'bill_to_country': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}), + 'bill_to_first': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}), + 'bill_to_last': ('django.db.models.fields.CharField', [], {'max_length': '64', 'blank': 'True'}), + 'bill_to_postalcode': ('django.db.models.fields.CharField', [], {'max_length': '16', 'blank': 'True'}), + 'bill_to_state': ('django.db.models.fields.CharField', [], {'max_length': '8', 'blank': 'True'}), + 'bill_to_street1': ('django.db.models.fields.CharField', [], {'max_length': '128', 'blank': 'True'}), + 'bill_to_street2': ('django.db.models.fields.CharField', [], {'max_length': '128', 'blank': 'True'}), + 'currency': ('django.db.models.fields.CharField', [], {'default': "'usd'", 'max_length': '8'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'processor_reply_dump': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'purchase_time': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'blank': 'True'}), + 'status': ('django.db.models.fields.CharField', [], {'default': "'cart'", 'max_length': '32'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'shoppingcart.orderitem': { + 'Meta': {'object_name': 'OrderItem'}, + 'currency': ('django.db.models.fields.CharField', [], {'default': "'usd'", 'max_length': '8'}), + 'fulfilled_time': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'line_desc': ('django.db.models.fields.CharField', [], {'default': "'Misc. Item'", 'max_length': '1024'}), + 'order': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['shoppingcart.Order']"}), + 'qty': ('django.db.models.fields.IntegerField', [], {'default': '1'}), + 'report_comments': ('django.db.models.fields.TextField', [], {'default': "''"}), + 'status': ('django.db.models.fields.CharField', [], {'default': "'cart'", 'max_length': '32'}), + 'unit_cost': ('django.db.models.fields.DecimalField', [], {'default': '0.0', 'max_digits': '30', 'decimal_places': '2'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'shoppingcart.paidcourseregistration': { + 'Meta': {'object_name': 'PaidCourseRegistration', '_ormbases': ['shoppingcart.OrderItem']}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '128', 'db_index': 'True'}), + 'mode': ('django.db.models.fields.SlugField', [], {'default': "'honor'", 'max_length': '50'}), + 'orderitem_ptr': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['shoppingcart.OrderItem']", 'unique': 'True', 'primary_key': 'True'}) + }, + 'shoppingcart.paidcourseregistrationannotation': { + 'Meta': {'object_name': 'PaidCourseRegistrationAnnotation'}, + 'annotation': ('django.db.models.fields.TextField', [], {'null': 'True'}), + 'course_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '128', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'student.courseenrollment': { + 'Meta': {'ordering': "('user', 'course_id')", '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'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'mode': ('django.db.models.fields.CharField', [], {'default': "'honor'", 'max_length': '100'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + } + } + + complete_apps = ['shoppingcart'] \ No newline at end of file diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 03be80861afe..f8422dbefcc3 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -2,6 +2,7 @@ import pytz import logging import smtplib +import unicodecsv from model_utils.managers import InheritanceManager from collections import namedtuple @@ -207,6 +208,8 @@ class OrderItem(models.Model): line_desc = models.CharField(default="Misc. Item", max_length=1024) currency = models.CharField(default="usd", max_length=8) # lower case ISO currency codes fulfilled_time = models.DateTimeField(null=True) + # general purpose field, not user-visible. Used for reporting + report_comments = models.TextField(default="") @property def line_cost(self): @@ -254,6 +257,66 @@ def generate_receipt_instructions(self): """ return self.pk_with_subclass, set([]) + @classmethod + def purchased_items_btw_dates(cls, start_date, end_date): + """ + Returns a QuerySet of the purchased items between start_date and end_date inclusive. + """ + return cls.objects.filter( + status="purchased", + fulfilled_time__gte=start_date, + fulfilled_time__lt=end_date, + ) + + @classmethod + def csv_purchase_report_btw_dates(cls, filelike, start_date, end_date): + """ + Outputs a CSV report into "filelike" (a file-like python object, such as an actual file, an HttpRequest, + or sys.stdout) of purchased items between start_date and end_date inclusive. + Opening and closing filelike (if applicable) should be taken care of by the caller + """ + items = cls.purchased_items_btw_dates(start_date, end_date).order_by("fulfilled_time") + + writer = unicodecsv.writer(filelike, encoding="utf-8") + writer.writerow(OrderItem.csv_report_header_row()) + + for item in items: + writer.writerow(item.csv_report_row) + + @classmethod + def csv_report_header_row(cls): + """ + Returns the "header" row for a csv report of purchases + """ + return [ + "Purchase Time", + "Order ID", + "Status", + "Quantity", + "Unit Cost", + "Total Cost", + "Currency", + "Description", + "Comments" + ] + + @property + def csv_report_row(self): + """ + Returns an array which can be fed into csv.writer to write out one csv row + """ + return [ + self.fulfilled_time, + self.order_id, # pylint: disable=no-member + self.status, + self.qty, + self.unit_cost, + self.line_cost, + self.currency, + self.line_desc, + self.report_comments, + ] + @property def pk_with_subclass(self): """ @@ -345,13 +408,13 @@ def add_to_order(cls, order, course_id, mode_slug=CourseMode.DEFAULT_MODE_SLUG, item, created = cls.objects.get_or_create(order=order, user=order.user, course_id=course_id) item.status = order.status - item.mode = course_mode.slug item.qty = 1 item.unit_cost = cost item.line_desc = 'Registration for Course: {0}'.format(course.display_name_with_default) item.currency = currency order.currency = currency + item.report_comments = item.csv_report_comments order.save() item.save() log.info("User {} added course registration {} to cart: order {}" @@ -391,6 +454,31 @@ def generate_receipt_instructions(self): return self.pk_with_subclass, set([notification]) + @property + def csv_report_comments(self): + """ + Tries to fetch an annotation associated with the course_id from the database. If not found, returns u"". + Otherwise returns the annotation + """ + try: + return PaidCourseRegistrationAnnotation.objects.get(course_id=self.course_id).annotation + except PaidCourseRegistrationAnnotation.DoesNotExist: + return u"" + + +class PaidCourseRegistrationAnnotation(models.Model): + """ + A model that maps course_id to an additional annotation. This is specifically needed because when Stanford + generates report for the paid courses, each report item must contain the payment account associated with a course. + And unfortunately we didn't have the concept of a "SKU" or stock item where we could keep this association, + so this is to retrofit it. + """ + course_id = models.CharField(unique=True, max_length=128, db_index=True) + annotation = models.TextField(null=True) + + def __unicode__(self): + return u"{} : {}".format(self.course_id, self.annotation) + class CertificateItem(OrderItem): """ diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index ecb76ac94160..a7196aa2a18c 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -2,6 +2,8 @@ Tests for the Shopping Cart Models """ import smtplib +import StringIO +from textwrap import dedent from boto.exception import BotoServerError # this is a super-class of SESError and catches connection errors from mock import patch, MagicMock @@ -15,7 +17,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE from shoppingcart.models import (Order, OrderItem, CertificateItem, InvalidCartItem, PaidCourseRegistration, - OrderItemSubclassPK) + OrderItemSubclassPK, PaidCourseRegistrationAnnotation) from student.tests.factories import UserFactory from student.models import CourseEnrollment from course_modes.models import CourseMode @@ -321,6 +323,87 @@ def test_purchased_callback_exception(self): self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course_id)) +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class PurchaseReportTest(ModuleStoreTestCase): + + FIVE_MINS = datetime.timedelta(minutes=5) + TEST_ANNOTATION = u'Ba\xfc\u5305' + + def setUp(self): + self.user = UserFactory.create() + self.course_id = "MITx/999/Robot_Super_Course" + self.cost = 40 + self.course = CourseFactory.create(org='MITx', number='999', display_name=u'Robot Super Course') + course_mode = CourseMode(course_id=self.course_id, + mode_slug="honor", + mode_display_name="honor cert", + min_price=self.cost) + course_mode.save() + course_mode2 = CourseMode(course_id=self.course_id, + mode_slug="verified", + mode_display_name="verified cert", + min_price=self.cost) + course_mode2.save() + self.annotation = PaidCourseRegistrationAnnotation(course_id=self.course_id, annotation=self.TEST_ANNOTATION) + self.annotation.save() + self.cart = Order.get_cart_for_user(self.user) + self.reg = PaidCourseRegistration.add_to_order(self.cart, self.course_id) + self.cert_item = CertificateItem.add_to_order(self.cart, self.course_id, self.cost, 'verified') + self.cart.purchase() + self.now = datetime.datetime.now(pytz.UTC) + + def test_purchased_items_btw_dates(self): + purchases = OrderItem.purchased_items_btw_dates(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) + self.assertEqual(len(purchases), 2) + self.assertIn(self.reg.orderitem_ptr, purchases) + self.assertIn(self.cert_item.orderitem_ptr, purchases) + no_purchases = OrderItem.purchased_items_btw_dates(self.now + self.FIVE_MINS, + self.now + self.FIVE_MINS + self.FIVE_MINS) + self.assertFalse(no_purchases) + + test_time = datetime.datetime.now(pytz.UTC) + + CORRECT_CSV = dedent(""" + Purchase Time,Order ID,Status,Quantity,Unit Cost,Total Cost,Currency,Description,Comments + {time_str},1,purchased,1,40,40,usd,Registration for Course: Robot Super Course,Ba\xc3\xbc\xe5\x8c\x85 + {time_str},1,purchased,1,40,40,usd,"Certificate of Achievement, verified cert for course Robot Super Course", + """.format(time_str=str(test_time))) + + def test_purchased_csv(self): + """ + Tests that a generated purchase report CSV is as we expect + """ + # coerce the purchase times to self.test_time so that the test can match. + # It's pretty hard to patch datetime.datetime b/c it's a python built-in, which is immutable, so we + # make the times match this way + for item in OrderItem.purchased_items_btw_dates(self.now - self.FIVE_MINS, self.now + self.FIVE_MINS): + item.fulfilled_time = self.test_time + item.save() + + # add annotation to the + csv_file = StringIO.StringIO() + OrderItem.csv_purchase_report_btw_dates(csv_file, self.now - self.FIVE_MINS, self.now + self.FIVE_MINS) + csv = csv_file.getvalue() + csv_file.close() + # Using excel mode csv, which automatically ends lines with \r\n, so need to convert to \n + self.assertEqual(csv.replace('\r\n', '\n').strip(), self.CORRECT_CSV.strip()) + + def test_csv_report_no_annotation(self): + """ + Fill in gap in test coverage. csv_report_comments for PaidCourseRegistration instance with no + matching annotation + """ + # delete the matching annotation + self.annotation.delete() + self.assertEqual(u"", self.reg.csv_report_comments) + + def test_paidcourseregistrationannotation_unicode(self): + """ + Fill in gap in test coverage. __unicode__ method of PaidCourseRegistrationAnnotation + """ + self.assertEqual(unicode(self.annotation), u'{} : {}'.format(self.course_id, self.TEST_ANNOTATION)) + + @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) class CertificateItemTest(ModuleStoreTestCase): """ diff --git a/lms/djangoapps/shoppingcart/tests/test_views.py b/lms/djangoapps/shoppingcart/tests/test_views.py index d60cab78d90e..0451277ce235 100644 --- a/lms/djangoapps/shoppingcart/tests/test_views.py +++ b/lms/djangoapps/shoppingcart/tests/test_views.py @@ -3,23 +3,23 @@ """ from urlparse import urlparse +from django.conf import settings from django.test import TestCase from django.test.utils import override_settings from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ +from django.contrib.auth.models import Group from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.exceptions import ItemNotFoundError from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE -from shoppingcart.views import add_course_to_cart -from shoppingcart.models import Order, OrderItem, CertificateItem, InvalidCartItem, PaidCourseRegistration +from shoppingcart.views import _can_download_report, _get_date_from_str +from shoppingcart.models import Order, CertificateItem, PaidCourseRegistration, OrderItem from student.tests.factories import UserFactory from student.models import CourseEnrollment from course_modes.models import CourseMode -from ..exceptions import PurchasedCallbackException from mitxmako.shortcuts import render_to_response -from shoppingcart.processors import render_purchase_form_html, process_postpay_callback +from shoppingcart.processors import render_purchase_form_html from mock import patch, Mock @@ -232,3 +232,143 @@ def test_show_receipt_success_custom_receipt_page(self): self.assertEqual(resp.status_code, 200) ((template, _context), _tmp) = render_mock.call_args self.assertEqual(template, cert_item.single_item_receipt_template) + + +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class CSVReportViewsTest(ModuleStoreTestCase): + """ + Test suite for CSV Purchase Reporting + """ + def setUp(self): + self.user = UserFactory.create() + self.user.set_password('password') + self.user.save() + self.course_id = "MITx/999/Robot_Super_Course" + self.cost = 40 + self.course = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course') + self.course_mode = CourseMode(course_id=self.course_id, + mode_slug="honor", + mode_display_name="honor cert", + min_price=self.cost) + self.course_mode.save() + self.verified_course_id = 'org/test/Test_Course' + CourseFactory.create(org='org', number='test', run='course1', display_name='Test Course') + self.cart = Order.get_cart_for_user(self.user) + self.dl_grp = Group(name=settings.PAYMENT_REPORT_GENERATOR_GROUP) + self.dl_grp.save() + + def login_user(self): + """ + Helper fn to login self.user + """ + self.client.login(username=self.user.username, password="password") + + def add_to_download_group(self, user): + """ + Helper fn to add self.user to group that's allowed to download report CSV + """ + user.groups.add(self.dl_grp) + + def test_report_csv_no_access(self): + self.login_user() + response = self.client.get(reverse('payment_csv_report')) + self.assertEqual(response.status_code, 403) + + def test_report_csv_bad_method(self): + self.login_user() + self.add_to_download_group(self.user) + response = self.client.put(reverse('payment_csv_report')) + self.assertEqual(response.status_code, 400) + + @patch('shoppingcart.views.render_to_response', render_mock) + def test_report_csv_get(self): + self.login_user() + self.add_to_download_group(self.user) + response = self.client.get(reverse('payment_csv_report')) + + ((template, context), unused_kwargs) = render_mock.call_args + self.assertEqual(template, 'shoppingcart/download_report.html') + self.assertFalse(context['total_count_error']) + self.assertFalse(context['date_fmt_error']) + self.assertIn(_("Download Purchase Report"), response.content) + + @patch('shoppingcart.views.render_to_response', render_mock) + def test_report_csv_bad_date(self): + self.login_user() + self.add_to_download_group(self.user) + response = self.client.post(reverse('payment_csv_report'), {'start_date': 'BAD', 'end_date': 'BAD'}) + + ((template, context), unused_kwargs) = render_mock.call_args + self.assertEqual(template, 'shoppingcart/download_report.html') + self.assertFalse(context['total_count_error']) + self.assertTrue(context['date_fmt_error']) + self.assertIn(_("There was an error in your date input. It should be formatted as YYYY-MM-DD"), + response.content) + + @patch('shoppingcart.views.render_to_response', render_mock) + @override_settings(PAYMENT_REPORT_MAX_ITEMS=0) + def test_report_csv_too_long(self): + PaidCourseRegistration.add_to_order(self.cart, self.course_id) + self.cart.purchase() + self.login_user() + self.add_to_download_group(self.user) + response = self.client.post(reverse('payment_csv_report'), {'start_date': '1970-01-01', + 'end_date': '2100-01-01'}) + + ((template, context), unused_kwargs) = render_mock.call_args + self.assertEqual(template, 'shoppingcart/download_report.html') + self.assertTrue(context['total_count_error']) + self.assertFalse(context['date_fmt_error']) + self.assertIn(_("There are too many results in your report.") + " (>0)", response.content) + + # just going to ignored the date in this test, since we already deal with date testing + # in test_models.py + CORRECT_CSV_NO_DATE = ",1,purchased,1,40,40,usd,Registration for Course: Robot Super Course," + + def test_report_csv(self): + PaidCourseRegistration.add_to_order(self.cart, self.course_id) + self.cart.purchase() + self.login_user() + self.add_to_download_group(self.user) + response = self.client.post(reverse('payment_csv_report'), {'start_date': '1970-01-01', + 'end_date': '2100-01-01'}) + self.assertEqual(response['Content-Type'], 'text/csv') + self.assertIn(",".join(OrderItem.csv_report_header_row()), response.content) + self.assertIn(self.CORRECT_CSV_NO_DATE, response.content) + + +class UtilFnsTest(TestCase): + """ + Tests for utility functions in views.py + """ + def setUp(self): + self.user = UserFactory.create() + + def test_can_download_report_no_group(self): + """ + Group controlling perms is not present + """ + self.assertFalse(_can_download_report(self.user)) + + def test_can_download_report_not_member(self): + """ + User is not part of group controlling perms + """ + Group(name=settings.PAYMENT_REPORT_GENERATOR_GROUP).save() + self.assertFalse(_can_download_report(self.user)) + + def test_can_download_report(self): + """ + User is part of group controlling perms + """ + grp = Group(name=settings.PAYMENT_REPORT_GENERATOR_GROUP) + grp.save() + self.user.groups.add(grp) + self.assertTrue(_can_download_report(self.user)) + + def test_get_date_from_str(self): + test_str = "2013-10-01" + date = _get_date_from_str(test_str) + self.assertEqual(2013, date.year) + self.assertEqual(10, date.month) + self.assertEqual(1, date.day) diff --git a/lms/djangoapps/shoppingcart/urls.py b/lms/djangoapps/shoppingcart/urls.py index 9522d15298cc..3653c9152469 100644 --- a/lms/djangoapps/shoppingcart/urls.py +++ b/lms/djangoapps/shoppingcart/urls.py @@ -12,6 +12,7 @@ url(r'^clear/$', 'clear_cart'), url(r'^remove_item/$', 'remove_item'), url(r'^add/course/(?P[^/]+/[^/]+/[^/]+)/$', 'add_course_to_cart', name='add_course_to_cart'), + url(r'^csv_report/$', 'csv_report', name='payment_csv_report'), ) if settings.MITX_FEATURES.get('ENABLE_PAYMENT_FAKE'): diff --git a/lms/djangoapps/shoppingcart/views.py b/lms/djangoapps/shoppingcart/views.py index 8c6d61d532e5..ad7ef6b0800a 100644 --- a/lms/djangoapps/shoppingcart/views.py +++ b/lms/djangoapps/shoppingcart/views.py @@ -1,4 +1,8 @@ import logging +import datetime +import pytz +from django.conf import settings +from django.contrib.auth.models import Group from django.http import (HttpResponse, HttpResponseRedirect, HttpResponseNotFound, HttpResponseBadRequest, HttpResponseForbidden, Http404) from django.utils.translation import ugettext as _ @@ -121,3 +125,73 @@ def show_receipt(request, ordernum): context.update(order_items[0].single_item_receipt_context) return render_to_response(receipt_template, context) + + +def _can_download_report(user): + """ + Tests if the user can download the payments report, based on membership in a group whose name is determined + in settings. If the group does not exist, denies all access + """ + try: + access_group = Group.objects.get(name=settings.PAYMENT_REPORT_GENERATOR_GROUP) + except Group.DoesNotExist: + return False + return access_group in user.groups.all() + + +def _get_date_from_str(date_input): + """ + Gets date from the date input string. Lets the ValueError raised by invalid strings be processed by the caller + """ + return datetime.datetime.strptime(date_input.strip(), "%Y-%m-%d").replace(tzinfo=pytz.UTC) + + +def _render_report_form(start_str, end_str, total_count_error=False, date_fmt_error=False): + """ + Helper function that renders the purchase form. Reduces repetition + """ + context = { + 'total_count_error': total_count_error, + 'date_fmt_error': date_fmt_error, + 'start_date': start_str, + 'end_date': end_str, + } + return render_to_response('shoppingcart/download_report.html', context) + + +@login_required +def csv_report(request): + """ + Downloads csv reporting of orderitems + """ + if not _can_download_report(request.user): + return HttpResponseForbidden(_('You do not have permission to view this page.')) + + if request.method == 'POST': + start_str = request.POST.get('start_date', '') + end_str = request.POST.get('end_date', '') + try: + start_date = _get_date_from_str(start_str) + end_date = _get_date_from_str(end_str) + datetime.timedelta(days=1) + except ValueError: + # Error case: there was a badly formatted user-input date string + return _render_report_form(start_str, end_str, date_fmt_error=True) + + items = OrderItem.purchased_items_btw_dates(start_date, end_date) + if items.count() > settings.PAYMENT_REPORT_MAX_ITEMS: + # Error case: too many items would be generated in the report and we're at risk of timeout + return _render_report_form(start_str, end_str, total_count_error=True) + + response = HttpResponse(mimetype='text/csv') + filename = "purchases_report_{}.csv".format(datetime.datetime.now(pytz.UTC).strftime("%Y-%m-%d-%H-%M-%S")) + response['Content-Disposition'] = 'attachment; filename="{}"'.format(filename) + OrderItem.csv_purchase_report_btw_dates(response, start_date, end_date) + return response + + elif request.method == 'GET': + end_date = datetime.datetime.now(pytz.UTC) + start_date = end_date - datetime.timedelta(days=30) + return _render_report_form(start_date.strftime("%Y-%m-%d"), end_date.strftime("%Y-%m-%d")) + + else: + return HttpResponseBadRequest("HTTP Method Not Supported") diff --git a/lms/envs/aws.py b/lms/envs/aws.py index d524474d5b33..204f317d8eda 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -148,6 +148,10 @@ PAID_COURSE_REGISTRATION_CURRENCY = ENV_TOKENS.get('PAID_COURSE_REGISTRATION_CURRENCY', PAID_COURSE_REGISTRATION_CURRENCY) +# Payment Report Settings +PAYMENT_REPORT_GENERATOR_GROUP = ENV_TOKENS.get('PAYMENT_REPORT_GENERATOR_GROUP', PAYMENT_REPORT_GENERATOR_GROUP) +PAYMENT_REPORT_MAX_ITEMS = ENV_TOKENS.get('PAYMENT_REPORT_MAX_ITEMS', PAYMENT_REPORT_MAX_ITEMS) + # Bulk Email overrides BULK_EMAIL_DEFAULT_FROM_EMAIL = ENV_TOKENS.get('BULK_EMAIL_DEFAULT_FROM_EMAIL', BULK_EMAIL_DEFAULT_FROM_EMAIL) BULK_EMAIL_EMAILS_PER_TASK = ENV_TOKENS.get('BULK_EMAIL_EMAILS_PER_TASK', BULK_EMAIL_EMAILS_PER_TASK) diff --git a/lms/envs/common.py b/lms/envs/common.py index c698ce24be8e..376fdb240543 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -542,6 +542,12 @@ } # Setting for PAID_COURSE_REGISTRATION, DOES NOT AFFECT VERIFIED STUDENTS PAID_COURSE_REGISTRATION_CURRENCY = ['usd', '$'] + +# Members of this group are allowed to generate payment reports +PAYMENT_REPORT_GENERATOR_GROUP = 'shoppingcart_report_access' +# Maximum number of rows the report can contain +PAYMENT_REPORT_MAX_ITEMS = 10000 + ################################# open ended grading config ##################### #By setting up the default settings with an incorrect user name and password, @@ -899,6 +905,8 @@ # parallel, and what the SES rate is. BULK_EMAIL_RETRY_DELAY_BETWEEN_SENDS = 0.02 + + ################################### APPS ###################################### INSTALLED_APPS = ( # Standard ones that are always installed... diff --git a/lms/static/sass/views/_shoppingcart.scss b/lms/static/sass/views/_shoppingcart.scss index d6861fb45630..1b3da66893df 100644 --- a/lms/static/sass/views/_shoppingcart.scss +++ b/lms/static/sass/views/_shoppingcart.scss @@ -5,6 +5,14 @@ padding: 30px 30px 0 30px; } +.error_msg { + margin: 20px; + padding: 5px; + color: $red; + border: 1px solid $red; + +} + .cart-list { padding: 30px; margin-top: 40px; diff --git a/lms/templates/shoppingcart/download_report.html b/lms/templates/shoppingcart/download_report.html new file mode 100644 index 000000000000..838b07f145ee --- /dev/null +++ b/lms/templates/shoppingcart/download_report.html @@ -0,0 +1,29 @@ +<%! from django.utils.translation import ugettext as _ %> +<%! from django.core.urlresolvers import reverse %> +<%inherit file="../main.html" /> + +<%block name="title">${_("Download Purchase Report")} + + +
+

${_("Download CSV of purchase data")}

+ % if date_fmt_error: +
+ ${_("There was an error in your date input. It should be formatted as YYYY-MM-DD")} +
+ % endif + % if total_count_error: +
+ ${_("There are too many results in your report.")} (>${settings.PAYMENT_REPORT_MAX_ITEMS}). + ${_("Try making the date range smaller.")} +
+ % endif +
+ + + + + + +
+
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index a0199378e7f0..f198b3ae1007 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -70,6 +70,7 @@ South==0.7.6 sympy==0.7.1 xmltodict==0.4.1 django-ratelimit-backend==0.6 +unicodecsv==0.9.4 # Used for debugging ipython==0.13.1 From 41b73d8f482ae218bb1c1795b3879541de835e40 Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Mon, 18 Nov 2013 20:03:01 +0000 Subject: [PATCH 06/87] Basic test fix --- common/djangoapps/student/tests/tests.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 06d61c0425cd..b90ba3a165c6 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -59,23 +59,28 @@ def setUp(self): self.user_bad_passwd.password = UNUSABLE_PASSWORD self.user_bad_passwd.save() + @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) def test_user_bad_password_reset(self): """Tests password reset behavior for user with password marked UNUSABLE_PASSWORD""" bad_pwd_req = self.request_factory.post('/password_reset/', {'email': self.user_bad_passwd.email}) bad_pwd_resp = password_reset(bad_pwd_req) + # If they've got an unusable password, fine, we should let them reset it self.assertEquals(bad_pwd_resp.status_code, 200) - self.assertEquals(bad_pwd_resp.content, json.dumps({'success': False, - 'error': 'Invalid e-mail or user'})) + self.assertEquals(bad_pwd_resp.content, json.dumps({'success': True, + 'value': "('registration/password_reset_done.html', [])"})) + @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) def test_nonexist_email_password_reset(self): """Now test the exception cases with of reset_password called with invalid email.""" bad_email_req = self.request_factory.post('/password_reset/', {'email': self.user.email+"makeItFail"}) bad_email_resp = password_reset(bad_email_req) + # Note: even if the email is bad, we return a successful response code + # This prevents someone potentially trying to "brute-force" find out which emails are and aren't registered with edX self.assertEquals(bad_email_resp.status_code, 200) - self.assertEquals(bad_email_resp.content, json.dumps({'success': False, - 'error': 'Invalid e-mail or user'})) + self.assertEquals(bad_email_resp.content, json.dumps({'success': True, + 'value': "('registration/password_reset_done.html', [])"})) @unittest.skipUnless(not settings.MITX_FEATURES.get('DISABLE_PASSWORD_RESET_EMAIL_TEST', False), dedent("""Skipping Test because CMS has not provided necessary templates for password reset. From 2e31ff8c35e75489cf89fa653af154052eac2fc8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 14 Nov 2013 17:25:34 -0500 Subject: [PATCH 07/87] Recover from error loading forum thread list When a user attempts to load more threads in the forum navigation sidebar, reset the state of the world so the user can retry, and alert the user appropriately. --- CHANGELOG.rst | 2 ++ common/static/coffee/src/discussion/discussion.coffee | 6 +++--- .../src/discussion/views/discussion_thread_list_view.coffee | 6 +++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 69522b058461..46fc3f9cdf00 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +LMS: Add error recovery when a user loads more threads in the forum sidebar. + LMS: Add a user-visible alert modal when a forums AJAX request fails. Blades: Add template for checkboxes response to studio. BLD-193. diff --git a/common/static/coffee/src/discussion/discussion.coffee b/common/static/coffee/src/discussion/discussion.coffee index 5a52cd4de016..954dd8012944 100644 --- a/common/static/coffee/src/discussion/discussion.coffee +++ b/common/static/coffee/src/discussion/discussion.coffee @@ -25,9 +25,8 @@ if Backbone? @add model model - retrieveAnotherPage: (mode, options={}, sort_options={})-> - @current_page += 1 - data = { page: @current_page } + retrieveAnotherPage: (mode, options={}, sort_options={}, error=null)-> + data = { page: @current_page + 1 } switch mode when 'search' url = DiscussionUtil.urlFor 'search' @@ -59,6 +58,7 @@ if Backbone? @reset new_collection @pages = response.num_pages @current_page = response.page + error: error sortByDate: (thread) -> # diff --git a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee index 5a1e3fa9ae12..ad527cf20b1c 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee @@ -156,7 +156,11 @@ if Backbone? $(".post-list a").first()?.focus() ) - @collection.retrieveAnotherPage(@mode, options, {sort_key: @sortBy}) + error = => + @renderThreads() + DiscussionUtil.discussionAlert("Sorry", "We had some trouble loading more threads. Please try again.") + + @collection.retrieveAnotherPage(@mode, options, {sort_key: @sortBy}, error) renderThread: (thread) => content = $(_.template($("#thread-list-item-template").html())(thread.toJSON())) From 95932610a7fe02fd416385314cfe23d4fbfacd9a Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 14 Nov 2013 17:03:30 -0500 Subject: [PATCH 08/87] Add focus trap on forum navigation thread loading For accessibility purposes, it is bad to allow a user to initiate loading of additional threads in the navigation sidebar and then shift focus away from the sidebar, only to have focus snap back when the additional threads are loaded. Now, we trap focus on the loading element as recommended by our accessibility consultant. JIRA: FOR-238 --- CHANGELOG.rst | 3 +++ common/static/coffee/src/discussion/utils.coffee | 14 ++++++++------ .../views/discussion_thread_list_view.coffee | 5 ++++- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 46fc3f9cdf00..91eac8f61167 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,9 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +LMS: Trap focus on the loading element when a user loads more threads +in the forum sidebar to improve accessibility. + LMS: Add error recovery when a user loads more threads in the forum sidebar. LMS: Add a user-visible alert modal when a forums AJAX request fails. diff --git a/common/static/coffee/src/discussion/utils.coffee b/common/static/coffee/src/discussion/utils.coffee index 73cfde8a0672..89014c5f57f1 100644 --- a/common/static/coffee/src/discussion/utils.coffee +++ b/common/static/coffee/src/discussion/utils.coffee @@ -87,6 +87,13 @@ class @DiscussionUtil "notifications_status" : "/notification_prefs/status" }[name] + @makeFocusTrap: (elem) -> + elem.keydown( + (event) -> + if event.which == 9 # Tab + event.preventDefault() + ) + @discussionAlert: (header, body) -> if $("#discussion-alert").length == 0 alertDiv = $("" ) - # Capture focus - alertDiv.find("button").keydown( - (event) -> - if event.which == 9 # Tab - event.preventDefault() - ) + @makeFocusTrap(alertDiv.find("button")) alertTrigger = $("").css("display", "none") alertTrigger.leanModal({closeButton: "#discussion-alert .dismiss", overlay: 1, top: 200}) $("body").append(alertDiv).append(alertTrigger) diff --git a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee index ad527cf20b1c..57385c15bd7e 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee @@ -124,8 +124,11 @@ if Backbone? loadMorePages: (event) -> if event event.preventDefault() - @$(".more-pages").html('
Loading more threads
') + @$(".more-pages").html('
Loading more threads
') @$(".more-pages").addClass("loading") + loadingDiv = @$(".more-pages .loading-animation") + DiscussionUtil.makeFocusTrap(loadingDiv) + loadingDiv.focus() options = {} switch @mode when 'search' From e3b8ce708f6fd2431a5cd5e412f508a676585e9d Mon Sep 17 00:00:00 2001 From: RobertMarks Date: Mon, 18 Nov 2013 09:20:00 -0800 Subject: [PATCH 09/87] changes to allow multiple choicetextresponses in one problem --- common/lib/capa/capa/templates/choicetext.html | 2 +- common/static/js/capa/choicetextinput.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/capa/capa/templates/choicetext.html b/common/lib/capa/capa/templates/choicetext.html index 5f587e214a02..e74e9f71e51b 100644 --- a/common/lib/capa/capa/templates/choicetext.html +++ b/common/lib/capa/capa/templates/choicetext.html @@ -55,7 +55,7 @@ % else: <% my_id = content_node.get('contents','') %> <% my_val = value.get(my_id,'') %> - + %endif ${content_node['tail_text']} diff --git a/common/static/js/capa/choicetextinput.js b/common/static/js/capa/choicetextinput.js index 4d7540f93807..514e3f67f5e3 100644 --- a/common/static/js/capa/choicetextinput.js +++ b/common/static/js/capa/choicetextinput.js @@ -1,13 +1,13 @@ (function () { var update = function () { // Whenever a value changes create a new serialized version of this - // problem's inputs and set the hidden input fields value to equal it. - var parent = $(this).closest('.problems-wrapper'); + // problem's inputs and set the hidden input field's value to equal it. + var parent = $(this).closest('section.choicetextinput'); // find the closest parent problems-wrapper and use that as the problem // grab the input id from the input // real_input is the hidden input field var real_input = $('input.choicetextvalue', parent); - var all_inputs = $('.choicetextinput .ctinput', parent); + var all_inputs = $('input.ctinput', parent); var user_inputs = {}; $(all_inputs).each(function (index, elt) { var node = $(elt); From fc46efb6c77033d1af0ec06f2dadfb625783ec0c Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 19 Nov 2013 14:03:05 -0500 Subject: [PATCH 10/87] Fix bug in grabbing course enrollments. LMS-1475 --- lms/djangoapps/certificates/queue.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 8dffa7ee247d..8ab9759b8cf7 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -10,6 +10,7 @@ from django.conf import settings from requests.auth import HTTPBasicAuth from student.models import UserProfile, CourseEnrollment +from verify_student.models import SoftwareSecurePhotoVerification import json import random @@ -174,7 +175,7 @@ def add_cert(self, student, course_id, course=None): grade = grades.grade(student, self.request, course) is_whitelisted = self.whitelist.filter( user=student, course_id=course_id, whitelist=True).exists() - enrollment = CourseEnrollment.objects.get(user=student) + enrollment = CourseEnrollment.objects.get(user=student, course_id=course_id) org = course_id.split('/')[0] course_num = course_id.split('/')[1] if enrollment.mode == CertificateModes.verified: From 954ca83c9000327a15b99f05466d0e0322148cf9 Mon Sep 17 00:00:00 2001 From: danielcebrian Date: Tue, 19 Nov 2013 14:37:07 -0500 Subject: [PATCH 11/87] Update AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 9326b6781a27..60ac912d49ed 100644 --- a/AUTHORS +++ b/AUTHORS @@ -97,3 +97,4 @@ Iain Dunning Olivier Marquez Florian Dufour Manuel Freire +Daniel Cebrián Robles From 87238e6d938e3b972b3404cfe3c00b2c74793e9f Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Tue, 19 Nov 2013 15:54:20 +0000 Subject: [PATCH 12/87] Removed null bits --- common/djangoapps/student/tests/tests.py | 2 +- common/djangoapps/student/views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index b90ba3a165c6..9aa5ad8279f3 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -65,7 +65,7 @@ def test_user_bad_password_reset(self): bad_pwd_req = self.request_factory.post('/password_reset/', {'email': self.user_bad_passwd.email}) bad_pwd_resp = password_reset(bad_pwd_req) - # If they've got an unusable password, fine, we should let them reset it + # If they've got an unusable password, we return a successful response code self.assertEquals(bad_pwd_resp.status_code, 200) self.assertEquals(bad_pwd_resp.content, json.dumps({'success': True, 'value': "('registration/password_reset_done.html', [])"})) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index d4a03dca377b..1702d7145e83 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1512,4 +1512,4 @@ def change_email_settings(request): log.info(u"User {0} ({1}) opted out of receiving emails from course {2}".format(user.username, user.email, course_id)) track.views.server_track(request, "change-email-settings", {"receive_emails": "no", "course": course_id}, page='dashboard') - return HttpResponse(json.dumps({'success': True})) \ No newline at end of file + return HttpResponse(json.dumps({'success': True})) \ No newline at end of file From bcb5f1b3689bbf9b76e23997caa4f713cda8aea1 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 19 Nov 2013 16:34:01 -0500 Subject: [PATCH 13/87] Increased timeout for element count in HTML test --- .../features/component_settings_editor_helpers.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py index 5473438571e1..d3293c474e72 100644 --- a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py +++ b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py @@ -6,14 +6,6 @@ from terrain.steps import reload_the_page -def _is_expected_element_count(css, expected_number): - """ - Returns whether the number of elements found on the page by css locator - the same number that you expected. - """ - return len(world.css_find(css)) == expected_number - - @world.absorb def create_component_instance(step, category, component_type=None, is_advanced=False): """ @@ -47,8 +39,11 @@ def create_component_instance(step, category, component_type=None, is_advanced=F world.wait_for_invisible(component_button_css) click_component_from_menu(category, component_type, is_advanced) - world.wait_for(lambda _: _is_expected_element_count(module_css, - module_count_before + 1)) + expected_count = module_count_before + 1 + world.wait_for( + lambda _: len(world.css_find(module_css)) == expected_count, + timeout=20 + ) @world.absorb From 6e0140b65a91f30096c60f3870a455ab41a85260 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Tue, 19 Nov 2013 16:28:46 -0500 Subject: [PATCH 14/87] revises .gitignore file to include static css directories and remnants of devstack setup --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index b3f7473dc077..d864ecf7d0fd 100644 --- a/.gitignore +++ b/.gitignore @@ -48,14 +48,18 @@ reports/ .prereqs_cache .vagrant/ node_modules +.bundle/ +bin/ ### Static assets pipeline artifacts *.scssc +lms/static/css/ lms/static/sass/*.css lms/static/sass/application.scss lms/static/sass/application-extend1.scss lms/static/sass/application-extend2.scss lms/static/sass/course.scss +cms/static/css/ cms/static/sass/*.css ### Logging artifacts From e7c06e3ab17b77e1be57ede079835130db9c21bf Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 15 Nov 2013 11:20:12 -0500 Subject: [PATCH 15/87] Change preview view method to use RESTful URL. STUD-848 --- .../contentstore/tests/test_contentstore.py | 39 +++++++++------- .../contentstore/views/component.py | 9 ++-- cms/djangoapps/contentstore/views/item.py | 28 +++++++++++- cms/djangoapps/contentstore/views/preview.py | 45 ++++--------------- cms/djangoapps/contentstore/views/tabs.py | 9 ++-- .../coffee/spec/views/module_edit_spec.coffee | 11 ++--- .../coffee/src/views/module_edit.coffee | 6 +-- cms/static/coffee/src/views/tabs.coffee | 3 +- cms/static/coffee/src/views/unit.coffee | 1 - cms/templates/edit-tabs.html | 4 +- cms/templates/unit.html | 4 +- cms/urls.py | 1 - 12 files changed, 74 insertions(+), 86 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 58cf3be70b0f..2b773e991d50 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -454,31 +454,36 @@ def test_xlint_fails(self): @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/toy/.*']) def test_module_preview_in_whitelist(self): - ''' + """ Tests the ajax callback to render an XModule - ''' - direct_store = modulestore('direct') - import_from_xml(direct_store, 'common/test/data/', ['toy']) - - # also try a custom response which will trigger the 'is this course in whitelist' logic - problem_module_location = Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None]) - url = reverse('preview_component', kwargs={'location': problem_module_location.url()}) - resp = self.client.get_html(url) - self.assertEqual(resp.status_code, 200) + """ + resp = self._test_preview(Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None])) + # These are the data-ids of the xblocks contained in the vertical. + # Ultimately, these must be converted to new locators. + self.assertContains(resp, 'i4x://edX/toy/video/sample_video') + self.assertContains(resp, 'i4x://edX/toy/video/separate_file_video') + self.assertContains(resp, 'i4x://edX/toy/video/video_with_end_time') + self.assertContains(resp, 'i4x://edX/toy/poll_question/T1_changemind_poll_foo_2') def test_video_module_caption_asset_path(self): - ''' + """ This verifies that a video caption url is as we expect it to be - ''' + """ + resp = self._test_preview(Location(['i4x', 'edX', 'toy', 'video', 'sample_video', None])) + self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"') + + def _test_preview(self, location): + """ Preview test case. """ direct_store = modulestore('direct') - import_from_xml(direct_store, 'common/test/data/', ['toy']) + _, course_items = import_from_xml(direct_store, 'common/test/data/', ['toy']) # also try a custom response which will trigger the 'is this course in whitelist' logic - video_module_location = Location(['i4x', 'edX', 'toy', 'video', 'sample_video', None]) - url = reverse('preview_component', kwargs={'location': video_module_location.url()}) - resp = self.client.get_html(url) + locator = loc_mapper().translate_location( + course_items[0].location.course_id, location, False, True + ) + resp = self.client.get_html(locator.url_reverse('xblock')) self.assertEqual(resp.status_code, 200) - self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"') + return resp def test_delete(self): direct_store = modulestore('direct') diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 327e75c7f47e..70d7bef7cece 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -249,12 +249,9 @@ def edit_unit(request, location): ) components = [ - [ - component.location.url(), - loc_mapper().translate_location( - course.location.course_id, component.location, False, True - ) - ] + loc_mapper().translate_location( + course.location.course_id, component.location, False, True + ) for component in item.get_children() ] diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 97bfde3b8285..d33d00377b5f 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -3,7 +3,9 @@ import logging from uuid import uuid4 +from functools import partial from static_replace import replace_static_urls +from xmodule_modifiers import wrap_xblock from django.core.exceptions import PermissionDenied from django.contrib.auth.decorators import login_required @@ -27,6 +29,8 @@ from student.models import CourseEnrollment from django.http import HttpResponseBadRequest from xblock.fields import Scope +from preview import handler_prefix, get_preview_html +from mitxmako.shortcuts import render_to_response, render_to_string __all__ = ['orphan_handler', 'xblock_handler'] @@ -51,6 +55,7 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid= all children and "all_versions" to delete from all (mongo) versions. GET json: returns representation of the xblock (locator id, data, and metadata). + html: returns HTML for rendering the xblock (which includes both the "preview" view and the "editor" view) PUT or POST json: if xblock location is specified, update the xblock instance. The json payload can contain these fields, all optional: @@ -76,8 +81,27 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid= old_location = loc_mapper().translate_locator_to_location(location) if request.method == 'GET': - rsp = _get_module_info(location) - return JsonResponse(rsp) + if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): + rsp = _get_module_info(location) + return JsonResponse(rsp) + else: + component = modulestore().get_item(old_location) + # Wrap the generated fragment in the xmodule_editor div so that the javascript + # can bind to it correctly + component.runtime.wrappers.append(partial(wrap_xblock, handler_prefix)) + + try: + content = component.render('studio_view').content + # catch exceptions indiscriminately, since after this point they escape the + # dungeon and surface as uneditable, unsaveable, and undeletable + # component-goblins. + except Exception as exc: # pylint: disable=W0703 + content = render_to_string('html_error.html', {'message': str(exc)}) + + return render_to_response('component.html', { + 'preview': get_preview_html(request, component), + 'editor': content + }) elif request.method == 'DELETE': delete_children = str_to_bool(request.REQUEST.get('recurse', 'False')) delete_all_versions = str_to_bool(request.REQUEST.get('all_versions', 'False')) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 6e86a6485b3d..3b2ec85326c8 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -3,7 +3,7 @@ from django.conf import settings from django.core.urlresolvers import reverse -from django.http import Http404, HttpResponseBadRequest, HttpResponseForbidden +from django.http import Http404, HttpResponseBadRequest from django.contrib.auth.decorators import login_required from mitxmako.shortcuts import render_to_response, render_to_string @@ -24,10 +24,9 @@ import static_replace from .session_kv_store import SessionKeyValueStore from .helpers import render_from_lms -from .access import has_access from ..utils import get_course_for_item -__all__ = ['preview_handler', 'preview_component'] +__all__ = ['preview_handler'] log = logging.getLogger(__name__) @@ -53,13 +52,13 @@ def preview_handler(request, usage_id, handler, suffix=''): usage_id: The usage-id of the block to dispatch to, passed through `quote_slashes` handler: The handler to execute - suffix: The remaineder of the url to be passed to the handler + suffix: The remainder of the url to be passed to the handler """ location = unquote_slashes(usage_id) descriptor = modulestore().get_item(location) - instance = load_preview_module(request, descriptor) + instance = _load_preview_module(request, descriptor) # Let the module handle the AJAX req = django_to_webob_request(request) try: @@ -85,32 +84,6 @@ def preview_handler(request, usage_id, handler, suffix=''): return webob_to_django_response(resp) -@login_required -def preview_component(request, location): - "Return the HTML preview of a component" - # TODO (vshnayder): change name from id to location in coffee+html as well. - if not has_access(request.user, location): - return HttpResponseForbidden() - - component = modulestore().get_item(location) - # Wrap the generated fragment in the xmodule_editor div so that the javascript - # can bind to it correctly - component.runtime.wrappers.append(partial(wrap_xblock, handler_prefix)) - - try: - content = component.render('studio_view').content - # catch exceptions indiscriminately, since after this point they escape the - # dungeon and surface as uneditable, unsaveable, and undeletable - # component-goblins. - except Exception as exc: # pylint: disable=W0703 - content = render_to_string('html_error.html', {'message': str(exc)}) - - return render_to_response('component.html', { - 'preview': get_preview_html(request, component), - 'editor': content - }) - - class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ An XModule ModuleSystem for use in Studio previews @@ -119,7 +92,7 @@ def handler_url(self, block, handler_name, suffix='', query=''): return handler_prefix(block, handler_name, suffix) + '?' + query -def preview_module_system(request, descriptor): +def _preview_module_system(request, descriptor): """ Returns a ModuleSystem for the specified descriptor that is specialized for rendering module previews. @@ -135,7 +108,7 @@ def preview_module_system(request, descriptor): # TODO (cpennington): Do we want to track how instructors are using the preview problems? track_function=lambda event_type, event: None, filestore=descriptor.runtime.resources_fs, - get_module=partial(load_preview_module, request), + get_module=partial(_load_preview_module, request), render_template=render_from_lms, debug=True, replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id), @@ -162,7 +135,7 @@ def preview_module_system(request, descriptor): ) -def load_preview_module(request, descriptor): +def _load_preview_module(request, descriptor): """ Return a preview XModule instantiated from the supplied descriptor. @@ -171,7 +144,7 @@ def load_preview_module(request, descriptor): """ student_data = DbModel(SessionKeyValueStore(request)) descriptor.bind_for_student( - preview_module_system(request, descriptor), + _preview_module_system(request, descriptor), LmsFieldData(descriptor._field_data, student_data), # pylint: disable=protected-access ) return descriptor @@ -182,7 +155,7 @@ def get_preview_html(request, descriptor): Returns the HTML returned by the XModule's student_view, specified by the descriptor and idx. """ - module = load_preview_module(request, descriptor) + module = _load_preview_module(request, descriptor) try: content = module.render("student_view").content except Exception as exc: # pylint: disable=W0703 diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 277445e3b929..40ea9bfd6bf3 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -125,12 +125,9 @@ def edit_tabs(request, org, course, coursename): static_tabs.append(modulestore('direct').get_item(static_tab_loc)) components = [ - [ - static_tab.location.url(), - loc_mapper().translate_location( - course_item.location.course_id, static_tab.location, False, True - ) - ] + loc_mapper().translate_location( + course_item.location.course_id, static_tab.location, False, True + ) for static_tab in static_tabs ] diff --git a/cms/static/coffee/spec/views/module_edit_spec.coffee b/cms/static/coffee/spec/views/module_edit_spec.coffee index 22d1052fa3a0..36716668d34c 100644 --- a/cms/static/coffee/spec/views/module_edit_spec.coffee +++ b/cms/static/coffee/spec/views/module_edit_spec.coffee @@ -1,12 +1,9 @@ -define ["coffee/src/views/module_edit", "xmodule"], (ModuleEdit) -> +define ["coffee/src/views/module_edit", "js/models/module_info", "xmodule"], (ModuleEdit, ModuleModel) -> describe "ModuleEdit", -> beforeEach -> - @stubModule = jasmine.createSpy("Module") - @stubModule.id = 'stub-id' - @stubModule.get = (param)-> - if param == 'old_id' - return 'stub-old-id' + @stubModule = new ModuleModel + id: "stub-id" setFixtures """
  • @@ -62,7 +59,7 @@ define ["coffee/src/views/module_edit", "xmodule"], (ModuleEdit) -> @moduleEdit.render() it "loads the module preview and editor via ajax on the view element", -> - expect(@moduleEdit.$el.load).toHaveBeenCalledWith("/preview_component/#{@moduleEdit.model.get('old_id')}", jasmine.any(Function)) + expect(@moduleEdit.$el.load).toHaveBeenCalledWith("/xblock/#{@moduleEdit.model.id}", jasmine.any(Function)) @moduleEdit.$el.load.mostRecentCall.args[1]() expect(@moduleEdit.loadDisplay).toHaveBeenCalled() expect(@moduleEdit.delegateEvents).toHaveBeenCalled() diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index a13e572887c8..729a17615e18 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -69,15 +69,13 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1", payload (data) => @model.set(id: data.locator) - @model.set(old_id: data.id) - @$el.data('id', data.id) @$el.data('locator', data.locator) @render() ) render: -> - if @model.get('old_id') - @$el.load("/preview_component/#{@model.get('old_id')}", => + if @model.id + @$el.load(@model.url(), => @loadDisplay() @delegateEvents() ) diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 0f72e8bddbf0..a85b3b786378 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -6,8 +6,7 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views initialize: => @$('.component').each((idx, element) => model = new ModuleModel({ - id: $(element).data('locator'), - old_id:$(element).data('id') + id: $(element).data('locator') }) new ModuleEditView( diff --git a/cms/static/coffee/src/views/unit.coffee b/cms/static/coffee/src/views/unit.coffee index 075b56d1b0a0..e6ba0e1382a3 100644 --- a/cms/static/coffee/src/views/unit.coffee +++ b/cms/static/coffee/src/views/unit.coffee @@ -63,7 +63,6 @@ define ["jquery", "jquery.ui", "gettext", "backbone", @$('.component').each (idx, element) => model = new ModuleModel id: $(element).data('locator') - old_id: $(element).data('id') new ModuleEditView el: element, onDelete: @deleteComponent, diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index f6b8e7b77a4d..54a30217ea7e 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -61,8 +61,8 @@

    ${_("Page Actions")}

      - % for id, locator in components: -
    1. + % for locator in components: +
    2. % endfor
    3. diff --git a/cms/templates/unit.html b/cms/templates/unit.html index cc7827c7d3a2..e83dd45da08f 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -48,8 +48,8 @@

        - % for id, locator in components: -
      1. + % for locator in components: +
      2. % endfor
      3. diff --git a/cms/urls.py b/cms/urls.py index 99e9cbfaba29..343e9f4d04f8 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -14,7 +14,6 @@ url(r'^$', 'contentstore.views.howitworks', name='homepage'), url(r'^edit/(?P.*?)$', 'contentstore.views.edit_unit', name='edit_unit'), url(r'^subsection/(?P.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'), - url(r'^preview_component/(?P.*?)$', 'contentstore.views.preview_component', name='preview_component'), url(r'^transcripts/upload$', 'contentstore.views.upload_transcripts', name='upload_transcripts'), url(r'^transcripts/download$', 'contentstore.views.download_transcripts', name='download_transcripts'), From 16568766994bc77b0f543dfdf4cbed5e39a51ecc Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 19 Nov 2013 17:12:49 -0500 Subject: [PATCH 16/87] If student has not passed verification, issue an honor code cert. Also, display a message on their dashboard. --- lms/djangoapps/certificates/models.py | 9 +++++---- lms/djangoapps/certificates/queue.py | 11 ++++++++--- .../dashboard/_dashboard_certificate_information.html | 8 +++++++- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 36ff18618eee..4e948d4b0697 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -93,9 +93,9 @@ class GeneratedCertificate(models.Model): mode = models.CharField(max_length=32, default=CertificateModes.honor) name = models.CharField(blank=True, max_length=255) created_date = models.DateTimeField( - auto_now_add=True, default=datetime.now) + auto_now_add=True, default=datetime.now) modified_date = models.DateTimeField( - auto_now=True, default=datetime.now) + auto_now=True, default=datetime.now) error_reason = models.CharField(max_length=512, blank=True, default='') class Meta: @@ -133,8 +133,9 @@ def certificate_status_for_student(student, course_id): try: generated_certificate = GeneratedCertificate.objects.get( - user=student, course_id=course_id) - d = {'status': generated_certificate.status} + user=student, course_id=course_id) + d = {'status': generated_certificate.status, + 'mode': generated_certificate.mode} if generated_certificate.grade: d['grade'] = generated_certificate.grade if generated_certificate.status == CertificateStatuses.downloadable: diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 8ab9759b8cf7..03a0947aa9c3 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -178,9 +178,15 @@ def add_cert(self, student, course_id, course=None): enrollment = CourseEnrollment.objects.get(user=student, course_id=course_id) org = course_id.split('/')[0] course_num = course_id.split('/')[1] - if enrollment.mode == CertificateModes.verified: + cert_mode = enrollment.mode + if enrollment.mode == CertificateModes.verified and SoftwareSecurePhotoVerification.user_is_verified(student): template_pdf = "certificate-template-{0}-{1}-verified.pdf".format( org, course_num) + elif (enrollment.mode == CertificateModes.verified and not + SoftwareSecurePhotoVerification.user_is_verified(student)): + template_pdf = "certificate-template-{0}-{1}.pdf".format( + org, course_num) + cert_mode = CertificateModes.honor else: # honor code and audit students template_pdf = "certificate-template-{0}-{1}.pdf".format( @@ -189,8 +195,7 @@ def add_cert(self, student, course_id, course=None): cert, created = GeneratedCertificate.objects.get_or_create( user=student, course_id=course_id) - cert.mode = enrollment.mode - + cert.mode = cert_mode cert.user = student cert.grade = grade['percent'] cert.course_id = course_id diff --git a/lms/templates/dashboard/_dashboard_certificate_information.html b/lms/templates/dashboard/_dashboard_certificate_information.html index ea5171c0ef9e..3222b6aae854 100644 --- a/lms/templates/dashboard/_dashboard_certificate_information.html +++ b/lms/templates/dashboard/_dashboard_certificate_information.html @@ -19,7 +19,7 @@ % elif cert_status['status'] in ('generating', 'ready', 'notpassing', 'restricted'):

        ${_("Your final grade:")} ${"{0:.0f}%".format(float(cert_status['grade'])*100)}. - % if cert_status['status'] == 'notpassing': + % if cert_status['status'] == 'notpassing' and enrollment.mode != 'audit': ${_("Grade required for a certificate:")} ${"{0:.0f}%".format(float(course.lowest_passing_grade)*100)}. % elif cert_status['status'] == 'restricted' and enrollment.mode == 'verified': @@ -44,6 +44,12 @@ ${_("Download Your Certificate (PDF)")}

        4. + % elif cert_status['show_download_url'] and enrollment.mode == 'verified' and cert_status['mode'] == 'honor': +
      4. +

        ${_('Since we did not have a valid set of verification photos from you when certificates were generated, we could not grant you a verified certificate. An honor code certificate has been granted instead.')}

        +         + ${_("Download Your Certificate (PDF)")}
        5. % elif cert_status['show_download_url'] and enrollment.mode == 'verified':
      5. Date: Tue, 19 Nov 2013 17:30:27 -0500 Subject: [PATCH 17/87] Use class methods to find the enrollment mode. --- lms/djangoapps/certificates/queue.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 03a0947aa9c3..0c07ed6e6f07 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -175,14 +175,14 @@ def add_cert(self, student, course_id, course=None): grade = grades.grade(student, self.request, course) is_whitelisted = self.whitelist.filter( user=student, course_id=course_id, whitelist=True).exists() - enrollment = CourseEnrollment.objects.get(user=student, course_id=course_id) + enrollment_mode = CourseEnrollment.enrollment_mode_for_user(student, course_id) org = course_id.split('/')[0] course_num = course_id.split('/')[1] - cert_mode = enrollment.mode - if enrollment.mode == CertificateModes.verified and SoftwareSecurePhotoVerification.user_is_verified(student): + cert_mode = enrollment_mode + if enrollment_mode == CertificateModes.verified and SoftwareSecurePhotoVerification.user_is_verified(student): template_pdf = "certificate-template-{0}-{1}-verified.pdf".format( org, course_num) - elif (enrollment.mode == CertificateModes.verified and not + elif (enrollment_mode == CertificateModes.verified and not SoftwareSecurePhotoVerification.user_is_verified(student)): template_pdf = "certificate-template-{0}-{1}.pdf".format( org, course_num) From cb113deade37141e65c2bceac2ddf3fff5ca10e0 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Fri, 11 Oct 2013 17:37:04 -0400 Subject: [PATCH 18/87] Separate all db ops from modulestore ops --- .../xmodule/modulestore/split_migrator.py | 2 +- .../split_mongo/definition_lazy_loader.py | 3 +- .../split_mongo/mongo_connection.py | 116 +++++++++++++++++ .../xmodule/modulestore/split_mongo/split.py | 122 ++++++------------ .../modulestore/tests/test_split_migrator.py | 6 +- .../tests/test_split_modulestore.py | 34 ++--- 6 files changed, 170 insertions(+), 113 deletions(-) create mode 100644 common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index 27a758c0839d..355f5bfa62d3 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -88,7 +88,7 @@ def _copy_published_modules_to_course(self, new_course, old_course_loc, old_cour index_info = self.split_modulestore.get_course_index_info(course_version_locator) versions = index_info['versions'] versions['draft'] = versions['published'] - self.split_modulestore.update_course_index(course_version_locator, {'versions': versions}, update_versions=True) + self.split_modulestore.update_course_index(index_info) # clean up orphans in published version: in old mongo, parents pointed to the union of their published and draft # children which meant some pointers were to non-existent locations in 'direct' diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py index 9b2a652a95ef..ded67104b4d1 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py @@ -22,5 +22,4 @@ def fetch(self): Fetch the definition. Note, the caller should replace this lazy loader pointer with the result so as not to fetch more than once """ - return self.modulestore.definitions.find_one( - {'_id': self.definition_locator.definition_id}) + return self.modulestore.db_connection.get_definition(self.definition_locator.definition_id) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py new file mode 100644 index 000000000000..510c100048fc --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -0,0 +1,116 @@ +""" +Segregation of pymongo functions from the data modeling mechanisms for split modulestore. +""" +import pymongo + +class MongoConnection(object): + """ + Segregation of pymongo functions from the data modeling mechanisms for split modulestore. + """ + def __init__( + self, db, collection, host, port=27017, tz_aware=True, user=None, password=None, **kwargs + ): + """ + Create & open the connection, authenticate, and provide pointers to the collections + """ + self.database = pymongo.database.Database( + pymongo.MongoClient( + host=host, + port=port, + tz_aware=tz_aware, + **kwargs + ), + db + ) + + if user is not None and password is not None: + self.database.authenticate(user, password) + + self.course_index = self.database[collection + '.active_versions'] + self.structures = self.database[collection + '.structures'] + self.definitions = self.database[collection + '.definitions'] + + # every app has write access to the db (v having a flag to indicate r/o v write) + # Force mongo to report errors, at the expense of performance + # pymongo docs suck but explanation: + # http://api.mongodb.org/java/2.10.1/com/mongodb/WriteConcern.html + self.course_index.write_concern = {'w': 1} + self.structures.write_concern = {'w': 1} + self.definitions.write_concern = {'w': 1} + + def get_structure(self, key): + """ + Get the structure from the persistence mechanism whose id is the given key + """ + return self.structures.find_one({'_id': key}) + + def find_matching_structures(self, query): + """ + Find the structure matching the query. Right now the query must be a legal mongo query + :param query: a mongo-style query of {key: [value|{$in ..}|..], ..} + """ + return self.structures.find(query) + + def insert_structure(self, structure): + """ + Create the structure in the db + """ + self.structures.insert(structure) + + def update_structure(self, structure): + """ + Update the db record for structure + """ + self.structures.update({'_id': structure['_id']}, structure) + + def get_course_index(self, key): + """ + Get the course_index from the persistence mechanism whose id is the given key + """ + return self.course_index.find_one({'_id': key}) + + def find_matching_course_indexes(self, query): + """ + Find the course_index matching the query. Right now the query must be a legal mongo query + :param query: a mongo-style query of {key: [value|{$in ..}|..], ..} + """ + return self.course_index.find(query) + + def insert_course_index(self, course_index): + """ + Create the course_index in the db + """ + self.course_index.insert(course_index) + + def update_course_index(self, course_index): + """ + Update the db record for course_index + """ + self.course_index.update({'_id': course_index['_id']}, course_index) + + def delete_course_index(self, key): + """ + Delete the course_index from the persistence mechanism whose id is the given key + """ + return self.course_index.remove({'_id': key}) + + def get_definition(self, key): + """ + Get the definition from the persistence mechanism whose id is the given key + """ + return self.definitions.find_one({'_id': key}) + + def find_matching_definitions(self, query): + """ + Find the definitions matching the query. Right now the query must be a legal mongo query + :param query: a mongo-style query of {key: [value|{$in ..}|..], ..} + """ + return self.definitions.find(query) + + def insert_definition(self, definition): + """ + Create the definition in the db + """ + self.definitions.insert(definition) + + diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 61524165968f..a6349e6113b6 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1,7 +1,6 @@ import threading import datetime import logging -import pymongo import re from importlib import import_module from path import path @@ -21,6 +20,7 @@ from xblock.fields import Scope from xblock.runtime import Mixologist from bson.objectid import ObjectId +from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection log = logging.getLogger(__name__) #============================================================================== @@ -49,7 +49,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): A Mongodb backed ModuleStore supporting versions, inheritance, and sharing. """ - # pylint: disable=W0201 def __init__(self, doc_store_config, fs_root, render_template, default_class=None, error_tracker=null_error_tracker, @@ -62,44 +61,13 @@ def __init__(self, doc_store_config, fs_root, render_template, super(SplitMongoModuleStore, self).__init__(**kwargs) self.loc_mapper = loc_mapper - def do_connection( - db, collection, host, port=27017, tz_aware=True, user=None, password=None, **kwargs - ): - """ - Create & open the connection, authenticate, and provide pointers to the collections - """ - self.db = pymongo.database.Database( - pymongo.MongoClient( - host=host, - port=port, - tz_aware=tz_aware, - **kwargs - ), - db - ) - - if user is not None and password is not None: - self.db.authenticate(user, password) - - self.course_index = self.db[collection + '.active_versions'] - self.structures = self.db[collection + '.structures'] - self.definitions = self.db[collection + '.definitions'] - - do_connection(**doc_store_config) + self.db_connection = MongoConnection(**doc_store_config) + self.db = self.db_connection.database # Code review question: How should I expire entries? # _add_cache could use a lru mechanism to control the cache size? self.thread_cache = threading.local() - - # every app has write access to the db (v having a flag to indicate r/o v write) - # Force mongo to report errors, at the expense of performance - # pymongo docs suck but explanation: - # http://api.mongodb.org/java/2.10.1/com/mongodb/WriteConcern.html - self.course_index.write_concern = {'w': 1} - self.structures.write_concern = {'w': 1} - self.definitions.write_concern = {'w': 1} - if default_class is not None: module_path, _, class_name = default_class.rpartition('.') class_ = getattr(import_module(module_path), class_name) @@ -138,7 +106,7 @@ def cache_items(self, system, base_usage_ids, depth=0, lazy=True): block['definition'] = DefinitionLazyLoader(self, block['definition']) else: # Load all descendants by id - descendent_definitions = self.definitions.find({ + descendent_definitions = self.db_connection.find_matching_definitions({ '_id': {'$in': [block['definition'] for block in new_module_data.itervalues()]}}) # turn into a map @@ -226,7 +194,7 @@ def _lookup_course(self, course_locator): if course_locator.course_id is not None and course_locator.branch is not None: # use the course_id - index = self.course_index.find_one({'_id': course_locator.course_id}) + index = self.db_connection.get_course_index(course_locator.course_id) if index is None: raise ItemNotFoundError(course_locator) if course_locator.branch not in index['versions']: @@ -241,7 +209,7 @@ def _lookup_course(self, course_locator): # cast string to ObjectId if necessary version_guid = course_locator.as_object_id(version_guid) - entry = self.structures.find_one({'_id': version_guid}) + entry = self.db_connection.get_structure(version_guid) # b/c more than one course can use same structure, the 'course_id' and 'branch' are not intrinsic to structure # and the one assoc'd w/ it by another fetch may not be the one relevant to this fetch; so, @@ -269,7 +237,7 @@ def get_courses(self, branch='published', qualifiers=None): if qualifiers is None: qualifiers = {} qualifiers.update({"versions.{}".format(branch): {"$exists": True}}) - matching = self.course_index.find(qualifiers) + matching = self.db_connection.find_matching_course_indexes(qualifiers) # collect ids and then query for those version_guids = [] @@ -279,7 +247,7 @@ def get_courses(self, branch='published', qualifiers=None): version_guids.append(version_guid) id_version_map[version_guid] = structure['_id'] - course_entries = self.structures.find({'_id': {'$in': version_guids}}) + course_entries = self.db_connection.find_matching_structures({'_id': {'$in': version_guids}}) # get the block for the course element (s/b the root) result = [] @@ -455,7 +423,7 @@ def get_course_index_info(self, course_locator): """ if course_locator.course_id is None: return None - index = self.course_index.find_one({'_id': course_locator.course_id}) + index = self.db_connection.get_course_index(course_locator.course_id) return index # TODO figure out a way to make this info accessible from the course descriptor @@ -487,7 +455,7 @@ def get_definition_history_info(self, definition_locator): 'edited_on': when the change was made } """ - definition = self.definitions.find_one({'_id': definition_locator.definition_id}) + definition = self.db_connection.get_definition(definition_locator.definition_id) if definition is None: return None return definition['edit_info'] @@ -509,14 +477,14 @@ def get_course_successors(self, course_locator, version_history_depth=1): # TODO if depth is significant, it may make sense to get all that have the same original_version # and reconstruct the subtree from version_guid - next_entries = self.structures.find({'previous_version' : version_guid}) + next_entries = self.db_connection.find_matching_structures({'previous_version' : version_guid}) # must only scan cursor's once next_versions = [struct for struct in next_entries] result = {version_guid: [CourseLocator(version_guid=struct['_id']) for struct in next_versions]} depth = 1 while depth < version_history_depth and len(next_versions) > 0: depth += 1 - next_entries = self.structures.find({'previous_version': + next_entries = self.db_connection.find_matching_structures({'previous_version': {'$in': [struct['_id'] for struct in next_versions]}}) next_versions = [struct for struct in next_entries] for course_structure in next_versions: @@ -537,7 +505,7 @@ def get_block_generations(self, block_locator): course_struct = self._lookup_course(block_locator.version_agnostic())['structure'] usage_id = block_locator.usage_id update_version_field = 'blocks.{}.edit_info.update_version'.format(usage_id) - all_versions_with_block = self.structures.find({'original_version': course_struct['original_version'], + all_versions_with_block = self.db_connection.find_matching_structures({'original_version': course_struct['original_version'], update_version_field: {'$exists': True}}) # find (all) root versions and build map previous: [successors] possible_roots = [] @@ -596,7 +564,7 @@ def create_definition_from_data(self, new_def_data, category, user_id): "original_version": new_id, } } - self.definitions.insert(document) + self.db_connection.insert_definition(document) definition_locator = DefinitionLocator(new_id) return definition_locator @@ -618,7 +586,7 @@ def needs_saved(): # if this looks in cache rather than fresh fetches, then it will probably not detect # actual change b/c the descriptor and cache probably point to the same objects - old_definition = self.definitions.find_one({'_id': definition_locator.definition_id}) + old_definition = self.db_connection.get_definition(definition_locator.definition_id) if old_definition is None: raise ItemNotFoundError(definition_locator.url()) @@ -630,7 +598,7 @@ def needs_saved(): old_definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) # previous version id old_definition['edit_info']['previous_version'] = definition_locator.definition_id - self.definitions.insert(old_definition) + self.db_connection.insert_definition(old_definition) return DefinitionLocator(old_definition['_id']), True else: return definition_locator, False @@ -657,7 +625,7 @@ def _generate_course_id(self, id_root): :param course_blocks: the current list of blocks. :param category: """ - existing_uses = self.course_index.find({"_id": {"$regex": id_root}}) + existing_uses = self.db_connection.find_matching_course_indexes({"_id": {"$regex": id_root}}) if existing_uses.count() > 0: max_found = 0 matcher = re.compile(id_root + r'(\d+)') @@ -779,11 +747,11 @@ def create_item( parent['edit_info']['update_version'] = new_id if continue_version: # db update - self.structures.update({'_id': new_id}, new_structure) + self.db_connection.update_structure(new_structure) # clear cache so things get refetched and inheritance recomputed self._clear_cache(new_id) else: - self.structures.insert(new_structure) + self.db_connection.insert_structure(new_structure) # update the index entry if appropriate if index_entry is not None: @@ -856,7 +824,7 @@ def create_course( 'original_version': definition_id, } } - self.definitions.insert(definition_entry) + self.db_connection.insert_definition(definition_entry) new_id = ObjectId() draft_structure = { @@ -880,7 +848,7 @@ def create_course( } } } - self.structures.insert(draft_structure) + self.db_connection.insert_structure(draft_structure) if versions_dict is None: versions_dict = {master_branch: new_id} @@ -898,20 +866,20 @@ def create_course( if block_fields is not None: root_block['fields'].update(block_fields) if definition_fields is not None: - definition = self.definitions.find_one({'_id': root_block['definition']}) + definition = self.db_connection.get_definition(root_block['definition']) definition['fields'].update(definition_fields) definition['edit_info']['previous_version'] = definition['_id'] definition['edit_info']['edited_by'] = user_id definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) definition['_id'] = ObjectId() - self.definitions.insert(definition) + self.db_connection.insert_definition(definition) root_block['definition'] = definition['_id'] root_block['edit_info']['edited_on'] = datetime.datetime.now(UTC) root_block['edit_info']['edited_by'] = user_id root_block['edit_info']['previous_version'] = root_block['edit_info'].get('update_version') root_block['edit_info']['update_version'] = new_id - self.structures.insert(draft_structure) + self.db_connection.insert_structure(draft_structure) versions_dict[master_branch] = new_id # create the index entry @@ -926,7 +894,7 @@ def create_course( 'edited_by': user_id, 'edited_on': datetime.datetime.now(UTC), 'versions': versions_dict} - self.course_index.insert(index_entry) + self.db_connection.insert_course_index(index_entry) return self.get_course(CourseLocator(course_id=new_id, branch=master_branch)) def update_item(self, descriptor, user_id, force=False): @@ -978,7 +946,7 @@ def update_item(self, descriptor, user_id, force=False): 'previous_version': block_data['edit_info']['update_version'], 'update_version': new_id, } - self.structures.insert(new_structure) + self.db_connection.insert_structure(new_structure) # update the index entry if appropriate if index_entry is not None: self._update_head(index_entry, descriptor.location.branch, new_id) @@ -1016,7 +984,7 @@ def persist_xblock_dag(self, xblock, user_id, force=False): is_updated = self._persist_subdag(xblock, user_id, new_structure['blocks'], new_id) if is_updated: - self.structures.insert(new_structure) + self.db_connection.insert_structure(new_structure) # update the index entry if appropriate if index_entry is not None: @@ -1115,31 +1083,18 @@ def update_metadata(self, location, metadata): '''Deprecated, use update_item.''' raise NotImplementedError('use update_item') - def update_course_index(self, course_locator, new_values_dict, update_versions=False): + def update_course_index(self, updated_index_entry): """ - Change the given course's index entry for the given fields. new_values_dict - should be a subset of the dict returned by get_course_index_info. - It cannot include '_id' (will raise IllegalArgument). - Provide update_versions=True if you intend this to replace the versions hash. - Note, this operation can be dangerous and break running courses. + Change the given course's index entry. - If the dict includes versions and not update_versions, it will raise an exception. - - If the dict includes edited_on or edited_by, it will raise an exception + Note, this operation can be dangerous and break running courses. Does not return anything useful. """ # TODO how should this log the change? edited_on and edited_by for this entry # has the semantic of who created the course and when; so, changing those will lose # that information. - if '_id' in new_values_dict: - raise ValueError("Cannot override _id") - if 'edited_on' in new_values_dict or 'edited_by' in new_values_dict: - raise ValueError("Cannot set edited_on or edited_by") - if not update_versions and 'versions' in new_values_dict: - raise ValueError("Cannot override versions without setting update_versions") - self.course_index.update({'_id': course_locator.course_id}, - {'$set': new_values_dict}) + self.db_connection.update_course_index(updated_index_entry) def delete_item(self, usage_locator, user_id, delete_children=False, force=False): """ @@ -1182,7 +1137,7 @@ def remove_subtree(usage_id): remove_subtree(usage_locator.usage_id) # update index if appropriate and structures - self.structures.insert(new_structure) + self.db_connection.insert_structure(new_structure) result = CourseLocator(version_guid=new_id) @@ -1204,11 +1159,11 @@ def delete_course(self, course_id): :param course_id: uses course_id rather than locator to emphasize its global effect """ - index = self.course_index.find_one({'_id': course_id}) + index = self.db_connection.get_course_index(course_id) if index is None: raise ItemNotFoundError(course_id) # this is the only real delete in the system. should it do something else? - self.course_index.remove(index['_id']) + self.db_connection.delete_course_index(index['_id']) def get_errored_courses(self): """ @@ -1296,7 +1251,7 @@ def internal_clean_children(self, course_locator): block['fields']["children"] = [ usage_id for usage_id in block['fields']["children"] if usage_id in original_structure['blocks'] ] - self.structures.update({'_id': original_structure['_id']}, original_structure) + self.db_connection.update_structure(original_structure) # clear cache again b/c inheritance may be wrong over orphans self._clear_cache(original_structure['_id']) @@ -1379,7 +1334,7 @@ def _get_index_if_valid(self, locator, force=False, continue_version=False): else: return None else: - index_entry = self.course_index.find_one({'_id': locator.course_id}) + index_entry = self.db_connection.get_course_index(locator.course_id) is_head = ( locator.version_guid is None or index_entry['versions'][locator.branch] == locator.version_guid @@ -1424,9 +1379,8 @@ def _update_head(self, index_entry, branch, new_id): :param course_locator: :param new_id: """ - self.course_index.update( - {"_id": index_entry["_id"]}, - {"$set": {"versions.{}".format(branch): new_id}}) + index_entry['versions'][branch] = new_id + self.db_connection.update_course_index(index_entry) def _partition_fields_by_scope(self, category, fields): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py index 049fbd2ef800..74975f28961b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py @@ -59,9 +59,9 @@ def tearDown(self): dbref = self.loc_mapper.db dbref.drop_collection(self.loc_mapper.location_map) split_db = self.split_mongo.db - split_db.drop_collection(split_db.course_index) - split_db.drop_collection(split_db.structures) - split_db.drop_collection(split_db.definitions) + split_db.drop_collection(self.split_mongo.db_connection.course_index) + split_db.drop_collection(self.split_mongo.db_connection.structures) + split_db.drop_collection(self.split_mongo.db_connection.definitions) # old_mongo doesn't give a db attr, but all of the dbs are the same dbref.drop_collection(self.old_mongo.collection) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index aa095d8e4c4a..92de35e39f78 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -1018,41 +1018,29 @@ def test_update_course_index(self): Test changing the org, pretty id, etc of a course. Test that it doesn't allow changing the id, etc. """ locator = CourseLocator(course_id="GreekHero", branch='draft') - modulestore().update_course_index(locator, {'org': 'funkyU'}) + course_info = modulestore().get_course_index_info(locator) + course_info['org'] = 'funkyU' + modulestore().update_course_index(course_info) course_info = modulestore().get_course_index_info(locator) self.assertEqual(course_info['org'], 'funkyU') - modulestore().update_course_index(locator, {'org': 'moreFunky', 'prettyid': 'Ancient Greek Demagods'}) + course_info['org'] = 'moreFunky' + course_info['prettyid'] = 'Ancient Greek Demagods' + modulestore().update_course_index(course_info) course_info = modulestore().get_course_index_info(locator) self.assertEqual(course_info['org'], 'moreFunky') self.assertEqual(course_info['prettyid'], 'Ancient Greek Demagods') - self.assertRaises(ValueError, modulestore().update_course_index, locator, {'_id': 'funkygreeks'}) - - with self.assertRaises(ValueError): - modulestore().update_course_index( - locator, - {'edited_on': datetime.datetime.now(UTC)} - ) - with self.assertRaises(ValueError): - modulestore().update_course_index( - locator, - {'edited_by': 'sneak'} - ) - - self.assertRaises(ValueError, modulestore().update_course_index, locator, - {'versions': {'draft': self.GUID_D1}}) - # an allowed but not necessarily recommended way to revert the draft version versions = course_info['versions'] versions['draft'] = self.GUID_D1 - modulestore().update_course_index(locator, {'versions': versions}, update_versions=True) + modulestore().update_course_index(course_info) course = modulestore().get_course(locator) self.assertEqual(str(course.location.version_guid), self.GUID_D1) # an allowed but not recommended way to publish a course versions['published'] = self.GUID_D1 - modulestore().update_course_index(locator, {'versions': versions}, update_versions=True) + modulestore().update_course_index(course_info) course = modulestore().get_course(CourseLocator(course_id=locator.course_id, branch="published")) self.assertEqual(str(course.location.version_guid), self.GUID_D1) @@ -1068,9 +1056,9 @@ def test_create_with_root(self): self.assertEqual(new_course.location.usage_id, 'top') self.assertEqual(new_course.category, 'chapter') # look at db to verify - db_structure = modulestore().structures.find_one({ - '_id': new_course.location.as_object_id(new_course.location.version_guid) - }) + db_structure = modulestore().db_connection.get_structure( + new_course.location.as_object_id(new_course.location.version_guid) + ) self.assertIsNotNone(db_structure, "Didn't find course") self.assertNotIn('course', db_structure['blocks']) self.assertIn('top', db_structure['blocks']) From 783e4b223fbd8c9837e39d16085b18b05bd6fb2c Mon Sep 17 00:00:00 2001 From: Zubair Afzal Date: Thu, 7 Nov 2013 17:08:58 +0500 Subject: [PATCH 19/87] Show error on invalid html in course handout edit + Added tests STUD-293 --- .../features/course-updates.feature | 14 ++++++ .../contentstore/features/course-updates.py | 29 ++++++++++++ .../coffee/spec/views/course_info_spec.coffee | 19 ++++++++ cms/static/js/views/course_info_handout.js | 46 ++++++++++++------- cms/static/js/views/course_info_helper.js | 5 +- .../js/course_info_handouts.underscore | 3 +- 6 files changed, 97 insertions(+), 19 deletions(-) diff --git a/cms/djangoapps/contentstore/features/course-updates.feature b/cms/djangoapps/contentstore/features/course-updates.feature index 6f24fba68c6d..152da9c3499f 100644 --- a/cms/djangoapps/contentstore/features/course-updates.feature +++ b/cms/djangoapps/contentstore/features/course-updates.feature @@ -76,3 +76,17 @@ Feature: CMS.Course updates Then I see the handout "/c4x/MITx/999/asset/modified.jpg" And when I reload the page Then I see the handout "/c4x/MITx/999/asset/modified.jpg" + + Scenario: Users cannot save handouts with bad html until edit or update it properly + Given I have opened a new course in Studio + And I go to the course updates page + When I modify the handout to "

        [LINK TEXT]

        " + Then I see the handout error text + And I see handout save button disabled + When I edit the handout to "

        home

        " + Then I see handout save button re-enabled + When I save handout edit + # Can only do partial text matches because of the quotes with in quotes (and regexp step matching). + Then I see the handout "https://www.google.com.pk/" + And when I reload the page + Then I see the handout "https://www.google.com.pk/" diff --git a/cms/djangoapps/contentstore/features/course-updates.py b/cms/djangoapps/contentstore/features/course-updates.py index da74f5aa4bb3..b41578c907c8 100644 --- a/cms/djangoapps/contentstore/features/course-updates.py +++ b/cms/djangoapps/contentstore/features/course-updates.py @@ -90,6 +90,35 @@ def check_handout(_step, handout): assert handout in world.css_html(handout_css) +@step(u'I see the handout error text') +def check_handout_error(_step): + handout_error_css = 'div#handout_error' + assert world.css_has_class(handout_error_css, 'is-shown') + + +@step(u'I see handout save button disabled') +def check_handout_error(_step): + handout_save_button = 'form.edit-handouts-form a.save-button' + assert world.css_has_class(handout_save_button, 'is-disabled') + + +@step(u'I edit the handout to "([^"]*)"$') +def edit_handouts(_step, text): + type_in_codemirror(0, text) + + +@step(u'I see handout save button re-enabled') +def check_handout_error(_step): + handout_save_button = 'form.edit-handouts-form a.save-button' + assert not world.css_has_class(handout_save_button, 'is-disabled') + + +@step(u'I save handout edit') +def check_handout_error(_step): + save_css = 'a.save-button' + world.css_click(save_css) + + def change_text(text): type_in_codemirror(0, text) save_css = 'a.save-button' diff --git a/cms/static/coffee/spec/views/course_info_spec.coffee b/cms/static/coffee/spec/views/course_info_spec.coffee index 3c388fa5936d..1e843d59fbb0 100644 --- a/cms/static/coffee/spec/views/course_info_spec.coffee +++ b/cms/static/coffee/spec/views/course_info_spec.coffee @@ -196,3 +196,22 @@ define ["js/views/course_info_handout", "js/views/course_info_update", "js/model @handoutsEdit.$el.find('.edit-button').click() expect(@handoutsEdit.$codeMirror.getValue().trim()).toEqual('/static/fromServer.jpg') + it "can open course handouts with bad html on edit", -> + # Enter some bad html in handouts section, verifying that the + # model/handoutform opens when "Edit" is clicked + + @model = new ModuleInfo({ + id: 'handouts-id', + data: '

        [LINK TEXT]

        ') + expect($('.edit-handouts-form').is(':hidden')).toEqual(false) \ No newline at end of file diff --git a/cms/static/js/views/course_info_handout.js b/cms/static/js/views/course_info_handout.js index f9804d03a445..9309deda1b47 100644 --- a/cms/static/js/views/course_info_handout.js +++ b/cms/static/js/views/course_info_handout.js @@ -30,6 +30,7 @@ define(["backbone", "underscore", "codemirror", "js/views/feedback_notification" model: this.model })) ); + $('.handouts-content').html(this.model.get('data')); this.$preview = this.$el.find('.handouts-content'); this.$form = this.$el.find(".edit-handouts-form"); this.$editor = this.$form.find('.handouts-content-editor'); @@ -50,32 +51,43 @@ define(["backbone", "underscore", "codemirror", "js/views/feedback_notification" }, onSave: function(event) { - this.model.set('data', this.$codeMirror.getValue()); - var saving = new NotificationView.Mini({ - title: gettext('Saving…') - }); - saving.show(); - this.model.save({}, { - success: function() { - saving.hide(); - } - }); - this.render(); - this.$form.hide(); - this.closeEditor(); - - analytics.track('Saved Course Handouts', { - 'course': course_location_analytics - }); + $('#handout_error').removeClass('is-shown'); + $('.save-button').removeClass('is-disabled'); + if ($('.CodeMirror-lines').find('.cm-error').length == 0){ + this.model.set('data', this.$codeMirror.getValue()); + var saving = new NotificationView.Mini({ + title: gettext('Saving…') + }); + saving.show(); + this.model.save({}, { + success: function() { + saving.hide(); + } + }); + this.render(); + this.$form.hide(); + this.closeEditor(); + analytics.track('Saved Course Handouts', { + 'course': course_location_analytics + }); + }else{ + $('#handout_error').addClass('is-shown'); + $('.save-button').addClass('is-disabled'); + event.preventDefault(); + } }, onCancel: function(event) { + $('#handout_error').removeClass('is-shown'); + $('.save-button').removeClass('is-disabled'); this.$form.hide(); this.closeEditor(); }, closeEditor: function() { + $('#handout_error').removeClass('is-shown'); + $('.save-button').removeClass('is-disabled'); this.$form.hide(); ModalUtils.hideModalCover(); this.$form.find('.CodeMirror').remove(); diff --git a/cms/static/js/views/course_info_helper.js b/cms/static/js/views/course_info_helper.js index ec4a6ba55075..fb3474cdb0ca 100644 --- a/cms/static/js/views/course_info_helper.js +++ b/cms/static/js/views/course_info_helper.js @@ -6,7 +6,10 @@ define(["codemirror", "utility"], var $codeMirror = CodeMirror.fromTextArea(textArea, { mode: "text/html", lineNumbers: true, - lineWrapping: true + lineWrapping: true, + onChange: function () { + $('.save-button').removeClass('is-disabled'); + } }); $codeMirror.setValue(content); $codeMirror.clearHistory(); diff --git a/cms/templates/js/course_info_handouts.underscore b/cms/templates/js/course_info_handouts.underscore index 7fbbe9bc33e4..6ce8518a32f7 100644 --- a/cms/templates/js/course_info_handouts.underscore +++ b/cms/templates/js/course_info_handouts.underscore @@ -3,12 +3,13 @@

        Course Handouts

        <%if (model.get('data') != null) { %>
        - <%= model.get('data') %> +
        <% } else {%>

        ${_("You have no handouts defined")}

        <% } %>
        +
        <%=gettext("There is invalid code in your content. Please check to make sure it is valid HTML.")%>
        From cc2f1e73bfd754cba2c6b875ce93df281d9d9780 Mon Sep 17 00:00:00 2001 From: polesye Date: Wed, 20 Nov 2013 15:48:15 +0200 Subject: [PATCH 20/87] BLD-524: Add missing assert in video test. --- cms/djangoapps/contentstore/features/video.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index 5408c482908c..c97dba10b929 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -181,7 +181,7 @@ def click_on_the_caption(_step, index): @step('I see caption line with data-index "([^"]*)" has class "([^"]*)"$') def caption_line_has_class(_step, index, className): SELECTOR = ".subtitles > li[data-index='{index}']".format(index=int(index.strip())) - world.css_has_class(SELECTOR, className.strip()) + assert world.css_has_class(SELECTOR, className.strip()) @step('I see a range on slider$') From f01b36b5d42923155a15ed7a86e78e125a4c4191 Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 18 Nov 2013 14:48:11 -0500 Subject: [PATCH 21/87] Test for i4x on returned pages. STUD-941 --- .../contentstore/tests/test_contentstore.py | 148 ++++++++++++++---- cms/static/js/models/course_info.js | 5 +- cms/templates/asset_index.html | 2 +- cms/templates/course_info.html | 1 - cms/templates/settings.html | 2 +- cms/templates/widgets/segment-io.html | 15 +- 6 files changed, 132 insertions(+), 41 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 2b773e991d50..34f15b6db80a 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1,6 +1,5 @@ #pylint: disable=E1101 -import json import shutil import mock @@ -15,6 +14,7 @@ import copy from json import loads from datetime import timedelta +from django.test import TestCase from django.contrib.auth.models import User from django.dispatch import Signal @@ -53,6 +53,7 @@ from uuid import uuid4 from pymongo import MongoClient from student.models import CourseEnrollment +import re from contentstore.utils import delete_course_and_groups from xmodule.modulestore.django import loc_mapper @@ -135,6 +136,8 @@ def check_components_on_page(self, component_types, expected_types): resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()})) self.assertEqual(resp.status_code, 200) + # TODO: uncomment after edit_unit no longer using locations. + # _test_no_locations(self, resp) for expected in expected_types: self.assertIn(expected, resp.content) @@ -160,15 +163,21 @@ def test_malformed_edit_unit_request(self): resp = self.client.get_html(reverse('edit_unit', kwargs={'location': location.url()})) self.assertEqual(resp.status_code, 400) + _test_no_locations(self, resp, status_code=400) def check_edit_unit(self, test_course_name): import_from_xml(modulestore('direct'), 'common/test/data/', [test_course_name]) - for descriptor in modulestore().get_items(Location(None, None, 'vertical', None, None)): + items = modulestore().get_items(Location('i4x', 'edX', test_course_name, 'vertical', None, None)) + # Assert is here to make sure that the course being tested actually has verticals. + self.assertGreater(len(items), 0) + for descriptor in items: print "Checking ", descriptor.location.url() print descriptor.__class__, descriptor.location resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()})) self.assertEqual(resp.status_code, 200) + # TODO: uncomment after edit_unit not using locations. + # _test_no_locations(self, resp) def lockAnAsset(self, content_store, course_location): """ @@ -483,6 +492,8 @@ def _test_preview(self, location): ) resp = self.client.get_html(locator.url_reverse('xblock')) self.assertEqual(resp.status_code, 200) + # TODO: uncomment when preview no longer has locations being returned. + # _test_no_locations(self, resp) return resp def test_delete(self): @@ -841,6 +852,7 @@ def test_illegal_draft_crud_ops(self): def test_bad_contentstore_request(self): resp = self.client.get_html('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png') self.assertEqual(resp.status_code, 400) + _test_no_locations(self, resp, 400) def test_rewrite_nonportable_links_on_import(self): module_store = modulestore('direct') @@ -1026,12 +1038,11 @@ def check_import(self, module_store, root_dir, draft_store, content_store, stub_ items = module_store.get_items(stub_location.replace(category='vertical', name=None)) self.assertGreater(len(items), 0) for descriptor in items: - # don't try to look at private verticals. Right now we're running - # the service in non-draft aware - if getattr(descriptor, 'is_draft', False): - print "Checking {0}....".format(descriptor.location.url()) - resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()})) - self.assertEqual(resp.status_code, 200) + print "Checking {0}....".format(descriptor.location.url()) + resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()})) + self.assertEqual(resp.status_code, 200) + # TODO: uncomment when edit_unit no longer has locations. + # _test_no_locations(self, resp) # verify that we have the content in the draft store as well vertical = draft_store.get_item( @@ -1508,6 +1519,7 @@ def test_course_index_view_with_no_courses(self): status_code=200, html=True ) + _test_no_locations(self, resp) def test_course_factory(self): """Test that the course factory works correctly.""" @@ -1530,6 +1542,8 @@ def test_course_index_view_with_course(self): status_code=200, html=True ) + # TODO: uncomment when course index no longer has locations being returned. + # _test_no_locations(self, resp) def test_course_overview_view_with_course(self): """Test viewing the course overview page with an existing course""" @@ -1589,6 +1603,13 @@ def test_cms_imported_course_walkthrough(self): Import and walk through some common URL endpoints. This just verifies non-500 and no other correct behavior, so it is not a deep test """ + def test_get_html(page): + # Helper function for getting HTML for a page in Studio and + # checking that it does not error. + resp = self.client.get_html(new_location.url_reverse(page)) + self.assertEqual(resp.status_code, 200) + _test_no_locations(self, resp) + import_from_xml(modulestore('direct'), 'common/test/data/', ['simple']) loc = Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None]) new_location = loc_mapper().translate_location(loc.course_id, loc, False, True) @@ -1598,42 +1619,46 @@ def test_cms_imported_course_walkthrough(self): self.assertContains(resp, 'Chapter 2') # go to various pages - - # import page - resp = self.client.get_html(new_location.url_reverse('import/', '')) - self.assertEqual(resp.status_code, 200) - - # export page - resp = self.client.get_html(new_location.url_reverse('export/', '')) - self.assertEqual(resp.status_code, 200) - - # course team - url = new_location.url_reverse('course_team/', '') - resp = self.client.get_html(url) - self.assertEqual(resp.status_code, 200) - - # course info - resp = self.client.get(new_location.url_reverse('course_info')) - self.assertEqual(resp.status_code, 200) + test_get_html('import') + test_get_html('export') + test_get_html('course_team') + test_get_html('course_info') + test_get_html('checklists') + test_get_html('assets') # settings_details - resp = self.client.get(reverse('settings_details', + resp = self.client.get_html(reverse('settings_details', kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) self.assertEqual(resp.status_code, 200) + _test_no_locations(self, resp) # settings_details - resp = self.client.get(reverse('settings_grading', + resp = self.client.get_html(reverse('settings_grading', kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) self.assertEqual(resp.status_code, 200) - - # assets_handler (HTML for full page content) - url = new_location.url_reverse('assets/', '') - resp = self.client.get_html(url) + # TODO: uncomment when grading is not using old locations. + # _test_no_locations(self, resp) + + # advanced settings + resp = self.client.get_html(reverse('course_advanced_settings', + kwargs={'org': loc.org, + 'course': loc.course, + 'name': loc.name})) self.assertEqual(resp.status_code, 200) + # TODO: uncomment when advanced settings not using old locations. + # _test_no_locations(self, resp) + + # textbook index + resp = self.client.get_html(reverse('textbook_index', + kwargs={'org': loc.org, + 'course': loc.course, + 'name': loc.name})) + self.assertEqual(resp.status_code, 200) + _test_no_locations(self, resp) # go look at a subsection page subsection_location = loc.replace(category='sequential', name='test_sequence') @@ -1641,12 +1666,23 @@ def test_cms_imported_course_walkthrough(self): reverse('edit_subsection', kwargs={'location': subsection_location.url()}) ) self.assertEqual(resp.status_code, 200) + # TODO: uncomment when grading and outline not using old locations. + # _test_no_locations(self, resp) # go look at the Edit page unit_location = loc.replace(category='vertical', name='test_vertical') resp = self.client.get_html( reverse('edit_unit', kwargs={'location': unit_location.url()})) self.assertEqual(resp.status_code, 200) + # TODO: uncomment when edit_unit not using old locations. + # _test_no_locations(self, resp) + + resp = self.client.get_html(reverse('edit_tabs', + kwargs={'org': loc.org, + 'course': loc.course, + 'coursename': loc.name})) + self.assertEqual(resp.status_code, 200) + _test_no_locations(self, resp) def delete_item(category, name): """ Helper method for testing the deletion of an xblock item. """ @@ -1654,6 +1690,7 @@ def delete_item(category, name): del_location = loc_mapper().translate_location(loc.course_id, del_loc, False, True) resp = self.client.delete(del_location.url_reverse('xblock')) self.assertEqual(resp.status_code, 204) + _test_no_locations(self, resp, status_code=204, html=False) # delete a component delete_item(category='html', name='test_html') @@ -1853,7 +1890,10 @@ def _show_course_overview(self, location): Show the course overview page. """ new_location = loc_mapper().translate_location(location.course_id, location, False, True) - return self.client.get_html(new_location.url_reverse('course/', '')) + resp = self.client.get_html(new_location.url_reverse('course/', '')) + # TODO: uncomment when i4x no longer in overview. + # _test_no_locations(self, resp) + return resp @override_settings(MODULESTORE=TEST_MODULESTORE) @@ -1920,6 +1960,32 @@ def test_metadata_persistence(self): pass +class EntryPageTestCase(TestCase): + """ + Tests entry pages that aren't specific to a course. + """ + def setUp(self): + self.client = AjaxEnabledTestClient() + + def _test_page(self, page, status_code=200): + resp = self.client.get_html(reverse(page)) + self.assertEqual(resp.status_code, status_code) + _test_no_locations(self, resp, status_code) + + def test_how_it_works(self): + self._test_page("howitworks") + + def test_signup(self): + self._test_page("signup") + + def test_login(self): + self._test_page("login") + + def test_logout(self): + # Logout redirects. + self._test_page("logout", 302) + + def _create_course(test, course_data): """ Creates a course via an AJAX request and verifies the URL returned in the response. @@ -1945,3 +2011,21 @@ def _course_factory_create_course(): def _get_course_id(test_course_data): """Returns the course ID (org/number/run).""" return "{org}/{number}/{run}".format(**test_course_data) + + +def _test_no_locations(test, resp, status_code=200, html=True): + """ + Verifies that "i4x", which appears in old locations, but not + new locators, does not appear in the HTML response output. + Used to verify that database refactoring is complete. + """ + test.assertNotContains(resp, 'i4x', status_code=status_code, html=html) + if html: + # For HTML pages, it is nice to call the method with html=True because + # it checks that the HTML properly parses. However, it won't find i4x usages + # in JavaScript blocks. + content = resp.content + num_jump_to = len(re.findall(r"8000(\S)*jump_to/i4x", content)) + total_i4x = len(re.findall(r"i4x", content)) + + test.assertEqual(total_i4x - num_jump_to, 0, "i4x found outside of LMS jump-to links") diff --git a/cms/static/js/models/course_info.js b/cms/static/js/models/course_info.js index e5a6114dff08..e4c816ccf3e1 100644 --- a/cms/static/js/models/course_info.js +++ b/cms/static/js/models/course_info.js @@ -5,12 +5,9 @@ define(["backbone"], function(Backbone) { url: '', defaults: { - "courseId": "", // the location url "updates" : null, // UpdateCollection "handouts": null // HandoutCollection - }, - - idAttribute : "courseId" + } }); return CourseInfo; }); diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index 5576664df7f3..4f6f14a4669c 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -187,7 +187,7 @@

        ${_("What can I do on this page?")}

        ${_('close')}

        ${_("Upload New File")}

        -

        +

        diff --git a/cms/templates/course_info.html b/cms/templates/course_info.html index 401d32ccb21f..8cddb8a8c569 100644 --- a/cms/templates/course_info.html +++ b/cms/templates/course_info.html @@ -33,7 +33,6 @@ var editor = new CourseInfoEditView({ el: $('.main-wrapper'), model : new CourseInfoModel({ - courseId : '${context_course.location}', updates : course_updates, base_asset_url : '${base_asset_url}', handouts : course_handouts diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 33a5360da051..ca60bb2e90b7 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -87,7 +87,7 @@

        ${_("Basic Information")}

        ${_("Course Summary Page")} ${_("(for student enrollment and access)")}

        -

        https:${utils.get_lms_link_for_about_page(course_location)}

        +

        https:${utils.get_lms_link_for_about_page(course_location)}

          diff --git a/cms/templates/widgets/segment-io.html b/cms/templates/widgets/segment-io.html index a6a186b2a3c2..d250e9c23b5d 100644 --- a/cms/templates/widgets/segment-io.html +++ b/cms/templates/widgets/segment-io.html @@ -1,9 +1,20 @@ +<%! +from xmodule.modulestore.django import loc_mapper +%> + +% if context_course: +<% + ctx_loc = context_course.location + locator = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True) +%> +% endif + % if settings.MITX_FEATURES.get('SEGMENT_IO'): - + But in this there should be
          Great ideas require offsetting. - + bad tests require drivel
          - + [code] Code should be nicely monospaced. [/code] @@ -322,7 +363,7 @@ describe 'MarkdownEditingDescriptor', -> expect(data).toEqual("""

          Not a header

          A header

          - +

          Multiple choice w/ parentheticals

          @@ -332,7 +373,7 @@ describe 'MarkdownEditingDescriptor', -> no space b4 close paren - +

          Choice checks

          @@ -343,25 +384,25 @@ describe 'MarkdownEditingDescriptor', -> no space - +

          Option with multiple correct ones

          - + - +

          Option with embedded parens

          - + - +

          What happens w/ empty correct options?

          - + - +

          Explanation

          @@ -379,14 +420,14 @@ describe 'MarkdownEditingDescriptor', -> console.log(two * 2); - +

          But in this there should be

          Great ideas require offsetting.

          - +

          bad tests require drivel

          - + 
          
         Code should be nicely monospaced.
          diff --git a/common/lib/xmodule/xmodule/js/src/problem/edit.coffee b/common/lib/xmodule/xmodule/js/src/problem/edit.coffee index ba85db574b00..9f5d36100949 100644 --- a/common/lib/xmodule/xmodule/js/src/problem/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/problem/edit.coffee @@ -228,11 +228,13 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor }); // replace string and numerical - xml = xml.replace(/^\=\s*(.*?$)/gm, function(match, p) { - var string; - var floatValue = parseFloat(p); + xml = xml.replace(/(^\=\s*(.*?$)(\n*or\=\s*(.*?$))*)+/gm, function(match, p) { + var string, + answersList = p.replace(/^(or)?=\s*/gm, '').split('\n'), + floatValue = parseFloat(answersList[0]); + if(!isNaN(floatValue)) { - var params = /(.*?)\+\-\s*(.*?$)/.exec(p); + var params = /(.*?)\+\-\s*(.*?$)/.exec(answersList[0]); if(params) { string = '\n'; string += ' \n'; @@ -242,10 +244,16 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor string += ' \n'; string += '\n\n'; } else { - string = '\n \n\n\n'; + var answers = []; + + for(var i = 0; i < answersList.length; i++) { + answers.push(answersList[i]) + } + + string = '\n \n\n\n'; } return string; - }); + }); // replace selects xml = xml.replace(/\[\[(.+?)\]\]/g, function(match, p) { @@ -262,13 +270,13 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor selectString += '\n\n'; return selectString; }); - + // replace explanations xml = xml.replace(/\[explanation\]\n?([^\]]*)\[\/?explanation\]/gmi, function(match, p1) { var selectString = '\n
          \nExplanation\n\n' + p1 + '\n
          \n          '; return selectString; }); - + // replace code blocks xml = xml.replace(/\[code\]\n?([^\]]*)\[\/?code\]/gmi, function(match, p1) { var selectString = '
          \n' + p1 + '
          '; @@ -293,7 +301,7 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor // rid white space xml = xml.replace(/\n\n\n/g, '\n'); - + // surround w/ problem tag xml = '\n' + xml + '\n'; From 1b2be30c45aeb434728216c26d7870c85399a19e Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 14 Nov 2013 15:45:16 -0500 Subject: [PATCH 32/87] Convert tabs to RESTful URL. STUD-850 --- .../features/static-pages.feature | 19 +- .../contentstore/features/static-pages.py | 44 ++++ .../contentstore/tests/test_contentstore.py | 94 +++++--- cms/djangoapps/contentstore/views/tabs.py | 208 +++++++++--------- cms/static/coffee/spec/main.coffee | 3 +- cms/static/coffee/src/views/tabs.coffee | 7 +- cms/static/js/models/explicit_url.js | 14 ++ .../js/spec/models/explicit_url_spec.js | 12 + cms/templates/edit-tabs.html | 11 +- cms/templates/widgets/header.html | 17 +- cms/urls.py | 5 +- 11 files changed, 271 insertions(+), 163 deletions(-) create mode 100644 cms/static/js/models/explicit_url.js create mode 100644 cms/static/js/spec/models/explicit_url_spec.js diff --git a/cms/djangoapps/contentstore/features/static-pages.feature b/cms/djangoapps/contentstore/features/static-pages.feature index 54d23d985de2..39399fb20725 100644 --- a/cms/djangoapps/contentstore/features/static-pages.feature +++ b/cms/djangoapps/contentstore/features/static-pages.feature @@ -9,10 +9,8 @@ Feature: CMS.Static Pages Then I should see a static page named "Empty" Scenario: Users can delete static pages - Given I have opened a new course in Studio - And I go to the static pages page - And I add a new page - And I "delete" the static page + Given I have created a static page + When I "delete" the static page Then I am shown a prompt When I confirm the prompt Then I should not see any static pages @@ -20,9 +18,16 @@ Feature: CMS.Static Pages # Safari won't update the name properly @skip_safari Scenario: Users can edit static pages - Given I have opened a new course in Studio - And I go to the static pages page - And I add a new page + Given I have created a static page When I "edit" the static page And I change the name to "New" Then I should see a static page named "New" + + # Safari won't update the name properly + @skip_safari + Scenario: Users can reorder static pages + Given I have created two different static pages + When I reorder the tabs + Then the tabs are in the reverse order + And I reload the page + Then the tabs are in the reverse order diff --git a/cms/djangoapps/contentstore/features/static-pages.py b/cms/djangoapps/contentstore/features/static-pages.py index 58932ad8e2bc..0adb4b1e54fa 100644 --- a/cms/djangoapps/contentstore/features/static-pages.py +++ b/cms/djangoapps/contentstore/features/static-pages.py @@ -48,3 +48,47 @@ def change_name(step, new_name): world.trigger_event(input_css) save_button = 'a.save-button' world.css_click(save_button) + + +@step(u'I reorder the tabs') +def reorder_tabs(_step): + # For some reason, the drag_and_drop method did not work in this case. + draggables = world.css_find('.drag-handle') + source = draggables.first + target = draggables.last + source.action_chains.click_and_hold(source._element).perform() + source.action_chains.move_to_element_with_offset(target._element, 0, 50).perform() + source.action_chains.release().perform() + + +@step(u'I have created a static page') +def create_static_page(step): + step.given('I have opened a new course in Studio') + step.given('I go to the static pages page') + step.given('I add a new page') + + +@step(u'I have created two different static pages') +def create_two_pages(step): + step.given('I have created a static page') + step.given('I "edit" the static page') + step.given('I change the name to "First"') + step.given('I add a new page') + # Verify order of tabs + _verify_tab_names('First', 'Empty') + + +@step(u'the tabs are in the reverse order') +def tabs_in_reverse_order(step): + _verify_tab_names('Empty', 'First') + + +def _verify_tab_names(first, second): + world.wait_for( + func=lambda _: len(world.css_find('.xmodule_StaticTabModule')) == 2, + timeout=200, + timeout_msg="Timed out waiting for two tabs to be present" + ) + tabs = world.css_find('.xmodule_StaticTabModule') + assert tabs[0].text == first + assert tabs[1].text == second diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 34f15b6db80a..cc0e50fbf6bf 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -179,7 +179,7 @@ def check_edit_unit(self, test_course_name): # TODO: uncomment after edit_unit not using locations. # _test_no_locations(self, resp) - def lockAnAsset(self, content_store, course_location): + def _lock_an_asset(self, content_store, course_location): """ Lock an arbitrary asset in the course :param course_location: @@ -407,24 +407,7 @@ def test_create_static_tab_and_rename(self): self.assertEqual(course.tabs, expected_tabs) def test_static_tab_reordering(self): - def get_tab_locator(tab): - tab_location = 'i4x://MITx/999/static_tab/{0}'.format(tab['url_slug']) - return unicode(loc_mapper().translate_location( - course.location.course_id, Location(tab_location), False, True - )) - - module_store = modulestore('direct') - locator = _course_factory_create_course() - course_location = loc_mapper().translate_locator_to_location(locator) - - ItemFactory.create( - parent_location=course_location, - category="static_tab", - display_name="Static_1") - ItemFactory.create( - parent_location=course_location, - category="static_tab", - display_name="Static_2") + module_store, course_location, new_location = self._create_static_tabs() course = module_store.get_item(course_location) @@ -432,9 +415,9 @@ def get_tab_locator(tab): reverse_tabs = [] for tab in course.tabs: if tab['type'] == 'static_tab': - reverse_tabs.insert(0, get_tab_locator(tab)) + reverse_tabs.insert(0, unicode(self._get_tab_locator(course, tab))) - self.client.ajax_post(reverse('reorder_static_tabs'), {'tabs': reverse_tabs}) + self.client.ajax_post(new_location.url_reverse('tabs'), {'tabs': reverse_tabs}) course = module_store.get_item(course_location) @@ -442,10 +425,57 @@ def get_tab_locator(tab): course_tabs = [] for tab in course.tabs: if tab['type'] == 'static_tab': - course_tabs.append(get_tab_locator(tab)) + course_tabs.append(unicode(self._get_tab_locator(course, tab))) self.assertEqual(reverse_tabs, course_tabs) + def test_static_tab_deletion(self): + module_store, course_location, _ = self._create_static_tabs() + + course = module_store.get_item(course_location) + num_tabs = len(course.tabs) + last_tab = course.tabs[num_tabs - 1] + url_slug = last_tab['url_slug'] + delete_url = self._get_tab_locator(course, last_tab).url_reverse('xblock') + + self.client.delete(delete_url) + + course = module_store.get_item(course_location) + self.assertEqual(num_tabs - 1, len(course.tabs)) + + def tab_matches(tab): + """ Checks if the tab matches the one we deleted """ + return tab['type'] == 'static_tab' and tab['url_slug'] == url_slug + + tab_found = any(tab_matches(tab) for tab in course.tabs) + + self.assertFalse(tab_found, "tab should have been deleted") + + def _get_tab_locator(self, course, tab): + """ Returns the locator for a given tab. """ + tab_location = 'i4x://MITx/999/static_tab/{0}'.format(tab['url_slug']) + return loc_mapper().translate_location( + course.location.course_id, Location(tab_location), False, True + ) + + def _create_static_tabs(self): + """ Creates two static tabs in a dummy course. """ + module_store = modulestore('direct') + CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') + course_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) + new_location = loc_mapper().translate_location(course_location.course_id, course_location, False, True) + + ItemFactory.create( + parent_location=course_location, + category="static_tab", + display_name="Static_1") + ItemFactory.create( + parent_location=course_location, + category="static_tab", + display_name="Static_2") + + return module_store, course_location, new_location + def test_import_polls(self): module_store = modulestore('direct') import_from_xml(module_store, 'common/test/data/', ['toy']) @@ -633,7 +663,7 @@ def test_asset_delete_and_restore(self): thumbnail = content_store.find(thumbnail_location, throw_on_not_found=False) self.assertIsNotNone(thumbnail) - def _delete_asset_in_course (self): + def _delete_asset_in_course(self): """ Helper method for: 1) importing course from xml @@ -972,7 +1002,7 @@ def test_export_course(self, mock_get): self.assertIn(private_location_no_draft.url(), sequential.children) - locked_asset = self.lockAnAsset(content_store, location) + locked_asset = self._lock_an_asset(content_store, location) locked_asset_attrs = content_store.get_attrs(locked_asset) # the later import will reupload del locked_asset_attrs['uploadDate'] @@ -1027,7 +1057,7 @@ def test_export_course(self, mock_get): shutil.rmtree(root_dir) def check_import(self, module_store, root_dir, draft_store, content_store, stub_location, course_location, - locked_asset, locked_asset_attrs): + locked_asset, locked_asset_attrs): # reimport import_from_xml( module_store, root_dir, ['test_export'], draft_store=draft_store, @@ -1226,7 +1256,7 @@ def test_course_handouts_rewrites(self): handouts_locator = loc_mapper().translate_location('edX/toy/2012_Fall', handout_location) # get module info (json) - resp = self.client.get(handouts_locator.url_reverse('/xblock', '')) + resp = self.client.get(handouts_locator.url_reverse('/xblock')) # make sure we got a successful response self.assertEqual(resp.status_code, 200) @@ -1403,7 +1433,7 @@ def test_forum_unseeding_with_multiple_courses(self): second_course_data = self.assert_created_course(number_suffix=uuid4().hex) # unseed the forums for the first course - course_id =_get_course_id(test_course_data) + course_id = _get_course_id(test_course_data) delete_course_and_groups(course_id, commit=True) self.assertFalse(are_permissions_roles_seeded(course_id)) @@ -1625,6 +1655,7 @@ def test_get_html(page): test_get_html('course_info') test_get_html('checklists') test_get_html('assets') + test_get_html('tabs') # settings_details resp = self.client.get_html(reverse('settings_details', @@ -1677,13 +1708,6 @@ def test_get_html(page): # TODO: uncomment when edit_unit not using old locations. # _test_no_locations(self, resp) - resp = self.client.get_html(reverse('edit_tabs', - kwargs={'org': loc.org, - 'course': loc.course, - 'coursename': loc.name})) - self.assertEqual(resp.status_code, 200) - _test_no_locations(self, resp) - def delete_item(category, name): """ Helper method for testing the deletion of an xblock item. """ del_loc = loc.replace(category=category, name=name) @@ -1997,7 +2021,7 @@ def _create_course(test, course_data): test.assertEqual(response.status_code, 200) data = parse_json(response) test.assertNotIn('ErrMsg', data) - test.assertEqual(data['url'], new_location.url_reverse("course/", "")) + test.assertEqual(data['url'], new_location.url_reverse("course")) def _course_factory_create_course(): diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 40ea9bfd6bf3..46791ddc26f3 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -2,12 +2,13 @@ Views related to course tabs """ from access import has_access -from util.json_request import expect_json +from util.json_request import expect_json, JsonResponse -from django.http import HttpResponse, HttpResponseBadRequest +from django.http import HttpResponseNotFound from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django_future.csrf import ensure_csrf_cookie +from django.views.decorators.http import require_http_methods from mitxmako.shortcuts import render_to_response from xmodule.modulestore import Location from xmodule.modulestore.inheritance import own_metadata @@ -19,7 +20,7 @@ from django.utils.translation import ugettext as _ -__all__ = ['edit_tabs', 'reorder_static_tabs'] +__all__ = ['tabs_handler'] def initialize_course_tabs(course): @@ -43,104 +44,113 @@ def initialize_course_tabs(course): modulestore('direct').update_metadata(course.location.url(), own_metadata(course)) - -@login_required @expect_json -def reorder_static_tabs(request): - "Order the static tabs in the requested order" - def get_location_for_tab(tab): - tab_locator = BlockUsageLocator(tab) - return loc_mapper().translate_locator_to_location(tab_locator) - - tabs = request.json['tabs'] - course_location = loc_mapper().translate_locator_to_location(BlockUsageLocator(tabs[0]), get_course=True) - - if not has_access(request.user, course_location): - raise PermissionDenied() - - course = get_modulestore(course_location).get_item(course_location) - - # get list of existing static tabs in course - # make sure they are the same lengths (i.e. the number of passed in tabs equals the number - # that we know about) otherwise we can drop some! - - existing_static_tabs = [t for t in course.tabs if t['type'] == 'static_tab'] - if len(existing_static_tabs) != len(tabs): - return HttpResponseBadRequest() - - # load all reference tabs, return BadRequest if we can't find any of them - tab_items = [] - for tab in tabs: - item = modulestore('direct').get_item(get_location_for_tab(tab)) - if item is None: - return HttpResponseBadRequest() - - tab_items.append(item) - - # now just go through the existing course_tabs and re-order the static tabs - reordered_tabs = [] - static_tab_idx = 0 - for tab in course.tabs: - if tab['type'] == 'static_tab': - reordered_tabs.append({'type': 'static_tab', - 'name': tab_items[static_tab_idx].display_name, - 'url_slug': tab_items[static_tab_idx].location.name}) - static_tab_idx += 1 - else: - reordered_tabs.append(tab) - - # OK, re-assemble the static tabs in the new order - course.tabs = reordered_tabs - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - course.save() - modulestore('direct').update_metadata(course.location, own_metadata(course)) - # TODO: above two lines are used for the primitive-save case. Maybe factor them out? - return HttpResponse() - - @login_required @ensure_csrf_cookie -def edit_tabs(request, org, course, coursename): - "Edit tabs" - location = ['i4x', org, course, 'course', coursename] - store = get_modulestore(location) - course_item = store.get_item(location) - - # check that logged in user has permissions to this item - if not has_access(request.user, location): - raise PermissionDenied() - - # see tabs have been uninitialized (e.g. supporing courses created before tab support in studio) - if course_item.tabs is None or len(course_item.tabs) == 0: - initialize_course_tabs(course_item) - - # first get all static tabs from the tabs list - # we do this because this is also the order in which items are displayed in the LMS - static_tabs_refs = [t for t in course_item.tabs if t['type'] == 'static_tab'] +@require_http_methods(("GET", "POST", "PUT")) +def tabs_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None): + """ + The restful handler for static tabs. - static_tabs = [] - for static_tab_ref in static_tabs_refs: - static_tab_loc = Location(location)._replace(category='static_tab', name=static_tab_ref['url_slug']) - static_tabs.append(modulestore('direct').get_item(static_tab_loc)) + GET + html: return page for editing static tabs + json: not supported + PUT or POST + json: update the tab order. It is expected that the request body contains a JSON-encoded dict with entry "tabs". + The value for "tabs" is an array of tab locators, indicating the desired order of the tabs. - components = [ - loc_mapper().translate_location( - course_item.location.course_id, static_tab.location, False, True - ) - for static_tab - in static_tabs - ] + Creating a tab, deleting a tab, or changing its contents is not supported through this method. + Instead use the general xblock URL (see item.xblock_handler). + """ + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, locator): + raise PermissionDenied() - course_locator = loc_mapper().translate_location( - course_item.location.course_id, course_item.location, False, True - ) + old_location = loc_mapper().translate_locator_to_location(locator) + store = get_modulestore(old_location) + course_item = store.get_item(old_location) - return render_to_response('edit-tabs.html', { - 'context_course': course_item, - 'components': components, - 'locator': course_locator - }) + if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): + if request.method == 'GET': + raise NotImplementedError('coming soon') + else: + if 'tabs' in request.json: + def get_location_for_tab(tab): + """ Returns the location (old-style) for a tab. """ + return loc_mapper().translate_locator_to_location(BlockUsageLocator(tab)) + + tabs = request.json['tabs'] + + # get list of existing static tabs in course + # make sure they are the same lengths (i.e. the number of passed in tabs equals the number + # that we know about) otherwise we will inadvertently drop some! + existing_static_tabs = [t for t in course_item.tabs if t['type'] == 'static_tab'] + if len(existing_static_tabs) != len(tabs): + return JsonResponse( + {"error": "number of tabs must be {}".format(len(existing_static_tabs))}, status=400 + ) + + # load all reference tabs, return BadRequest if we can't find any of them + tab_items = [] + for tab in tabs: + item = modulestore('direct').get_item(get_location_for_tab(tab)) + if item is None: + return JsonResponse( + {"error": "no tab for found location {}".format(tab)}, status=400 + ) + + tab_items.append(item) + + # now just go through the existing course_tabs and re-order the static tabs + reordered_tabs = [] + static_tab_idx = 0 + for tab in course_item.tabs: + if tab['type'] == 'static_tab': + reordered_tabs.append( + {'type': 'static_tab', + 'name': tab_items[static_tab_idx].display_name, + 'url_slug': tab_items[static_tab_idx].location.name, + } + ) + static_tab_idx += 1 + else: + reordered_tabs.append(tab) + + # OK, re-assemble the static tabs in the new order + course_item.tabs = reordered_tabs + modulestore('direct').update_metadata(course_item.location, own_metadata(course_item)) + return JsonResponse() + else: + raise NotImplementedError('Creating or changing tab content is not supported.') + elif request.method == 'GET': # assume html + # see tabs have been uninitialized (e.g. supporting courses created before tab support in studio) + if course_item.tabs is None or len(course_item.tabs) == 0: + initialize_course_tabs(course_item) + + # first get all static tabs from the tabs list + # we do this because this is also the order in which items are displayed in the LMS + static_tabs_refs = [t for t in course_item.tabs if t['type'] == 'static_tab'] + + static_tabs = [] + for static_tab_ref in static_tabs_refs: + static_tab_loc = old_location.replace(category='static_tab', name=static_tab_ref['url_slug']) + static_tabs.append(modulestore('direct').get_item(static_tab_loc)) + + components = [ + loc_mapper().translate_location( + course_item.location.course_id, static_tab.location, False, True + ) + for static_tab + in static_tabs + ] + + return render_to_response('edit-tabs.html', { + 'context_course': course_item, + 'components': components, + 'course_locator': locator + }) + else: + return HttpResponseNotFound() # "primitive" tab edit functions driven by the command line. @@ -164,7 +174,7 @@ def primitive_delete(course, num): # Note for future implementations: if you delete a static_tab, then Chris Dodge # points out that there's other stuff to delete beyond this element. # This code happens to not delete static_tab so it doesn't come up. - primitive_save(course) + modulestore('direct').update_metadata(course.location, own_metadata(course)) def primitive_insert(course, num, tab_type, name): @@ -173,11 +183,5 @@ def primitive_insert(course, num, tab_type, name): new_tab = {u'type': unicode(tab_type), u'name': unicode(name)} tabs = course.tabs tabs.insert(num, new_tab) - primitive_save(course) - - -def primitive_save(course): - "Saves the course back to modulestore." - # This code copied from reorder_static_tabs above - course.save() modulestore('direct').update_metadata(course.location, own_metadata(course)) + diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 2ee8d1797958..c84b60be61a8 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -197,7 +197,8 @@ define([ "js/spec/transcripts/videolist_spec", "js/spec/transcripts/message_manager_spec", "js/spec/transcripts/file_uploader_spec", - "js/spec/utils/module_spec" + "js/spec/utils/module_spec", + "js/spec/models/explicit_url_spec" # these tests are run separate in the cms-squire suite, due to process # isolation issues with Squire.js diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index a85b3b786378..83ca7dc2fedc 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -37,14 +37,17 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views analytics.track "Reordered Static Pages", course: course_location_analytics + saving = new NotificationView.Mini({title: gettext("Saving…")}) + saving.show() + $.ajax({ type:'POST', - url: '/reorder_static_tabs', + url: @model.url(), data: JSON.stringify({ tabs : tabs }), contentType: 'application/json' - }) + }).success(=> saving.hide()) addNewTab: (event) => event.preventDefault() diff --git a/cms/static/js/models/explicit_url.js b/cms/static/js/models/explicit_url.js new file mode 100644 index 000000000000..aae69608af85 --- /dev/null +++ b/cms/static/js/models/explicit_url.js @@ -0,0 +1,14 @@ +/** + * A model that simply allows the update URL to be passed + * in as an argument. + */ +define(["backbone"], function(Backbone){ + return Backbone.Model.extend({ + defaults: { + "explicit_url": "" + }, + url: function() { + return this.get("explicit_url"); + } + }); +}); diff --git a/cms/static/js/spec/models/explicit_url_spec.js b/cms/static/js/spec/models/explicit_url_spec.js new file mode 100644 index 000000000000..df70d47b63ff --- /dev/null +++ b/cms/static/js/spec/models/explicit_url_spec.js @@ -0,0 +1,12 @@ +define(['js/models/explicit_url'], + function (Model) { + describe('Model ', function () { + it('allows url to be passed in constructor', function () { + expect(new Model({'explicit_url': '/fancy/url'}).url()).toBe('/fancy/url'); + }); + it('returns empty string if url not set', function () { + expect(new Model().url()).toBe(''); + }); + }); + } +); diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index ca6e0259abc2..70c3e61801db 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -9,12 +9,15 @@ <%block name="jsextra"> +{% endaddtoblock %} + + {% include "wiki/includes/editor.html" %}
          - - + + {% trans "Delete article" %} - +
          - + {% include "wiki/includes/cheatsheet.html" %} {% endblock %} - - - diff --git a/lms/templates/wiki/history.html b/lms/templates/wiki/history.html index 5488abb97db2..798b030c0367 100644 --- a/lms/templates/wiki/history.html +++ b/lms/templates/wiki/history.html @@ -10,7 +10,7 @@ {% endaddtoblock %} @@ -81,7 +95,7 @@ {% trans "Click each revision to see a list of edited lines. Click the Preview button to see how the article looked at this stage. At the bottom of this page, you can change to a particular revision or merge an old revision with the current one." %}

          -
          +
          {% for revision in revisions %}
          @@ -107,16 +121,29 @@
          {% if not revision == article.current_revision %} - - {% endif %} - - {% if article|can_write:user %} - + + + {% trans "Preview this revision" %} + + + {% if article|can_write:user %} + + {% endif %} + {% endif %} - +
          @@ -140,84 +167,103 @@
        {% endfor %} - + {% include "wiki/includes/pagination.html" %} - + {% if revisions.count > 1 %}
        {% if article|can_write:user %} - + {% else %} {% endif %} -
        - {% endif %} - +
        - diff --git a/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore b/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore index fa328208fde6..c17d83f71078 100644 --- a/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore +++ b/cms/templates/js/transcripts/messages/transcripts-uploaded.underscore @@ -10,7 +10,7 @@ - "> + "> <%= gettext("Download to Edit") %>
    diff --git a/cms/templates/unit.html b/cms/templates/unit.html index 9f3daf0b20c8..2318e9cd9725 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -48,8 +48,8 @@

      - % for id, locator in components: -
    1. + % for locator in components: +
    2. % endfor
    3. From 050031f7d5bc04389a5018f75c92b7b25dbfec83 Mon Sep 17 00:00:00 2001 From: Zubair Afzal Date: Wed, 4 Dec 2013 16:43:17 +0500 Subject: [PATCH 84/87] Disable Peer Track Changes STUD-1008 --- common/lib/xmodule/xmodule/combined_open_ended_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 68a0d65617c1..2eb8585d854e 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -508,5 +508,5 @@ def get_context(self): def non_editable_metadata_fields(self): non_editable_fields = super(CombinedOpenEndedDescriptor, self).non_editable_metadata_fields non_editable_fields.extend([CombinedOpenEndedDescriptor.due, CombinedOpenEndedDescriptor.graceperiod, - CombinedOpenEndedDescriptor.markdown, CombinedOpenEndedDescriptor.version]) + CombinedOpenEndedDescriptor.markdown, CombinedOpenEndedDescriptor.version, CombinedOpenEndedDescriptor.track_changes]) return non_editable_fields From 9d48bb0ef1459e8ca091c3e88e52d8f7567f26f6 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Thu, 5 Dec 2013 12:08:39 -0500 Subject: [PATCH 85/87] removes temporary error messages --- lms/templates/instructor/staff_grading.html | 19 ------------------- .../combined_notifications.html | 19 ------------------- .../open_ended_flagged_problems.html | 19 ------------------- .../open_ended_problems.html | 19 ------------------- lms/templates/peer_grading/peer_grading.html | 19 ------------------- 5 files changed, 95 deletions(-) diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index 209c4b00eb48..59585308c182 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -22,25 +22,6 @@

      ${_("Staff grading")}

      -
      -
      - - ${_("We're sorry, some student responses have been temporarily " - "hidden. We appreciate your patience as we work hard " - "to restore their visibility. Rest assured, no student " - "data has been lost.")} - -
      - - ${_("Update: We have recovered the majority of student responses. " - "Those responses submitted while the issue was active " - "(approximately 2:00pm EST Dec 3 to 9:00am EST Dec 5) are " - "temporarily unavailable, but we are working on restoring " - "them, and they should be visible again shortly.")} - -
      -
      -
      diff --git a/lms/templates/open_ended_problems/combined_notifications.html b/lms/templates/open_ended_problems/combined_notifications.html index 2f54b173c7ed..47bb01fc8ed5 100644 --- a/lms/templates/open_ended_problems/combined_notifications.html +++ b/lms/templates/open_ended_problems/combined_notifications.html @@ -16,25 +16,6 @@
      ${error_text}
      -
      -
      - - ${_("We're sorry, some student responses have been temporarily " - "hidden. We appreciate your patience as we work hard " - "to restore their visibility. Rest assured, no student " - "data has been lost.")} - -
      - - ${_("Update: We have recovered the majority of student responses. " - "Those responses submitted while the issue was active " - "(approximately 2:00pm EST Dec 3 to 9:00am EST Dec 5) are " - "temporarily unavailable, but we are working on restoring " - "them, and they should be visible again shortly.")} - -
      -
      -

      ${_("Open Ended Console")}

      ${_("Instructions")}

      diff --git a/lms/templates/open_ended_problems/open_ended_flagged_problems.html b/lms/templates/open_ended_problems/open_ended_flagged_problems.html index 9a43a9ff5cf4..87d5a11bd806 100644 --- a/lms/templates/open_ended_problems/open_ended_flagged_problems.html +++ b/lms/templates/open_ended_problems/open_ended_flagged_problems.html @@ -19,25 +19,6 @@
      ${error_text}
      -
      -
      - - ${_("We're sorry, some student responses have been temporarily " - "hidden. We appreciate your patience as we work hard " - "to restore their visibility. Rest assured, no student " - "data has been lost.")} - -
      - - ${_("Update: We have recovered the majority of student responses. " - "Those responses submitted while the issue was active " - "(approximately 2:00pm EST Dec 3 to 9:00am EST Dec 5) are " - "temporarily unavailable, but we are working on restoring " - "them, and they should be visible again shortly.")} - -
      -
      -

      ${_("Flagged Open Ended Problems")}

      ${_("Instructions")}

      diff --git a/lms/templates/open_ended_problems/open_ended_problems.html b/lms/templates/open_ended_problems/open_ended_problems.html index 2a7c74855600..d5edc8edaf6b 100644 --- a/lms/templates/open_ended_problems/open_ended_problems.html +++ b/lms/templates/open_ended_problems/open_ended_problems.html @@ -16,25 +16,6 @@
      ${error_text}
      -
      -
      - - ${_("We're sorry, some student responses have been temporarily " - "hidden. We appreciate your patience as we work hard " - "to restore their visibility. Rest assured, no student " - "data has been lost.")} - -
      - - ${_("Update: We have recovered the majority of student responses. " - "Those responses submitted while the issue was active " - "(approximately 2:00pm EST Dec 3 to 9:00am EST Dec 5) are " - "temporarily unavailable, but we are working on restoring " - "them, and they should be visible again shortly.")} - -
      -
      -

      ${_("Open Ended Problems")}

      ${_("Instructions")}

      ${_("Here is a list of open ended problems for this course.")}

      diff --git a/lms/templates/peer_grading/peer_grading.html b/lms/templates/peer_grading/peer_grading.html index ce8e495ff76d..101e6341f527 100644 --- a/lms/templates/peer_grading/peer_grading.html +++ b/lms/templates/peer_grading/peer_grading.html @@ -15,25 +15,6 @@
      ${error_text}
      -
      -
      - - ${_("We're sorry, some student responses have been temporarily " - "hidden. We appreciate your patience as we work hard " - "to restore their visibility. Rest assured, no student " - "data has been lost.")} - -
      - - ${_("Update: We have recovered the majority of student responses. " - "Those responses submitted while the issue was active " - "(approximately 2:00pm EST Dec 3 to 9:00am EST Dec 5) are " - "temporarily unavailable, but we are working on restoring " - "them, and they should be visible again shortly.")} - -
      -
      -

      ${_("Peer Grading")}

      ${_("Instructions")}

      From 108e02e1ed75c7368139f5a65f5cd9890819ffdc Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Thu, 5 Dec 2013 14:41:30 -0500 Subject: [PATCH 86/87] Remove the one-time-use managemant command. --- .../recover_truncated_anonymous_ids.py | 69 ------------------- 1 file changed, 69 deletions(-) delete mode 100644 common/djangoapps/student/management/commands/recover_truncated_anonymous_ids.py diff --git a/common/djangoapps/student/management/commands/recover_truncated_anonymous_ids.py b/common/djangoapps/student/management/commands/recover_truncated_anonymous_ids.py deleted file mode 100644 index 09c5edce7ae2..000000000000 --- a/common/djangoapps/student/management/commands/recover_truncated_anonymous_ids.py +++ /dev/null @@ -1,69 +0,0 @@ -""" -Generate sql commands to fix truncated anonymous student ids in the ORA database -""" -import sys - -from django.core.management.base import NoArgsCommand - -from student.models import AnonymousUserId, anonymous_id_for_user - - -class Command(NoArgsCommand): - help = __doc__ - - def handle_noargs(self, **options): - """ - Reads a list of ids (newline separated) from stdin, and - dumps sql queries to run on the ORA database to fix those ids - from their truncated form to the full 32 character change. - - The following query will generate the list of ids needed to be fixed - from the ORA database: - - SELECT student_id FROM peer_grading_calibrationhistory WHERE LENGTH(student_id) = 16 - UNION SELECT student_id FROM controller_submission WHERE LENGTH(student_id) = 16 - UNION SELECT student_id FROM metrics_timing WHERE LENGTH(student_id) = 16 - UNION SELECT student_id FROM metrics_studentcourseprofile WHERE LENGTH(student_id) = 16 - UNION SELECT student_id FROM metrics_studentprofile WHERE LENGTH(student_id) = 16; - """ - - ids = [line.strip() for line in sys.stdin] - - old_ids = AnonymousUserId.objects.raw( - """ - SELECT * - FROM student_anonymoususerid_temp_archive - WHERE anonymous_user_id IN ({}) - """.format(','.join(['%s']*len(ids))), - ids - ) - - for old_id in old_ids: - new_id = anonymous_id_for_user(old_id.user, old_id.course_id) - - for table in ('peer_grading_calibrationhistory', 'controller_submission', 'metrics_timing'): - self.stdout.write( - "UPDATE {} " - "SET student_id = '{}' " - "WHERE student_id = '{}';\n".format( - table, - new_id, - old_id.anonymous_user_id, - ) - ) - - self.stdout.write( - "DELETE FROM metrics_studentcourseprofile " - "WHERE student_id = '{}' " - "AND problems_attempted = 0;\n".format(old_id.anonymous_user_id) - ) - - self.stdout.write( - "DELETE FROM metrics_studentprofile " - "WHERE student_id = '{}' " - "AND messages_sent = 0 " - "AND messages_received = 0 " - "AND average_message_feedback_length = 0 " - "AND student_is_staff_banned = 0 " - "AND student_cannot_submit_more_for_peer_grading = 0;\n".format(old_id.anonymous_user_id) - ) From be3ab1c5e9e3d3d4f4b087302ef8e544ee7f631b Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Thu, 5 Dec 2013 14:56:30 -0500 Subject: [PATCH 87/87] Drop the temp table --- ...op_student_anonymoususerid_temp_archive.py | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 common/djangoapps/student/migrations/0031_drop_student_anonymoususerid_temp_archive.py diff --git a/common/djangoapps/student/migrations/0031_drop_student_anonymoususerid_temp_archive.py b/common/djangoapps/student/migrations/0031_drop_student_anonymoususerid_temp_archive.py new file mode 100644 index 000000000000..ac7d0ed1172c --- /dev/null +++ b/common/djangoapps/student/migrations/0031_drop_student_anonymoususerid_temp_archive.py @@ -0,0 +1,186 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import DataMigration +from django.db import models + +class Migration(DataMigration): + + def forwards(self, orm): + "Write your forwards methods here." + # Note: Remember to use orm['appname.ModelName'] rather than "from appname.models..." + db.execute("DROP TABLE student_anonymoususerid_temp_archive") + + def backwards(self, orm): + "Write your backwards methods here." + + 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.anonymoususerid': { + 'Meta': {'object_name': 'AnonymousUserId'}, + 'anonymous_user_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32'}), + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.courseenrollment': { + 'Meta': {'ordering': "('user', 'course_id')", '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'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'mode': ('django.db.models.fields.CharField', [], {'default': "'honor'", 'max_length': '100'}), + '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', [], {'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.userstanding': { + 'Meta': {'object_name': 'UserStanding'}, + 'account_status': ('django.db.models.fields.CharField', [], {'max_length': '31', 'blank': 'True'}), + 'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'standing_last_changed_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'standing'", 'unique': 'True', 'to': "orm['auth.User']"}) + }, + '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'] + symmetrical = True