From 4f807dd702bec7abdd1be0ee33299f0c55c6db03 Mon Sep 17 00:00:00 2001 From: Peter Pinch Date: Mon, 6 May 2019 16:43:43 -0400 Subject: [PATCH] INCR-155 Modernize openedx/core/djangoapps/video_config (#20408) * run modernizer on openedx/core/djangoapps/embargo/ * sorting imports * sort imports * docstring * try removing pylint disable --- openedx/core/djangoapps/embargo/admin.py | 1 + openedx/core/djangoapps/embargo/api.py | 1 + openedx/core/djangoapps/embargo/forms.py | 1 + openedx/core/djangoapps/embargo/messages.py | 1 + openedx/core/djangoapps/embargo/middleware.py | 1 + .../embargo/migrations/0001_initial.py | 2 +- .../migrations/0002_data__add_countries.py | 2 +- openedx/core/djangoapps/embargo/models.py | 35 ++++++++++--------- openedx/core/djangoapps/embargo/test_utils.py | 1 + .../djangoapps/embargo/tests/factories.py | 2 ++ .../core/djangoapps/embargo/tests/test_api.py | 1 + .../djangoapps/embargo/tests/test_forms.py | 14 ++++---- .../embargo/tests/test_middleware.py | 18 +++++----- .../djangoapps/embargo/tests/test_models.py | 34 +++++++++++------- .../djangoapps/embargo/tests/test_views.py | 5 +-- openedx/core/djangoapps/embargo/urls.py | 1 + openedx/core/djangoapps/embargo/views.py | 1 + 17 files changed, 75 insertions(+), 46 deletions(-) diff --git a/openedx/core/djangoapps/embargo/admin.py b/openedx/core/djangoapps/embargo/admin.py index 9944d3edce..5d7f769b54 100644 --- a/openedx/core/djangoapps/embargo/admin.py +++ b/openedx/core/djangoapps/embargo/admin.py @@ -1,6 +1,7 @@ """ Django admin page for embargo models """ +from __future__ import absolute_import import textwrap from config_models.admin import ConfigurationModelAdmin diff --git a/openedx/core/djangoapps/embargo/api.py b/openedx/core/djangoapps/embargo/api.py index 9b2d790493..0690e69a66 100644 --- a/openedx/core/djangoapps/embargo/api.py +++ b/openedx/core/djangoapps/embargo/api.py @@ -5,6 +5,7 @@ business logic that is not directly tied to the data itself. This API is exposed via the middleware(emabargo/middileware.py) layer but may be used directly in-process. """ +from __future__ import absolute_import import logging from django.conf import settings diff --git a/openedx/core/djangoapps/embargo/forms.py b/openedx/core/djangoapps/embargo/forms.py index 6c4e8608e7..5441a51b5d 100644 --- a/openedx/core/djangoapps/embargo/forms.py +++ b/openedx/core/djangoapps/embargo/forms.py @@ -2,6 +2,7 @@ Defines forms for providing validation of embargo admin details. """ +from __future__ import absolute_import import ipaddress from django import forms from django.utils.translation import ugettext as _ diff --git a/openedx/core/djangoapps/embargo/messages.py b/openedx/core/djangoapps/embargo/messages.py index 7cf0c11c80..ba5b0c0f32 100644 --- a/openedx/core/djangoapps/embargo/messages.py +++ b/openedx/core/djangoapps/embargo/messages.py @@ -4,6 +4,7 @@ These messages are displayed to users when they are blocked from either enrolling in or accessing a course. """ +from __future__ import absolute_import from collections import namedtuple BlockedMessage = namedtuple('BlockedMessage', [ diff --git a/openedx/core/djangoapps/embargo/middleware.py b/openedx/core/djangoapps/embargo/middleware.py index 5d4dbceee5..817726cae7 100644 --- a/openedx/core/djangoapps/embargo/middleware.py +++ b/openedx/core/djangoapps/embargo/middleware.py @@ -25,6 +25,7 @@ Usage: configure a whitelist or blacklist of countries for that course. """ +from __future__ import absolute_import import logging import re diff --git a/openedx/core/djangoapps/embargo/migrations/0001_initial.py b/openedx/core/djangoapps/embargo/migrations/0001_initial.py index 045aa06fb7..8c1a88411e 100644 --- a/openedx/core/djangoapps/embargo/migrations/0001_initial.py +++ b/openedx/core/djangoapps/embargo/migrations/0001_initial.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from django.db import migrations, models import django_countries.fields diff --git a/openedx/core/djangoapps/embargo/migrations/0002_data__add_countries.py b/openedx/core/djangoapps/embargo/migrations/0002_data__add_countries.py index 86ee543f9e..490320ace0 100644 --- a/openedx/core/djangoapps/embargo/migrations/0002_data__add_countries.py +++ b/openedx/core/djangoapps/embargo/migrations/0002_data__add_countries.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals # Converted from the original South migration 0003_add_countries.py diff --git a/openedx/core/djangoapps/embargo/models.py b/openedx/core/djangoapps/embargo/models.py index daa5b09f6f..0ef5149f6c 100644 --- a/openedx/core/djangoapps/embargo/models.py +++ b/openedx/core/djangoapps/embargo/models.py @@ -11,15 +11,18 @@ file and check it in at the same time as your model changes. To do that, 3. Add the migration file created in edx-platform/openedx/core/djangoapps/embargo/migrations/ """ +from __future__ import absolute_import + +import ipaddress import json import logging -import ipaddress +import six from config_models.models import ConfigurationModel from django.core.cache import cache -from django.urls import reverse from django.db import models from django.db.models.signals import post_delete, post_save +from django.urls import reverse from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_lazy from django_countries import countries @@ -122,12 +125,12 @@ class RestrictedCourse(models.Model): ENROLL_MSG_KEY_CHOICES = tuple([ (msg_key, msg.description) - for msg_key, msg in ENROLL_MESSAGES.iteritems() + for msg_key, msg in six.iteritems(ENROLL_MESSAGES) ]) COURSEWARE_MSG_KEY_CHOICES = tuple([ (msg_key, msg.description) - for msg_key, msg in COURSEWARE_MESSAGES.iteritems() + for msg_key, msg in six.iteritems(COURSEWARE_MESSAGES) ]) course_key = CourseKeyField( @@ -169,7 +172,7 @@ class RestrictedCourse(models.Model): Boolean True if course is in restricted course list. """ - return unicode(course_id) in cls._get_restricted_courses_from_cache() + return six.text_type(course_id) in cls._get_restricted_courses_from_cache() @classmethod def is_disabled_access_check(cls, course_id): @@ -187,8 +190,8 @@ class RestrictedCourse(models.Model): # checking is_restricted_course method also here to make sure course exists in the list otherwise in case of # no course found it will throw the key not found error on 'disable_access_check' return ( - cls.is_restricted_course(unicode(course_id)) - and cls._get_restricted_courses_from_cache().get(unicode(course_id))["disable_access_check"] + cls.is_restricted_course(six.text_type(course_id)) + and cls._get_restricted_courses_from_cache().get(six.text_type(course_id))["disable_access_check"] ) @classmethod @@ -199,7 +202,7 @@ class RestrictedCourse(models.Model): restricted_courses = cache.get(cls.COURSE_LIST_CACHE_KEY) if restricted_courses is None: restricted_courses = { - unicode(course.course_key): { + six.text_type(course.course_key): { 'disable_access_check': course.disable_access_check } for course in RestrictedCourse.objects.all() @@ -237,7 +240,7 @@ class RestrictedCourse(models.Model): 'access_msg': self.access_msg_key, 'country_rules': [ { - 'country': unicode(rule.country.country), + 'country': six.text_type(rule.country.country), 'rule_type': rule.rule_type } for rule in country_rules_for_course @@ -265,7 +268,7 @@ class RestrictedCourse(models.Model): return self.access_msg_key def __unicode__(self): - return unicode(self.course_key) + return six.text_type(self.course_key) @classmethod def message_url_path(cls, course_key, access_point): @@ -385,8 +388,8 @@ class Country(models.Model): def __unicode__(self): return u"{name} ({code})".format( - name=unicode(self.country.name), - code=unicode(self.country) + name=six.text_type(self.country.name), + code=six.text_type(self.country) ) class Meta(object): @@ -519,13 +522,13 @@ class CountryAccessRule(models.Model): def __unicode__(self): if self.rule_type == self.WHITELIST_RULE: return _(u"Whitelist {country} for {course}").format( - course=unicode(self.restricted_course.course_key), - country=unicode(self.country), + course=six.text_type(self.restricted_course.course_key), + country=six.text_type(self.country), ) elif self.rule_type == self.BLACKLIST_RULE: return _(u"Blacklist {country} for {course}").format( - course=unicode(self.restricted_course.course_key), - country=unicode(self.country), + course=six.text_type(self.restricted_course.course_key), + country=six.text_type(self.country), ) @classmethod diff --git a/openedx/core/djangoapps/embargo/test_utils.py b/openedx/core/djangoapps/embargo/test_utils.py index 10e95eaab3..551932302d 100644 --- a/openedx/core/djangoapps/embargo/test_utils.py +++ b/openedx/core/djangoapps/embargo/test_utils.py @@ -1,4 +1,5 @@ """Utilities for writing unit tests that involve course embargos. """ +from __future__ import absolute_import import contextlib import maxminddb diff --git a/openedx/core/djangoapps/embargo/tests/factories.py b/openedx/core/djangoapps/embargo/tests/factories.py index 5936226172..7398e1f3a4 100644 --- a/openedx/core/djangoapps/embargo/tests/factories.py +++ b/openedx/core/djangoapps/embargo/tests/factories.py @@ -1,3 +1,5 @@ +"""Factories for generating fake embargo data.""" +from __future__ import absolute_import import factory from factory.django import DjangoModelFactory from xmodule.modulestore.tests.factories import CourseFactory diff --git a/openedx/core/djangoapps/embargo/tests/test_api.py b/openedx/core/djangoapps/embargo/tests/test_api.py index dc0ab9d7f6..6ed4dc3f1f 100644 --- a/openedx/core/djangoapps/embargo/tests/test_api.py +++ b/openedx/core/djangoapps/embargo/tests/test_api.py @@ -1,6 +1,7 @@ """ Tests for EmbargoMiddleware """ +from __future__ import absolute_import from contextlib import contextmanager import geoip2.database diff --git a/openedx/core/djangoapps/embargo/tests/test_forms.py b/openedx/core/djangoapps/embargo/tests/test_forms.py index 26c1e23db4..59845239b9 100644 --- a/openedx/core/djangoapps/embargo/tests/test_forms.py +++ b/openedx/core/djangoapps/embargo/tests/test_forms.py @@ -3,18 +3,20 @@ Unit tests for embargo app admin forms. """ -from django.test import TestCase - -from opaque_keys.edx.locator import CourseLocator +from __future__ import absolute_import +import six # Explicitly import the cache from ConfigurationModel so we can reset it after each test from config_models.models import cache -from ..models import IPFilter -from ..forms import RestrictedCourseForm, IPFilterForm +from django.test import TestCase +from opaque_keys.edx.locator import CourseLocator from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from ..forms import IPFilterForm, RestrictedCourseForm +from ..models import IPFilter + class RestrictedCourseFormTest(ModuleStoreTestCase): """Test the course form properly validates course IDs""" @@ -22,7 +24,7 @@ class RestrictedCourseFormTest(ModuleStoreTestCase): def test_save_valid_data(self): course = CourseFactory.create() data = { - 'course_key': unicode(course.id), + 'course_key': six.text_type(course.id), 'enroll_msg_key': 'default', 'access_msg_key': 'default' } diff --git a/openedx/core/djangoapps/embargo/tests/test_middleware.py b/openedx/core/djangoapps/embargo/tests/test_middleware.py index 04e0882f54..c09c0f8460 100644 --- a/openedx/core/djangoapps/embargo/tests/test_middleware.py +++ b/openedx/core/djangoapps/embargo/tests/test_middleware.py @@ -2,21 +2,23 @@ Tests for EmbargoMiddleware with CountryAccessRules """ -from mock import patch -import ddt +from __future__ import absolute_import -from django.urls import reverse +import ddt +import six +from config_models.models import cache as config_cache from django.conf import settings from django.core.cache import cache as django_cache +from django.urls import reverse +from mock import patch -from config_models.models import cache as config_cache from openedx.core.djangolib.testing.utils import skip_unless_lms -from util.testing import UrlResetMixin from student.tests.factories import UserFactory -from xmodule.modulestore.tests.factories import CourseFactory +from util.testing import UrlResetMixin from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory -from ..models import RestrictedCourse, IPFilter +from ..models import IPFilter, RestrictedCourse from ..test_utils import restrict_course @@ -44,7 +46,7 @@ class EmbargoMiddlewareAccessTests(UrlResetMixin, ModuleStoreTestCase): self.courseware_url = reverse( 'openedx.course_experience.course_home', - kwargs={'course_id': unicode(self.course.id)} + kwargs={'course_id': six.text_type(self.course.id)} ) self.non_courseware_url = reverse('dashboard') diff --git a/openedx/core/djangoapps/embargo/tests/test_models.py b/openedx/core/djangoapps/embargo/tests/test_models.py index a9b175d66a..22f8c5b9c5 100644 --- a/openedx/core/djangoapps/embargo/tests/test_models.py +++ b/openedx/core/djangoapps/embargo/tests/test_models.py @@ -1,15 +1,25 @@ """Test of models for embargo app""" +from __future__ import absolute_import + import json -from django.test import TestCase + +import six from django.db.utils import IntegrityError +from django.test import TestCase from opaque_keys.edx.locator import CourseLocator -from ..models import ( - EmbargoedCourse, EmbargoedState, IPFilter, RestrictedCourse, - Country, CountryAccessRule, CourseAccessRuleHistory -) from openedx.core.djangolib.testing.utils import CacheIsolationTestCase +from ..models import ( + Country, + CountryAccessRule, + CourseAccessRuleHistory, + EmbargoedCourse, + EmbargoedState, + IPFilter, + RestrictedCourse +) + class EmbargoModelsTest(CacheIsolationTestCase): """Test each of the 3 models in embargo.models""" @@ -28,7 +38,7 @@ class EmbargoModelsTest(CacheIsolationTestCase): # Now, course should be embargoed self.assertTrue(EmbargoedCourse.is_embargoed(course_id)) self.assertEquals( - unicode(cauth), + six.text_type(cauth), u"Course '{course_id}' is Embargoed".format(course_id=course_id) ) @@ -38,7 +48,7 @@ class EmbargoModelsTest(CacheIsolationTestCase): # Test that course is now unauthorized self.assertFalse(EmbargoedCourse.is_embargoed(course_id)) self.assertEquals( - unicode(cauth), + six.text_type(cauth), u"Course '{course_id}' is Not Embargoed".format(course_id=course_id) ) @@ -115,8 +125,8 @@ class RestrictedCourseTest(CacheIsolationTestCase): course_id = CourseLocator('abc', '123', 'doremi') restricted_course = RestrictedCourse.objects.create(course_key=course_id) self.assertEquals( - unicode(restricted_course), - unicode(course_id) + six.text_type(restricted_course), + six.text_type(course_id) ) def test_restricted_course_cache_with_save_delete(self): @@ -166,7 +176,7 @@ class CountryTest(TestCase): def test_unicode_values(self): country = Country.objects.create(country='NZ') - self.assertEquals(unicode(country), "New Zealand (NZ)") + self.assertEquals(six.text_type(country), "New Zealand (NZ)") class CountryAccessRuleTest(CacheIsolationTestCase): @@ -184,7 +194,7 @@ class CountryAccessRuleTest(CacheIsolationTestCase): ) self.assertEquals( - unicode(access_rule), + six.text_type(access_rule), u"Whitelist New Zealand (NZ) for {course_key}".format(course_key=course_id) ) @@ -197,7 +207,7 @@ class CountryAccessRuleTest(CacheIsolationTestCase): ) self.assertEquals( - unicode(access_rule), + six.text_type(access_rule), u"Blacklist New Zealand (NZ) for {course_key}".format(course_key=course_id) ) diff --git a/openedx/core/djangoapps/embargo/tests/test_views.py b/openedx/core/djangoapps/embargo/tests/test_views.py index ac5333f8b7..ffa5b6699a 100644 --- a/openedx/core/djangoapps/embargo/tests/test_views.py +++ b/openedx/core/djangoapps/embargo/tests/test_views.py @@ -1,5 +1,6 @@ """Tests for embargo app views. """ +from __future__ import absolute_import import ddt import maxminddb import geoip2.database @@ -47,11 +48,11 @@ class CourseAccessMessageViewTest(CacheIsolationTestCase, UrlResetMixin): def setUp(self): super(CourseAccessMessageViewTest, self).setUp() - @ddt.data(*messages.ENROLL_MESSAGES.keys()) + @ddt.data(*list(messages.ENROLL_MESSAGES.keys())) def test_enrollment_messages(self, msg_key): self._load_page('enrollment', msg_key) - @ddt.data(*messages.COURSEWARE_MESSAGES.keys()) + @ddt.data(*list(messages.COURSEWARE_MESSAGES.keys())) def test_courseware_messages(self, msg_key): self._load_page('courseware', msg_key) diff --git a/openedx/core/djangoapps/embargo/urls.py b/openedx/core/djangoapps/embargo/urls.py index 83baab424f..180d6cd394 100644 --- a/openedx/core/djangoapps/embargo/urls.py +++ b/openedx/core/djangoapps/embargo/urls.py @@ -1,5 +1,6 @@ """URLs served by the embargo app. """ +from __future__ import absolute_import from django.conf.urls import url from .views import CheckCourseAccessView, CourseAccessMessageView diff --git a/openedx/core/djangoapps/embargo/views.py b/openedx/core/djangoapps/embargo/views.py index a38c1511e9..9402763987 100644 --- a/openedx/core/djangoapps/embargo/views.py +++ b/openedx/core/djangoapps/embargo/views.py @@ -1,5 +1,6 @@ """Views served by the embargo app. """ +from __future__ import absolute_import from django.contrib.auth.models import User from django.http import Http404 from django.views.generic.base import View