From dba39e243174f28a1245103721194ef33a7300ca Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 18 Jan 2013 12:50:25 -0500 Subject: [PATCH 01/82] add group support to lms --- lms/djangoapps/courseware/courses.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 7c0d30ebd8..90ebbd33da 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -58,6 +58,29 @@ def get_opt_course_with_access(user, course_id, action): return get_course_with_access(user, course_id, action) + + +def is_course_cohorted(course_id): + """ + given a course id, return a boolean for whether or not the course is cohorted + + """ + +def get_cohort_id(user, course_id): + """ + given a course id and a user, return the id of the cohort that user is assigned to + and if the course is not cohorted or the user is an instructor, return None + + """ + +def is_commentable_cohorted(course_id,commentable_id) + """ + given a course and a commentable id, return whether or not this commentable is cohorted + + """ + + + def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" From b8e3cfcf96a143294df5482f904d8e45c10becf9 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 18 Jan 2013 13:50:05 -0500 Subject: [PATCH 02/82] added get_cohort_ids --- lms/djangoapps/courseware/courses.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 90ebbd33da..8fd4497baf 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -78,8 +78,12 @@ def is_commentable_cohorted(course_id,commentable_id) given a course and a commentable id, return whether or not this commentable is cohorted """ + - +def get_cohort_ids(course_id): + """ + given a course id, return an array of all cohort ids for that course (needed for UI + """ def course_image_url(course): """Try to look up the image url for the course. If it's not found, From b26c59bda0292c801400d104abb0ba5097ce18cf Mon Sep 17 00:00:00 2001 From: Kevin Chugh Date: Sun, 20 Jan 2013 01:53:25 -0500 Subject: [PATCH 03/82] fix syntax error --- lms/djangoapps/courseware/courses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 8fd4497baf..551e459492 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -73,7 +73,7 @@ def get_cohort_id(user, course_id): """ -def is_commentable_cohorted(course_id,commentable_id) +def is_commentable_cohorted(course_id,commentable_id): """ given a course and a commentable id, return whether or not this commentable is cohorted From 59b3dc61a286dbac4a66d90cea1a98b43dab07a6 Mon Sep 17 00:00:00 2001 From: Kevin Chugh Date: Wed, 23 Jan 2013 16:25:26 -0500 Subject: [PATCH 04/82] lms producing grouped content --- lms/djangoapps/courseware/courses.py | 1 + .../django_comment_client/base/views.py | 19 +++++++++++++++++++ .../django_comment_client/forum/views.py | 7 +++++++ lms/lib/comment_client/thread.py | 4 ++-- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 551e459492..057ef42a58 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -72,6 +72,7 @@ def get_cohort_id(user, course_id): and if the course is not cohorted or the user is an instructor, return None """ + return 101 def is_commentable_cohorted(course_id,commentable_id): """ diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 63d69427c9..d1948c3fc7 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -21,6 +21,7 @@ from django.contrib.auth.models import User from mitxmako.shortcuts import render_to_response, render_to_string from courseware.courses import get_course_with_access +from courseware.courses import get_cohort_id from django_comment_client.utils import JsonResponse, JsonError, extract, get_courseware_context @@ -83,6 +84,24 @@ def create_thread(request, course_id, commentable_id): 'course_id' : course_id, 'user_id' : request.user.id, }) + + #now cohort id + #if the group id came in from the form, set it there, otherwise, + #see if the user and the commentable are cohorted + print post + + group_id = None + + if 'group_id' in post: + group_id = post['group_id'] + + + if group_id is None: + group_id = get_cohort_id(request.user, course_id) + + if group_id is not None: + thread.update_attributes(**{'group_id' :group_id}) + thread.save() if post.get('auto_subscribe', 'false').lower() == 'true': user = cc.User.from_django_user(request.user) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 35e7fd6618..0e8a044097 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -11,6 +11,7 @@ from django.contrib.auth.models import User from mitxmako.shortcuts import render_to_response, render_to_string from courseware.courses import get_course_with_access +from courseware.courses import get_cohort_id from courseware.access import has_access from urllib import urlencode @@ -58,6 +59,12 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG user.default_sort_key = request.GET.get('sort_key') user.save() + + #if the course-user is cohorted, then add the group id + group_id = get_cohort_id(user,course_id); + if group_id: + default_query_params["group_id"] = group_id; + query_params = merge_dict(default_query_params, strip_none(extract(request.GET, ['page', 'sort_key', 'sort_order', 'text', 'tags', 'commentable_ids']))) diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 424250033e..6fd31b0823 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -10,12 +10,12 @@ class Thread(models.Model): 'closed', 'tags', 'votes', 'commentable_id', 'username', 'user_id', 'created_at', 'updated_at', 'comments_count', 'unread_comments_count', 'at_position_list', 'children', 'type', 'highlighted_title', - 'highlighted_body', 'endorsed', 'read' + 'highlighted_body', 'endorsed', 'read', 'group_id' ] updatable_fields = [ 'title', 'body', 'anonymous', 'anonymous_to_peers', 'course_id', - 'closed', 'tags', 'user_id', 'commentable_id', + 'closed', 'tags', 'user_id', 'commentable_id', 'group_id' ] initializable_fields = updatable_fields From 1d13b1a9bd6ec595b35c727468c484b2a36f3047 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 09:45:22 -0500 Subject: [PATCH 05/82] Make various changes to handle the s3/sftp part of the pearson process. --- .../student/management/commands/pearson.py | 74 +++++++++++++++++++ .../management/commands/pearson_export_cdd.py | 32 +++----- .../management/commands/pearson_export_ead.py | 26 +++---- 3 files changed, 93 insertions(+), 39 deletions(-) create mode 100644 common/djangoapps/student/management/commands/pearson.py diff --git a/common/djangoapps/student/management/commands/pearson.py b/common/djangoapps/student/management/commands/pearson.py new file mode 100644 index 0000000000..2a8b02928c --- /dev/null +++ b/common/djangoapps/student/management/commands/pearson.py @@ -0,0 +1,74 @@ +from optparse import make_option + +from django.contrib.auth.models import User +from django.core.management.base import BaseCommand, CommandError +import re +from dogapi import dog_http_api, dog_stats_api +import paramiko + +dog_http_api.api_key = settings.DATADOG_API + + +class Command(BaseCommand): + + option_list = BaseCommand.option_list + args = '' + help = """ + Mode should be import or export depending on if you're fetching from pearson or + sending to them. + """ + + def handle(self, *args): + if len(args) < 1: + raise CommandError('Usage is pearson {0}'.format(self.args)) + + for mode in args: + if mode == 'export': + sftp(settings.PEARSON_LOCAL_IMPORT, settings.PEARSON_SFTP_IMPORT) + s3(settings.PEARSON_LOCAL, settings.PEARSON_BUCKET) + elif mode == 'import': + sftp(settings.PEARSON_SFTP_EXPORT, settings.PEARSON_LOCAL_EXPORT) + s3(settings.PEARSON_LOCAL_EXPORT, settings.PEARSON_BUCKET) + else: + print("ERROR: Mode must be export or import.") + + def sftp(files_from, files_to): + with dog_stats_api.timer('pearson.{0}'.format(mode), tags='sftp'): + try: + t = paramiko.Transport((hostname, 22)) + t.connect(username=settings.PEARSON_SFTP_USERNAME, + password=settings.PEARSON_SFTP_PASSWORD) + sftp = paramiko.SFTPClient.from_transport(t) + if os.path.isdir(files_from): + for file in os.listdir(files_from): + sftp.put(files_from+'/'+filename, + files_to+'/'+filename) + else: + for file in sftp.listdir(files_from): + sftp.get(files_from+'/'+filename, + files_to+'/'+filename) + except: + dog_http_api.event('pearson {0}'.format(mode), + 'sftp uploading failed', alert_type='error') + raise + + def s3(files_from, bucket): + with dog_stats_api.timer('pearson.{0}'.format(mode), tags='s3'): + try: + for filename in os.listdir(files): + upload_file_to_s3(bucket, files_from+'/'+filename) + except: + dog_http_api.event('pearson {0}'.format(mode), 's3 archiving failed') + raise + + + def upload_file_to_s3(bucket, filename): + """ + Upload file to S3 + """ + s3 = boto.connect_s3() + from boto.s3.key import Key + b = s3.get_bucket(bucket) + k = Key(b) + k.key = "{filename}".format(filename=filename) + k.set_contents_from_filename(filename) diff --git a/common/djangoapps/student/management/commands/pearson_export_cdd.py b/common/djangoapps/student/management/commands/pearson_export_cdd.py index 67230c7f74..cebc098080 100644 --- a/common/djangoapps/student/management/commands/pearson_export_cdd.py +++ b/common/djangoapps/student/management/commands/pearson_export_cdd.py @@ -37,39 +37,25 @@ class Command(BaseCommand): ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store ]) - option_list = BaseCommand.option_list + ( - make_option( - '--dump_all', - action='store_true', - dest='dump_all', - ), - ) - - args = '' - help = """ - Export user demographic information from TestCenterUser model into a tab delimited - text file with a format that Pearson expects. - """ - def handle(self, *args, **kwargs): - if len(args) < 1: - print Command.help - return - + def handle(self, **kwargs): # update time should use UTC in order to be comparable to the user_updated_at # field uploaded_at = datetime.utcnow() # if specified destination is an existing directory, then # create a filename for it automatically. If it doesn't exist, - # or exists as a file, then we will just write to it. + # then we will create the directory. # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - dest = args[0] - if isdir(dest): - destfile = os.path.join(dest, uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) + if not os.path.isdir(settings.PEARSON_LOCAL_EXPORT): + os.makedirs(settings.PEARSON_LOCAL_EXPORT) + destfile = os.path.join(settings.PEARSON_LOCAL_EXPORT, + uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) else: - destfile = dest + destfile = os.path.join(settings.PEARSON_LOCAL_EXPORT, + uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) + # strings must be in latin-1 format. CSV parser will # otherwise convert unicode objects to ascii. diff --git a/common/djangoapps/student/management/commands/pearson_export_ead.py b/common/djangoapps/student/management/commands/pearson_export_ead.py index de3bfc04ee..5423282346 100644 --- a/common/djangoapps/student/management/commands/pearson_export_ead.py +++ b/common/djangoapps/student/management/commands/pearson_export_ead.py @@ -23,11 +23,6 @@ class Command(BaseCommand): ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store ]) - args = '' - help = """ - Export user registration information from TestCenterRegistration model into a tab delimited - text file with a format that Pearson expects. - """ option_list = BaseCommand.option_list + ( make_option( @@ -43,26 +38,25 @@ class Command(BaseCommand): ) - def handle(self, *args, **kwargs): - if len(args) < 1: - print Command.help - return + def handle(self, **kwargs): # update time should use UTC in order to be comparable to the user_updated_at # field uploaded_at = datetime.utcnow() - # if specified destination is an existing directory, then + # if specified destination is an existing directory, then # create a filename for it automatically. If it doesn't exist, - # or exists as a file, then we will just write to it. + # then we will create the directory. # Name will use timestamp -- this is UTC, so it will look funny, - # but it should at least be consistent with the other timestamps + # but it should at least be consistent with the other timestamps # used in the system. - dest = args[0] - if isdir(dest): - destfile = join(dest, uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) + if not os.path.isdir(settings.PEARSON_LOCAL_EXPORT): + os.makedirs(settings.PEARSON_LOCAL_EXPORT) + destfile = os.path.join(settings.PEARSON_LOCAL_EXPORT, + uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) else: - destfile = dest + destfile = os.path.join(settings.PEARSON_LOCAL_EXPORT, + uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) dump_all = kwargs['dump_all'] From 1acf2dbba350272b06df29fe25d71f248060f1c4 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 09:52:04 -0500 Subject: [PATCH 06/82] Use a dictionary for all the pearson stuff to keep the auth/env stuff clean. --- .../student/management/commands/pearson.py | 12 +++++++----- .../management/commands/pearson_export_cdd.py | 8 ++++---- .../management/commands/pearson_export_ead.py | 8 ++++---- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson.py b/common/djangoapps/student/management/commands/pearson.py index 2a8b02928c..a317b08d62 100644 --- a/common/djangoapps/student/management/commands/pearson.py +++ b/common/djangoapps/student/management/commands/pearson.py @@ -5,6 +5,7 @@ from django.core.management.base import BaseCommand, CommandError import re from dogapi import dog_http_api, dog_stats_api import paramiko +import boto dog_http_api.api_key = settings.DATADOG_API @@ -24,11 +25,11 @@ class Command(BaseCommand): for mode in args: if mode == 'export': - sftp(settings.PEARSON_LOCAL_IMPORT, settings.PEARSON_SFTP_IMPORT) - s3(settings.PEARSON_LOCAL, settings.PEARSON_BUCKET) + sftp(settings.PEARSON[LOCAL_IMPORT], settings.PEARSON[SFTP_IMPORT]) + s3(settings.PEARSON_LOCAL, settings.PEARSON[BUCKET]) elif mode == 'import': - sftp(settings.PEARSON_SFTP_EXPORT, settings.PEARSON_LOCAL_EXPORT) - s3(settings.PEARSON_LOCAL_EXPORT, settings.PEARSON_BUCKET) + sftp(settings.PEARSON[SFTP_EXPORT], settings.PEARSON[LOCAL_EXPORT]) + s3(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[BUCKET]) else: print("ERROR: Mode must be export or import.") @@ -66,7 +67,8 @@ class Command(BaseCommand): """ Upload file to S3 """ - s3 = boto.connect_s3() + s3 = boto.connect_s3(settings.AWS_ACCESS_KEY_ID, + settings.AWS_SECRET_ACCESS_KEY) from boto.s3.key import Key b = s3.get_bucket(bucket) k = Key(b) diff --git a/common/djangoapps/student/management/commands/pearson_export_cdd.py b/common/djangoapps/student/management/commands/pearson_export_cdd.py index cebc098080..dcb9f5cd97 100644 --- a/common/djangoapps/student/management/commands/pearson_export_cdd.py +++ b/common/djangoapps/student/management/commands/pearson_export_cdd.py @@ -48,12 +48,12 @@ class Command(BaseCommand): # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - if not os.path.isdir(settings.PEARSON_LOCAL_EXPORT): - os.makedirs(settings.PEARSON_LOCAL_EXPORT) - destfile = os.path.join(settings.PEARSON_LOCAL_EXPORT, + if not os.path.isdir(settings.PEARSON[LOCAL_EXPORT]): + os.makedirs(settings.PEARSON[LOCAL_EXPORT]) + destfile = os.path.join(settings.PEARSON[LOCAL_EXPORT], uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) else: - destfile = os.path.join(settings.PEARSON_LOCAL_EXPORT, + destfile = os.path.join(settings.PEARSON[LOCAL_EXPORT], uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) diff --git a/common/djangoapps/student/management/commands/pearson_export_ead.py b/common/djangoapps/student/management/commands/pearson_export_ead.py index 5423282346..9520e9d013 100644 --- a/common/djangoapps/student/management/commands/pearson_export_ead.py +++ b/common/djangoapps/student/management/commands/pearson_export_ead.py @@ -50,12 +50,12 @@ class Command(BaseCommand): # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - if not os.path.isdir(settings.PEARSON_LOCAL_EXPORT): - os.makedirs(settings.PEARSON_LOCAL_EXPORT) - destfile = os.path.join(settings.PEARSON_LOCAL_EXPORT, + if not os.path.isdir(settings.PEARSON[LOCAL_EXPORT]): + os.makedirs(settings.PEARSON[LOCAL_EXPORT]) + destfile = os.path.join(settings.PEARSON[LOCAL_EXPORT], uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) else: - destfile = os.path.join(settings.PEARSON_LOCAL_EXPORT, + destfile = os.path.join(settings.PEARSON[LOCAL_EXPORT], uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) dump_all = kwargs['dump_all'] From 61ea2d7b5da004053073c575c6bb4a8a066296dc Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 10:02:10 -0500 Subject: [PATCH 07/82] Couple of fixes to the settings data. --- .../student/management/commands/pearson.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson.py b/common/djangoapps/student/management/commands/pearson.py index a317b08d62..0752aea8be 100644 --- a/common/djangoapps/student/management/commands/pearson.py +++ b/common/djangoapps/student/management/commands/pearson.py @@ -25,11 +25,11 @@ class Command(BaseCommand): for mode in args: if mode == 'export': - sftp(settings.PEARSON[LOCAL_IMPORT], settings.PEARSON[SFTP_IMPORT]) - s3(settings.PEARSON_LOCAL, settings.PEARSON[BUCKET]) - elif mode == 'import': - sftp(settings.PEARSON[SFTP_EXPORT], settings.PEARSON[LOCAL_EXPORT]) + sftp(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[SFTP_EXPORT]) s3(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[BUCKET]) + elif mode == 'import': + sftp(settings.PEARSON[SFTP_IMPORT], settings.PEARSON[LOCAL_IMPORT]) + s3(settings.PEARSON[LOCAL_IMPORT], settings.PEARSON[BUCKET]) else: print("ERROR: Mode must be export or import.") @@ -37,8 +37,8 @@ class Command(BaseCommand): with dog_stats_api.timer('pearson.{0}'.format(mode), tags='sftp'): try: t = paramiko.Transport((hostname, 22)) - t.connect(username=settings.PEARSON_SFTP_USERNAME, - password=settings.PEARSON_SFTP_PASSWORD) + t.connect(username=settings.PEARSON[SFTP_USERNAME], + password=settings.PEARSON[SFTP_PASSWORD]) sftp = paramiko.SFTPClient.from_transport(t) if os.path.isdir(files_from): for file in os.listdir(files_from): From ced29a25ece11ea50799515f22f307a1e19d74e7 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 10:17:30 -0500 Subject: [PATCH 08/82] Add paramiko to requirements. --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 996388a51d..fa4688b711 100644 --- a/requirements.txt +++ b/requirements.txt @@ -58,4 +58,4 @@ factory_boy Shapely==1.2.16 ipython==0.13.1 xmltodict==0.4.1 - +paramiko==1.9.0 From 378b1ff0c35c809558bb081f5acc09be9a516db4 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 11:26:39 -0500 Subject: [PATCH 09/82] Various changes thanks to feedback from Brian to make the existing export commands handle --dest-from-settings and --destination and fail unless one is provided as well as rename pearson.py to pearson_transfer and allow is to call the import/export commands directly. I've set it to die in pearson_transfer.py if the django PEARSON settings aren't available. I don't want to try and provide defaults, these must exist or it simply fails. --- .../management/commands/pearson_export_cdd.py | 37 ++++++++++--- .../management/commands/pearson_export_ead.py | 52 ++++++++++++------- .../{pearson.py => pearson_transfer.py} | 50 +++++++++++------- 3 files changed, 95 insertions(+), 44 deletions(-) rename common/djangoapps/student/management/commands/{pearson.py => pearson_transfer.py} (63%) diff --git a/common/djangoapps/student/management/commands/pearson_export_cdd.py b/common/djangoapps/student/management/commands/pearson_export_cdd.py index dcb9f5cd97..c2c13916ab 100644 --- a/common/djangoapps/student/management/commands/pearson_export_cdd.py +++ b/common/djangoapps/student/management/commands/pearson_export_cdd.py @@ -37,7 +37,20 @@ class Command(BaseCommand): ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store ]) - def handle(self, **kwargs): + option_list = BaseCommand.option_list + ( + make_option('--dest-from-settings', + action='store_true', + dest='dest-from-settings', + default=False, + help='Retrieve the destination to export to from django? True/False'), + make_option('--destination', + action='store_true', + dest='destination', + default=None, + help='Where to store the exported files') + ) + + def handle(self, **options): # update time should use UTC in order to be comparable to the user_updated_at # field uploaded_at = datetime.utcnow() @@ -48,13 +61,23 @@ class Command(BaseCommand): # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - if not os.path.isdir(settings.PEARSON[LOCAL_EXPORT]): - os.makedirs(settings.PEARSON[LOCAL_EXPORT]) - destfile = os.path.join(settings.PEARSON[LOCAL_EXPORT], - uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) + if options['dest-from-settings'] is True: + if settings.PEARSON[LOCAL_EXPORT]: + dest = settings.PEARSON[LOCAL_EXPORT] + else: + raise CommandError('--dest-from-settings was enabled but the' + 'PEARSON[LOCAL_EXPORT] setting was not set.') + elif options['destination']: + dest = options['destination'] else: - destfile = os.path.join(settings.PEARSON[LOCAL_EXPORT], - uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) + raise ComamndError('--destination or --dest-from-settings must be used') + + + if not os.path.isdir(dest): + os.makedirs(dest) + destfile = os.path.join(dest, uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) + else: + destfile = os.path.join(dest, uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) # strings must be in latin-1 format. CSV parser will diff --git a/common/djangoapps/student/management/commands/pearson_export_ead.py b/common/djangoapps/student/management/commands/pearson_export_ead.py index 9520e9d013..6e3149778d 100644 --- a/common/djangoapps/student/management/commands/pearson_export_ead.py +++ b/common/djangoapps/student/management/commands/pearson_export_ead.py @@ -23,24 +23,29 @@ class Command(BaseCommand): ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store ]) - option_list = BaseCommand.option_list + ( - make_option( - '--dump_all', - action='store_true', - dest='dump_all', + make_option('--dest-from-settings', + action='store_true', + dest='dest-from-settings', + default=False, + help='Retrieve the destination to export to from django? True/False'), + make_option('--destination', + action='store_true', + dest='destination', + default=None, + help='Where to store the exported files'), + make_option('--dump_all', + action='store_true', + dest='dump_all', ), - make_option( - '--force_add', - action='store_true', - dest='force_add', + make_option('--force_add', + action='store_true', + dest='force_add', ), ) - - - def handle(self, **kwargs): - # update time should use UTC in order to be comparable to the user_updated_at + def handle(self, **options): + # update time should use UTC in order to be comparable to the user_updated_at # field uploaded_at = datetime.utcnow() @@ -50,13 +55,22 @@ class Command(BaseCommand): # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - if not os.path.isdir(settings.PEARSON[LOCAL_EXPORT]): - os.makedirs(settings.PEARSON[LOCAL_EXPORT]) - destfile = os.path.join(settings.PEARSON[LOCAL_EXPORT], - uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) + if options['dest-from-settings'] is True: + if settings.PEARSON[LOCAL_EXPORT]: + dest = settings.PEARSON[LOCAL_EXPORT] + else: + raise CommandError('--dest-from-settings was enabled but the' + 'PEARSON[LOCAL_EXPORT] setting was not set.') + elif options['destination']: + dest = options['destination'] else: - destfile = os.path.join(settings.PEARSON[LOCAL_EXPORT], - uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) + raise ComamndError('--destination or --dest-from-settings must be used') + + if not os.path.isdir(dest): + os.makedirs(dest) + destfile = os.path.join(dest, uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) + else: + destfile = os.path.join(dest, uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) dump_all = kwargs['dump_all'] diff --git a/common/djangoapps/student/management/commands/pearson.py b/common/djangoapps/student/management/commands/pearson_transfer.py similarity index 63% rename from common/djangoapps/student/management/commands/pearson.py rename to common/djangoapps/student/management/commands/pearson_transfer.py index 0752aea8be..1d04936216 100644 --- a/common/djangoapps/student/management/commands/pearson.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -12,26 +12,40 @@ dog_http_api.api_key = settings.DATADOG_API class Command(BaseCommand): - option_list = BaseCommand.option_list - args = '' - help = """ - Mode should be import or export depending on if you're fetching from pearson or - sending to them. - """ + option_list = BaseCommand.option_list + ( + make_option('--mode', + action='store_true', + dest='mode', + default='both', + help='mode is import, export, or both'), + ) - def handle(self, *args): - if len(args) < 1: - raise CommandError('Usage is pearson {0}'.format(self.args)) + def handle(self, **options): + + if not settings.PEARSON: + raise CommandError('No PEARSON entries in auth/env.json.') + + def import_pearson(): + sftp(settings.PEARSON[SFTP_IMPORT], settings.PEARSON[LOCAL_IMPORT]) + s3(settings.PEARSON[LOCAL_IMPORT], settings.PEARSON[BUCKET]) + call_command('pearson_import', 'dest_from_settings=True') + + def export_pearson(): + call_command('pearson_export_ccd', 'dest_from_settings=True') + call_command('pearson_export_ead', 'dest_from_settings=True') + sftp(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[SFTP_EXPORT]) + s3(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[BUCKET]) + + if options['mode'] == 'both': + export_pearson() + import_pearson() + elif options['mode'] == 'export': + export_pearson() + elif options['mode'] == 'import': + import_pearson() + else: + print("ERROR: Mode must be export or import.") - for mode in args: - if mode == 'export': - sftp(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[SFTP_EXPORT]) - s3(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[BUCKET]) - elif mode == 'import': - sftp(settings.PEARSON[SFTP_IMPORT], settings.PEARSON[LOCAL_IMPORT]) - s3(settings.PEARSON[LOCAL_IMPORT], settings.PEARSON[BUCKET]) - else: - print("ERROR: Mode must be export or import.") def sftp(files_from, files_to): with dog_stats_api.timer('pearson.{0}'.format(mode), tags='sftp'): From 0555ebc4347c9eaa0943067386f55b3d0bf6d139 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 12:04:53 -0500 Subject: [PATCH 10/82] Bunch of fixes to how I do if/else checks, fix a typo in Command and repair the for filename part of sftp. --- .../management/commands/pearson_export_cdd.py | 14 ++++++-------- .../management/commands/pearson_export_ead.py | 15 +++++++-------- .../management/commands/pearson_transfer.py | 6 +++--- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_export_cdd.py b/common/djangoapps/student/management/commands/pearson_export_cdd.py index c2c13916ab..2655960ff5 100644 --- a/common/djangoapps/student/management/commands/pearson_export_cdd.py +++ b/common/djangoapps/student/management/commands/pearson_export_cdd.py @@ -44,7 +44,7 @@ class Command(BaseCommand): default=False, help='Retrieve the destination to export to from django? True/False'), make_option('--destination', - action='store_true', + action='store', dest='destination', default=None, help='Where to store the exported files') @@ -61,24 +61,22 @@ class Command(BaseCommand): # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - if options['dest-from-settings'] is True: - if settings.PEARSON[LOCAL_EXPORT]: + if 'dest-from-settings' in options: + if LOCAL_EXPORT in settings.PEARSON: dest = settings.PEARSON[LOCAL_EXPORT] else: raise CommandError('--dest-from-settings was enabled but the' 'PEARSON[LOCAL_EXPORT] setting was not set.') - elif options['destination']: + elif 'destination' in options: dest = options['destination'] else: - raise ComamndError('--destination or --dest-from-settings must be used') + raise CommandError('--destination or --dest-from-settings must be used') if not os.path.isdir(dest): os.makedirs(dest) - destfile = os.path.join(dest, uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) - else: - destfile = os.path.join(dest, uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) + destfile = os.path.join(dest, uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) # strings must be in latin-1 format. CSV parser will # otherwise convert unicode objects to ascii. diff --git a/common/djangoapps/student/management/commands/pearson_export_ead.py b/common/djangoapps/student/management/commands/pearson_export_ead.py index 6e3149778d..7697c99e4a 100644 --- a/common/djangoapps/student/management/commands/pearson_export_ead.py +++ b/common/djangoapps/student/management/commands/pearson_export_ead.py @@ -30,7 +30,7 @@ class Command(BaseCommand): default=False, help='Retrieve the destination to export to from django? True/False'), make_option('--destination', - action='store_true', + action='store', dest='destination', default=None, help='Where to store the exported files'), @@ -55,22 +55,21 @@ class Command(BaseCommand): # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - if options['dest-from-settings'] is True: - if settings.PEARSON[LOCAL_EXPORT]: + if 'dest-from-settings' in options: + if LOCAL_EXPORT in settings.PEARSON: dest = settings.PEARSON[LOCAL_EXPORT] else: raise CommandError('--dest-from-settings was enabled but the' 'PEARSON[LOCAL_EXPORT] setting was not set.') - elif options['destination']: + elif destinations in options: dest = options['destination'] else: - raise ComamndError('--destination or --dest-from-settings must be used') + raise CommandError('--destination or --dest-from-settings must be used') if not os.path.isdir(dest): os.makedirs(dest) - destfile = os.path.join(dest, uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) - else: - destfile = os.path.join(dest, uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) + + destfile = os.path.join(dest, uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) dump_all = kwargs['dump_all'] diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 1d04936216..7d4a2ead3f 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -14,7 +14,7 @@ class Command(BaseCommand): option_list = BaseCommand.option_list + ( make_option('--mode', - action='store_true', + action='store', dest='mode', default='both', help='mode is import, export, or both'), @@ -55,11 +55,11 @@ class Command(BaseCommand): password=settings.PEARSON[SFTP_PASSWORD]) sftp = paramiko.SFTPClient.from_transport(t) if os.path.isdir(files_from): - for file in os.listdir(files_from): + for filename in os.listdir(files_from): sftp.put(files_from+'/'+filename, files_to+'/'+filename) else: - for file in sftp.listdir(files_from): + for filename in sftp.listdir(files_from): sftp.get(files_from+'/'+filename, files_to+'/'+filename) except: From 3d5599c829fb8d7d06e89ce331b67fa6c419997d Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 12:12:22 -0500 Subject: [PATCH 11/82] Further fixes to close the ssh connection explictly, make both the default option if nothing is provided, and not bother passing true when calling the subcommands. --- .../management/commands/pearson_transfer.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 7d4a2ead3f..d07f75d011 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -28,23 +28,21 @@ class Command(BaseCommand): def import_pearson(): sftp(settings.PEARSON[SFTP_IMPORT], settings.PEARSON[LOCAL_IMPORT]) s3(settings.PEARSON[LOCAL_IMPORT], settings.PEARSON[BUCKET]) - call_command('pearson_import', 'dest_from_settings=True') + call_command('pearson_import', 'dest_from_settings') def export_pearson(): - call_command('pearson_export_ccd', 'dest_from_settings=True') - call_command('pearson_export_ead', 'dest_from_settings=True') + call_command('pearson_export_ccd', 'dest_from_settings') + call_command('pearson_export_ead', 'dest_from_settings') sftp(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[SFTP_EXPORT]) s3(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[BUCKET]) - if options['mode'] == 'both': - export_pearson() - import_pearson() - elif options['mode'] == 'export': + if options['mode'] == 'export': export_pearson() elif options['mode'] == 'import': import_pearson() else: - print("ERROR: Mode must be export or import.") + export_pearson() + import_pearson() def sftp(files_from, files_to): @@ -62,6 +60,7 @@ class Command(BaseCommand): for filename in sftp.listdir(files_from): sftp.get(files_from+'/'+filename, files_to+'/'+filename) + t.close() except: dog_http_api.event('pearson {0}'.format(mode), 'sftp uploading failed', alert_type='error') From 5431332cecf5773f33b56856df2f41c0b0b6156f Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 12:14:43 -0500 Subject: [PATCH 12/82] Fix up help. --- .../student/management/commands/pearson_export_cdd.py | 2 +- .../student/management/commands/pearson_export_ead.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_export_cdd.py b/common/djangoapps/student/management/commands/pearson_export_cdd.py index 2655960ff5..2f68ccc7d1 100644 --- a/common/djangoapps/student/management/commands/pearson_export_cdd.py +++ b/common/djangoapps/student/management/commands/pearson_export_cdd.py @@ -42,7 +42,7 @@ class Command(BaseCommand): action='store_true', dest='dest-from-settings', default=False, - help='Retrieve the destination to export to from django? True/False'), + help='Retrieve the destination to export to from django.'), make_option('--destination', action='store', dest='destination', diff --git a/common/djangoapps/student/management/commands/pearson_export_ead.py b/common/djangoapps/student/management/commands/pearson_export_ead.py index 7697c99e4a..0f81cb1df9 100644 --- a/common/djangoapps/student/management/commands/pearson_export_ead.py +++ b/common/djangoapps/student/management/commands/pearson_export_ead.py @@ -28,7 +28,7 @@ class Command(BaseCommand): action='store_true', dest='dest-from-settings', default=False, - help='Retrieve the destination to export to from django? True/False'), + help='Retrieve the destination to export to from django.'), make_option('--destination', action='store', dest='destination', From 482cefd246539169d3c0de8704af29ca6e3c5bf2 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Wed, 16 Jan 2013 13:37:33 -0500 Subject: [PATCH 13/82] Bunch of fixes to pep8 formatting, missing imports, change of kwargs to options, quoting variables and a few other fixups. --- .../management/commands/pearson_export_cdd.py | 27 +++++----- .../management/commands/pearson_export_ead.py | 27 +++++----- .../management/commands/pearson_transfer.py | 53 +++++++++++-------- 3 files changed, 55 insertions(+), 52 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_export_cdd.py b/common/djangoapps/student/management/commands/pearson_export_cdd.py index 2f68ccc7d1..14c652f11e 100644 --- a/common/djangoapps/student/management/commands/pearson_export_cdd.py +++ b/common/djangoapps/student/management/commands/pearson_export_cdd.py @@ -1,15 +1,16 @@ import csv +import os from collections import OrderedDict from datetime import datetime -from os.path import isdir from optparse import make_option from django.core.management.base import BaseCommand from student.models import TestCenterUser + class Command(BaseCommand): - + CSV_TO_MODEL_FIELDS = OrderedDict([ # Skipping optional field CandidateID ("ClientCandidateID", "client_candidate_id"), @@ -34,7 +35,7 @@ class Command(BaseCommand): ("FAXCountryCode", "fax_country_code"), ("CompanyName", "company_name"), # Skipping optional field CustomQuestion - ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store + ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store ]) option_list = BaseCommand.option_list + ( @@ -55,29 +56,28 @@ class Command(BaseCommand): # field uploaded_at = datetime.utcnow() - # if specified destination is an existing directory, then + # if specified destination is an existing directory, then # create a filename for it automatically. If it doesn't exist, # then we will create the directory. # Name will use timestamp -- this is UTC, so it will look funny, - # but it should at least be consistent with the other timestamps + # but it should at least be consistent with the other timestamps # used in the system. if 'dest-from-settings' in options: - if LOCAL_EXPORT in settings.PEARSON: - dest = settings.PEARSON[LOCAL_EXPORT] + if 'LOCAL_EXPORT' in settings.PEARSON: + dest = settings.PEARSON['LOCAL_EXPORT'] else: raise CommandError('--dest-from-settings was enabled but the' - 'PEARSON[LOCAL_EXPORT] setting was not set.') + 'PEARSON[LOCAL_EXPORT] setting was not set.') elif 'destination' in options: dest = options['destination'] else: raise CommandError('--destination or --dest-from-settings must be used') - if not os.path.isdir(dest): os.makedirs(dest) destfile = os.path.join(dest, uploaded_at.strftime("cdd-%Y%m%d-%H%M%S.dat")) - + # strings must be in latin-1 format. CSV parser will # otherwise convert unicode objects to ascii. def ensure_encoding(value): @@ -85,8 +85,8 @@ class Command(BaseCommand): return value.encode('iso-8859-1') else: return value - - dump_all = kwargs['dump_all'] + + dump_all = options['dump_all'] with open(destfile, "wb") as outfile: writer = csv.DictWriter(outfile, @@ -104,6 +104,3 @@ class Command(BaseCommand): writer.writerow(record) tcu.uploaded_at = uploaded_at tcu.save() - - - diff --git a/common/djangoapps/student/management/commands/pearson_export_ead.py b/common/djangoapps/student/management/commands/pearson_export_ead.py index 0f81cb1df9..9368ac5ddf 100644 --- a/common/djangoapps/student/management/commands/pearson_export_ead.py +++ b/common/djangoapps/student/management/commands/pearson_export_ead.py @@ -1,15 +1,16 @@ import csv +import os from collections import OrderedDict from datetime import datetime -from os.path import isdir, join from optparse import make_option from django.core.management.base import BaseCommand from student.models import TestCenterRegistration + class Command(BaseCommand): - + CSV_TO_MODEL_FIELDS = OrderedDict([ ('AuthorizationTransactionType', 'authorization_transaction_type'), ('AuthorizationID', 'authorization_id'), @@ -20,7 +21,7 @@ class Command(BaseCommand): ('Accommodations', 'accommodation_code'), ('EligibilityApptDateFirst', 'eligibility_appointment_date_first'), ('EligibilityApptDateLast', 'eligibility_appointment_date_last'), - ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store + ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store ]) option_list = BaseCommand.option_list + ( @@ -37,11 +38,11 @@ class Command(BaseCommand): make_option('--dump_all', action='store_true', dest='dump_all', - ), + ), make_option('--force_add', action='store_true', dest='force_add', - ), + ), ) def handle(self, **options): @@ -56,12 +57,12 @@ class Command(BaseCommand): # but it should at least be consistent with the other timestamps # used in the system. if 'dest-from-settings' in options: - if LOCAL_EXPORT in settings.PEARSON: - dest = settings.PEARSON[LOCAL_EXPORT] + if 'LOCAL_EXPORT' in settings.PEARSON: + dest = settings.PEARSON['LOCAL_EXPORT'] else: raise CommandError('--dest-from-settings was enabled but the' - 'PEARSON[LOCAL_EXPORT] setting was not set.') - elif destinations in options: + 'PEARSON[LOCAL_EXPORT] setting was not set.') + elif 'destinations' in options: dest = options['destination'] else: raise CommandError('--destination or --dest-from-settings must be used') @@ -71,7 +72,7 @@ class Command(BaseCommand): destfile = os.path.join(dest, uploaded_at.strftime("ead-%Y%m%d-%H%M%S.dat")) - dump_all = kwargs['dump_all'] + dump_all = options['dump_all'] with open(destfile, "wb") as outfile: writer = csv.DictWriter(outfile, @@ -88,13 +89,9 @@ class Command(BaseCommand): record["LastUpdate"] = record["LastUpdate"].strftime("%Y/%m/%d %H:%M:%S") record["EligibilityApptDateFirst"] = record["EligibilityApptDateFirst"].strftime("%Y/%m/%d") record["EligibilityApptDateLast"] = record["EligibilityApptDateLast"].strftime("%Y/%m/%d") - if kwargs['force_add']: + if options['force_add']: record['AuthorizationTransactionType'] = 'Add' writer.writerow(record) tcr.uploaded_at = uploaded_at tcr.save() - - - - diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index d07f75d011..8183bf3c05 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -1,11 +1,10 @@ from optparse import make_option -from django.contrib.auth.models import User from django.core.management.base import BaseCommand, CommandError -import re from dogapi import dog_http_api, dog_stats_api import paramiko import boto +import os dog_http_api.api_key = settings.DATADOG_API @@ -25,16 +24,26 @@ class Command(BaseCommand): if not settings.PEARSON: raise CommandError('No PEARSON entries in auth/env.json.') + for value in ['LOCAL_IMPORT', 'SFTP_IMPORT', 'BUCKET', 'LOCAL_EXPORT', + 'SFTP_EXPORT']: + if value not in settings.PEARSON: + raise CommandError('No entry in the PEARSON settings' + '(env/auth.json) for {0}'.format(value)) + def import_pearson(): - sftp(settings.PEARSON[SFTP_IMPORT], settings.PEARSON[LOCAL_IMPORT]) - s3(settings.PEARSON[LOCAL_IMPORT], settings.PEARSON[BUCKET]) + sftp(settings.PEARSON['SFTP_IMPORT'], + settings.PEARSON['LOCAL_IMPORT'], options['mode']) + s3(settings.PEARSON['LOCAL_IMPORT'], + settings.PEARSON['BUCKET'], options['mode']) call_command('pearson_import', 'dest_from_settings') def export_pearson(): call_command('pearson_export_ccd', 'dest_from_settings') call_command('pearson_export_ead', 'dest_from_settings') - sftp(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[SFTP_EXPORT]) - s3(settings.PEARSON[LOCAL_EXPORT], settings.PEARSON[BUCKET]) + sftp(settings.PEARSON['LOCAL_EXPORT'], + settings.PEARSON['SFTP_EXPORT'], options['mode']) + s3(settings.PEARSON['LOCAL_EXPORT'], + settings.PEARSON['BUCKET'], options['mode']) if options['mode'] == 'export': export_pearson() @@ -44,44 +53,44 @@ class Command(BaseCommand): export_pearson() import_pearson() - - def sftp(files_from, files_to): + def sftp(files_from, files_to, mode): with dog_stats_api.timer('pearson.{0}'.format(mode), tags='sftp'): try: - t = paramiko.Transport((hostname, 22)) - t.connect(username=settings.PEARSON[SFTP_USERNAME], - password=settings.PEARSON[SFTP_PASSWORD]) + t = paramiko.Transport((settings.PEARSON['SFTP_HOSTNAME'], 22)) + t.connect(username=settings.PEARSON['SFTP_USERNAME'], + password=settings.PEARSON['SFTP_PASSWORD']) sftp = paramiko.SFTPClient.from_transport(t) if os.path.isdir(files_from): for filename in os.listdir(files_from): - sftp.put(files_from+'/'+filename, - files_to+'/'+filename) + sftp.put(files_from + '/' + filename, + files_to + '/' + filename) else: for filename in sftp.listdir(files_from): - sftp.get(files_from+'/'+filename, - files_to+'/'+filename) + sftp.get(files_from + '/' + filename, + files_to + '/' + filename) t.close() except: dog_http_api.event('pearson {0}'.format(mode), - 'sftp uploading failed', alert_type='error') + 'sftp uploading failed', + alert_type='error') raise - def s3(files_from, bucket): + def s3(files_from, bucket, mode): with dog_stats_api.timer('pearson.{0}'.format(mode), tags='s3'): try: - for filename in os.listdir(files): - upload_file_to_s3(bucket, files_from+'/'+filename) + for filename in os.listdir(files_from): + upload_file_to_s3(bucket, files_from + '/' + filename) except: - dog_http_api.event('pearson {0}'.format(mode), 's3 archiving failed') + dog_http_api.event('pearson {0}'.format(mode), + 's3 archiving failed') raise - def upload_file_to_s3(bucket, filename): """ Upload file to S3 """ s3 = boto.connect_s3(settings.AWS_ACCESS_KEY_ID, - settings.AWS_SECRET_ACCESS_KEY) + settings.AWS_SECRET_ACCESS_KEY) from boto.s3.key import Key b = s3.get_bucket(bucket) k = Key(b) From 46e9a8f6ac58c1a79ee0097cd6fbfd829eae9b1d Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 10:09:11 -0500 Subject: [PATCH 14/82] Add help string. --- .../student/management/commands/pearson_transfer.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 8183bf3c05..4888806c73 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -10,6 +10,14 @@ dog_http_api.api_key = settings.DATADOG_API class Command(BaseCommand): + help = """ + This command handles the importing and exporting of student records for + Pearson. It uses some other Django commands to export and import the + files and then uploads over SFTP to pearson and stuffs the entry in an + S3 bucket for archive purposes. + + Usage: django-admin.py pearson-transfer --mode [import|export|both] + """ option_list = BaseCommand.option_list + ( make_option('--mode', From 5e76c9d033960221bb167d8e32caeae5898d93f5 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Thu, 17 Jan 2013 10:43:20 -0500 Subject: [PATCH 15/82] add pearson_import_conf_zip command --- .../commands/pearson_import_conf_zip.py | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 common/djangoapps/student/management/commands/pearson_import_conf_zip.py diff --git a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py new file mode 100644 index 0000000000..7b69a29dc2 --- /dev/null +++ b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py @@ -0,0 +1,108 @@ +import csv + +from zipfile import ZipFile, is_zipfile +from time import strptime, strftime + +from collections import OrderedDict +from datetime import datetime +from os.path import isdir +from optparse import make_option + +from django.core.management.base import BaseCommand, CommandError + +from student.models import TestCenterUser, TestCenterRegistration + +class Command(BaseCommand): + + + args = '' + help = """ + Import Pearson confirmation files and update TestCenterUser and TestCenterRegistration tables + with status. + """ + def handle(self, *args, **kwargs): + if len(args) < 1: + print Command.help + return + + source_zip = args[0] + if not is_zipfile(source_zip): + raise CommandError("Input file is not a zipfile: \"{}\"".format(source_zip)) + + # loop through all files in zip, and process them based on filename prefix: + with ZipFile(source_zip, 'r') as zipfile: + for fileinfo in zipfile.infolist(): + with zipfile.open(fileinfo) as zipentry: + if fileinfo.filename.startswith("eac-"): + self.process_eac(zipentry) + elif fileinfo.filename.startswith("vcdc-"): + self.process_vcdc(zipentry) + else: + raise CommandError("Unrecognized confirmation file type \"{}\" in confirmation zip file \"{}\"".format(fileinfo.filename, zipfile)) + + def process_eac(self, eacfile): + print "processing eac" + reader = csv.DictReader(eacfile, delimiter="\t") + for row in reader: + client_authorization_id = row['ClientAuthorizationID'] + if not client_authorization_id: + if row['Status'] == 'Error': + print "Error in EAD file processing ({}): {}".format(row['Date'], row['Message']) + else: + print "Encountered bad record: {}".format(row) + else: + try: + registration = TestCenterRegistration.objects.get(client_authorization_id=client_authorization_id) + print "Found authorization record for user {}".format(registration.testcenter_user.user.username) + # now update the record: + registration.upload_status = row['Status'] + registration.upload_error_message = row['Message'] + try: + registration.processed_at = strftime('%Y-%m-%d %H:%M:%S', strptime(row['Date'], '%Y/%m/%d %H:%M:%S')) + except ValueError as ve: + print "Bad Date value found for {}: message {}".format(client_authorization_id, ve) + # store the authorization Id if one is provided. (For debugging) + if row['AuthorizationID']: + try: + registration.authorization_id = int(row['AuthorizationID']) + except ValueError as ve: + print "Bad AuthorizationID value found for {}: message {}".format(client_authorization_id, ve) + + registration.confirmed_at = datetime.utcnow() + registration.save() + except TestCenterRegistration.DoesNotExist: + print " Failed to find record for client_auth_id {}".format(client_authorization_id) + + + def process_vcdc(self, vcdcfile): + print "processing vcdc" + reader = csv.DictReader(vcdcfile, delimiter="\t") + for row in reader: + client_candidate_id = row['ClientCandidateID'] + if not client_candidate_id: + if row['Status'] == 'Error': + print "Error in CDD file processing ({}): {}".format(row['Date'], row['Message']) + else: + print "Encountered bad record: {}".format(row) + else: + try: + tcuser = TestCenterUser.objects.get(client_candidate_id=client_candidate_id) + print "Found demographics record for user {}".format(tcuser.user.username) + # now update the record: + tcuser.upload_status = row['Status'] + tcuser.upload_error_message = row['Message'] + try: + tcuser.processed_at = strftime('%Y-%m-%d %H:%M:%S', strptime(row['Date'], '%Y/%m/%d %H:%M:%S')) + except ValueError as ve: + print "Bad Date value found for {}: message {}".format(client_candidate_id, ve) + # store the candidate Id if one is provided. (For debugging) + if row['CandidateID']: + try: + tcuser.candidate_id = int(row['CandidateID']) + except ValueError as ve: + print "Bad CandidateID value found for {}: message {}".format(client_candidate_id, ve) + tcuser.confirmed_at = datetime.utcnow() + tcuser.save() + except TestCenterUser.DoesNotExist: + print " Failed to find record for client_candidate_id {}".format(client_candidate_id) + From 333f2e5167556043e2f09de06aa297ab08ee50de Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 10:48:16 -0500 Subject: [PATCH 16/82] Fix ccd->cdd, I typed ccd so many times while working on this code. I am bad at typing! --- .../djangoapps/student/management/commands/pearson_transfer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 4888806c73..57401897dc 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -46,7 +46,7 @@ class Command(BaseCommand): call_command('pearson_import', 'dest_from_settings') def export_pearson(): - call_command('pearson_export_ccd', 'dest_from_settings') + call_command('pearson_export_cdd', 'dest_from_settings') call_command('pearson_export_ead', 'dest_from_settings') sftp(settings.PEARSON['LOCAL_EXPORT'], settings.PEARSON['SFTP_EXPORT'], options['mode']) From 3a4091b798df7d4c1f7c9f1f3686eac77235cba3 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 13:54:43 -0500 Subject: [PATCH 17/82] Tweaks to enable datadog error events as well as some pep8 tidyup vim was shouting at me about. --- .../commands/pearson_import_conf_zip.py | 55 ++++++++++--------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py index 7b69a29dc2..74037b708f 100644 --- a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py +++ b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py @@ -7,19 +7,25 @@ from collections import OrderedDict from datetime import datetime from os.path import isdir from optparse import make_option +from dogapi import dog_http_api, dog_stats_api from django.core.management.base import BaseCommand, CommandError from student.models import TestCenterUser, TestCenterRegistration + class Command(BaseCommand): - - + + dog_http_api.api_key = settings.DATADOG_API args = '' help = """ - Import Pearson confirmation files and update TestCenterUser and TestCenterRegistration tables - with status. + Import Pearson confirmation files and update TestCenterUser + and TestCenterRegistration tables with status. """ + + def datadog_error(string): + dog_http_api.event("Pearson Import", string, alert_type='error') + def handle(self, *args, **kwargs): if len(args) < 1: print Command.help @@ -27,8 +33,8 @@ class Command(BaseCommand): source_zip = args[0] if not is_zipfile(source_zip): - raise CommandError("Input file is not a zipfile: \"{}\"".format(source_zip)) - + raise CommandError("Input file is not a zipfile: \"{}\"".format(source_zip)) + # loop through all files in zip, and process them based on filename prefix: with ZipFile(source_zip, 'r') as zipfile: for fileinfo in zipfile.infolist(): @@ -39,70 +45,69 @@ class Command(BaseCommand): self.process_vcdc(zipentry) else: raise CommandError("Unrecognized confirmation file type \"{}\" in confirmation zip file \"{}\"".format(fileinfo.filename, zipfile)) - - def process_eac(self, eacfile): + + def process_eac(self, eacfile): print "processing eac" reader = csv.DictReader(eacfile, delimiter="\t") for row in reader: client_authorization_id = row['ClientAuthorizationID'] if not client_authorization_id: if row['Status'] == 'Error': - print "Error in EAD file processing ({}): {}".format(row['Date'], row['Message']) + Command.datadog_error("Error in EAD file processing ({}): {}".format(row['Date'], row['Message'])) else: - print "Encountered bad record: {}".format(row) + Command.datadog_error("Encountered bad record: {}".format(row)) else: try: registration = TestCenterRegistration.objects.get(client_authorization_id=client_authorization_id) - print "Found authorization record for user {}".format(registration.testcenter_user.user.username) + Command.datadog_error("Found authorization record for user {}".format(registration.testcenter_user.user.username)) # now update the record: registration.upload_status = row['Status'] registration.upload_error_message = row['Message'] try: registration.processed_at = strftime('%Y-%m-%d %H:%M:%S', strptime(row['Date'], '%Y/%m/%d %H:%M:%S')) except ValueError as ve: - print "Bad Date value found for {}: message {}".format(client_authorization_id, ve) + Command.datadog_error("Bad Date value found for {}: message {}".format(client_authorization_id, ve)) # store the authorization Id if one is provided. (For debugging) if row['AuthorizationID']: try: registration.authorization_id = int(row['AuthorizationID']) except ValueError as ve: - print "Bad AuthorizationID value found for {}: message {}".format(client_authorization_id, ve) - + Command.datadog_error("Bad AuthorizationID value found for {}: message {}".format(client_authorization_id, ve)) + registration.confirmed_at = datetime.utcnow() registration.save() except TestCenterRegistration.DoesNotExist: - print " Failed to find record for client_auth_id {}".format(client_authorization_id) + Command.datadog_error("Failed to find record for client_auth_id {}".format(client_authorization_id)) - - def process_vcdc(self, vcdcfile): + def process_vcdc(self, vcdcfile): print "processing vcdc" reader = csv.DictReader(vcdcfile, delimiter="\t") for row in reader: client_candidate_id = row['ClientCandidateID'] if not client_candidate_id: if row['Status'] == 'Error': - print "Error in CDD file processing ({}): {}".format(row['Date'], row['Message']) + Command.datadog_error("Error in CDD file processing ({}): {}".format(row['Date'], row['Message'])) else: - print "Encountered bad record: {}".format(row) + Command.datadog_error("Encountered bad record: {}".format(row)) else: try: tcuser = TestCenterUser.objects.get(client_candidate_id=client_candidate_id) - print "Found demographics record for user {}".format(tcuser.user.username) + Command.datadog_error("Found demographics record for user {}".format(tcuser.user.username)) # now update the record: tcuser.upload_status = row['Status'] - tcuser.upload_error_message = row['Message'] + tcuser.upload_error_message = row['Message'] try: tcuser.processed_at = strftime('%Y-%m-%d %H:%M:%S', strptime(row['Date'], '%Y/%m/%d %H:%M:%S')) except ValueError as ve: - print "Bad Date value found for {}: message {}".format(client_candidate_id, ve) + Command.datadog_error("Bad Date value found for {}: message {}".format(client_candidate_id, ve)) # store the candidate Id if one is provided. (For debugging) if row['CandidateID']: try: tcuser.candidate_id = int(row['CandidateID']) except ValueError as ve: - print "Bad CandidateID value found for {}: message {}".format(client_candidate_id, ve) + Command.datadog_error("Bad CandidateID value found for {}: message {}".format(client_candidate_id, ve)) tcuser.confirmed_at = datetime.utcnow() tcuser.save() except TestCenterUser.DoesNotExist: - print " Failed to find record for client_candidate_id {}".format(client_candidate_id) - + Command.datadog_error(" Failed to find record for client_candidate_id {}".format(client_candidate_id)) + From 05b36bdd092de2294381fb2fcbbc2fbb03b63ed6 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 15:43:37 -0500 Subject: [PATCH 18/82] Add tags to the datadog event which will be set to the appropriate filename so we can see the cause of failures. --- .../commands/pearson_import_conf_zip.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py index 74037b708f..a491a73868 100644 --- a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py +++ b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py @@ -23,8 +23,8 @@ class Command(BaseCommand): and TestCenterRegistration tables with status. """ - def datadog_error(string): - dog_http_api.event("Pearson Import", string, alert_type='error') + def datadog_error(string, tags): + dog_http_api.event("Pearson Import", string, alert_type='error', tags=tags) def handle(self, *args, **kwargs): if len(args) < 1: @@ -44,7 +44,9 @@ class Command(BaseCommand): elif fileinfo.filename.startswith("vcdc-"): self.process_vcdc(zipentry) else: - raise CommandError("Unrecognized confirmation file type \"{}\" in confirmation zip file \"{}\"".format(fileinfo.filename, zipfile)) + error = "Unrecognized confirmation file type\"{}\" in confirmation zip file \"{}\"".format(fileinfo.filename, zipfile) + Command.datadog_error(error, source_zip) + raise CommandError(error) def process_eac(self, eacfile): print "processing eac" @@ -53,31 +55,31 @@ class Command(BaseCommand): client_authorization_id = row['ClientAuthorizationID'] if not client_authorization_id: if row['Status'] == 'Error': - Command.datadog_error("Error in EAD file processing ({}): {}".format(row['Date'], row['Message'])) + Command.datadog_error("Error in EAD file processing ({}): {}".format(row['Date'], row['Message']), eacfile) else: - Command.datadog_error("Encountered bad record: {}".format(row)) + Command.datadog_error("Encountered bad record: {}".format(row), eacfile) else: try: registration = TestCenterRegistration.objects.get(client_authorization_id=client_authorization_id) - Command.datadog_error("Found authorization record for user {}".format(registration.testcenter_user.user.username)) + Command.datadog_error("Found authorization record for user {}".format(registration.testcenter_user.user.username), eacfile) # now update the record: registration.upload_status = row['Status'] registration.upload_error_message = row['Message'] try: registration.processed_at = strftime('%Y-%m-%d %H:%M:%S', strptime(row['Date'], '%Y/%m/%d %H:%M:%S')) except ValueError as ve: - Command.datadog_error("Bad Date value found for {}: message {}".format(client_authorization_id, ve)) + Command.datadog_error("Bad Date value found for {}: message {}".format(client_authorization_id, ve), eacfile) # store the authorization Id if one is provided. (For debugging) if row['AuthorizationID']: try: registration.authorization_id = int(row['AuthorizationID']) except ValueError as ve: - Command.datadog_error("Bad AuthorizationID value found for {}: message {}".format(client_authorization_id, ve)) + Command.datadog_error("Bad AuthorizationID value found for {}: message {}".format(client_authorization_id, ve), eacfile) registration.confirmed_at = datetime.utcnow() registration.save() except TestCenterRegistration.DoesNotExist: - Command.datadog_error("Failed to find record for client_auth_id {}".format(client_authorization_id)) + Command.datadog_error("Failed to find record for client_auth_id {}".format(client_authorization_id), eacfile) def process_vcdc(self, vcdcfile): print "processing vcdc" @@ -86,28 +88,28 @@ class Command(BaseCommand): client_candidate_id = row['ClientCandidateID'] if not client_candidate_id: if row['Status'] == 'Error': - Command.datadog_error("Error in CDD file processing ({}): {}".format(row['Date'], row['Message'])) + Command.datadog_error("Error in CDD file processing ({}): {}".format(row['Date'], row['Message']), vcdcfile) else: - Command.datadog_error("Encountered bad record: {}".format(row)) + Command.datadog_error("Encountered bad record: {}".format(row), vcdcfile) else: try: tcuser = TestCenterUser.objects.get(client_candidate_id=client_candidate_id) - Command.datadog_error("Found demographics record for user {}".format(tcuser.user.username)) + Command.datadog_error("Found demographics record for user {}".format(tcuser.user.username), vcdcfile) # now update the record: tcuser.upload_status = row['Status'] tcuser.upload_error_message = row['Message'] try: tcuser.processed_at = strftime('%Y-%m-%d %H:%M:%S', strptime(row['Date'], '%Y/%m/%d %H:%M:%S')) except ValueError as ve: - Command.datadog_error("Bad Date value found for {}: message {}".format(client_candidate_id, ve)) + Command.datadog_error("Bad Date value found for {}: message {}".format(client_candidate_id, ve), vcdcfile) # store the candidate Id if one is provided. (For debugging) if row['CandidateID']: try: tcuser.candidate_id = int(row['CandidateID']) except ValueError as ve: - Command.datadog_error("Bad CandidateID value found for {}: message {}".format(client_candidate_id, ve)) + Command.datadog_error("Bad CandidateID value found for {}: message {}".format(client_candidate_id, ve), vcdcfile) tcuser.confirmed_at = datetime.utcnow() tcuser.save() except TestCenterUser.DoesNotExist: - Command.datadog_error(" Failed to find record for client_candidate_id {}".format(client_candidate_id)) + Command.datadog_error(" Failed to find record for client_candidate_id {}".format(client_candidate_id), vcdcfile) From 3aacba1f63b48b6d72883c2fd0a8b56d26573cc1 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 15:44:08 -0500 Subject: [PATCH 19/82] Run over each file and run the import. We could probably do this as a try/except and not delete if the output of the import failed but it may be simply easier to refetch those files from the S3 backup and try again. --- .../student/management/commands/pearson_transfer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 57401897dc..0037231a38 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -43,7 +43,9 @@ class Command(BaseCommand): settings.PEARSON['LOCAL_IMPORT'], options['mode']) s3(settings.PEARSON['LOCAL_IMPORT'], settings.PEARSON['BUCKET'], options['mode']) - call_command('pearson_import', 'dest_from_settings') + for file in os.listdir(settings.PEARSON['LOCAL_IMPORT']): + call_command('pearson_import_conf_zip', 'dest_from_settings') + os.remove(file) def export_pearson(): call_command('pearson_export_cdd', 'dest_from_settings') From 274cb8d865f00734e383a17ae3bdc29883acae45 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 15:50:37 -0500 Subject: [PATCH 20/82] Actually call the appropriate file. --- .../djangoapps/student/management/commands/pearson_transfer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 0037231a38..aede8fe92d 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -44,7 +44,8 @@ class Command(BaseCommand): s3(settings.PEARSON['LOCAL_IMPORT'], settings.PEARSON['BUCKET'], options['mode']) for file in os.listdir(settings.PEARSON['LOCAL_IMPORT']): - call_command('pearson_import_conf_zip', 'dest_from_settings') + call_command('pearson_import_conf_zip', + settings.PEARSON['LOCAL_IMPORT'] + '/' + file) os.remove(file) def export_pearson(): From d0ecae30d7edaa64ae24b383c532a34faebd03d3 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 16:24:44 -0500 Subject: [PATCH 21/82] Fix datadog logging to use .name on the file objects and add an additional logging line. --- .../commands/pearson_import_conf_zip.py | 26 ++++++++++--------- .../management/commands/pearson_transfer.py | 21 +++++++++------ 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py index a491a73868..fa9741dc68 100644 --- a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py +++ b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py @@ -33,7 +33,9 @@ class Command(BaseCommand): source_zip = args[0] if not is_zipfile(source_zip): - raise CommandError("Input file is not a zipfile: \"{}\"".format(source_zip)) + error = "Input file is not a zipfile: \"{}\"".format(source_zip) + Command.datadog_error(error, source_zip) + raise CommandError(error) # loop through all files in zip, and process them based on filename prefix: with ZipFile(source_zip, 'r') as zipfile: @@ -55,9 +57,9 @@ class Command(BaseCommand): client_authorization_id = row['ClientAuthorizationID'] if not client_authorization_id: if row['Status'] == 'Error': - Command.datadog_error("Error in EAD file processing ({}): {}".format(row['Date'], row['Message']), eacfile) + Command.datadog_error("Error in EAD file processing ({}): {}".format(row['Date'], row['Message']), eacfile.name) else: - Command.datadog_error("Encountered bad record: {}".format(row), eacfile) + Command.datadog_error("Encountered bad record: {}".format(row), eacfile.name) else: try: registration = TestCenterRegistration.objects.get(client_authorization_id=client_authorization_id) @@ -68,18 +70,18 @@ class Command(BaseCommand): try: registration.processed_at = strftime('%Y-%m-%d %H:%M:%S', strptime(row['Date'], '%Y/%m/%d %H:%M:%S')) except ValueError as ve: - Command.datadog_error("Bad Date value found for {}: message {}".format(client_authorization_id, ve), eacfile) + Command.datadog_error("Bad Date value found for {}: message {}".format(client_authorization_id, ve), eacfile.name) # store the authorization Id if one is provided. (For debugging) if row['AuthorizationID']: try: registration.authorization_id = int(row['AuthorizationID']) except ValueError as ve: - Command.datadog_error("Bad AuthorizationID value found for {}: message {}".format(client_authorization_id, ve), eacfile) + Command.datadog_error("Bad AuthorizationID value found for {}: message {}".format(client_authorization_id, ve), eacfile.name) registration.confirmed_at = datetime.utcnow() registration.save() except TestCenterRegistration.DoesNotExist: - Command.datadog_error("Failed to find record for client_auth_id {}".format(client_authorization_id), eacfile) + Command.datadog_error("Failed to find record for client_auth_id {}".format(client_authorization_id), eacfile.name) def process_vcdc(self, vcdcfile): print "processing vcdc" @@ -88,28 +90,28 @@ class Command(BaseCommand): client_candidate_id = row['ClientCandidateID'] if not client_candidate_id: if row['Status'] == 'Error': - Command.datadog_error("Error in CDD file processing ({}): {}".format(row['Date'], row['Message']), vcdcfile) + Command.datadog_error("Error in CDD file processing ({}): {}".format(row['Date'], row['Message']), vcdcfile.name) else: - Command.datadog_error("Encountered bad record: {}".format(row), vcdcfile) + Command.datadog_error("Encountered bad record: {}".format(row), vcdcfile.name) else: try: tcuser = TestCenterUser.objects.get(client_candidate_id=client_candidate_id) - Command.datadog_error("Found demographics record for user {}".format(tcuser.user.username), vcdcfile) + Command.datadog_error("Found demographics record for user {}".format(tcuser.user.username), vcdcfile.name) # now update the record: tcuser.upload_status = row['Status'] tcuser.upload_error_message = row['Message'] try: tcuser.processed_at = strftime('%Y-%m-%d %H:%M:%S', strptime(row['Date'], '%Y/%m/%d %H:%M:%S')) except ValueError as ve: - Command.datadog_error("Bad Date value found for {}: message {}".format(client_candidate_id, ve), vcdcfile) + Command.datadog_error("Bad Date value found for {}: message {}".format(client_candidate_id, ve), vcdcfile.name) # store the candidate Id if one is provided. (For debugging) if row['CandidateID']: try: tcuser.candidate_id = int(row['CandidateID']) except ValueError as ve: - Command.datadog_error("Bad CandidateID value found for {}: message {}".format(client_candidate_id, ve), vcdcfile) + Command.datadog_error("Bad CandidateID value found for {}: message {}".format(client_candidate_id, ve), vcdcfile.name) tcuser.confirmed_at = datetime.utcnow() tcuser.save() except TestCenterUser.DoesNotExist: - Command.datadog_error(" Failed to find record for client_candidate_id {}".format(client_candidate_id), vcdcfile) + Command.datadog_error(" Failed to find record for client_candidate_id {}".format(client_candidate_id), vcdcfile.name) diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index aede8fe92d..5f126a24f0 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -39,14 +39,18 @@ class Command(BaseCommand): '(env/auth.json) for {0}'.format(value)) def import_pearson(): - sftp(settings.PEARSON['SFTP_IMPORT'], - settings.PEARSON['LOCAL_IMPORT'], options['mode']) - s3(settings.PEARSON['LOCAL_IMPORT'], - settings.PEARSON['BUCKET'], options['mode']) - for file in os.listdir(settings.PEARSON['LOCAL_IMPORT']): - call_command('pearson_import_conf_zip', - settings.PEARSON['LOCAL_IMPORT'] + '/' + file) - os.remove(file) + try: + sftp(settings.PEARSON['SFTP_IMPORT'], + settings.PEARSON['LOCAL_IMPORT'], options['mode']) + s3(settings.PEARSON['LOCAL_IMPORT'], + settings.PEARSON['BUCKET'], options['mode']) + except Exception as e: + dog_http_api.event('Pearson Import failure', str(e)) + else: + for file in os.listdir(settings.PEARSON['LOCAL_IMPORT']): + call_command('pearson_import_conf_zip', + settings.PEARSON['LOCAL_IMPORT'] + '/' + file) + os.remove(file) def export_pearson(): call_command('pearson_export_cdd', 'dest_from_settings') @@ -79,6 +83,7 @@ class Command(BaseCommand): for filename in sftp.listdir(files_from): sftp.get(files_from + '/' + filename, files_to + '/' + filename) + sftp.remove(files_from + '/' + filename) t.close() except: dog_http_api.event('pearson {0}'.format(mode), From 491dd408aa7193553e8d5f38e7386e150d7f3e7c Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 16:31:21 -0500 Subject: [PATCH 22/82] Add dogapi for datadog. --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index fa4688b711..7e4d913c30 100644 --- a/requirements.txt +++ b/requirements.txt @@ -59,3 +59,4 @@ Shapely==1.2.16 ipython==0.13.1 xmltodict==0.4.1 paramiko==1.9.0 +dogapi==1.1.2 From 60ae54b2e15a14095af8c5c2bef5111ef57d9e06 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 16:32:34 -0500 Subject: [PATCH 23/82] Actually, lets use Cale's fixes. --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 7e4d913c30..b1d769e9c2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -59,4 +59,4 @@ Shapely==1.2.16 ipython==0.13.1 xmltodict==0.4.1 paramiko==1.9.0 -dogapi==1.1.2 +git+ssh://git@github.com/MITx/dogapi.git@003a4fc9#egg=dogapi% From c5979f8efa749376b32f03b4c112c1a39fd514ca Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 17 Jan 2013 16:38:35 -0500 Subject: [PATCH 24/82] Stray % --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b1d769e9c2..1b1384912b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -59,4 +59,4 @@ Shapely==1.2.16 ipython==0.13.1 xmltodict==0.4.1 paramiko==1.9.0 -git+ssh://git@github.com/MITx/dogapi.git@003a4fc9#egg=dogapi% +git+ssh://git@github.com/MITx/dogapi.git@003a4fc9#egg=dogapi From 7bcfc44b7141d0171dc3a372647d36a81689f4ae Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Thu, 17 Jan 2013 17:50:26 -0500 Subject: [PATCH 25/82] add first test shell --- .../management/commands/test/__init__.py | 0 .../management/commands/test/test_pearson.py | 53 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 common/djangoapps/student/management/commands/test/__init__.py create mode 100644 common/djangoapps/student/management/commands/test/test_pearson.py diff --git a/common/djangoapps/student/management/commands/test/__init__.py b/common/djangoapps/student/management/commands/test/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/student/management/commands/test/test_pearson.py b/common/djangoapps/student/management/commands/test/test_pearson.py new file mode 100644 index 0000000000..b7f24ce232 --- /dev/null +++ b/common/djangoapps/student/management/commands/test/test_pearson.py @@ -0,0 +1,53 @@ +''' +Created on Jan 17, 2013 + +@author: brian +''' +import logging + +from django.test import TestCase +from student.models import User, TestCenterRegistration, TestCenterUser, unique_id_for_user +from mock import Mock +from datetime import datetime +from django.core import management + +COURSE_1 = 'edX/toy/2012_Fall' +COURSE_2 = 'edx/full/6.002_Spring_2012' + +log = logging.getLogger(__name__) + +class PearsonTestCase(TestCase): + ''' + Base class for tests running Pearson-related commands + ''' + + def test_create_good_testcenter_user(self): + username = "rusty" +# user = Mock(username=username) +# # id = unique_id_for_user(user) +# course = Mock(end_of_course_survey_url=survey_url) + + + newuser = User.objects.create_user(username, 'rusty@edx.org', 'fakepass') +# newuser.first_name='Rusty' +# newuser.last_name='Skids' +# newuser.is_staff=True +# newuser.is_active=True +# newuser.is_superuser=True +# newuser.last_login=datetime(2012, 1, 1) +# newuser.date_joined=datetime(2011, 1, 1) + +# newuser.save(using='default') + options = { + 'first_name' : 'TestFirst', + 'last_name' : 'TestLast', + 'address_1' : 'Test Address', + 'city' : 'TestCity', + 'state' : 'Alberta', + 'postal_code' : 'A0B 1C2', + 'country' : 'CAN', + 'phone' : '252-1866', + 'phone_country_code' : '1', + } + management.call_command('pearson_make_tc_user', username, options) + \ No newline at end of file From d30974b560e23e0a0f8e9c1ecd495c689f69c21d Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Fri, 18 Jan 2013 04:00:32 -0500 Subject: [PATCH 26/82] Get pearson export working in a unit test --- .../management/commands/pearson_export_cdd.py | 5 +- .../management/commands/pearson_export_ead.py | 1 + .../management/commands/test/test_pearson.py | 84 ++++++++++++------- common/djangoapps/student/models.py | 4 + 4 files changed, 62 insertions(+), 32 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_export_cdd.py b/common/djangoapps/student/management/commands/pearson_export_cdd.py index 14c652f11e..ba43dd3bc0 100644 --- a/common/djangoapps/student/management/commands/pearson_export_cdd.py +++ b/common/djangoapps/student/management/commands/pearson_export_cdd.py @@ -4,6 +4,7 @@ from collections import OrderedDict from datetime import datetime from optparse import make_option +from django.conf import settings from django.core.management.base import BaseCommand from student.models import TestCenterUser @@ -86,7 +87,7 @@ class Command(BaseCommand): else: return value - dump_all = options['dump_all'] +# dump_all = options['dump_all'] with open(destfile, "wb") as outfile: writer = csv.DictWriter(outfile, @@ -96,7 +97,7 @@ class Command(BaseCommand): extrasaction='ignore') writer.writeheader() for tcu in TestCenterUser.objects.order_by('id'): - if dump_all or tcu.needs_uploading: + if tcu.needs_uploading: # or dump_all record = dict((csv_field, ensure_encoding(getattr(tcu, model_field))) for csv_field, model_field in Command.CSV_TO_MODEL_FIELDS.items()) diff --git a/common/djangoapps/student/management/commands/pearson_export_ead.py b/common/djangoapps/student/management/commands/pearson_export_ead.py index 9368ac5ddf..492cba154b 100644 --- a/common/djangoapps/student/management/commands/pearson_export_ead.py +++ b/common/djangoapps/student/management/commands/pearson_export_ead.py @@ -4,6 +4,7 @@ from collections import OrderedDict from datetime import datetime from optparse import make_option +from django.conf import settings from django.core.management.base import BaseCommand from student.models import TestCenterRegistration diff --git a/common/djangoapps/student/management/commands/test/test_pearson.py b/common/djangoapps/student/management/commands/test/test_pearson.py index b7f24ce232..e0ded22a1d 100644 --- a/common/djangoapps/student/management/commands/test/test_pearson.py +++ b/common/djangoapps/student/management/commands/test/test_pearson.py @@ -6,39 +6,18 @@ Created on Jan 17, 2013 import logging from django.test import TestCase -from student.models import User, TestCenterRegistration, TestCenterUser, unique_id_for_user -from mock import Mock -from datetime import datetime +from student.models import User, TestCenterRegistration, TestCenterUser +# This is stupid! Because I import a function with the word "test" in the name, +# the unittest framework tries to run *it* as a test?! Crazy! +from student.models import get_testcenter_registration as get_tc_registration from django.core import management -COURSE_1 = 'edX/toy/2012_Fall' -COURSE_2 = 'edx/full/6.002_Spring_2012' - log = logging.getLogger(__name__) -class PearsonTestCase(TestCase): - ''' - Base class for tests running Pearson-related commands - ''' - def test_create_good_testcenter_user(self): - username = "rusty" -# user = Mock(username=username) -# # id = unique_id_for_user(user) -# course = Mock(end_of_course_survey_url=survey_url) - - - newuser = User.objects.create_user(username, 'rusty@edx.org', 'fakepass') -# newuser.first_name='Rusty' -# newuser.last_name='Skids' -# newuser.is_staff=True -# newuser.is_active=True -# newuser.is_superuser=True -# newuser.last_login=datetime(2012, 1, 1) -# newuser.date_joined=datetime(2011, 1, 1) - -# newuser.save(using='default') - options = { +def create_tc_user(username): + user = User.objects.create_user(username, '{}@edx.org'.format(username), 'fakepass') + options = { 'first_name' : 'TestFirst', 'last_name' : 'TestLast', 'address_1' : 'Test Address', @@ -49,5 +28,50 @@ class PearsonTestCase(TestCase): 'phone' : '252-1866', 'phone_country_code' : '1', } - management.call_command('pearson_make_tc_user', username, options) - \ No newline at end of file + management.call_command('pearson_make_tc_user', username, **options) + return TestCenterUser.objects.get(user=user) + + +def create_tc_registration(username, course_id, exam_code, accommodation_code): + + options = { 'exam_series_code' : exam_code, + 'eligibility_appointment_date_first' : '2013-01-01T00:00', + 'eligibility_appointment_date_last' : '2013-12-31T23:59', + 'accommodation_code' : accommodation_code, + } + + management.call_command('pearson_make_tc_registration', username, course_id, **options) + user = User.objects.get(username=username) + registrations = get_tc_registration(user, course_id, exam_code) + return registrations[0] + +class PearsonTestCase(TestCase): + ''' + Base class for tests running Pearson-related commands + ''' + + def test_create_good_testcenter_user(self): + testcenter_user = create_tc_user("test1") + + def test_create_good_testcenter_registration(self): + username = 'test1' + course_id = 'org1/course1/term1' + exam_code = 'exam1' + accommodation_code = 'NONE' + testcenter_user = create_tc_user(username) + registration = create_tc_registration(username, course_id, exam_code, accommodation_code) + + def test_export(self): + username = 'test1' + course_id = 'org1/course1/term1' + exam_code = 'exam1' + accommodation_code = 'NONE' + testcenter_user = create_tc_user(username) + registration = create_tc_registration(username, course_id, exam_code, accommodation_code) + output_dir = "./tmpOutput" + options = { 'destination' : output_dir } + with self.settings(PEARSON={ 'LOCAL_EXPORT' : output_dir }): + management.call_command('pearson_export_cdd', **options) + management.call_command('pearson_export_ead', **options) + # TODO: check that files were output.... + diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index f13a691215..c9cb94d81a 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -428,6 +428,10 @@ class TestCenterRegistration(models.Model): # TODO: figure out if this should really go in the database (with a default value). return 1 + @property + def needs_uploading(self): + return self.uploaded_at is None or self.uploaded_at < self.user_updated_at + @classmethod def create(cls, testcenter_user, exam, accommodation_request): registration = cls(testcenter_user = testcenter_user) From 553528cd1c900e654e0c1556c0afe26dbbe57267 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Fri, 18 Jan 2013 14:52:36 -0500 Subject: [PATCH 27/82] change get_testcenter_registration to get_tc_registration, so it's not treated as a test. --- .../management/commands/pearson_make_tc_registration.py | 6 +++--- .../student/management/commands/{test => tests}/__init__.py | 0 .../management/commands/{test => tests}/test_pearson.py | 2 +- common/djangoapps/student/models.py | 2 +- common/djangoapps/student/views.py | 6 +++--- 5 files changed, 8 insertions(+), 8 deletions(-) rename common/djangoapps/student/management/commands/{test => tests}/__init__.py (100%) rename common/djangoapps/student/management/commands/{test => tests}/test_pearson.py (97%) diff --git a/common/djangoapps/student/management/commands/pearson_make_tc_registration.py b/common/djangoapps/student/management/commands/pearson_make_tc_registration.py index 81a478d19d..545c8cd8a8 100644 --- a/common/djangoapps/student/management/commands/pearson_make_tc_registration.py +++ b/common/djangoapps/student/management/commands/pearson_make_tc_registration.py @@ -4,7 +4,7 @@ from time import strftime from django.contrib.auth.models import User from django.core.management.base import BaseCommand, CommandError -from student.models import TestCenterUser, TestCenterRegistration, TestCenterRegistrationForm, get_testcenter_registration +from student.models import TestCenterUser, TestCenterRegistration, TestCenterRegistrationForm, get_tc_registration from student.views import course_from_id from xmodule.course_module import CourseDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError @@ -134,7 +134,7 @@ class Command(BaseCommand): # create and save the registration: needs_updating = False - registrations = get_testcenter_registration(student, course_id, exam_code) + registrations = get_tc_registration(student, course_id, exam_code) if len(registrations) > 0: registration = registrations[0] for fieldname in UPDATE_FIELDS: @@ -181,7 +181,7 @@ class Command(BaseCommand): change_internal = False if 'exam_series_code' in our_options: exam_code = our_options['exam_series_code'] - registration = get_testcenter_registration(student, course_id, exam_code)[0] + registration = get_tc_registration(student, course_id, exam_code)[0] for internal_field in [ 'upload_error_message', 'upload_status', 'authorization_id']: if internal_field in our_options: registration.__setattr__(internal_field, our_options[internal_field]) diff --git a/common/djangoapps/student/management/commands/test/__init__.py b/common/djangoapps/student/management/commands/tests/__init__.py similarity index 100% rename from common/djangoapps/student/management/commands/test/__init__.py rename to common/djangoapps/student/management/commands/tests/__init__.py diff --git a/common/djangoapps/student/management/commands/test/test_pearson.py b/common/djangoapps/student/management/commands/tests/test_pearson.py similarity index 97% rename from common/djangoapps/student/management/commands/test/test_pearson.py rename to common/djangoapps/student/management/commands/tests/test_pearson.py index e0ded22a1d..29f9120e98 100644 --- a/common/djangoapps/student/management/commands/test/test_pearson.py +++ b/common/djangoapps/student/management/commands/tests/test_pearson.py @@ -9,7 +9,7 @@ from django.test import TestCase from student.models import User, TestCenterRegistration, TestCenterUser # This is stupid! Because I import a function with the word "test" in the name, # the unittest framework tries to run *it* as a test?! Crazy! -from student.models import get_testcenter_registration as get_tc_registration +from student.models import get_tc_registration from django.core import management log = logging.getLogger(__name__) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index c9cb94d81a..84bbc76a80 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -547,7 +547,7 @@ class TestCenterRegistrationForm(ModelForm): -def get_testcenter_registration(user, course_id, exam_series_code): +def get_tc_registration(user, course_id, exam_series_code): try: tcu = TestCenterUser.objects.get(user=user) except TestCenterUser.DoesNotExist: diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 61b49e6022..650f0a0280 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -31,7 +31,7 @@ from student.models import (Registration, UserProfile, TestCenterUser, TestCente TestCenterRegistration, TestCenterRegistrationForm, PendingNameChange, PendingEmailChange, CourseEnrollment, unique_id_for_user, - get_testcenter_registration) + get_tc_registration) from certificates.models import CertificateStatuses, certificate_status_for_student @@ -612,7 +612,7 @@ def exam_registration_info(user, course): return None exam_code = exam_info.exam_series_code - registrations = get_testcenter_registration(user, course.id, exam_code) + registrations = get_tc_registration(user, course.id, exam_code) if registrations: registration = registrations[0] else: @@ -712,7 +712,7 @@ def create_exam_registration(request, post_override=None): needs_saving = False exam = course.current_test_center_exam exam_code = exam.exam_series_code - registrations = get_testcenter_registration(user, course_id, exam_code) + registrations = get_tc_registration(user, course_id, exam_code) if registrations: registration = registrations[0] # NOTE: we do not bother to check here to see if the registration has changed, From f4703b40cb7098dbe44b91f050d23ead8359464f Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Fri, 18 Jan 2013 18:34:51 -0500 Subject: [PATCH 28/82] add test-with-settings --- .../management/commands/tests/test_pearson.py | 54 ++++++++++++++++--- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/management/commands/tests/test_pearson.py b/common/djangoapps/student/management/commands/tests/test_pearson.py index 29f9120e98..fd9eb31631 100644 --- a/common/djangoapps/student/management/commands/tests/test_pearson.py +++ b/common/djangoapps/student/management/commands/tests/test_pearson.py @@ -4,14 +4,16 @@ Created on Jan 17, 2013 @author: brian ''' import logging +import os +from tempfile import mkdtemp +import cStringIO +import sys from django.test import TestCase -from student.models import User, TestCenterRegistration, TestCenterUser -# This is stupid! Because I import a function with the word "test" in the name, -# the unittest framework tries to run *it* as a test?! Crazy! -from student.models import get_tc_registration from django.core import management +from student.models import User, TestCenterRegistration, TestCenterUser, get_tc_registration + log = logging.getLogger(__name__) @@ -49,7 +51,40 @@ class PearsonTestCase(TestCase): ''' Base class for tests running Pearson-related commands ''' + import_dir = mkdtemp(prefix="import") + export_dir = mkdtemp(prefix="export") + + + def tearDown(self): + def delete_temp_dir(dirname): + if os.path.exists(dirname): + for filename in os.listdir(dirname): + os.remove(os.path.join(dirname, filename)) + os.rmdir(dirname) + + # clean up after any test data was dumped to temp directory + delete_temp_dir(self.import_dir) + delete_temp_dir(self.export_dir) + def test_missing_demographic_fields(self): + old_stdout = sys.stdout + sys.stdout = cStringIO.StringIO() + username = 'baduser' + User.objects.create_user(username, '{}@edx.org'.format(username), 'fakepass') + options = {} + + self.assertRaises(BaseException, management.call_command, 'pearson_make_tc_user', username, **options) + output_string = sys.stdout.getvalue() + self.assertTrue(output_string.find('Field Form errors encountered:') >= 0) + self.assertTrue(output_string.find('Field Form Error: city') >= 0) + self.assertTrue(output_string.find('Field Form Error: first_name') >= 0) + self.assertTrue(output_string.find('Field Form Error: last_name') >= 0) + self.assertTrue(output_string.find('Field Form Error: country') >= 0) + self.assertTrue(output_string.find('Field Form Error: phone_country_code') >= 0) + self.assertTrue(output_string.find('Field Form Error: phone') >= 0) + self.assertTrue(output_string.find('Field Form Error: address_1') >= 0) + sys.stdout = old_stdout + def test_create_good_testcenter_user(self): testcenter_user = create_tc_user("test1") @@ -68,10 +103,15 @@ class PearsonTestCase(TestCase): accommodation_code = 'NONE' testcenter_user = create_tc_user(username) registration = create_tc_registration(username, course_id, exam_code, accommodation_code) - output_dir = "./tmpOutput" - options = { 'destination' : output_dir } - with self.settings(PEARSON={ 'LOCAL_EXPORT' : output_dir }): + #options = { 'destination' : self.export_dir } + options = { '--dest-from-settings' : None } + with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }): management.call_command('pearson_export_cdd', **options) + print 'Files found: {}'.format(os.listdir(self.export_dir)) + self.assertEquals(len(os.listdir(self.export_dir)), 1, "Expect cdd file to be created") management.call_command('pearson_export_ead', **options) + print 'Files found: {}'.format(os.listdir(self.export_dir)) + self.assertEquals(len(os.listdir(self.export_dir)), 2, "Expect ead file to also be created") + # TODO: check that files were output.... From 740d0403e908d32d656b1b8c8631db759d4b4c7b Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 22 Jan 2013 11:32:39 -0500 Subject: [PATCH 29/82] change name of function back to get_testcenter_registration, and disable its use as a test --- .../management/commands/pearson_make_tc_registration.py | 6 +++--- .../student/management/commands/tests/test_pearson.py | 4 ++-- common/djangoapps/student/models.py | 8 ++++++-- common/djangoapps/student/views.py | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_make_tc_registration.py b/common/djangoapps/student/management/commands/pearson_make_tc_registration.py index 545c8cd8a8..81a478d19d 100644 --- a/common/djangoapps/student/management/commands/pearson_make_tc_registration.py +++ b/common/djangoapps/student/management/commands/pearson_make_tc_registration.py @@ -4,7 +4,7 @@ from time import strftime from django.contrib.auth.models import User from django.core.management.base import BaseCommand, CommandError -from student.models import TestCenterUser, TestCenterRegistration, TestCenterRegistrationForm, get_tc_registration +from student.models import TestCenterUser, TestCenterRegistration, TestCenterRegistrationForm, get_testcenter_registration from student.views import course_from_id from xmodule.course_module import CourseDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError @@ -134,7 +134,7 @@ class Command(BaseCommand): # create and save the registration: needs_updating = False - registrations = get_tc_registration(student, course_id, exam_code) + registrations = get_testcenter_registration(student, course_id, exam_code) if len(registrations) > 0: registration = registrations[0] for fieldname in UPDATE_FIELDS: @@ -181,7 +181,7 @@ class Command(BaseCommand): change_internal = False if 'exam_series_code' in our_options: exam_code = our_options['exam_series_code'] - registration = get_tc_registration(student, course_id, exam_code)[0] + registration = get_testcenter_registration(student, course_id, exam_code)[0] for internal_field in [ 'upload_error_message', 'upload_status', 'authorization_id']: if internal_field in our_options: registration.__setattr__(internal_field, our_options[internal_field]) diff --git a/common/djangoapps/student/management/commands/tests/test_pearson.py b/common/djangoapps/student/management/commands/tests/test_pearson.py index fd9eb31631..538cd2812a 100644 --- a/common/djangoapps/student/management/commands/tests/test_pearson.py +++ b/common/djangoapps/student/management/commands/tests/test_pearson.py @@ -12,7 +12,7 @@ import sys from django.test import TestCase from django.core import management -from student.models import User, TestCenterRegistration, TestCenterUser, get_tc_registration +from student.models import User, TestCenterRegistration, TestCenterUser, get_testcenter_registration log = logging.getLogger(__name__) @@ -44,7 +44,7 @@ def create_tc_registration(username, course_id, exam_code, accommodation_code): management.call_command('pearson_make_tc_registration', username, course_id, **options) user = User.objects.get(username=username) - registrations = get_tc_registration(user, course_id, exam_code) + registrations = get_testcenter_registration(user, course_id, exam_code) return registrations[0] class PearsonTestCase(TestCase): diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 84bbc76a80..0d8a643ecb 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -547,13 +547,17 @@ class TestCenterRegistrationForm(ModelForm): -def get_tc_registration(user, course_id, exam_series_code): +def get_testcenter_registration(user, course_id, exam_series_code): try: tcu = TestCenterUser.objects.get(user=user) except TestCenterUser.DoesNotExist: return [] return TestCenterRegistration.objects.filter(testcenter_user=tcu, course_id=course_id, exam_series_code=exam_series_code) - + +# nosetests thinks that anything with _test_ in the name is a test. +# Correct this (https://nose.readthedocs.org/en/latest/finding_tests.html) +get_testcenter_registration.__test__ = False + def unique_id_for_user(user): """ Return a unique id for a user, suitable for inserting into diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 650f0a0280..61b49e6022 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -31,7 +31,7 @@ from student.models import (Registration, UserProfile, TestCenterUser, TestCente TestCenterRegistration, TestCenterRegistrationForm, PendingNameChange, PendingEmailChange, CourseEnrollment, unique_id_for_user, - get_tc_registration) + get_testcenter_registration) from certificates.models import CertificateStatuses, certificate_status_for_student @@ -612,7 +612,7 @@ def exam_registration_info(user, course): return None exam_code = exam_info.exam_series_code - registrations = get_tc_registration(user, course.id, exam_code) + registrations = get_testcenter_registration(user, course.id, exam_code) if registrations: registration = registrations[0] else: @@ -712,7 +712,7 @@ def create_exam_registration(request, post_override=None): needs_saving = False exam = course.current_test_center_exam exam_code = exam.exam_series_code - registrations = get_tc_registration(user, course_id, exam_code) + registrations = get_testcenter_registration(user, course_id, exam_code) if registrations: registration = registrations[0] # NOTE: we do not bother to check here to see if the registration has changed, From d395c4448d09e77446ac83533c6342b4e0b738fa Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 22 Jan 2013 17:58:40 -0500 Subject: [PATCH 30/82] add more pearson tests, and update commands in response --- .../management/commands/pearson_export_cdd.py | 9 +- .../management/commands/pearson_export_ead.py | 8 +- .../management/commands/pearson_transfer.py | 11 +- .../management/commands/tests/test_pearson.py | 192 ++++++++++++++---- 4 files changed, 176 insertions(+), 44 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_export_cdd.py b/common/djangoapps/student/management/commands/pearson_export_cdd.py index ba43dd3bc0..463eec6b70 100644 --- a/common/djangoapps/student/management/commands/pearson_export_cdd.py +++ b/common/djangoapps/student/management/commands/pearson_export_cdd.py @@ -5,7 +5,7 @@ from datetime import datetime from optparse import make_option from django.conf import settings -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError from student.models import TestCenterUser @@ -39,6 +39,9 @@ class Command(BaseCommand): ("LastUpdate", "user_updated_at"), # in UTC, so same as what we store ]) + # define defaults, even thought 'store_true' shouldn't need them. + # (call_command will set None as default value for all options that don't have one, + # so one cannot rely on presence/absence of flags in that world.) option_list = BaseCommand.option_list + ( make_option('--dest-from-settings', action='store_true', @@ -63,13 +66,13 @@ class Command(BaseCommand): # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - if 'dest-from-settings' in options: + if 'dest-from-settings' in options and options['dest-from-settings']: if 'LOCAL_EXPORT' in settings.PEARSON: dest = settings.PEARSON['LOCAL_EXPORT'] else: raise CommandError('--dest-from-settings was enabled but the' 'PEARSON[LOCAL_EXPORT] setting was not set.') - elif 'destination' in options: + elif 'destination' in options and options['destination']: dest = options['destination'] else: raise CommandError('--destination or --dest-from-settings must be used') diff --git a/common/djangoapps/student/management/commands/pearson_export_ead.py b/common/djangoapps/student/management/commands/pearson_export_ead.py index 492cba154b..49cdc9957a 100644 --- a/common/djangoapps/student/management/commands/pearson_export_ead.py +++ b/common/djangoapps/student/management/commands/pearson_export_ead.py @@ -5,7 +5,7 @@ from datetime import datetime from optparse import make_option from django.conf import settings -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError from student.models import TestCenterRegistration @@ -39,10 +39,12 @@ class Command(BaseCommand): make_option('--dump_all', action='store_true', dest='dump_all', + default=False, ), make_option('--force_add', action='store_true', dest='force_add', + default=False, ), ) @@ -57,13 +59,13 @@ class Command(BaseCommand): # Name will use timestamp -- this is UTC, so it will look funny, # but it should at least be consistent with the other timestamps # used in the system. - if 'dest-from-settings' in options: + if 'dest-from-settings' in options and options['dest-from-settings']: if 'LOCAL_EXPORT' in settings.PEARSON: dest = settings.PEARSON['LOCAL_EXPORT'] else: raise CommandError('--dest-from-settings was enabled but the' 'PEARSON[LOCAL_EXPORT] setting was not set.') - elif 'destinations' in options: + elif 'destination' in options and options['destination']: dest = options['destination'] else: raise CommandError('--destination or --dest-from-settings must be used') diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 5f126a24f0..c216d2ceac 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -1,10 +1,12 @@ +import os from optparse import make_option +from django.conf import settings from django.core.management.base import BaseCommand, CommandError +from django.core.management import call_command from dogapi import dog_http_api, dog_stats_api import paramiko import boto -import os dog_http_api.api_key = settings.DATADOG_API @@ -13,7 +15,7 @@ class Command(BaseCommand): help = """ This command handles the importing and exporting of student records for Pearson. It uses some other Django commands to export and import the - files and then uploads over SFTP to pearson and stuffs the entry in an + files and then uploads over SFTP to Pearson and stuffs the entry in an S3 bucket for archive purposes. Usage: django-admin.py pearson-transfer --mode [import|export|both] @@ -29,11 +31,12 @@ class Command(BaseCommand): def handle(self, **options): + # TODO: this doesn't work. Need to check if it's a property. if not settings.PEARSON: raise CommandError('No PEARSON entries in auth/env.json.') - for value in ['LOCAL_IMPORT', 'SFTP_IMPORT', 'BUCKET', 'LOCAL_EXPORT', - 'SFTP_EXPORT']: + for value in ['LOCAL_IMPORT', 'SFTP_IMPORT', 'LOCAL_EXPORT', + 'SFTP_EXPORT', 'SFTP_HOSTNAME', 'SFTP_USERNAME', 'SFTP_PASSWORD']: if value not in settings.PEARSON: raise CommandError('No entry in the PEARSON settings' '(env/auth.json) for {0}'.format(value)) diff --git a/common/djangoapps/student/management/commands/tests/test_pearson.py b/common/djangoapps/student/management/commands/tests/test_pearson.py index 538cd2812a..d5594926d2 100644 --- a/common/djangoapps/student/management/commands/tests/test_pearson.py +++ b/common/djangoapps/student/management/commands/tests/test_pearson.py @@ -10,7 +10,7 @@ import cStringIO import sys from django.test import TestCase -from django.core import management +from django.core.management import call_command from student.models import User, TestCenterRegistration, TestCenterUser, get_testcenter_registration @@ -30,11 +30,11 @@ def create_tc_user(username): 'phone' : '252-1866', 'phone_country_code' : '1', } - management.call_command('pearson_make_tc_user', username, **options) + call_command('pearson_make_tc_user', username, **options) return TestCenterUser.objects.get(user=user) -def create_tc_registration(username, course_id, exam_code, accommodation_code): +def create_tc_registration(username, course_id = 'org1/course1/term1', exam_code = 'exam1', accommodation_code = None): options = { 'exam_series_code' : exam_code, 'eligibility_appointment_date_first' : '2013-01-01T00:00', @@ -42,11 +42,55 @@ def create_tc_registration(username, course_id, exam_code, accommodation_code): 'accommodation_code' : accommodation_code, } - management.call_command('pearson_make_tc_registration', username, course_id, **options) + call_command('pearson_make_tc_registration', username, course_id, **options) user = User.objects.get(username=username) registrations = get_testcenter_registration(user, course_id, exam_code) return registrations[0] + +def get_error_string_for_management_call(*args, **options): + stdout_string = None + old_stdout = sys.stdout + old_stderr = sys.stderr + sys.stdout = cStringIO.StringIO() + sys.stderr = cStringIO.StringIO() + try: + call_command(*args, **options) + except BaseException, why1: + # The goal here is to catch CommandError calls. + # But these are actually translated into nice messages, + # and sys.exit(1) is then called. For testing, we + # want to catch what sys.exit throws, and get the + # relevant text either from stdout or stderr. + # TODO: this should really check to see that we + # arrived here because of a sys.exit(1). Otherwise + # we should just raise the exception. + stdout_string = sys.stdout.getvalue() + stderr_string = sys.stderr.getvalue() + except Exception, why: + raise why + finally: + sys.stdout = old_stdout + sys.stderr = old_stderr + + if stdout_string is None: + raise Exception("Expected call to {} to fail, but it succeeded!".format(args[0])) + return stdout_string, stderr_string + + +def get_file_info(dirpath): + filelist = os.listdir(dirpath) + print 'Files found: {}'.format(filelist) + numfiles = len(filelist) + if numfiles == 1: + filepath = os.path.join(dirpath, filelist[0]) + with open(filepath, 'r') as cddfile: + filecontents = cddfile.readlines() + numlines = len(filecontents) + return filepath, numlines + else: + raise Exception("Expected to find a single file in {}, but found {}".format(dirpath,filelist)) + class PearsonTestCase(TestCase): ''' Base class for tests running Pearson-related commands @@ -54,7 +98,9 @@ class PearsonTestCase(TestCase): import_dir = mkdtemp(prefix="import") export_dir = mkdtemp(prefix="export") - + def assertErrorContains(self, error_message, expected): + self.assertTrue(error_message.find(expected) >= 0, 'error message "{}" did not contain "{}"'.format(error_message, expected)) + def tearDown(self): def delete_temp_dir(dirname): if os.path.exists(dirname): @@ -67,14 +113,13 @@ class PearsonTestCase(TestCase): delete_temp_dir(self.export_dir) def test_missing_demographic_fields(self): - old_stdout = sys.stdout - sys.stdout = cStringIO.StringIO() + # We won't bother to test all details of form validation here. + # It is enough to show that it works here, but deal with test cases for the form + # validation in the student tests, not these management tests. username = 'baduser' User.objects.create_user(username, '{}@edx.org'.format(username), 'fakepass') options = {} - - self.assertRaises(BaseException, management.call_command, 'pearson_make_tc_user', username, **options) - output_string = sys.stdout.getvalue() + output_string, _ = get_error_string_for_management_call('pearson_make_tc_user', username, **options) self.assertTrue(output_string.find('Field Form errors encountered:') >= 0) self.assertTrue(output_string.find('Field Form Error: city') >= 0) self.assertTrue(output_string.find('Field Form Error: first_name') >= 0) @@ -83,35 +128,114 @@ class PearsonTestCase(TestCase): self.assertTrue(output_string.find('Field Form Error: phone_country_code') >= 0) self.assertTrue(output_string.find('Field Form Error: phone') >= 0) self.assertTrue(output_string.find('Field Form Error: address_1') >= 0) - sys.stdout = old_stdout + self.assertErrorContains(output_string, 'Field Form Error: address_1') def test_create_good_testcenter_user(self): testcenter_user = create_tc_user("test1") + self.assertIsNotNone(testcenter_user) def test_create_good_testcenter_registration(self): username = 'test1' - course_id = 'org1/course1/term1' - exam_code = 'exam1' - accommodation_code = 'NONE' - testcenter_user = create_tc_user(username) - registration = create_tc_registration(username, course_id, exam_code, accommodation_code) - - def test_export(self): - username = 'test1' - course_id = 'org1/course1/term1' - exam_code = 'exam1' - accommodation_code = 'NONE' - testcenter_user = create_tc_user(username) - registration = create_tc_registration(username, course_id, exam_code, accommodation_code) - #options = { 'destination' : self.export_dir } - options = { '--dest-from-settings' : None } - with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }): - management.call_command('pearson_export_cdd', **options) - print 'Files found: {}'.format(os.listdir(self.export_dir)) - self.assertEquals(len(os.listdir(self.export_dir)), 1, "Expect cdd file to be created") - management.call_command('pearson_export_ead', **options) - print 'Files found: {}'.format(os.listdir(self.export_dir)) - self.assertEquals(len(os.listdir(self.export_dir)), 2, "Expect ead file to also be created") + create_tc_user(username) + registration = create_tc_registration(username) + self.assertIsNotNone(registration) - # TODO: check that files were output.... + def test_cdd_missing_option(self): + _, error_string = get_error_string_for_management_call('pearson_export_cdd', **{}) + self.assertErrorContains(error_string, 'Error: --destination or --dest-from-settings must be used') + + def test_ead_missing_option(self): + _, error_string = get_error_string_for_management_call('pearson_export_ead', **{}) + self.assertErrorContains(error_string, 'Error: --destination or --dest-from-settings must be used') + + def test_export_single_cdd(self): + # before we generate any tc_users, we expect there to be nothing to output: + options = { 'dest-from-settings' : True } + with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }): + call_command('pearson_export_cdd', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 1, "Expect cdd file to have no non-header lines") + os.remove(filepath) + + # generating a tc_user should result in a line in the output + username = 'test_single_cdd' + create_tc_user(username) + call_command('pearson_export_cdd', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 2, "Expect cdd file to have one non-header line") + os.remove(filepath) + + # output after registration should not have any entries again. + call_command('pearson_export_cdd', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 1, "Expect cdd file to have no non-header lines") + os.remove(filepath) + + # if we modify the record, then it should be output again: + user_options = { 'first_name' : 'NewTestFirst', } + call_command('pearson_make_tc_user', username, **user_options) + call_command('pearson_export_cdd', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 2, "Expect cdd file to have one non-header line") + os.remove(filepath) + + def test_export_single_ead(self): + # before we generate any registrations, we expect there to be nothing to output: + options = { 'dest-from-settings' : True } + with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }): + call_command('pearson_export_ead', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 1, "Expect ead file to have no non-header lines") + os.remove(filepath) + + # generating a registration should result in a line in the output + username = 'test_single_ead' + create_tc_user(username) + create_tc_registration(username) + call_command('pearson_export_ead', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 2, "Expect ead file to have one non-header line") + os.remove(filepath) + + # output after registration should not have any entries again. + call_command('pearson_export_ead', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 1, "Expect ead file to have no non-header lines") + os.remove(filepath) + # if we modify the record, then it should be output again: + create_tc_registration(username, accommodation_code='EQPMNT') + call_command('pearson_export_ead', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 2, "Expect ead file to have one non-header line") + os.remove(filepath) + + def test_export_multiple(self): + username1 = 'test_multiple1' + create_tc_user(username1) + create_tc_registration(username1) + create_tc_registration(username1, course_id = 'org1/course2/term1') + create_tc_registration(username1, exam_code = 'exam2') + username2 = 'test_multiple2' + create_tc_user(username2) + create_tc_registration(username2) + username3 = 'test_multiple3' + create_tc_user(username3) + create_tc_registration(username3, course_id = 'org1/course2/term1') + username4 = 'test_multiple4' + create_tc_user(username4) + create_tc_registration(username4, exam_code = 'exam2') + + with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }): + options = { 'dest-from-settings' : True } + call_command('pearson_export_cdd', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 5, "Expect cdd file to have four non-header lines: total was {}".format(numlines)) + os.remove(filepath) + + call_command('pearson_export_ead', **options) + (filepath, numlines) = get_file_info(self.export_dir) + self.assertEquals(numlines, 7, "Expect ead file to have six non-header lines: total was {}".format(numlines)) + os.remove(filepath) + + From 1199b1ecfa97cf623d1275ea0220af99119ba8e3 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Wed, 23 Jan 2013 18:22:18 -0500 Subject: [PATCH 31/82] debug pearson import and update unit tests --- .../commands/pearson_import_conf_zip.py | 2 + .../commands/pearson_make_tc_user.py | 19 +- .../management/commands/pearson_transfer.py | 133 +++++++---- .../management/commands/tests/test_pearson.py | 209 +++++++++++++++--- 4 files changed, 275 insertions(+), 88 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py index fa9741dc68..bf7c4481fd 100644 --- a/common/djangoapps/student/management/commands/pearson_import_conf_zip.py +++ b/common/djangoapps/student/management/commands/pearson_import_conf_zip.py @@ -10,6 +10,7 @@ from optparse import make_option from dogapi import dog_http_api, dog_stats_api from django.core.management.base import BaseCommand, CommandError +from django.conf import settings from student.models import TestCenterUser, TestCenterRegistration @@ -23,6 +24,7 @@ class Command(BaseCommand): and TestCenterRegistration tables with status. """ + @staticmethod def datadog_error(string, tags): dog_http_api.event("Pearson Import", string, alert_type='error', tags=tags) diff --git a/common/djangoapps/student/management/commands/pearson_make_tc_user.py b/common/djangoapps/student/management/commands/pearson_make_tc_user.py index da9bfc3bd0..87e0b4dadd 100644 --- a/common/djangoapps/student/management/commands/pearson_make_tc_user.py +++ b/common/djangoapps/student/management/commands/pearson_make_tc_user.py @@ -1,7 +1,7 @@ from optparse import make_option from django.contrib.auth.models import User -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError from student.models import TestCenterUser, TestCenterUserForm @@ -161,15 +161,16 @@ class Command(BaseCommand): if form.is_valid(): form.update_and_save() else: + errorlist = [] if (len(form.errors) > 0): - print "Field Form errors encountered:" - for fielderror in form.errors: - print "Field Form Error: %s" % fielderror - if (len(form.non_field_errors()) > 0): - print "Non-field Form errors encountered:" - for nonfielderror in form.non_field_errors: - print "Non-field Form Error: %s" % nonfielderror - + errorlist.append("Field Form errors encountered:") + for fielderror in form.errors: + errorlist.append("Field Form Error: {}".format(fielderror)) + if (len(form.non_field_errors()) > 0): + errorlist.append("Non-field Form errors encountered:") + for nonfielderror in form.non_field_errors: + errorlist.append("Non-field Form Error: {}".format(nonfielderror)) + raise CommandError("\n".join(errorlist)) else: print "No changes necessary to make to existing user's demographics." diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index c216d2ceac..2124bdceb6 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -1,5 +1,6 @@ import os from optparse import make_option +from stat import S_ISDIR from django.conf import settings from django.core.management.base import BaseCommand, CommandError @@ -26,85 +27,99 @@ class Command(BaseCommand): action='store', dest='mode', default='both', + choices=('import', 'export', 'both'), help='mode is import, export, or both'), ) def handle(self, **options): - # TODO: this doesn't work. Need to check if it's a property. - if not settings.PEARSON: + if not hasattr(settings, 'PEARSON'): raise CommandError('No PEARSON entries in auth/env.json.') - for value in ['LOCAL_IMPORT', 'SFTP_IMPORT', 'LOCAL_EXPORT', - 'SFTP_EXPORT', 'SFTP_HOSTNAME', 'SFTP_USERNAME', 'SFTP_PASSWORD']: + # check settings needed for either import or export: + for value in ['SFTP_HOSTNAME', 'SFTP_USERNAME', 'SFTP_PASSWORD', 'S3_BUCKET']: if value not in settings.PEARSON: raise CommandError('No entry in the PEARSON settings' '(env/auth.json) for {0}'.format(value)) - def import_pearson(): - try: - sftp(settings.PEARSON['SFTP_IMPORT'], - settings.PEARSON['LOCAL_IMPORT'], options['mode']) - s3(settings.PEARSON['LOCAL_IMPORT'], - settings.PEARSON['BUCKET'], options['mode']) - except Exception as e: - dog_http_api.event('Pearson Import failure', str(e)) - else: - for file in os.listdir(settings.PEARSON['LOCAL_IMPORT']): - call_command('pearson_import_conf_zip', - settings.PEARSON['LOCAL_IMPORT'] + '/' + file) - os.remove(file) + for value in ['AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY']: + if not hasattr(settings, value): + raise CommandError('No entry in the AWS settings' + '(env/auth.json) for {0}'.format(value)) + + # check additional required settings for import and export: + if options['mode'] in ('export', 'both'): + for value in ['LOCAL_EXPORT','SFTP_EXPORT']: + if value not in settings.PEARSON: + raise CommandError('No entry in the PEARSON settings' + '(env/auth.json) for {0}'.format(value)) + # make sure that the import directory exists or can be created: + source_dir = settings.PEARSON['LOCAL_EXPORT'] + if not os.path.isdir(source_dir): + os.makedirs(source_dir) + + if options['mode'] in ('import', 'both'): + for value in ['LOCAL_IMPORT','SFTP_IMPORT']: + if value not in settings.PEARSON: + raise CommandError('No entry in the PEARSON settings' + '(env/auth.json) for {0}'.format(value)) + # make sure that the import directory exists or can be created: + dest_dir = settings.PEARSON['LOCAL_IMPORT'] + if not os.path.isdir(dest_dir): + os.makedirs(dest_dir) - def export_pearson(): - call_command('pearson_export_cdd', 'dest_from_settings') - call_command('pearson_export_ead', 'dest_from_settings') - sftp(settings.PEARSON['LOCAL_EXPORT'], - settings.PEARSON['SFTP_EXPORT'], options['mode']) - s3(settings.PEARSON['LOCAL_EXPORT'], - settings.PEARSON['BUCKET'], options['mode']) - if options['mode'] == 'export': - export_pearson() - elif options['mode'] == 'import': - import_pearson() - else: - export_pearson() - import_pearson() - - def sftp(files_from, files_to, mode): + def sftp(files_from, files_to, mode, deleteAfterCopy=False): with dog_stats_api.timer('pearson.{0}'.format(mode), tags='sftp'): try: t = paramiko.Transport((settings.PEARSON['SFTP_HOSTNAME'], 22)) t.connect(username=settings.PEARSON['SFTP_USERNAME'], password=settings.PEARSON['SFTP_PASSWORD']) sftp = paramiko.SFTPClient.from_transport(t) - if os.path.isdir(files_from): + + if mode == 'export': + try: + sftp.chdir(files_to) + except IOError: + raise CommandError('SFTP destination path does not exist: {}'.format(files_to)) for filename in os.listdir(files_from): - sftp.put(files_from + '/' + filename, - files_to + '/' + filename) + sftp.put(files_from + '/' + filename, filename) + if deleteAfterCopy: + os.remove(os.path.join(files_from, filename)) else: - for filename in sftp.listdir(files_from): - sftp.get(files_from + '/' + filename, - files_to + '/' + filename) - sftp.remove(files_from + '/' + filename) - t.close() + try: + sftp.chdir(files_from) + except IOError: + raise CommandError('SFTP source path does not exist: {}'.format(files_from)) + for filename in sftp.listdir('.'): + # skip subdirectories + if not S_ISDIR(sftp.stat(filename).st_mode): + sftp.get(filename, files_to + '/' + filename) + # delete files from sftp server once they are successfully pulled off: + if deleteAfterCopy: + sftp.remove(filename) except: dog_http_api.event('pearson {0}'.format(mode), 'sftp uploading failed', alert_type='error') raise + finally: + sftp.close() + t.close() - def s3(files_from, bucket, mode): + def s3(files_from, bucket, mode, deleteAfterCopy=False): with dog_stats_api.timer('pearson.{0}'.format(mode), tags='s3'): try: for filename in os.listdir(files_from): - upload_file_to_s3(bucket, files_from + '/' + filename) + upload_file_to_s3(bucket, files_from, filename) + if deleteAfterCopy: + os.remove(files_from + '/' + filename) except: dog_http_api.event('pearson {0}'.format(mode), 's3 archiving failed') raise - def upload_file_to_s3(bucket, filename): + def upload_file_to_s3(bucket, source_dir, filename): """ Upload file to S3 """ @@ -114,4 +129,32 @@ class Command(BaseCommand): b = s3.get_bucket(bucket) k = Key(b) k.key = "{filename}".format(filename=filename) - k.set_contents_from_filename(filename) + k.set_contents_from_filename(os.path.join(source_dir, filename)) + + def export_pearson(): + options = { 'dest-from-settings' : True } + call_command('pearson_export_cdd', **options) + call_command('pearson_export_ead', **options) + mode = 'export' + sftp(settings.PEARSON['LOCAL_EXPORT'], settings.PEARSON['SFTP_EXPORT'], mode, deleteAfterCopy = False) + s3(settings.PEARSON['LOCAL_EXPORT'], settings.PEARSON['S3_BUCKET'], mode, deleteAfterCopy=True) + + def import_pearson(): + mode = 'import' + try: + sftp(settings.PEARSON['SFTP_IMPORT'], settings.PEARSON['LOCAL_IMPORT'], mode, deleteAfterCopy = True) + s3(settings.PEARSON['LOCAL_IMPORT'], settings.PEARSON['S3_BUCKET'], mode, deleteAfterCopy=False) + except Exception as e: + dog_http_api.event('Pearson Import failure', str(e)) + raise e + else: + for filename in os.listdir(settings.PEARSON['LOCAL_IMPORT']): + filepath = os.path.join(settings.PEARSON['LOCAL_IMPORT'], filename) + call_command('pearson_import_conf_zip', filepath) + os.remove(filepath) + + # actually do the work! + if options['mode'] in ('export', 'both'): + export_pearson() + if options['mode'] in ('import', 'both'): + import_pearson() diff --git a/common/djangoapps/student/management/commands/tests/test_pearson.py b/common/djangoapps/student/management/commands/tests/test_pearson.py index d5594926d2..199557bf87 100644 --- a/common/djangoapps/student/management/commands/tests/test_pearson.py +++ b/common/djangoapps/student/management/commands/tests/test_pearson.py @@ -11,12 +11,12 @@ import sys from django.test import TestCase from django.core.management import call_command +from nose.plugins.skip import SkipTest from student.models import User, TestCenterRegistration, TestCenterUser, get_testcenter_registration log = logging.getLogger(__name__) - def create_tc_user(username): user = User.objects.create_user(username, '{}@edx.org'.format(username), 'fakepass') options = { @@ -47,6 +47,48 @@ def create_tc_registration(username, course_id = 'org1/course1/term1', exam_code registrations = get_testcenter_registration(user, course_id, exam_code) return registrations[0] +def create_multiple_registrations(prefix='test'): + username1 = '{}_multiple1'.format(prefix) + create_tc_user(username1) + create_tc_registration(username1) + create_tc_registration(username1, course_id = 'org1/course2/term1') + create_tc_registration(username1, exam_code = 'exam2') + username2 = '{}_multiple2'.format(prefix) + create_tc_user(username2) + create_tc_registration(username2) + username3 = '{}_multiple3'.format(prefix) + create_tc_user(username3) + create_tc_registration(username3, course_id = 'org1/course2/term1') + username4 = '{}_multiple4'.format(prefix) + create_tc_user(username4) + create_tc_registration(username4, exam_code = 'exam2') + +def get_command_error_text(*args, **options): + stderr_string = None + old_stderr = sys.stderr + sys.stderr = cStringIO.StringIO() + try: + call_command(*args, **options) + except SystemExit, why1: + # The goal here is to catch CommandError calls. + # But these are actually translated into nice messages, + # and sys.exit(1) is then called. For testing, we + # want to catch what sys.exit throws, and get the + # relevant text either from stdout or stderr. + if (why1.message > 0): + stderr_string = sys.stderr.getvalue() + else: + raise why1 + except Exception, why: + raise why + + finally: + sys.stderr = old_stderr + + if stderr_string is None: + raise Exception("Expected call to {} to fail, but it succeeded!".format(args[0])) + return stderr_string + def get_error_string_for_management_call(*args, **options): stdout_string = None old_stdout = sys.stdout @@ -55,17 +97,17 @@ def get_error_string_for_management_call(*args, **options): sys.stderr = cStringIO.StringIO() try: call_command(*args, **options) - except BaseException, why1: + except SystemExit, why1: # The goal here is to catch CommandError calls. # But these are actually translated into nice messages, # and sys.exit(1) is then called. For testing, we # want to catch what sys.exit throws, and get the # relevant text either from stdout or stderr. - # TODO: this should really check to see that we - # arrived here because of a sys.exit(1). Otherwise - # we should just raise the exception. - stdout_string = sys.stdout.getvalue() - stderr_string = sys.stderr.getvalue() + if (why1.message == 1): + stdout_string = sys.stdout.getvalue() + stderr_string = sys.stderr.getvalue() + else: + raise why1 except Exception, why: raise why @@ -111,6 +153,12 @@ class PearsonTestCase(TestCase): # clean up after any test data was dumped to temp directory delete_temp_dir(self.import_dir) delete_temp_dir(self.export_dir) + + # and clean up the database: +# TestCenterUser.objects.all().delete() +# TestCenterRegistration.objects.all().delete() + +class PearsonCommandTestCase(PearsonTestCase): def test_missing_demographic_fields(self): # We won't bother to test all details of form validation here. @@ -119,16 +167,16 @@ class PearsonTestCase(TestCase): username = 'baduser' User.objects.create_user(username, '{}@edx.org'.format(username), 'fakepass') options = {} - output_string, _ = get_error_string_for_management_call('pearson_make_tc_user', username, **options) - self.assertTrue(output_string.find('Field Form errors encountered:') >= 0) - self.assertTrue(output_string.find('Field Form Error: city') >= 0) - self.assertTrue(output_string.find('Field Form Error: first_name') >= 0) - self.assertTrue(output_string.find('Field Form Error: last_name') >= 0) - self.assertTrue(output_string.find('Field Form Error: country') >= 0) - self.assertTrue(output_string.find('Field Form Error: phone_country_code') >= 0) - self.assertTrue(output_string.find('Field Form Error: phone') >= 0) - self.assertTrue(output_string.find('Field Form Error: address_1') >= 0) - self.assertErrorContains(output_string, 'Field Form Error: address_1') + error_string = get_command_error_text('pearson_make_tc_user', username, **options) + self.assertTrue(error_string.find('Field Form errors encountered:') >= 0) + self.assertTrue(error_string.find('Field Form Error: city') >= 0) + self.assertTrue(error_string.find('Field Form Error: first_name') >= 0) + self.assertTrue(error_string.find('Field Form Error: last_name') >= 0) + self.assertTrue(error_string.find('Field Form Error: country') >= 0) + self.assertTrue(error_string.find('Field Form Error: phone_country_code') >= 0) + self.assertTrue(error_string.find('Field Form Error: phone') >= 0) + self.assertTrue(error_string.find('Field Form Error: address_1') >= 0) + self.assertErrorContains(error_string, 'Field Form Error: address_1') def test_create_good_testcenter_user(self): testcenter_user = create_tc_user("test1") @@ -141,11 +189,11 @@ class PearsonTestCase(TestCase): self.assertIsNotNone(registration) def test_cdd_missing_option(self): - _, error_string = get_error_string_for_management_call('pearson_export_cdd', **{}) + error_string = get_command_error_text('pearson_export_cdd', **{}) self.assertErrorContains(error_string, 'Error: --destination or --dest-from-settings must be used') def test_ead_missing_option(self): - _, error_string = get_error_string_for_management_call('pearson_export_ead', **{}) + error_string = get_command_error_text('pearson_export_ead', **{}) self.assertErrorContains(error_string, 'Error: --destination or --dest-from-settings must be used') def test_export_single_cdd(self): @@ -211,21 +259,7 @@ class PearsonTestCase(TestCase): os.remove(filepath) def test_export_multiple(self): - username1 = 'test_multiple1' - create_tc_user(username1) - create_tc_registration(username1) - create_tc_registration(username1, course_id = 'org1/course2/term1') - create_tc_registration(username1, exam_code = 'exam2') - username2 = 'test_multiple2' - create_tc_user(username2) - create_tc_registration(username2) - username3 = 'test_multiple3' - create_tc_user(username3) - create_tc_registration(username3, course_id = 'org1/course2/term1') - username4 = 'test_multiple4' - create_tc_user(username4) - create_tc_registration(username4, exam_code = 'exam2') - + create_multiple_registrations("export") with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }): options = { 'dest-from-settings' : True } call_command('pearson_export_cdd', **options) @@ -239,3 +273,110 @@ class PearsonTestCase(TestCase): os.remove(filepath) +# def test_bad_demographic_option(self): +# username = 'nonuser' +# output_string, stderrmsg = get_error_string_for_management_call('pearson_make_tc_user', username, **{'--garbage' : None }) +# print stderrmsg +# self.assertErrorContains(stderrmsg, 'Unexpected option') +# +# def test_missing_demographic_user(self): +# username = 'nonuser' +# output_string, error_string = get_error_string_for_management_call('pearson_make_tc_user', username, **{}) +# self.assertErrorContains(error_string, 'User matching query does not exist') + +# credentials for a test SFTP site: +SFTP_HOSTNAME = 'ec2-23-20-150-101.compute-1.amazonaws.com' +SFTP_USERNAME = 'pearsontest' +SFTP_PASSWORD = 'password goes here' + +S3_BUCKET = 'edx-pearson-archive' +AWS_ACCESS_KEY_ID = 'put yours here' +AWS_SECRET_ACCESS_KEY = 'put yours here' + +class PearsonTransferTestCase(PearsonTestCase): + ''' + Class for tests running Pearson transfers + ''' + + def test_transfer_config(self): + with self.settings(DATADOG_API='FAKE_KEY'): + # TODO: why is this failing with the wrong error message?! + stderrmsg = get_command_error_text('pearson_transfer', **{'mode' : 'garbage'}) + self.assertErrorContains(stderrmsg, 'Error: No PEARSON entries') + with self.settings(DATADOG_API='FAKE_KEY'): + stderrmsg = get_command_error_text('pearson_transfer') + self.assertErrorContains(stderrmsg, 'Error: No PEARSON entries') + with self.settings(DATADOG_API='FAKE_KEY', + PEARSON={'LOCAL_EXPORT' : self.export_dir, + 'LOCAL_IMPORT' : self.import_dir }): + stderrmsg = get_command_error_text('pearson_transfer') + self.assertErrorContains(stderrmsg, 'Error: No entry in the PEARSON settings') + + def test_transfer_export_missing_dest_dir(self): + raise SkipTest() + create_multiple_registrations('export_missing_dest') + with self.settings(DATADOG_API='FAKE_KEY', + PEARSON={'LOCAL_EXPORT' : self.export_dir, + 'SFTP_EXPORT' : 'this/does/not/exist', + 'SFTP_HOSTNAME' : SFTP_HOSTNAME, + 'SFTP_USERNAME' : SFTP_USERNAME, + 'SFTP_PASSWORD' : SFTP_PASSWORD, + 'S3_BUCKET' : S3_BUCKET, + }, + AWS_ACCESS_KEY_ID = AWS_ACCESS_KEY_ID, + AWS_SECRET_ACCESS_KEY = AWS_SECRET_ACCESS_KEY): + options = { 'mode' : 'export'} + stderrmsg = get_command_error_text('pearson_transfer', **options) + self.assertErrorContains(stderrmsg, 'Error: SFTP destination path does not exist') + + def test_transfer_export(self): + raise SkipTest() + create_multiple_registrations("transfer_export") + with self.settings(DATADOG_API='FAKE_KEY', + PEARSON={'LOCAL_EXPORT' : self.export_dir, + 'SFTP_EXPORT' : 'results/topvue', + 'SFTP_HOSTNAME' : SFTP_HOSTNAME, + 'SFTP_USERNAME' : SFTP_USERNAME, + 'SFTP_PASSWORD' : SFTP_PASSWORD, + 'S3_BUCKET' : S3_BUCKET, + }, + AWS_ACCESS_KEY_ID = AWS_ACCESS_KEY_ID, + AWS_SECRET_ACCESS_KEY = AWS_SECRET_ACCESS_KEY): + options = { 'mode' : 'export'} +# call_command('pearson_transfer', **options) +# # confirm that the export directory is still empty: +# self.assertEqual(len(os.listdir(self.export_dir)), 0, "expected export directory to be empty") + + def test_transfer_import_missing_source_dir(self): + raise SkipTest() + create_multiple_registrations('import_missing_src') + with self.settings(DATADOG_API='FAKE_KEY', + PEARSON={'LOCAL_IMPORT' : self.import_dir, + 'SFTP_IMPORT' : 'this/does/not/exist', + 'SFTP_HOSTNAME' : SFTP_HOSTNAME, + 'SFTP_USERNAME' : SFTP_USERNAME, + 'SFTP_PASSWORD' : SFTP_PASSWORD, + 'S3_BUCKET' : S3_BUCKET, + }, + AWS_ACCESS_KEY_ID = AWS_ACCESS_KEY_ID, + AWS_SECRET_ACCESS_KEY = AWS_SECRET_ACCESS_KEY): + options = { 'mode' : 'import'} + stderrmsg = get_command_error_text('pearson_transfer', **options) + self.assertErrorContains(stderrmsg, 'Error: SFTP source path does not exist') + + def test_transfer_import(self): + raise SkipTest() + create_multiple_registrations('import_missing_src') + with self.settings(DATADOG_API='FAKE_KEY', + PEARSON={'LOCAL_IMPORT' : self.import_dir, + 'SFTP_IMPORT' : 'results', + 'SFTP_HOSTNAME' : SFTP_HOSTNAME, + 'SFTP_USERNAME' : SFTP_USERNAME, + 'SFTP_PASSWORD' : SFTP_PASSWORD, + 'S3_BUCKET' : S3_BUCKET, + }, + AWS_ACCESS_KEY_ID = AWS_ACCESS_KEY_ID, + AWS_SECRET_ACCESS_KEY = AWS_SECRET_ACCESS_KEY): + options = { 'mode' : 'import'} + call_command('pearson_transfer', **options) + self.assertEqual(len(os.listdir(self.import_dir)), 0, "expected import directory to be empty") From 645cfbce21fef12f5793764f5ff9b6bc5111a920 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 24 Jan 2013 10:00:13 -0500 Subject: [PATCH 32/82] Move this into the github requirements where it belongs. --- github-requirements.txt | 1 + requirements.txt | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/github-requirements.txt b/github-requirements.txt index 468d55ce65..32193ec853 100644 --- a/github-requirements.txt +++ b/github-requirements.txt @@ -3,3 +3,4 @@ -e git://github.com/MITx/django-pipeline.git#egg=django-pipeline -e git://github.com/MITx/django-wiki.git@e2e84558#egg=django-wiki -e git://github.com/dementrock/pystache_custom.git@776973740bdaad83a3b029f96e415a7d1e8bec2f#egg=pystache_custom-dev +-e git+ssh://git@github.com/MITx/dogapi.git@003a4fc9#egg=dogapi diff --git a/requirements.txt b/requirements.txt index 1b1384912b..fa4688b711 100644 --- a/requirements.txt +++ b/requirements.txt @@ -59,4 +59,3 @@ Shapely==1.2.16 ipython==0.13.1 xmltodict==0.4.1 paramiko==1.9.0 -git+ssh://git@github.com/MITx/dogapi.git@003a4fc9#egg=dogapi From 126ff55cd530d15cd7c7b07fcf62029512da8767 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Thu, 24 Jan 2013 10:06:09 -0500 Subject: [PATCH 33/82] Not sure why this was set to use SSH. --- github-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-requirements.txt b/github-requirements.txt index 32193ec853..62e47a328f 100644 --- a/github-requirements.txt +++ b/github-requirements.txt @@ -3,4 +3,4 @@ -e git://github.com/MITx/django-pipeline.git#egg=django-pipeline -e git://github.com/MITx/django-wiki.git@e2e84558#egg=django-wiki -e git://github.com/dementrock/pystache_custom.git@776973740bdaad83a3b029f96e415a7d1e8bec2f#egg=pystache_custom-dev --e git+ssh://git@github.com/MITx/dogapi.git@003a4fc9#egg=dogapi +-e git://github.com/MITx/dogapi.git@003a4fc9#egg=dogapi From cb2d8db57c60952fbec3d4eb6c9359ea437b76e2 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Thu, 24 Jan 2013 12:46:00 -0500 Subject: [PATCH 34/82] store files in S3 bucket in separate directories by mode --- .../student/management/commands/pearson_transfer.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/student/management/commands/pearson_transfer.py b/common/djangoapps/student/management/commands/pearson_transfer.py index 2124bdceb6..6811e1833d 100644 --- a/common/djangoapps/student/management/commands/pearson_transfer.py +++ b/common/djangoapps/student/management/commands/pearson_transfer.py @@ -111,7 +111,10 @@ class Command(BaseCommand): with dog_stats_api.timer('pearson.{0}'.format(mode), tags='s3'): try: for filename in os.listdir(files_from): - upload_file_to_s3(bucket, files_from, filename) + source_file = os.path.join(files_from, filename) + # use mode as name of directory into which to write files + dest_file = os.path.join(mode, filename) + upload_file_to_s3(bucket, source_file, dest_file) if deleteAfterCopy: os.remove(files_from + '/' + filename) except: @@ -119,7 +122,7 @@ class Command(BaseCommand): 's3 archiving failed') raise - def upload_file_to_s3(bucket, source_dir, filename): + def upload_file_to_s3(bucket, source_file, dest_file): """ Upload file to S3 """ @@ -128,8 +131,8 @@ class Command(BaseCommand): from boto.s3.key import Key b = s3.get_bucket(bucket) k = Key(b) - k.key = "{filename}".format(filename=filename) - k.set_contents_from_filename(os.path.join(source_dir, filename)) + k.key = "{filename}".format(filename=dest_file) + k.set_contents_from_filename(source_file) def export_pearson(): options = { 'dest-from-settings' : True } From def2d8adf22eea0e941deb615b6543e1c531ab5d Mon Sep 17 00:00:00 2001 From: Kevin Chugh Date: Thu, 24 Jan 2013 14:20:18 -0500 Subject: [PATCH 35/82] fix cohorts in create, and read, and update regular expressions to fix courses with periods not working in General commentable --- lms/djangoapps/courseware/courses.py | 10 +---- lms/djangoapps/courseware/views.py | 1 + .../django_comment_client/base/urls.py | 8 ++-- .../django_comment_client/base/views.py | 40 ++++++++++++------- .../django_comment_client/forum/urls.py | 4 +- .../django_comment_client/forum/views.py | 3 +- .../commands/seed_permissions_roles.py | 2 +- 7 files changed, 37 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index b4e8da2633..74f5e4c54f 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -84,21 +84,13 @@ def get_opt_course_with_access(user, course_id, action): return get_course_with_access(user, course_id, action) - - -def is_course_cohorted(course_id): - """ - given a course id, return a boolean for whether or not the course is cohorted - - """ - def get_cohort_id(user, course_id): """ given a course id and a user, return the id of the cohort that user is assigned to and if the course is not cohorted or the user is an instructor, return None """ - return 101 + return 127 def is_commentable_cohorted(course_id,commentable_id): """ diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index b1c4a5e9a9..f170f3fb86 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -39,6 +39,7 @@ log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} + def user_groups(user): """ TODO (vshnayder): This is not used. When we have a new plan for groups, adjust appropriately. diff --git a/lms/djangoapps/django_comment_client/base/urls.py b/lms/djangoapps/django_comment_client/base/urls.py index f2cb4ccb15..23f2afa037 100644 --- a/lms/djangoapps/django_comment_client/base/urls.py +++ b/lms/djangoapps/django_comment_client/base/urls.py @@ -24,9 +24,9 @@ urlpatterns = patterns('django_comment_client.base.views', url(r'comments/(?P[\w\-]+)/downvote$', 'vote_for_comment', {'value': 'down'}, name='downvote_comment'), url(r'comments/(?P[\w\-]+)/unvote$', 'undo_vote_for_comment', name='undo_vote_for_comment'), - url(r'(?P[\w\-]+)/threads/create$', 'create_thread', name='create_thread'), + url(r'^(?P[\w\-.]+)/threads/create$', 'create_thread', name='create_thread'), # TODO should we search within the board? - url(r'(?P[\w\-]+)/threads/search_similar$', 'search_similar_threads', name='search_similar_threads'), - url(r'(?P[\w\-]+)/follow$', 'follow_commentable', name='follow_commentable'), - url(r'(?P[\w\-]+)/unfollow$', 'unfollow_commentable', name='unfollow_commentable'), + url(r'^(?P[\w\-.]+)/threads/search_similar$', 'search_similar_threads', name='search_similar_threads'), + url(r'^(?P[\w\-.]+)/follow$', 'follow_commentable', name='follow_commentable'), + url(r'^(?P[\w\-.]+)/unfollow$', 'unfollow_commentable', name='unfollow_commentable'), ) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index d1948c3fc7..c1e188ff1a 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -21,11 +21,11 @@ from django.contrib.auth.models import User from mitxmako.shortcuts import render_to_response, render_to_string from courseware.courses import get_course_with_access -from courseware.courses import get_cohort_id +from courseware.courses import get_cohort_id,is_commentable_cohorted from django_comment_client.utils import JsonResponse, JsonError, extract, get_courseware_context -from django_comment_client.permissions import check_permissions_by_view +from django_comment_client.permissions import check_permissions_by_view, cached_has_permission from django_comment_client.models import Role def permitted(fn): @@ -59,10 +59,17 @@ def ajax_content_response(request, course_id, content, template_name): 'annotated_content_info': annotated_content_info, }) + + +def is_moderator(user, course_id): + cached_has_permission(user, "see_all_cohorts", course_id) + @require_POST @login_required @permitted def create_thread(request, course_id, commentable_id): + print "\n\n\n\n\n*******************" + print commentable_id course = get_course_with_access(request.user, course_id, 'load') post = request.POST @@ -85,23 +92,28 @@ def create_thread(request, course_id, commentable_id): 'user_id' : request.user.id, }) - #now cohort id + + #now cohort the thread if the commentable is cohorted #if the group id came in from the form, set it there, otherwise, #see if the user and the commentable are cohorted - print post + if is_commentable_cohorted(course_id,commentable_id): + if 'group_id' in post: #if a group id was submitted in the form + posted_group_id = post['group_id'] + else: + post_group_id = None - group_id = None - - if 'group_id' in post: - group_id = post['group_id'] - - - if group_id is None: - group_id = get_cohort_id(request.user, course_id) + user_group_id = get_cohort_id(request.user, course_id) - if group_id is not None: + if is_moderator(request.user,course_id): + if post_group_id is None: + group_id = user_group_id + else: + group_id = post_group_id + else: + group_id = user_group_id + thread.update_attributes(**{'group_id' :group_id}) - + thread.save() if post.get('auto_subscribe', 'false').lower() == 'true': user = cc.User.from_django_user(request.user) diff --git a/lms/djangoapps/django_comment_client/forum/urls.py b/lms/djangoapps/django_comment_client/forum/urls.py index 526ae3e582..1e676dee87 100644 --- a/lms/djangoapps/django_comment_client/forum/urls.py +++ b/lms/djangoapps/django_comment_client/forum/urls.py @@ -4,7 +4,7 @@ import django_comment_client.forum.views urlpatterns = patterns('django_comment_client.forum.views', url(r'users/(?P\w+)/followed$', 'followed_threads', name='followed_threads'), url(r'users/(?P\w+)$', 'user_profile', name='user_profile'), - url(r'(?P[\w\-]+)/threads/(?P\w+)$', 'single_thread', name='single_thread'), - url(r'(?P[\w\-]+)/inline$', 'inline_discussion', name='inline_discussion'), + url(r'^(?P[\w\-.]+)/threads/(?P\w+)$', 'single_thread', name='single_thread'), + url(r'^(?P[\w\-.]+)/inline$', 'inline_discussion', name='inline_discussion'), url(r'', 'forum_form_discussion', name='forum_form_discussion'), ) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 0e8a044097..443329ec1f 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -34,7 +34,6 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG This may raise cc.utils.CommentClientError or cc.utils.CommentClientUnknownError if something goes wrong. """ - default_query_params = { 'page': 1, 'per_page': per_page, @@ -64,6 +63,8 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG group_id = get_cohort_id(user,course_id); if group_id: default_query_params["group_id"] = group_id; + print("\n\n\n\n\n****************GROUP ID IS ") + print group_id query_params = merge_dict(default_query_params, strip_none(extract(request.GET, ['page', 'sort_key', 'sort_order', 'text', 'tags', 'commentable_ids']))) diff --git a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py index 3faa846033..958b67cdb3 100644 --- a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py +++ b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py @@ -23,7 +23,7 @@ class Command(BaseCommand): student_role.add_permission(per) for per in ["edit_content", "delete_thread", "openclose_thread", - "endorse_comment", "delete_comment"]: + "endorse_comment", "delete_comment", "see_all_cohorts"]: moderator_role.add_permission(per) for per in ["manage_moderator"]: From 4f37ea91497068bc7c9ac9c28cf9bf8ec5bdf258 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Thu, 24 Jan 2013 14:45:31 -0500 Subject: [PATCH 36/82] add --create_dummy_exam option to pearson_make_tc_registration --- .../commands/pearson_make_tc_registration.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/management/commands/pearson_make_tc_registration.py b/common/djangoapps/student/management/commands/pearson_make_tc_registration.py index 81a478d19d..0f28ceb283 100644 --- a/common/djangoapps/student/management/commands/pearson_make_tc_registration.py +++ b/common/djangoapps/student/management/commands/pearson_make_tc_registration.py @@ -71,6 +71,12 @@ class Command(BaseCommand): dest='ignore_registration_dates', help='find exam info for course based on exam_series_code, even if the exam is not active.' ), + make_option( + '--create_dummy_exam', + action='store_true', + dest='create_dummy_exam', + help='create dummy exam info for course, even if course exists' + ), ) args = "" help = "Create or modify a TestCenterRegistration entry for a given Student" @@ -99,6 +105,7 @@ class Command(BaseCommand): raise CommandError("User \"{}\" does not have an existing demographics record".format(username)) # check to see if a course_id was specified, and use information from that: + create_dummy_exam = 'create_dummy_exam' in our_options and our_options['create_dummy_exam'] try: course = course_from_id(course_id) if 'ignore_registration_dates' in our_options: @@ -107,6 +114,9 @@ class Command(BaseCommand): else: exam = course.current_test_center_exam except ItemNotFoundError: + create_dummy_exam = True + + if exam is None and create_dummy_exam: # otherwise use explicit values (so we don't have to define a course): exam_name = "Dummy Placeholder Name" exam_info = { 'Exam_Series_Code': our_options['exam_series_code'], @@ -120,7 +130,7 @@ class Command(BaseCommand): our_options['eligibility_appointment_date_last'] = strftime("%Y-%m-%d", exam.last_eligible_appointment_date) if exam is None: - raise CommandError("Exam for course_id {%s} does not exist".format(course_id)) + raise CommandError("Exam for course_id {} does not exist".format(course_id)) exam_code = exam.exam_series_code From c655f62f7a8f0690efa910989aacc09b32ce02f2 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 12:08:31 -0500 Subject: [PATCH 37/82] Remove out-of-date text about user replication, askbot --- common/djangoapps/student/models.py | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index f13a691215..00819d7dc4 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1,30 +1,5 @@ """ -Models for Student Information - -Replication Notes - -TODO: Update this to be consistent with reality (no portal servers, no more askbot) - -In our live deployment, we intend to run in a scenario where there is a pool of -Portal servers that hold the canoncial user information and that user -information is replicated to slave Course server pools. Each Course has a set of -servers that serves only its content and has users that are relevant only to it. - -We replicate the following tables into the Course DBs where the user is -enrolled. Only the Portal servers should ever write to these models. -* UserProfile -* CourseEnrollment - -We do a partial replication of: -* User -- Askbot extends this and uses the extra fields, so we replicate only - the stuff that comes with basic django_auth and ignore the rest.) - -There are a couple different scenarios: - -1. There's an update of User or UserProfile -- replicate it to all Course DBs - that the user is enrolled in (found via CourseEnrollment). -2. There's a change in CourseEnrollment. We need to push copies of UserProfile, - CourseEnrollment, and the base fields in User +Models for User Information (students, staff, etc) Migration Notes From 0e78eaaf80f21bbc5bdb76f019d4e33396c62a21 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 13:20:34 -0500 Subject: [PATCH 38/82] Add a course_groups djangoapp with a CourseUserGroup model. --- common/djangoapps/course_groups/__init__.py | 0 common/djangoapps/course_groups/models.py | 27 +++++++++++++++++++++ lms/envs/common.py | 1 + 3 files changed, 28 insertions(+) create mode 100644 common/djangoapps/course_groups/__init__.py create mode 100644 common/djangoapps/course_groups/models.py diff --git a/common/djangoapps/course_groups/__init__.py b/common/djangoapps/course_groups/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py new file mode 100644 index 0000000000..5423b6ec16 --- /dev/null +++ b/common/djangoapps/course_groups/models.py @@ -0,0 +1,27 @@ +from django.contrib.auth.models import User +from django.db import models + +class CourseUserGroup(models.Model): + """ + This model represents groups of users in a course. Groups may have different types, + which may be treated specially. For example, a user can be in at most one cohort per + course, and cohorts are used to split up the forums by group. + """ + class Meta: + unique_together = (('name', 'course_id'), ) + + name = models.CharField(max_length=255, + help_text=("What is the name of this group? " + "Must be unique within a course.")) + users = models.ManyToManyField(User, db_index=True, related_name='course_groups', + help_text="Who is in this group?") + + # Note: groups associated with particular runs of a course. E.g. Fall 2012 and Spring + # 2013 versions of 6.00x will have separate groups. + course_id = models.CharField(max_length=255, db_index=True, + help_text="Which course is this group associated with?") + + # For now, only have group type 'cohort', but adding a type field to support + # things like 'question_discussion', 'friends', 'off-line-class', etc + GROUP_TYPE_CHOICES = (('cohort', 'Cohort'),) + group_type = models.CharField(max_length=20, choices=GROUP_TYPE_CHOICES) diff --git a/lms/envs/common.py b/lms/envs/common.py index 5e5ea86a4e..16472795e0 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -577,6 +577,7 @@ INSTALLED_APPS = ( 'open_ended_grading', 'psychometrics', 'licenses', + 'course_groups', #For the wiki 'wiki', # The new django-wiki from benjaoming From 1b9a3557eb60b8be8539e6adcfb7e4a067b0cc37 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 13:40:13 -0500 Subject: [PATCH 39/82] get_cohort() function and test --- common/djangoapps/course_groups/models.py | 29 ++++++++++++++++++- .../djangoapps/course_groups/tests/tests.py | 21 ++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 common/djangoapps/course_groups/tests/tests.py diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 5423b6ec16..36c9927e18 100644 --- a/common/djangoapps/course_groups/models.py +++ b/common/djangoapps/course_groups/models.py @@ -23,5 +23,32 @@ class CourseUserGroup(models.Model): # For now, only have group type 'cohort', but adding a type field to support # things like 'question_discussion', 'friends', 'off-line-class', etc - GROUP_TYPE_CHOICES = (('cohort', 'Cohort'),) + COHORT = 'cohort' + GROUP_TYPE_CHOICES = ((COHORT, 'Cohort'),) group_type = models.CharField(max_length=20, choices=GROUP_TYPE_CHOICES) + + +def get_cohort(user, course_id): + """ + Given a django User and a course_id, return the user's cohort. In classes with + auto-cohorting, put the user in a cohort if they aren't in one already. + + Arguments: + user: a Django User object. + course_id: string in the format 'org/course/run' + + Returns: + A CourseUserGroup object if the User has a cohort, or None. + """ + group_type = CourseUserGroup.COHORT + try: + group = CourseUserGroup.objects.get(course_id=course_id, group_type=group_type, + users__id=user.id) + except CourseUserGroup.DoesNotExist: + group = None + + if group: + return group + + # TODO: add auto-cohorting logic here + return None diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py new file mode 100644 index 0000000000..89c77c5b65 --- /dev/null +++ b/common/djangoapps/course_groups/tests/tests.py @@ -0,0 +1,21 @@ +from django.contrib.auth.models import User +from nose.tools import assert_equals + +from course_groups.models import CourseUserGroup, get_cohort + +def test_get_cohort(): + course_id = "a/b/c" + cohort = CourseUserGroup.objects.create(name="TestCohort", course_id=course_id, + group_type=CourseUserGroup.COHORT) + + user = User.objects.create(username="test", email="a@b.com") + other_user = User.objects.create(username="test2", email="a2@b.com") + + cohort.users.add(user) + + got = get_cohort(user, course_id) + assert_equals(got.id, cohort.id, "Should find the right cohort") + + got = get_cohort(other_user, course_id) + assert_equals(got, None, "other_user shouldn't have a cohort") + From fa17913a91e4906e1fb53563cb3a4dfac69f041c Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 14:17:35 -0500 Subject: [PATCH 40/82] is_cohorted course descriptor property, docs --- common/lib/xmodule/xmodule/course_module.py | 11 +++++++++++ doc/xml-format.md | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 5416dae583..c0b3000258 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -360,6 +360,17 @@ class CourseDescriptor(SequenceDescriptor): def show_calculator(self): return self.metadata.get("show_calculator", None) == "Yes" + @property + def is_cohorted(self): + """ + Return whether the course is cohorted. + """ + config = self.metadata.get("cohort-config") + if config is None: + return False + + return bool(config.get("cohorted")) + @property def is_new(self): """ diff --git a/doc/xml-format.md b/doc/xml-format.md index 8138de4d7e..8f9e512ac1 100644 --- a/doc/xml-format.md +++ b/doc/xml-format.md @@ -258,6 +258,10 @@ Supported fields at the course level: * "discussion_blackouts" -- An array of time intervals during which you want to disable a student's ability to create or edit posts in the forum. Moderators, Community TAs, and Admins are unaffected. You might use this during exam periods, but please be aware that the forum is often a very good place to catch mistakes and clarify points to students. The better long term solution would be to have better flagging/moderation mechanisms, but this is the hammer we have today. Format by example: [["2012-10-29T04:00", "2012-11-03T04:00"], ["2012-12-30T04:00", "2013-01-02T04:00"]] * "show_calculator" (value "Yes" if desired) * "days_early_for_beta" -- number of days (floating point ok) early that students in the beta-testers group get to see course content. Can also be specified for any other course element, and overrides values set at higher levels. +* "cohort-config" : dictionary with keys + - "cohorted" : boolean. Set to true if this course uses student cohorts. If so, all inline discussions are automatically cohorted, and top-level discussion topics are configurable with an optional 'cohorted': bool parameter (with default value false). + - ... more to come. ('auto-cohort', how to auto cohort, etc) + * TODO: there are others ### Grading policy file contents From f005b70f3b8b5b4c68519c22f85fa153a173a212 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 14:17:58 -0500 Subject: [PATCH 41/82] Minor test cleanups --- .../lib/xmodule/xmodule/tests/test_import.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 554e89ac74..8fc8916dc3 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -52,6 +52,16 @@ class ImportTestCase(unittest.TestCase): '''Get a dummy system''' return DummySystem(load_error_modules) + def get_course(self, name): + """Get a test course by directory name. If there's more than one, error.""" + print "Importing {0}".format(name) + + modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) + courses = modulestore.get_courses() + self.assertEquals(len(courses), 1) + return courses[0] + + def test_fallback(self): '''Check that malformed xml loads as an ErrorDescriptor.''' @@ -207,11 +217,7 @@ class ImportTestCase(unittest.TestCase): """Make sure that metadata is inherited properly""" print "Starting import" - initial_import = XMLModuleStore(DATA_DIR, course_dirs=['toy']) - - courses = initial_import.get_courses() - self.assertEquals(len(courses), 1) - course = courses[0] + course = self.get_course('toy') def check_for_key(key, node): "recursive check for presence of key" @@ -227,16 +233,8 @@ class ImportTestCase(unittest.TestCase): """Make sure that when two courses share content with the same org and course names, policy applies to the right one.""" - def get_course(name): - print "Importing {0}".format(name) - - modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name]) - courses = modulestore.get_courses() - self.assertEquals(len(courses), 1) - return courses[0] - - toy = get_course('toy') - two_toys = get_course('two_toys') + toy = self.get_course('toy') + two_toys = self.get_course('two_toys') self.assertEqual(toy.url_name, "2012_Fall") self.assertEqual(two_toys.url_name, "TT_2012_Fall") @@ -279,8 +277,8 @@ class ImportTestCase(unittest.TestCase): """Ensure that colons in url_names convert to file paths properly""" print "Starting import" + # Not using get_courses because we need the modulestore object too afterward modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy']) - courses = modulestore.get_courses() self.assertEquals(len(courses), 1) course = courses[0] @@ -317,7 +315,7 @@ class ImportTestCase(unittest.TestCase): toy_id = "edX/toy/2012_Fall" - course = modulestore.get_courses()[0] + course = modulestore.get_course(toy_id) chapters = course.get_children() ch1 = chapters[0] sections = ch1.get_children() From 90a5703419b9322eb3d5c8130af6a655b938cb3e Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 14:18:06 -0500 Subject: [PATCH 42/82] Test for cohort config --- .../lib/xmodule/xmodule/tests/test_import.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 8fc8916dc3..8a5eda3882 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -353,3 +353,28 @@ class ImportTestCase(unittest.TestCase): \ """.strip() self.assertEqual(gst_sample.definition['render'], render_string_from_sample_gst_xml) + + def test_cohort_config(self): + """ + Check that cohort config parsing works right. + """ + modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy']) + + toy_id = "edX/toy/2012_Fall" + + course = modulestore.get_course(toy_id) + + # No config -> False + self.assertFalse(course.is_cohorted) + + # empty config -> False + course.metadata['cohort-config'] = {} + self.assertFalse(course.is_cohorted) + + # false config -> False + course.metadata['cohort-config'] = {'cohorted': False} + self.assertFalse(course.is_cohorted) + + # and finally... + course.metadata['cohort-config'] = {'cohorted': True} + self.assertTrue(course.is_cohorted) From 3488083e6bccbfbfcb3833350d8c7f4e2c2a2962 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 14:58:46 -0500 Subject: [PATCH 43/82] Add a get_course_cohorts function and test --- common/djangoapps/course_groups/models.py | 21 ++++++-- .../djangoapps/course_groups/tests/tests.py | 49 ++++++++++++++----- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 36c9927e18..46859a900e 100644 --- a/common/djangoapps/course_groups/models.py +++ b/common/djangoapps/course_groups/models.py @@ -40,15 +40,28 @@ def get_cohort(user, course_id): Returns: A CourseUserGroup object if the User has a cohort, or None. """ - group_type = CourseUserGroup.COHORT try: - group = CourseUserGroup.objects.get(course_id=course_id, group_type=group_type, - users__id=user.id) + group = CourseUserGroup.objects.get(course_id=course_id, + group_type=CourseUserGroup.COHORT, + users__id=user.id) except CourseUserGroup.DoesNotExist: group = None - + if group: return group # TODO: add auto-cohorting logic here return None + +def get_course_cohorts(course_id): + """ + Get a list of all the cohorts in the given course. + + Arguments: + course_id: string in the format 'org/course/run' + + Returns: + A list of CourseUserGroup objects. Empty if there are no cohorts. + """ + return list(CourseUserGroup.objects.filter(course_id=course_id, + group_type=CourseUserGroup.COHORT)) diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py index 89c77c5b65..676643567d 100644 --- a/common/djangoapps/course_groups/tests/tests.py +++ b/common/djangoapps/course_groups/tests/tests.py @@ -1,21 +1,44 @@ +import django.test from django.contrib.auth.models import User -from nose.tools import assert_equals -from course_groups.models import CourseUserGroup, get_cohort +from course_groups.models import CourseUserGroup, get_cohort, get_course_cohorts -def test_get_cohort(): - course_id = "a/b/c" - cohort = CourseUserGroup.objects.create(name="TestCohort", course_id=course_id, - group_type=CourseUserGroup.COHORT) +class TestCohorts(django.test.TestCase): - user = User.objects.create(username="test", email="a@b.com") - other_user = User.objects.create(username="test2", email="a2@b.com") + def test_get_cohort(self): + course_id = "a/b/c" + cohort = CourseUserGroup.objects.create(name="TestCohort", course_id=course_id, + group_type=CourseUserGroup.COHORT) - cohort.users.add(user) + user = User.objects.create(username="test", email="a@b.com") + other_user = User.objects.create(username="test2", email="a2@b.com") - got = get_cohort(user, course_id) - assert_equals(got.id, cohort.id, "Should find the right cohort") + cohort.users.add(user) - got = get_cohort(other_user, course_id) - assert_equals(got, None, "other_user shouldn't have a cohort") + got = get_cohort(user, course_id) + self.assertEquals(got.id, cohort.id, "Should find the right cohort") + + got = get_cohort(other_user, course_id) + self.assertEquals(got, None, "other_user shouldn't have a cohort") + + + def test_get_course_cohorts(self): + course1_id = "a/b/c" + course2_id = "e/f/g" + + # add some cohorts to course 1 + cohort = CourseUserGroup.objects.create(name="TestCohort", + course_id=course1_id, + group_type=CourseUserGroup.COHORT) + + cohort = CourseUserGroup.objects.create(name="TestCohort2", + course_id=course1_id, + group_type=CourseUserGroup.COHORT) + + + # second course should have no cohorts + self.assertEqual(get_course_cohorts(course2_id), []) + + cohorts = sorted([c.name for c in get_course_cohorts(course1_id)]) + self.assertEqual(cohorts, ['TestCohort', 'TestCohort2']) From 0d41a04490a20deac30e1216e63982e8940d88e2 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 15:00:43 -0500 Subject: [PATCH 44/82] fix line length --- common/lib/xmodule/xmodule/discussion_module.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index 1deceac5d0..57d7780d95 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -18,8 +18,10 @@ class DiscussionModule(XModule): } return self.system.render_template('discussion/_discussion_module.html', context) - def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) + def __init__(self, system, location, definition, descriptor, + instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, descriptor, + instance_state, shared_state, **kwargs) if isinstance(instance_state, str): instance_state = json.loads(instance_state) From ac8c59126d40e4fd7d70fc4113634885bcbd67c0 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 15:05:41 -0500 Subject: [PATCH 45/82] Add note about non-unique topic id across course runs bug --- lms/djangoapps/django_comment_client/utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 3094367491..3c9669ac37 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -207,6 +207,9 @@ def initialize_discussion_info(course): "sort_key": entry["sort_key"], "start_date": entry["start_date"]} + # TODO. BUG! : course location is not unique across multiple course runs! + # (I think Kevin already noticed this) Need to send course_id with requests, store it + # in the backend. default_topics = {'General': {'id' :course.location.html_id()}} discussion_topics = course.metadata.get('discussion_topics', default_topics) for topic, entry in discussion_topics.items(): From bab3b0c39e94b6e060b80e6e31a41a090bb3f972 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 21 Jan 2013 15:48:58 -0500 Subject: [PATCH 46/82] Add initial set of views for managing cohorts - lets user see and add cohorts - needs styling - needs to allow them to actually manage membership! --- common/djangoapps/course_groups/models.py | 51 +++++++- common/djangoapps/course_groups/views.py | 108 ++++++++++++++++ common/static/js/course_groups/cohorts.js | 118 ++++++++++++++++++ lms/djangoapps/instructor/views.py | 1 + .../course_groups/cohort_management.html | 25 ++++ lms/templates/course_groups/debug.html | 16 +++ .../courseware/instructor_dashboard.html | 7 +- lms/urls.py | 17 +++ 8 files changed, 341 insertions(+), 2 deletions(-) create mode 100644 common/djangoapps/course_groups/views.py create mode 100644 common/static/js/course_groups/cohorts.js create mode 100644 lms/templates/course_groups/cohort_management.html create mode 100644 lms/templates/course_groups/debug.html diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 46859a900e..701cce0e6c 100644 --- a/common/djangoapps/course_groups/models.py +++ b/common/djangoapps/course_groups/models.py @@ -1,6 +1,10 @@ +import logging + from django.contrib.auth.models import User from django.db import models +log = logging.getLogger(__name__) + class CourseUserGroup(models.Model): """ This model represents groups of users in a course. Groups may have different types, @@ -27,7 +31,6 @@ class CourseUserGroup(models.Model): GROUP_TYPE_CHOICES = ((COHORT, 'Cohort'),) group_type = models.CharField(max_length=20, choices=GROUP_TYPE_CHOICES) - def get_cohort(user, course_id): """ Given a django User and a course_id, return the user's cohort. In classes with @@ -65,3 +68,49 @@ def get_course_cohorts(course_id): """ return list(CourseUserGroup.objects.filter(course_id=course_id, group_type=CourseUserGroup.COHORT)) + +### Helpers for cohor management views + +def get_cohort_by_name(course_id, name): + """ + Return the CourseUserGroup object for the given cohort. Raises DoesNotExist + it isn't present. + """ + return CourseUserGroup.objects.get(course_id=course_id, + group_type=CourseUserGroup.COHORT, + name=name) + +def add_cohort(course_id, name): + """ + Add a cohort to a course. Raises ValueError if a cohort of the same name already + exists. + """ + log.debug("Adding cohort %s to %s", name, course_id) + if CourseUserGroup.objects.filter(course_id=course_id, + group_type=CourseUserGroup.COHORT, + name=name).exists(): + raise ValueError("Can't create two cohorts with the same name") + + return CourseUserGroup.objects.create(course_id=course_id, + group_type=CourseUserGroup.COHORT, + name=name) + +def get_course_cohort_names(course_id): + """ + Return a list of the cohort names in a course. + """ + return [c.name for c in get_course_cohorts(course_id)] + + +def delete_empty_cohort(course_id, name): + """ + Remove an empty cohort. Raise ValueError if cohort is not empty. + """ + cohort = get_cohort_by_name(course_id, name) + if cohort.users.exists(): + raise ValueError( + "Can't delete non-empty cohort {0} in course {1}".format( + name, course_id)) + + cohort.delete() + diff --git a/common/djangoapps/course_groups/views.py b/common/djangoapps/course_groups/views.py new file mode 100644 index 0000000000..9ee9935c3e --- /dev/null +++ b/common/djangoapps/course_groups/views.py @@ -0,0 +1,108 @@ +import json +from django_future.csrf import ensure_csrf_cookie +from django.contrib.auth.decorators import login_required +from django.core.context_processors import csrf +from django.core.urlresolvers import reverse +from django.http import HttpResponse, HttpResponseForbidden, Http404 +from django.shortcuts import redirect +import logging + +from courseware.courses import get_course_with_access +from mitxmako.shortcuts import render_to_response, render_to_string +from .models import CourseUserGroup +from . import models + +import track.views + + +log = logging.getLogger(__name__) + +def JsonHttpReponse(data): + """ + Return an HttpResponse with the data json-serialized and the right content type + header. Named to look like a class. + """ + return HttpResponse(json.dumps(data), content_type="application/json") + +@ensure_csrf_cookie +def list_cohorts(request, course_id): + """ + Return json dump of dict: + + {'success': True, + 'cohorts': [{'name': name, 'id': id}, ...]} + """ + get_course_with_access(request.user, course_id, 'staff') + + cohorts = [{'name': c.name, 'id': c.id} + for c in models.get_course_cohorts(course_id)] + + return JsonHttpReponse({'success': True, + 'cohorts': cohorts}) + + +@ensure_csrf_cookie +def add_cohort(request, course_id): + """ + Return json of dict: + {'success': True, + 'cohort': {'id': id, + 'name': name}} + + or + + {'success': False, + 'msg': error_msg} if there's an error + """ + get_course_with_access(request.user, course_id, 'staff') + + + if request.method != "POST": + raise Http404("Must POST to add cohorts") + + name = request.POST.get("name") + if not name: + return JsonHttpReponse({'success': False, + 'msg': "No name specified"}) + + try: + cohort = models.add_cohort(course_id, name) + except ValueError as err: + return JsonHttpReponse({'success': False, + 'msg': str(err)}) + + return JsonHttpReponse({'success': 'True', + 'cohort': { + 'id': cohort.id, + 'name': cohort.name + }}) + + +@ensure_csrf_cookie +def users_in_cohort(request, course_id, cohort_id): + """ + """ + get_course_with_access(request.user, course_id, 'staff') + + return JsonHttpReponse({'error': 'Not implemented'}) + + +@ensure_csrf_cookie +def add_users_to_cohort(request, course_id): + """ + """ + get_course_with_access(request.user, course_id, 'staff') + + return JsonHttpReponse({'error': 'Not implemented'}) + + +def debug_cohort_mgmt(request, course_id): + """ + Debugging view for dev. + """ + # add staff check to make sure it's safe if it's accidentally deployed. + get_course_with_access(request.user, course_id, 'staff') + + context = {'cohorts_ajax_url': reverse('cohorts', + kwargs={'course_id': course_id})} + return render_to_response('/course_groups/debug.html', context) diff --git a/common/static/js/course_groups/cohorts.js b/common/static/js/course_groups/cohorts.js new file mode 100644 index 0000000000..7b1793dcf8 --- /dev/null +++ b/common/static/js/course_groups/cohorts.js @@ -0,0 +1,118 @@ +// structure stolen from http://briancray.com/posts/javascript-module-pattern + +var CohortManager = (function ($) { + // private variables and functions + + // using jQuery + function getCookie(name) { + var cookieValue = null; + if (document.cookie && document.cookie != '') { + var cookies = document.cookie.split(';'); + for (var i = 0; i < cookies.length; i++) { + var cookie = $.trim(cookies[i]); + // Does this cookie string begin with the name we want? + if (cookie.substring(0, name.length + 1) == (name + '=')) { + cookieValue = decodeURIComponent(cookie.substring(name.length + 1)); + break; + } + } + } + return cookieValue; + } + var csrftoken = getCookie('csrftoken'); + + function csrfSafeMethod(method) { + // these HTTP methods do not require CSRF protection + return (/^(GET|HEAD|OPTIONS|TRACE)$/.test(method)); + } + $.ajaxSetup({ + crossDomain: false, // obviates need for sameOrigin test + beforeSend: function(xhr, settings) { + if (!csrfSafeMethod(settings.type)) { + xhr.setRequestHeader("X-CSRFToken", csrftoken); + } + } + }); + + // constructor + var module = function () { + var url = $(".cohort_manager").data('ajax_url'); + var self = this; + var error_list = $(".cohort_errors"); + var cohort_list = $(".cohort_list"); + var cohorts_display = $(".cohorts_display"); + var show_cohorts_button = $(".cohort_controls .show_cohorts"); + var add_cohort_input = $("#cohort-name"); + var add_cohort_button = $(".add_cohort"); + + function show_cohort(item) { + // item is a li that has a data-href link to the cohort base url + var el = $(this); + alert("would show you data about " + el.text() + " from " + el.data('href')); + } + + function add_to_cohorts_list(item) { + var li = $('
    • '); + $("a", li).text(item.name) + .data('href', url + '/' + item.id) + .addClass('link') + .click(show_cohort); + cohort_list.append(li); + }; + + function log_error(msg) { + error_list.empty(); + error_list.append($("
  • ").text(msg).addClass("error")); + }; + + function load_cohorts(response) { + cohort_list.empty(); + if (response && response.success) { + response.cohorts.forEach(add_to_cohorts_list); + } else { + log_error(response.msg || "There was an error loading cohorts"); + } + cohorts_display.show(); + }; + + function added_cohort(response) { + if (response && response.success) { + add_to_cohorts_list(response.cohort); + } else { + log_error(response.msg || "There was an error adding a cohort"); + } + } + + show_cohorts_button.click(function() { + $.ajax(url).done(load_cohorts); + }); + + add_cohort_input.change(function() { + if (!$(this).val()) { + add_cohort_button.removeClass('button').addClass('button-disabled'); + } else { + add_cohort_button.removeClass('button-disabled').addClass('button'); + } + }); + + add_cohort_button.click(function() { + var add_url = url + '/add'; + data = {'name': add_cohort_input.val()} + $.post(add_url, data).done(added_cohort); + }); + + }; + + // prototype + module.prototype = { + constructor: module, + }; + + // return module + return module; +})(jQuery); + +$(window).load(function () { + var my_module = new CohortManager(); +}) + diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 2cf3bbb0a9..f8872082da 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -562,6 +562,7 @@ def instructor_dashboard(request, course_id): 'djangopid' : os.getpid(), 'mitx_version' : getattr(settings,'MITX_VERSION_STRING',''), 'offline_grade_log' : offline_grades_available(course_id), + 'cohorts_ajax_url' : reverse('cohorts', kwargs={'course_id': course_id}), } return render_to_response('courseware/instructor_dashboard.html', context) diff --git a/lms/templates/course_groups/cohort_management.html b/lms/templates/course_groups/cohort_management.html new file mode 100644 index 0000000000..1512d09689 --- /dev/null +++ b/lms/templates/course_groups/cohort_management.html @@ -0,0 +1,25 @@ +
    +

    Cohort groups

    + +
    + Show cohorts +
    + +
      +
    + + + + +
    diff --git a/lms/templates/course_groups/debug.html b/lms/templates/course_groups/debug.html new file mode 100644 index 0000000000..d8bbc324de --- /dev/null +++ b/lms/templates/course_groups/debug.html @@ -0,0 +1,16 @@ + + + + <%block name="title">edX + + + + + + + +<%include file="/course_groups/cohort_management.html" /> + + + + diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 4d46505705..e9b9ae0354 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -6,6 +6,8 @@ <%static:css group='course'/> + + <%include file="/courseware/course_navigation.html" args="active_page='instructor'" /> @@ -282,16 +284,19 @@ function goto( mode)

    + + <%include file="/course_groups/cohort_management.html" /> + %endif %endif +##----------------------------------------------------------------------------- %if msg:

    ${msg}

    %endif ##----------------------------------------------------------------------------- -##----------------------------------------------------------------------------- %if datatable and modeflag.get('Psychometrics') is None: diff --git a/lms/urls.py b/lms/urls.py index 9d590387e4..54f41e2c87 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -274,6 +274,23 @@ if settings.COURSEWARE_ENABLED: 'open_ended_grading.peer_grading_service.save_grade', name='peer_grading_save_grade'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/peer_grading/save_calibration_essay$', 'open_ended_grading.peer_grading_service.save_calibration_essay', name='peer_grading_save_calibration_essay'), + + # Cohorts management + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts$', + 'course_groups.views.list_cohorts', name="cohorts"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/add$', + 'course_groups.views.add_cohort', + name="add_cohort"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/(?P[0-9]+)$', + 'course_groups.views.users_in_cohort', + name="list_cohort"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/(?P[0-9]+)/add$', + 'course_groups.views.add_users_to_cohort', + name="add_to_cohort"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/debug$', + 'course_groups.views.debug_cohort_mgmt', + name="debug_cohort_mgmt"), + ) # discussion forums live within courseware, so courseware must be enabled first From 64f820828e8ace406552f53dc29e5d5cc34d0155 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 21 Jan 2013 15:49:27 -0500 Subject: [PATCH 47/82] whitespace --- lms/djangoapps/instructor/views.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index f8872082da..8069d3f184 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -193,8 +193,8 @@ def instructor_dashboard(request, course_id): # get the form data unique_student_identifier=request.POST.get('unique_student_identifier','') problem_to_reset=request.POST.get('problem_to_reset','') - - if problem_to_reset[-4:]==".xml": + + if problem_to_reset[-4:]==".xml": problem_to_reset=problem_to_reset[:-4] # try to uniquely id student by email address or username @@ -213,8 +213,8 @@ def instructor_dashboard(request, course_id): try: (org, course_name, run)=course_id.split("/") module_state_key="i4x://"+org+"/"+course_name+"/problem/"+problem_to_reset - module_to_reset=StudentModule.objects.get(student_id=student_to_reset.id, - course_id=course_id, + module_to_reset=StudentModule.objects.get(student_id=student_to_reset.id, + course_id=course_id, module_state_key=module_state_key) msg+="Found module to reset. " except Exception as e: @@ -230,14 +230,14 @@ def instructor_dashboard(request, course_id): # save module_to_reset.state=json.dumps(problem_state) module_to_reset.save() - track.views.server_track(request, + track.views.server_track(request, '{instructor} reset attempts from {old_attempts} to 0 for {student} on problem {problem} in {course}'.format( old_attempts=old_number_of_attempts, student=student_to_reset, problem=module_to_reset.module_state_key, instructor=request.user, course=course_id), - {}, + {}, page='idashboard') msg+="Module state successfully reset!" except: @@ -252,12 +252,12 @@ def instructor_dashboard(request, course_id): else: student_to_reset=User.objects.get(username=unique_student_identifier) progress_url=reverse('student_progress',kwargs={'course_id':course_id,'student_id': student_to_reset.id}) - track.views.server_track(request, + track.views.server_track(request, '{instructor} requested progress page for {student} in {course}'.format( student=student_to_reset, instructor=request.user, course=course_id), - {}, + {}, page='idashboard') msg+=" Progress page for username: {1} with email address: {2}.".format(progress_url,student_to_reset.username,student_to_reset.email) except: From 7bdb5c23e1205198b324daf539e95472eeaee45d Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 21 Jan 2013 19:29:17 -0500 Subject: [PATCH 48/82] Allow listing of users in cohorts and adding users - still needs style, more features, but basic functionality works --- common/djangoapps/course_groups/models.py | 37 +++++ common/djangoapps/course_groups/views.py | 77 +++++++++- common/lib/string_util.py | 11 ++ common/static/js/course_groups/cohorts.js | 135 ++++++++++++++++-- lms/djangoapps/instructor/views.py | 19 +-- .../course_groups/cohort_management.html | 26 +++- 6 files changed, 268 insertions(+), 37 deletions(-) create mode 100644 common/lib/string_util.py diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 701cce0e6c..dd46e5a055 100644 --- a/common/djangoapps/course_groups/models.py +++ b/common/djangoapps/course_groups/models.py @@ -80,6 +80,15 @@ def get_cohort_by_name(course_id, name): group_type=CourseUserGroup.COHORT, name=name) +def get_cohort_by_id(course_id, cohort_id): + """ + Return the CourseUserGroup object for the given cohort. Raises DoesNotExist + it isn't present. Uses the course_id for extra validation... + """ + return CourseUserGroup.objects.get(course_id=course_id, + group_type=CourseUserGroup.COHORT, + id=cohort_id) + def add_cohort(course_id, name): """ Add a cohort to a course. Raises ValueError if a cohort of the same name already @@ -95,6 +104,34 @@ def add_cohort(course_id, name): group_type=CourseUserGroup.COHORT, name=name) +def add_user_to_cohort(cohort, username_or_email): + """ + Look up the given user, and if successful, add them to the specified cohort. + + Arguments: + cohort: CourseUserGroup + username_or_email: string. Treated as email if has '@' + + Returns: + User object. + + Raises: + User.DoesNotExist if can't find user. + + ValueError if user already present. + """ + if '@' in username_or_email: + user = User.objects.get(email=username_or_email) + else: + user = User.objects.get(username=username_or_email) + + if cohort.users.filter(id=user.id).exists(): + raise ValueError("User {0} already present".format(user.username)) + + cohort.users.add(user) + return user + + def get_course_cohort_names(course_id): """ Return a list of the cohort names in a course. diff --git a/common/djangoapps/course_groups/views.py b/common/djangoapps/course_groups/views.py index 9ee9935c3e..f02bff2d00 100644 --- a/common/djangoapps/course_groups/views.py +++ b/common/djangoapps/course_groups/views.py @@ -1,7 +1,9 @@ import json from django_future.csrf import ensure_csrf_cookie from django.contrib.auth.decorators import login_required +from django.contrib.auth.models import User from django.core.context_processors import csrf +from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.core.urlresolvers import reverse from django.http import HttpResponse, HttpResponseForbidden, Http404 from django.shortcuts import redirect @@ -9,6 +11,8 @@ import logging from courseware.courses import get_course_with_access from mitxmako.shortcuts import render_to_response, render_to_string +from string_util import split_by_comma_and_whitespace + from .models import CourseUserGroup from . import models @@ -81,19 +85,84 @@ def add_cohort(request, course_id): @ensure_csrf_cookie def users_in_cohort(request, course_id, cohort_id): """ + Return users in the cohort. Show up to 100 per page, and page + using the 'page' GET attribute in the call. Format: + + Returns: + Json dump of dictionary in the following format: + {'success': True, + 'page': page, + 'num_pages': paginator.num_pages, + 'users': [{'username': ..., 'email': ..., 'name': ...}] + } """ get_course_with_access(request.user, course_id, 'staff') - return JsonHttpReponse({'error': 'Not implemented'}) + cohort = models.get_cohort_by_id(course_id, int(cohort_id)) + + paginator = Paginator(cohort.users.all(), 100) + page = request.GET.get('page') + try: + users = paginator.page(page) + except PageNotAnInteger: + # return the first page + page = 1 + users = paginator.page(page) + except EmptyPage: + # Page is out of range. Return last page + page = paginator.num_pages + contacts = paginator.page(page) + + user_info = [{'username': u.username, + 'email': u.email, + 'name': '{0} {1}'.format(u.first_name, u.last_name)} + for u in users] + + return JsonHttpReponse({'success': True, + 'page': page, + 'num_pages': paginator.num_pages, + 'users': user_info}) @ensure_csrf_cookie -def add_users_to_cohort(request, course_id): +def add_users_to_cohort(request, course_id, cohort_id): """ + Return json dict of: + + {'success': True, + 'added': [{'username': username, + 'name': name, + 'email': email}, ...], + 'present': [str1, str2, ...], # already there + 'unknown': [str1, str2, ...]} """ get_course_with_access(request.user, course_id, 'staff') - return JsonHttpReponse({'error': 'Not implemented'}) + if request.method != "POST": + raise Http404("Must POST to add users to cohorts") + + cohort = models.get_cohort_by_id(course_id, cohort_id) + + users = request.POST.get('users', '') + added = [] + present = [] + unknown = [] + for username_or_email in split_by_comma_and_whitespace(users): + try: + user = models.add_user_to_cohort(cohort, username_or_email) + added.append({'username': user.username, + 'name': "{0} {1}".format(user.first_name, user.last_name), + 'email': user.email, + }) + except ValueError: + present.append(username_or_email) + except User.DoesNotExist: + unknown.append(username_or_email) + + return JsonHttpReponse({'success': True, + 'added': added, + 'present': present, + 'unknown': unknown}) def debug_cohort_mgmt(request, course_id): @@ -102,7 +171,7 @@ def debug_cohort_mgmt(request, course_id): """ # add staff check to make sure it's safe if it's accidentally deployed. get_course_with_access(request.user, course_id, 'staff') - + context = {'cohorts_ajax_url': reverse('cohorts', kwargs={'course_id': course_id})} return render_to_response('/course_groups/debug.html', context) diff --git a/common/lib/string_util.py b/common/lib/string_util.py new file mode 100644 index 0000000000..0db385f2d6 --- /dev/null +++ b/common/lib/string_util.py @@ -0,0 +1,11 @@ +import itertools + +def split_by_comma_and_whitespace(s): + """ + Split a string both by on commas and whitespice. + """ + # Note: split() with no args removes empty strings from output + lists = [x.split() for x in s.split(',')] + # return all of them + return itertools.chain(*lists) + diff --git a/common/static/js/course_groups/cohorts.js b/common/static/js/course_groups/cohorts.js index 7b1793dcf8..531ce51923 100644 --- a/common/static/js/course_groups/cohorts.js +++ b/common/static/js/course_groups/cohorts.js @@ -36,19 +36,51 @@ var CohortManager = (function ($) { // constructor var module = function () { - var url = $(".cohort_manager").data('ajax_url'); + var el = $(".cohort_manager"); + // localized jquery + var $$ = function (selector) { + return $(selector, el) + } + var state_init = "init"; + var state_summary = "summary"; + var state_detail = "detail"; + var state = state_init; + + var url = el.data('ajax_url'); var self = this; - var error_list = $(".cohort_errors"); - var cohort_list = $(".cohort_list"); - var cohorts_display = $(".cohorts_display"); - var show_cohorts_button = $(".cohort_controls .show_cohorts"); - var add_cohort_input = $("#cohort-name"); - var add_cohort_button = $(".add_cohort"); + + // Pull out the relevant parts of the html + // global stuff + var errors = $$(".errors"); + + // cohort summary display + var summary = $$(".summary"); + var cohorts = $$(".cohorts"); + var show_cohorts_button = $$(".controls .show_cohorts"); + var add_cohort_input = $$(".cohort_name"); + var add_cohort_button = $$(".add_cohort"); + + // single cohort user display + var detail = $$(".detail"); + var detail_header = $(".header", detail); + var detail_users = $$(".users"); + var detail_page_num = $$(".page_num"); + var users_area = $$(".users_area"); + var add_members_button = $$(".add_members"); + var op_results = $$("op_results"); + var cohort_title = null; + var detail_url = null; + var page = null; + + // *********** Summary view methods function show_cohort(item) { // item is a li that has a data-href link to the cohort base url var el = $(this); - alert("would show you data about " + el.text() + " from " + el.data('href')); + cohort_title = el.text(); + detail_url = el.data('href'); + state = state_detail; + render(); } function add_to_cohorts_list(item) { @@ -57,24 +89,25 @@ var CohortManager = (function ($) { .data('href', url + '/' + item.id) .addClass('link') .click(show_cohort); - cohort_list.append(li); + cohorts.append(li); }; function log_error(msg) { - error_list.empty(); - error_list.append($("
  • ").text(msg).addClass("error")); + errors.empty(); + errors.append($("
  • ").text(msg).addClass("error")); }; function load_cohorts(response) { - cohort_list.empty(); + cohorts.empty(); if (response && response.success) { response.cohorts.forEach(add_to_cohorts_list); } else { log_error(response.msg || "There was an error loading cohorts"); } - cohorts_display.show(); + summary.show(); }; + function added_cohort(response) { if (response && response.success) { add_to_cohorts_list(response.cohort); @@ -83,8 +116,75 @@ var CohortManager = (function ($) { } } + // *********** Detail view methods + + function add_to_users_list(item) { + var tr = $('' + + ''); + $(".name", tr).text(item.name); + $(".username", tr).text(item.username); + $(".email", tr).text(item.email); + detail_users.append(tr); + }; + + + function show_users(response) { + detail_users.html("NameUsernameEmail"); + if (response && response.success) { + response.users.forEach(add_to_users_list); + detail_page_num.text("Page " + response.page + " of " + response.num_pages); + } else { + log_error(response.msg || + "There was an error loading users for " + cohort.title); + } + detail.show(); + } + + + function added_users(response) { + function adder(note, color) { + return function(item) { + var li = $('
  • ') + li.text(note + ' ' + item.name + ', ' + item.username + ', ' + item.email); + li.css('color', color); + op_results.append(li); + } + } + if (response && response.success) { + response.added.forEach(adder("Added", "green")); + response.present.forEach(adder("Already present:", "black")); + response.unknown.forEach(adder("Already present:", "red")); + } else { + log_error(response.msg || "There was an error adding users"); + } + } + + // ******* Rendering + + + function render() { + // Load and render the right thing based on the state + + // start with both divs hidden + summary.hide(); + detail.hide(); + // and clear out the errors + errors.empty(); + if (state == state_summary) { + $.ajax(url).done(load_cohorts).fail(function() { + log_error("Error trying to load cohorts"); + }); + } else if (state == state_detail) { + detail_header.text("Members of " + cohort_title); + $.ajax(detail_url).done(show_users).fail(function() { + log_error("Error trying to load users in cohort"); + }); + } + } + show_cohorts_button.click(function() { - $.ajax(url).done(load_cohorts); + state = state_summary; + render(); }); add_cohort_input.change(function() { @@ -101,6 +201,13 @@ var CohortManager = (function ($) { $.post(add_url, data).done(added_cohort); }); + add_members_button.click(function() { + var add_url = detail_url + '/add'; + data = {'users': users_area.val()} + $.post(add_url, data).done(added_users); + }); + + }; // prototype diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 8069d3f184..1b51698834 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -24,14 +24,15 @@ from courseware import grades from courseware.access import (has_access, get_access_group_name, course_beta_test_group_name) from courseware.courses import get_course_with_access +from courseware.models import StudentModule from django_comment_client.models import (Role, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA) from django_comment_client.utils import has_forum_access from psychometrics import psychoanalyze +from string_util import split_by_comma_and_whitespace from student.models import CourseEnrollment, CourseEnrollmentAllowed -from courseware.models import StudentModule from xmodule.course_module import CourseDescriptor from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore @@ -392,14 +393,14 @@ def instructor_dashboard(request, course_id): users = request.POST['betausers'] log.debug("users: {0!r}".format(users)) group = get_beta_group(course) - for username_or_email in _split_by_comma_and_whitespace(users): + for username_or_email in split_by_comma_and_whitespace(users): msg += "

    {0}

    ".format( add_user_to_group(request, username_or_email, group, 'beta testers', 'beta-tester')) elif action == 'Remove beta testers': users = request.POST['betausers'] group = get_beta_group(course) - for username_or_email in _split_by_comma_and_whitespace(users): + for username_or_email in split_by_comma_and_whitespace(users): msg += "

    {0}

    ".format( remove_user_from_group(request, username_or_email, group, 'beta testers', 'beta-tester')) @@ -871,21 +872,11 @@ def grade_summary(request, course_id): #----------------------------------------------------------------------------- # enrollment - -def _split_by_comma_and_whitespace(s): - """ - Split a string both by on commas and whitespice. - """ - # Note: split() with no args removes empty strings from output - lists = [x.split() for x in s.split(',')] - # return all of them - return itertools.chain(*lists) - def _do_enroll_students(course, course_id, students, overload=False): """Do the actual work of enrolling multiple students, presented as a string of emails separated by commas or returns""" - new_students = _split_by_comma_and_whitespace(students) + new_students = split_by_comma_and_whitespace(students) new_students = [str(s.strip()) for s in new_students] new_students_lc = [x.lower() for x in new_students] diff --git a/lms/templates/course_groups/cohort_management.html b/lms/templates/course_groups/cohort_management.html index 1512d09689..962d4de645 100644 --- a/lms/templates/course_groups/cohort_management.html +++ b/lms/templates/course_groups/cohort_management.html @@ -1,24 +1,40 @@

    Cohort groups

    -
    +
    Show cohorts
    -
      +
      -