From 689d2084f5ed43a9ffde0386e0251f2939d749d5 Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Wed, 30 Jun 2021 12:17:32 +0530 Subject: [PATCH] Review feedback --- cms/djangoapps/contentstore/views/item.py | 1 - .../discussion/rest_api/serializers.py | 29 ++++++++++++++----- .../discussion/rest_api/tests/utils.py | 2 +- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index a0f6ef1f6d..3dcc3e4cec 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -30,7 +30,6 @@ from xblock.fields import Scope from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG from cms.djangoapps.models.settings.course_grading import CourseGradingModel -from cms.djangoapps.xblock_config.models import CourseEditLTIFieldsEnabledFlag from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW from common.djangoapps.edxmako.shortcuts import render_to_string from common.djangoapps.static_replace import replace_static_urls diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index be6eca5218..515938b14a 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -1,7 +1,7 @@ """ Discussion API serializers """ - +from typing import Dict from urllib.parse import urlencode, urlunparse from django.contrib.auth import get_user_model @@ -86,6 +86,24 @@ def validate_not_blank(value): raise ValidationError("This field may not be blank.") +def _validate_privileged_access(context: Dict) -> bool: + """ + Return the field specified by ``field_name`` if requesting user is privileged. + + Checks that the course exists in the context, and that the user has privileged + access. + + Args: + context (Dict): The serializer context. + + Returns: + bool: Course exists and the user has privileged access. + """ + course = context.get('course', None) + is_requester_privileged = context.get('is_requester_privileged') + return course and is_requester_privileged + + class _ContentSerializer(serializers.Serializer): # pylint: disable=abstract-method """ @@ -237,9 +255,7 @@ class ThreadSerializer(_ContentSerializer): """ Returns the number of users that flagged content as abusive only if user has staff permissions """ - course = self.context.get('course', None) - is_requester_privileged = self.context.get('is_requester_privileged') - if course and is_requester_privileged: + if _validate_privileged_access(self.context): return obj.get("abuse_flagged_count") def get_pinned(self, obj): @@ -408,10 +424,7 @@ class CommentSerializer(_ContentSerializer): Returns a boolean indicating whether any user has flagged the content as abusive. """ - course = self.context.get('course', None) - is_requester_privileged = self.context.get('is_requester_privileged') - - if course and is_requester_privileged: + if _validate_privileged_access(self.context): return len(obj.get("abuse_flaggers", [])) > 0 def validate(self, attrs): diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index 509555738b..c05e3231e3 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -358,7 +358,7 @@ class CommentsServiceMockMixin: """ actual_params = dict(httpretty_request.querystring) actual_params.pop("request_id") # request_id is random - assert actual_params == expected_params, f"""[\n\t{actual_params} \n\t{expected_params}\n]""" + assert actual_params == expected_params def assert_last_query_params(self, expected_params): """