Skip to content

Commit

Permalink
Merge pull request openedx#17133 from edx/jmbowman/PLAT-1873_2
Browse files Browse the repository at this point in the history
PLAT-1873 to_deprecated_string() cleanup part 2
  • Loading branch information
Jeremy Bowman authored Jan 10, 2018
2 parents c2094d4 + 669aa13 commit 4026c25
Show file tree
Hide file tree
Showing 68 changed files with 429 additions and 352 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def export_courses_to_output_path(output_path):
print("-" * 80)
print("Exporting course id = {0} to {1}".format(course_id, output_path))
try:
course_dir = course_id.to_deprecated_string().replace('/', '...')
course_dir = text_type(course_id).replace('/', '...')
export_course_to_xml(module_store, content_store, course_id, root_dir, course_dir)
except Exception as err: # pylint: disable=broad-except
failed_export_courses.append(text_type(course_id))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from student.models import anonymous_id_for_user
from opaque_keys.edx.keys import CourseKey
from six import text_type


class Command(BaseCommand):
Expand All @@ -34,12 +35,12 @@ def handle(self, *args, **options):

# Generate the output filename from the course ID.
# Change slashes to dashes first, and then append .csv extension.
output_filename = course_key.to_deprecated_string().replace('/', '-') + ".csv"
output_filename = text_type(course_key).replace('/', '-') + ".csv"

# Figure out which students are enrolled in the course
students = User.objects.filter(courseenrollment__course_id=course_key)
if len(students) == 0:
self.stdout.write("No students enrolled in %s" % course_key.to_deprecated_string())
self.stdout.write("No students enrolled in %s" % text_type(course_key))
return

# Write mapping to output file in CSV format with a simple header
Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/modulestore/mongo/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
'_id': <location.as_dict>,
'metadata': <dict containing all Scope.settings fields>
'definition': <dict containing all Scope.content fields>
'definition.children': <list of all child location.to_deprecated_string()s>
'definition.children': <list of all child text_type(location)s>
}
"""

Expand Down
3 changes: 2 additions & 1 deletion common/lib/xmodule/xmodule/modulestore/mongo/draft.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locations import Location
from six import text_type
from openedx.core.lib.cache_utils import memoize_in_request_cache
from xmodule.exceptions import InvalidVersionError
from xmodule.modulestore import ModuleStoreEnum
Expand Down Expand Up @@ -263,7 +264,7 @@ def _get_raw_parent_locations(self, location, key_revision):

# create a query to find all items in the course that have the given location listed as a child
query = self._course_key_to_son(location.course_key)
query['definition.children'] = location.to_deprecated_string()
query['definition.children'] = text_type(location)

# find all the items that satisfy the query
parents = self.collection.find(query, {'_id': True}, sort=[SORT_REVISION_FAVOR_DRAFT])
Expand Down
9 changes: 5 additions & 4 deletions common/lib/xmodule/xmodule/modulestore/mongoengine_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from opaque_keys.edx.locations import Location
from types import NoneType
from opaque_keys.edx.keys import CourseKey, UsageKey
from six import text_type


class CourseKeyField(mongoengine.StringField):
Expand All @@ -22,7 +23,7 @@ def to_mongo(self, course_key):
assert isinstance(course_key, (NoneType, CourseKey))
if course_key:
# don't call super as base.BaseField.to_mongo calls to_python() for some odd reason
return course_key.to_deprecated_string()
return text_type(course_key)
else:
return None

Expand All @@ -43,7 +44,7 @@ def to_python(self, course_key):
def validate(self, value):
assert isinstance(value, (NoneType, basestring, CourseKey))
if isinstance(value, CourseKey):
return super(CourseKeyField, self).validate(value.to_deprecated_string())
return super(CourseKeyField, self).validate(text_type(value))
else:
return super(CourseKeyField, self).validate(value)

Expand All @@ -62,7 +63,7 @@ def to_mongo(self, location):
assert isinstance(location, (NoneType, UsageKey))
if location is None:
return None
return super(UsageKeyField, self).to_mongo(location.to_deprecated_string())
return super(UsageKeyField, self).to_mongo(text_type(location))

def to_python(self, location):
"""
Expand All @@ -80,7 +81,7 @@ def to_python(self, location):
def validate(self, value):
assert isinstance(value, (NoneType, basestring, UsageKey))
if isinstance(value, UsageKey):
return super(UsageKeyField, self).validate(value.to_deprecated_string())
return super(UsageKeyField, self).validate(text_type(value))
else:
return super(UsageKeyField, self).validate(value)

Expand Down
4 changes: 2 additions & 2 deletions common/lib/xmodule/xmodule/modulestore/xml_exporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ def _export_drafts(modulestore, course_key, export_fs, xml_centric_course_key):
# if module has no parent, set its parent_url to `None`
parent_url = None
if parent_loc is not None:
parent_url = parent_loc.to_deprecated_string()
parent_url = text_type(parent_loc)

draft_node = draft_node_constructor(
draft_module,
location=draft_module.location,
url=draft_module.location.to_deprecated_string(),
url=text_type(draft_module.location),
parent_location=parent_loc,
parent_url=parent_url,
)
Expand Down
5 changes: 3 additions & 2 deletions common/lib/xmodule/xmodule/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from functools import wraps
from mock import Mock
from path import Path as path
from six import text_type

from opaque_keys.edx.keys import CourseKey
from xblock.field_data import DictFieldData
Expand Down Expand Up @@ -610,8 +611,8 @@ def assertAssetEqual(self, expected_course_key, expected_asset, actual_course_ke

expected_filename = expected_asset.pop('filename')
actual_filename = actual_asset.pop('filename')
self.assertEqual(expected_key.to_deprecated_string(), expected_filename)
self.assertEqual(actual_key.to_deprecated_string(), actual_filename)
self.assertEqual(text_type(expected_key), expected_filename)
self.assertEqual(text_type(actual_key), actual_filename)
self.assertEqual(expected_asset, actual_asset)

def _assertAssetsEqual(self, expected_course_key, expected_assets, actual_course_key, actual_assets): # pylint: disable=invalid-name
Expand Down
3 changes: 2 additions & 1 deletion common/lib/xmodule/xmodule/tests/test_conditional.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from fs.memoryfs import MemoryFS
from lxml import etree
from mock import Mock, patch
from six import text_type

from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
Expand Down Expand Up @@ -273,7 +274,7 @@ def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', cours
'conditional_ajax.html',
{
# Test ajax url is just usage-id / handler_name
'ajax_url': '{}/xmodule_handler'.format(location.to_deprecated_string()),
'ajax_url': '{}/xmodule_handler'.format(text_type(location)),
'element_id': u'i4x-HarvardX-ER22x-conditional-condone',
'depends': u'i4x-HarvardX-ER22x-problem-choiceprob'
}
Expand Down
7 changes: 4 additions & 3 deletions common/lib/xmodule/xmodule/tests/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from datetime import datetime, timedelta, tzinfo
from fs.osfs import OSFS
from path import Path as path
from six import text_type
from tempfile import mkdtemp
from textwrap import dedent

Expand All @@ -30,7 +31,7 @@ def strip_filenames(descriptor):
"""
Recursively strips 'filename' from all children's definitions.
"""
print "strip filename from {desc}".format(desc=descriptor.location.to_deprecated_string())
print "strip filename from {desc}".format(desc=text_type(descriptor.location))
if descriptor._field_data.has(descriptor, 'filename'):
descriptor._field_data.delete(descriptor, 'filename')

Expand Down Expand Up @@ -171,10 +172,10 @@ def utcoffset(self, _dt):

def test_encode_location(self):
loc = Location('org', 'course', 'run', 'category', 'name', None)
self.assertEqual(loc.to_deprecated_string(), self.encoder.default(loc))
self.assertEqual(text_type(loc), self.encoder.default(loc))

loc = Location('org', 'course', 'run', 'category', 'name', 'version')
self.assertEqual(loc.to_deprecated_string(), self.encoder.default(loc))
self.assertEqual(text_type(loc), self.encoder.default(loc))

def test_encode_naive_datetime(self):
self.assertEqual(
Expand Down
3 changes: 2 additions & 1 deletion common/lib/xmodule/xmodule/tests/test_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from mock import Mock, patch

from pytz import UTC
from six import text_type

from xmodule.xml_module import is_pointer_tag
from opaque_keys.edx.locations import Location
Expand Down Expand Up @@ -446,7 +447,7 @@ def test_metadata_inherit(self):

def check_for_key(key, node, value):
"recursive check for presence of key"
print "Checking {0}".format(node.location.to_deprecated_string())
print "Checking {0}".format(text_type(node.location))
self.assertEqual(getattr(node, key), value)
for c in node.get_children():
check_for_key(key, c, value)
Expand Down
9 changes: 5 additions & 4 deletions common/lib/xmodule/xmodule/tests/test_lti_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from lxml import etree
from webob.request import Request
from copy import copy
from six import text_type
import urllib

from xmodule.fields import Timedelta
Expand Down Expand Up @@ -276,7 +277,7 @@ def test_good_request(self):
self.assertEqual(self.xmodule.module_score, float(self.defaults['grade']))

def test_user_id(self):
expected_user_id = unicode(urllib.quote(self.xmodule.runtime.anonymous_student_id))
expected_user_id = text_type(urllib.quote(self.xmodule.runtime.anonymous_student_id))
real_user_id = self.xmodule.get_user_id()
self.assertEqual(real_user_id, expected_user_id)

Expand All @@ -295,13 +296,13 @@ def mock_handler_url(block, handler_name, **kwargs): # pylint: disable=unused-a
def test_resource_link_id(self):
with patch('xmodule.lti_module.LTIModule.location', new_callable=PropertyMock):
self.xmodule.location.html_id = lambda: 'i4x-2-3-lti-31de800015cf4afb973356dbe81496df'
expected_resource_link_id = unicode(urllib.quote(self.unquoted_resource_link_id))
expected_resource_link_id = text_type(urllib.quote(self.unquoted_resource_link_id))
real_resource_link_id = self.xmodule.get_resource_link_id()
self.assertEqual(real_resource_link_id, expected_resource_link_id)

def test_lis_result_sourcedid(self):
expected_sourced_id = u':'.join(urllib.quote(i) for i in (
self.system.course_id.to_deprecated_string(),
text_type(self.system.course_id),
self.xmodule.get_resource_link_id(),
self.user_id
))
Expand Down Expand Up @@ -520,4 +521,4 @@ def test_context_id(self):
"""
Tests that LTI parameter context_id is equal to course_id.
"""
self.assertEqual(self.system.course_id.to_deprecated_string(), self.xmodule.context_id)
self.assertEqual(text_type(self.system.course_id), self.xmodule.context_id)
5 changes: 3 additions & 2 deletions common/lib/xmodule/xmodule/tests/xml/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pprint
from lxml import etree
from mock import Mock
from six import text_type
from unittest import TestCase

from xmodule.x_module import XMLParsingSystem, policy_key
Expand Down Expand Up @@ -47,12 +48,12 @@ def process_xml(self, xml): # pylint: disable=method-hidden
None,
CourseLocationManager(self.course_id),
)
self._descriptors[descriptor.location.to_deprecated_string()] = descriptor
self._descriptors[text_type(descriptor.location)] = descriptor
return descriptor

def load_item(self, location, for_parent=None): # pylint: disable=method-hidden, unused-argument
"""Return the descriptor loaded for `location`"""
return self._descriptors[location.to_deprecated_string()]
return self._descriptors[text_type(location)]


class XModuleXmlImportTest(TestCase):
Expand Down
7 changes: 4 additions & 3 deletions lms/djangoapps/bulk_email/tests/test_err_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from mock import Mock, patch
from nose.plugins.attrib import attr
from opaque_keys.edx.locator import CourseLocator
from six import text_type

from bulk_email.models import SEND_TO_MYSELF, BulkEmailFlag, CourseEmail
from bulk_email.tasks import perform_delegate_email_batches, send_course_email
Expand Down Expand Up @@ -56,10 +57,10 @@ def setUp(self):

# load initial content (since we don't run migrations as part of tests):
call_command("loaddata", "course_email_template.json")
self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()})
self.send_mail_url = reverse('send_email', kwargs={'course_id': self.course.id.to_deprecated_string()})
self.url = reverse('instructor_dashboard', kwargs={'course_id': text_type(self.course.id)})
self.send_mail_url = reverse('send_email', kwargs={'course_id': text_type(self.course.id)})
self.success_content = {
'course_id': self.course.id.to_deprecated_string(),
'course_id': text_type(self.course.id),
'success': True,
}

Expand Down
11 changes: 6 additions & 5 deletions lms/djangoapps/bulk_email/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from nose.plugins.attrib import attr
from opaque_keys.edx.locator import CourseLocator
from six import text_type

from bulk_email.forms import CourseAuthorizationAdminForm, CourseEmailTemplateForm
from bulk_email.models import BulkEmailFlag, CourseEmailTemplate
Expand All @@ -30,7 +31,7 @@ def test_authorize_mongo_course(self):
# Initially course shouldn't be authorized
self.assertFalse(BulkEmailFlag.feature_enabled(self.course.id))
# Test authorizing the course, which should totally work
form_data = {'course_id': self.course.id.to_deprecated_string(), 'email_enabled': True}
form_data = {'course_id': text_type(self.course.id), 'email_enabled': True}
form = CourseAuthorizationAdminForm(data=form_data)
# Validation should work
self.assertTrue(form.is_valid())
Expand All @@ -42,7 +43,7 @@ def test_repeat_course(self):
# Initially course shouldn't be authorized
self.assertFalse(BulkEmailFlag.feature_enabled(self.course.id))
# Test authorizing the course, which should totally work
form_data = {'course_id': self.course.id.to_deprecated_string(), 'email_enabled': True}
form_data = {'course_id': text_type(self.course.id), 'email_enabled': True}
form = CourseAuthorizationAdminForm(data=form_data)
# Validation should work
self.assertTrue(form.is_valid())
Expand All @@ -51,7 +52,7 @@ def test_repeat_course(self):
self.assertTrue(BulkEmailFlag.feature_enabled(self.course.id))

# Now make a new course authorization with the same course id that tries to turn email off
form_data = {'course_id': self.course.id.to_deprecated_string(), 'email_enabled': False}
form_data = {'course_id': text_type(self.course.id), 'email_enabled': False}
form = CourseAuthorizationAdminForm(data=form_data)
# Validation should not work because course_id field is unique
self.assertFalse(form.is_valid())
Expand All @@ -72,13 +73,13 @@ def test_form_typo(self):
# Munge course id
bad_id = CourseLocator(u'Broken{}'.format(self.course.id.org), 'hello', self.course.id.run + '_typo')

form_data = {'course_id': bad_id.to_deprecated_string(), 'email_enabled': True}
form_data = {'course_id': text_type(bad_id), 'email_enabled': True}
form = CourseAuthorizationAdminForm(data=form_data)
# Validation shouldn't work
self.assertFalse(form.is_valid())

msg = u'COURSE NOT FOUND'
msg += u' --- Entered course id was: "{0}". '.format(bad_id.to_deprecated_string())
msg += u' --- Entered course id was: "{0}". '.format(text_type(bad_id))
msg += 'Please recheck that you have supplied a valid course id.'
self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.core.management.base import BaseCommand, CommandError
from django.db.models import Count
from opaque_keys.edx.keys import CourseKey
from six import text_type

from certificates.models import GeneratedCertificate

Expand Down Expand Up @@ -118,7 +119,7 @@ def handle(self, *args, **options):
print ' '.join(["{:>16}".format(heading) for heading in status_headings])

# print the report
print "{0:>26}".format(course_id.to_deprecated_string()),
print "{0:>26}".format(text_type(course_id)),
for heading in status_headings:
if heading in cert_data[course_id]:
print "{:>16}".format(cert_data[course_id][heading]),
Expand Down
Loading

0 comments on commit 4026c25

Please sign in to comment.