Merge pull request #502 from edx/awais786/ECOM-6628-org-ext-permissions

Publisher app: Add/Remove Permissions.

course_discovery/apps/publisher/api/permissions.py

 from rest_framework.permissions import BasePermission
 
-from course_discovery.apps.publisher.mixins import check_view_permission
+from course_discovery.apps.publisher.mixins import check_user_course_access
 from course_discovery.apps.publisher.utils import is_internal_user
 
 
@@ -8,7 +8,7 @@ class CanViewAssociatedCourse(BasePermission):
     """ Permission class to check user can view a publisher course. """
 
     def has_object_permission(self, request, view, obj):
-        return check_view_permission(request.user, obj.course)
+        return check_user_course_access(request.user, obj.course)
 
 
 class InternalUserPermission(BasePermission):

course_discovery/apps/publisher/api/tests/test_views.py

@@ -5,13 +5,10 @@ import ddt
 from django.contrib.auth.models import Group
 from django.core.urlresolvers import reverse
 from django.test import TestCase
-from guardian.shortcuts import assign_perm
 
 from course_discovery.apps.core.tests.factories import UserFactory, USER_PASSWORD
-from course_discovery.apps.course_metadata.tests.factories import OrganizationFactory
 from course_discovery.apps.publisher.choices import PublisherUserRole
 from course_discovery.apps.publisher.constants import INTERNAL_USER_GROUP_NAME
-from course_discovery.apps.publisher.models import Course
 from course_discovery.apps.publisher.tests import factories, JSON_CONTENT_TYPE
 
 
@@ -33,10 +30,8 @@ class CourseRoleAssignmentViewTests(TestCase):
             self.other_internal_users.append(user)
             internal_user_group.user_set.add(user)
 
-        assign_perm(Course.VIEW_PERMISSION, internal_user_group, self.course)
-
-        organization = OrganizationFactory()
-        self.course.organizations.add(organization)
+        organization_extension = factories.OrganizationExtensionFactory()
+        self.course.organizations.add(organization_extension.organization)
 
         # Create three internal user course roles for internal users against a course
         # so we can test change role assignment on these roles.
@@ -54,7 +49,6 @@ class CourseRoleAssignmentViewTests(TestCase):
         """ Verify non-internal users cannot change role assignments. """
 
         non_internal_user = UserFactory()
-        assign_perm(Course.VIEW_PERMISSION, non_internal_user, self.course)
 
         self.client.logout()
         self.client.login(username=non_internal_user.username, password=USER_PASSWORD)
@@ -103,15 +97,12 @@ class OrganizationGroupUserViewTests(TestCase):
         user = UserFactory.create(username="test_user", password=USER_PASSWORD)
         self.client.login(username=user.username, password=USER_PASSWORD)
 
-        # create group and add test users in the group
-        group = factories.GroupFactory()
+        organization_extension = factories.OrganizationExtensionFactory()
         self.org_user1 = UserFactory.create(full_name="org user1")
         self.org_user2 = UserFactory.create(full_name="org user2")
-        group.user_set.add(self.org_user1)
-        group.user_set.add(self.org_user2)
-
-        self.organization = OrganizationFactory()
-        factories.OrganizationExtensionFactory.create(organization=self.organization, group=group)
+        organization_extension.group.user_set.add(self.org_user1)
+        organization_extension.group.user_set.add(self.org_user2)
+        self.organization = organization_extension.organization
 
     def test_get_organization_user_group(self):
         """ Verify that view returns list of users associated with the group

course_discovery/apps/publisher/emails.py

@@ -22,7 +22,7 @@ def send_email_for_change_state(course_run):
         txt_template = 'publisher/email/change_state.txt'
         html_template = 'publisher/email/change_state.html'
 
-        to_addresses = course_run.course.get_group_users_emails()
+        to_addresses = course_run.course.get_course_users_emails()
         from_address = settings.PUBLISHER_FROM_EMAIL
         page_path = reverse('publisher:publisher_course_run_detail', kwargs={'pk': course_run.id})
         context = {
@@ -61,7 +61,7 @@ def send_email_for_studio_instance_created(course_run):
         object_path = reverse('publisher:publisher_course_run_detail', kwargs={'pk': course_run.id})
         subject = _('Studio instance created')
 
-        to_addresses = course_run.course.get_group_users_emails()
+        to_addresses = course_run.course.get_course_users_emails()
         from_address = settings.PUBLISHER_FROM_EMAIL
 
         context = {

course_discovery/apps/publisher/forms.py

@@ -53,7 +53,7 @@ class CustomCourseForm(CourseForm):
     )
     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)
+    team_admin = forms.ModelChoiceField(queryset=User.objects.all(), required=True)
 
     class Meta(CourseForm.Meta):
         model = Course
@@ -69,7 +69,7 @@ class UpdateCourseForm(BaseCourseForm):
     """ Course form to update specific fields for already created course. """
 
     number = forms.CharField(label=_('Course Number'), required=True)
-    team_admin = forms.ModelChoiceField(queryset=User.objects.filter(is_staff=True), required=True)
+    team_admin = forms.ModelChoiceField(queryset=User.objects.all(), required=True)
 
     class Meta:
         model = Course

course_discovery/apps/publisher/migrations/0022_auto_20161222_2135.py

+# -*- coding: utf-8 -*-
+# Generated by Django 1.9.11 on 2016-12-22 21:35
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('publisher', '0021_auto_20161214_1356'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='organizationextension',
+            options={'get_latest_by': 'modified', 'ordering': ('-modified', '-created'), 'permissions': (('edit_course_run', 'Can edit course run'), ('publisher_view_course', 'Can view course'), ('publisher_view_course_run', 'Can view the course run'))},
+        ),
+    ]

course_discovery/apps/publisher/mixins.py

@@ -2,7 +2,8 @@ from django.contrib.auth.decorators import login_required
 from django.http import HttpResponseForbidden, HttpResponseRedirect
 from django.utils.decorators import method_decorator
 
-from course_discovery.apps.publisher.models import Course, Seat
+from course_discovery.apps.publisher.models import Course, Seat, OrganizationExtension
+from course_discovery.apps.publisher.utils import is_publisher_admin, is_internal_user
 
 
 class ViewPermissionMixin(object):
@@ -20,7 +21,7 @@ class ViewPermissionMixin(object):
 
     def check_user(self, user):
         course = self.get_course()
-        return check_view_permission(user, course)
+        return check_user_course_access(user, course)
 
     def permission_failed(self):
         return HttpResponseForbidden()
@@ -56,5 +57,28 @@ class FormValidMixin(object):
         return HttpResponseRedirect(self.get_success_url())
 
 
-def check_view_permission(user, course):
-    return user.is_staff or user.has_perm(Course.VIEW_PERMISSION, course)
+def check_roles_access(user):
+    """ Return True if user is part of a role that gives implicit access. """
+    if is_publisher_admin(user) or is_internal_user(user):
+        return True
+
+    return False
+
+
+def check_course_organization_permission(user, course):
+    """ Return True if user has view permission on organization. """
+    if not hasattr(course, 'organizations'):
+        return False
+
+    return any(
+        [
+            user.has_perm(OrganizationExtension.VIEW_COURSE, org.organization_extension)
+            for org in course.organizations.all()
+        ]
+    )
+
+
+def check_user_course_access(user, course):
+    """ Return True if user is admin/internal user or has access permission. """
+
+    return check_roles_access(user) or check_course_organization_permission(user, course)

course_discovery/apps/publisher/models.py

@@ -8,7 +8,6 @@ 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_users_with_perms
 from simple_history.models import HistoricalRecords
 from sortedm2m.fields import SortedManyToManyField
 from stdimage.models import StdImageField
@@ -85,7 +84,6 @@ class State(TimeStampedModel, ChangedByMixin):
 
 class Course(TimeStampedModel, ChangedByMixin):
     """ Publisher Course model. It contains fields related to the course intake form."""
-    VIEW_PERMISSION = 'view_course'
 
     title = models.CharField(max_length=255, default=None, null=True, blank=True, verbose_name=_('Course title'))
     number = models.CharField(max_length=50, null=True, blank=True, verbose_name=_('Course number'))
@@ -152,21 +150,14 @@ class Course(TimeStampedModel, ChangedByMixin):
             ('view_course', 'Can view course'),
         )
 
-    def assign_permission_by_group(self, group):
-        """ Assigns permission on the course against the group. """
-        assign_perm(self.VIEW_PERMISSION, group, self)
-
-    def get_group_users_emails(self):
+    def get_course_users_emails(self):
         """ Returns the list of users emails with enable email notifications
-        against a course group. By default if attribute value does not exists
+        against a course. By default if attribute value does not exists
         then user will be eligible for emails.
-        Also get users from course-user-role table.
         """
-        users_list_perms = get_users_with_perms(self)
         users_list_roles = [obj.user for obj in self.course_user_roles.all()]
 
-        users_list = set(list(users_list_perms) + list(users_list_roles))
-        emails = [user.email for user in users_list if is_email_notification_enabled(user)]
+        emails = [user.email for user in users_list_roles if is_email_notification_enabled(user)]
 
         return emails
 
@@ -416,6 +407,8 @@ class CourseUserRole(TimeStampedModel, ChangedByMixin):
 class OrganizationExtension(TimeStampedModel):
     """ Organization-Extension relation model. """
     EDIT_COURSE_RUN = 'edit_course_run'
+    VIEW_COURSE = 'publisher_view_course'
+    VIEW_COURSE_RUN = 'publisher_view_course_run'
 
     organization = models.OneToOneField(Organization, related_name='organization_extension')
     group = models.OneToOneField(Group, related_name='organization_extension')
@@ -425,6 +418,8 @@ class OrganizationExtension(TimeStampedModel):
     class Meta(TimeStampedModel.Meta):
         permissions = (
             ('edit_course_run', 'Can edit course run'),
+            ('publisher_view_course', 'Can view course'),
+            ('publisher_view_course_run', 'Can view the course run'),
         )
 
     def __str__(self):

course_discovery/apps/publisher/tests/test_emails.py

@@ -6,7 +6,6 @@ from django.contrib.sites.models import Site
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from django.core import mail
-from guardian.shortcuts import assign_perm
 import pytz
 import mock
 from testfixtures import LogCapture
@@ -14,7 +13,8 @@ from testfixtures import LogCapture
 from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.course_metadata.tests import toggle_switch
 from course_discovery.apps.publisher import emails
-from course_discovery.apps.publisher.models import State, Course
+from course_discovery.apps.publisher.choices import PublisherUserRole
+from course_discovery.apps.publisher.models import State, CourseUserRole
 from course_discovery.apps.publisher.tests import factories
 from course_discovery.apps.publisher.tests.factories import UserAttributeFactory
 
@@ -42,7 +42,16 @@ class StateChangeEmailTests(TestCase):
         cls.course_run = cls.seat.course_run
         cls.course = cls.course_run.course
 
-        assign_perm(Course.VIEW_PERMISSION, cls.group, cls.course)
+        # add user in course-user-role table
+        factories.CourseUserRoleFactory(
+            course=cls.course, role=PublisherUserRole.PartnerCoordinator, user=cls.user
+        )
+        factories.CourseUserRoleFactory(
+            course=cls.course, role=PublisherUserRole.MarketingReviewer, user=cls.user_2
+        )
+        factories.CourseUserRoleFactory(
+            course=cls.course, role=PublisherUserRole.Publisher, user=cls.user_3
+        )
 
         # NOTE: We intentionally do NOT create an attribute for user_2.
         # By default this user WILL receive email notifications.
@@ -94,10 +103,8 @@ class StateChangeEmailTests(TestCase):
         State.FINALIZED, State.PUBLISHED, State.DRAFT
     )
     def test_email_without_group(self, target_state):
-        """ Verify that no email send 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)
+        """ Verify that no email send if course user role has no users. """
+        CourseUserRole.objects.all().delete()
 
         self.course_run.change_state(target=target_state)
         self.assertEqual(len(mail.outbox), 0)
@@ -138,7 +145,10 @@ class StudioInstanceCreatedEmailTests(TestCase):
 
         self.course_run = factories.CourseRunFactory()
 
-        assign_perm(Course.VIEW_PERMISSION, self.group, self.course_run.course)
+        # add user in course-user-role table
+        factories.CourseUserRoleFactory(
+            course=self.course_run.course, role=PublisherUserRole.PartnerCoordinator, user=self.user
+        )
 
         UserAttributeFactory(user=self.user, enable_email_notification=True)
 

course_discovery/apps/publisher/tests/test_model.py

@@ -4,12 +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 guardian.shortcuts import assign_perm
 
 from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.publisher.choices import PublisherUserRole
+from course_discovery.apps.publisher.mixins import check_course_organization_permission
 from course_discovery.apps.publisher.models import (
-    State, Course, CourseUserRole, OrganizationExtension, OrganizationUserRole
+    State, CourseUserRole, OrganizationExtension, OrganizationUserRole
 )
 from course_discovery.apps.publisher.tests import factories
 
@@ -80,6 +81,19 @@ class CourseTests(TestCase):
         self.course.organizations.add(self.org_extension_1.organization)
         self.course2.organizations.add(self.org_extension_2.organization)
 
+        # add user in course-user-role table
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.PartnerCoordinator, user=self.user1
+        )
+
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.MarketingReviewer, user=self.user2
+        )
+
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.Publisher, user=self.user3
+        )
+
     def test_str(self):
         """ Verify casting an instance to a string returns a string containing the course title. """
         self.assertEqual(str(self.course), self.course.title)
@@ -90,13 +104,16 @@ class CourseTests(TestCase):
             reverse('publisher:publisher_courses_edit', kwargs={'pk': self.course.id})
         )
 
-    def test_assign_permission_by_group(self):
-        """ Verify that permission can be assigned using the group. """
+    def test_assign_permission_organization_extension(self):
+        """ Verify that permission can be assigned using the organization extension. """
         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.org_extension_1.group)
-        self.course2.assign_permission_by_group(self.org_extension_2.group)
+        self.course.organizations.add(self.org_extension_1.organization)
+        self.course2.organizations.add(self.org_extension_2.organization)
+
+        assign_perm(OrganizationExtension.VIEW_COURSE, self.user1, self.org_extension_1)
+        assign_perm(OrganizationExtension.VIEW_COURSE, self.user2, self.org_extension_2)
 
         self.assert_user_can_view_course(self.user1, self.course)
         self.assert_user_can_view_course(self.user2, self.course2)
@@ -109,39 +126,24 @@ class CourseTests(TestCase):
 
     def assert_user_cannot_view_course(self, user, course):
         """ Asserts the user can NOT view the course. """
-        self.assertFalse(user.has_perm(Course.VIEW_PERMISSION, course))
+        self.assertFalse(check_course_organization_permission(user, course, ))
 
     def assert_user_can_view_course(self, user, course):
         """ Asserts the user can view the course. """
-        self.assertTrue(user.has_perm(Course.VIEW_PERMISSION, course))
+        self.assertTrue(check_course_organization_permission(user, course))
 
-    def test_group_by_permission(self):
-        """ Verify the method returns groups permitted to access the course."""
-        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):
+    def test_get_course_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.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])
-
-        # add user in course-user-role table
-        factories.CourseUserRoleFactory(
-            course=self.course, role=PublisherUserRole.PartnerCoordinator, user=self.user2
-        )
-
         self.assertListEqual(
-            self.course.get_group_users_emails(),
+            self.course.get_course_users_emails(),
             [self.user1.email, self.user2.email, self.user3.email]
         )
 
         # The email addresses of users who have disabled email notifications should NOT be returned.
         factories.UserAttributeFactory(user=self.user1, enable_email_notification=False)
-        self.assertListEqual(self.course.get_group_users_emails(), [self.user2.email, self.user3.email])
+        self.assertListEqual(self.course.get_course_users_emails(), [self.user2.email, self.user3.email])
 
     def test_keywords_data(self):
         """ Verify that the property returns the keywords as comma separated string. """
@@ -155,13 +157,13 @@ class CourseTests(TestCase):
 
     def test_partner_coordinator(self):
         """ Verify that the partner_coordinator property returns user if exist. """
-        self.assertIsNone(self.course.partner_coordinator)
+        self.assertIsNone(self.course2.partner_coordinator)
 
         factories.CourseUserRoleFactory(
-            course=self.course, user=self.user1, role=PublisherUserRole.PartnerCoordinator
+            course=self.course2, user=self.user1, role=PublisherUserRole.PartnerCoordinator
         )
 
-        self.assertEqual(self.user1, self.course.partner_coordinator)
+        self.assertEqual(self.user1, self.course2.partner_coordinator)
 
 
 class SeatTests(TestCase):

course_discovery/apps/publisher/tests/test_utils.py

 """ Tests publisher.utils"""
 from django.contrib.auth.models import Group
 from django.test import TestCase
+from guardian.shortcuts import assign_perm
 
 from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.publisher.constants import (
     ADMIN_GROUP_NAME, INTERNAL_USER_GROUP_NAME, PARTNER_COORDINATOR_GROUP_NAME
 )
+from course_discovery.apps.publisher.mixins import (
+    check_roles_access, check_course_organization_permission, check_user_course_access
+)
+from course_discovery.apps.publisher.models import OrganizationExtension
 from course_discovery.apps.publisher.tests import factories
 from course_discovery.apps.publisher.utils import (
     is_email_notification_enabled, is_publisher_admin, is_internal_user,
@@ -18,7 +23,12 @@ class PublisherUtilsTests(TestCase):
 
     def setUp(self):
         super(PublisherUtilsTests, self).setUp()
-        self.user = UserFactory(is_staff=True, is_superuser=True)
+        self.user = UserFactory()
+        self.course = factories.CourseFactory()
+        self.admin_group = Group.objects.get(name=ADMIN_GROUP_NAME)
+        self.internal_user_group = Group.objects.get(name=INTERNAL_USER_GROUP_NAME)
+        self.organization_extension = factories.OrganizationExtensionFactory()
+        self.course.organizations.add(self.organization_extension.organization)
 
     def test_email_notification_enabled_by_default(self):
         """ Test email notification is enabled for the user by default."""
@@ -82,3 +92,51 @@ class PublisherUtilsTests(TestCase):
         partner_coordinator_group = Group.objects.get(name=PARTNER_COORDINATOR_GROUP_NAME)
         self.user.groups.add(partner_coordinator_group)
         self.assertTrue(is_partner_coordinator_user(self.user))
+
+    def test_check_roles_access_with_admin(self):
+        """ Verify the function returns a boolean indicating if the user
+        access role wise.
+        """
+        self.assertFalse(check_roles_access(self.user))
+        self.user.groups.add(self.admin_group)
+        self.assertTrue(check_roles_access(self.user))
+
+    def test_check_roles_access_with_internal_user(self):
+        """ Verify the function returns a boolean indicating if the user
+        access role wise.
+        """
+        self.assertFalse(check_roles_access(self.user))
+        self.user.groups.add(self.internal_user_group)
+        self.assertTrue(check_roles_access(self.user))
+
+    def test_check_organization_permission_without_org(self):
+        """ Verify the function returns a boolean indicating if the user has
+        organization permission on given course.
+        """
+        self.assertFalse(check_course_organization_permission(self.user, self.course))
+        assign_perm(
+            OrganizationExtension.VIEW_COURSE, self.user, self.organization_extension
+        )
+        self.assertTrue(check_course_organization_permission(self.user, self.course))
+
+    def test_check_user_access_with_roles(self):
+        """ Verify the function returns a boolean indicating if the user
+        organization permission on given course or user is internal or admin user.
+        """
+        self.assertFalse(check_user_course_access(self.user, self.course))
+        self.user.groups.add(self.admin_group)
+        self.assertTrue(check_user_course_access(self.user, self.course))
+        self.user.groups.remove(self.admin_group)
+        self.assertFalse(check_user_course_access(self.user, self.course))
+        self.user.groups.add(self.internal_user_group)
+        self.assertTrue(check_user_course_access(self.user, self.course))
+
+    def test_check_user_access_with_permission(self):
+        """ Verify the function returns a boolean indicating if the user
+        has view permission on organization
+        """
+        self.assertFalse(check_course_organization_permission(self.user, self.course))
+        assign_perm(
+            OrganizationExtension.VIEW_COURSE, self.user, self.organization_extension
+        )
+        self.assertTrue(check_course_organization_permission(self.user, self.course))

course_discovery/apps/publisher/views.py

@@ -59,9 +59,14 @@ class Dashboard(mixins.LoginRequiredMixin, ListView):
             internal_user_courses = Course.objects.filter(course_user_roles__user=user)
             course_runs = CourseRun.objects.filter(course__in=internal_user_courses).select_related('course').all()
         else:
-            # in future we will change permission from course to OrganizationExtension model
-            courses = get_objects_for_user(user, Course.VIEW_PERMISSION, Course)
-            course_runs = CourseRun.objects.filter(course__in=courses).select_related('course').all()
+            organizations = get_objects_for_user(
+                user, OrganizationExtension.VIEW_COURSE, OrganizationExtension,
+                use_groups=False,
+                with_superuser=False
+            ).values_list('organization')
+            course_runs = CourseRun.objects.filter(
+                course__organizations__in=organizations
+            ).select_related('course').all()
 
         return course_runs
 
@@ -201,8 +206,6 @@ class CreateCourseView(mixins.LoginRequiredMixin, CreateView):
                         OrganizationExtension, organization=course_form.data['organization']
                     )
                     course.organizations.add(organization_extension.organization)
-                    # assign guardian permission.
-                    course.assign_permission_by_group(organization_extension.group)
 
                     messages.success(
                         request, _('Course created successfully.')
@@ -226,7 +229,7 @@ class UpdateCourseView(mixins.LoginRequiredMixin, mixins.ViewPermissionMixin, mi
     """ Update Course View."""
     model = Course
     form_class = CourseForm
-    permission_required = Course.VIEW_PERMISSION
+    permission_required = OrganizationExtension.VIEW_COURSE
     template_name = 'publisher/course_form.html'
     success_url = 'publisher:publisher_courses_edit'
 
@@ -324,7 +327,7 @@ class UpdateCourseRunView(mixins.LoginRequiredMixin, mixins.ViewPermissionMixin,
     """ Update Course Run View."""
     model = CourseRun
     form_class = CourseRunForm
-    permission_required = Course.VIEW_PERMISSION
+    permission_required = OrganizationExtension.VIEW_COURSE
     template_name = 'publisher/course_run_form.html'
     success_url = 'publisher:publisher_course_runs_edit'
     change_state = True
@@ -361,7 +364,7 @@ class UpdateSeatView(mixins.LoginRequiredMixin, mixins.ViewPermissionMixin, mixi
     """ Update Seat View."""
     model = Seat
     form_class = SeatForm
-    permission_required = Course.VIEW_PERMISSION
+    permission_required = OrganizationExtension.EDIT_COURSE_RUN
     template_name = 'publisher/seat_form.html'
     success_url = 'publisher:publisher_seats_edit'
 
@@ -383,7 +386,7 @@ class ChangeStateView(mixins.LoginRequiredMixin, View):
         try:
             course_run = CourseRun.objects.get(id=course_run_id)
 
-            if not mixins.check_view_permission(request.user, course_run.course):
+            if not mixins.check_user_course_access(request.user, course_run.course):
                 return HttpResponseForbidden()
 
             course_run.change_state(target=state, user=self.request.user)
@@ -425,6 +428,13 @@ class CourseListView(mixins.LoginRequiredMixin, ListView):
         elif is_internal_user(user):
             courses = Course.objects.filter(course_user_roles__user=user).distinct()
         else:
-            courses = get_objects_for_user(user, Course.VIEW_PERMISSION, Course)
+            organizations = get_objects_for_user(
+                user,
+                OrganizationExtension.VIEW_COURSE,
+                OrganizationExtension,
+                use_groups=False,
+                with_superuser=False
+            ).values_list('organization')
+            courses = Course.objects.filter(organizations__in=organizations)
 
         return courses

course_discovery/apps/publisher_comments/emails.py

@@ -53,7 +53,7 @@ def send_email_for_comment(comment, created=False):
                 title=course.title
             )
 
-        to_addresses = course.get_group_users_emails()
+        to_addresses = course.get_course_users_emails()
         from_address = settings.PUBLISHER_FROM_EMAIL
 
         context = {

course_discovery/apps/publisher_comments/tests/test_emails.py

@@ -34,23 +34,24 @@ class CommentsEmailTests(TestCase):
         self.course_run = self.seat.course_run
         self.course = self.course_run.course
 
-        # assign the role against a course
+        self.course.organizations.add(self.organization_extension.organization)
+
+        # NOTE: We intentionally do NOT create an attribute for user_2.
+        # By default this user WILL receive email notifications.
+
+        # add user in course-user-role table
         factories.CourseUserRoleFactory(
-            course=self.course, role=PublisherUserRole.MarketingReviewer, user=self.user
+            course=self.course, role=PublisherUserRole.PartnerCoordinator, user=self.user
         )
+
         factories.CourseUserRoleFactory(
             course=self.course, role=PublisherUserRole.PartnerCoordinator, user=self.user_2
         )
 
         factories.CourseUserRoleFactory(
-            course=self.course, role=PublisherUserRole.Publisher, user=self.user_3
+            course=self.course, role=PublisherUserRole.PartnerCoordinator, user=self.user_3
         )
 
-        self.course.organizations.add(self.organization_extension.organization)
-
-        # NOTE: We intentionally do NOT create an attribute for user_2.
-        # By default this user WILL receive email notifications.
-
         UserAttributeFactory(user=self.user, enable_email_notification=True)
         UserAttributeFactory(user=self.user_3, enable_email_notification=False)
         toggle_switch('enable_publisher_email_notifications', True)

course_discovery/apps/publisher_comments/tests/test_views.py

@@ -31,7 +31,6 @@ class CommentsTests(TestCase):
         self.seat = factories.SeatFactory(type=Seat.PROFESSIONAL, credit_hours=0)
         self.course_run = self.seat.course_run
         self.course = self.course_run.course
-
         self.course.organizations.add(self.organization_extension.organization)
 
         # assign the role against a course