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

Merge pull request #18654 from edx/ri/EDUCATOR-2120-allow-posts-community-ta

Browse Source 
EDUCATOR-2120 allow Community TA to see all discussion posts regardless of cohort and enrollment track
This commit is contained in:
Rabia Iftikhar
2018-08-10 00:22:18 +05:00
committed by GitHub
parent 2ff9bc66ec 1f34924573
commit 7933b5125a
5 changed files with 157 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -177,6 +177,33 @@ def make_mock_thread_data(
return thread_data
def make_mock_collection_data(
course,
text,
thread_id,
num_children=None,
group_id=None,
commentable_id=None,
thread_list=None
):
if thread_list:
return [
make_mock_thread_data(course=course, text=text, num_children=num_children, **thread)
for thread in thread_list
]
else:
return [
make_mock_thread_data(
course=course,
text=text,
thread_id=thread_id,
num_children=num_children,
group_id=group_id,
commentable_id=commentable_id,
)
]
def make_mock_perform_request_impl(
course,
text,
@@ -184,21 +211,15 @@ def make_mock_perform_request_impl(
group_id=None,
commentable_id=None,
num_thread_responses=1,
thread_list=None
):
def mock_perform_request_impl(*args, **kwargs):
url = args[1]
if url.endswith("threads") or url.endswith("user_profile"):
return {
"collection": [
make_mock_thread_data(
course=course,
text=text,
thread_id=thread_id,
num_children=None,
group_id=group_id,
commentable_id=commentable_id,
)
]
"collection": make_mock_collection_data(
course, text, thread_id, None, group_id, commentable_id, thread_list
)
}
elif thread_id and url.endswith(thread_id):
return make_mock_thread_data(
@@ -235,6 +256,7 @@ def make_mock_request_impl(
group_id=None,
commentable_id=None,
num_thread_responses=1,
thread_list=None,
):
impl = make_mock_perform_request_impl(
course,
@@ -242,7 +264,8 @@ def make_mock_request_impl(
thread_id=thread_id,
group_id=group_id,
commentable_id=commentable_id,
num_thread_responses=num_thread_responses
num_thread_responses=num_thread_responses,
thread_list=thread_list
)
def mock_request_impl(*args, **kwargs):
@@ -407,18 +430,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, 16, 4),
(ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 16, 4),
(ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 17, 5),
(ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 17, 5),
# split mongo: 3 queries, regardless of thread response size.
(ModuleStoreEnum.Type.split, False, 1, 3, 3, 16, 4),
(ModuleStoreEnum.Type.split, False, 50, 3, 3, 16, 4),
(ModuleStoreEnum.Type.split, False, 1, 3, 3, 17, 5),
(ModuleStoreEnum.Type.split, False, 50, 3, 3, 17, 5),
# Enabling Enterprise integration should have no effect on the number of mongo queries made.
(ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 16, 4),
(ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 16, 4),
(ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 17, 5),
(ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 17, 5),
# split mongo: 3 queries, regardless of thread response size.
(ModuleStoreEnum.Type.split, True, 1, 3, 3, 16, 4),
(ModuleStoreEnum.Type.split, True, 50, 3, 3, 16, 4),
(ModuleStoreEnum.Type.split, True, 1, 3, 3, 17, 5),
(ModuleStoreEnum.Type.split, True, 50, 3, 3, 17, 5),
)
@ddt.unpack
def test_number_of_mongo_queries(
@@ -675,6 +698,80 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, GroupIdAssertionMixin):
)
@patch('requests.request', autospec=True)
class ForumFormDiscussionContentGroupTestCase(ForumsEnableMixin, ContentGroupTestCase):
"""
Tests `forum_form_discussion api` works with different content groups.
Discussion modules are setup in ContentGroupTestCase class i.e
alpha_module => alpha_group_discussion => alpha_cohort => alpha_user/community_ta
beta_module => beta_group_discussion => beta_cohort => beta_user
"""
shard = 4
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
def setUp(self):
super(ForumFormDiscussionContentGroupTestCase, self).setUp()
self.thread_list = [
{"thread_id": "test_general_thread_id"},
{"thread_id": "test_global_group_thread_id", "commentable_id": self.global_module.discussion_id},
{"thread_id": "test_alpha_group_thread_id", "group_id": self.alpha_module.group_access[0][0], "commentable_id": self.alpha_module.discussion_id}, # pylint: disable=line-too-long
{"thread_id": "test_beta_group_thread_id", "group_id": self.beta_module.group_access[0][0], "commentable_id": self.beta_module.discussion_id} # pylint: disable=line-too-long
]
def assert_has_access(self, response, expected_discussion_threads):
"""
Verify that a users have access to the threads in their assigned
cohorts and non-cohorted modules.
"""
discussion_data = json.loads(response.content)['discussion_data']
self.assertEqual(len(discussion_data), expected_discussion_threads)
def call_view(self, mock_request, user):
mock_request.side_effect = make_mock_request_impl(
course=self.course,
text="dummy content",
thread_list=self.thread_list
)
self.client.login(username=user.username, password='test')
return self.client.get(
reverse("forum_form_discussion", args=[unicode(self.course.id)]),
HTTP_X_REQUESTED_WITH="XMLHttpRequest"
)
def test_community_ta_user(self, mock_request):
"""
Verify that community_ta user has access to all threads regardless
of cohort.
"""
response = self.call_view(
mock_request,
self.community_ta
)
self.assert_has_access(response, 4)
def test_alpha_cohort_user(self, mock_request):
"""
Verify that alpha_user has access to alpha_cohort and non-cohorted
threads.
"""
response = self.call_view(
mock_request,
self.alpha_user
)
self.assert_has_access(response, 3)
def test_beta_cohort_user(self, mock_request):
"""
Verify that beta_user has access to beta_cohort and non-cohorted
threads.
"""
response = self.call_view(
mock_request,
self.beta_user
)
self.assert_has_access(response, 3)
@patch('requests.request', autospec=True)
class SingleThreadContentGroupTestCase(ForumsEnableMixin, UrlResetMixin, ContentGroupTestCase):
shard = 4

5
lms/djangoapps/discussion/views.py
View File

@@ -149,7 +149,6 @@ def get_threads(request, course, user_info, discussion_id=None, per_page=THREADS
)
)
)
paginated_results = cc.Thread.search(query_params)
threads = paginated_results.collection
@@ -248,6 +247,7 @@ def forum_form_discussion(request, course_key):
Renders the main Discussion page, potentially filtered by a search query
"""
course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True)
request.user.is_community_ta = utils.is_user_community_ta(request.user, course.id)
if request.is_ajax():
user = cc.User.from_django_user(request.user)
user_info = user.to_dict()
@@ -292,7 +292,7 @@ def single_thread(request, course_key, discussion_id, thread_id):
Depending on the HTTP headers, we'll adjust our response accordingly.
"""
course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True)
request.user.is_community_ta = utils.is_user_community_ta(request.user, course.id)
if request.is_ajax():
cc_user = cc.User.from_django_user(request.user)
user_info = cc_user.to_dict()
@@ -350,7 +350,6 @@ def _find_thread(request, course, discussion_id, thread_id):
)
except cc.utils.CommentClientRequestError:
return None
# Verify that the student has access to this thread if belongs to a course discussion module
thread_context = getattr(thread, "context", "course")
if thread_context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id):

22
lms/djangoapps/django_comment_client/utils.py
View File

@@ -18,7 +18,13 @@ from courseware.access import has_access
from django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY
from django_comment_client.permissions import check_permissions_by_view, get_team, has_permission
from django_comment_client.settings import MAX_COMMENT_DEPTH
from django_comment_common.models import FORUM_ROLE_STUDENT, CourseDiscussionSettings, DiscussionsIdMapping, Role
from django_comment_common.models import (
FORUM_ROLE_STUDENT,
FORUM_ROLE_COMMUNITY_TA,
CourseDiscussionSettings,
DiscussionsIdMapping,
Role
)
from django_comment_common.utils import get_course_discussion_settings
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id, get_cohort_names, is_course_cohorted
from openedx.core.djangoapps.request_cache.middleware import request_cached
@@ -99,6 +105,13 @@ def has_forum_access(uname, course_id, rolename):
return role.users.filter(username=uname).exists()
def is_user_community_ta(user, course_id):
"""
Boolean operation to check whether a user's role is Community TA or not
"""
return has_forum_access(user, course_id, FORUM_ROLE_COMMUNITY_TA)
def has_required_keys(xblock):
"""
Returns True iff xblock has the proper attributes for generating metadata
@@ -120,6 +133,7 @@ def get_accessible_discussion_xblocks(course, user, include_all=False): # pylin
Return a list of all valid discussion xblocks in this course that
are accessible to the given user.
"""
include_all = getattr(user, 'is_community_ta', False)
return get_accessible_discussion_xblocks_by_course_id(course.id, user, include_all=include_all)
@@ -189,6 +203,7 @@ def get_cached_discussion_id_map_by_course_id(course_id, discussion_ids, user):
Returns a dict mapping discussion_ids to respective discussion xblock metadata if it is cached and visible to the
user. If not, returns the result of get_discussion_id_map
"""
include_all = getattr(user, 'is_community_ta', False)
try:
entries = []
for discussion_id in discussion_ids:
@@ -196,7 +211,7 @@ def get_cached_discussion_id_map_by_course_id(course_id, discussion_ids, user):
if not key:
continue
xblock = _get_item_from_modulestore(key)
if not (has_required_keys(xblock) and has_access(user, 'load', xblock, course_id)):
if not (has_required_keys(xblock) and (include_all or has_access(user, 'load', xblock, course_id))):
continue
entries.append(get_discussion_id_map_entry(xblock))
return dict(entries)
@@ -429,6 +444,7 @@ def discussion_category_id_access(course, user, discussion_id, xblock=None):
Uses the discussion id cache if available, falling back to
get_discussion_categories_ids if there is no cache.
"""
include_all = getattr(user, 'is_community_ta', False)
if discussion_id in course.top_level_discussion_topic_ids:
return True
try:
@@ -437,7 +453,7 @@ def discussion_category_id_access(course, user, discussion_id, xblock=None):
if not key:
return False
xblock = _get_item_from_modulestore(key)
return has_required_keys(xblock) and has_access(user, 'load', xblock, course.id)
return has_required_keys(xblock) and (include_all or has_access(user, 'load', xblock, course.id))
except DiscussionIdMapIsNotCached:
return discussion_id in get_discussion_categories_ids(course, user)

7
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
View File

@@ -1191,7 +1191,7 @@ class TestProblemReportCohortedContent(TestReportMixin, ContentGroupTestCase, In
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded')
self.assertDictContainsSubset(
{'action_name': 'graded', 'attempted': 4, 'succeeded': 4, 'failed': 0}, result
{'action_name': 'graded', 'attempted': 5, 'succeeded': 5, 'failed': 0}, result
)
problem_names = [u'Homework 1: Subsection - Problem0', u'Homework 1: Subsection - Problem1']
header_row = [u'Student ID', u'Email', u'Username', u'Enrollment Status', u'Grade']
@@ -1219,6 +1219,11 @@ class TestProblemReportCohortedContent(TestReportMixin, ContentGroupTestCase, In
'enrollment_status': ENROLLED_IN_COURSE,
'grade': [u'0.0', u'Not Available', u'Not Available', u'Not Available', u'Not Available'],
},
{
'user': self.community_ta,
'enrollment_status': ENROLLED_IN_COURSE,
'grade': [u'0.0', u'Not Attempted', u'2.0', u'Not Available', u'Not Available'],
},
]
# Verify generated grades and expected grades match

16
openedx/core/djangoapps/util/testing.py
View File

@@ -3,6 +3,8 @@
from datetime import datetime
from pytz import UTC
from django_comment_common.models import Role
from django_comment_common.utils import seed_permissions_roles
from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory
@@ -50,17 +52,27 @@ class ContentGroupTestCase(ModuleStoreTestCase):
discussion_topics={}
)
seed_permissions_roles(self.course.id)
self.staff_user = UserFactory.create(is_staff=True)
self.alpha_user = UserFactory.create()
self.beta_user = UserFactory.create()
self.non_cohorted_user = UserFactory.create()
for user in [self.staff_user, self.alpha_user, self.beta_user, self.non_cohorted_user]:
self.community_ta = UserFactory.create(username="community_ta")
self.community_ta.roles.add(Role.objects.get(name="Community TA", course_id=self.course.id))
for user in [
self.staff_user,
self.alpha_user,
self.beta_user,
self.non_cohorted_user,
self.community_ta
]:
CourseEnrollmentFactory.create(user=user, course_id=self.course.id)
alpha_cohort = CohortFactory(
course_id=self.course.id,
name='Cohort Alpha',
users=[self.alpha_user]
users=[self.alpha_user, self.community_ta]
)
beta_cohort = CohortFactory(
course_id=self.course.id,