Merge 4a32dc1 into 1442bfd

.coveragerc

@@ -3,3 +3,7 @@ branch = True
 source = organizations
 omit =
     **/migrations/*
+
+[report]
+exclude_lines =
+    pragma: no cover

manage.py

@@ -0,0 +1,29 @@
+#!/usr/bin/env python
+"""
+Django administration utility.
+"""
+
+import os
+import sys
+
+PWD = os.path.abspath(os.path.dirname(__file__))
+
+if __name__ == '__main__':
+    os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'test_settings')
+    sys.path.append(PWD)
+    try:
+        from django.core.management import execute_from_command_line  # pylint: disable=wrong-import-position
+    except ImportError:
+        # The above import may fail for some other reason. Ensure that the
+        # issue is really that Django is missing to avoid masking other
+        # exceptions on Python 2.
+        try:
+            import django  # pylint: disable=unused-import, wrong-import-position
+        except ImportError:
+            raise ImportError(
+                "Couldn't import Django. Are you sure it's installed and "
+                "available on your PYTHONPATH environment variable? Did you "
+                "forget to activate a virtual environment?"
+            )
+        raise
+    execute_from_command_line(sys.argv)

organizations/__init__.py

@@ -1,4 +1,4 @@
 """
 edx-organizations app initialization module
 """
-__version__ = '5.3.0'  # pragma: no cover
+__version__ = '6.0.0'  # pragma: no cover

organizations/admin.py

@@ -19,7 +19,7 @@ def get_actions(self, request):
         actions = super(OrganizationAdmin, self).get_actions(request)
 
         # Remove the delete action.
-        if 'delete_selected' in actions:
+        if 'delete_selected' in actions:  # pragma: no-cover
             del actions['delete_selected']
 
         return actions
@@ -63,7 +63,7 @@ class OrganizationCourseAdmin(admin.ModelAdmin):
 
     def formfield_for_foreignkey(self, db_field, request=None, **kwargs):
         # Only display active Organizations.
-        if db_field.name == 'organization':
+        if db_field.name == 'organization':  # pragma: no-branch
             kwargs['queryset'] = Organization.objects.filter(active=True).order_by('name')
 
         return super(OrganizationCourseAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs)

organizations/data.py

@@ -100,26 +100,27 @@ def create_organization(organization):
     """
     Inserts a new organization into app/local state given the following dictionary:
     {
+        'short_name': string,
         'name': string,
-        'description': string
+        'description': string (optional),
+        'logo': string (optional),
     }
     Returns an updated dictionary including a new 'id': integer field/value
     """
-    # Trust, but verify...
-    if not organization.get('name'):
+    if not (organization.get('name') and organization.get('short_name')):
         exceptions.raise_exception("organization", organization, exceptions.InvalidOrganizationException)
     organization_obj = serializers.deserialize_organization(organization)
     try:
         organization = internal.Organization.objects.get(
-            name=organization_obj.name,
+            short_name=organization_obj.short_name,
         )
         # If the organization exists, but was inactivated, we can simply turn it back on
         if not organization.active:
             _activate_organization(organization.id)
     except internal.Organization.DoesNotExist:
         organization = internal.Organization.objects.create(
-            name=organization_obj.name,
             short_name=organization_obj.short_name,
+            name=organization_obj.name,
             description=organization_obj.description,
             logo=organization_obj.logo,
             active=True

organizations/migrations/0002_unique_short_name.py

@@ -0,0 +1,23 @@
+# Generated by Django 2.2.16 on 2020-11-09 16:10
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('organizations', '0001_squashed_0007_historicalorganization'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='historicalorganization',
+            name='short_name',
+            field=models.CharField(db_index=True, help_text='Unique, short string identifier for organization. Please do not use spaces or special characters. Only allowed special characters are period (.), hyphen (-) and underscore (_).', max_length=255, verbose_name='Short Name'),
+        ),
+        migrations.AlterField(
+            model_name='organization',
+            name='short_name',
+            field=models.CharField(help_text='Unique, short string identifier for organization. Please do not use spaces or special characters. Only allowed special characters are period (.), hyphen (-) and underscore (_).', max_length=255, unique=True, verbose_name='Short Name'),
+        ),
+    ]

organizations/models.py

@@ -20,9 +20,14 @@ class Organization(TimeStampedModel):
     """
     name = models.CharField(max_length=255, db_index=True)
     short_name = models.CharField(
-        max_length=255, db_index=True, verbose_name=u'Short Name',
-        help_text=_('Please do not use spaces or special characters. Only allowed special characters '
-                    'are period (.), hyphen (-) and underscore (_).')
+        max_length=255,
+        unique=True,
+        verbose_name=u'Short Name',
+        help_text=_(
+            'Unique, short string identifier for organization. '
+            'Please do not use spaces or special characters. '
+            'Only allowed special characters are period (.), hyphen (-) and underscore (_).'
+        ),
     )
     description = models.TextField(null=True, blank=True)
     logo = models.ImageField(

organizations/serializers.py

@@ -20,7 +20,7 @@ class Meta:
                   'active', 'logo_url',)
 
     def update_logo(self, obj, logo_url):
-        if logo_url:
+        if logo_url:  # pragma: no-cover
             logo = requests.get(logo_url)
             obj.logo.save(logo_url.split('/')[-1], ContentFile(logo.content))
 

organizations/tests/test_admin.py

@@ -12,12 +12,13 @@
 from organizations.tests.factories import UserFactory
 
 
-def create_organization(active=True):
+def create_organization(index, active=True):
     """
     Create an organization.
     """
     Organization.objects.create(
-        name='test organization',
+        short_name='test_org_{}'.format(index),
+        name='test organization {}'.format(index),
         description='test organization description',
         active=active
     )
@@ -57,7 +58,7 @@ def test_deactivate_selected_should_deactivate_active_organizations(self):
         """
         Test: action deactivate_selected should deactivate an activated organization.
         """
-        create_organization(active=True)
+        create_organization(1, active=True)
         queryset = Organization.objects.filter(pk=1)
         self.org_admin.deactivate_selected(self.request, queryset)
         self.assertFalse(Organization.objects.get(pk=1).active)
@@ -66,8 +67,8 @@ def test_deactivate_selected_should_deactivate_multiple_active_organizations(sel
         """
         Test: action deactivate_selected should deactivate the multiple activated organization.
         """
-        for __ in range(2):
-            create_organization(active=True)
+        for i in range(2):
+            create_organization(i, active=True)
         queryset = Organization.objects.all()
         self.org_admin.deactivate_selected(self.request, queryset)
         self.assertFalse(Organization.objects.get(pk=1).active)
@@ -77,7 +78,7 @@ def test_activate_selected_should_activate_deactivated_organizations(self):
         """
         Test: action activate_selected should activate an deactivated organization.
         """
-        create_organization(active=False)
+        create_organization(1, active=False)
         queryset = Organization.objects.filter(pk=1)
         self.org_admin.activate_selected(self.request, queryset)
         self.assertTrue(Organization.objects.get(pk=1).active)
@@ -86,8 +87,8 @@ def test_activate_selected_should_activate_multiple_deactivated_organizations(se
         """
         Test: action activate_selected should activate the multiple deactivated organization.
         """
-        for __ in range(2):
-            create_organization(active=True)
+        for i in range(2):
+            create_organization(i, active=True)
         queryset = Organization.objects.all()
         self.org_admin.activate_selected(self.request, queryset)
         self.assertTrue(Organization.objects.get(pk=1).active)
@@ -108,17 +109,17 @@ def test_foreign_key_field_active_choices(self):
         """
         Test: organization course foreignkey widget has active organization choices.
         """
-        create_organization(active=True)
+        create_organization(1, active=True)
         self.assertEqual(
             list(self.org_course_admin.get_form(self.request).base_fields['organization'].widget.choices),
-            [('', '---------'), (1, 'test organization ()')]
+            [('', '---------'), (1, 'test organization 1 (test_org_1)')]
         )
 
     def test_foreign_key_field_inactive_choices(self):
         """
         Test: organization course foreignkey widget has not inactive organization choices.
         """
-        create_organization(active=False)
+        create_organization(1, active=False)
         self.assertEqual(
             list(self.org_course_admin.get_form(self.request).base_fields['organization'].widget.choices),
             [('', '---------')]

organizations/tests/test_api.py

@@ -4,13 +4,15 @@
 """
 from unittest.mock import patch
 
+import ddt
 from django.test import override_settings
 
 import organizations.api as api
 import organizations.exceptions as exceptions
 import organizations.tests.utils as utils
 
 
+@ddt.ddt
 class OrganizationsApiTestCase(utils.OrganizationsTestCaseBase):
     """
     Main Test Case module for Organizations API
@@ -21,6 +23,7 @@ def setUp(self):
         """
         super(OrganizationsApiTestCase, self).setUp()
         self.test_organization = api.add_organization({
+            'short_name': 'test_organization',
             'name': 'test_organizationßßß',
             'description': 'Test Organization Descriptionßßß'
         })
@@ -29,6 +32,7 @@ def test_add_organization(self):
         """ Unit Test: test_add_organization"""
         with self.assertNumQueries(3):
             organization = api.add_organization({
+                'short_name': 'local_organization',
                 'name': 'local_organizationßßß',
                 'description': 'Local Organization Descriptionßßß'
             })
@@ -37,6 +41,7 @@ def test_add_organization(self):
     def test_add_organization_active_exists(self):
         """ Unit Test: test_add_organization_active_exists"""
         organization_data = {
+            'short_name': 'local_organization',
             'name': 'local_organizationßßß',
             'description': 'Local Organization Descriptionßßß'
         }
@@ -48,6 +53,7 @@ def test_add_organization_active_exists(self):
     def test_add_organization_inactive_to_active(self):
         """ Unit Test: test_add_organization_inactive_to_active"""
         organization_data = {
+            'short_name': 'local_organization',
             'name': 'local_organizationßßß',
             'description': 'Local Organization Descriptionßßß'
         }
@@ -65,6 +71,7 @@ def test_add_organization_inactive_to_active(self):
     def test_add_organization_inactive_organization_with_relationships(self):
         """ Unit Test: test_add_organization_inactive_organization_with_relationships"""
         organization_data = {
+            'short_name': 'local_organization',
             'name': 'local_organizationßßß',
             'description': 'Local Organization Descriptionßßß'
         }
@@ -77,21 +84,35 @@ def test_add_organization_inactive_organization_with_relationships(self):
         with self.assertNumQueries(1):
             organization = api.add_organization(organization_data)
 
-    def test_add_organization_invalid_data_throws_exceptions(self):
-        """ Unit Test: test_add_organization_invalid_namespaces_throw_exceptions"""
-        with self.assertNumQueries(0):
-            with self.assertRaises(exceptions.InvalidOrganizationException):
-                api.add_organization({
-                    'name': '',  # Empty name should throw an exception on create
-                    'description': 'Local Organization Description 2ßßß'
-                })
-
+    @ddt.data(
+        # Empty short name
+        {
+            'short_name': '',
+            'name': 'local_organizationßßß',
+            'description': 'Local Organization Description 2ßßß'
+        },
+        # Empty name
+        {
+            'short_name': 'local_organization',
+            'name': '',
+            'description': 'Local Organization Description 2ßßß'
+        },
+        # Missing short name
+        {
+            'name': 'local_organizationßßß',
+            'description': 'Local Organization Description 2ßßß'
+        },
+        # Missing name
+        {
+            'short_name': 'local_organization',
+            'description': 'Local Organization Description 2ßßß'
+        },
+    )
+    def test_add_organization_invalid_data_throws_exceptions(self, organization_data):
+        """ Unit Test: test_add_organization_invalid_data_throws_exceptions"""
         with self.assertNumQueries(0):
             with self.assertRaises(exceptions.InvalidOrganizationException):
-                api.add_organization({
-                    # Missing name should throw exception
-                    'description': 'Local Organization Description 2ßßß'
-                })
+                api.add_organization(organization_data)
 
     def test_edit_organization(self):
         """ Unit Test: test_edit_organization"""
@@ -159,10 +180,12 @@ def test_get_organizations(self):
         """ Unit Test: test_get_organizations """
         api.add_organization({
             'name': 'local_organization_1ßßß',
+            'short_name': 'Orgx1',
             'description': 'Local Organization 1 Descriptionßßß'
         })
         api.add_organization({
             'name': 'local_organization_2ßßß',
+            'short_name': 'Orgx2',
             'description': 'Local Organization 2 Descriptionßßß'
         })
         with self.assertNumQueries(1):