Merge pull request #18638 from open-craft/jill/reinstate-enrollments-admin
Gate access to the CourseEnrollmentAdmin view with a waffle switch
This commit is contained in:
@@ -0,0 +1,8 @@
|
||||
"""
|
||||
Student app helpers and settings
|
||||
"""
|
||||
from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace
|
||||
|
||||
|
||||
# Namespace for student app waffle switches
|
||||
STUDENT_WAFFLE_NAMESPACE = WaffleSwitchNamespace(name='student')
|
||||
|
||||
@@ -9,7 +9,9 @@ from django.utils.translation import ugettext_lazy as _
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
|
||||
from openedx.core.djangoapps.waffle_utils import WaffleSwitch
|
||||
from openedx.core.lib.courses import clean_course_id
|
||||
from student import STUDENT_WAFFLE_NAMESPACE
|
||||
from student.models import (
|
||||
CourseAccessRole,
|
||||
CourseEnrollment,
|
||||
@@ -28,6 +30,11 @@ from xmodule.modulestore.django import modulestore
|
||||
|
||||
User = get_user_model() # pylint:disable=invalid-name
|
||||
|
||||
# This switch exists because the CourseEnrollment admin views make DB queries that impact performance.
|
||||
# In a large enough deployment of Open edX, this is enough to cause a site outage.
|
||||
# See https://openedx.atlassian.net/browse/OPS-2943
|
||||
COURSE_ENROLLMENT_ADMIN_SWITCH = WaffleSwitch(STUDENT_WAFFLE_NAMESPACE, 'courseenrollment_admin')
|
||||
|
||||
|
||||
class CourseAccessRoleForm(forms.ModelForm):
|
||||
"""Form for adding new Course Access Roles view the Django Admin Panel."""
|
||||
@@ -165,13 +172,7 @@ class CourseEnrollmentForm(forms.ModelForm):
|
||||
fields = '__all__'
|
||||
|
||||
|
||||
# Page disabled because it makes DB quries that impact performance enough to
|
||||
# cause a site outage. It may be re-enabled when it is updated to make more
|
||||
# efficent DB queries
|
||||
# https://openedx.atlassian.net/browse/OPS-2943
|
||||
# Learner ticket to add functionality to /support
|
||||
# https://openedx.atlassian.net/browse/LEARNER-4744
|
||||
#@admin.register(CourseEnrollment)
|
||||
@admin.register(CourseEnrollment)
|
||||
class CourseEnrollmentAdmin(admin.ModelAdmin):
|
||||
""" Admin interface for the CourseEnrollment model. """
|
||||
list_display = ('id', 'course_id', 'mode', 'user', 'is_active',)
|
||||
@@ -183,6 +184,38 @@ class CourseEnrollmentAdmin(admin.ModelAdmin):
|
||||
def queryset(self, request):
|
||||
return super(CourseEnrollmentAdmin, self).queryset(request).select_related('user')
|
||||
|
||||
def has_permission(self, request, method):
|
||||
"""
|
||||
Returns True if the given admin method is allowed.
|
||||
"""
|
||||
if COURSE_ENROLLMENT_ADMIN_SWITCH.is_enabled():
|
||||
return getattr(super(CourseEnrollmentAdmin, self), method)(request)
|
||||
return False
|
||||
|
||||
def has_add_permission(self, request):
|
||||
"""
|
||||
Returns True if CourseEnrollment objects can be added via the admin view.
|
||||
"""
|
||||
return self.has_permission(request, 'has_add_permission')
|
||||
|
||||
def has_change_permission(self, request, obj=None):
|
||||
"""
|
||||
Returns True if CourseEnrollment objects can be modified via the admin view.
|
||||
"""
|
||||
return self.has_permission(request, 'has_change_permission')
|
||||
|
||||
def has_delete_permission(self, request, obj=None):
|
||||
"""
|
||||
Returns True if CourseEnrollment objects can be deleted via the admin view.
|
||||
"""
|
||||
return self.has_permission(request, 'has_delete_permission')
|
||||
|
||||
def has_module_permission(self, request):
|
||||
"""
|
||||
Returns True if links to the CourseEnrollment admin view can be displayed.
|
||||
"""
|
||||
return self.has_permission(request, 'has_module_permission')
|
||||
|
||||
|
||||
class UserProfileInline(admin.StackedInline):
|
||||
""" Inline admin interface for UserProfile model. """
|
||||
|
||||
@@ -1,14 +1,15 @@
|
||||
"""
|
||||
Tests student admin.py
|
||||
"""
|
||||
import ddt
|
||||
from django.contrib.admin.sites import AdminSite
|
||||
from django.contrib.auth.models import User
|
||||
from django.urls import reverse
|
||||
from django.test import TestCase
|
||||
from mock import Mock
|
||||
|
||||
from student.admin import UserAdmin
|
||||
from student.tests.factories import UserFactory
|
||||
from student.admin import COURSE_ENROLLMENT_ADMIN_SWITCH, UserAdmin
|
||||
from student.tests.factories import CourseEnrollmentFactory, UserFactory
|
||||
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
|
||||
@@ -186,3 +187,48 @@ class AdminUserPageTest(TestCase):
|
||||
request = Mock()
|
||||
user = Mock()
|
||||
self.assertIn('username', self.admin.get_readonly_fields(request, user))
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class CourseEnrollmentAdminTest(SharedModuleStoreTestCase):
|
||||
"""
|
||||
Unit tests for the CourseEnrollmentAdmin view.
|
||||
"""
|
||||
ADMIN_URLS = (
|
||||
('get', reverse('admin:student_courseenrollment_add')),
|
||||
('get', reverse('admin:student_courseenrollment_changelist')),
|
||||
('get', reverse('admin:student_courseenrollment_change', args=(1,))),
|
||||
('get', reverse('admin:student_courseenrollment_delete', args=(1,))),
|
||||
('post', reverse('admin:student_courseenrollment_add')),
|
||||
('post', reverse('admin:student_courseenrollment_changelist')),
|
||||
('post', reverse('admin:student_courseenrollment_change', args=(1,))),
|
||||
('post', reverse('admin:student_courseenrollment_delete', args=(1,))),
|
||||
)
|
||||
|
||||
def setUp(self):
|
||||
super(CourseEnrollmentAdminTest, self).setUp()
|
||||
self.user = UserFactory.create(is_staff=True, is_superuser=True)
|
||||
self.client.login(username=self.user.username, password='test')
|
||||
CourseEnrollmentFactory(
|
||||
user=self.user,
|
||||
course_id=CourseFactory().id, # pylint: disable=no-member
|
||||
)
|
||||
|
||||
@ddt.data(*ADMIN_URLS)
|
||||
@ddt.unpack
|
||||
def test_view_disabled(self, method, url):
|
||||
"""
|
||||
All CourseEnrollmentAdmin views are disabled by default.
|
||||
"""
|
||||
response = getattr(self.client, method)(url)
|
||||
self.assertEqual(response.status_code, 403)
|
||||
|
||||
@ddt.data(*ADMIN_URLS)
|
||||
@ddt.unpack
|
||||
def test_view_enabled(self, method, url):
|
||||
"""
|
||||
Ensure CourseEnrollmentAdmin views can be enabled with the waffle switch.
|
||||
"""
|
||||
with COURSE_ENROLLMENT_ADMIN_SWITCH.override(active=True):
|
||||
response = getattr(self.client, method)(url)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
Reference in New Issue
Block a user