From 519384edca745ea90ddf4ed969fb5b7c059dd47d Mon Sep 17 00:00:00 2001 From: Usama Sadiq Date: Thu, 4 Mar 2021 16:00:19 +0500 Subject: [PATCH] refactor: ran pyupgrade on lms/djangoapps (#26733) ran pyupgrade on bulk_enroll & ccx apps. --- lms/djangoapps/bulk_enroll/serializers.py | 5 +- .../bulk_enroll/tests/test_views.py | 20 ++-- lms/djangoapps/bulk_enroll/views.py | 2 +- lms/djangoapps/ccx/api/v0/paginators.py | 2 +- lms/djangoapps/ccx/api/v0/serializers.py | 5 +- lms/djangoapps/ccx/api/v0/tests/test_views.py | 81 +++++++------ lms/djangoapps/ccx/api/v0/urls.py | 2 +- lms/djangoapps/ccx/api/v0/views.py | 29 +++-- lms/djangoapps/ccx/migrations/0001_initial.py | 7 +- .../0002_customcourseforedx_structure_json.py | 5 +- .../0003_add_master_course_staff_in_ccx.py | 7 +- .../0004_seed_forum_roles_in_ccx_courses.py | 5 +- .../0005_change_ccx_coach_to_staff.py | 15 +-- .../0006_set_display_name_as_override.py | 1 - lms/djangoapps/ccx/models.py | 11 +- lms/djangoapps/ccx/modulestore.py | 6 +- lms/djangoapps/ccx/overrides.py | 2 +- lms/djangoapps/ccx/permissions.py | 1 + lms/djangoapps/ccx/tasks.py | 9 +- lms/djangoapps/ccx/tests/factories.py | 6 +- .../ccx/tests/test_ccx_modulestore.py | 8 +- .../tests/test_field_override_performance.py | 19 ++- lms/djangoapps/ccx/tests/test_models.py | 2 +- lms/djangoapps/ccx/tests/test_overrides.py | 14 +-- lms/djangoapps/ccx/tests/test_tasks.py | 11 +- lms/djangoapps/ccx/tests/test_utils.py | 16 +-- lms/djangoapps/ccx/tests/test_views.py | 113 +++++++++--------- lms/djangoapps/ccx/tests/utils.py | 14 +-- lms/djangoapps/ccx/utils.py | 31 +++-- lms/djangoapps/ccx/views.py | 40 +++---- 30 files changed, 230 insertions(+), 259 deletions(-) diff --git a/lms/djangoapps/bulk_enroll/serializers.py b/lms/djangoapps/bulk_enroll/serializers.py index 5e5e29e558..ec63ae70c2 100644 --- a/lms/djangoapps/bulk_enroll/serializers.py +++ b/lms/djangoapps/bulk_enroll/serializers.py @@ -6,7 +6,6 @@ Serializers for Bulk Enrollment. from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from rest_framework import serializers -from six.moves import zip from openedx.core.djangoapps.course_groups.cohorts import is_cohort_exists @@ -47,7 +46,7 @@ class BulkEnrollmentSerializer(serializers.Serializer): # lint-amnesty, pylint: try: CourseKey.from_string(course) except InvalidKeyError: - raise serializers.ValidationError(u"Course key not valid: {}".format(course)) # lint-amnesty, pylint: disable=raise-missing-from + raise serializers.ValidationError(f"Course key not valid: {course}") # lint-amnesty, pylint: disable=raise-missing-from return value def validate(self, attrs): @@ -63,7 +62,7 @@ class BulkEnrollmentSerializer(serializers.Serializer): # lint-amnesty, pylint: for course_id, cohort_name in zip(attrs['courses'], attrs['cohorts']): if not is_cohort_exists(course_key=CourseKey.from_string(course_id), name=cohort_name): - raise serializers.ValidationError(u"cohort {cohort_name} not found in course {course_id}.".format( + raise serializers.ValidationError("cohort {cohort_name} not found in course {course_id}.".format( cohort_name=cohort_name, course_id=course_id) ) diff --git a/lms/djangoapps/bulk_enroll/tests/test_views.py b/lms/djangoapps/bulk_enroll/tests/test_views.py index 603f489eca..1ce5b1775b 100644 --- a/lms/djangoapps/bulk_enroll/tests/test_views.py +++ b/lms/djangoapps/bulk_enroll/tests/test_views.py @@ -6,7 +6,6 @@ Tests for the Bulk Enrollment views. import json import ddt -import six from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core import mail @@ -15,14 +14,19 @@ from django.urls import reverse from opaque_keys.edx.keys import CourseKey from rest_framework.test import APIRequestFactory, APITestCase, force_authenticate +from common.djangoapps.student.models import ( # lint-amnesty, pylint: disable=line-too-long + ENROLLED_TO_UNENROLLED, + UNENROLLED_TO_ENROLLED, + CourseEnrollment, + ManualEnrollmentAudit +) +from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.bulk_enroll.serializers import BulkEnrollmentSerializer from lms.djangoapps.bulk_enroll.views import BulkEnrollView from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts from openedx.core.djangoapps.site_configuration.helpers import get_value as get_site_value -from common.djangoapps.student.models import ENROLLED_TO_UNENROLLED, UNENROLLED_TO_ENROLLED, CourseEnrollment, ManualEnrollmentAudit # lint-amnesty, pylint: disable=line-too-long -from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -40,7 +44,7 @@ class BulkEnrollmentTest(ModuleStoreTestCase, LoginEnrollmentTestCase, APITestCa def setUp(self): """ Create a course and user, then log in. """ - super(BulkEnrollmentTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.view = BulkEnrollView.as_view() self.request_factory = APIRequestFactory() @@ -54,7 +58,7 @@ class BulkEnrollmentTest(ModuleStoreTestCase, LoginEnrollmentTestCase, APITestCa ) self.course = CourseFactory.create() - self.course_key = six.text_type(self.course.id) + self.course_key = str(self.course.id) self.enrolled_student = UserFactory(username='EnrolledStudent', first_name='Enrolled', last_name='Student') CourseEnrollment.enroll( self.enrolled_student, @@ -68,8 +72,8 @@ class BulkEnrollmentTest(ModuleStoreTestCase, LoginEnrollmentTestCase, APITestCa 'SITE_NAME', settings.SITE_NAME ) - self.about_path = '/courses/{}/about'.format(self.course.id) - self.course_path = '/courses/{}/'.format(self.course.id) + self.about_path = f'/courses/{self.course.id}/about' + self.course_path = f'/courses/{self.course.id}/' def request_bulk_enroll(self, data=None, use_json=False, **extra): """ Make an authenticated request to the bulk enrollment API. """ @@ -368,7 +372,7 @@ class BulkEnrollmentTest(ModuleStoreTestCase, LoginEnrollmentTestCase, APITestCa }) self.assertContains( response, - u'cohort {cohort_name} not found in course {course_id}.'.format( + 'cohort {cohort_name} not found in course {course_id}.'.format( cohort_name='cohort1', course_id=self.course_key ), status_code=400, diff --git a/lms/djangoapps/bulk_enroll/views.py b/lms/djangoapps/bulk_enroll/views.py index 6ce792d38e..6db97c6802 100644 --- a/lms/djangoapps/bulk_enroll/views.py +++ b/lms/djangoapps/bulk_enroll/views.py @@ -13,6 +13,7 @@ from rest_framework.response import Response from rest_framework.views import APIView from six.moves import zip_longest +from common.djangoapps.util.disable_rate_limit import can_disable_rate_limit from lms.djangoapps.bulk_enroll.serializers import BulkEnrollmentSerializer from lms.djangoapps.instructor.views.api import students_update_enrollment from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, get_cohort_by_name @@ -20,7 +21,6 @@ from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.djangoapps.enrollments.views import EnrollmentUserThrottle from openedx.core.lib.api.authentication import BearerAuthentication from openedx.core.lib.api.permissions import IsStaff -from common.djangoapps.util.disable_rate_limit import can_disable_rate_limit @can_disable_rate_limit diff --git a/lms/djangoapps/ccx/api/v0/paginators.py b/lms/djangoapps/ccx/api/v0/paginators.py index 241d510cc4..f8bfffd50c 100644 --- a/lms/djangoapps/ccx/api/v0/paginators.py +++ b/lms/djangoapps/ccx/api/v0/paginators.py @@ -14,7 +14,7 @@ class CCXAPIPagination(DefaultPagination): """ Annotate the response with pagination information. """ - response = super(CCXAPIPagination, self).get_paginated_response(data) # lint-amnesty, pylint: disable=super-with-arguments + response = super().get_paginated_response(data) # Add the current page to the response. response.data["current_page"] = self.page.number diff --git a/lms/djangoapps/ccx/api/v0/serializers.py b/lms/djangoapps/ccx/api/v0/serializers.py index 10e7581fb5..c94a458f49 100644 --- a/lms/djangoapps/ccx/api/v0/serializers.py +++ b/lms/djangoapps/ccx/api/v0/serializers.py @@ -1,7 +1,6 @@ """ CCX API v0 Serializers. """ -import six from ccx_keys.locator import CCXLocator from rest_framework import serializers @@ -21,7 +20,7 @@ class CCXCourseSerializer(serializers.ModelSerializer): max_students_allowed = serializers.IntegerField(source='max_student_enrollments_allowed') course_modules = serializers.SerializerMethodField() - class Meta(object): + class Meta: model = CustomCourseForEdX fields = ( "ccx_course_id", @@ -45,7 +44,7 @@ class CCXCourseSerializer(serializers.ModelSerializer): """ Getter for the CCX Course ID """ - return six.text_type(CCXLocator.from_course_locator(obj.course.id, obj.id)) + return str(CCXLocator.from_course_locator(obj.course.id, obj.id)) @staticmethod def get_course_modules(obj): diff --git a/lms/djangoapps/ccx/api/v0/tests/test_views.py b/lms/djangoapps/ccx/api/v0/tests/test_views.py index 700fcfdc05..e0528eabe5 100644 --- a/lms/djangoapps/ccx/api/v0/tests/test_views.py +++ b/lms/djangoapps/ccx/api/v0/tests/test_views.py @@ -7,10 +7,10 @@ import json import math import string from datetime import timedelta +from unittest import mock +import urllib import pytest import ddt -import mock -import six from ccx_keys.locator import CCXLocator from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user @@ -20,8 +20,10 @@ from oauth2_provider import models as dot_models from opaque_keys.edx.keys import CourseKey from rest_framework import status from rest_framework.test import APITestCase -from six.moves import range, zip +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole +from common.djangoapps.student.tests.factories import AdminFactory, UserFactory from lms.djangoapps.ccx.api.v0 import views from lms.djangoapps.ccx.models import CcxFieldOverride, CustomCourseForEdX from lms.djangoapps.ccx.overrides import override_field_for_ccx @@ -30,9 +32,6 @@ from lms.djangoapps.ccx.utils import ccx_course as ccx_course_cm from lms.djangoapps.courseware import courses from lms.djangoapps.instructor.access import allow_access, list_with_level from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params -from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole -from common.djangoapps.student.tests.factories import AdminFactory, UserFactory USER_PASSWORD = 'test' @@ -43,16 +42,16 @@ class CcxRestApiTest(CcxTestCase, APITestCase): """ @classmethod def setUpClass(cls): - super(CcxRestApiTest, cls).setUpClass() + super().setUpClass() def setUp(self): """ Set up tests """ - super(CcxRestApiTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() # add some info about the course for easy access self.master_course_key = self.course.location.course_key - self.master_course_key_str = six.text_type(self.master_course_key) + self.master_course_key_str = str(self.master_course_key) # OAUTH2 setup # create a specific user for the application self.app_user = app_user = UserFactory( @@ -96,7 +95,7 @@ class CcxRestApiTest(CcxTestCase, APITestCase): token='16MGyP3OaQYHmpT1lK7Q6MMNAZsjwF' ) - auth_header_oauth2_provider = u"Bearer {0}".format(auth_oauth2_provider) + auth_header_oauth2_provider = f"Bearer {auth_oauth2_provider}" return auth_header_oauth2_provider @@ -118,7 +117,7 @@ class CcxRestApiTest(CcxTestCase, APITestCase): assert 'field_errors' in resp_obj.data # restructure the error dictionary for a easier comparison resp_dict_error = {} - for field_name, error_dict in six.iteritems(resp_obj.data['field_errors']): + for field_name, error_dict in resp_obj.data['field_errors'].items(): resp_dict_error[field_name] = error_dict.get('error_code', '') assert expected_field_errors == resp_dict_error @@ -132,17 +131,17 @@ class CcxListTest(CcxRestApiTest): @classmethod def setUpClass(cls): - super(CcxListTest, cls).setUpClass() + super().setUpClass() def setUp(self): """ Set up tests """ - super(CcxListTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.list_url = reverse('ccx_api:v0:ccx:list') - self.list_url_master_course = six.moves.urllib.parse.urljoin( + self.list_url_master_course = urllib.parse.urljoin( self.list_url, - '?master_course_id={0}'.format(six.moves.urllib.parse.quote_plus(self.master_course_key_str)) + '?master_course_id={}'.format(urllib.parse.quote_plus(self.master_course_key_str)) ) def test_authorization(self): @@ -251,18 +250,18 @@ class CcxListTest(CcxRestApiTest): resp = self.client.get(self.list_url, {}, HTTP_AUTHORIZATION=self.auth) self.expect_error(status.HTTP_400_BAD_REQUEST, 'master_course_id_not_provided', resp) - base_url = six.moves.urllib.parse.urljoin(self.list_url, '?master_course_id=') + base_url = urllib.parse.urljoin(self.list_url, '?master_course_id=') # case with empty master_course_id resp = self.client.get(base_url, {}, HTTP_AUTHORIZATION=self.auth) self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_id_not_valid', resp) # case with invalid master_course_id - url = '{0}invalid_master_course_str'.format(base_url) + url = f'{base_url}invalid_master_course_str' resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_id_not_valid', resp) # case with inexistent master_course_id - url = '{0}course-v1%3Aorg_foo.0%2Bcourse_bar_0%2BRun_0'.format(base_url) + url = f'{base_url}course-v1%3Aorg_foo.0%2Bcourse_bar_0%2BRun_0' resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) self.expect_error(status.HTTP_404_NOT_FOUND, 'course_id_does_not_exist', resp) @@ -298,13 +297,13 @@ class CcxListTest(CcxRestApiTest): all_ccx = CustomCourseForEdX.objects.all() all_ccx = all_ccx.order_by('id') assert len(all_ccx) == num_ccx - title_str = u'Title CCX {0}' + title_str = 'Title CCX {0}' for num, ccx in enumerate(all_ccx): ccx.display_name = title_str.format(string.ascii_lowercase[-(num + 1)]) ccx.save() # sort by display name - url = '{0}&order_by=display_name'.format(self.list_url_master_course) + url = f'{self.list_url_master_course}&order_by=display_name' resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_200_OK assert len(resp.data['results']) == num_ccx @@ -313,7 +312,7 @@ class CcxListTest(CcxRestApiTest): assert title_str.format(string.ascii_lowercase[(- (num_ccx - num))]) == ccx['display_name'] # add sort order desc - url = '{0}&order_by=display_name&sort_order=desc'.format(self.list_url_master_course) + url = f'{self.list_url_master_course}&order_by=display_name&sort_order=desc' resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) # the only thing I can check is that the display name is in alphabetically reversed order # in the same way when the field has been updated above, so with the id asc @@ -341,7 +340,7 @@ class CcxListTest(CcxRestApiTest): assert resp.data['previous'] is None # get a page in the middle - url = '{0}&page=24'.format(self.list_url_master_course) + url = f'{self.list_url_master_course}&page=24' resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_200_OK assert resp.data['count'] == num_ccx @@ -352,7 +351,7 @@ class CcxListTest(CcxRestApiTest): assert resp.data['previous'] is not None # get last page - url = '{0}&page={1}'.format(self.list_url_master_course, num_pages) + url = f'{self.list_url_master_course}&page={num_pages}' resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_200_OK assert resp.data['count'] == num_ccx @@ -363,7 +362,7 @@ class CcxListTest(CcxRestApiTest): assert resp.data['previous'] is not None # last page + 1 - url = '{0}&page={1}'.format(self.list_url_master_course, num_pages + 1) + url = '{}&page={}'.format(self.list_url_master_course, num_pages + 1) resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_404_NOT_FOUND @@ -583,13 +582,13 @@ class CcxListTest(CcxRestApiTest): resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_201_CREATED # check if the response has at least the same data of the request - for key, val in six.iteritems(data): + for key, val in data.items(): assert resp.data.get(key) == val assert 'ccx_course_id' in resp.data # check that the new CCX actually exists course_key = CourseKey.from_string(resp.data.get('ccx_course_id')) ccx_course = CustomCourseForEdX.objects.get(pk=course_key.ccx) - assert six.text_type(CCXLocator.from_course_locator(ccx_course.course.id, ccx_course.id)) ==\ + assert str(CCXLocator.from_course_locator(ccx_course.course.id, ccx_course.id)) ==\ resp.data.get('ccx_course_id') # check that the coach user has coach role on the master course coach_role_on_master_course = CourseCcxCoachRole(self.master_course_key) @@ -692,12 +691,12 @@ class CcxDetailTest(CcxRestApiTest): """ Set up tests """ - super(CcxDetailTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.make_coach() # create a ccx self.ccx = self.make_ccx(max_students_allowed=123) self.ccx_key = CCXLocator.from_course_locator(self.ccx.course.id, self.ccx.id) - self.ccx_key_str = six.text_type(self.ccx_key) + self.ccx_key_str = str(self.ccx_key) self.detail_url = reverse('ccx_api:v0:ccx:detail', kwargs={'ccx_course_id': self.ccx_key_str}) def make_ccx(self, max_students_allowed=200): @@ -705,7 +704,7 @@ class CcxDetailTest(CcxRestApiTest): Overridden method to replicate (part of) the actual creation of ccx courses """ - ccx = super(CcxDetailTest, self).make_ccx(max_students_allowed=max_students_allowed) # lint-amnesty, pylint: disable=super-with-arguments + ccx = super().make_ccx(max_students_allowed=max_students_allowed) ccx.structure_json = json.dumps(self.master_course_chapters) ccx.save() @@ -824,7 +823,7 @@ class CcxDetailTest(CcxRestApiTest): that only an URL with a valid course id string can reach the detail view. """ # get the base url from the valid one to build invalid urls - base_url = '{0}/'.format(self.detail_url.rsplit('/', 1)[0]) + base_url = '{}/'.format(self.detail_url.rsplit('/', 1)[0]) # this url should be the same of the ccx list view resolver = resolve(base_url) assert views.CCXListView.__name__ == resolver.func.__name__ @@ -832,13 +831,13 @@ class CcxDetailTest(CcxRestApiTest): # invalid urls for invalid_ccx_id in ('foo', 'ccx-v1:org.0', 'ccx-v1:org.0+course_0'): with pytest.raises(Resolver404): - resolve('{0}{1}'.format(base_url, invalid_ccx_id)) + resolve(f'{base_url}{invalid_ccx_id}') # the following course ID works even if it is not a CCX valid course id (the regex matches course ID strings) - resolver = resolve('{0}{1}'.format(base_url, 'ccx-v1:org.0+course_0+Run_0')) + resolver = resolve('{}{}'.format(base_url, 'ccx-v1:org.0+course_0+Run_0')) assert views.CCXDetailView.__name__ == resolver.func.__name__ assert views.CCXDetailView.__module__ == resolver.func.__module__ # and of course a valid ccx course id - resolver = resolve('{0}{1}'.format(base_url, self.ccx_key_str)) + resolver = resolve(f'{base_url}{self.ccx_key_str}') assert views.CCXDetailView.__name__ == resolver.func.__name__ assert views.CCXDetailView.__module__ == resolver.func.__module__ @@ -880,7 +879,7 @@ class CcxDetailTest(CcxRestApiTest): self.expect_error(status.HTTP_404_NOT_FOUND, 'ccx_course_id_does_not_exist', resp) # get a valid ccx key and add few 0s to get a non existing ccx for a valid course - ccx_key_str = '{0}000000'.format(self.ccx_key_str) + ccx_key_str = f'{self.ccx_key_str}000000' url = reverse('ccx_api:v0:ccx:detail', kwargs={'ccx_course_id': ccx_key_str}) # the permission class will give a 403 error because will not find the CCX resp = client_request(url, {}, HTTP_AUTHORIZATION=self.auth) @@ -902,8 +901,8 @@ class CcxDetailTest(CcxRestApiTest): assert resp.data.get('display_name') == self.ccx.display_name assert resp.data.get('max_students_allowed') == self.ccx.max_student_enrollments_allowed assert resp.data.get('coach_email') == self.ccx.coach.email - assert resp.data.get('master_course_id') == six.text_type(self.ccx.course_id) - six.assertCountEqual(self, resp.data.get('course_modules'), self.master_course_chapters) + assert resp.data.get('master_course_id') == str(self.ccx.course_id) + assert len(resp.data.get('course_modules')) == len(self.master_course_chapters) def test_delete_detail(self): """ @@ -1068,19 +1067,19 @@ class CcxDetailTest(CcxRestApiTest): resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_204_NO_CONTENT ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) - six.assertCountEqual(self, ccx_from_db.structure, data['course_modules']) + assert len(ccx_from_db.structure) == len(data['course_modules']) data = {'course_modules': []} resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_204_NO_CONTENT ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) - six.assertCountEqual(self, ccx_from_db.structure, []) + assert len(ccx_from_db.structure) == len([]) data = {'course_modules': self.master_course_chapters} resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_204_NO_CONTENT ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) - six.assertCountEqual(self, ccx_from_db.structure, self.master_course_chapters) + assert len(ccx_from_db.structure) == len(self.master_course_chapters) data = {'course_modules': None} resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) @@ -1093,7 +1092,7 @@ class CcxDetailTest(CcxRestApiTest): resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) assert resp.status_code == status.HTTP_204_NO_CONTENT ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) - six.assertCountEqual(self, ccx_from_db.structure, chapters) + assert len(ccx_from_db.structure) == len(chapters) @ddt.data( True, @@ -1114,7 +1113,7 @@ class CcxDetailTest(CcxRestApiTest): else: assert resp.status_code == status.HTTP_204_NO_CONTENT ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) - six.assertCountEqual(self, ccx_from_db.structure, chapters) + assert len(ccx_from_db.structure) == len(chapters) @ddt.data( True, diff --git a/lms/djangoapps/ccx/api/v0/urls.py b/lms/djangoapps/ccx/api/v0/urls.py index 5d9374a501..a89829fcec 100644 --- a/lms/djangoapps/ccx/api/v0/urls.py +++ b/lms/djangoapps/ccx/api/v0/urls.py @@ -12,7 +12,7 @@ CCX_COURSE_ID_PATTERN = settings.COURSE_ID_PATTERN.replace('course_id', 'ccx_cou CCX_URLS = ([ url(r'^$', views.CCXListView.as_view(), name='list'), - url(r'^{}/?$'.format(CCX_COURSE_ID_PATTERN), views.CCXDetailView.as_view(), name='detail'), + url(fr'^{CCX_COURSE_ID_PATTERN}/?$', views.CCXDetailView.as_view(), name='detail'), ], 'ccx') app_name = 'v0' diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py index 94b93c007c..c698c645c0 100644 --- a/lms/djangoapps/ccx/api/v0/views.py +++ b/lms/djangoapps/ccx/api/v0/views.py @@ -6,7 +6,6 @@ import json import logging import pytz -import six from ccx_keys.locator import CCXLocator from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.db import transaction @@ -20,15 +19,15 @@ from rest_framework.generics import GenericAPIView from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response -from lms.djangoapps.courseware import courses +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.roles import CourseCcxCoachRole from lms.djangoapps.ccx.models import CcxFieldOverride, CustomCourseForEdX from lms.djangoapps.ccx.overrides import override_field_for_ccx from lms.djangoapps.ccx.utils import add_master_course_staff_to_ccx, assign_staff_role_to_ccx, is_email +from lms.djangoapps.courseware import courses from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api import authentication, permissions -from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.student.roles import CourseCcxCoachRole from xmodule.modulestore.django import SignalHandler from .paginators import CCXAPIPagination @@ -66,14 +65,14 @@ def get_valid_course(course_id, is_ccx=False, advanced_course_check=False): try: course_key = CourseKey.from_string(course_id) except InvalidKeyError: - log.info(u'Course ID string "%s" is not valid', course_id) + log.info('Course ID string "%s" is not valid', course_id) return None, None, 'course_id_not_valid', status.HTTP_400_BAD_REQUEST if not is_ccx: try: course_object = courses.get_course_by_id(course_key) except Http404: - log.info(u'Master Course with ID "%s" not found', course_id) + log.info('Master Course with ID "%s" not found', course_id) return None, None, 'course_id_does_not_exist', status.HTTP_404_NOT_FOUND if advanced_course_check: if course_object.id.deprecated: @@ -85,7 +84,7 @@ def get_valid_course(course_id, is_ccx=False, advanced_course_check=False): try: ccx_id = course_key.ccx except AttributeError: - log.info(u'Course ID string "%s" is not a valid CCX ID', course_id) + log.info('Course ID string "%s" is not a valid CCX ID', course_id) return None, None, 'course_id_not_valid_ccx_id', status.HTTP_400_BAD_REQUEST # get the master_course key master_course_key = course_key.to_course_locator() @@ -93,7 +92,7 @@ def get_valid_course(course_id, is_ccx=False, advanced_course_check=False): ccx_course = CustomCourseForEdX.objects.get(id=ccx_id, course_id=master_course_key) return ccx_course, course_key, None, None except CustomCourseForEdX.DoesNotExist: - log.info(u'CCX Course with ID "%s" not found', course_id) + log.info('CCX Course with ID "%s" not found', course_id) return None, None, 'ccx_course_id_does_not_exist', status.HTTP_404_NOT_FOUND @@ -118,7 +117,7 @@ def get_valid_input(request_data, ignore_missing=False): if not ignore_missing: for field in mandatory_fields: if field not in request_data: - field_errors[field] = {'error_code': 'missing_field_{0}'.format(field)} + field_errors[field] = {'error_code': f'missing_field_{field}'} if field_errors: return valid_input, field_errors @@ -389,7 +388,7 @@ class CCXListView(GenericAPIView): sort_direction = '' if sort_order_input == 'desc': sort_direction = '-' - queryset = queryset.order_by('{0}{1}'.format(sort_direction, order_by_input)) + queryset = queryset.order_by(f'{sort_direction}{order_by_input}') page = self.paginate_queryset(queryset) serializer = self.get_serializer(page, many=True) response = self.get_paginated_response(serializer.data) @@ -492,7 +491,7 @@ class CCXListView(GenericAPIView): # pull the ccx course key ccx_course_key = CCXLocator.from_course_locator( master_course_object.id, - six.text_type(ccx_course_object.id) + str(ccx_course_object.id) ) # enroll the coach in the newly created ccx email_params = get_email_params( @@ -526,7 +525,7 @@ class CCXListView(GenericAPIView): course_key=ccx_course_key ) for rec, response in responses: - log.info(u'Signal fired when course is published. Receiver: %s. Response: %s', rec, response) + log.info('Signal fired when course is published. Receiver: %s. Response: %s', rec, response) return Response( status=status.HTTP_201_CREATED, data=serializer.data @@ -694,7 +693,7 @@ class CCXDetailView(GenericAPIView): ) master_course_id = request.data.get('master_course_id') - if master_course_id is not None and six.text_type(ccx_course_object.course_id) != master_course_id: + if master_course_id is not None and str(ccx_course_object.course_id) != master_course_id: return Response( status=status.HTTP_403_FORBIDDEN, data={ @@ -712,7 +711,7 @@ class CCXDetailView(GenericAPIView): ) # get the master course key and master course object - master_course_object, master_course_key, _, _ = get_valid_course(six.text_type(ccx_course_object.course_id)) + master_course_object, master_course_key, _, _ = get_valid_course(str(ccx_course_object.course_id)) with transaction.atomic(): # update the display name @@ -780,7 +779,7 @@ class CCXDetailView(GenericAPIView): course_key=ccx_course_key ) for rec, response in responses: - log.info(u'Signal fired when course is published. Receiver: %s. Response: %s', rec, response) + log.info('Signal fired when course is published. Receiver: %s. Response: %s', rec, response) return Response( status=status.HTTP_204_NO_CONTENT, diff --git a/lms/djangoapps/ccx/migrations/0001_initial.py b/lms/djangoapps/ccx/migrations/0001_initial.py index 7c281f208c..5d873ba7e4 100644 --- a/lms/djangoapps/ccx/migrations/0001_initial.py +++ b/lms/djangoapps/ccx/migrations/0001_initial.py @@ -1,6 +1,3 @@ -# -*- coding: utf-8 -*- - - from django.conf import settings from django.db import migrations, models from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField @@ -19,7 +16,7 @@ class Migration(migrations.Migration): ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), ('location', UsageKeyField(max_length=255, db_index=True)), ('field', models.CharField(max_length=255)), - ('value', models.TextField(default=u'null')), + ('value', models.TextField(default='null')), ], ), migrations.CreateModel( @@ -38,6 +35,6 @@ class Migration(migrations.Migration): ), migrations.AlterUniqueTogether( name='ccxfieldoverride', - unique_together=set([('ccx', 'location', 'field')]), + unique_together={('ccx', 'location', 'field')}, ), ] diff --git a/lms/djangoapps/ccx/migrations/0002_customcourseforedx_structure_json.py b/lms/djangoapps/ccx/migrations/0002_customcourseforedx_structure_json.py index 3e18742f40..3fc44f4c79 100644 --- a/lms/djangoapps/ccx/migrations/0002_customcourseforedx_structure_json.py +++ b/lms/djangoapps/ccx/migrations/0002_customcourseforedx_structure_json.py @@ -1,6 +1,3 @@ -# -*- coding: utf-8 -*- - - from django.db import migrations, models @@ -14,6 +11,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name='customcourseforedx', name='structure_json', - field=models.TextField(null=True, verbose_name=u'Structure JSON', blank=True), + field=models.TextField(null=True, verbose_name='Structure JSON', blank=True), ), ] diff --git a/lms/djangoapps/ccx/migrations/0003_add_master_course_staff_in_ccx.py b/lms/djangoapps/ccx/migrations/0003_add_master_course_staff_in_ccx.py index 4adec76f15..18f555b887 100644 --- a/lms/djangoapps/ccx/migrations/0003_add_master_course_staff_in_ccx.py +++ b/lms/djangoapps/ccx/migrations/0003_add_master_course_staff_in_ccx.py @@ -1,6 +1,3 @@ -# -*- coding: utf-8 -*- - - import logging import six @@ -29,7 +26,7 @@ def add_master_course_staff_to_ccx_for_existing_ccx(apps, schema_editor): if not ccx.course_id or ccx.course_id.deprecated: # prevent migration for deprecated course ids or invalid ids. continue - ccx_locator = CCXLocator.from_course_locator(ccx.course_id, six.text_type(ccx.id)) + ccx_locator = CCXLocator.from_course_locator(ccx.course_id, str(ccx.id)) try: course = get_course_by_id(ccx.course_id) add_master_course_staff_to_ccx( @@ -61,7 +58,7 @@ def remove_master_course_staff_from_ccx_for_existing_ccx(apps, schema_editor): if not ccx.course_id or ccx.course_id.deprecated: # prevent migration for deprecated course ids or invalid ids. continue - ccx_locator = CCXLocator.from_course_locator(ccx.course_id, six.text_type(ccx.id)) + ccx_locator = CCXLocator.from_course_locator(ccx.course_id, str(ccx.id)) try: course = get_course_by_id(ccx.course_id) remove_master_course_staff_from_ccx( diff --git a/lms/djangoapps/ccx/migrations/0004_seed_forum_roles_in_ccx_courses.py b/lms/djangoapps/ccx/migrations/0004_seed_forum_roles_in_ccx_courses.py index 7ed5ead9e7..4dad911089 100644 --- a/lms/djangoapps/ccx/migrations/0004_seed_forum_roles_in_ccx_courses.py +++ b/lms/djangoapps/ccx/migrations/0004_seed_forum_roles_in_ccx_courses.py @@ -1,6 +1,3 @@ -# -*- coding: utf-8 -*- - - import logging import six @@ -52,7 +49,7 @@ def seed_forum_roles_for_existing_ccx(apps, schema_editor): ) continue - ccx_locator = CCXLocator.from_course_locator(ccx.course_id, six.text_type(ccx.id)) + ccx_locator = CCXLocator.from_course_locator(ccx.course_id, str(ccx.id)) # Create forum roles. admin_role, _ = Role.objects.get_or_create(name=FORUM_ROLE_ADMINISTRATOR, course_id=ccx_locator) diff --git a/lms/djangoapps/ccx/migrations/0005_change_ccx_coach_to_staff.py b/lms/djangoapps/ccx/migrations/0005_change_ccx_coach_to_staff.py index a15e698c5b..a7b5e57695 100644 --- a/lms/djangoapps/ccx/migrations/0005_change_ccx_coach_to_staff.py +++ b/lms/djangoapps/ccx/migrations/0005_change_ccx_coach_to_staff.py @@ -1,6 +1,3 @@ -# -*- coding: utf-8 -*- - - import logging import six @@ -32,18 +29,18 @@ def change_existing_ccx_coaches_to_staff(apps, schema_editor): return list_ccx = CustomCourseForEdX.objects.using(db_alias).all() for ccx in list_ccx: - ccx_locator = CCXLocator.from_course_locator(ccx.course_id, six.text_type(ccx.id)) + ccx_locator = CCXLocator.from_course_locator(ccx.course_id, str(ccx.id)) try: course = get_course_by_id(ccx_locator) except Http404: - log.error('Could not migrate access for CCX course: %s', six.text_type(ccx_locator)) + log.error('Could not migrate access for CCX course: %s', str(ccx_locator)) else: coach = User.objects.get(id=ccx.coach.id) allow_access(course, coach, 'staff', send_email=False) revoke_access(course, coach, 'ccx_coach', send_email=False) log.info( 'The CCX coach of CCX %s has been switched from "CCX Coach" to "Staff".', - six.text_type(ccx_locator) + str(ccx_locator) ) def revert_ccx_staff_to_coaches(apps, schema_editor): @@ -62,18 +59,18 @@ def revert_ccx_staff_to_coaches(apps, schema_editor): return list_ccx = CustomCourseForEdX.objects.using(db_alias).all() for ccx in list_ccx: - ccx_locator = CCXLocator.from_course_locator(ccx.course_id, six.text_type(ccx.id)) + ccx_locator = CCXLocator.from_course_locator(ccx.course_id, str(ccx.id)) try: course = get_course_by_id(ccx_locator) except Http404: - log.error('Could not migrate access for CCX course: %s', six.text_type(ccx_locator)) + log.error('Could not migrate access for CCX course: %s', str(ccx_locator)) else: coach = User.objects.get(id=ccx.coach.id) allow_access(course, coach, 'ccx_coach', send_email=False) revoke_access(course, coach, 'staff', send_email=False) log.info( 'The CCX coach of CCX %s has been switched from "Staff" to "CCX Coach".', - six.text_type(ccx_locator) + str(ccx_locator) ) class Migration(migrations.Migration): diff --git a/lms/djangoapps/ccx/migrations/0006_set_display_name_as_override.py b/lms/djangoapps/ccx/migrations/0006_set_display_name_as_override.py index 19e0573405..9c538039f1 100644 --- a/lms/djangoapps/ccx/migrations/0006_set_display_name_as_override.py +++ b/lms/djangoapps/ccx/migrations/0006_set_display_name_as_override.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- # Generated by Django 1.11.15 on 2018-08-31 18:13 diff --git a/lms/djangoapps/ccx/models.py b/lms/djangoapps/ccx/models.py index bf8114570c..46400b5357 100644 --- a/lms/djangoapps/ccx/models.py +++ b/lms/djangoapps/ccx/models.py @@ -7,7 +7,6 @@ import json import logging from datetime import datetime -import six from ccx_keys.locator import CCXLocator from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.db import models @@ -32,9 +31,9 @@ class CustomCourseForEdX(models.Model): coach = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) # if not empty, this field contains a json serialized list of # the master course modules - structure_json = models.TextField(verbose_name=u'Structure JSON', blank=True, null=True) + structure_json = models.TextField(verbose_name='Structure JSON', blank=True, null=True) - class Meta(object): + class Meta: app_label = 'ccx' @lazy @@ -103,7 +102,7 @@ class CustomCourseForEdX(models.Model): Returns: The CCXLocator corresponding to this CCX. """ - return CCXLocator.from_course_locator(self.course_id, six.text_type(self.id)) + return CCXLocator.from_course_locator(self.course_id, str(self.id)) class CcxFieldOverride(models.Model): @@ -116,8 +115,8 @@ class CcxFieldOverride(models.Model): location = UsageKeyField(max_length=255, db_index=True) field = models.CharField(max_length=255) - class Meta(object): + class Meta: app_label = 'ccx' unique_together = (('ccx', 'location', 'field'),) - value = models.TextField(default=u'null') + value = models.TextField(default='null') diff --git a/lms/djangoapps/ccx/modulestore.py b/lms/djangoapps/ccx/modulestore.py index 2d9983abfb..893ec1609f 100644 --- a/lms/djangoapps/ccx/modulestore.py +++ b/lms/djangoapps/ccx/modulestore.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- """A modulestore wrapper It will 'unwrap' ccx keys on the way in and re-wrap them on the way out @@ -14,7 +13,6 @@ version that was passed in. from contextlib import contextmanager from functools import partial -import six from ccx_keys.locator import CCXBlockUsageLocator, CCXLocator from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator @@ -71,7 +69,7 @@ def restore_ccx_collection(field_value, ccx_id=None): if isinstance(field_value, list): field_value = [restore_ccx(fv, ccx_id) for fv in field_value] elif isinstance(field_value, dict): - for key, val in six.iteritems(field_value): + for key, val in field_value.items(): field_value[key] = restore_ccx(val, ccx_id) else: field_value = restore_ccx(field_value, ccx_id) @@ -88,7 +86,7 @@ def remove_ccx(to_strip): yield stripped, partial(restore_ccx_collection, ccx_id=ccx) -class CCXModulestoreWrapper(object): +class CCXModulestoreWrapper: """This class wraps a modulestore The purpose is to remove ccx-specific identifiers during lookup and restore diff --git a/lms/djangoapps/ccx/overrides.py b/lms/djangoapps/ccx/overrides.py index 8f86150e0c..a85b83ed60 100644 --- a/lms/djangoapps/ccx/overrides.py +++ b/lms/djangoapps/ccx/overrides.py @@ -40,7 +40,7 @@ class CustomCoursesForEdxOverrideProvider(FieldOverrideProvider): elif hasattr(block, 'location'): course_key = block.location.course_key else: - msg = u"Unable to get course id when calculating ccx overide for block type %r" + msg = "Unable to get course id when calculating ccx overide for block type %r" log.error(msg, type(block)) if course_key is not None: ccx = get_current_ccx(course_key) diff --git a/lms/djangoapps/ccx/permissions.py b/lms/djangoapps/ccx/permissions.py index ba36b3cdb2..2acc34e1f4 100644 --- a/lms/djangoapps/ccx/permissions.py +++ b/lms/djangoapps/ccx/permissions.py @@ -3,6 +3,7 @@ Permission definitions for the ccx djangoapp """ from bridgekeeper import perms + from lms.djangoapps.courseware.rules import HasAccessRule VIEW_CCX_COACH_DASHBOARD = 'ccx.view_ccx_coach_dashboard' diff --git a/lms/djangoapps/ccx/tasks.py b/lms/djangoapps/ccx/tasks.py index 347ff45bea..8da98c10dd 100644 --- a/lms/djangoapps/ccx/tasks.py +++ b/lms/djangoapps/ccx/tasks.py @@ -5,7 +5,6 @@ Asynchronous tasks for the CCX app. import logging -import six from ccx_keys.locator import CCXLocator from django.dispatch import receiver from opaque_keys import InvalidKeyError @@ -24,7 +23,7 @@ def course_published_handler(sender, course_key, **kwargs): # pylint: disable=u Consume signals that indicate course published. If course already a CCX, do nothing. """ if not isinstance(course_key, CCXLocator): - send_ccx_course_published.delay(six.text_type(course_key)) + send_ccx_course_published.delay(str(course_key)) @CELERY_APP.task @@ -35,13 +34,13 @@ def send_ccx_course_published(course_key): course_key = CourseLocator.from_string(course_key) for ccx in CustomCourseForEdX.objects.filter(course_id=course_key): try: - ccx_key = CCXLocator.from_course_locator(course_key, six.text_type(ccx.id)) + ccx_key = CCXLocator.from_course_locator(course_key, str(ccx.id)) except InvalidKeyError: - log.info(u'Attempt to publish course with deprecated id. Course: %s. CCX: %s', course_key, ccx.id) + log.info('Attempt to publish course with deprecated id. Course: %s. CCX: %s', course_key, ccx.id) continue responses = SignalHandler.course_published.send( sender=ccx, course_key=ccx_key ) for rec, response in responses: - log.info(u'Signal fired when course is published. Receiver: %s. Response: %s', rec, response) + log.info('Signal fired when course is published. Receiver: %s. Response: %s', rec, response) diff --git a/lms/djangoapps/ccx/tests/factories.py b/lms/djangoapps/ccx/tests/factories.py index b622a99e4a..3130af38be 100644 --- a/lms/djangoapps/ccx/tests/factories.py +++ b/lms/djangoapps/ccx/tests/factories.py @@ -6,15 +6,15 @@ Dummy factories for tests from factory import Sequence, SubFactory from factory.django import DjangoModelFactory -from lms.djangoapps.ccx.models import CustomCourseForEdX from common.djangoapps.student.tests.factories import UserFactory +from lms.djangoapps.ccx.models import CustomCourseForEdX # pylint: disable=missing-class-docstring class CcxFactory(DjangoModelFactory): - class Meta(object): + class Meta: model = CustomCourseForEdX - display_name = Sequence(lambda n: u'Test CCX #{0}'.format(n)) # pylint: disable=unnecessary-lambda + display_name = Sequence(lambda n: f'Test CCX #{n}') # pylint: disable=unnecessary-lambda id = None # pylint: disable=invalid-name coach = SubFactory(UserFactory) diff --git a/lms/djangoapps/ccx/tests/test_ccx_modulestore.py b/lms/djangoapps/ccx/tests/test_ccx_modulestore.py index 8c710cd43a..a33146dbd4 100644 --- a/lms/djangoapps/ccx/tests/test_ccx_modulestore.py +++ b/lms/djangoapps/ccx/tests/test_ccx_modulestore.py @@ -9,10 +9,10 @@ from itertools import chain import pytz from ccx_keys.locator import CCXLocator -from six.moves import range, zip_longest +from six.moves import zip_longest -from lms.djangoapps.ccx.models import CustomCourseForEdX from common.djangoapps.student.tests.factories import AdminFactory, UserFactory +from lms.djangoapps.ccx.models import CustomCourseForEdX from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -24,7 +24,7 @@ class TestCCXModulestoreWrapper(SharedModuleStoreTestCase): @classmethod def setUpClass(cls): - super(TestCCXModulestoreWrapper, cls).setUpClass() + super().setUpClass() cls.course = CourseFactory.create() start = datetime.datetime(2010, 5, 12, 2, 42, tzinfo=pytz.UTC) due = datetime.datetime(2010, 7, 7, 0, 0, tzinfo=pytz.UTC) @@ -57,7 +57,7 @@ class TestCCXModulestoreWrapper(SharedModuleStoreTestCase): """ Set up tests """ - super(TestCCXModulestoreWrapper, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.ccx = ccx = CustomCourseForEdX( course_id=self.course.id, display_name='Test CCX', diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 9d689bab10..8cfca804d3 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -1,4 +1,3 @@ -# coding=UTF-8 """ Performance tests for field overrides. """ @@ -6,12 +5,10 @@ Performance tests for field overrides. import itertools from datetime import datetime +from unittest import mock import ddt -import mock import pytest -import six -from six.moves import range from ccx_keys.locator import CCXLocator from django.conf import settings from django.contrib.messages.storage.fallback import FallbackStorage @@ -24,15 +21,15 @@ from opaque_keys.edx.keys import CourseKey from pytz import UTC from xblock.core import XBlock -from lms.djangoapps.courseware.testutils import FieldOverrideTestMixin -from lms.djangoapps.courseware.views.views import progress +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.ccx.tests.factories import CcxFactory from lms.djangoapps.courseware.field_overrides import OverrideFieldData +from lms.djangoapps.courseware.testutils import FieldOverrideTestMixin +from lms.djangoapps.courseware.views.views import progress from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.features.content_type_gating.models import ContentTypeGatingConfig -from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ( TEST_DATA_MONGO_MODULESTORE, TEST_DATA_SPLIT_MODULESTORE, @@ -67,7 +64,7 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT """ Create a test client, course, and user. """ - super(FieldOverridePerformanceTestCase, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.request_factory = RequestFactory() self.student = UserFactory.create() @@ -140,7 +137,7 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT self.student, course_key ) - return CourseKey.from_string(six.text_type(course_key)) + return CourseKey.from_string(str(course_key)) def grade_course(self, course_key): """ @@ -148,7 +145,7 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT """ return progress( self.request, - course_id=six.text_type(course_key), + course_id=str(course_key), student_id=self.student.id ) diff --git a/lms/djangoapps/ccx/tests/test_models.py b/lms/djangoapps/ccx/tests/test_models.py index 28f7b44f47..006ba60de5 100644 --- a/lms/djangoapps/ccx/tests/test_models.py +++ b/lms/djangoapps/ccx/tests/test_models.py @@ -27,7 +27,7 @@ class TestCCX(ModuleStoreTestCase): def setUp(self): """common setup for all tests""" - super(TestCCX, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.course = CourseFactory.create() self.coach = AdminFactory.create() role = CourseCcxCoachRole(self.course.id) diff --git a/lms/djangoapps/ccx/tests/test_overrides.py b/lms/djangoapps/ccx/tests/test_overrides.py index 83753f00d0..3545be00e6 100644 --- a/lms/djangoapps/ccx/tests/test_overrides.py +++ b/lms/djangoapps/ccx/tests/test_overrides.py @@ -1,26 +1,24 @@ -# coding=UTF-8 """ tests for overrides """ import datetime +from unittest import mock -import mock import pytz from ccx_keys.locator import CCXLocator from django.test.utils import override_settings from edx_django_utils.cache import RequestCache -from six.moves import range -from lms.djangoapps.courseware.courses import get_course_by_id -from lms.djangoapps.courseware.testutils import FieldOverrideTestMixin +from common.djangoapps.student.tests.factories import AdminFactory from lms.djangoapps.ccx.models import CustomCourseForEdX from lms.djangoapps.ccx.overrides import override_field_for_ccx from lms.djangoapps.ccx.tests.utils import flatten, iter_blocks +from lms.djangoapps.courseware.courses import get_course_by_id from lms.djangoapps.courseware.field_overrides import OverrideFieldData from lms.djangoapps.courseware.tests.test_field_overrides import inject_field_overrides -from common.djangoapps.student.tests.factories import AdminFactory +from lms.djangoapps.courseware.testutils import FieldOverrideTestMixin from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -40,7 +38,7 @@ class TestFieldOverrides(FieldOverrideTestMixin, SharedModuleStoreTestCase): """ Course is created here and shared by all the class's tests. """ - super(TestFieldOverrides, cls).setUpClass() + super().setUpClass() cls.course = CourseFactory.create() cls.course.enable_ccx = True @@ -63,7 +61,7 @@ class TestFieldOverrides(FieldOverrideTestMixin, SharedModuleStoreTestCase): """ Set up tests """ - super(TestFieldOverrides, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.ccx = ccx = CustomCourseForEdX( course_id=self.course.id, diff --git a/lms/djangoapps/ccx/tests/test_tasks.py b/lms/djangoapps/ccx/tests/test_tasks.py index e431e7face..828324ecc0 100644 --- a/lms/djangoapps/ccx/tests/test_tasks.py +++ b/lms/djangoapps/ccx/tests/test_tasks.py @@ -4,16 +4,15 @@ Tests for celery tasks defined in tasks module import contextlib +from unittest import mock -import mock -import six from ccx_keys.locator import CCXLocator +from common.djangoapps.student.roles import CourseCcxCoachRole +from common.djangoapps.student.tests.factories import AdminFactory from lms.djangoapps.ccx.tasks import send_ccx_course_published from lms.djangoapps.ccx.tests.factories import CcxFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from common.djangoapps.student.roles import CourseCcxCoachRole -from common.djangoapps.student.tests.factories import AdminFactory from xmodule.modulestore.django import SignalHandler from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -39,7 +38,7 @@ class TestSendCCXCoursePublished(ModuleStoreTestCase): """ Set up tests """ - super(TestSendCCXCoursePublished, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() course = self.course = CourseFactory.create(org="edX", course="999", display_name="Run 666") course2 = self.course2 = CourseFactory.create(org="edX", course="999a", display_name="Run 667") coach = AdminFactory.create() @@ -54,7 +53,7 @@ class TestSendCCXCoursePublished(ModuleStoreTestCase): """ Call the function under test """ - send_ccx_course_published(six.text_type(course_key)) + send_ccx_course_published(str(course_key)) def test_signal_not_sent_for_ccx(self): """ diff --git a/lms/djangoapps/ccx/tests/test_utils.py b/lms/djangoapps/ccx/tests/test_utils.py index f48ddad7c3..21fbe96288 100644 --- a/lms/djangoapps/ccx/tests/test_utils.py +++ b/lms/djangoapps/ccx/tests/test_utils.py @@ -5,17 +5,17 @@ test utils import uuid from smtplib import SMTPException +from unittest import mock -import mock from ccx_keys.locator import CCXLocator +from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentException +from common.djangoapps.student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole +from common.djangoapps.student.tests.factories import AdminFactory from lms.djangoapps.ccx.tests.factories import CcxFactory from lms.djangoapps.ccx.tests.utils import CcxTestCase from lms.djangoapps.ccx.utils import add_master_course_staff_to_ccx, ccx_course, remove_master_course_staff_from_ccx from lms.djangoapps.instructor.access import list_with_level -from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentException -from common.djangoapps.student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole -from common.djangoapps.student.tests.factories import AdminFactory from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -27,7 +27,7 @@ class TestGetCCXFromCCXLocator(ModuleStoreTestCase): def setUp(self): """Set up a course, coach, ccx and user""" - super(TestGetCCXFromCCXLocator, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.course = CourseFactory.create() coach = self.coach = AdminFactory.create() role = CourseCcxCoachRole(self.course.id) @@ -60,7 +60,7 @@ class TestStaffOnCCX(CcxTestCase): MODULESTORE = TEST_DATA_SPLIT_MODULESTORE def setUp(self): - super(TestStaffOnCCX, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() # Create instructor account self.client.login(username=self.coach.username, password="test") @@ -237,7 +237,7 @@ class TestStaffOnCCX(CcxTestCase): assert CourseInstructorRole(self.course.id).has_user(instructor) outbox = self.get_outbox() # create a unique display name - display_name = 'custom_display_{}'.format(uuid.uuid4()) + display_name = f'custom_display_{uuid.uuid4()}' list_staff_master_course = list_with_level(self.course, 'staff') list_instructor_master_course = list_with_level(self.course, 'instructor') assert len(outbox) == 0 @@ -262,7 +262,7 @@ class TestStaffOnCCX(CcxTestCase): outbox = self.get_outbox() add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name, send_email=False) # create a unique display name - display_name = 'custom_display_{}'.format(uuid.uuid4()) + display_name = f'custom_display_{uuid.uuid4()}' list_staff_master_course = list_with_level(self.course, 'staff') list_instructor_master_course = list_with_level(self.course, 'instructor') assert len(outbox) == 0 diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index c02040bcb4..c5086d736a 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -6,10 +6,10 @@ test views import datetime import json import re +from unittest.mock import MagicMock, patch import ddt import six -from six.moves import range, zip from ccx_keys.locator import CCXLocator from django.conf import settings from django.test import RequestFactory @@ -17,32 +17,31 @@ from django.test.utils import override_settings from django.urls import resolve, reverse from django.utils.translation import ugettext as _ from edx_django_utils.cache import RequestCache -from mock import MagicMock, patch from opaque_keys.edx.keys import CourseKey from pytz import UTC from capa.tests.response_xml_factory import StringResponseXMLFactory -from lms.djangoapps.courseware.courses import get_course_by_id -from lms.djangoapps.courseware.tabs import get_course_tab_list -from lms.djangoapps.courseware.tests.factories import StudentModuleFactory -from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase -from lms.djangoapps.courseware.testutils import FieldOverrideTestMixin from common.djangoapps.edxmako.shortcuts import render_to_response +from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed +from common.djangoapps.student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole +from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory from lms.djangoapps.ccx.models import CustomCourseForEdX from lms.djangoapps.ccx.overrides import get_override_for_ccx, override_field_for_ccx from lms.djangoapps.ccx.tests.factories import CcxFactory from lms.djangoapps.ccx.tests.utils import CcxTestCase, flatten from lms.djangoapps.ccx.utils import ccx_course, is_email from lms.djangoapps.ccx.views import get_date +from lms.djangoapps.courseware.courses import get_course_by_id +from lms.djangoapps.courseware.tabs import get_course_tab_list +from lms.djangoapps.courseware.tests.factories import StudentModuleFactory +from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase +from lms.djangoapps.courseware.testutils import FieldOverrideTestMixin from lms.djangoapps.discussion.django_comment_client.utils import has_forum_access from lms.djangoapps.grades.api import task_compute_all_grades_for_course from lms.djangoapps.instructor.access import allow_access, list_with_level from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR from openedx.core.djangoapps.django_comment_common.utils import are_permissions_roles_seeded -from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed -from common.djangoapps.student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole -from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( @@ -88,7 +87,7 @@ def setup_students_and_grades(context): context.student = student = UserFactory.create() CourseEnrollmentFactory.create(user=student, course_id=context.course.id) - context.student2 = student2 = UserFactory.create(username=u'u\u0131\u028c\u0279\u0250\u026f') + context.student2 = student2 = UserFactory.create(username='u\u0131\u028c\u0279\u0250\u026f') CourseEnrollmentFactory.create(user=student2, course_id=context.course.id) # create grades for self.student as if they'd submitted the ccx @@ -112,7 +111,7 @@ def setup_students_and_grades(context): module_state_key=problem.location ) - task_compute_all_grades_for_course.apply_async(kwargs={'course_key': six.text_type(context.course.id)}) + task_compute_all_grades_for_course.apply_async(kwargs={'course_key': str(context.course.id)}) def unhide(unit): @@ -132,7 +131,7 @@ class TestAdminAccessCoachDashboard(CcxTestCase, LoginEnrollmentTestCase): MODULESTORE = TEST_DATA_SPLIT_MODULESTORE def setUp(self): - super(TestAdminAccessCoachDashboard, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() self.make_coach() ccx = self.make_ccx() ccx_key = CCXLocator.from_course_locator(self.course.id, ccx.id) @@ -182,24 +181,24 @@ class TestCCXProgressChanges(CcxTestCase, LoginEnrollmentTestCase): """ Set up tests """ - super(TestCCXProgressChanges, cls).setUpClass() + super().setUpClass() start = datetime.datetime(2016, 7, 1, 0, 0, tzinfo=UTC) due = datetime.datetime(2016, 7, 8, 0, 0, tzinfo=UTC) cls.course = course = CourseFactory.create(enable_ccx=True, start=start) - chapter = ItemFactory.create(start=start, parent=course, category=u'chapter') + chapter = ItemFactory.create(start=start, parent=course, category='chapter') sequential = ItemFactory.create( parent=chapter, start=start, due=due, - category=u'sequential', + category='sequential', metadata={'graded': True, 'format': 'Homework'} ) vertical = ItemFactory.create( parent=sequential, start=start, due=due, - category=u'vertical', + category='vertical', metadata={'graded': True, 'format': 'Homework'} ) @@ -231,7 +230,7 @@ class TestCCXProgressChanges(CcxTestCase, LoginEnrollmentTestCase): grade_summary = progress_page_response.mako_context['courseware_summary'] chapter = grade_summary[0] section = chapter['sections'][0] - progress_page_due_date = section.due.strftime(u"%Y-%m-%d %H:%M") + progress_page_due_date = section.due.strftime("%Y-%m-%d %H:%M") assert progress_page_due_date == due @patch('lms.djangoapps.ccx.views.render_to_response', intercept_renderer) @@ -243,7 +242,7 @@ class TestCCXProgressChanges(CcxTestCase, LoginEnrollmentTestCase): """ self.make_coach() ccx = self.make_ccx() - ccx_course_key = CCXLocator.from_course_locator(self.course.id, six.text_type(ccx.id)) + ccx_course_key = CCXLocator.from_course_locator(self.course.id, str(ccx.id)) self.client.login(username=self.coach.username, password="test") url = reverse('ccx_coach_dashboard', kwargs={'course_id': ccx_course_key}) @@ -256,8 +255,8 @@ class TestCCXProgressChanges(CcxTestCase, LoginEnrollmentTestCase): # edit schedule date = datetime.datetime.now() - datetime.timedelta(days=5) - start = date.strftime(u"%Y-%m-%d %H:%M") - due = (date + datetime.timedelta(days=3)).strftime(u"%Y-%m-%d %H:%M") + start = date.strftime("%Y-%m-%d %H:%M") + due = (date + datetime.timedelta(days=3)).strftime("%Y-%m-%d %H:%M") schedule[0]['start'] = start schedule[0]['children'][0]['start'] = start @@ -293,7 +292,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase): @classmethod def setUpClass(cls): - super(TestCoachDashboard, cls).setUpClass() + super().setUpClass() cls.course_disable_ccx = CourseFactory.create(enable_ccx=False) cls.course_with_ccx_connect_set = CourseFactory.create(enable_ccx=True, ccx_connector="http://ccx.com") @@ -301,7 +300,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase): """ Set up tests """ - super(TestCoachDashboard, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments + super().setUp() # Login with the instructor account self.client.login(username=self.coach.username, password="test") @@ -338,7 +337,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase): self.make_coach() url = reverse( 'ccx_coach_dashboard', - kwargs={'course_id': six.text_type(self.course.id)}) + kwargs={'course_id': str(self.course.id)}) response = self.client.get(url) assert response.status_code == 200 assert re.search('