From b6b8c3b8d6029a7a9fe71a9016612f262f5d4e29 Mon Sep 17 00:00:00 2001 From: Saad Yousaf Date: Thu, 11 Mar 2021 11:48:35 +0500 Subject: [PATCH] [TNL-7970] - Convert anonymized id report code to an instructor task. (#26778) Co-authored-by: SaadYousaf --- lms/djangoapps/instructor/tests/test_api.py | 33 ------------ lms/djangoapps/instructor/views/api.py | 35 ++---------- lms/djangoapps/instructor_task/api.py | 15 +++++- lms/djangoapps/instructor_task/tasks.py | 17 +++++- .../instructor_task/tasks_helper/misc.py | 53 +++++++++++++++++++ .../instructor_task/tests/test_api.py | 22 +++++++- .../js/instructor_dashboard/data_download.js | 25 ++++++++- .../instructor_dashboard_2/data_download.html | 9 ++-- 8 files changed, 138 insertions(+), 71 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index e036bb651d..cab8473704 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -2705,39 +2705,6 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment decorated_func(request, str(self.course.id)) assert func.called - @patch('lms.djangoapps.instructor.views.api.anonymous_id_for_user', Mock(return_value='42')) - @patch('lms.djangoapps.instructor.views.api.unique_id_for_user', Mock(return_value='41')) - def test_get_anon_ids(self): - """ - Test the CSV output for the anonymized user ids. - """ - base_time = datetime.datetime.now(UTC) - url = reverse('get_anon_ids', kwargs={'course_id': str(self.course.id)}) - with freeze_time(base_time): - response = self.client.post(url, {}) - - assert response['Content-Type'] == 'text/csv' - body = response.content.decode("utf-8").replace('\r', '') - assert body.startswith( - f'"User ID","Anonymized User ID","Course Specific Anonymized User ID"\n"{self.students[0].id}","41","42"\n') - assert body.endswith('"{user_id}","41","42"\n'.format(user_id=self.students[(- 1)].id)) - assert 'attachment; filename=org' in response['Content-Disposition'] - - # Test rate-limiting - # The get_anon_ids view is computationally intensive and its execution time can vary - # depending on the number of enrollments in a course. We are rate limiting it to - # prevent too many concurrent calls which could result in a denial of service for - # other users of the lms. - # Note: We use the base_time exactly so that we don't accidentally end up on the wrong - # side of the rate limit time chunking boundary. - with freeze_time(base_time): - response = self.client.post(url, {}) - assert response.status_code == 429 - - with freeze_time(base_time + datetime.timedelta(minutes=5)): - response = self.client.post(url, {}) - assert response.status_code == 200 - @patch('lms.djangoapps.instructor_task.models.logger.error') @patch.dict(settings.GRADES_DOWNLOAD, {'STORAGE_TYPE': 's3', 'ROOT_PATH': 'tmp/edx-s3/grades'}) def test_list_report_downloads_error(self, mock_error): diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 4948ec2695..6fc9cedf89 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -32,7 +32,6 @@ from edx_rest_framework_extensions.auth.session.authentication import SessionAut from edx_when.api import get_date_for_block from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey -from ratelimit.decorators import ratelimit from rest_framework import status from rest_framework.permissions import IsAdminUser, IsAuthenticated from rest_framework.response import Response @@ -57,10 +56,8 @@ from common.djangoapps.student.models import ( ManualEnrollmentAudit, Registration, UserProfile, - anonymous_id_for_user, get_user_by_username_or_email, is_email_retired, - unique_id_for_user ) from common.djangoapps.student.roles import CourseFinanceAdminRole, CourseSalesAdminRole from common.djangoapps.util.file import ( @@ -1359,40 +1356,18 @@ def get_proctored_exam_results(request, course_id): return JsonResponse({"status": success_status}) +@transaction.non_atomic_requests @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@ratelimit(key="user", rate="1/5m", block=True) @require_course_permission(permissions.CAN_RESEARCH) def get_anon_ids(request, course_id): """ Respond with 2-column CSV output of user-id, anonymized-user-id """ - # TODO: the User.objects query and CSV generation here could be - # centralized into instructor_analytics. Currently instructor_analytics - # has similar functionality but not quite what's needed. - course_id = CourseKey.from_string(course_id) - - def csv_response(filename, header, rows): - """Returns a CSV http response for the given header and rows (excel/utf-8).""" - response = HttpResponse(content_type='text/csv') - response['Content-Disposition'] = 'attachment; filename={}'.format(str(filename)) - writer = csv.writer(response, dialect='excel', quotechar='"', quoting=csv.QUOTE_ALL) - # In practice, there should not be non-ascii data in this query, - # but trying to do the right thing anyway. - encoded = [str(s) for s in header] - writer.writerow(encoded) - for row in rows: - encoded = [str(s) for s in row] - writer.writerow(encoded) - return response - - students = User.objects.filter( - courseenrollment__course_id=course_id, - ).order_by('id') - header = ['User ID', 'Anonymized User ID', 'Course Specific Anonymized User ID'] - rows = [[s.id, unique_id_for_user(s), anonymous_id_for_user(s, course_id)] - for s in students] - return csv_response(str(course_id).replace('/', '-') + '-anon-ids.csv', header, rows) + report_type = _('Anonymized User IDs') + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + task_api.generate_anonymous_ids(request, course_id) + return JsonResponse({"status": success_status}) @require_POST diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 2f2a963590..718b202997 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -42,7 +42,8 @@ from lms.djangoapps.instructor_task.tasks import ( proctored_exam_results_csv, rescore_problem, reset_problem_attempts, - send_bulk_course_email + send_bulk_course_email, + generate_anonymous_ids_for_course ) from xmodule.modulestore.django import modulestore @@ -547,3 +548,15 @@ def regenerate_certificates(request, course_key, statuses_to_regenerate): ) return instructor_task + + +def generate_anonymous_ids(request, course_key): + """ + Generate anonymize id CSV report. + """ + task_type = 'generate_anonymous_ids_for_course' + task_class = generate_anonymous_ids_for_course + task_input = {} + task_key = "" + + return submit_task(request, task_type, task_class, course_key, task_input, task_key) diff --git a/lms/djangoapps/instructor_task/tasks.py b/lms/djangoapps/instructor_task/tasks.py index 926c091fc8..b97252daa0 100644 --- a/lms/djangoapps/instructor_task/tasks.py +++ b/lms/djangoapps/instructor_task/tasks.py @@ -38,8 +38,10 @@ from lms.djangoapps.instructor_task.tasks_helper.misc import ( upload_ora2_data, upload_ora2_submission_files, upload_ora2_summary, - upload_proctored_exam_results_report + upload_proctored_exam_results_report, + generate_anonymous_ids ) + from lms.djangoapps.instructor_task.tasks_helper.module_state import ( delete_problem_module_state, override_score_module_state, @@ -295,6 +297,19 @@ def cohort_students(entry_id, xmodule_instance_args): return run_main_task(entry_id, task_fn, action_name) +@shared_task(base=BaseInstructorTask) +@set_code_owner_attribute +def generate_anonymous_ids_for_course(entry_id, xmodule_instance_args): + """ + Generate a CSV of anonymize IDs for enrolled learner for course. + """ + # Translators: This is a past-tense verb that is inserted into task progress messages as {action}. + # An example of such a message is: "Progress: {action} {succeeded} of {attempted} so far" + action_name = ugettext_noop('generate_anonymized_id') + task_fn = partial(generate_anonymous_ids, xmodule_instance_args) + return run_main_task(entry_id, task_fn, action_name) + + @shared_task(base=BaseInstructorTask) @set_code_owner_attribute def export_ora2_data(entry_id, xmodule_instance_args): diff --git a/lms/djangoapps/instructor_task/tasks_helper/misc.py b/lms/djangoapps/instructor_task/tasks_helper/misc.py index 1641d7178d..390cd3ba34 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/misc.py +++ b/lms/djangoapps/instructor_task/tasks_helper/misc.py @@ -19,6 +19,7 @@ from django.core.files.storage import DefaultStorage from openassessment.data import OraAggregateData, OraDownloadData from pytz import UTC +from common.djangoapps.student.models import unique_id_for_user, anonymous_id_for_user from lms.djangoapps.instructor_analytics.basic import get_proctored_exam_results from lms.djangoapps.instructor_analytics.csvs import format_dictlist from lms.djangoapps.survey.models import SurveyAnswer @@ -469,3 +470,55 @@ def upload_ora2_submission_files( TASK_LOG.info('%s, Task type: %s, Upload complete.', task_info_string, action_name) return UPDATE_STATUS_SUCCEEDED + + +def generate_anonymous_ids(_xmodule_instance_args, _entry_id, course_id, task_input, action_name): # lint-amnesty, pylint: disable=too-many-statements + """ + Generate a 2-column CSV output of user-id, anonymized-user-id + """ + def _log_and_update_progress(step): + """ + Updates progress task and logs + + Arguments: + step: current step task is on + """ + TASK_LOG.info( + '%s, Task type: %s, Current step: %s for all learners', + task_info_string, + action_name, + step, + ) + task_progress.update_task_state(extra_meta=step) + TASK_LOG.info('ANONYMOUS_IDS_TASK: Starting task execution.') + + task_info_string_format = 'Task: {task_id}, InstructorTask ID: {entry_id}, Course: {course_id}, Input: {task_input}' + task_info_string = task_info_string_format.format( + task_id=_xmodule_instance_args.get('task_id') if _xmodule_instance_args is not None else None, + entry_id=_entry_id, + course_id=course_id, + task_input=task_input + ) + TASK_LOG.info('%s, Task type: %s, Starting task execution', task_info_string, action_name) + + start_time = time() + start_date = datetime.now(UTC) + + students = User.objects.filter( + courseenrollment__course_id=course_id, + ).order_by('id') + + task_progress = TaskProgress(action_name, students.count, start_time) + _log_and_update_progress({'step': "Compiling learner rows"}) + + header = ['User ID', 'Anonymized User ID', 'Course Specific Anonymized User ID'] + rows = [[s.id, unique_id_for_user(s), anonymous_id_for_user(s, course_id)] + for s in students] + + task_progress.attempted = students.count + _log_and_update_progress({'step': "Finished compiling learner rows"}) + + filename = str(course_id).replace('/', '-') + '-anon-ids' + upload_csv_to_report_store([header] + rows, filename, course_id, start_date) + + return UPDATE_STATUS_SUCCEEDED diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index 10dcd05765..1fec1a466d 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -33,11 +33,13 @@ from lms.djangoapps.instructor_task.api import ( submit_rescore_problem_for_all_students, submit_rescore_problem_for_student, submit_reset_problem_attempts_for_all_students, - submit_reset_problem_attempts_in_entrance_exam + submit_reset_problem_attempts_in_entrance_exam, + generate_anonymous_ids ) from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError, QueueConnectionError from lms.djangoapps.instructor_task.models import PROGRESS, InstructorTask -from lms.djangoapps.instructor_task.tasks import export_ora2_data, export_ora2_submission_files +from lms.djangoapps.instructor_task.tasks import export_ora2_data, export_ora2_submission_files, \ + generate_anonymous_ids_for_course from lms.djangoapps.instructor_task.tests.test_base import ( TEST_COURSE_KEY, InstructorTaskCourseTestCase, @@ -368,3 +370,19 @@ class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCa # Validate that record was added to CertificateGenerationHistory assert certificate_generation_history.exists() + + def test_submit_anonymized_id_report_generation(self): + request = self.create_task_request(self.instructor) + + with patch('lms.djangoapps.instructor_task.api.submit_task') as mock_submit_task: + mock_submit_task.return_value = MagicMock() + generate_anonymous_ids(request, self.course.id) + + mock_submit_task.assert_called_once_with( + request, + 'generate_anonymous_ids_for_course', + generate_anonymous_ids_for_course, + self.course.id, + {}, + '' + ) diff --git a/lms/static/js/instructor_dashboard/data_download.js b/lms/static/js/instructor_dashboard/data_download.js index 39071c06f3..feb8a8e88e 100644 --- a/lms/static/js/instructor_dashboard/data_download.js +++ b/lms/static/js/instructor_dashboard/data_download.js @@ -118,7 +118,30 @@ this.instructor_tasks = new (PendingInstructorTasks())(this.$section); this.clear_display(); this.$list_anon_btn.click(function() { - location.href = dataDownloadObj.$list_anon_btn.data('endpoint'); + var url = dataDownloadObj.$list_anon_btn.data('endpoint'); + var errorMessage = gettext('Error generating anonymous IDs. Please try again.'); + return $.ajax({ + type: 'POST', + dataType: 'json', + url: url, + error: function(error) { + if (error.responseText) { + errorMessage = JSON.parse(error.responseText); + } + dataDownloadObj.clear_display(); + dataDownloadObj.$reports_request_response_error.text(errorMessage); + return dataDownloadObj.$reports_request_response_error.css({ + display: 'block' + }); + }, + success: function(data) { + dataDownloadObj.clear_display(); + dataDownloadObj.$reports_request_response.text(data.status); + return $('.msg-confirm').css({ + display: 'block' + }); + } + }); }); this.$proctored_exam_csv_btn.click(function() { var url = dataDownloadObj.$proctored_exam_csv_btn.data('endpoint'); diff --git a/lms/templates/instructor/instructor_dashboard_2/data_download.html b/lms/templates/instructor/instructor_dashboard_2/data_download.html index fefcfef668..20950eca11 100644 --- a/lms/templates/instructor/instructor_dashboard_2/data_download.html +++ b/lms/templates/instructor/instructor_dashboard_2/data_download.html @@ -16,9 +16,9 @@ from openedx.core.djangolib.markup import HTML, Text

-

${_("Click to download a CSV of anonymized student IDs:")}

-

- +

${_("The CSV of anonymized student IDs download has been relocated to the Reports section below.")}

+ + %if settings.FEATURES.get('ENABLE_GRADE_DOWNLOADS'):
@@ -37,6 +37,9 @@ from openedx.core.djangolib.markup import HTML, Text

+

${_("Click to download a CSV of anonymized student IDs:")}

+

+ %if section_data['show_generate_proctored_exam_report_button']:

${_("Click to generate a CSV file of all proctored exam results in this course.")}