Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #19342 from edx/bessiesteinberg/REVE-104-forum-mods

Browse Source 
REVE-104 & REVE-108: Ensure Course Roles' Access
This commit is contained in:
Matthew Piatetsky
2018-12-07 17:38:46 -05:00
committed by GitHub
parent e9d55a2110 14a899a65d
commit a55ed88378
22 changed files with 419 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
common/djangoapps/django_comment_common/models.py
View File

@@ -109,6 +109,13 @@ class Role(models.Model):
return self.permissions.filter(name=permission).exists()
@staticmethod
def user_has_role_for_course(user, course_id, role_names):
"""
Returns True if the user has one of the given roles for the given course
"""
return Role.objects.filter(course_id=course_id, name__in=role_names, users=user).exists()
class Permission(models.Model):
name = models.CharField(max_length=30, null=False, blank=False, primary_key=True)

10
lms/djangoapps/ccx/tests/test_field_override_performance.py
View File

@@ -240,7 +240,7 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
__test__ = True
# TODO: decrease query count as part of REVO-28
QUERY_COUNT = 33
QUERY_COUNT = 38
TEST_DATA = {
# (providers, course_width, enable_ccx, view_as_ccx): (
# # of sql queries to default,
@@ -269,7 +269,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True
# TODO: decrease query count as part of REVO-28
QUERY_COUNT = 33
QUERY_COUNT = 38
TEST_DATA = {
('no_overrides', 1, True, False): (QUERY_COUNT, 3),
('no_overrides', 2, True, False): (QUERY_COUNT, 3),
@@ -277,9 +277,9 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
('ccx', 1, True, False): (QUERY_COUNT, 3),
('ccx', 2, True, False): (QUERY_COUNT, 3),
('ccx', 3, True, False): (QUERY_COUNT, 3),
('ccx', 1, True, True): (34, 3),
('ccx', 2, True, True): (34, 3),
('ccx', 3, True, True): (34, 3),
('ccx', 1, True, True): (QUERY_COUNT + 1, 3),
('ccx', 2, True, True): (QUERY_COUNT + 1, 3),
('ccx', 3, True, True): (QUERY_COUNT + 1, 3),
('no_overrides', 1, False, False): (QUERY_COUNT, 3),
('no_overrides', 2, False, False): (QUERY_COUNT, 3),
('no_overrides', 3, False, False): (QUERY_COUNT, 3),

12
lms/djangoapps/course_api/blocks/tests/test_api.py
View File

@@ -162,7 +162,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase):
self._get_blocks(
course,
expected_mongo_queries=0,
expected_sql_queries=7 if with_storage_backing else 6,
expected_sql_queries=8 if with_storage_backing else 7,
)
@ddt.data(
@@ -179,9 +179,9 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase):
clear_course_from_cache(course.id)
if with_storage_backing:
num_sql_queries = 17
num_sql_queries = 18
else:
num_sql_queries = 7
num_sql_queries = 8
self._get_blocks(
course,
@@ -211,7 +211,7 @@ class TestQueryCountsWithIndividualOverrideProvider(TestGetBlocksQueryCountsBase
self._get_blocks(
course,
expected_mongo_queries=0,
expected_sql_queries=8 if with_storage_backing else 7,
expected_sql_queries=9 if with_storage_backing else 8,
)
@ddt.data(
@@ -228,9 +228,9 @@ class TestQueryCountsWithIndividualOverrideProvider(TestGetBlocksQueryCountsBase
clear_course_from_cache(course.id)
if with_storage_backing:
num_sql_queries = 18
num_sql_queries = 19
else:
num_sql_queries = 8
num_sql_queries = 9
self._get_blocks(
course,

4
lms/djangoapps/courseware/tests/test_access.py
View File

@@ -842,12 +842,12 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase):
if user_attr_name == 'user_staff' and action == 'see_exists':
# always checks staff role, and if the course has started, check the duration configuration
if course_attr_name == 'course_started':
num_queries = 5
num_queries = 3
else:
num_queries = 1
elif user_attr_name == 'user_normal' and action == 'see_exists':
if course_attr_name == 'course_started':
num_queries = 5
num_queries = 6
else:
# checks staff role and enrollment data
num_queries = 2

4
lms/djangoapps/courseware/tests/test_course_info.py
View File

@@ -436,8 +436,8 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest
def test_num_queries_instructor_paced(self):
# TODO: decrease query count as part of REVO-28
self.fetch_course_info_with_queries(self.instructor_paced_course, 40, 3)
self.fetch_course_info_with_queries(self.instructor_paced_course, 42, 3)
def test_num_queries_self_paced(self):
# TODO: decrease query count as part of REVO-28
self.fetch_course_info_with_queries(self.self_paced_course, 40, 3)
self.fetch_course_info_with_queries(self.self_paced_course, 42, 3)

12
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -213,8 +213,8 @@ class IndexQueryTestCase(ModuleStoreTestCase):
NUM_PROBLEMS = 20
@ddt.data(
(ModuleStoreEnum.Type.mongo, 10, 171),
(ModuleStoreEnum.Type.split, 4, 167),
(ModuleStoreEnum.Type.mongo, 10, 178),
(ModuleStoreEnum.Type.split, 4, 172),
)
@ddt.unpack
def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):
@@ -1440,8 +1440,8 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertContains(resp, u"Download Your Certificate")
@ddt.data(
(True, 53),
(False, 52)
(True, 58),
(False, 57)
)
@ddt.unpack
def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1454,8 +1454,8 @@ class ProgressPageTests(ProgressPageBaseTests):
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
@ddt.data(
(False, 60, 40),
(True, 52, 36)
(False, 65, 45),
(True, 57, 41)
)
@ddt.unpack
def test_progress_queries(self, enable_waffle, initial, subsequent):

16
lms/djangoapps/discussion/tests/test_views.py
View File

@@ -431,18 +431,18 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase):
# course is outside the context manager that is verifying the number of queries,
# and with split mongo, that method ends up querying disabled_xblocks (which is then
# cached and hence not queried as part of call_single_thread).
(ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 22, 7),
(ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 22, 7),
(ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 23, 8),
(ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 23, 8),
# split mongo: 3 queries, regardless of thread response size.
(ModuleStoreEnum.Type.split, False, 1, 3, 3, 22, 7),
(ModuleStoreEnum.Type.split, False, 50, 3, 3, 22, 7),
(ModuleStoreEnum.Type.split, False, 1, 3, 3, 23, 8),
(ModuleStoreEnum.Type.split, False, 50, 3, 3, 23, 8),
# Enabling Enterprise integration should have no effect on the number of mongo queries made.
(ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 22, 7),
(ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 22, 7),
(ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 23, 8),
(ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 23, 8),
# split mongo: 3 queries, regardless of thread response size.
(ModuleStoreEnum.Type.split, True, 1, 3, 3, 22, 7),
(ModuleStoreEnum.Type.split, True, 50, 3, 3, 22, 7),
(ModuleStoreEnum.Type.split, True, 1, 3, 3, 23, 8),
(ModuleStoreEnum.Type.split, True, 50, 3, 3, 23, 8),
)
@ddt.unpack
def test_number_of_mongo_queries(

8
lms/djangoapps/django_comment_client/base/tests.py
View File

@@ -403,8 +403,8 @@ class ViewsQueryCountTestCase(
return inner
@ddt.data(
(ModuleStoreEnum.Type.mongo, 3, 4, 40),
(ModuleStoreEnum.Type.split, 3, 13, 40),
(ModuleStoreEnum.Type.mongo, 3, 4, 41),
(ModuleStoreEnum.Type.split, 3, 13, 41),
)
@ddt.unpack
@count_queries
@@ -412,8 +412,8 @@ class ViewsQueryCountTestCase(
self.create_thread_helper(mock_request)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 3, 3, 36),
(ModuleStoreEnum.Type.split, 3, 10, 36),
(ModuleStoreEnum.Type.mongo, 3, 3, 37),
(ModuleStoreEnum.Type.split, 3, 10, 37),
)
@ddt.unpack
@count_queries

18
lms/djangoapps/grades/tests/test_course_grade_factory.py
View File

@@ -92,35 +92,35 @@ class TestCourseGradeFactory(GradeTestBase):
[self.sequence.display_name, self.sequence2.display_name]
)
with self.assertNumQueries(4), mock_get_score(1, 2):
with self.assertNumQueries(5), mock_get_score(1, 2):
_assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0
num_queries = 43
num_queries = 44
with self.assertNumQueries(num_queries), mock_get_score(1, 2):
grade_factory.update(self.request.user, self.course, force_update_subsections=True)
with self.assertNumQueries(4):
with self.assertNumQueries(5):
_assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5
num_queries = 8
num_queries = 9
with self.assertNumQueries(num_queries), mock_get_score(1, 4):
grade_factory.update(self.request.user, self.course, force_update_subsections=False)
with self.assertNumQueries(4):
with self.assertNumQueries(5):
_assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25
num_queries = 22
num_queries = 23
with self.assertNumQueries(num_queries), mock_get_score(2, 2):
grade_factory.update(self.request.user, self.course, force_update_subsections=True)
with self.assertNumQueries(4):
with self.assertNumQueries(5):
_assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0
num_queries = 25
num_queries = 26
with self.assertNumQueries(num_queries), mock_get_score(0, 0): # the subsection now is worth zero
grade_factory.update(self.request.user, self.course, force_update_subsections=True)
with self.assertNumQueries(4):
with self.assertNumQueries(5):
_assert_read(expected_pass=False, expected_percent=0.0) # updated to grade of 0.0
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})

20
lms/djangoapps/grades/tests/test_tasks.py
View File

@@ -176,10 +176,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
self.assertEquals(mock_block_structure_create.call_count, 1)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 1, 32, True),
(ModuleStoreEnum.Type.mongo, 1, 32, False),
(ModuleStoreEnum.Type.split, 3, 32, True),
(ModuleStoreEnum.Type.split, 3, 32, False),
(ModuleStoreEnum.Type.mongo, 1, 33, True),
(ModuleStoreEnum.Type.mongo, 1, 33, False),
(ModuleStoreEnum.Type.split, 3, 33, True),
(ModuleStoreEnum.Type.split, 3, 33, False),
)
@ddt.unpack
def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
@@ -191,8 +191,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
self._apply_recalculate_subsection_grade()
@ddt.data(
(ModuleStoreEnum.Type.mongo, 1, 32),
(ModuleStoreEnum.Type.split, 3, 32),
(ModuleStoreEnum.Type.mongo, 1, 33),
(ModuleStoreEnum.Type.split, 3, 33),
)
@ddt.unpack
def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
@@ -237,8 +237,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 1, 16),
(ModuleStoreEnum.Type.split, 3, 16),
(ModuleStoreEnum.Type.mongo, 1, 17),
(ModuleStoreEnum.Type.split, 3, 17),
)
@ddt.unpack
def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):
@@ -252,8 +252,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 1, 33),
(ModuleStoreEnum.Type.split, 3, 33),
(ModuleStoreEnum.Type.mongo, 1, 34),
(ModuleStoreEnum.Type.split, 3, 34),
)
@ddt.unpack
def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):

39
openedx/features/content_type_gating/helpers.py Normal file
View File

@@ -0,0 +1,39 @@
"""
Helper functions used by both content_type_gating and course_duration_limits.
"""
from django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_COMMUNITY_TA,
Role
)
from student.roles import (
CourseBetaTesterRole,
CourseInstructorRole,
CourseStaffRole,
OrgStaffRole,
OrgInstructorRole,
GlobalStaff
)
def has_staff_roles(user, course_key):
"""
Disable feature based enrollments for the enrollment if a user has any of the following roles
Staff, Instructor, Beta Tester, Forum Community TA, Forum Group Moderator, Forum Moderator, Forum Administrator
"""
forum_roles = [FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_MODERATOR, FORUM_ROLE_ADMINISTRATOR]
is_staff = CourseStaffRole(course_key).has_user(user)
is_instructor = CourseInstructorRole(course_key).has_user(user)
is_beta_tester = CourseBetaTesterRole(course_key).has_user(user)
is_org_staff = OrgStaffRole(course_key.org).has_user(user)
is_org_instructor = OrgInstructorRole(course_key.org).has_user(user)
is_global_staff = GlobalStaff().has_user(user)
has_forum_role = Role.user_has_role_for_course(user, course_key, forum_roles)
if any([is_staff, is_instructor, is_beta_tester, is_org_staff,
is_org_instructor, is_global_staff, has_forum_role]):
return True
return False

27
openedx/features/content_type_gating/models.py
View File

@@ -10,16 +10,17 @@ from django.db import models
from django.utils.encoding import python_2_unicode_compatible
from django.utils.translation import ugettext_lazy as _
from django.utils import timezone
from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_specific_student
from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_specific_student
from experiments.models import ExperimentData
from student.models import CourseEnrollment
from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel
from openedx.features.content_type_gating.helpers import has_staff_roles
from openedx.features.course_duration_limits.config import (
CONTENT_TYPE_GATING_FLAG,
EXPERIMENT_ID,
EXPERIMENT_DATA_HOLDBACK_KEY
)
from student.models import CourseEnrollment
@python_2_unicode_compatible
@@ -85,6 +86,24 @@ class ContentTypeGatingConfig(StackedConfigurationModel):
if enrollment is None:
enrollment = CourseEnrollment.get_enrollment(user, course_key)
if user is None and enrollment is not None:
user = enrollment.user
no_masquerade = get_course_masquerade(user, course_key) is None
student_masquerade = is_masquerading_as_specific_student(user, course_key)
# We can only use the user variable for the code below when the request is not in a masquerade state
# or is masquerading as a specific user.
# When a request is not in a masquerade state the user variable represents the correct user.
# When a request is in a masquerade state and not masquerading as a specific user,
# then then user variable will be the incorrect (original) user, not the masquerade user.
# If a request is masquerading as a specific user, the user variable will represent the correct user.
user_variable_represents_correct_user = (no_masquerade or student_masquerade)
if user and user.id:
# TODO: Move masquerade checks to enabled_for_enrollment from content_type_gating/partitions.py
# TODO: Consolidate masquerade checks into shared function like has_staff_roles below
if user_variable_represents_correct_user and has_staff_roles(user, course_key):
return False
# enrollment might be None if the user isn't enrolled. In that case,
# return enablement as if the user enrolled today
if enrollment is None:
@@ -93,9 +112,7 @@ class ContentTypeGatingConfig(StackedConfigurationModel):
# TODO: clean up as part of REV-100
experiment_data_holdback_key = EXPERIMENT_DATA_HOLDBACK_KEY.format(user)
is_in_holdback = False
no_masquerade = get_course_masquerade(user, course_key) is None
student_masquerade = is_masquerading_as_specific_student(user, course_key)
if user and (no_masquerade or student_masquerade):
if user and (user_variable_represents_correct_user):
try:
holdback_value = ExperimentData.objects.get(
user=user,

7
openedx/features/content_type_gating/partitions.py
View File

@@ -14,8 +14,8 @@ from django.apps import apps
from django.conf import settings
from django.template.loader import render_to_string
from django.utils.translation import ugettext_lazy as _
from web_fragments.fragment import Fragment
from lms.djangoapps.commerce.utils import EcommerceService
from lms.djangoapps.courseware.masquerade import (
get_course_masquerade,
@@ -25,7 +25,6 @@ from lms.djangoapps.courseware.masquerade import (
from xmodule.partitions.partitions import Group, UserPartition, UserPartitionError, ENROLLMENT_TRACK_PARTITION_ID
from openedx.core.lib.mobile_utils import is_request_from_mobile_app
from openedx.features.content_type_gating.models import ContentTypeGatingConfig
from student.roles import CourseBetaTesterRole
LOG = logging.getLogger(__name__)
@@ -171,10 +170,6 @@ class ContentTypeGatingPartitionScheme(object):
if not course_mode.has_verified_mode(modes_dict):
return cls.FULL_ACCESS
# If the user is a beta tester for this course they are granted FULL_ACCESS
if CourseBetaTesterRole(course_key).has_user(user):
return cls.FULL_ACCESS
course_enrollment = apps.get_model('student.CourseEnrollment')
mode_slug, is_active = course_enrollment.enrollment_mode_for_user(user, course_key)

105
openedx/features/content_type_gating/tests/test_access.py
View File

@@ -11,6 +11,14 @@ from django.urls import reverse
from django.utils import timezone
from mock import patch
from django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_COMMUNITY_TA,
Role
)
from django_comment_client.tests.factories import RoleFactory
from course_modes.tests.factories import CourseModeFactory
from experiments.models import ExperimentKeyValue
from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID
@@ -39,6 +47,14 @@ from student.tests.factories import (
UserFactory,
TEST_PASSWORD
)
from lms.djangoapps.courseware.tests.factories import (
InstructorFactory,
StaffFactory,
BetaTesterFactory,
OrgStaffFactory,
OrgInstructorFactory,
GlobalStaffFactory,
)
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
@@ -424,6 +440,7 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
BetaTesterFactory,
OrgStaffFactory,
OrgInstructorFactory,
GlobalStaffFactory,
)
def test_access_course_team_users(self, role_factory):
"""
@@ -432,7 +449,10 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
# There are two types of course team members: instructor and staff
# they have different privileges, but for the purpose of this test the important thing is that they should both
# have access to all graded content
user = role_factory.create(course_key=self.course.id)
if role_factory == GlobalStaffFactory:
user = role_factory.create()
else:
user = role_factory.create(course_key=self.course.id)
# assert that course team members have access to graded content
_assert_block_is_gated(
block=self.blocks_dict['problem'],
@@ -443,17 +463,19 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
)
@ddt.data(
GlobalStaffFactory,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_GROUP_MODERATOR
)
def test_access_global_users(self, role_factory):
def test_access_user_with_forum_role(self, role_name):
"""
Test that members of the course team do not lose access to graded content
Test that users with a given forum role do not lose access to graded content
"""
# There are two types of course team members: instructor and staff
# they have different privileges, but for the purpose of this test the important thing is that they should both
# have access to all graded content
user = role_factory.create()
# assert that course team members have access to graded content
user = UserFactory.create()
role = RoleFactory(name=role_name, course_id=self.course.id)
role.users.add(user)
_assert_block_is_gated(
block=self.blocks_dict['problem'],
user_id=user.id,
@@ -549,6 +571,71 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
self.assertEqual(response.status_code, 200)
return response
@ddt.data(
InstructorFactory,
StaffFactory,
BetaTesterFactory,
OrgStaffFactory,
OrgInstructorFactory,
GlobalStaffFactory,
)
def test_access_masquerade_as_course_team_users(self, role_factory):
"""
Test that when masquerading as members of the course team you do not lose access to graded content
"""
# There are two types of course team members: instructor and staff
# they have different privileges, but for the purpose of this test the important thing is that they should both
# have access to all graded content
staff_user = StaffFactory.create(password=TEST_PASSWORD, course_key=self.course.id)
CourseEnrollmentFactory.create(
user=staff_user,
course_id=self.course.id,
mode='audit'
)
self.client.login(username=staff_user.username, password=TEST_PASSWORD)
if role_factory == GlobalStaffFactory:
user = role_factory.create()
else:
user = role_factory.create(course_key=self.course.id)
self.update_masquerade(username=user.username)
block = self.blocks_dict['problem']
block_view_url = reverse('render_xblock', kwargs={'usage_key_string': unicode(block.scope_ids.usage_id)})
response = self.client.get(block_view_url)
self.assertEquals(response.status_code, 200)
@ddt.data(
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_GROUP_MODERATOR
)
def test_access_masquerade_as_user_with_forum_role(self, role_name):
"""
Test that when masquerading as a user with a given forum role you do not lose access to graded content
"""
staff_user = StaffFactory.create(password=TEST_PASSWORD, course_key=self.course.id)
CourseEnrollmentFactory.create(
user=staff_user,
course_id=self.course.id,
mode='audit'
)
self.client.login(username=staff_user.username, password=TEST_PASSWORD)
user = UserFactory.create()
role = RoleFactory(name=role_name, course_id=self.course.id)
role.users.add(user)
self.update_masquerade(username=user.username)
_assert_block_is_gated(
block=self.blocks_dict['problem'],
user_id=user.id,
course=self.course,
is_gated=False,
request_factory=self.factory,
)
@override_settings(FIELD_OVERRIDE_PROVIDERS=(
'openedx.features.content_type_gating.field_override.ContentTypeGatingFieldOverride',

6
openedx/features/content_type_gating/tests/test_models.py
View File

@@ -71,11 +71,11 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase):
course_key = self.course_overview.id
if already_enrolled and pass_enrollment:
query_count = 4
query_count = 7
elif not pass_enrollment and already_enrolled:
query_count = 6
query_count = 8
else:
query_count = 5
query_count = 7
with self.assertNumQueries(query_count):
enabled = ContentTypeGatingConfig.enabled_for_enrollment(

3
openedx/features/course_duration_limits/access.py
View File

@@ -12,7 +12,6 @@ from django.utils.translation import ugettext as _
from util.date_utils import DEFAULT_SHORT_DATE_FORMAT, strftime_localized
from course_modes.models import CourseMode
from lms.djangoapps.courseware.access_response import AccessError
from lms.djangoapps.courseware.access_utils import ACCESS_GRANTED
from lms.djangoapps.courseware.date_summary import verified_upgrade_deadline_link
@@ -122,7 +121,7 @@ def register_course_expired_message(request, course):
return
if is_masquerading_as_student(request.user, course.id) and timezone.now() > expiration_date:
upgrade_message = _('This learner would not have access to this course. '
upgrade_message = _('This learner does not have access to this course. '
'Their access expired on {expiration_date}.')
PageLevelMessages.register_warning_message(
request,

60
openedx/features/course_duration_limits/models.py
View File

@@ -11,14 +11,16 @@ from django.db import models
from django.utils.encoding import python_2_unicode_compatible
from django.utils.translation import ugettext_lazy as _
from django.utils import timezone
from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID
from course_modes.models import CourseMode
from lms.djangoapps.courseware.masquerade import get_masquerade_role, get_course_masquerade, \
is_masquerading_as_specific_student
from experiments.models import ExperimentData
from lms.djangoapps.courseware.masquerade import (
get_course_masquerade,
get_masquerade_role,
is_masquerading_as_specific_student
)
from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel
from openedx.features.content_type_gating.helpers import has_staff_roles
from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID, CONTENT_TYPE_GATE_GROUP_IDS
from openedx.features.course_duration_limits.config import (
CONTENT_TYPE_GATING_FLAG,
@@ -26,7 +28,7 @@ from openedx.features.course_duration_limits.config import (
EXPERIMENT_DATA_HOLDBACK_KEY
)
from student.models import CourseEnrollment
from student.roles import CourseBetaTesterRole, CourseInstructorRole, CourseStaffRole
from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID
@python_2_unicode_compatible
@@ -48,6 +50,34 @@ class CourseDurationLimitConfig(StackedConfigurationModel):
)
)
@classmethod
def has_full_access_role_in_masquerade(cls, user, course_key, course_masquerade):
"""
When masquerading, the course duration limits will never trigger the course to expire, redirecting the user.
The roles of the masquerade user are still used to determine whether the course duration limit banner displays.
Another banner also displays if the course is expired for the masquerade user.
Both banners will appear if the masquerade user does not have any of the following roles:
Staff, Instructor, Beta Tester, Forum Community TA, Forum Group Moderator, Forum Moderator, Forum Administrator
"""
masquerade_role = get_masquerade_role(user, course_key)
verified_mode_id = settings.COURSE_ENROLLMENT_MODES.get(CourseMode.VERIFIED, {}).get('id')
# Masquerading users can select the the role of a verified users without selecting a specific user
is_verified = (course_masquerade.user_partition_id == ENROLLMENT_TRACK_PARTITION_ID
and course_masquerade.group_id == verified_mode_id)
# Masquerading users can select the role of staff without selecting a specific user
is_staff = masquerade_role == 'staff'
# Masquerading users can select other full access roles for which content type gating is disabled
is_full_access = (course_masquerade.user_partition_id == CONTENT_GATING_PARTITION_ID
and course_masquerade.group_id == CONTENT_TYPE_GATE_GROUP_IDS['full_access'])
# When masquerading as a specific user, we can check that user's staff roles as we would with a normal user
is_staff_role = False
if course_masquerade.user_name:
is_staff_role = has_staff_roles(user, course_key)
if is_verified or is_full_access or is_staff or is_staff_role:
return True
return False
@classmethod
def enabled_for_enrollment(cls, enrollment=None, user=None, course_key=None):
"""
@@ -83,28 +113,16 @@ class CourseDurationLimitConfig(StackedConfigurationModel):
if enrollment is None:
enrollment = CourseEnrollment.get_enrollment(user, course_key)
# if the user is has a role of staff, instructor or beta tester their access should not expire
if user is None and enrollment is not None:
user = enrollment.user
if user:
if user and user.id:
course_masquerade = get_course_masquerade(user, course_key)
if course_masquerade:
verified_mode_id = settings.COURSE_ENROLLMENT_MODES.get(CourseMode.VERIFIED, {}).get('id')
is_verified = (course_masquerade.user_partition_id == ENROLLMENT_TRACK_PARTITION_ID
and course_masquerade.group_id == verified_mode_id)
is_full_access = (course_masquerade.user_partition_id == CONTENT_GATING_PARTITION_ID
and course_masquerade.group_id == CONTENT_TYPE_GATE_GROUP_IDS['full_access'])
is_staff = get_masquerade_role(user, course_key) == 'staff'
if is_verified or is_full_access or is_staff:
return False
else:
staff_role = CourseStaffRole(course_key).has_user(user)
instructor_role = CourseInstructorRole(course_key).has_user(user)
beta_tester_role = CourseBetaTesterRole(course_key).has_user(user)
if staff_role or instructor_role or beta_tester_role:
if cls.has_full_access_role_in_masquerade(user, course_key, course_masquerade):
return False
elif has_staff_roles(user, course_key):
return False
# enrollment might be None if the user isn't enrolled. In that case,
# return enablement as if the user enrolled today

121
openedx/features/course_duration_limits/tests/test_course_expiration.py
View File

@@ -11,8 +11,24 @@ import ddt
import mock
from course_modes.models import CourseMode
from django_comment_client.tests.factories import RoleFactory
from django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_COMMUNITY_TA
)
from experiments.models import ExperimentData
from lms.djangoapps.courseware.tests.factories import (
InstructorFactory,
StaffFactory,
BetaTesterFactory,
OrgStaffFactory,
OrgInstructorFactory,
GlobalStaffFactory,
)
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory
from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID, CONTENT_TYPE_GATE_GROUP_IDS
from openedx.features.course_duration_limits.access import get_user_course_expiration_date, MIN_DURATION, MAX_DURATION
from openedx.features.course_duration_limits.config import EXPERIMENT_ID, EXPERIMENT_DATA_HOLDBACK_KEY
@@ -20,7 +36,7 @@ from openedx.features.course_experience.tests.views.helpers import add_course_mo
from openedx.features.course_duration_limits.models import CourseDurationLimitConfig
from student.models import CourseEnrollment
from student.roles import CourseInstructorRole
from student.tests.factories import UserFactory, CourseEnrollmentFactory
from student.tests.factories import UserFactory, CourseEnrollmentFactory, TEST_PASSWORD
from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@@ -35,6 +51,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
start=now() - timedelta(weeks=10),
)
self.user = UserFactory()
self.THREE_YEARS_AGO = now() - timedelta(days=(365 * 3))
# Make this a verified course so we can test expiration date
add_course_mode(self.course, upgrade_deadline_expired=False)
@@ -264,5 +281,105 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
response = self.client.get(course_home_url, follow=True)
self.assertEqual(response.status_code, 200)
self.assertItemsEqual(response.redirect_chain, [])
banner_text = 'This learner would not have access to this course. Their access expired on'
banner_text = 'This learner does not have access to this course. Their access expired on'
self.assertIn(banner_text, response.content)
@mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@ddt.data(
InstructorFactory,
StaffFactory,
BetaTesterFactory,
OrgStaffFactory,
OrgInstructorFactory,
GlobalStaffFactory,
)
def test_no_banner_when_masquerading_as_staff(self, role_factory, mock_get_course_run_details):
"""
When masquerading as a specific expired user, if that user has a staff role
the expired course banner will not show up.
"""
mock_get_course_run_details.return_value = {'weeks_to_complete': 1}
if role_factory == GlobalStaffFactory:
expired_staff = role_factory.create(password=TEST_PASSWORD)
else:
expired_staff = role_factory.create(password=TEST_PASSWORD, course_key=self.course.id)
ScheduleFactory(
start=self.THREE_YEARS_AGO,
enrollment__mode=CourseMode.AUDIT,
enrollment__course_id=self.course.id,
enrollment__user=expired_staff
)
CourseDurationLimitConfig.objects.create(
enabled=True,
course=CourseOverview.get_from_id(self.course.id),
enabled_as_of=self.course.start,
)
staff_user = StaffFactory.create(password=TEST_PASSWORD, course_key=self.course.id)
CourseEnrollmentFactory.create(
user=staff_user,
course_id=self.course.id,
mode='audit'
)
self.client.login(username=staff_user.username, password='test')
self.update_masquerade(username=expired_staff.username)
course_home_url = reverse('openedx.course_experience.course_home', args=[unicode(self.course.id)])
response = self.client.get(course_home_url, follow=True)
self.assertEqual(response.status_code, 200)
self.assertItemsEqual(response.redirect_chain, [])
banner_text = 'This learner does not have access to this course. Their access expired on'
self.assertNotIn(banner_text, response.content)
@mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@ddt.data(
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_ADMINISTRATOR,
)
def test_no_banner_when_masquerading_as_forum_staff(self, role_name, mock_get_course_run_details):
"""
When masquerading as a specific expired user, if that user has a forum staff role
the expired course banner will not show up.
"""
mock_get_course_run_details.return_value = {'weeks_to_complete': 1}
expired_staff = UserFactory.create()
role = RoleFactory(name=role_name, course_id=self.course.id)
role.users.add(expired_staff)
ScheduleFactory(
start=self.THREE_YEARS_AGO,
enrollment__mode=CourseMode.AUDIT,
enrollment__course_id=self.course.id,
enrollment__user=expired_staff
)
CourseDurationLimitConfig.objects.create(
enabled=True,
course=CourseOverview.get_from_id(self.course.id),
enabled_as_of=self.course.start,
)
staff_user = StaffFactory.create(password=TEST_PASSWORD, course_key=self.course.id)
CourseEnrollmentFactory.create(
user=staff_user,
course_id=self.course.id,
mode='audit'
)
self.client.login(username=staff_user.username, password='test')
self.update_masquerade(username=expired_staff.username)
course_home_url = reverse('openedx.course_experience.course_home', args=[unicode(self.course.id)])
response = self.client.get(course_home_url, follow=True)
self.assertEqual(response.status_code, 200)
self.assertItemsEqual(response.redirect_chain, [])
banner_text = 'This learner does not have access to this course. Their access expired on'
self.assertNotIn(banner_text, response.content)

4
openedx/features/course_duration_limits/tests/test_models.py
View File

@@ -76,9 +76,9 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase):
user = self.user
course_key = self.course_overview.id
query_count = 6
query_count = 7
if not pass_enrollment and already_enrolled:
query_count = 7
query_count = 8
with self.assertNumQueries(query_count):
enabled = CourseDurationLimitConfig.enabled_for_enrollment(

39
openedx/features/course_experience/tests/views/test_course_home.py
View File

@@ -15,6 +15,13 @@ from pytz import UTC
from waffle.models import Flag
from waffle.testutils import override_flag
from django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_COMMUNITY_TA
)
from django_comment_client.tests.factories import RoleFactory
from course_modes.models import CourseMode
from course_modes.tests.factories import CourseModeFactory
from courseware.tests.helpers import get_expiration_banner_text
@@ -195,7 +202,7 @@ class TestCourseHomePage(CourseHomePageTestCase):
# Fetch the view and verify the query counts
# TODO: decrease query count as part of REVO-28
with self.assertNumQueries(78, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with self.assertNumQueries(86, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with check_mongo_calls(4):
url = course_home_url(self.course)
self.client.get(url)
@@ -374,7 +381,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
OrgStaffFactory,
OrgInstructorFactory,
)
def test_course_does_not_expire_for_course_roles(self, role_factory):
def test_course_does_not_expire_for_course_staff(self, role_factory):
"""
There are a number of different roles/users that should not lose access after the expiration date.
Ensure that users who should not lose access get a 200 (ok) response
@@ -391,7 +398,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
enrollment__user=user
)
# ensure that the user who has indefinite access
# ensure that the user has indefinite access
self.client.login(username=user.username, password=self.TEST_PASSWORD)
response = self.client.get(url)
self.assertEqual(
@@ -400,6 +407,32 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
"Should not expire access for user",
)
@ddt.data(
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_ADMINISTRATOR
)
def test_course_does_not_expire_for_user_with_course_role(self, role_name):
"""
Test that users with the above roles for a course do not lose access
"""
course = CourseFactory.create(start=THREE_YEARS_AGO)
url = course_home_url(course)
user = UserFactory.create()
role = RoleFactory(name=role_name, course_id=course.id)
role.users.add(user)
# ensure the user has indefinite access
self.client.login(username=user.username, password=self.TEST_PASSWORD)
response = self.client.get(url)
self.assertEqual(
response.status_code,
200,
"Should not expire access for user"
)
@mock.patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False})
@ddt.data(
GlobalStaffFactory,

2
openedx/features/course_experience/tests/views/test_course_updates.py
View File

@@ -130,7 +130,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase):
# Fetch the view and verify that the query counts haven't changed
# TODO: decrease query count as part of REVO-28
with self.assertNumQueries(52, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with self.assertNumQueries(56, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with check_mongo_calls(4):
url = course_updates_url(self.course)
self.client.get(url)

1
openedx/tests/settings.py
View File

@@ -54,6 +54,7 @@ DATABASES = {
FEATURES = {}
INSTALLED_APPS = (
'django_comment_common',
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',