From b4ccd03740e33e599a6f043bed2598fa33fd2a3e Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Mon, 18 Mar 2019 10:29:49 -0400 Subject: [PATCH] This adds a new django app -- edx-when -- that will copy start and due dates to mysql and allow per-learner overrides in the instructor dashboard, using the existing IDDE interface. It adds a data migration for existing IDDE data. --- .../lib/xmodule/xmodule/modulestore/django.py | 4 + .../tests/test_field_override_performance.py | 5 +- .../course_api/blocks/tests/test_api.py | 57 +----- lms/djangoapps/course_blocks/api.py | 10 +- .../transformers/access_denied_filter.py | 4 +- .../course_blocks/transformers/hide_empty.py | 4 +- .../transformers/library_content.py | 5 +- .../transformers/load_override_data.py | 3 +- .../migrations/0008_move_idde_to_edx_when.py | 37 ++++ lms/djangoapps/courseware/module_render.py | 7 +- lms/djangoapps/courseware/tests/test_views.py | 12 +- lms/djangoapps/grades/tests/test_tasks.py | 20 +-- lms/djangoapps/instructor/tests/test_api.py | 167 +++++++++--------- .../tests/test_api_email_localization.py | 2 +- .../instructor/tests/test_certificates.py | 10 +- .../instructor/tests/test_enrollment.py | 17 +- .../instructor/tests/test_proctoring.py | 10 +- .../tests/test_registration_codes.py | 2 +- lms/djangoapps/instructor/tests/test_tools.py | 61 +++---- lms/djangoapps/instructor/views/api.py | 154 ++++++++-------- .../instructor/views/gradebook_api.py | 2 +- .../instructor/views/instructor_dashboard.py | 22 +-- lms/djangoapps/instructor/views/tools.py | 65 ++----- .../tests/views/test_course_home.py | 2 +- .../tests/views/test_course_updates.py | 2 +- openedx/tests/settings.py | 1 + requirements/edx/base.in | 1 + requirements/edx/base.txt | 1 + requirements/edx/development.txt | 1 + requirements/edx/testing.txt | 1 + 30 files changed, 326 insertions(+), 363 deletions(-) create mode 100644 lms/djangoapps/courseware/migrations/0008_move_idde_to_edx_when.py diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 0013cac544..75010a0f6a 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -198,6 +198,10 @@ class SignalHandler(object): log.info('Sent %s signal to %s with kwargs %s. Response was: %s', signal_name, receiver, kwargs, response) +# to allow easy imports +globals().update({sig.name.upper(): sig for sig in SignalHandler.all_signals()}) + + def load_function(path): """ Load a function by name. diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 36aeb39ee9..7b67ee3d46 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -239,7 +239,7 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): __test__ = True # TODO: decrease query count as part of REVO-28 - QUERY_COUNT = 30 + QUERY_COUNT = 32 TEST_DATA = { # (providers, course_width, enable_ccx, view_as_ccx): ( # # of sql queries to default, @@ -268,7 +268,8 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True # TODO: decrease query count as part of REVO-28 - QUERY_COUNT = 30 + QUERY_COUNT = 32 + TEST_DATA = { ('no_overrides', 1, True, False): (QUERY_COUNT, 3), ('no_overrides', 2, True, False): (QUERY_COUNT, 3), diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index a6e2d58436..5cc3084bb4 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -7,9 +7,6 @@ from mock import patch import ddt from django.test.client import RequestFactory -from django.test.utils import override_settings - -import course_blocks.api as course_blocks_api from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE, waffle @@ -209,7 +206,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): self._get_blocks( course, expected_mongo_queries=0, - expected_sql_queries=10 if with_storage_backing else 9, + expected_sql_queries=12 if with_storage_backing else 11, ) @ddt.data( @@ -226,57 +223,9 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): clear_course_from_cache(course.id) if with_storage_backing: - num_sql_queries = 20 + num_sql_queries = 22 else: - num_sql_queries = 10 - - self._get_blocks( - course, - expected_mongo_queries, - expected_sql_queries=num_sql_queries, - ) - - -@ddt.ddt -@override_settings(FIELD_OVERRIDE_PROVIDERS=(course_blocks_api.INDIVIDUAL_STUDENT_OVERRIDE_PROVIDER, )) -class TestQueryCountsWithIndividualOverrideProvider(TestGetBlocksQueryCountsBase): - """ - Tests query counts for the get_blocks function when IndividualStudentOverrideProvider is set. - """ - - @ddt.data( - *product( - (ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split), - (True, False), - ) - ) - @ddt.unpack - def test_query_counts_cached(self, store_type, with_storage_backing): - with waffle().override(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - course = self._create_course(store_type) - self._get_blocks( - course, - expected_mongo_queries=0, - expected_sql_queries=11 if with_storage_backing else 10, - ) - - @ddt.data( - *product( - ((ModuleStoreEnum.Type.mongo, 5), (ModuleStoreEnum.Type.split, 3)), - (True, False), - ) - ) - @ddt.unpack - def test_query_counts_uncached(self, store_type_tuple, with_storage_backing): - store_type, expected_mongo_queries = store_type_tuple - with waffle().override(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - course = self._create_course(store_type) - clear_course_from_cache(course.id) - - if with_storage_backing: - num_sql_queries = 21 - else: - num_sql_queries = 11 + num_sql_queries = 12 self._get_blocks( course, diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py index e7139b64b1..9e8fc46941 100644 --- a/lms/djangoapps/course_blocks/api.py +++ b/lms/djangoapps/course_blocks/api.py @@ -4,17 +4,12 @@ get_course_blocks function. """ from django.conf import settings +from edx_when import field_data from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers from openedx.features.content_type_gating.block_transformers import ContentTypeGateTransformer -from .transformers import ( - library_content, - start_date, - user_partitions, - visibility, - load_override_data, -) +from .transformers import library_content, load_override_data, start_date, user_partitions, visibility from .usage_info import CourseUsageInfo INDIVIDUAL_STUDENT_OVERRIDE_PROVIDER = ( @@ -46,6 +41,7 @@ def get_course_block_access_transformers(user): ContentTypeGateTransformer(), user_partitions.UserPartitionTransformer(), visibility.VisibilityTransformer(), + field_data.DateOverrideTransformer(user), ] if has_individual_student_override_provider(): diff --git a/lms/djangoapps/course_blocks/transformers/access_denied_filter.py b/lms/djangoapps/course_blocks/transformers/access_denied_filter.py index adbcd5f740..be20d7ca6c 100644 --- a/lms/djangoapps/course_blocks/transformers/access_denied_filter.py +++ b/lms/djangoapps/course_blocks/transformers/access_denied_filter.py @@ -2,9 +2,7 @@ Access Denied Message Filter Transformer implementation. """ # TODO: Remove this file after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic -from openedx.core.djangoapps.content.block_structure.transformer import ( - BlockStructureTransformer -) +from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer class AccessDeniedMessageFilterTransformer(BlockStructureTransformer): diff --git a/lms/djangoapps/course_blocks/transformers/hide_empty.py b/lms/djangoapps/course_blocks/transformers/hide_empty.py index cbae63cc04..a81a51624d 100644 --- a/lms/djangoapps/course_blocks/transformers/hide_empty.py +++ b/lms/djangoapps/course_blocks/transformers/hide_empty.py @@ -2,9 +2,7 @@ Hide Empty Transformer implementation. """ # TODO: Remove this file after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic -from openedx.core.djangoapps.content.block_structure.transformer import ( - BlockStructureTransformer -) +from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer class HideEmptyTransformer(BlockStructureTransformer): diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index e566a1a0c7..de1a82580a 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -3,8 +3,9 @@ Content Library Transformer. """ import json -from courseware.models import StudentModule from eventtracking import tracker + +from courseware.models import StudentModule from openedx.core.djangoapps.content.block_structure.transformer import ( BlockStructureTransformer, FilteringTransformerMixin @@ -98,7 +99,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo # Save back any changes if any(block_keys[changed] for changed in ('invalid', 'overlimit', 'added')): state_dict['selected'] = list(selected) - StudentModule.save_state( # pylint: disable=no-value-for-parameter + StudentModule.save_state( student=usage_info.user, course_id=usage_info.course_key, module_state_key=block_key, diff --git a/lms/djangoapps/course_blocks/transformers/load_override_data.py b/lms/djangoapps/course_blocks/transformers/load_override_data.py index 7ab6eec772..6f59681aeb 100644 --- a/lms/djangoapps/course_blocks/transformers/load_override_data.py +++ b/lms/djangoapps/course_blocks/transformers/load_override_data.py @@ -3,9 +3,8 @@ Load Override Data Transformer """ import json -from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer - from courseware.models import StudentFieldOverride +from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer # The list of fields are in support of Individual due dates and could be expanded for other use cases. REQUESTED_FIELDS = [ diff --git a/lms/djangoapps/courseware/migrations/0008_move_idde_to_edx_when.py b/lms/djangoapps/courseware/migrations/0008_move_idde_to_edx_when.py new file mode 100644 index 0000000000..bc35678610 --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0008_move_idde_to_edx_when.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-03-21 15:25 +from __future__ import unicode_literals + +import json +import logging +from django.db import migrations + + +def move_overrides_to_edx_when(apps, schema_editor): + from xmodule.fields import Date + from edx_when import api + date_field = Date() + StudentFieldOverride = apps.get_model('courseware', 'StudentFieldOverride') + log = logging.getLogger(__name__) + for override in StudentFieldOverride.objects.filter(field='due'): + try: + abs_date = date_field.from_json(json.loads(override.value)) + api.set_date_for_block( + override.course_id, + override.location, + 'due', + abs_date, + user=override.student) + except Exception: # pylint: disable=broad-except + log.exception("migrating %d %r: %r", override.id, override.location, override.value) + + +class Migration(migrations.Migration): + + dependencies = [ + ('courseware', '0007_remove_done_index'), + ] + + operations = [ + migrations.RunPython(move_overrides_to_edx_when) + ] diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 55f42f0a9d..a75814094e 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -96,6 +96,8 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.x_module import XModuleDescriptor +from edx_when.field_data import DateLookupFieldData + log = logging.getLogger(__name__) @@ -164,7 +166,6 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ field_data_cache must include data from the course module and 2 levels of its descendants ''' - with modulestore().bulk_operations(course.id): course_module = get_module_for_descriptor( user, request, course, field_data_cache, course.id, course=course @@ -660,6 +661,7 @@ def get_module_system_for_user( inner_system, real_user.id, [ + partial(DateLookupFieldData, course_id=course_id, user=user), partial(OverrideFieldData.wrap, real_user, course), partial(LmsFieldData, student_data=inner_student_data), ], @@ -755,7 +757,8 @@ def get_module_system_for_user( else: anonymous_student_id = anonymous_id_for_user(user, None) - field_data = LmsFieldData(descriptor._field_data, student_data) # pylint: disable=protected-access + field_data = DateLookupFieldData(descriptor._field_data, course_id, user) # pylint: disable=protected-access + field_data = LmsFieldData(field_data, student_data) user_is_staff = bool(has_access(user, u'staff', descriptor.location, course_id)) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index a8dac6a1b2..5135f7e8a4 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -213,8 +213,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 176), - (ModuleStoreEnum.Type.split, 4, 170), + (ModuleStoreEnum.Type.mongo, 10, 178), + (ModuleStoreEnum.Type.split, 4, 172), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): @@ -1465,8 +1465,8 @@ class ProgressPageTests(ProgressPageBaseTests): self.assertContains(resp, u"Download Your Certificate") @ddt.data( - (True, 50), - (False, 49) + (True, 52), + (False, 51) ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1479,8 +1479,8 @@ class ProgressPageTests(ProgressPageBaseTests): @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 58, 38), - (True, 49, 33) + (False, 60, 40), + (True, 51, 35) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 64a15aaa45..64b4c214b9 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -175,10 +175,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 36, True), - (ModuleStoreEnum.Type.mongo, 1, 36, False), - (ModuleStoreEnum.Type.split, 3, 36, True), - (ModuleStoreEnum.Type.split, 3, 36, False), + (ModuleStoreEnum.Type.mongo, 1, 38, True), + (ModuleStoreEnum.Type.mongo, 1, 38, False), + (ModuleStoreEnum.Type.split, 3, 38, True), + (ModuleStoreEnum.Type.split, 3, 38, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -190,8 +190,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 36), - (ModuleStoreEnum.Type.split, 3, 36), + (ModuleStoreEnum.Type.mongo, 1, 38), + (ModuleStoreEnum.Type.split, 3, 38), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -236,8 +236,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 19), - (ModuleStoreEnum.Type.split, 3, 19), + (ModuleStoreEnum.Type.mongo, 1, 21), + (ModuleStoreEnum.Type.split, 3, 21), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -251,8 +251,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 37), - (ModuleStoreEnum.Type.split, 3, 37), + (ModuleStoreEnum.Type.mongo, 1, 39), + (ModuleStoreEnum.Type.split, 3, 39), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 0e377c185e..f7b045b6ca 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -3,6 +3,7 @@ Unit tests for instructor.api methods. """ from __future__ import print_function + import datetime import functools import io @@ -18,25 +19,21 @@ from django.conf import settings from django.contrib.auth.models import User from django.core import mail from django.core.files.uploadedfile import SimpleUploadedFile -from django.urls import reverse as django_reverse from django.http import HttpRequest, HttpResponse from django.test import RequestFactory, TestCase from django.test.utils import override_settings -from pytz import UTC +from django.urls import reverse as django_reverse from django.utils.translation import ugettext as _ from mock import Mock, NonCallableMock, patch from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import UsageKey +from pytz import UTC from six import text_type -import lms.djangoapps.instructor.views.api -import lms.djangoapps.instructor_task.api from bulk_email.models import BulkEmailFlag, CourseEmail, CourseEmailTemplate -from lms.djangoapps.certificates.models import CertificateStatuses -from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory -from courseware.models import StudentFieldOverride, StudentModule +from courseware.models import StudentModule from courseware.tests.factories import ( BetaTesterFactory, GlobalStaffFactory, @@ -47,6 +44,8 @@ from courseware.tests.factories import ( from courseware.tests.helpers import LoginEnrollmentTestCase from django_comment_common.models import FORUM_ROLE_COMMUNITY_TA from django_comment_common.utils import seed_permissions_roles +from lms.djangoapps.certificates.models import CertificateStatuses +from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.instructor.tests.utils import FakeContentTask, FakeEmail, FakeEmailInfo from lms.djangoapps.instructor.views.api import ( _split_input_list, @@ -63,6 +62,9 @@ from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.core.lib.xblock_utils import grade_histogram +from edx_when.api import get_overrides_for_user +from edx_when.signals import extract_dates + from shoppingcart.models import ( Coupon, CouponRedemption, @@ -351,7 +353,7 @@ class TestEndpointHttpMethods(SharedModuleStoreTestCase, LoginEnrollmentTestCase """ Tests that POST endpoints are rejected with 405 when using GET. """ - url = reverse(data, kwargs={'course_id': unicode(self.course.id)}) + url = reverse(data, kwargs={'course_id': text_type(self.course.id)}) response = self.client.get(url) self.assertEqual( @@ -366,7 +368,7 @@ class TestEndpointHttpMethods(SharedModuleStoreTestCase, LoginEnrollmentTestCase """ Tests that GET endpoints are not rejected with 405 when using GET. """ - url = reverse(data, kwargs={'course_id': unicode(self.course.id)}) + url = reverse(data, kwargs={'course_id': text_type(self.course.id)}) response = self.client.get(url) self.assertNotEqual( @@ -630,10 +632,10 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas CourseModeFactory.create(course_id=cls.audit_course.id, mode_slug=CourseMode.AUDIT) cls.url = reverse( - 'register_and_enroll_students', kwargs={'course_id': unicode(cls.course.id)} + 'register_and_enroll_students', kwargs={'course_id': text_type(cls.course.id)} ) cls.audit_course_url = reverse( - 'register_and_enroll_students', kwargs={'course_id': unicode(cls.audit_course.id)} + 'register_and_enroll_students', kwargs={'course_id': text_type(cls.audit_course.id)} ) def setUp(self): @@ -649,7 +651,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas ) self.white_label_course_url = reverse( - 'register_and_enroll_students', kwargs={'course_id': unicode(self.white_label_course.id)} + 'register_and_enroll_students', kwargs={'course_id': text_type(self.white_label_course.id)} ) self.request = RequestFactory().request() @@ -969,8 +971,8 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas manual_enrollments = ManualEnrollmentAudit.objects.all() self.assertEqual(manual_enrollments.count(), 2) - @patch.object(lms.djangoapps.instructor.views.api, 'generate_random_string', - Mock(side_effect=['first', 'first', 'second'])) + @patch('lms.djangoapps.instructor.views.api', 'generate_random_string', + Mock(side_effect=['first', 'first', 'second'])) def test_generate_unique_password_no_reuse(self): """ generate_unique_password should generate a unique password string that hasn't been generated before. @@ -2899,7 +2901,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment """ url = reverse( 'get_problem_responses', - kwargs={'course_id': unicode(self.course.id)} + kwargs={'course_id': text_type(self.course.id)} ) problem_location = '' @@ -2934,7 +2936,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment """ url = reverse( 'get_problem_responses', - kwargs={'course_id': unicode(self.course.id)} + kwargs={'course_id': text_type(self.course.id)} ) problem_location = '' @@ -2954,7 +2956,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment """ url = reverse( 'get_problem_responses', - kwargs={'course_id': unicode(self.course.id)} + kwargs={'course_id': text_type(self.course.id)} ) task_type = 'problem_responses_csv' already_running_status = generate_already_running_error_message(task_type) @@ -3030,7 +3032,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment """ url = reverse( 'get_students_who_may_enroll', - kwargs={'course_id': unicode(self.course.id)} + kwargs={'course_id': text_type(self.course.id)} ) # Successful case: response = self.client.post(url, {}) @@ -3052,7 +3054,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment """ url = reverse( 'get_proctored_exam_results', - kwargs={'course_id': unicode(self.course.id)} + kwargs={'course_id': text_type(self.course.id)} ) # Successful case: @@ -3254,8 +3256,8 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment response = self.client.post(url, {}) self.assertIn('The detailed enrollment report is being created.', response.content) - @patch.object(lms.djangoapps.instructor.views.api, 'anonymous_id_for_user', Mock(return_value='42')) - @patch.object(lms.djangoapps.instructor.views.api, 'unique_id_for_user', Mock(return_value='41')) + @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. @@ -3323,7 +3325,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment @ddt.unpack @valid_problem_location def test_calculate_report_csv_success(self, report_type, instructor_api_endpoint, task_api_endpoint, extra_instructor_api_kwargs): - kwargs = {'course_id': unicode(self.course.id)} + kwargs = {'course_id': text_type(self.course.id)} kwargs.update(extra_instructor_api_kwargs) url = reverse(instructor_api_endpoint, kwargs=kwargs) success_status = u"The {report_type} report is being created.".format(report_type=report_type) @@ -3348,7 +3350,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment task_api_endpoint, extra_instructor_api_kwargs ): # pylint: disable=unused-argument - kwargs = {'course_id': unicode(self.course.id)} + kwargs = {'course_id': text_type(self.course.id)} kwargs.update(extra_instructor_api_kwargs) url = reverse(instructor_api_endpoint, kwargs=kwargs) @@ -3370,7 +3372,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment task_api_endpoint, extra_instructor_api_kwargs ): - kwargs = {'course_id': unicode(self.course.id)} + kwargs = {'course_id': text_type(self.course.id)} kwargs.update(extra_instructor_api_kwargs) url = reverse(instructor_api_endpoint, kwargs=kwargs) @@ -3494,7 +3496,7 @@ class TestInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginEnrollmentTes ) # mock out the function which should be called to execute the action. - @patch.object(lms.djangoapps.instructor_task.api, 'submit_reset_problem_attempts_for_all_students') + @patch('lms.djangoapps.instructor_task.api.submit_reset_problem_attempts_for_all_students') def test_reset_student_attempts_all(self, act): """ Test reset all student attempts. """ url = reverse('reset_student_attempts', kwargs={'course_id': text_type(self.course.id)}) @@ -3544,7 +3546,7 @@ class TestInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginEnrollmentTes }) self.assertEqual(response.status_code, 400) - @patch.object(lms.djangoapps.instructor_task.api, 'submit_rescore_problem_for_student') + @patch('lms.djangoapps.instructor_task.api.submit_rescore_problem_for_student') def test_rescore_problem_single(self, act): """ Test rescoring of a single student. """ url = reverse('rescore_problem', kwargs={'course_id': text_type(self.course.id)}) @@ -3555,7 +3557,7 @@ class TestInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginEnrollmentTes self.assertEqual(response.status_code, 200) self.assertTrue(act.called) - @patch.object(lms.djangoapps.instructor_task.api, 'submit_rescore_problem_for_student') + @patch('lms.djangoapps.instructor_task.api.submit_rescore_problem_for_student') def test_rescore_problem_single_from_uname(self, act): """ Test rescoring of a single student. """ url = reverse('rescore_problem', kwargs={'course_id': text_type(self.course.id)}) @@ -3566,7 +3568,7 @@ class TestInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginEnrollmentTes self.assertEqual(response.status_code, 200) self.assertTrue(act.called) - @patch.object(lms.djangoapps.instructor_task.api, 'submit_rescore_problem_for_all_students') + @patch('lms.djangoapps.instructor_task.api.submit_rescore_problem_for_all_students') def test_rescore_problem_all(self, act): """ Test rescoring for all students. """ url = reverse('rescore_problem', kwargs={'course_id': text_type(self.course.id)}) @@ -3694,7 +3696,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_reset_entrance_exam_student_attempts_delete_all(self): """ Make sure no one can delete all students state on entrance exam. """ url = reverse('reset_student_attempts_for_entrance_exam', - kwargs={'course_id': unicode(self.course.id)}) + kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'all_students': True, 'delete_module': True, @@ -3704,7 +3706,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_reset_entrance_exam_student_attempts_single(self): """ Test reset single student attempts for entrance exam. """ url = reverse('reset_student_attempts_for_entrance_exam', - kwargs={'course_id': unicode(self.course.id)}) + kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, }) @@ -3718,11 +3720,11 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE ) # mock out the function which should be called to execute the action. - @patch.object(lms.djangoapps.instructor_task.api, 'submit_reset_problem_attempts_in_entrance_exam') + @patch('lms.djangoapps.instructor_task.api.submit_reset_problem_attempts_in_entrance_exam') def test_reset_entrance_exam_all_student_attempts(self, act): """ Test reset all student attempts for entrance exam. """ url = reverse('reset_student_attempts_for_entrance_exam', - kwargs={'course_id': unicode(self.course.id)}) + kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'all_students': True, }) @@ -3732,7 +3734,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_reset_student_attempts_invalid_entrance_exam(self): """ Test reset for invalid entrance exam. """ url = reverse('reset_student_attempts_for_entrance_exam', - kwargs={'course_id': unicode(self.course_with_invalid_ee.id)}) + kwargs={'course_id': text_type(self.course_with_invalid_ee.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, }) @@ -3741,7 +3743,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_entrance_exam_student_delete_state(self): """ Test delete single student entrance exam state. """ url = reverse('reset_student_attempts_for_entrance_exam', - kwargs={'course_id': unicode(self.course.id)}) + kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, 'delete_module': True, @@ -3757,7 +3759,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE staff_user = StaffFactory(course_key=self.course.id) self.client.login(username=staff_user.username, password='test') url = reverse('reset_student_attempts_for_entrance_exam', - kwargs={'course_id': unicode(self.course.id)}) + kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, 'delete_module': True, @@ -3767,17 +3769,17 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_entrance_exam_reset_student_attempts_nonsense(self): """ Test failure with both unique_student_identifier and all_students. """ url = reverse('reset_student_attempts_for_entrance_exam', - kwargs={'course_id': unicode(self.course.id)}) + kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, 'all_students': True, }) self.assertEqual(response.status_code, 400) - @patch.object(lms.djangoapps.instructor_task.api, 'submit_rescore_entrance_exam_for_student') + @patch('lms.djangoapps.instructor_task.api.submit_rescore_entrance_exam_for_student') def test_rescore_entrance_exam_single_student(self, act): """ Test re-scoring of entrance exam for single student. """ - url = reverse('rescore_entrance_exam', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('rescore_entrance_exam', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, }) @@ -3786,7 +3788,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_rescore_entrance_exam_all_student(self): """ Test rescoring for all students. """ - url = reverse('rescore_entrance_exam', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('rescore_entrance_exam', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'all_students': True, }) @@ -3794,7 +3796,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_rescore_entrance_exam_if_higher_all_student(self): """ Test rescoring for all students only if higher. """ - url = reverse('rescore_entrance_exam', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('rescore_entrance_exam', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'all_students': True, 'only_if_higher': True, @@ -3803,7 +3805,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_rescore_entrance_exam_all_student_and_single(self): """ Test re-scoring with both all students and single student parameters. """ - url = reverse('rescore_entrance_exam', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('rescore_entrance_exam', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, 'all_students': True, @@ -3812,7 +3814,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_rescore_entrance_exam_with_invalid_exam(self): """ Test re-scoring of entrance exam with invalid exam. """ - url = reverse('rescore_entrance_exam', kwargs={'course_id': unicode(self.course_with_invalid_ee.id)}) + url = reverse('rescore_entrance_exam', kwargs={'course_id': text_type(self.course_with_invalid_ee.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, }) @@ -3821,13 +3823,13 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_list_entrance_exam_instructor_tasks_student(self): """ Test list task history for entrance exam AND student. """ # create a re-score entrance exam task - url = reverse('rescore_entrance_exam', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('rescore_entrance_exam', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, }) self.assertEqual(response.status_code, 200) - url = reverse('list_entrance_exam_instructor_tasks', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('list_entrance_exam_instructor_tasks', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, }) @@ -3840,7 +3842,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_list_entrance_exam_instructor_tasks_all_student(self): """ Test list task history for entrance exam AND all student. """ - url = reverse('list_entrance_exam_instructor_tasks', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('list_entrance_exam_instructor_tasks', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, {}) self.assertEqual(response.status_code, 200) @@ -3851,7 +3853,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_list_entrance_exam_instructor_with_invalid_exam_key(self): """ Test list task history for entrance exam failure if course has invalid exam. """ url = reverse('list_entrance_exam_instructor_tasks', - kwargs={'course_id': unicode(self.course_with_invalid_ee.id)}) + kwargs={'course_id': text_type(self.course_with_invalid_ee.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, }) @@ -3860,7 +3862,7 @@ class TestEntranceExamInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginE def test_skip_entrance_exam_student(self): """ Test skip entrance exam api for student. """ # create a re-score entrance exam task - url = reverse('mark_student_can_skip_entrance_exam', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('mark_student_can_skip_entrance_exam', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.student.email, }) @@ -4095,7 +4097,7 @@ class TestInstructorAPITaskLists(SharedModuleStoreTestCase, LoginEnrollmentTestC self.tasks = [self.FakeTask(mock_factory.mock_get_task_completion_info) for _ in xrange(7)] self.tasks[-1].make_invalid_output() - @patch.object(lms.djangoapps.instructor_task.api, 'get_running_instructor_tasks') + @patch('lms.djangoapps.instructor_task.api.get_running_instructor_tasks') def test_list_instructor_tasks_running(self, act): """ Test list of all running tasks. """ act.return_value = self.tasks @@ -4116,7 +4118,7 @@ class TestInstructorAPITaskLists(SharedModuleStoreTestCase, LoginEnrollmentTestC self.assertDictEqual(exp_task, act_task) self.assertEqual(actual_tasks, expected_tasks) - @patch.object(lms.djangoapps.instructor_task.api, 'get_instructor_task_history') + @patch('lms.djangoapps.instructor_task.api.get_instructor_task_history') def test_list_background_email_tasks(self, act): """Test list of background email tasks.""" act.return_value = self.tasks @@ -4137,7 +4139,7 @@ class TestInstructorAPITaskLists(SharedModuleStoreTestCase, LoginEnrollmentTestC self.assertDictEqual(exp_task, act_task) self.assertEqual(actual_tasks, expected_tasks) - @patch.object(lms.djangoapps.instructor_task.api, 'get_instructor_task_history') + @patch('lms.djangoapps.instructor_task.api.get_instructor_task_history') def test_list_instructor_tasks_problem(self, act): """ Test list task history for problem. """ act.return_value = self.tasks @@ -4160,7 +4162,7 @@ class TestInstructorAPITaskLists(SharedModuleStoreTestCase, LoginEnrollmentTestC self.assertDictEqual(exp_task, act_task) self.assertEqual(actual_tasks, expected_tasks) - @patch.object(lms.djangoapps.instructor_task.api, 'get_instructor_task_history') + @patch('lms.djangoapps.instructor_task.api.get_instructor_task_history') def test_list_instructor_tasks_problem_student(self, act): """ Test list task history for problem AND student. """ act.return_value = self.tasks @@ -4186,7 +4188,7 @@ class TestInstructorAPITaskLists(SharedModuleStoreTestCase, LoginEnrollmentTestC self.assertEqual(actual_tasks, expected_tasks) -@patch.object(lms.djangoapps.instructor_task.api, 'get_instructor_task_history', autospec=True) +@patch('lms.djangoapps.instructor_task.api.get_instructor_task_history', autospec=True) class TestInstructorEmailContentList(SharedModuleStoreTestCase, LoginEnrollmentTestCase): """ Test the instructor email content history endpoint. @@ -4347,7 +4349,7 @@ class TestInstructorAPIHelpers(TestCase): course_id = CourseKey.from_string('MITx/6.002x/2013_Spring') name = 'L2Node1' output = 'i4x://MITx/6.002x/problem/L2Node1' - self.assertEqual(unicode(msk_from_problem_urlname(course_id, name)), output) + self.assertEqual(text_type(msk_from_problem_urlname(course_id, name)), output) def test_msk_from_problem_urlname_error(self): args = ('notagoodcourse', 'L2Node1') @@ -4360,16 +4362,14 @@ def get_extended_due(course, unit, user): Gets the overridden due date for the given user on the given unit. Returns `None` if there is no override set. """ - try: - override = StudentFieldOverride.objects.get( - course_id=course.id, - student=user, - location=unit.location, - field='due' - ) - return DATE_FIELD.from_json(json.loads(override.value)) - except StudentFieldOverride.DoesNotExist: - return None + location = text_type(unit.location) + dates = get_overrides_for_user(course.id, user) + for override in dates: + if text_type(override['location']) == location: + return override['actual_date'] + print(unit.location) + print(dates) + return None class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @@ -4453,6 +4453,7 @@ class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.user2 = user2 self.instructor = InstructorFactory(course_key=self.course.id) self.client.login(username=self.instructor.username, password='test') + extract_dates(None, self.course.id) def test_change_due_date(self): url = reverse('change_due_date', kwargs={'course_id': text_type(self.course.id)}) @@ -4500,18 +4501,10 @@ class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase): }) self.assertEqual(response.status_code, 200, response.content) self.assertEqual( - None, + self.due, get_extended_due(self.course, self.week1, self.user1) ) - def test_reset_nonexistent_extension(self): - url = reverse('reset_due_date', kwargs={'course_id': text_type(self.course.id)}) - response = self.client.post(url, { - 'student': self.user1.username, - 'url': text_type(self.week1.location), - }) - self.assertEqual(response.status_code, 400, response.content) - def test_show_unit_extensions(self): self.test_change_due_date() url = reverse('show_unit_extensions', @@ -4619,6 +4612,7 @@ class TestDueDateExtensionsDeletedDate(ModuleStoreTestCase, LoginEnrollmentTestC self.user2 = user2 self.instructor = InstructorFactory(course_key=self.course.id) self.client.login(username=self.instructor.username, password='test') + extract_dates(None, self.course.id) def test_reset_extension_to_deleted_date(self): """ @@ -4638,6 +4632,7 @@ class TestDueDateExtensionsDeletedDate(ModuleStoreTestCase, LoginEnrollmentTestC self.week1.due = None self.week1 = self.store.update_item(self.week1, self.user1.id) + extract_dates(None, self.course.id) # Now, week1's normal due date is deleted but the extension still exists. url = reverse('reset_due_date', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, { @@ -4646,7 +4641,7 @@ class TestDueDateExtensionsDeletedDate(ModuleStoreTestCase, LoginEnrollmentTestC }) self.assertEqual(response.status_code, 200, response.content) self.assertEqual( - None, + self.due, get_extended_due(self.course, self.week1, self.user1) ) @@ -4681,7 +4676,7 @@ class TestCourseIssuedCertificatesData(SharedModuleStoreTestCase): """ Test certificates with status 'downloadable' should be in the response. """ - url = reverse('get_issued_certificates', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('get_issued_certificates', kwargs={'course_id': text_type(self.course.id)}) # firstly generating downloadable certificates with 'honor' mode certificate_count = 3 for __ in xrange(certificate_count): @@ -4703,7 +4698,7 @@ class TestCourseIssuedCertificatesData(SharedModuleStoreTestCase): """ Test for certificate csv features against mode. Certificates should be group by 'mode' in reponse. """ - url = reverse('get_issued_certificates', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('get_issued_certificates', kwargs={'course_id': text_type(self.course.id)}) # firstly generating downloadable certificates with 'honor' mode certificate_count = 3 for __ in xrange(certificate_count): @@ -4744,7 +4739,7 @@ class TestCourseIssuedCertificatesData(SharedModuleStoreTestCase): """ Test for certificate csv features. """ - url = reverse('get_issued_certificates', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('get_issued_certificates', kwargs={'course_id': text_type(self.course.id)}) # firstly generating downloadable certificates with 'honor' mode certificate_count = 3 for __ in xrange(certificate_count): @@ -4929,8 +4924,8 @@ class TestCourseRegistrationCodes(SharedModuleStoreTestCase): and row_data[1].endswith('/shoppingcart/register/redeem/{0}/"'.format(code))) index += 1 - @patch.object(lms.djangoapps.instructor.views.api, 'random_code_generator', - Mock(side_effect=['first', 'second', 'third', 'fourth'])) + @patch('lms.djangoapps.instructor.views.api.random_code_generator', + Mock(side_effect=['first', 'second', 'third', 'fourth'])) def test_generate_course_registration_codes_matching_existing_coupon_code(self): """ Test the generated course registration code is already in the Coupon Table @@ -4955,8 +4950,8 @@ class TestCourseRegistrationCodes(SharedModuleStoreTestCase): self.assertTrue(body.startswith(EXPECTED_CSV_HEADER)) self.assertEqual(len(body.split('\n')), 5) # 1 for headers, 1 for new line at the end and 3 for the actual data - @patch.object(lms.djangoapps.instructor.views.api, 'random_code_generator', - Mock(side_effect=['first', 'first', 'second', 'third'])) + @patch('lms.djangoapps.instructor.views.api.random_code_generator', + Mock(side_effect=['first', 'first', 'second', 'third'])) def test_generate_course_registration_codes_integrity_error(self): """ Test for the Integrity error against the generated code @@ -5248,7 +5243,7 @@ class TestBulkCohorting(SharedModuleStoreTestCase): with open(file_name, 'w') as file_pointer: file_pointer.write(csv_data.encode('utf-8')) with open(file_name, 'r') as file_pointer: - url = reverse('add_users_to_cohorts', kwargs={'course_id': unicode(self.course.id)}) + url = reverse('add_users_to_cohorts', kwargs={'course_id': text_type(self.course.id)}) return self.client.post(url, {'uploaded-file': file_pointer}) def expect_error_on_file_content(self, file_content, error, file_suffix='.csv'): @@ -5318,7 +5313,7 @@ class TestBulkCohorting(SharedModuleStoreTestCase): response = self.call_add_users_to_cohorts('') self.assertEqual(response.status_code, 403) - @patch('lms.djangoapps.instructor.views.api.lms.djangoapps.instructor_task.api.submit_cohort_students') + @patch('lms.djangoapps.instructor_task.api.submit_cohort_students') @patch('lms.djangoapps.instructor.views.api.store_uploaded_file') def test_success_username(self, mock_store_upload, mock_cohort_task): """ @@ -5329,7 +5324,7 @@ class TestBulkCohorting(SharedModuleStoreTestCase): 'username,cohort\nfoo_username,bar_cohort', mock_store_upload, mock_cohort_task ) - @patch('lms.djangoapps.instructor.views.api.lms.djangoapps.instructor_task.api.submit_cohort_students') + @patch('lms.djangoapps.instructor_task.api.submit_cohort_students') @patch('lms.djangoapps.instructor.views.api.store_uploaded_file') def test_success_email(self, mock_store_upload, mock_cohort_task): """ @@ -5340,7 +5335,7 @@ class TestBulkCohorting(SharedModuleStoreTestCase): 'email,cohort\nfoo_email,bar_cohort', mock_store_upload, mock_cohort_task ) - @patch('lms.djangoapps.instructor.views.api.lms.djangoapps.instructor_task.api.submit_cohort_students') + @patch('lms.djangoapps.instructor_task.api.submit_cohort_students') @patch('lms.djangoapps.instructor.views.api.store_uploaded_file') def test_success_username_and_email(self, mock_store_upload, mock_cohort_task): """ @@ -5351,7 +5346,7 @@ class TestBulkCohorting(SharedModuleStoreTestCase): 'username,email,cohort\nfoo_username,bar_email,baz_cohort', mock_store_upload, mock_cohort_task ) - @patch('lms.djangoapps.instructor.views.api.lms.djangoapps.instructor_task.api.submit_cohort_students') + @patch('lms.djangoapps.instructor_task.api.submit_cohort_students') @patch('lms.djangoapps.instructor.views.api.store_uploaded_file') def test_success_carriage_return(self, mock_store_upload, mock_cohort_task): """ @@ -5362,7 +5357,7 @@ class TestBulkCohorting(SharedModuleStoreTestCase): 'username,email,cohort\rfoo_username,bar_email,baz_cohort', mock_store_upload, mock_cohort_task ) - @patch('lms.djangoapps.instructor.views.api.lms.djangoapps.instructor_task.api.submit_cohort_students') + @patch('lms.djangoapps.instructor_task.api.submit_cohort_students') @patch('lms.djangoapps.instructor.views.api.store_uploaded_file') def test_success_carriage_return_line_feed(self, mock_store_upload, mock_cohort_task): """ diff --git a/lms/djangoapps/instructor/tests/test_api_email_localization.py b/lms/djangoapps/instructor/tests/test_api_email_localization.py index fc578f5942..d7f38b21b8 100644 --- a/lms/djangoapps/instructor/tests/test_api_email_localization.py +++ b/lms/djangoapps/instructor/tests/test_api_email_localization.py @@ -4,8 +4,8 @@ Unit tests for the localization of emails sent by instructor.api methods. """ from django.core import mail -from django.urls import reverse from django.test.utils import override_settings +from django.urls import reverse from six import text_type from courseware.tests.factories import InstructorFactory diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index 50bc3282bb..e285dbb704 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -4,17 +4,19 @@ import io import json from datetime import datetime, timedelta -import ddt import mock import pytz -from config_models.models import cache from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.core.files.uploadedfile import SimpleUploadedFile -from django.urls import reverse from django.test.utils import override_settings +from django.urls import reverse +import ddt from capa.xqueue_interface import XQueueInterface +from config_models.models import cache +from course_modes.models import CourseMode +from courseware.tests.factories import GlobalStaffFactory, InstructorFactory, UserFactory from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, @@ -28,8 +30,6 @@ from lms.djangoapps.certificates.tests.factories import ( CertificateWhitelistFactory, GeneratedCertificateFactory ) -from course_modes.models import CourseMode -from courseware.tests.factories import GlobalStaffFactory, InstructorFactory, UserFactory from lms.djangoapps.grades.tests.utils import mock_passing_grade from lms.djangoapps.verify_student.services import IDVerificationService from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index e844bcd3b9..af2f809741 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -8,15 +8,15 @@ import json from abc import ABCMeta import ddt -import mock from ccx_keys.locator import CCXLocator +from crum import set_current_request from django.conf import settings -from django.utils.translation import override as override_language from django.utils.translation import get_language +from django.utils.translation import override as override_language from mock import patch from opaque_keys.edx.locator import CourseLocator from six import text_type -from crum import set_current_request +from submissions import api as sub_api from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from courseware.models import StudentModule @@ -38,7 +38,6 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, get_moc from student.models import CourseEnrollment, CourseEnrollmentAllowed, anonymous_id_for_user from student.roles import CourseCcxCoachRole from student.tests.factories import AdminFactory, UserFactory -from submissions import api as sub_api from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -372,7 +371,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): reset_student_attempts(self.course_key, self.user, msk, requesting_user=self.user) self.assertEqual(json.loads(module().state)['attempts'], 0) - @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send') + @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send') def test_delete_student_attempts(self, _mock_signal): msk = self.course_key.make_usage_key('dummy', 'module') original_state = json.dumps({'attempts': 32, 'otherstuff': 'alsorobots'}) @@ -398,9 +397,9 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): # Disable the score change signal to prevent other components from being # pulled into tests. - @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send') - @mock.patch('lms.djangoapps.grades.signals.handlers.submissions_score_set_handler') - @mock.patch('lms.djangoapps.grades.signals.handlers.submissions_score_reset_handler') + @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send') + @patch('lms.djangoapps.grades.signals.handlers.submissions_score_set_handler') + @patch('lms.djangoapps.grades.signals.handlers.submissions_score_reset_handler') def test_delete_submission_scores(self, _mock_send_signal, mock_set_receiver, mock_reset_receiver): user = UserFactory() problem_location = self.course_key.make_usage_key('dummy', 'module') @@ -746,7 +745,7 @@ class TestGetEmailParams(SharedModuleStoreTestCase): def test_marketing_params(self): # For a site with a marketing front end, what do we expect to get for the URLs? # Also make sure `auto_enroll` is properly passed through. - with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): result = get_email_params(self.course, True) self.assertEqual(result['auto_enroll'], True) diff --git a/lms/djangoapps/instructor/tests/test_proctoring.py b/lms/djangoapps/instructor/tests/test_proctoring.py index 221f9cf84d..b82d803b4f 100644 --- a/lms/djangoapps/instructor/tests/test_proctoring.py +++ b/lms/djangoapps/instructor/tests/test_proctoring.py @@ -2,18 +2,16 @@ Unit tests for Edx Proctoring feature flag in new instructor dashboard. """ -import ddt from django.apps import apps from django.conf import settings from django.urls import reverse - -from edx_proctoring.api import create_exam -from edx_proctoring.backends.tests.test_backend import TestBackendProvider - from mock import patch from six import text_type -from student.roles import CourseStaffRole, CourseInstructorRole +import ddt +from edx_proctoring.api import create_exam +from edx_proctoring.backends.tests.test_backend import TestBackendProvider +from student.roles import CourseInstructorRole, CourseStaffRole from student.tests.factories import AdminFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory diff --git a/lms/djangoapps/instructor/tests/test_registration_codes.py b/lms/djangoapps/instructor/tests/test_registration_codes.py index b7cb56b0ef..601dcedbfb 100644 --- a/lms/djangoapps/instructor/tests/test_registration_codes.py +++ b/lms/djangoapps/instructor/tests/test_registration_codes.py @@ -3,8 +3,8 @@ Test for the registration code status information. """ import json -from django.urls import reverse from django.test.utils import override_settings +from django.urls import reverse from django.utils.translation import ugettext as _ from six import text_type diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py index 6a035c7ee0..f241dd4c3a 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -1,6 +1,7 @@ """ Tests for views/tools.py. """ +from __future__ import absolute_import, unicode_literals import datetime import json @@ -11,13 +12,11 @@ import six from django.contrib.auth.models import User from django.core.exceptions import MultipleObjectsReturned from django.test import TestCase -from django.test.utils import override_settings -from pytz import UTC from opaque_keys.edx.keys import CourseKey -from six import text_type +from pytz import UTC -from lms.djangoapps.courseware.field_overrides import OverrideFieldData -from lms.djangoapps.ccx.tests.test_overrides import inject_field_overrides +from edx_when import signals +from edx_when.field_data import DateLookupFieldData from student.tests.factories import UserFactory from xmodule.fields import Date from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase @@ -118,7 +117,7 @@ class TestFindUnit(SharedModuleStoreTestCase): """ Test finding a nested unit. """ - url = text_type(self.homework.location) + url = six.text_type(self.homework.location) found_unit = tools.find_unit(self.course, url) self.assertEqual(found_unit.location, self.homework.location) @@ -161,7 +160,7 @@ class TestGetUnitsWithDueDate(ModuleStoreTestCase): """ URLs for sequence of nodes. """ - return sorted(text_type(i.location) for i in seq) + return sorted(six.text_type(i.location) for i in seq) self.assertEquals( urls(tools.get_units_with_due_date(self.course)), @@ -177,6 +176,7 @@ class TestTitleOrUrl(unittest.TestCase): self.assertEquals(tools.title_or_url(unit), 'hello') def test_url(self): + # pylint: disable=unused-argument def mock_location_text(self): """ Mock implementation of __unicode__ or __str__ for the unit's location. @@ -191,10 +191,13 @@ class TestTitleOrUrl(unittest.TestCase): self.assertEquals(tools.title_or_url(unit), u'test:hello') -@override_settings( - FIELD_OVERRIDE_PROVIDERS=( - 'lms.djangoapps.courseware.student_field_overrides.IndividualStudentOverrideProvider',), -) +def inject_field_data(blocks, course, user): + use_cached = False + for block in blocks: + block._field_data = DateLookupFieldData(block._field_data, course.id, user, use_cached=use_cached) # pylint: disable=protected-access + use_cached = True + + class TestSetDueDateExtension(ModuleStoreTestCase): """ Test the set_due_date_extensions function. @@ -212,6 +215,7 @@ class TestSetDueDateExtension(ModuleStoreTestCase): week3 = ItemFactory.create(parent=course) homework = ItemFactory.create(parent=week1) assignment = ItemFactory.create(parent=homework, due=due) + signals.extract_dates(None, course.id) user = UserFactory.create() @@ -223,11 +227,7 @@ class TestSetDueDateExtension(ModuleStoreTestCase): self.week3 = week3 self.user = user - inject_field_overrides((course, week1, week2, week3, homework, assignment), course, user) - - def tearDown(self): - super(TestSetDueDateExtension, self).tearDown() - OverrideFieldData.provider_classes = None + inject_field_data((course, week1, week2, week3, homework, assignment), course, user) def _clear_field_data_cache(self): """ @@ -237,22 +237,18 @@ class TestSetDueDateExtension(ModuleStoreTestCase): """ for block in (self.week1, self.week2, self.week3, self.homework, self.assignment): + block._field_data._load_dates(self.course.id, self.user, use_cached=False) # pylint: disable=protected-access block.fields['due']._del_cached_value(block) # pylint: disable=protected-access def test_set_due_date_extension(self): extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=UTC) tools.set_due_date_extension(self.course, self.week1, self.user, extended) + tools.set_due_date_extension(self.course, self.assignment, self.user, extended) self._clear_field_data_cache() self.assertEqual(self.week1.due, extended) self.assertEqual(self.homework.due, extended) self.assertEqual(self.assignment.due, extended) - def test_set_due_date_extension_num_queries(self): - extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=UTC) - with self.assertNumQueries(5): - tools.set_due_date_extension(self.course, self.week1, self.user, extended) - self._clear_field_data_cache() - def test_set_due_date_extension_invalid_date(self): extended = datetime.datetime(2009, 1, 1, 0, 0, tzinfo=UTC) with self.assertRaises(tools.DashboardError): @@ -299,6 +295,7 @@ class TestDataDumps(ModuleStoreTestCase): self.week2 = week2 self.user1 = user1 self.user2 = user2 + signals.extract_dates(None, course.id) def test_dump_module_extensions(self): extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=UTC) @@ -307,12 +304,12 @@ class TestDataDumps(ModuleStoreTestCase): tools.set_due_date_extension(self.course, self.week1, self.user2, extended) report = tools.dump_module_extensions(self.course, self.week1) - self.assertEqual( - report['title'], u'Users with due date extensions for ' + + assert ( + report['title'] == 'Users with due date extensions for ' + self.week1.display_name) - self.assertEqual( - report['header'], ["Username", "Full Name", "Extended Due Date"]) - self.assertEqual(report['data'], [ + assert ( + report['header'] == ["Username", "Full Name", "Extended Due Date"]) + assert (report['data'] == [ {"Username": self.user1.username, "Full Name": self.user1.profile.name, "Extended Due Date": "2013-12-25 00:00"}, @@ -327,12 +324,12 @@ class TestDataDumps(ModuleStoreTestCase): tools.set_due_date_extension(self.course, self.week2, self.user1, extended) report = tools.dump_student_extensions(self.course, self.user1) - self.assertEqual( - report['title'], u'Due date extensions for %s (%s)' % + assert ( + report['title'] == 'Due date extensions for %s (%s)' % (self.user1.profile.name, self.user1.username)) - self.assertEqual( - report['header'], ["Unit", "Extended Due Date"]) - self.assertEqual(report['data'], [ + assert ( + report['header'] == ["Unit", "Extended Due Date"]) + assert (report['data'] == [ {"Unit": self.week1.display_name, "Extended Due Date": "2013-12-25 00:00"}, {"Unit": self.week2.display_name, diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 6786b53380..d4617fae8c 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -18,18 +18,13 @@ import time import unicodecsv from django.conf import settings from django.contrib.auth.models import User -from django.core.exceptions import ( - MultipleObjectsReturned, - ObjectDoesNotExist, - PermissionDenied, - ValidationError -) +from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist, PermissionDenied, ValidationError from django.core.mail.message import EmailMessage -from django.urls import reverse from django.core.validators import validate_email from django.db import IntegrityError, transaction from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound from django.shortcuts import redirect +from django.urls import reverse from django.utils.decorators import method_decorator from django.utils.html import strip_tags from django.utils.translation import ugettext as _ @@ -44,34 +39,37 @@ from rest_framework import permissions, status from rest_framework.response import Response from rest_framework.views import APIView from six import text_type +from submissions import api as sub_api # installed from the edx-submissions repository import instructor_analytics.basic import instructor_analytics.csvs import instructor_analytics.distributions -import lms.djangoapps.instructor.enrollment as enrollment -import lms.djangoapps.instructor_task.api from bulk_email.models import BulkEmailFlag, CourseEmail -from lms.djangoapps.certificates import api as certs_api -from lms.djangoapps.certificates.models import ( - CertificateInvalidation, CertificateStatuses, CertificateWhitelist, GeneratedCertificate, -) from courseware.access import has_access from courseware.courses import get_course_by_id, get_course_with_access from courseware.models import StudentModule from django_comment_client.utils import ( - has_forum_access, get_course_discussion_settings, + get_group_id_for_user, get_group_name, - get_group_id_for_user + has_forum_access ) from django_comment_common.models import ( - Role, FORUM_ROLE_ADMINISTRATOR, - FORUM_ROLE_MODERATOR, - FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_MODERATOR, + Role ) from edxmako.shortcuts import render_to_string +from lms.djangoapps.certificates import api as certs_api +from lms.djangoapps.certificates.models import ( + CertificateInvalidation, + CertificateStatuses, + CertificateWhitelist, + GeneratedCertificate +) +from lms.djangoapps.instructor import enrollment from lms.djangoapps.instructor.access import ROLES, allow_access, list_with_level, revoke_access, update_forum_role from lms.djangoapps.instructor.enrollment import ( enroll_email, @@ -83,7 +81,7 @@ from lms.djangoapps.instructor.enrollment import ( ) from lms.djangoapps.instructor.views import INVOICE_KEY from lms.djangoapps.instructor.views.instructor_task_helpers import extract_email_features, extract_task_features -from lms.djangoapps.instructor_task.api import submit_override_score +from lms.djangoapps.instructor_task import api as task_api from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError, QueueConnectionError from lms.djangoapps.instructor_task.models import ReportStore from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -119,11 +117,10 @@ from student.models import ( UserProfile, anonymous_id_for_user, get_user_by_username_or_email, - unique_id_for_user, - is_email_retired + is_email_retired, + unique_id_for_user ) from student.roles import CourseFinanceAdminRole, CourseSalesAdminRole -from submissions import api as sub_api # installed from the edx-submissions repository from util.file import ( FileValidationException, UniversalNewlineIterator, @@ -210,7 +207,7 @@ def require_post_params(*args, **kwargs): error_response_data['parameters'].append(param) error_response_data['info'][param] = extra - if len(error_response_data['parameters']) > 0: + if error_response_data['parameters']: return JsonResponse(error_response_data, status=400) else: return func(*args, **kwargs) @@ -319,11 +316,12 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man -If the email address already exists, but the username is different, match on the email address only and continue to enroll the user in the course using the email address - as the matching criteria. Note the change of username as a warning message (but not a failure). Send a standard enrollment email - which is the same as the existing manual enrollment + as the matching criteria. Note the change of username as a warning message (but not a failure). + Send a standard enrollment email which is the same as the existing manual enrollment - -If the username already exists (but not the email), assume it is a different user and fail to create the new account. - The failure will be messaged in a response in the browser. + -If the username already exists (but not the email), assume it is a different user and fail + to create the new account. + The failure will be messaged in a response in the browser. """ if not configuration_helpers.get_value( @@ -373,11 +371,13 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man # verify that we have exactly four columns in every row but allow for blank lines if len(student) != 4: - if len(student) > 0: + if student: + error = _(u'Data in row #{row_num} must have exactly four columns: ' + 'email, username, full name, and country').format(row_num=row_num) general_errors.append({ 'username': '', 'email': '', - 'response': _(u'Data in row #{row_num} must have exactly four columns: email, username, full name, and country').format(row_num=row_num) + 'response': error }) continue @@ -392,7 +392,10 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man validate_email(email) # Raises ValidationError if invalid except ValidationError: row_errors.append({ - 'username': username, 'email': email, 'response': _('Invalid email {email_address}.').format(email_address=email)}) + 'username': username, + 'email': email, + 'response': _(u'Invalid email {email_address}.').format(email_address=email) + }) else: if User.objects.filter(email=email).exists(): # Email address already exists. assume it is the correct user @@ -429,7 +432,11 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man reason='Enrolling via csv upload', state_transition=UNENROLLED_TO_ENROLLED, ) - enroll_email(course_id=course_id, student_email=email, auto_enroll=True, email_students=True, email_params=email_params) + enroll_email(course_id=course_id, + student_email=email, + auto_enroll=True, + email_students=True, + email_params=email_params) elif is_email_retired(email): # We are either attempting to enroll a retired user or create a new user with an email which is # already associated with a retired account. Simply block these attempts. @@ -573,7 +580,9 @@ def create_and_enroll_user(email, username, name, country, password, course_id, ) except IntegrityError: errors.append({ - 'username': username, 'email': email, 'response': _(u'Username {user} already exists.').format(user=username) + 'username': username, + 'email': email, + 'response': _(u'Username {user} already exists.').format(user=username) }) except Exception as ex: # pylint: disable=broad-except log.exception(type(ex).__name__) @@ -1027,7 +1036,7 @@ def get_problem_responses(request, course_id): except InvalidKeyError: return JsonResponseBadRequest(_("Could not find problem with this location.")) - task = lms.djangoapps.instructor_task.api.submit_calculate_problem_responses_csv( + task = task_api.submit_calculate_problem_responses_csv( request, course_key, problem_location ) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) @@ -1317,7 +1326,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red return JsonResponse(response_payload) else: - lms.djangoapps.instructor_task.api.submit_calculate_students_features_csv( + task_api.submit_calculate_students_features_csv( request, course_key, query_features @@ -1345,7 +1354,7 @@ def get_students_who_may_enroll(request, course_id): course_key = CourseKey.from_string(course_id) query_features = ['email'] report_type = _('enrollment') - lms.djangoapps.instructor_task.api.submit_calculate_may_enroll_csv(request, course_key, query_features) + task_api.submit_calculate_may_enroll_csv(request, course_key, query_features) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) return JsonResponse({"status": success_status}) @@ -1392,7 +1401,7 @@ def add_users_to_cohorts(request, course_id): validator=_cohorts_csv_validator ) # The task will assume the default file storage. - lms.djangoapps.instructor_task.api.submit_cohort_students(request, course_key, filename) + task_api.submit_cohort_students(request, course_key, filename) except (FileValidationException, PermissionDenied) as err: return JsonResponse({"error": unicode(err)}, status=400) @@ -1436,7 +1445,7 @@ class CohortCSV(DeveloperErrorViewMixin, APIView): max_file_size=2000000, # limit to 2 MB validator=_cohorts_csv_validator ) - lms.djangoapps.instructor_task.api.submit_cohort_students(request, course_key, file_name) + task_api.submit_cohort_students(request, course_key, file_name) except (FileValidationException, ValueError) as e: raise self.api_error(status.HTTP_400_BAD_REQUEST, str(e), 'failed-validation') return Response(status=status.HTTP_204_NO_CONTENT) @@ -1484,7 +1493,7 @@ def get_enrollment_report(request, course_id): """ course_key = CourseKey.from_string(course_id) report_type = _('detailed enrollment') - lms.djangoapps.instructor_task.api.submit_detailed_enrollment_features_csv(request, course_key) + task_api.submit_detailed_enrollment_features_csv(request, course_key) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) return JsonResponse({"status": success_status}) @@ -1503,7 +1512,7 @@ def get_exec_summary_report(request, course_id): """ course_key = CourseKey.from_string(course_id) report_type = _('executive summary') - lms.djangoapps.instructor_task.api.submit_executive_summary_report(request, course_key) + task_api.submit_executive_summary_report(request, course_key) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) return JsonResponse({"status": success_status}) @@ -1521,7 +1530,7 @@ def get_course_survey_results(request, course_id): """ course_key = CourseKey.from_string(course_id) report_type = _('survey') - lms.djangoapps.instructor_task.api.submit_course_survey_report(request, course_key) + task_api.submit_course_survey_report(request, course_key) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) return JsonResponse({"status": success_status}) @@ -1539,7 +1548,7 @@ def get_proctored_exam_results(request, course_id): """ course_key = CourseKey.from_string(course_id) report_type = _('proctored exam results') - lms.djangoapps.instructor_task.api.submit_proctored_exam_results_report(request, course_key) + task_api.submit_proctored_exam_results_report(request, course_key) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) return JsonResponse({"status": success_status}) @@ -1916,7 +1925,8 @@ def get_anon_ids(request, course_id): # pylint: disable=unused-argument 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, save=False), anonymous_id_for_user(s, course_id, save=False)] for s in students] + rows = [[s.id, unique_id_for_user(s, save=False), anonymous_id_for_user(s, course_id, save=False)] + for s in students] return csv_response(text_type(course_id).replace('/', '-') + '-anon-ids.csv', header, rows) @@ -2085,7 +2095,7 @@ def reset_student_attempts(request, course_id): return HttpResponse(error_msg, status=500) response_payload['student'] = student_identifier elif all_students: - lms.djangoapps.instructor_task.api.submit_reset_problem_attempts_for_all_students(request, module_state_key) + task_api.submit_reset_problem_attempts_for_all_students(request, module_state_key) response_payload['task'] = TASK_SUBMISSION_OK response_payload['student'] = 'All Students' else: @@ -2151,13 +2161,13 @@ def reset_student_attempts_for_entrance_exam(request, course_id): try: entrance_exam_key = UsageKey.from_string(course.entrance_exam_id).map_into_course(course_id) if delete_module: - lms.djangoapps.instructor_task.api.submit_delete_entrance_exam_state_for_student( + task_api.submit_delete_entrance_exam_state_for_student( request, entrance_exam_key, student ) else: - lms.djangoapps.instructor_task.api.submit_reset_problem_attempts_in_entrance_exam( + task_api.submit_reset_problem_attempts_in_entrance_exam( request, entrance_exam_key, student @@ -2220,7 +2230,7 @@ def rescore_problem(request, course_id): if student: response_payload['student'] = student_identifier try: - lms.djangoapps.instructor_task.api.submit_rescore_problem_for_student( + task_api.submit_rescore_problem_for_student( request, module_state_key, student, @@ -2231,7 +2241,7 @@ def rescore_problem(request, course_id): elif all_students: try: - lms.djangoapps.instructor_task.api.submit_rescore_problem_for_all_students( + task_api.submit_rescore_problem_for_all_students( request, module_state_key, only_if_higher, @@ -2286,7 +2296,7 @@ def override_problem_score(request, course_id): 'student': student_identifier } try: - submit_override_score( + task_api.submit_override_score( request, usage_key, student, @@ -2353,7 +2363,7 @@ def rescore_entrance_exam(request, course_id): else: response_payload['student'] = _("All Students") - lms.djangoapps.instructor_task.api.submit_rescore_entrance_exam_for_student( + task_api.submit_rescore_entrance_exam_for_student( request, entrance_exam_key, student, only_if_higher, ) response_payload['task'] = TASK_SUBMISSION_OK @@ -2371,7 +2381,7 @@ def list_background_email_tasks(request, course_id): # pylint: disable=unused-a course_id = CourseKey.from_string(course_id) task_type = 'bulk_course_email' # Specifying for the history of a single task type - tasks = lms.djangoapps.instructor_task.api.get_instructor_task_history( + tasks = task_api.get_instructor_task_history( course_id, task_type=task_type ) @@ -2393,7 +2403,7 @@ def list_email_content(request, course_id): # pylint: disable=unused-argument course_id = CourseKey.from_string(course_id) task_type = 'bulk_course_email' # First get tasks list of bulk emails sent - emails = lms.djangoapps.instructor_task.api.get_instructor_task_history(course_id, task_type=task_type) + emails = task_api.get_instructor_task_history(course_id, task_type=task_type) response_payload = { 'emails': map(extract_email_features, emails), @@ -2433,13 +2443,13 @@ def list_instructor_tasks(request, course_id): return HttpResponseBadRequest() if student: # Specifying for a single student's history on this problem - tasks = lms.djangoapps.instructor_task.api.get_instructor_task_history(course_id, module_state_key, student) + tasks = task_api.get_instructor_task_history(course_id, module_state_key, student) else: # Specifying for single problem's history - tasks = lms.djangoapps.instructor_task.api.get_instructor_task_history(course_id, module_state_key) + tasks = task_api.get_instructor_task_history(course_id, module_state_key) else: # If no problem or student, just get currently running tasks - tasks = lms.djangoapps.instructor_task.api.get_running_instructor_tasks(course_id) + tasks = task_api.get_running_instructor_tasks(course_id) response_payload = { 'tasks': map(extract_task_features, tasks), @@ -2471,14 +2481,14 @@ def list_entrance_exam_instructor_tasks(request, course_id): return HttpResponseBadRequest(_("Course has no valid entrance exam section.")) if student: # Specifying for a single student's entrance exam history - tasks = lms.djangoapps.instructor_task.api.get_entrance_exam_instructor_task_history( + tasks = task_api.get_entrance_exam_instructor_task_history( course_id, entrance_exam_key, student ) else: # Specifying for all student's entrance exam history - tasks = lms.djangoapps.instructor_task.api.get_entrance_exam_instructor_task_history( + tasks = task_api.get_entrance_exam_instructor_task_history( course_id, entrance_exam_key ) @@ -2546,7 +2556,7 @@ def export_ora2_data(request, course_id): """ course_key = CourseKey.from_string(course_id) report_type = _('ORA data') - lms.djangoapps.instructor_task.api.submit_export_ora2_data(request, course_key) + task_api.submit_export_ora2_data(request, course_key) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) return JsonResponse({"status": success_status}) @@ -2564,7 +2574,7 @@ def calculate_grades_csv(request, course_id): """ report_type = _('grade') course_key = CourseKey.from_string(course_id) - lms.djangoapps.instructor_task.api.submit_calculate_grades_csv(request, course_key) + task_api.submit_calculate_grades_csv(request, course_key) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) return JsonResponse({"status": success_status}) @@ -2586,7 +2596,7 @@ def problem_grade_report(request, course_id): """ course_key = CourseKey.from_string(course_id) report_type = _('problem grade') - lms.djangoapps.instructor_task.api.submit_problem_grade_report(request, course_key) + task_api.submit_problem_grade_report(request, course_key) success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) return JsonResponse({"status": success_status}) @@ -2730,7 +2740,7 @@ def send_email(request, course_id): return HttpResponseBadRequest(repr(err)) # Submit the task, so that the correct InstructorTask object gets created (for monitoring purposes) - lms.djangoapps.instructor_task.api.submit_bulk_course_email(request, course_id, email.id) + task_api.submit_bulk_course_email(request, course_id, email.id) response_payload = { 'course_id': text_type(course_id), @@ -2846,7 +2856,7 @@ def change_due_date(request, course_id): student = require_student_from_identifier(request.POST.get('student')) unit = find_unit(course, request.POST.get('url')) due_date = parse_datetime(request.POST.get('due_datetime')) - set_due_date_extension(course, unit, student, due_date) + set_due_date_extension(course, unit, student, due_date, request.user) return JsonResponse(_( u'Successfully changed due date for student {0} for {1} ' @@ -2867,7 +2877,7 @@ def reset_due_date(request, course_id): course = get_course_by_id(CourseKey.from_string(course_id)) student = require_student_from_identifier(request.POST.get('student')) unit = find_unit(course, request.POST.get('url')) - set_due_date_extension(course, unit, student, None) + set_due_date_extension(course, unit, student, None, request.user) if not getattr(unit, "due", None): # It's possible the normal due date was deleted after an extension was granted: return JsonResponse( @@ -3020,7 +3030,7 @@ def start_certificate_generation(request, course_id): Start generating certificates for all students enrolled in given course. """ course_key = CourseKey.from_string(course_id) - task = lms.djangoapps.instructor_task.api.generate_certificates_for_students(request, course_key) + task = task_api.generate_certificates_for_students(request, course_key) message = _('Certificate generation task for all students of this course has been started. ' 'You can view the status of the generation task in the "Pending Tasks" section.') response_payload = { @@ -3064,7 +3074,7 @@ def start_certificate_regeneration(request, course_id): status=400 ) - lms.djangoapps.instructor_task.api.regenerate_certificates(request, course_key, certificates_statuses) + task_api.regenerate_certificates(request, course_key, certificates_statuses) response_payload = { 'message': _('Certificate regeneration task has been started. ' 'You can view the status of the generation task in the "Pending Tasks" section.'), @@ -3121,7 +3131,7 @@ def add_certificate_exception(course_key, student, certificate_exception): :param certificate_exception: A dict object containing certificate exception info. :return: CertificateWhitelist item in dict format containing certificate exception info. """ - if len(CertificateWhitelist.get_certificate_white_list(course_key, student)) > 0: + if CertificateWhitelist.get_certificate_white_list(course_key, student): raise ValueError( _(u"Student (username/email={user}) already in certificate exception list.").format(user=student.username) ) @@ -3283,7 +3293,7 @@ def generate_certificate_exceptions(request, course_id, generate_for=None): status=400 ) - lms.djangoapps.instructor_task.api.generate_certificates_for_students(request, course_key, student_set=students) + task_api.generate_certificates_for_students(request, course_key, student_set=students) response_payload = { 'success': True, 'message': _('Certificate generation started for white listed students.'), @@ -3343,7 +3353,7 @@ def generate_bulk_certificate_exceptions(request, course_id): # verify that we have exactly two column in every row either email or username and notes but allow for # blank lines if len(student) != 2: - if len(student) > 0: + if student: build_row_errors('data_format_error', student[user_index], row_num) log.info(u'invalid data/format in csv row# %s', row_num) continue @@ -3355,7 +3365,7 @@ def generate_bulk_certificate_exceptions(request, course_id): build_row_errors('user_not_exist', user, row_num) log.info(u'student %s does not exist', user) else: - if len(CertificateWhitelist.get_certificate_white_list(course_key, user)) > 0: + if CertificateWhitelist.get_certificate_white_list(course_key, user): build_row_errors('user_already_white_listed', user, row_num) log.warning(u'student %s already exist.', user.username) @@ -3433,10 +3443,10 @@ def invalidate_certificate(request, generated_certificate, certificate_invalidat :param certificate_invalidation_data: dict object containing data for CertificateInvalidation. :return: dict object containing updated certificate invalidation data. """ - if len(CertificateInvalidation.get_certificate_invalidations( + if CertificateInvalidation.get_certificate_invalidations( generated_certificate.course_id, generated_certificate.user, - )) > 0: + ): raise ValueError( _(u"Certificate of {user} has already been invalidated. Please check your spelling and retry.").format( user=generated_certificate.user.username, @@ -3493,7 +3503,7 @@ def re_validate_certificate(request, course_key, generated_certificate): # We need to generate certificate only for a single student here student = certificate_invalidation.generated_certificate.user - lms.djangoapps.instructor_task.api.generate_certificates_for_students( + task_api.generate_certificates_for_students( request, course_key, student_set="specific_student", specific_student_id=student.id ) @@ -3542,4 +3552,4 @@ def _create_error_response(request, msg): Creates the appropriate error response for the current request, in JSON form. """ - return JsonResponse({"error": _(msg)}, 400) + return JsonResponse({"error": msg}, 400) diff --git a/lms/djangoapps/instructor/views/gradebook_api.py b/lms/djangoapps/instructor/views/gradebook_api.py index 922d351d08..f4603e59d4 100644 --- a/lms/djangoapps/instructor/views/gradebook_api.py +++ b/lms/djangoapps/instructor/views/gradebook_api.py @@ -5,8 +5,8 @@ which is currently use by ccx and instructor apps. import math from django.contrib.auth.models import User -from django.urls import reverse from django.db import transaction +from django.urls import reverse from django.views.decorators.cache import cache_control from opaque_keys.edx.keys import CourseKey diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index c60fc0167f..8e9de402c2 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -10,8 +10,8 @@ from urlparse import urljoin import pytz from django.conf import settings from django.contrib.auth.decorators import login_required -from django.urls import reverse from django.http import Http404, HttpResponseServerError +from django.urls import reverse from django.utils.html import escape from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_noop @@ -26,6 +26,13 @@ from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from bulk_email.models import BulkEmailFlag +from class_dashboard.dashboard_data import get_array_section_has_problem, get_section_display_name +from course_modes.models import CourseMode, CourseModesArchive +from courseware.access import has_access +from courseware.courses import get_course_by_id, get_studio_url +from django_comment_client.utils import available_division_schemes, has_forum_access +from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, CourseDiscussionSettings +from edxmako.shortcuts import render_to_response from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, @@ -35,15 +42,8 @@ from lms.djangoapps.certificates.models import ( CertificateWhitelist, GeneratedCertificate ) -from class_dashboard.dashboard_data import get_array_section_has_problem, get_section_display_name -from course_modes.models import CourseMode, CourseModesArchive -from courseware.access import has_access -from courseware.courses import get_course_by_id, get_studio_url -from django_comment_client.utils import available_division_schemes, has_forum_access -from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, CourseDiscussionSettings -from edxmako.shortcuts import render_to_response from lms.djangoapps.courseware.module_render import get_module_by_usage_id -from lms.djangoapps.grades.config.waffle import waffle_flags, WRITABLE_GRADEBOOK +from lms.djangoapps.grades.config.waffle import WRITABLE_GRADEBOOK, waffle_flags from openedx.core.djangoapps.course_groups.cohorts import DEFAULT_COHORT_NAME, get_course_cohorts, is_course_cohorted from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse @@ -52,7 +52,7 @@ from openedx.core.lib.url_utils import quote_slashes from openedx.core.lib.xblock_utils import wrap_xblock from shoppingcart.models import Coupon, CourseRegCodeItem, PaidCourseRegistration from student.models import CourseEnrollment -from student.roles import CourseFinanceAdminRole, CourseSalesAdminRole, CourseStaffRole, CourseInstructorRole +from student.roles import CourseFinanceAdminRole, CourseInstructorRole, CourseSalesAdminRole, CourseStaffRole from util.json_request import JsonResponse from xmodule.html_module import HtmlDescriptor from xmodule.modulestore.django import modulestore @@ -159,7 +159,7 @@ def instructor_dashboard_2(request, course_id): unicode(course_key), len(paid_modes) ) - if settings.FEATURES.get('INDIVIDUAL_DUE_DATES') and access['instructor']: + if access['instructor']: sections.insert(3, _section_extensions(course)) # Gate access to course email by feature flag & by course-specific authorization diff --git a/lms/djangoapps/instructor/views/tools.py b/lms/djangoapps/instructor/views/tools.py index e9109ff4a6..80e52e9b1a 100644 --- a/lms/djangoapps/instructor/views/tools.py +++ b/lms/djangoapps/instructor/views/tools.py @@ -2,26 +2,18 @@ Tools for the instructor dashboard """ import json +import operator import dateutil from django.contrib.auth.models import User from django.http import HttpResponseBadRequest -from pytz import UTC from django.utils.translation import ugettext as _ from opaque_keys.edx.keys import UsageKey -from six import text_type, string_types +from pytz import UTC +from six import string_types, text_type -from courseware.models import StudentFieldOverride -from lms.djangoapps.courseware.field_overrides import disable_overrides -from lms.djangoapps.courseware.student_field_overrides import ( - clear_override_for_user, - get_override_for_user, - override_field_for_user, -) +from edx_when import api from student.models import get_user_by_username_or_email -from xmodule.fields import Date - -DATE_FIELD = Date() class DashboardError(Exception): @@ -158,29 +150,20 @@ def title_or_url(node): return title -def set_due_date_extension(course, unit, student, due_date): +def set_due_date_extension(course, unit, student, due_date, actor=None): """ Sets a due date extension. Raises DashboardError if the unit or extended due date is invalid. """ if due_date: - # Check that the new due date is valid: - with disable_overrides(): - original_due_date = getattr(unit, 'due', None) - - if not original_due_date: + try: + api.set_date_for_block(course.id, unit.location, 'due', due_date, user=student, reason=None, actor=actor) + except api.MissingDateError: raise DashboardError(_(u"Unit {0} has no due date to extend.").format(unit.location)) - if due_date < original_due_date: + except api.InvalidDateError: raise DashboardError(_("An extended due date must be later than the original due date.")) - - override_field_for_user(student, unit, 'due', due_date) - else: - # We are deleting a due date extension. Check that it exists: - if not get_override_for_user(student, unit, 'due'): - raise DashboardError(_("No due date extension is set for that student and unit.")) - - clear_override_for_user(student, unit, 'due') + api.set_date_for_block(course.id, unit.location, 'due', None, user=student, reason=None, actor=actor) def dump_module_extensions(course, unit): @@ -188,20 +171,12 @@ def dump_module_extensions(course, unit): Dumps data about students with due date extensions for a particular module, specified by 'url', in a particular course. """ - data = [] header = [_("Username"), _("Full Name"), _("Extended Due Date")] - query = StudentFieldOverride.objects.filter( - course_id=course.id, - location=unit.location, - field='due') - for override in query: - due = DATE_FIELD.from_json(json.loads(override.value)) - due = due.strftime(u"%Y-%m-%d %H:%M") - fullname = override.student.profile.name - data.append(dict(zip( - header, - (override.student.username, fullname, due)))) - data.sort(key=lambda x: x[header[0]]) + data = [] + for username, fullname, due_date in api.get_overrides_for_block(course.id, unit.location): + due_date = due_date.strftime(u'%Y-%m-%d %H:%M') + data.append(dict(zip(header, (username, fullname, due_date)))) + data.sort(key=operator.itemgetter(_("Username"))) return { "header": header, "title": _(u"Users with due date extensions for {0}").format( @@ -219,18 +194,16 @@ def dump_student_extensions(course, student): header = [_("Unit"), _("Extended Due Date")] units = get_units_with_due_date(course) units = {u.location: u for u in units} - query = StudentFieldOverride.objects.filter( - course_id=course.id, - student=student, - field='due') + query = api.get_overrides_for_user(course.id, student) for override in query: - location = override.location.replace(course_key=course.id) + location = override['location'].replace(course_key=course.id) if location not in units: continue - due = DATE_FIELD.from_json(json.loads(override.value)) + due = override['actual_date'] due = due.strftime(u"%Y-%m-%d %H:%M") title = title_or_url(units[location]) data.append(dict(zip(header, (title, due)))) + data.sort(key=operator.itemgetter(_("Unit"))) return { "header": header, "title": _(u"Due date extensions for {0} {1} ({2})").format( diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index ba9730a022..e30b359900 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -213,7 +213,7 @@ class TestCourseHomePage(CourseHomePageTestCase): # Fetch the view and verify the query counts # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(89, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(91, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index 41f5f8932f..809640d55e 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -130,7 +130,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(50, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(52, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url) diff --git a/openedx/tests/settings.py b/openedx/tests/settings.py index 7403435b4f..1fe06b2d7d 100644 --- a/openedx/tests/settings.py +++ b/openedx/tests/settings.py @@ -88,6 +88,7 @@ INSTALLED_APPS = ( 'milestones', 'celery_utils', 'waffle', + 'edx_when', # Django 1.11 demands to have imported models supported by installed apps. 'completion', diff --git a/requirements/edx/base.in b/requirements/edx/base.in index 139674ea5f..aa994d0ac9 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -85,6 +85,7 @@ edx-rest-api-client edx-search edx-submissions edx-user-state-client +edx-when edxval enum34==1.1.6 # Backport of Enum from Python 3.4+ event-tracking diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 020b4af75b..25c478c1fa 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -125,6 +125,7 @@ edx-rest-api-client==1.9.2 edx-search==1.2.2 edx-submissions==2.1.1 edx-user-state-client==1.0.4 +edx-when==0.1.1 edxval==1.1.25 elasticsearch==1.9.0 # via edx-search enum34==1.1.6 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 83b3cafe6d..1155d0594a 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -148,6 +148,7 @@ edx-search==1.2.2 edx-sphinx-theme==1.4.0 edx-submissions==2.1.1 edx-user-state-client==1.0.4 +edx-when==0.1.1 edxval==1.1.25 elasticsearch==1.9.0 entrypoints==0.3 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 1de98e45e4..d48d833889 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -143,6 +143,7 @@ edx-rest-api-client==1.9.2 edx-search==1.2.2 edx-submissions==2.1.1 edx-user-state-client==1.0.4 +edx-when==0.1.1 edxval==1.1.25 elasticsearch==1.9.0 entrypoints==0.3 # via flake8