Merge pull request #261 from edx/clintonb/code-cleanup

Code Cleanup

course_discovery/apps/course_metadata/data_loaders/tests/test_api.py

@@ -11,12 +11,11 @@ from course_discovery.apps.core.tests.utils import mock_api_callback, mock_jpeg_
 from course_discovery.apps.course_metadata.data_loaders.api import (
     OrganizationsApiDataLoader, CoursesApiDataLoader, EcommerceApiDataLoader, AbstractDataLoader, ProgramsApiDataLoader
 )
-from course_discovery.apps.course_metadata.data_loaders.tests import JSON, JPEG
+from course_discovery.apps.course_metadata.data_loaders.tests import JSON, JPEG, mock_data
 from course_discovery.apps.course_metadata.data_loaders.tests.mixins import ApiClientTestMixin, DataLoaderTestMixin
 from course_discovery.apps.course_metadata.models import (
     Course, CourseRun, Organization, Seat, Program, ProgramType,
 )
-from course_discovery.apps.course_metadata.tests import mock_data
 from course_discovery.apps.course_metadata.tests.factories import (
     CourseRunFactory, SeatFactory, ImageFactory, VideoFactory, OrganizationFactory, CourseFactory,
 )

course_discovery/apps/course_metadata/data_loaders/tests/test_marketing_site.py

@@ -12,13 +12,12 @@ from course_discovery.apps.course_metadata.data_loaders.marketing_site import (
     DrupalApiDataLoader, XSeriesMarketingSiteDataLoader, SubjectMarketingSiteDataLoader, SchoolMarketingSiteDataLoader,
     SponsorMarketingSiteDataLoader, PersonMarketingSiteDataLoader,
 )
-from course_discovery.apps.course_metadata.data_loaders.tests import JSON
+from course_discovery.apps.course_metadata.data_loaders.tests import JSON, mock_data
 from course_discovery.apps.course_metadata.data_loaders.tests.mixins import ApiClientTestMixin, DataLoaderTestMixin
 from course_discovery.apps.course_metadata.models import (
     Course, CourseOrganization, CourseRun, Organization, Subject, Program, Video, Person,
 )
-from course_discovery.apps.course_metadata.tests import mock_data
-from course_discovery.apps.course_metadata.tests.factories import ProgramFactory, OrganizationFactory
+from course_discovery.apps.course_metadata.tests import factories
 from course_discovery.apps.ietf_language_tags.models import LanguageTag
 
 ENGLISH_LANGUAGE_TAG = LanguageTag(code='en-us', name='English - United States')
@@ -212,6 +211,8 @@ class DrupalApiDataLoaderTests(ApiClientTestMixin, DataLoaderTestMixin, TestCase
 
 
 class AbstractMarketingSiteDataLoaderTestMixin(DataLoaderTestMixin):
+    mocked_data = []
+
     @property
     def api_url(self):
         return self.partner.marketing_site_url_root
@@ -241,6 +242,19 @@ class AbstractMarketingSiteDataLoaderTestMixin(DataLoaderTestMixin):
 
         return request_callback
 
+    def mock_api(self):
+        bodies = self.mocked_data
+        url = self.api_url + 'node.json'
+
+        responses.add_callback(
+            responses.GET,
+            url,
+            callback=self.mock_api_callback(url, bodies),
+            content_type=JSON
+        )
+
+        return bodies
+
     def mock_login_response(self, failure=False):
         url = self.api_url + 'user'
         landing_url = '{base}users/{username}'.format(base=self.api_url,
@@ -293,24 +307,16 @@ class AbstractMarketingSiteDataLoaderTestMixin(DataLoaderTestMixin):
 
 class XSeriesMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMixin, TestCase):
     loader_class = XSeriesMarketingSiteDataLoader
+    mocked_data = mock_data.MARKETING_SITE_API_XSERIES_BODIES
 
     def create_mock_programs(self, programs):
         for program in programs:
             marketing_slug = program['url'].split('/')[-1]
-            ProgramFactory(marketing_slug=marketing_slug, partner=self.partner)
+            factories.ProgramFactory(marketing_slug=marketing_slug, partner=self.partner)
 
     def mock_api(self):
-        bodies = mock_data.MARKETING_SITE_API_XSERIES_BODIES
+        bodies = super().mock_api()
         self.create_mock_programs(bodies)
-        url = self.api_url + 'node.json'
-
-        responses.add_callback(
-            responses.GET,
-            url,
-            callback=self.mock_api_callback(url, bodies),
-            content_type=JSON
-        )
-
         return bodies
 
     def assert_program_loaded(self, data):
@@ -362,19 +368,7 @@ class XSeriesMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMix
 
 class SubjectMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMixin, TestCase):
     loader_class = SubjectMarketingSiteDataLoader
-
-    def mock_api(self):
-        bodies = mock_data.MARKETING_SITE_API_SUBJECT_BODIES
-        url = self.api_url + 'node.json'
-
-        responses.add_callback(
-            responses.GET,
-            url,
-            callback=self.mock_api_callback(url, bodies),
-            content_type=JSON
-        )
-
-        return bodies
+    mocked_data = mock_data.MARKETING_SITE_API_SUBJECT_BODIES
 
     def assert_subject_loaded(self, data):
         slug = data['field_subject_url_slug']
@@ -404,19 +398,7 @@ class SubjectMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMix
 
 class SchoolMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMixin, TestCase):
     loader_class = SchoolMarketingSiteDataLoader
-
-    def mock_api(self):
-        bodies = mock_data.MARKETING_SITE_API_SCHOOL_BODIES
-        url = self.api_url + 'node.json'
-
-        responses.add_callback(
-            responses.GET,
-            url,
-            callback=self.mock_api_callback(url, bodies),
-            content_type=JSON
-        )
-
-        return bodies
+    mocked_data = mock_data.MARKETING_SITE_API_SCHOOL_BODIES
 
     def assert_school_loaded(self, data):
         key = data['title']
@@ -448,19 +430,7 @@ class SchoolMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMixi
 
 class SponsorMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMixin, TestCase):
     loader_class = SponsorMarketingSiteDataLoader
-
-    def mock_api(self):
-        bodies = mock_data.MARKETING_SITE_API_SPONSOR_BODIES
-        url = self.api_url + 'node.json'
-
-        responses.add_callback(
-            responses.GET,
-            url,
-            callback=self.mock_api_callback(url, bodies),
-            content_type=JSON
-        )
-
-        return bodies
+    mocked_data = mock_data.MARKETING_SITE_API_SPONSOR_BODIES
 
     def assert_sponsor_loaded(self, data):
         uuid = data['uuid']
@@ -494,19 +464,7 @@ class SponsorMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMix
 
 class PersonMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMixin, TestCase):
     loader_class = PersonMarketingSiteDataLoader
-
-    def mock_api(self):
-        bodies = mock_data.MARKETING_SITE_API_PERSON_BODIES
-        url = self.api_url + 'node.json'
-
-        responses.add_callback(
-            responses.GET,
-            url,
-            callback=self.mock_api_callback(url, bodies),
-            content_type=JSON
-        )
-
-        return bodies
+    mocked_data = mock_data.MARKETING_SITE_API_PERSON_BODIES
 
     def assert_person_loaded(self, data):
         uuid = data['uuid']
@@ -537,7 +495,7 @@ class PersonMarketingSiteDataLoaderTests(AbstractMarketingSiteDataLoaderTestMixi
     def test_ingest(self):
         self.mock_login_response()
         people = self.mock_api()
-        OrganizationFactory(name='MIT')
+        factories.OrganizationFactory(name='MIT')
 
         self.loader.ingest()
 

course_discovery/apps/course_metadata/management/commands/tests/test_refresh_course_metadata.py

@@ -13,8 +13,8 @@ from course_discovery.apps.course_metadata.data_loaders.api import (
 from course_discovery.apps.course_metadata.data_loaders.marketing_site import (
     DrupalApiDataLoader, XSeriesMarketingSiteDataLoader,
 )
+from course_discovery.apps.course_metadata.data_loaders.tests import mock_data
 from course_discovery.apps.course_metadata.models import Course, CourseRun, Organization, Program
-from course_discovery.apps.course_metadata.tests import mock_data
 
 ACCESS_TOKEN = 'secret'
 JSON = 'application/json'

course_discovery/apps/course_metadata/tests/factories.py

@@ -12,6 +12,13 @@ from course_discovery.apps.ietf_language_tags.models import LanguageTag
 
 # pylint: disable=no-member, unused-argument
 
+def add_m2m_data(m2m_relation, data):
+    """ Helper function to enable factories to easily associate many-to-many data with created objects. """
+    if data:
+        for datum in data:
+            m2m_relation.add(datum)
+
+
 class AbstractMediaModelFactory(factory.DjangoModelFactory):
     src = FuzzyURL()
     description = FuzzyText()
@@ -83,13 +90,8 @@ class CourseFactory(factory.DjangoModelFactory):
 
     @factory.post_generation
     def subjects(self, create, extracted, **kwargs):
-        if not create:  # pragma: no cover
-            # Simple build, do nothing.
-            return
-
-        if extracted:
-            for subject in extracted:
-                self.subjects.add(subject)
+        if create:  # pragma: no cover
+            add_m2m_data(self.subjects, extracted)
 
 
 class CourseRunFactory(factory.DjangoModelFactory):
@@ -116,16 +118,12 @@ class CourseRunFactory(factory.DjangoModelFactory):
 
     @factory.post_generation
     def transcript_languages(self, create, extracted, **kwargs):
-        if not create:  # pragma: no cover
-            # Simple build, do nothing.
-            return
-
-        if extracted:
-            for transcript_language in extracted:
-                self.transcript_languages.add(transcript_language)
+        if create:  # pragma: no cover
+            add_m2m_data(self.transcript_languages, extracted)
 
 
 class OrganizationFactory(factory.DjangoModelFactory):
+    uuid = factory.LazyFunction(uuid4)
     key = FuzzyText(prefix='Org.fake/')
     name = FuzzyText()
     description = FuzzyText()
@@ -167,13 +165,8 @@ class ProgramTypeFactory(factory.django.DjangoModelFactory):
 
     @factory.post_generation
     def applicable_seat_types(self, create, extracted, **kwargs):
-        if not create:  # pragma: no cover
-            # Simple build, do nothing.
-            return
-
-        if extracted:
-            for seat_type in extracted:
-                self.applicable_seat_types.add(seat_type)
+        if create:  # pragma: no cover
+            add_m2m_data(self.applicable_seat_types, extracted)
 
 
 class ProgramFactory(factory.django.DjangoModelFactory):
@@ -192,34 +185,18 @@ class ProgramFactory(factory.django.DjangoModelFactory):
 
     @factory.post_generation
     def courses(self, create, extracted, **kwargs):
-        if not create:  # pragma: no cover
-            # Simple build, do nothing.
-            return
-
-        if extracted:
-            # Use the passed in list of courses
-            for course in extracted:
-                self.courses.add(course)
+        if create:  # pragma: no cover
+            add_m2m_data(self.courses, extracted)
 
     @factory.post_generation
     def excluded_course_runs(self, create, extracted, **kwargs):
-        if not create:  # pragma: no cover
-            # Simple build, do nothing.
-            return
-
-        if extracted:
-            for course_run in extracted:
-                self.excluded_course_runs.add(course_run)
+        if create:  # pragma: no cover
+            add_m2m_data(self.excluded_course_runs, extracted)
 
     @factory.post_generation
     def authoring_organizations(self, create, extracted, **kwargs):
-        if not create:  # pragma: no cover
-            # Simple build, do nothing.
-            return
-
-        if extracted:
-            for organization in extracted:
-                self.authoring_organizations.add(organization)
+        if create:  # pragma: no cover
+            add_m2m_data(self.authoring_organizations, extracted)
 
 
 class AbstractSocialNetworkModelFactory(factory.DjangoModelFactory):