address feedback

edxval/admin.py

@@ -5,7 +5,7 @@
 from django.contrib import admin
 
 from .models import (CourseVideo, EncodedVideo, Profile, TranscriptPreference,
-                     Video, VideoImage, VideoTranscript)
+                     Video, VideoImage, VideoTranscript, OrganizationTranscriptCredentialsState)
 
 
 class ProfileAdmin(admin.ModelAdmin):  # pylint: disable=C0111
@@ -48,9 +48,16 @@ class CourseVideoAdmin(admin.ModelAdmin):
     verbose_name_plural = 'Course Videos'
 
 
+class OrganizationTranscriptCredentialsStateAdmin(admin.ModelAdmin):
+    model = OrganizationTranscriptCredentialsState
+    verbose_name = 'Organization Transcript Credential State'
+    verbose_name_plural = 'Organization Transcript Credentials State'
+
+
 admin.site.register(Profile, ProfileAdmin)
 admin.site.register(Video, VideoAdmin)
 admin.site.register(VideoTranscript)
 admin.site.register(TranscriptPreference)
 admin.site.register(VideoImage, VideoImageAdmin)
 admin.site.register(CourseVideo, CourseVideoAdmin)
+admin.site.register(OrganizationTranscriptCredentialsState, OrganizationTranscriptCredentialsStateAdmin)

edxval/api.py

@@ -182,7 +182,7 @@ def get_organization_transcript_credentials_state(org, provider=None):
 
 def update_organization_transcript_credentials_state(org, provider, exists):
     """
-    Update transcript credentials state for a course organization.
+    Updates transcript credentials state for a course organization.
 
     Arguments:
         org (unicode): course organization

edxval/migrations/0008_auto_20170920_0324.py → edxval/migrations/0008_auto_20170922_0238.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# Generated by Django 1.11.4 on 2017-09-20 07:24
+# Generated by Django 1.11.4 on 2017-09-22 06:38
 from __future__ import unicode_literals
 
 from django.db import migrations, models
@@ -16,8 +16,8 @@ class Migration(migrations.Migration):
             name='OrganizationTranscriptCredentialsState',
             fields=[
                 ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
-                ('org', models.CharField(max_length=255, verbose_name=b'Course Organization')),
-                ('provider', models.CharField(choices=[(b'Custom', b'Custom'), (b'3PlayMedia', b'3PlayMedia'), (b'Cielo24', b'Cielo24')], max_length=20, verbose_name=b'Provider')),
+                ('org', models.CharField(max_length=32, verbose_name=b'Course Organization')),
+                ('provider', models.CharField(choices=[(b'Custom', b'Custom'), (b'3PlayMedia', b'3PlayMedia'), (b'Cielo24', b'Cielo24')], max_length=20, verbose_name=b'Transcript Provider')),
                 ('exists', models.BooleanField(default=False, help_text=b'Transcript credentials state')),
             ],
         ),

edxval/models.py

@@ -619,9 +619,9 @@ class OrganizationTranscriptCredentialsState(models.Model):
     class Meta:
         unique_together = ('org', 'provider')
 
-    org = models.CharField(verbose_name='Course Organization', max_length=255)
+    org = models.CharField(verbose_name='Course Organization', max_length=32)
     provider = models.CharField(
-        verbose_name='Provider',
+        verbose_name='Transcript Provider',
         max_length=20,
         choices=TranscriptProviderType.CHOICES,
     )
@@ -640,15 +640,22 @@ def update_or_create(cls, org, provider, exists):
         return instance, created
 
     def __unicode__(self):
-        return u'OrganizationTranscriptCredentialsState(org={}, provider={}, exists={})'.format(
-            self.org, self.provider, self.exists
+        """
+        Returns unicode representation of provider credentials state for an organization.
+
+        NOTE: Message will look like below:
+            edX has Cielo24 credentials
+            edX has no 3PlayMedia credentials
+        """
+        return u'{org} {state} {provider} credentials'.format(
+            org=self.org, provider=self.provider, state='has' if self.exists else 'has no'
         )
 
 
 @receiver(models.signals.pre_save, sender=OrganizationTranscriptCredentialsState)
 def validate_provider(sender, instance, **kwargs):  # pylint: disable=unused-argument
     """
-    Validate that input provider value mus be one of TranscriptProviderType.CHOICES.
+    Validate that input provider value must be one of TranscriptProviderType.CHOICES.
     """
     if instance.provider not in dict(TranscriptProviderType.CHOICES).keys():
         raise ValidationError('Invalid transcript provider value')

edxval/tests/test_api.py

@@ -2063,12 +2063,6 @@ def setUp(self):
             org='edX', provider='3PlayMedia', exists=False
         )
 
-    def assert_credential_state():
-        """
-        Verify credential state.
-        """
-        pass
-
     @data(
         {'provider': ''},
         {'provider': 'fake'},
@@ -2087,6 +2081,7 @@ def test_provider_validation(self, provider):
     @data(
         {'org': 'MAX', 'provider': 'Cielo24', 'exists': True},
         {'org': 'MAX', 'provider': '3PlayMedia', 'exists': True},
+        {'org': 'edx', 'provider': '3PlayMedia', 'exists': True},
     )
     @unpack
     def test_credentials_state_update(self, **kwargs):
@@ -2102,30 +2097,41 @@ def test_credentials_state_update(self, **kwargs):
             self.assertEqual(getattr(credentials_state, key), kwargs[key])
 
     @data(
-        {'org': 'edX', 'provider': 'Cielo24'},
-        {'org': 'edX', 'provider': '3PlayMedia'},
+        {
+            'org': 'edX',
+            'provider': 'Cielo24',
+            'result': {
+                'org': 'edX',
+                'Cielo24': False,
+            }
+        },
+        {
+            'org': 'edX',
+            'provider': '3PlayMedia',
+            'result': {
+                'org': 'edX',
+                '3PlayMedia': False,
+            }
+        },
+        {
+            'org': 'edX',
+            'provider': None,
+            'result': {
+                'org': 'edX',
+                'Cielo24': False,
+                '3PlayMedia': False,
+            }
+        },
+        {
+            'org': 'does_not_exist',
+            'provider': None,
+            'result': None
+        },
     )
     @unpack
-    def test_get_credentials_state(self, org, provider):
+    def test_get_credentials_state(self, org, provider, result):
         """
         Verify that `get_organization_transcript_credentials_state` method works as expected
         """
         state_info = api.get_organization_transcript_credentials_state(org=org, provider=provider)
-        credentials_state = OrganizationTranscriptCredentialsState.objects.get(org=org, provider=provider)
-        self.assertEqual(state_info['org'], credentials_state.org)
-        self.assertEqual(state_info[provider], credentials_state.exists)
-
-    def test_get_credentials_state_with_org_only(self):
-        """
-        Verify that `get_organization_transcript_credentials_state`
-        method works as expected when pnly org is given
-        """
-        state_info = api.get_organization_transcript_credentials_state(org='edX')
-        self.assertDictEqual(
-            state_info,
-            {
-                'org': 'edX',
-                'Cielo24': False,
-                '3PlayMedia': False,
-            }
-        )
+        self.assertEqual(state_info, result)