Merge pull request #460 from edx/awais786/ECOM-6142

Awais786/ecom 6142

course_discovery/apps/publisher/admin.py

 from django.contrib import admin
 
 from course_discovery.apps.publisher.models import (
-    Course, CourseRun, CourseUserRole, OrganizationUserRole, Seat, State, UserAttributes
+    Course, CourseRun, CourseUserRole, OrganizationExtension, OrganizationUserRole, Seat, State, UserAttributes
 )
 
 admin.site.register(Course)
 admin.site.register(CourseRun)
+admin.site.register(OrganizationExtension)
 admin.site.register(OrganizationUserRole)
 admin.site.register(Seat)
 admin.site.register(State)

course_discovery/apps/publisher/forms.py

 """
 Course publisher forms.
 """
-from django.contrib.auth.models import Group
 from django import forms
 from django.utils.translation import ugettext_lazy as _
 
 from course_discovery.apps.course_metadata.choices import CourseRunPacing
-from course_discovery.apps.course_metadata.models import Person
+from course_discovery.apps.course_metadata.models import Person, Organization
 from course_discovery.apps.publisher.models import Course, CourseRun, Seat, User
 
 
@@ -48,8 +47,10 @@ class CourseForm(BaseCourseForm):
 
 class CustomCourseForm(CourseForm):
     """ Course Form. """
-
-    institution = forms.ModelChoiceField(queryset=Group.objects.all(), required=True)
+    organization = forms.ModelChoiceField(
+        queryset=Organization.objects.filter(organization_extension__organization_id__isnull=False),
+        required=True
+    )
     title = forms.CharField(label=_('Course Title'), required=True)
     number = forms.CharField(label=_('Course Number'), required=True)
     team_admin = forms.ModelChoiceField(queryset=User.objects.filter(is_staff=True), required=True)
@@ -60,7 +61,7 @@ class CustomCourseForm(CourseForm):
             'title', 'number', 'short_description', 'full_description',
             'expected_learnings', 'level_type', 'primary_subject', 'secondary_subject',
             'tertiary_subject', 'prerequisites', 'level_type', 'image', 'team_admin',
-            'level_type', 'institution', 'is_seo_review', 'keywords',
+            'level_type', 'organization', 'is_seo_review', 'keywords',
         )
 
 

course_discovery/apps/publisher/migrations/0020_auto_20161214_1304.py

+# -*- coding: utf-8 -*-
+# Generated by Django 1.9.11 on 2016-12-14 12:21
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+import django_extensions.db.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_metadata', '0039_programtype_logo_image'),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('auth', '0007_alter_validators_add_error_messages'),
+        ('publisher', '0019_create_user_groups'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='HistoricalOrganizationExtension',
+            fields=[
+                ('id', models.IntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')),
+                ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')),
+                ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')),
+                ('history_id', models.AutoField(primary_key=True, serialize=False)),
+                ('history_date', models.DateTimeField()),
+                ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)),
+                ('group', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='auth.Group')),
+                ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)),
+                ('organization', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='course_metadata.Organization')),
+            ],
+            options={
+                'ordering': ('-history_date', '-history_id'),
+                'get_latest_by': 'history_date',
+                'verbose_name': 'historical organization extension',
+            },
+        ),
+        migrations.CreateModel(
+            name='OrganizationExtension',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')),
+                ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')),
+                ('group', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='organization_extension', to='auth.Group')),
+                ('organization', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='organization_extension', to='course_metadata.Organization')),
+            ],
+            options={
+                'ordering': ('-modified', '-created'),
+                'get_latest_by': 'modified',
+                'abstract': False,
+            },
+        ),
+    ]

course_discovery/apps/publisher/models.py

 import logging
 
+from django.contrib.auth.models import Group
 from django.core.urlresolvers import reverse
 from django.db import models
 from django.db.models.signals import pre_save
@@ -7,7 +8,7 @@ from django.dispatch import receiver
 from django.utils.translation import ugettext_lazy as _
 from django_extensions.db.models import TimeStampedModel
 from django_fsm import FSMField, transition
-from guardian.shortcuts import assign_perm, get_groups_with_perms, get_users_with_perms
+from guardian.shortcuts import assign_perm, get_users_with_perms
 from simple_history.models import HistoricalRecords
 from sortedm2m.fields import SortedManyToManyField
 from stdimage.models import StdImageField
@@ -151,17 +152,9 @@ class Course(TimeStampedModel, ChangedByMixin):
             ('view_course', 'Can view course'),
         )
 
-    def assign_permission_by_group(self, institution):
+    def assign_permission_by_group(self, group):
         """ Assigns permission on the course against the group. """
-        assign_perm(self.VIEW_PERMISSION, institution, self)
-
-    @property
-    def group_institution(self):
-        """ Returns the group object having permissions on the given course.
-        Course will be associated with one group only.
-        """
-        available_groups = get_groups_with_perms(self)
-        return available_groups[0] if available_groups else None
+        assign_perm(self.VIEW_PERMISSION, group, self)
 
     def get_group_users_emails(self):
         """ Returns the list of users emails with enable email notifications
@@ -407,3 +400,16 @@ class CourseUserRole(TimeStampedModel, ChangedByMixin):
             user=self.user,
             role=self.role
         )
+
+
+class OrganizationExtension(TimeStampedModel):
+    """ Organization-Extension relation model. """
+    organization = models.OneToOneField(Organization, related_name='organization_extension')
+    group = models.OneToOneField(Group, related_name='organization_extension')
+
+    history = HistoricalRecords()
+
+    def __str__(self):
+        return '{organization}: {group}'.format(
+            organization=self.organization, group=self.group
+        )

course_discovery/apps/publisher/tests/factories.py

@@ -13,7 +13,7 @@ from course_discovery.apps.course_metadata.tests import factories
 from course_discovery.apps.ietf_language_tags.models import LanguageTag
 from course_discovery.apps.publisher.choices import PublisherUserRole
 from course_discovery.apps.publisher.models import (
-    Course, CourseRun, CourseUserRole, OrganizationUserRole, Seat, State, UserAttributes
+    Course, CourseRun, CourseUserRole, OrganizationExtension, OrganizationUserRole, Seat, State, UserAttributes
 )
 
 
@@ -105,3 +105,11 @@ class CourseUserRoleFactory(factory.DjangoModelFactory):
 
     class Meta:
         model = CourseUserRole
+
+
+class OrganizationExtensionFactory(factory.DjangoModelFactory):
+    organization = factory.SubFactory(factories.OrganizationFactory)
+    group = factory.SubFactory(GroupFactory)
+
+    class Meta:
+        model = OrganizationExtension

course_discovery/apps/publisher/tests/test_emails.py

@@ -31,8 +31,9 @@ class StateChangeEmailTests(TestCase):
         cls.user_3 = UserFactory()
 
         cls.site = Site.objects.get(pk=settings.SITE_ID)
+        cls.organization_extension = factories.OrganizationExtensionFactory()
 
-        cls.group = factories.GroupFactory()
+        cls.group = cls.organization_extension.group
         cls.user.groups.add(cls.group)
         cls.user_2.groups.add(cls.group)
         cls.user_3.groups.add(cls.group)

course_discovery/apps/publisher/tests/test_model.py

@@ -4,10 +4,13 @@ from django.db import IntegrityError
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from django_fsm import TransitionNotAllowed
+from guardian.shortcuts import get_groups_with_perms
 
 from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.publisher.choices import PublisherUserRole
-from course_discovery.apps.publisher.models import State, Course, CourseUserRole, OrganizationUserRole
+from course_discovery.apps.publisher.models import (
+    State, Course, CourseUserRole, OrganizationExtension, OrganizationUserRole
+)
 from course_discovery.apps.publisher.tests import factories
 
 
@@ -67,10 +70,15 @@ class CourseTests(TestCase):
         self.user1 = UserFactory()
         self.user2 = UserFactory()
         self.user3 = UserFactory()
-        self.group_a = factories.GroupFactory()
-        self.group_b = factories.GroupFactory()
-        self.user1.groups.add(self.group_a)
-        self.user2.groups.add(self.group_b)
+
+        self.org_extension_1 = factories.OrganizationExtensionFactory()
+        self.org_extension_2 = factories.OrganizationExtensionFactory()
+
+        self.user1.groups.add(self.org_extension_1.group)
+        self.user2.groups.add(self.org_extension_2.group)
+
+        self.course.organizations.add(self.org_extension_1.organization)
+        self.course2.organizations.add(self.org_extension_2.organization)
 
     def test_str(self):
         """ Verify casting an instance to a string returns a string containing the course title. """
@@ -87,8 +95,8 @@ class CourseTests(TestCase):
         self.assert_user_cannot_view_course(self.user1, self.course)
         self.assert_user_cannot_view_course(self.user2, self.course2)
 
-        self.course.assign_permission_by_group(self.group_a)
-        self.course2.assign_permission_by_group(self.group_b)
+        self.course.assign_permission_by_group(self.org_extension_1.group)
+        self.course2.assign_permission_by_group(self.org_extension_2.group)
 
         self.assert_user_can_view_course(self.user1, self.course)
         self.assert_user_can_view_course(self.user2, self.course2)
@@ -96,8 +104,8 @@ class CourseTests(TestCase):
         self.assert_user_cannot_view_course(self.user1, self.course2)
         self.assert_user_cannot_view_course(self.user2, self.course)
 
-        self.assertEqual(self.course.group_institution, self.group_a)
-        self.assertEqual(self.course2.group_institution, self.group_b)
+        self.assertEqual(self.course.organizations.first().organization_extension.group, self.org_extension_1.group)
+        self.assertEqual(self.course2.organizations.first().organization_extension.group, self.org_extension_2.group)
 
     def assert_user_cannot_view_course(self, user, course):
         """ Asserts the user can NOT view the course. """
@@ -107,18 +115,18 @@ class CourseTests(TestCase):
         """ Asserts the user can view the course. """
         self.assertTrue(user.has_perm(Course.VIEW_PERMISSION, course))
 
-    def test_group_institution(self):
+    def test_group_by_permission(self):
         """ Verify the method returns groups permitted to access the course."""
-        self.assertEqual(self.course.group_institution, None)
-        self.course.assign_permission_by_group(self.group_a)
-        self.assertEqual(self.course.group_institution, self.group_a)
+        self.assertFalse(get_groups_with_perms(self.course))
+        self.course.assign_permission_by_group(self.org_extension_1.group)
+        self.assertEqual(get_groups_with_perms(self.course)[0], self.org_extension_1.group)
 
     def test_get_group_users_emails(self):
         """ Verify the method returns the email addresses of users who are
         permitted to access the course AND have not disabled email notifications.
         """
-        self.user3.groups.add(self.group_a)
-        self.course.assign_permission_by_group(self.group_a)
+        self.user3.groups.add(self.org_extension_1.group)
+        self.course.assign_permission_by_group(self.org_extension_1.group)
         self.assertListEqual(self.course.get_group_users_emails(), [self.user1.email, self.user3.email])
 
         # The email addresses of users who have disabled email notifications should NOT be returned.
@@ -240,3 +248,28 @@ class CourseUserRoleTests(TestCase):
             CourseUserRole.objects.create(
                 course=self.course_user_role.course, user=self.course_user_role.user, role=self.course_user_role.role
             )
+
+
+class GroupOrganizationTests(TestCase):
+    """Tests of the GroupOrganization model."""
+
+    def setUp(self):
+        super(GroupOrganizationTests, self).setUp()
+        self.organization_extension = factories.OrganizationExtensionFactory()
+        self.group_2 = factories.GroupFactory()
+
+    def test_str(self):
+        """Verify that a GroupOrganization is properly converted to a str."""
+        expected_str = '{organization}: {group}'.format(
+            organization=self.organization_extension.organization, group=self.organization_extension.group
+        )
+        self.assertEqual(str(self.organization_extension), expected_str)
+
+    def test_one_to_one_constraint(self):
+        """ Verify that same group or organization have only one record."""
+
+        with self.assertRaises(IntegrityError):
+            OrganizationExtension.objects.create(
+                group=self.group_2,
+                organization=self.organization_extension.organization
+            )

course_discovery/apps/publisher/tests/test_views.py

@@ -44,16 +44,16 @@ class CreateUpdateCourseViewTests(TestCase):
     def setUp(self):
         super(CreateUpdateCourseViewTests, self).setUp()
         self.user = UserFactory(is_staff=True, is_superuser=True)
-        self.group = factories.GroupFactory()
+        self.organization_extension = factories.OrganizationExtensionFactory()
+        self.group = self.organization_extension.group
 
         self.course = factories.CourseFactory(team_admin=self.user)
         self.course_run = factories.CourseRunFactory(course=self.course)
         self.seat = factories.SeatFactory(course_run=self.course_run, type=Seat.VERIFIED, price=2)
 
-        self.user.groups.add(self.group)
+        self.user.groups.add(self.organization_extension.group)
         self.site = Site.objects.get(pk=settings.SITE_ID)
         self.client.login(username=self.user.username, password=USER_PASSWORD)
-        self.group_2 = factories.GroupFactory()
         self.start_date_time = datetime.now().strftime('%Y-%m-%d %H:%M:%S')
 
     def test_course_form_without_login(self):
@@ -257,7 +257,7 @@ class CreateUpdateCourseViewTests(TestCase):
             course_dict.pop('end')
             course_dict.pop('priority')
             course_dict['start'] = self.start_date_time
-            course_dict['institution'] = self.group.id
+            course_dict['organization'] = self.organization_extension.organization.id
         if seat:
             course_dict.update(**model_to_dict(seat))
             course_dict.pop('verification_deadline')
@@ -288,7 +288,7 @@ class CreateUpdateCourseViewTests(TestCase):
             status_code=302,
             target_status_code=200
         )
-        self.assertEqual(course.group_institution, self.group)
+        self.assertEqual(course.organizations.first(), self.organization_extension.organization)
         self.assertEqual(course.team_admin, self.user)
         self.assertTrue(self.user.has_perm(Course.VIEW_PERMISSION, course))
         course_run = course.publisher_course_runs.all()[0]
@@ -303,6 +303,7 @@ class CreateUpdateCourseViewTests(TestCase):
 
         # django-taggit stores data without any order. For test .
         self.assertEqual(sorted([c.name for c in course.keywords.all()]), ['abc', 'def', 'xyz'])
+        self.assertEqual(course.organizations.first(), self.organization_extension.organization)
 
 
 class CreateUpdateCourseRunViewTests(TestCase):
@@ -1265,9 +1266,9 @@ class UpdateCourseKeyViewTests(TestCase):
         self.course_run = factories.CourseRunFactory()
         self.user = UserFactory(is_staff=True, is_superuser=True)
 
-        self.group = factories.GroupFactory()
-        self.user.groups.add(self.group)
-        assign_perm(Course.VIEW_PERMISSION, self.group, self.course_run.course)
+        self.organization_extension = factories.OrganizationExtensionFactory()
+        self.user.groups.add(self.organization_extension.group)
+        assign_perm(Course.VIEW_PERMISSION, self.organization_extension.group, self.course_run.course)
 
         self.client.login(username=self.user.username, password=USER_PASSWORD)
         self.update_course_key_url = reverse(

course_discovery/apps/publisher/views.py

@@ -6,7 +6,6 @@ import logging
 from datetime import datetime, timedelta
 
 from django.contrib import messages
-from django.contrib.auth.models import Group
 from django.core.urlresolvers import reverse
 from django.db import transaction
 from django.http import HttpResponseRedirect, HttpResponseForbidden, JsonResponse
@@ -24,7 +23,8 @@ from course_discovery.apps.publisher.forms import (
 )
 from course_discovery.apps.publisher import mixins
 from course_discovery.apps.publisher.models import (
-    Course, CourseRun, Seat, State, UserAttributes
+    Course, CourseRun, Seat, State, UserAttributes,
+    OrganizationExtension
 )
 from course_discovery.apps.publisher.serializers import UpdateCourseKeySerializer
 from course_discovery.apps.publisher.utils import (
@@ -192,9 +192,12 @@ class CreateCourseView(mixins.LoginRequiredMixin, CreateView):
                     seat.changed_by = self.request.user
                     seat.save()
 
-                    institution = get_object_or_404(Group, pk=course_form.data['institution'])
+                    organization_extension = get_object_or_404(
+                        OrganizationExtension, organization=course_form.data['organization']
+                    )
+                    course.organizations.add(organization_extension.organization)
                     # assign guardian permission.
-                    course.assign_permission_by_group(institution)
+                    course.assign_permission_by_group(organization_extension.group)
 
                     messages.success(
                         request, _('Course created successfully.')

course_discovery/apps/publisher_comments/tests/test_emails.py

@@ -28,17 +28,18 @@ class CommentsEmailTests(TestCase):
 
         self.site = Site.objects.get(pk=settings.SITE_ID)
 
-        self.group = factories.GroupFactory(name='abc')
+        self.organization_extension = factories.OrganizationExtensionFactory()
 
-        self.user.groups.add(self.group)
-        self.user_2.groups.add(self.group)
-        self.user_3.groups.add(self.group)
+        self.user.groups.add(self.organization_extension.group)
+        self.user_2.groups.add(self.organization_extension.group)
+        self.user_3.groups.add(self.organization_extension.group)
 
         self.seat = factories.SeatFactory()
         self.course_run = self.seat.course_run
         self.course = self.course_run.course
 
-        assign_perm(Course.VIEW_PERMISSION, self.group, self.course)
+        self.course.organizations.add(self.organization_extension.organization)
+        assign_perm(Course.VIEW_PERMISSION, self.organization_extension.group, self.course)
 
         # NOTE: We intentionally do NOT create an attribute for user_2.
         # By default this user WILL receive email notifications.
@@ -89,9 +90,9 @@ class CommentsEmailTests(TestCase):
 
     def test_email_without_different_group(self):
         """ Verify the emails behaviour if course group has no users. """
-        self.user.groups.remove(self.group)
-        self.user_2.groups.remove(self.group)
-        self.user_3.groups.remove(self.group)
+        self.user.groups.remove(self.organization_extension.group)
+        self.user_2.groups.remove(self.organization_extension.group)
+        self.user_3.groups.remove(self.organization_extension.group)
         self.create_comment(content_object=self.course)
         self.assertEqual(len(mail.outbox), 0)
 

course_discovery/apps/publisher_comments/tests/test_views.py

@@ -18,8 +18,9 @@ class CommentsTests(TestCase):
     def setUp(self):
         super(CommentsTests, self).setUp()
         self.user = UserFactory(is_staff=True, is_superuser=True)
-        self.group = factories.GroupFactory()
-        self.user.groups.add(self.group)
+        self.organization_extension = factories.OrganizationExtensionFactory()
+
+        self.user.groups.add(self.organization_extension.group)
 
         self.client.login(username=self.user.username, password=USER_PASSWORD)
         self.site = Site.objects.get(pk=settings.SITE_ID)
@@ -32,7 +33,8 @@ class CommentsTests(TestCase):
         self.course_run = self.seat.course_run
         self.course = self.course_run.course
 
-        self.course.assign_permission_by_group(self.group)
+        self.course.organizations.add(self.organization_extension.organization)
+        self.course.assign_permission_by_group(self.organization_extension.group)
         toggle_switch('enable_publisher_email_notifications', True)
 
     def test_course_edit_page_with_multiple_comments(self):

course_discovery/templates/publisher/add_course_form.html

@@ -39,10 +39,10 @@
                         {% trans "Please choose the school that will be providing the course. Once chosen then you can select an administrator for the studio shell." %}
                     </div>
                     <div class="col col-6">
-                        <label class="field-label">{{ course_form.institution.label_tag }}
+                        <label class="field-label">{{ course_form.organization.label_tag }}
                             <span class="required">* {% trans "required" %}</span>
                         </label>
-                        {{ course_form.institution}}
+                        {{ course_form.organization }}
 
                         <label class="field-label">
                             {{ course_form.team_admin.label_tag }}

course_discovery/templates/publisher/dashboard/_in_progress.html

@@ -31,9 +31,8 @@
                     <td>
                         <a href="{% url 'publisher:publisher_course_run_detail' course_run.id %}">{{ course_run.title }}</a>
                     </td>
-                    <td>{% if course_run.course.group_institution %}
-                            {{ course_run.course.group_institution }}
-                        {% endif %}
+                    <td>
+                        {% if course_run.course.organizations.first %}{{ course_run.course.organizations.first.name }}{% endif %}
                     </td>
                     <td>
                          {{ course_run.start|date:"Y-m-d" }}

course_discovery/templates/publisher/dashboard/_preview_ready.html

@@ -31,7 +31,9 @@
                         <td id="course-title-{{ course_run.title }}">
                             <a href="{% url 'publisher:publisher_course_run_detail' course_run.id %}">{{ course_run.title }}</a>
                         </td>
-                        <td>{{ course_run.course.group_institution }}</td>
+                        <td>
+                        {% if course_run.course.organizations.first %}{{ course_run.course.organizations.first.name }}{% endif %}
+                        </td>
                         <td>
                              {{ course_run.state }}
                         </td>

course_discovery/templates/publisher/dashboard/_published.html

@@ -36,9 +36,8 @@
                     <td>
                         <a href="{% url 'publisher:publisher_course_run_detail' course_run.id %}">{{ course_run.title }}</a>
                     </td>
-                    <td>{% if course_run.course.group_institution %}
-                            {{ course_run.course.group_institution }}
-                        {% endif %}
+                    <td>
+                        {% if course_run.course.organizations.first %}{{ course_run.course.organizations.first.name }}{% endif %}
                     </td>
                     <td>
                          {{ course_run.start|date:"Y-m-d" }}

course_discovery/templates/publisher/dashboard/_studio_requests.html

@@ -33,7 +33,9 @@
                             <td id="course-title-{{ course_run.title }}">
                                 <a href="{{ run_page_url }}" id="course-title">{{ course_run.title }}</a>
                             </td>
-                            <td>{% if course_run.course.group_institution %}{{ course_run.course.group_institution }}{% endif %}</td>
+                            <td>
+                                {% if course_run.course.organizations.first %}{{ course_run.course.organizations.first.name }}{% endif %}
+                            </td>
                             <td id="course-start">
                                  {{ course_run.start|date:"Y-m-d" }}
                             </td>