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

[TNL-7729] - Add check to discussion rest API to prevent users in blackout period.

Browse Source
This commit is contained in:
SaadYousaf
2021-02-11 16:29:48 +05:00
parent d5c4ca3c68
commit 50e71479ee
5 changed files with 289 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
lms/djangoapps/discussion/rest_api/api.py
View File

@@ -26,12 +26,13 @@ from lms.djangoapps.discussion.django_comment_client.base.views import (
from lms.djangoapps.discussion.django_comment_client.utils import (
get_accessible_discussion_xblocks,
get_group_id_for_user,
is_commentable_divided
is_commentable_divided,
)
from lms.djangoapps.discussion.rest_api.exceptions import (
CommentNotFoundError,
ThreadNotFoundError,
DiscussionDisabledError,
ThreadNotFoundError
DiscussionBlackOutException
)
from lms.djangoapps.discussion.rest_api.forms import CommentActionsForm, ThreadActionsForm
from lms.djangoapps.discussion.rest_api.pagination import DiscussionAPIPagination
@@ -47,6 +48,7 @@ from lms.djangoapps.discussion.rest_api.serializers import (
ThreadSerializer,
get_context
)
from lms.djangoapps.discussion.rest_api.utils import discussion_open_for_user
from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment
from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread
from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError
@@ -868,6 +870,9 @@ def create_thread(request, thread_data):
except InvalidKeyError:
raise ValidationError({"course_id": ["Invalid value."]}) # lint-amnesty, pylint: disable=raise-missing-from
if not discussion_open_for_user(course, user):
raise DiscussionBlackOutException
context = get_context(course, request)
_check_initializable_thread_fields(thread_data, context)
discussion_settings = get_course_discussion_settings(course_key)
@@ -913,8 +918,12 @@ def create_comment(request, comment_data):
raise ValidationError({"thread_id": ["This field is required."]})
cc_thread, context = _get_thread_and_context(request, thread_id)
course = context["course"]
if not discussion_open_for_user(course, request.user):
raise DiscussionBlackOutException
# if a thread is closed; no new comments could be made to it
if cc_thread['closed']:
if cc_thread["closed"]:
raise PermissionDenied
_check_initializable_comment_fields(comment_data, context)
@@ -928,7 +937,7 @@ def create_comment(request, comment_data):
api_comment = serializer.data
_do_extra_actions(api_comment, cc_comment, list(comment_data.keys()), actions_form, context, request)
track_comment_created_event(request, context["course"], cc_comment, cc_thread["commentable_id"], followed=False)
track_comment_created_event(request, course, cc_comment, cc_thread["commentable_id"], followed=False)
return api_comment

7
lms/djangoapps/discussion/rest_api/exceptions.py
View File

@@ -2,6 +2,7 @@
from django.core.exceptions import ObjectDoesNotExist
from rest_framework.exceptions import APIException
class DiscussionDisabledError(ObjectDoesNotExist):
@@ -17,3 +18,9 @@ class ThreadNotFoundError(ObjectDoesNotExist):
class CommentNotFoundError(ObjectDoesNotExist):
""" Comment was not found. """
pass # lint-amnesty, pylint: disable=unnecessary-pass
class DiscussionBlackOutException(APIException):
""" Discussions are in blackout period. """
status_code = 403
default_detail = 'Discussions are in blackout period.'

240
lms/djangoapps/discussion/rest_api/tests/test_api.py
View File

@@ -38,7 +38,8 @@ from lms.djangoapps.discussion.rest_api.api import (
from lms.djangoapps.discussion.rest_api.exceptions import (
CommentNotFoundError,
DiscussionDisabledError,
ThreadNotFoundError
ThreadNotFoundError,
DiscussionBlackOutException
)
from lms.djangoapps.discussion.rest_api.tests.utils import (
CommentsServiceMockMixin,
@@ -88,6 +89,19 @@ def _discussion_disabled_course_for(user):
return course_with_disabled_forums
def _assign_role_to_user(user, course_id, role):
"""
Unset the blackout period for course discussions.
Arguments:
user: User to assign role to
course_id: Course id of the course user will be assigned role in
role: Role assigned to user for course
"""
role = Role.objects.create(name=role, course_id=course_id)
role.users.set([user])
def _create_course_and_cohort_with_user_role(course_is_cohorted, user, role_name):
"""
Creates a course with the value of `course_is_cohorted`, plus `always_cohort_inline_discussions`
@@ -101,11 +115,26 @@ def _create_course_and_cohort_with_user_role(course_is_cohorted, user, role_name
)
CourseEnrollmentFactory.create(user=user, course_id=cohort_course.id)
cohort = CohortFactory.create(course_id=cohort_course.id, users=[user])
role = Role.objects.create(name=role_name, course_id=cohort_course.id)
role.users.set([user])
_assign_role_to_user(user=user, course_id=cohort_course.id, role=role_name)
return [cohort_course, cohort]
def _set_course_discussion_blackout(course, user_id):
"""
Set the blackout period for course discussions.
Arguments:
course: Course for which blackout period is set
user_id: User id of user enrolled in the course
"""
course.discussion_blackouts = [
datetime.now(UTC) - timedelta(days=3),
datetime.now(UTC) + timedelta(days=3)
]
modulestore().update_item(course, user_id)
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class GetCourseTest(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase):
"""Test for get_course"""
@@ -766,8 +795,7 @@ class GetThreadListTest(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMix
cohort_course = CourseFactory.create(cohort_config={"cohorted": course_is_cohorted})
CourseEnrollmentFactory.create(user=self.user, course_id=cohort_course.id)
CohortFactory.create(course_id=cohort_course.id, users=[self.user])
role = Role.objects.create(name=role_name, course_id=cohort_course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=cohort_course.id, role=role_name)
self.get_thread_list([], course=cohort_course)
actual_has_group = "group_id" in httpretty.last_request().querystring # lint-amnesty, pylint: disable=no-member
expected_has_group = (course_is_cohorted and role_name == FORUM_ROLE_STUDENT)
@@ -1082,8 +1110,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
)
CourseEnrollmentFactory.create(user=self.user, course_id=cohort_course.id)
cohort = CohortFactory.create(course_id=cohort_course.id, users=[self.user])
role = Role.objects.create(name=role_name, course_id=cohort_course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=cohort_course.id, role=role_name)
thread = self.make_minimal_cs_thread({
"course_id": six.text_type(cohort_course.id),
"commentable_id": "test_topic",
@@ -1470,14 +1497,10 @@ class CreateThreadTest(
'nonummy metus.'
)
@classmethod
def setUpClass(cls):
super(CreateThreadTest, cls).setUpClass()
cls.course = CourseFactory.create()
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
def setUp(self):
super(CreateThreadTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments
self.course = CourseFactory.create()
httpretty.reset()
httpretty.enable()
self.addCleanup(httpretty.reset)
@@ -1545,6 +1568,76 @@ class CreateThreadTest(
}
)
def test_basic_in_blackout_period(self):
"""
Test case when course is in blackout period and user does not have special privileges.
"""
_set_course_discussion_blackout(course=self.course, user_id=self.user.id)
with self.assertRaises(DiscussionBlackOutException) as assertion:
create_thread(self.request, self.minimal_data)
self.assertEqual(assertion.exception.status_code, 403)
self.assertEqual(assertion.exception.detail, "Discussions are in blackout period.")
@mock.patch("eventtracking.tracker.emit")
def test_basic_in_blackout_period_with_user_access(self, mock_emit):
"""
Test case when course is in blackout period and user has special privileges.
"""
cs_thread = make_minimal_cs_thread({
"id": "test_id",
"username": self.user.username,
"read": True,
})
self.register_post_thread_response(cs_thread)
_set_course_discussion_blackout(course=self.course, user_id=self.user.id)
_assign_role_to_user(user=self.user, course_id=self.course.id, role=FORUM_ROLE_MODERATOR)
with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)):
actual = create_thread(self.request, self.minimal_data)
expected = self.expected_thread_data({
"author_label": "Staff",
"id": "test_id",
"course_id": six.text_type(self.course.id),
"comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_id",
"read": True,
})
self.assertEqual(actual, expected)
self.assertEqual(
httpretty.last_request().parsed_body, # lint-amnesty, pylint: disable=no-member
{
"course_id": [six.text_type(self.course.id)],
"commentable_id": ["test_topic"],
"thread_type": ["discussion"],
"title": ["Test Title"],
"body": ["Test body"],
"user_id": [str(self.user.id)],
}
)
event_name, event_data = mock_emit.call_args[0]
self.assertEqual(event_name, "edx.forum.thread.created")
self.assertEqual(
event_data,
{
"commentable_id": "test_topic",
"group_id": None,
"thread_type": "discussion",
"title": "Test Title",
"title_truncated": False,
"anonymous": False,
"anonymous_to_peers": False,
"options": {"followed": False},
"id": "test_id",
"truncated": False,
"body": "Test body",
"url": "",
"user_forums_roles": [FORUM_ROLE_STUDENT, FORUM_ROLE_MODERATOR],
"user_course_roles": [],
}
)
@mock.patch("eventtracking.tracker.emit")
def test_title_truncation(self, mock_emit):
data = self.minimal_data.copy()
@@ -1614,8 +1707,7 @@ class CreateThreadTest(
CourseEnrollmentFactory.create(user=self.user, course_id=cohort_course.id)
if course_is_cohorted:
cohort = CohortFactory.create(course_id=cohort_course.id, users=[self.user])
role = Role.objects.create(name=role_name, course_id=cohort_course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=cohort_course.id, role=role_name)
self.register_post_thread_response({"username": self.user.username})
data = self.minimal_data.copy()
data["course_id"] = six.text_type(cohort_course.id)
@@ -1734,16 +1826,13 @@ class CreateCommentTest(
MockSignalHandlerMixin
):
"""Tests for create_comment"""
@classmethod
def setUpClass(cls):
super(CreateCommentTest, cls).setUpClass()
cls.course = CourseFactory.create()
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
def setUp(self):
super(CreateCommentTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments
httpretty.reset()
httpretty.enable()
self.course = CourseFactory.create()
self.addCleanup(httpretty.reset)
self.addCleanup(httpretty.disable)
self.user = UserFactory.create()
@@ -1842,6 +1931,103 @@ class CreateCommentTest(
self.assertEqual(actual_event_name, expected_event_name)
self.assertEqual(actual_event_data, expected_event_data)
@ddt.data(None, "test_parent")
@mock.patch("eventtracking.tracker.emit")
def test_success_in_black_out_with_user_access(self, parent_id, mock_emit):
"""
Test case when course is in blackout period and user has special privileges.
"""
if parent_id:
self.register_get_comment_response({"id": parent_id, "thread_id": "test_thread"})
self.register_post_comment_response(
{
"id": "test_comment",
"username": self.user.username,
"created_at": "2015-05-27T00:00:00Z",
"updated_at": "2015-05-27T00:00:00Z",
},
thread_id="test_thread",
parent_id=parent_id
)
data = self.minimal_data.copy()
if parent_id:
data["parent_id"] = parent_id
_set_course_discussion_blackout(course=self.course, user_id=self.user.id)
_assign_role_to_user(user=self.user, course_id=self.course.id, role=FORUM_ROLE_MODERATOR)
with self.assert_signal_sent(api, 'comment_created', sender=None, user=self.user, exclude_args=('post',)):
actual = create_comment(self.request, data)
expected = {
"id": "test_comment",
"thread_id": "test_thread",
"parent_id": parent_id,
"author": self.user.username,
"author_label": "Staff",
"created_at": "2015-05-27T00:00:00Z",
"updated_at": "2015-05-27T00:00:00Z",
"raw_body": "Test body",
"rendered_body": "<p>Test body</p>",
"endorsed": False,
"endorsed_by": None,
"endorsed_by_label": None,
"endorsed_at": None,
"abuse_flagged": False,
"voted": False,
"vote_count": 0,
"children": [],
"editable_fields": ["abuse_flagged", "endorsed", "raw_body", "voted"],
"child_count": 0,
}
self.assertEqual(actual, expected)
expected_url = (
"/api/v1/comments/{}".format(parent_id) if parent_id else
"/api/v1/threads/test_thread/comments"
)
self.assertEqual(
urlparse(httpretty.last_request().path).path, # lint-amnesty, pylint: disable=no-member
expected_url
)
self.assertEqual(
httpretty.last_request().parsed_body, # lint-amnesty, pylint: disable=no-member
{
"course_id": [six.text_type(self.course.id)],
"body": ["Test body"],
"user_id": [str(self.user.id)]
}
)
expected_event_name = (
"edx.forum.comment.created" if parent_id else
"edx.forum.response.created"
)
expected_event_data = {
"discussion": {"id": "test_thread"},
"commentable_id": "test_topic",
"options": {"followed": False},
"id": "test_comment",
"truncated": False,
"body": "Test body",
"url": "",
"user_forums_roles": [FORUM_ROLE_STUDENT, FORUM_ROLE_MODERATOR],
"user_course_roles": [],
}
if parent_id:
expected_event_data["response"] = {"id": parent_id}
actual_event_name, actual_event_data = mock_emit.call_args[0]
self.assertEqual(actual_event_name, expected_event_name)
self.assertEqual(actual_event_data, expected_event_data)
def test_error_in_black_out(self):
"""
Test case when course is in blackout period and user does not have special privileges.
"""
_set_course_discussion_blackout(course=self.course, user_id=self.user.id)
with self.assertRaises(DiscussionBlackOutException) as assertion:
create_comment(self.request, self.minimal_data)
self.assertEqual(assertion.exception.status_code, 403)
self.assertEqual(assertion.exception.detail, "Discussions are in blackout period.")
@ddt.data(
*itertools.product(
[
@@ -1856,8 +2042,7 @@ class CreateCommentTest(
)
@ddt.unpack
def test_endorsed(self, role_name, is_thread_author, thread_type):
role = Role.objects.create(name=role_name, course_id=self.course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_get_thread_response(
make_minimal_cs_thread({
"id": "test_thread",
@@ -2139,8 +2324,7 @@ class UpdateThreadTest(
FORUM_ROLE_STUDENT,
)
def test_author_only_fields(self, role_name):
role = Role.objects.create(name=role_name, course_id=self.course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_thread({"user_id": str(self.user.id + 1)})
data = {field: "edited" for field in ["topic_id", "title", "raw_body"]}
data["type"] = "question"
@@ -2547,8 +2731,7 @@ class UpdateCommentTest(
))
@ddt.unpack
def test_raw_body_access(self, role_name, is_thread_author, is_comment_author):
role = Role.objects.create(name=role_name, course_id=self.course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_comment(
{"user_id": str(self.user.id if is_comment_author else (self.user.id + 1))},
thread_overrides={
@@ -2579,8 +2762,7 @@ class UpdateCommentTest(
))
@ddt.unpack
def test_endorsed_access(self, role_name, is_thread_author, thread_type, is_comment_author):
role = Role.objects.create(name=role_name, course_id=self.course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_comment(
{"user_id": str(self.user.id if is_comment_author else (self.user.id + 1))},
thread_overrides={
@@ -2853,8 +3035,7 @@ class DeleteThreadTest(
FORUM_ROLE_STUDENT,
)
def test_non_author_delete_allowed(self, role_name):
role = Role.objects.create(name=role_name, course_id=self.course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_thread({"user_id": str(self.user.id + 1)})
expected_error = role_name == FORUM_ROLE_STUDENT
try:
@@ -3004,8 +3185,7 @@ class DeleteCommentTest(
FORUM_ROLE_STUDENT,
)
def test_non_author_delete_allowed(self, role_name):
role = Role.objects.create(name=role_name, course_id=self.course.id)
role.users.set([self.user])
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_comment_and_thread(
overrides={"user_id": str(self.user.id + 1)}
)

43
lms/djangoapps/discussion/rest_api/tests/test_utils.py Normal file
View File

@@ -0,0 +1,43 @@
"""
Tests for Discussion REST API utils.
"""
from datetime import datetime, timedelta
from pytz import UTC
from common.djangoapps.student.tests.factories import UserFactory, CourseEnrollmentFactory
from common.lib.xmodule.xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
from lms.djangoapps.discussion.rest_api.utils import discussion_open_for_user
from xmodule.modulestore.tests.factories import CourseFactory
class DiscussionAPIUtilsTestCase(ModuleStoreTestCase):
"""
Base test-case class for utils for Discussion REST API.
"""
CREATE_USER = False
def setUp(self):
super(DiscussionAPIUtilsTestCase, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments
self.course = CourseFactory.create()
self.course.discussion_blackouts = [datetime.now(UTC) - timedelta(days=3),
datetime.now(UTC) + timedelta(days=3)]
self.student_role = RoleFactory(name='Student', course_id=self.course.id)
self.moderator_role = RoleFactory(name='Moderator', course_id=self.course.id)
self.community_ta_role = RoleFactory(name='Community TA', course_id=self.course.id)
self.student = UserFactory(username='student', email='student@edx.org')
self.student_enrollment = CourseEnrollmentFactory(user=self.student)
self.student_role.users.add(self.student)
self.moderator = UserFactory(username='moderator', email='staff@edx.org', is_staff=True)
self.moderator_enrollment = CourseEnrollmentFactory(user=self.moderator)
self.moderator_role.users.add(self.moderator)
self.community_ta = UserFactory(username='community_ta1', email='community_ta1@edx.org')
self.community_ta_role.users.add(self.community_ta)
def test_discussion_open_for_user(self):
self.assertFalse(discussion_open_for_user(self.course, self.student))
self.assertTrue(discussion_open_for_user(self.course, self.moderator))
self.assertTrue(discussion_open_for_user(self.course, self.community_ta))

16
lms/djangoapps/discussion/rest_api/utils.py Normal file
View File

@@ -0,0 +1,16 @@
"""
Utils for discussion API.
"""
from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges
def discussion_open_for_user(course, user):
"""
Check if course discussion are open or not for user.
Arguments:
course: Course to check discussions for
user: User to check for privileges in course
"""
return course.forum_posts_allowed or has_discussion_privileges(user, course.id)