Merge pull request #234 from edx/clintonb/organization

Updated Organization model and loader

course_discovery/apps/api/serializers.py

@@ -157,11 +157,10 @@ class PersonSerializer(serializers.ModelSerializer):
 
 class OrganizationSerializer(serializers.ModelSerializer):
     """Serializer for the ``Organization`` model."""
-    logo_image = ImageSerializer()
 
     class Meta(object):
         model = Organization
-        fields = ('key', 'name', 'description', 'logo_image', 'homepage_url',)
+        fields = ('key', 'name', 'description', 'homepage_url',)
 
 
 class CatalogSerializer(serializers.ModelSerializer):

course_discovery/apps/api/tests/test_serializers.py

@@ -387,7 +387,6 @@ class VideoSerializerTests(TestCase):
 class OrganizationSerializerTests(TestCase):
     def test_data(self):
         organization = OrganizationFactory()
-        image = organization.logo_image
         serializer = OrganizationSerializer(organization)
 
         expected = {
@@ -395,12 +394,6 @@ class OrganizationSerializerTests(TestCase):
             'name': organization.name,
             'description': organization.description,
             'homepage_url': organization.homepage_url,
-            'logo_image': {
-                'src': image.src,
-                'description': image.description,
-                'height': image.height,
-                'width': image.width
-            }
         }
 
         self.assertDictEqual(serializer.data, expected)

course_discovery/apps/course_metadata/admin.py

 from django.contrib import admin
+from simple_history.admin import SimpleHistoryAdmin
 
-from course_discovery.apps.course_metadata.models import (
-    Seat, Image, Video, LevelType, Subject, Prerequisite, ExpectedLearningItem, Expertise, Course, CourseRun,
-    CourseRunSocialNetwork, MajorWork, Organization, Person, PersonSocialNetwork, CourseOrganization, SyllabusItem,
-    Program, JobOutlookItem, SeatType, Endorsement, CorporateEndorsement, FAQ, ProgramType)
+from course_discovery.apps.course_metadata.models import *  # pylint: disable=wildcard-import
 
 
 class CourseOrganizationInline(admin.TabularInline):
@@ -69,9 +67,11 @@ class FAQAdmin(admin.ModelAdmin):
 
 
 @admin.register(Organization)
-class OrganizationAdmin(admin.ModelAdmin):
-    list_display = ('key', 'name',)
+class OrganizationAdmin(SimpleHistoryAdmin):
+    list_display = ('uuid', 'key', 'name',)
     list_filter = ('partner',)
+    readonly_fields = ('uuid',)
+    search_fields = ('uuid', 'name', 'key',)
 
 
 @admin.register(Subject)

course_discovery/apps/course_metadata/data_loaders/api.py

@@ -6,8 +6,8 @@ from opaque_keys.edx.keys import CourseKey
 from course_discovery.apps.core.models import Currency
 from course_discovery.apps.course_metadata.data_loaders import AbstractDataLoader
 from course_discovery.apps.course_metadata.models import (
-    Image, Video, Organization, Seat, CourseRun, Program, Course, CourseOrganization,
-    ProgramType)
+    Video, Organization, Seat, CourseRun, Program, Course, CourseOrganization, ProgramType,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -41,18 +41,14 @@ class OrganizationsApiDataLoader(AbstractDataLoader):
         self.delete_orphans()
 
     def update_organization(self, body):
-        image = None
-        image_url = body['logo']
-        if image_url:
-            image, __ = Image.objects.get_or_create(src=image_url)
         defaults = {
             'name': body['name'],
             'description': body['description'],
-            'logo_image': image,
+            'logo_image_url': body['logo'],
             'partner': self.partner,
         }
         Organization.objects.update_or_create(key=body['short_name'], defaults=defaults)
-        logger.info('Created/updated organization "%s"', body['short_name'])
+        logger.info('Processed organization "%s"', body['short_name'])
 
 
 class CoursesApiDataLoader(AbstractDataLoader):

course_discovery/apps/course_metadata/data_loaders/marketing_site.py

@@ -84,10 +84,9 @@ class DrupalApiDataLoader(AbstractDataLoader):
         """Update `course` with sponsors from `body`."""
         course.courseorganization_set.filter(relation_type=CourseOrganization.SPONSOR).delete()
         for sponsor_body in body['sponsors']:
-            image, __ = Image.objects.get_or_create(src=sponsor_body['image'])
             defaults = {
                 'name': sponsor_body['title'],
-                'logo_image': image,
+                'logo_image_url': sponsor_body['image'],
                 'homepage_url': urljoin(self.partner.marketing_site_url_root, sponsor_body['uri']),
             }
             organization, __ = Organization.objects.update_or_create(key=sponsor_body['uuid'], defaults=defaults)

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

@@ -14,7 +14,7 @@ from course_discovery.apps.course_metadata.data_loaders.api import (
 from course_discovery.apps.course_metadata.data_loaders.tests import JSON
 from course_discovery.apps.course_metadata.data_loaders.tests.mixins import ApiClientTestMixin, DataLoaderTestMixin
 from course_discovery.apps.course_metadata.models import (
-    Course, CourseRun, Image, Organization, Seat, Program, ProgramType,
+    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 (
@@ -83,13 +83,7 @@ class OrganizationsApiDataLoaderTests(ApiClientTestMixin, DataLoaderTestMixin, T
         organization = Organization.objects.get(key=AbstractDataLoader.clean_string(body['short_name']))
         self.assertEqual(organization.name, AbstractDataLoader.clean_string(body['name']))
         self.assertEqual(organization.description, AbstractDataLoader.clean_string(body['description']))
-
-        image = None
-        image_url = AbstractDataLoader.clean_string(body['logo'])
-        if image_url:
-            image = Image.objects.get(src=image_url)
-
-        self.assertEqual(organization.logo_image, image)
+        self.assertEqual(organization.logo_image_url, AbstractDataLoader.clean_string(body['logo']))
 
     @responses.activate
     def test_ingest(self):

course_discovery/apps/course_metadata/migrations/0013_auto_20160809_1259.py

@@ -82,7 +82,7 @@ class Migration(migrations.Migration):
             name='subject',
             unique_together=set([('partner', 'name'), ('partner', 'slug'), ('partner', 'uuid')]),
         ),
-        migrations.RunPython(update_subjects, lambda *args: None),
+        migrations.RunPython(update_subjects, reverse_code=migrations.RunPython.noop),
         migrations.AlterField(
             model_name='subject',
             name='slug',

course_discovery/apps/course_metadata/migrations/0014_auto_20160811_0436.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+import uuid
+
+from django.db import migrations, models
+
+
+def update_organizations(apps, schema_editor):
+    Organization = apps.get_model('course_metadata', 'Organization')
+    HistoricalOrganization = apps.get_model('course_metadata', 'HistoricalOrganization')
+
+    # Clear history to avoid null constraint issues
+    HistoricalOrganization.objects.all().delete()
+
+    for organization in Organization.objects.all():
+        organization.name = organization.name or organization.key
+        organization.uuid = uuid.uuid4()
+        organization.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('course_metadata', '0013_auto_20160809_1259'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='organization',
+            options={},
+        ),
+        migrations.RemoveField(
+            model_name='historicalorganization',
+            name='banner_image',
+        ),
+        migrations.RemoveField(
+            model_name='historicalorganization',
+            name='logo_image',
+        ),
+        migrations.AddField(
+            model_name='historicalorganization',
+            name='banner_image_url',
+            field=models.URLField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='historicalorganization',
+            name='logo_image_url',
+            field=models.URLField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='historicalorganization',
+            name='uuid',
+            field=models.UUIDField(default=uuid.uuid4, editable=False, verbose_name='UUID'),
+        ),
+        migrations.AddField(
+            model_name='organization',
+            name='banner_image_url',
+            field=models.URLField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='organization',
+            name='logo_image_url',
+            field=models.URLField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='organization',
+            name='uuid',
+            field=models.UUIDField(default=uuid.uuid4, editable=False, verbose_name='UUID'),
+        ),
+        migrations.AlterField(
+            model_name='historicalorganization',
+            name='key',
+            field=models.CharField(max_length=255),
+        ),
+        migrations.AlterField(
+            model_name='organization',
+            name='key',
+            field=models.CharField(max_length=255),
+        ),
+        migrations.RemoveField(
+            model_name='organization',
+            name='banner_image',
+        ),
+        migrations.RemoveField(
+            model_name='organization',
+            name='logo_image',
+        ),
+        migrations.RunPython(update_organizations, reverse_code=migrations.RunPython.noop),
+        migrations.AlterUniqueTogether(
+            name='organization',
+            unique_together=set([('partner', 'uuid'), ('partner', 'key')]),
+        ),
+        migrations.AlterField(
+            model_name='historicalorganization',
+            name='name',
+            field=models.CharField(max_length=255),
+        ),
+        migrations.AlterField(
+            model_name='organization',
+            name='name',
+            field=models.CharField(max_length=255),
+        ),
+    ]

course_discovery/apps/course_metadata/models.py

@@ -152,18 +152,22 @@ class MajorWork(AbstractNamedModel):
 
 class Organization(TimeStampedModel):
     """ Organization model. """
-    key = models.CharField(max_length=255, unique=True)
-    name = models.CharField(max_length=255, null=True, blank=True)
+    partner = models.ForeignKey(Partner, null=True, blank=False)
+    uuid = models.UUIDField(blank=False, null=False, default=uuid4, editable=False, verbose_name=_('UUID'))
+    key = models.CharField(max_length=255)
+    name = models.CharField(max_length=255)
     description = models.TextField(null=True, blank=True)
     homepage_url = models.URLField(max_length=255, null=True, blank=True)
-    # NOTE (CCB): The related_name values are here to prevent the images from being treated as orphans.
-    logo_image = models.ForeignKey(Image, null=True, blank=True, related_name='logoed_organizations')
-    banner_image = models.ForeignKey(Image, null=True, blank=True, related_name='bannered_organizations')
-
-    partner = models.ForeignKey(Partner, null=True, blank=False)
-
+    logo_image_url = models.URLField(null=True, blank=True)
+    banner_image_url = models.URLField(null=True, blank=True)
     history = HistoricalRecords()
 
+    class Meta:
+        unique_together = (
+            ('partner', 'key'),
+            ('partner', 'uuid'),
+        )
+
     def __str__(self):
         return '{key}: {name}'.format(key=self.key, name=self.name)
 

course_discovery/apps/course_metadata/tests/factories.py

@@ -137,7 +137,8 @@ class OrganizationFactory(factory.DjangoModelFactory):
     name = FuzzyText()
     description = FuzzyText()
     homepage_url = FuzzyURL()
-    logo_image = factory.SubFactory(ImageFactory)
+    logo_image_url = FuzzyURL()
+    banner_image_url = FuzzyURL()
     partner = factory.SubFactory(PartnerFactory)
 
     class Meta:

course_discovery/apps/edx_catalog_extensions/migrations/0002_convert_program_category_to_type.py

@@ -18,5 +18,5 @@ class Migration(migrations.Migration):
     ]
 
     operations = [
-        migrations.RunPython(set_program_types, lambda *args: None)
+        migrations.RunPython(set_program_types, reverse_code=migrations.RunPython.noop)
     ]