Address PR review:
* uses openedx waffle_utils instead of straight waffle * adds student.STUDENT_WAFFLE_NAMESPACE * improves comment Also changed the waffle switch to use _ instead of . to respect current conventions.
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')
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
""" Django admin pages for student app """
|
||||
from config_models.admin import ConfigurationModelAdmin
|
||||
import waffle
|
||||
from django import forms
|
||||
from django.contrib import admin
|
||||
from django.contrib.admin.sites import NotRegistered
|
||||
@@ -10,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,
|
||||
@@ -29,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."""
|
||||
@@ -180,14 +186,9 @@ class CourseEnrollmentAdmin(admin.ModelAdmin):
|
||||
|
||||
def has_permission(self, request, method):
|
||||
"""
|
||||
Returns True if the given method is allowed.
|
||||
|
||||
Access to these admin views is restricted because it makes DB quries that impact performance enough to cause a
|
||||
site outage, cf https://openedx.atlassian.net/browse/OPS-2943
|
||||
|
||||
Enable these views using the waffle switch: `student.coursenrollment.admin`
|
||||
Returns True if the given admin method is allowed.
|
||||
"""
|
||||
if waffle.switch_is_active('student.coursenrollment.admin'):
|
||||
if COURSE_ENROLLMENT_ADMIN_SWITCH.is_enabled():
|
||||
return getattr(super(CourseEnrollmentAdmin, self), method)(request)
|
||||
return False
|
||||
|
||||
|
||||
@@ -2,14 +2,13 @@
|
||||
Tests student admin.py
|
||||
"""
|
||||
import ddt
|
||||
from waffle.testutils import override_switch
|
||||
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.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
|
||||
@@ -224,13 +223,12 @@ class CourseEnrollmentAdminTest(SharedModuleStoreTestCase):
|
||||
response = getattr(self.client, method)(url)
|
||||
self.assertEqual(response.status_code, 403)
|
||||
|
||||
@override_switch('student.coursenrollment.admin', active=True)
|
||||
@ddt.data(*ADMIN_URLS)
|
||||
@ddt.unpack
|
||||
def test_view_enabled(self, method, url):
|
||||
"""
|
||||
Ensure CourseEnrollmentAdmin views can be enabled with the waffle flag.
|
||||
Ensure CourseEnrollmentAdmin views can be enabled with the waffle switch.
|
||||
"""
|
||||
self.client.login(username=self.user.username, password='test')
|
||||
response = getattr(self.client, method)(url)
|
||||
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