From c606aaeb574ff659f9c6f37e106b76535e1b238a Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 29 Apr 2019 02:44:44 -0400 Subject: [PATCH] Discussion consolidation: quality --- .../tests/test_self_paced_overrides.py | 2 +- .../django_comment_client/base/__init__.py | 3 +- .../base/event_transformers.py | 1 + .../django_comment_client/base/tests.py | 1 + .../django_comment_client/base/views.py | 42 +++++++++---------- .../django_comment_client/middleware.py | 1 + .../django_comment_client/permissions.py | 1 + .../django_comment_client/settings.py | 6 ++- .../django_comment_client/tests/factories.py | 1 + .../django_comment_client/tests/group_id.py | 1 + .../tests/mock_cs_server/mock_cs_server.py | 1 + .../mock_cs_server/test_mock_cs_server.py | 1 + .../tests/test_middleware.py | 1 + .../django_comment_client/tests/test_utils.py | 11 ++--- .../django_comment_client/tests/unicode.py | 5 ++- .../discussion/django_comment_client/utils.py | 13 ++++-- .../management/commands/assign_role.py | 1 + .../commands/assign_roles_for_course.py | 1 + .../commands/get_discussion_link.py | 1 + .../management/commands/reload_forum_users.py | 1 + .../commands/seed_permissions_roles.py | 1 + .../management/commands/show_permissions.py | 1 + .../discussion/notification_prefs/__init__.py | 1 + .../features/unsubscribe.py | 1 + .../discussion/notification_prefs/tests.py | 1 + .../discussion/notification_prefs/views.py | 3 +- .../discussion/notifier_api/serializers.py | 1 + .../discussion/notifier_api/tests.py | 1 + .../discussion/notifier_api/views.py | 3 ++ lms/djangoapps/discussion/rest_api/api.py | 37 +++++++++------- lms/djangoapps/discussion/rest_api/forms.py | 6 +-- .../discussion/rest_api/serializers.py | 20 ++++++--- .../discussion/rest_api/tests/test_api.py | 6 ++- .../rest_api/tests/test_serializers.py | 6 ++- .../discussion/rest_api/tests/test_views.py | 4 +- .../discussion/rest_api/tests/utils.py | 2 +- lms/djangoapps/discussion/rest_api/urls.py | 3 +- lms/djangoapps/discussion/rest_api/views.py | 4 +- lms/djangoapps/discussion/tasks.py | 4 +- lms/djangoapps/discussion/views.py | 10 ++--- .../discussion/_filter_dropdown.html | 2 +- 41 files changed, 139 insertions(+), 73 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_self_paced_overrides.py b/lms/djangoapps/courseware/tests/test_self_paced_overrides.py index 27bc610dbb3a..e2b30ff00948 100644 --- a/lms/djangoapps/courseware/tests/test_self_paced_overrides.py +++ b/lms/djangoapps/courseware/tests/test_self_paced_overrides.py @@ -10,7 +10,7 @@ from courseware.tests.factories import BetaTesterFactory from lms.djangoapps.ccx.tests.test_overrides import inject_field_overrides from lms.djangoapps.courseware.field_overrides import OverrideFieldData, OverrideModulestoreFieldData -from lms.djangoapps.django_comment_client.utils import get_accessible_discussion_xblocks +from lms.djangoapps.discussion.django_comment_client.utils import get_accessible_discussion_xblocks from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory diff --git a/lms/djangoapps/discussion/django_comment_client/base/__init__.py b/lms/djangoapps/discussion/django_comment_client/base/__init__.py index 99f72e3428ae..46c35edb1df3 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/__init__.py +++ b/lms/djangoapps/discussion/django_comment_client/base/__init__.py @@ -1,2 +1,3 @@ +# pylint: disable=missing-docstring,relative-import # This import registers the ForumThreadViewedEventTransformer -import event_transformers # pylint: disable=relative-import +import event_transformers diff --git a/lms/djangoapps/discussion/django_comment_client/base/event_transformers.py b/lms/djangoapps/discussion/django_comment_client/base/event_transformers.py index 989d66970625..4f90f198ef4a 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/event_transformers.py +++ b/lms/djangoapps/discussion/django_comment_client/base/event_transformers.py @@ -1,3 +1,4 @@ +# pylint: skip-file """ Transformers for Discussion-related events. """ diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests.py b/lms/djangoapps/discussion/django_comment_client/base/tests.py index 5d03e364486d..e83c800af0c5 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py @@ -1,3 +1,4 @@ +# pylint: skip-file # -*- coding: utf-8 -*- """Tests for django comment client views.""" import json diff --git a/lms/djangoapps/discussion/django_comment_client/base/views.py b/lms/djangoapps/discussion/django_comment_client/base/views.py index 0a4c129b0741..5961d9881526 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/views.py +++ b/lms/djangoapps/discussion/django_comment_client/base/views.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring,unused-argument """Views for discussion forums.""" from __future__ import print_function @@ -20,11 +21,28 @@ from opaque_keys.edx.keys import CourseKey from six import text_type -import lms.djangoapps.discussion.django_comment_client.settings as cc_settings import django_comment_common.comment_client as cc +from django_comment_common.signals import ( + comment_created, + comment_deleted, + comment_edited, + comment_endorsed, + comment_voted, + thread_created, + thread_deleted, + thread_edited, + thread_voted, + thread_followed, + thread_unfollowed, +) +from django_comment_common.utils import ThreadContext from courseware.access import has_access from courseware.courses import get_course_by_id, get_course_overview_with_access, get_course_with_access -from lms.djangoapps.discussion.django_comment_client.permissions import check_permissions_by_view, get_team, has_permission +from lms.djangoapps.courseware.exceptions import CourseAccessRedirect +from lms.djangoapps.discussion.django_comment_client.permissions import ( + check_permissions_by_view, get_team, has_permission, +) +import lms.djangoapps.discussion.django_comment_client.settings as cc_settings from lms.djangoapps.discussion.django_comment_client.utils import ( JsonError, JsonResponse, @@ -38,22 +56,7 @@ is_comment_too_deep, prepare_content ) -from django_comment_common.signals import ( - comment_created, - comment_deleted, - comment_edited, - comment_endorsed, - comment_voted, - thread_created, - thread_deleted, - thread_edited, - thread_voted, - thread_followed, - thread_unfollowed, -) -from django_comment_common.utils import ThreadContext import eventtracking -from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from util.file import store_uploaded_file log = logging.getLogger(__name__) @@ -96,10 +99,7 @@ def track_created_event(request, event_name, course, obj, data): """ Send analytics event for a newly created thread, response or comment. """ - if len(obj.body) > TRACKING_MAX_FORUM_BODY: - data['truncated'] = True - else: - data['truncated'] = False + data['truncated'] = bool(len(obj.body) > TRACKING_MAX_FORUM_BODY) data['body'] = obj.body[:TRACKING_MAX_FORUM_BODY] track_forum_event(request, event_name, course, obj, data) diff --git a/lms/djangoapps/discussion/django_comment_client/middleware.py b/lms/djangoapps/discussion/django_comment_client/middleware.py index 0fb344fdc84b..d109afd9d788 100644 --- a/lms/djangoapps/discussion/django_comment_client/middleware.py +++ b/lms/djangoapps/discussion/django_comment_client/middleware.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring import json import logging diff --git a/lms/djangoapps/discussion/django_comment_client/permissions.py b/lms/djangoapps/discussion/django_comment_client/permissions.py index 4c4146cf50a8..cd777dd03036 100644 --- a/lms/djangoapps/discussion/django_comment_client/permissions.py +++ b/lms/djangoapps/discussion/django_comment_client/permissions.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring """ Module for checking permissions with the comment_client backend """ diff --git a/lms/djangoapps/discussion/django_comment_client/settings.py b/lms/djangoapps/discussion/django_comment_client/settings.py index b9a8d180818c..b812c3562928 100644 --- a/lms/djangoapps/discussion/django_comment_client/settings.py +++ b/lms/djangoapps/discussion/django_comment_client/settings.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring from django.conf import settings MAX_COMMENT_DEPTH = None @@ -7,4 +8,7 @@ if hasattr(settings, 'DISCUSSION_SETTINGS'): MAX_COMMENT_DEPTH = settings.DISCUSSION_SETTINGS.get('MAX_COMMENT_DEPTH') MAX_UPLOAD_FILE_SIZE = settings.DISCUSSION_SETTINGS.get('MAX_UPLOAD_FILE_SIZE') or MAX_UPLOAD_FILE_SIZE - ALLOWED_UPLOAD_FILE_TYPES = settings.DISCUSSION_SETTINGS.get('ALLOWED_UPLOAD_FILE_TYPES') or ALLOWED_UPLOAD_FILE_TYPES + ALLOWED_UPLOAD_FILE_TYPES = ( + settings.DISCUSSION_SETTINGS.get('ALLOWED_UPLOAD_FILE_TYPES') or + ALLOWED_UPLOAD_FILE_TYPES + ) diff --git a/lms/djangoapps/discussion/django_comment_client/tests/factories.py b/lms/djangoapps/discussion/django_comment_client/tests/factories.py index 5a22db6be24f..f95c92abbc79 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/factories.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/factories.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring from factory.django import DjangoModelFactory from django_comment_common.models import Permission, Role diff --git a/lms/djangoapps/discussion/django_comment_client/tests/group_id.py b/lms/djangoapps/discussion/django_comment_client/tests/group_id.py index a0ce2710f8dd..a3953d8213ba 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/group_id.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/group_id.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring import json import re diff --git a/lms/djangoapps/discussion/django_comment_client/tests/mock_cs_server/mock_cs_server.py b/lms/djangoapps/discussion/django_comment_client/tests/mock_cs_server/mock_cs_server.py index e482e0acc1ae..6061d320948e 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/mock_cs_server/mock_cs_server.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/mock_cs_server/mock_cs_server.py @@ -1,3 +1,4 @@ +# pylint: skip-file import json from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer from logging import getLogger diff --git a/lms/djangoapps/discussion/django_comment_client/tests/mock_cs_server/test_mock_cs_server.py b/lms/djangoapps/discussion/django_comment_client/tests/mock_cs_server/test_mock_cs_server.py index fd4cf3bfd949..98a0bdaf7ad6 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/mock_cs_server/test_mock_cs_server.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/mock_cs_server/test_mock_cs_server.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring import json import threading import unittest diff --git a/lms/djangoapps/discussion/django_comment_client/tests/test_middleware.py b/lms/djangoapps/discussion/django_comment_client/tests/test_middleware.py index 75477cb9ecb9..4574a89c3738 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/test_middleware.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/test_middleware.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring import json import django.http diff --git a/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py b/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py index 8df9ae808025..8a52ec3c3836 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/test_utils.py @@ -1,3 +1,4 @@ +# pylint: skip-file # -*- coding: utf-8 -*- import datetime import json @@ -12,15 +13,10 @@ from pytz import UTC from six import text_type -import lms.djangoapps.discussion.django_comment_client.utils as utils from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory from courseware.tabs import get_course_tab_list from courseware.tests.factories import InstructorFactory -from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY -from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory -from lms.djangoapps.discussion.django_comment_client.tests.unicode import UnicodeTestMixin -from lms.djangoapps.discussion.django_comment_client.tests.utils import config_course_discussions, topic_name_to_id from django_comment_common.comment_client.utils import CommentClientMaintenanceError, perform_request from django_comment_common.models import ( CourseDiscussionSettings, @@ -33,6 +29,11 @@ seed_permissions_roles, set_course_discussion_settings ) +from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY +from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory +from lms.djangoapps.discussion.django_comment_client.tests.unicode import UnicodeTestMixin +from lms.djangoapps.discussion.django_comment_client.tests.utils import config_course_discussions, topic_name_to_id +import lms.djangoapps.discussion.django_comment_client.utils as utils from lms.djangoapps.teams.tests.factories import CourseTeamFactory from openedx.core.djangoapps.course_groups import cohorts from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted diff --git a/lms/djangoapps/discussion/django_comment_client/tests/unicode.py b/lms/djangoapps/discussion/django_comment_client/tests/unicode.py index 700d31be15f8..a750b6ba0b61 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/unicode.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/unicode.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring # coding=utf-8 @@ -15,7 +16,9 @@ def test_non_BMP(self): self._test_unicode_data(u"𝕋𝕙𝕚𝕤 𝕡𝕠𝕤𝕥 𝕔𝕠𝕟𝕥𝕒𝕚𝕟𝕤 𝕔𝕙𝕒𝕣𝕒𝕔𝕥𝕖𝕣𝕤 𝕠𝕦𝕥𝕤𝕚𝕕𝕖 𝕥𝕙𝕖 𝔹𝕄ℙ") def test_special_chars(self): - self._test_unicode_data(u"\" This , post > contains < delimiter ] and [ other } special { characters ; that & may ' break things") + self._test_unicode_data( + u"\" This , post > contains < delimiter ] and [ other } special { characters ; that & may ' break things" + ) def test_string_interp(self): self._test_unicode_data(u"This post contains %s string interpolation #{syntax}") diff --git a/lms/djangoapps/discussion/django_comment_client/utils.py b/lms/djangoapps/discussion/django_comment_client/utils.py index 0afd062c2f60..3a58366b52b5 100644 --- a/lms/djangoapps/discussion/django_comment_client/utils.py +++ b/lms/djangoapps/discussion/django_comment_client/utils.py @@ -1,3 +1,4 @@ +# pylint: skip-file import json import logging from collections import defaultdict @@ -16,7 +17,9 @@ from courseware import courses from courseware.access import has_access from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY -from lms.djangoapps.discussion.django_comment_client.permissions import check_permissions_by_view, get_team, has_permission +from lms.djangoapps.discussion.django_comment_client.permissions import ( + check_permissions_by_view, get_team, has_permission, +) from lms.djangoapps.discussion.django_comment_client.settings import MAX_COMMENT_DEPTH from django_comment_common.models import ( FORUM_ROLE_STUDENT, @@ -271,7 +274,9 @@ def _filter_unstarted_categories(category_map, course): if key != "start_date": filtered_map["entries"][child][key] = unfiltered_map["entries"][child][key] else: - log.debug(u"Filtering out:%s with start_date: %s", child, unfiltered_map["entries"][child]["start_date"]) + log.debug( + u"Filtering out:%s with start_date: %s", child, unfiltered_map["entries"][child]["start_date"] + ) else: if course.self_paced or unfiltered_map["subcategories"][child]["start_date"] < now: filtered_map["children"].append((child, c_type)) @@ -568,7 +573,9 @@ def get_ability(course_id, content, user): user_group_id, content_user_group_id ), - 'can_reply': check_permissions_by_view(user, course_id, content, "create_comment" if content['type'] == 'thread' else "create_sub_comment"), + 'can_reply': check_permissions_by_view( + user, course_id, content, "create_comment" if content['type'] == 'thread' else "create_sub_comment", + ), 'can_delete': check_permissions_by_view( user, course_id, diff --git a/lms/djangoapps/discussion/management/commands/assign_role.py b/lms/djangoapps/discussion/management/commands/assign_role.py index 053a8777d643..40cadd511383 100644 --- a/lms/djangoapps/discussion/management/commands/assign_role.py +++ b/lms/djangoapps/discussion/management/commands/assign_role.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring from __future__ import print_function from django.contrib.auth.models import User diff --git a/lms/djangoapps/discussion/management/commands/assign_roles_for_course.py b/lms/djangoapps/discussion/management/commands/assign_roles_for_course.py index 4ea0295e01b2..855af4a75f6d 100644 --- a/lms/djangoapps/discussion/management/commands/assign_roles_for_course.py +++ b/lms/djangoapps/discussion/management/commands/assign_roles_for_course.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring """ This must be run only after seed_permissions_roles.py! diff --git a/lms/djangoapps/discussion/management/commands/get_discussion_link.py b/lms/djangoapps/discussion/management/commands/get_discussion_link.py index d6c659191cd3..455ada839ad0 100644 --- a/lms/djangoapps/discussion/management/commands/get_discussion_link.py +++ b/lms/djangoapps/discussion/management/commands/get_discussion_link.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring from courseware.courses import get_course from django.core.management.base import BaseCommand, CommandError from opaque_keys.edx.keys import CourseKey diff --git a/lms/djangoapps/discussion/management/commands/reload_forum_users.py b/lms/djangoapps/discussion/management/commands/reload_forum_users.py index a561722e3b91..bb7c79a55028 100644 --- a/lms/djangoapps/discussion/management/commands/reload_forum_users.py +++ b/lms/djangoapps/discussion/management/commands/reload_forum_users.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring,broad-except """ Reload forum (comment client) users from existing users. """ diff --git a/lms/djangoapps/discussion/management/commands/seed_permissions_roles.py b/lms/djangoapps/discussion/management/commands/seed_permissions_roles.py index 4266c6791556..5586df0d15dc 100644 --- a/lms/djangoapps/discussion/management/commands/seed_permissions_roles.py +++ b/lms/djangoapps/discussion/management/commands/seed_permissions_roles.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring """ Management command to seed default permissions and roles. """ diff --git a/lms/djangoapps/discussion/management/commands/show_permissions.py b/lms/djangoapps/discussion/management/commands/show_permissions.py index e1a40029831e..609e6a264f77 100644 --- a/lms/djangoapps/discussion/management/commands/show_permissions.py +++ b/lms/djangoapps/discussion/management/commands/show_permissions.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring,too-many-format-args from __future__ import print_function from django.contrib.auth.models import User diff --git a/lms/djangoapps/discussion/notification_prefs/__init__.py b/lms/djangoapps/discussion/notification_prefs/__init__.py index daed38e2b050..ea3cc6902102 100644 --- a/lms/djangoapps/discussion/notification_prefs/__init__.py +++ b/lms/djangoapps/discussion/notification_prefs/__init__.py @@ -1 +1,2 @@ +# pylint: disable=missing-docstring NOTIFICATION_PREF_KEY = "notification_pref" diff --git a/lms/djangoapps/discussion/notification_prefs/features/unsubscribe.py b/lms/djangoapps/discussion/notification_prefs/features/unsubscribe.py index 60219b4d5ee0..6cc72db3d0bb 100644 --- a/lms/djangoapps/discussion/notification_prefs/features/unsubscribe.py +++ b/lms/djangoapps/discussion/notification_prefs/features/unsubscribe.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring,unused-argument,no-member from django.contrib.auth.models import User from lettuce import step, world diff --git a/lms/djangoapps/discussion/notification_prefs/tests.py b/lms/djangoapps/discussion/notification_prefs/tests.py index f44e0bc1f09b..2f5504f6010f 100644 --- a/lms/djangoapps/discussion/notification_prefs/tests.py +++ b/lms/djangoapps/discussion/notification_prefs/tests.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring,consider-iterating-dictionary import json from django.contrib.auth.models import AnonymousUser diff --git a/lms/djangoapps/discussion/notification_prefs/views.py b/lms/djangoapps/discussion/notification_prefs/views.py index db21239f5748..7db557fa2a1b 100644 --- a/lms/djangoapps/discussion/notification_prefs/views.py +++ b/lms/djangoapps/discussion/notification_prefs/views.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring """ Views to support notification preferences. """ @@ -90,7 +91,7 @@ def decrypt(token): try: unpadded = unpadder.update(decrypted) + unpadder.finalize() - if len(unpadded) == 0: + if len(unpadded) == 0: # pylint: disable=len-as-condition raise UsernameDecryptionException("padding") return unpadded except ValueError: diff --git a/lms/djangoapps/discussion/notifier_api/serializers.py b/lms/djangoapps/discussion/notifier_api/serializers.py index e886e815609a..206d4958ecee 100644 --- a/lms/djangoapps/discussion/notifier_api/serializers.py +++ b/lms/djangoapps/discussion/notifier_api/serializers.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring from django.contrib.auth.models import User from django.http import Http404 from rest_framework import serializers diff --git a/lms/djangoapps/discussion/notifier_api/tests.py b/lms/djangoapps/discussion/notifier_api/tests.py index ea9e686d376d..a088b3077002 100644 --- a/lms/djangoapps/discussion/notifier_api/tests.py +++ b/lms/djangoapps/discussion/notifier_api/tests.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-docstring from __future__ import absolute_import import itertools diff --git a/lms/djangoapps/discussion/notifier_api/views.py b/lms/djangoapps/discussion/notifier_api/views.py index 71fe38cf036d..d09ecaeb000e 100644 --- a/lms/djangoapps/discussion/notifier_api/views.py +++ b/lms/djangoapps/discussion/notifier_api/views.py @@ -1,3 +1,6 @@ +""" +Django views for the Notifier. +""" from django.contrib.auth.models import User from rest_framework import pagination from rest_framework.response import Response diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index 4f4f4a8e6527..8b9b9ecae811 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -3,29 +3,17 @@ """ import itertools from collections import defaultdict +from enum import Enum from urllib import urlencode from urlparse import urlunparse from django.core.exceptions import ValidationError from django.urls import reverse from django.http import Http404 -from enum import Enum from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import CourseKey from rest_framework.exceptions import PermissionDenied -from courseware.courses import get_course_with_access -from lms.djangoapps.discussion.rest_api.exceptions import CommentNotFoundError, DiscussionDisabledError, ThreadNotFoundError -from lms.djangoapps.discussion.rest_api.forms import CommentActionsForm, ThreadActionsForm -from lms.djangoapps.discussion.rest_api.permissions import ( - can_delete, - get_editable_fields, - get_initializable_comment_fields, - get_initializable_thread_fields -) -from lms.djangoapps.discussion.rest_api.serializers import CommentSerializer, DiscussionTopicSerializer, ThreadSerializer, get_context -from lms.djangoapps.discussion.django_comment_client.base.views import track_comment_created_event, track_thread_created_event, track_voted_event -from lms.djangoapps.discussion.django_comment_client.utils import get_accessible_discussion_xblocks, get_group_id_for_user, is_commentable_divided from django_comment_common.comment_client.comment import Comment from django_comment_common.comment_client.thread import Thread from django_comment_common.comment_client.utils import CommentClientRequestError @@ -40,6 +28,27 @@ thread_voted ) from django_comment_common.utils import get_course_discussion_settings + +from lms.djangoapps.courseware.courses import get_course_with_access +from lms.djangoapps.discussion.rest_api.exceptions import ( + CommentNotFoundError, DiscussionDisabledError, ThreadNotFoundError, +) +from lms.djangoapps.discussion.rest_api.forms import CommentActionsForm, ThreadActionsForm +from lms.djangoapps.discussion.rest_api.permissions import ( + can_delete, + get_editable_fields, + get_initializable_comment_fields, + get_initializable_thread_fields +) +from lms.djangoapps.discussion.rest_api.serializers import ( + CommentSerializer, DiscussionTopicSerializer, ThreadSerializer, get_context, +) +from lms.djangoapps.discussion.django_comment_client.base.views import ( + track_comment_created_event, track_thread_created_event, track_voted_event, +) +from lms.djangoapps.discussion.django_comment_client.utils import ( + get_accessible_discussion_xblocks, get_group_id_for_user, is_commentable_divided, +) from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from lms.djangoapps.discussion.rest_api.pagination import DiscussionAPIPagination from openedx.core.djangoapps.user_api.accounts.views import AccountViewSet @@ -1042,7 +1051,7 @@ def get_response_comments(request, comment_id, page, page_size, requested_fields response_skip = page_size * (page - 1) paged_response_comments = response_comments[response_skip:(response_skip + page_size)] - if len(paged_response_comments) == 0 and page != 1: + if not paged_response_comments and page != 1: raise PageNotFoundError("Page not found (No results on this page).") results = _serialize_discussion_entities( diff --git a/lms/djangoapps/discussion/rest_api/forms.py b/lms/djangoapps/discussion/rest_api/forms.py index 346b83afec94..773b7a58de94 100644 --- a/lms/djangoapps/discussion/rest_api/forms.py +++ b/lms/djangoapps/discussion/rest_api/forms.py @@ -146,7 +146,7 @@ def clean_course_id(self): self.cleaned_data['course_key'] = course_key return course_id except InvalidKeyError: - raise ValidationError("'{}' is not a valid course key".format(text_type(course_id))) + raise ValidationError(u"'{}' is not a valid course key".format(text_type(course_id))) class CourseDiscussionRolesForm(CourseDiscussionSettingsForm): @@ -160,7 +160,7 @@ class CourseDiscussionRolesForm(CourseDiscussionSettingsForm): ) rolename = ChoiceField( ROLE_CHOICES, - error_messages={"invalid_choice": "Role '%(value)s' does not exist"} + error_messages={u"invalid_choice": u"Role '%(value)s' does not exist"} ) def clean_rolename(self): @@ -171,7 +171,7 @@ def clean_rolename(self): try: role = Role.objects.get(name=rolename, course_id=course_id) except Role.DoesNotExist: - raise ValidationError("Role '{}' does not exist".format(rolename)) + raise ValidationError(u"Role '{}' does not exist".format(rolename)) self.cleaned_data['role'] = role return rolename diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index 96d6f4ee5794..9e4d113926f2 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -9,10 +9,6 @@ from django.urls import reverse from rest_framework import serializers -from discussion.views import get_divided_discussions -from lms.djangoapps.discussion.rest_api.permissions import NON_UPDATABLE_COMMENT_FIELDS, NON_UPDATABLE_THREAD_FIELDS, get_editable_fields -from lms.djangoapps.discussion.rest_api.render import render_body -from lms.djangoapps.discussion.django_comment_client.utils import is_comment_too_deep, get_group_id_for_user, get_group_name from django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, @@ -24,7 +20,17 @@ from django_comment_common.comment_client.user import User as CommentClientUser from django_comment_common.comment_client.utils import CommentClientRequestError from django_comment_common.utils import get_course_discussion_settings -from lms.djangoapps.django_comment_client.utils import course_discussion_division_enabled, get_group_names_by_id +from lms.djangoapps.discussion.django_comment_client.utils import ( + is_comment_too_deep, get_group_id_for_user, get_group_name, +) +from lms.djangoapps.discussion.rest_api.permissions import ( + NON_UPDATABLE_COMMENT_FIELDS, NON_UPDATABLE_THREAD_FIELDS, get_editable_fields, +) +from lms.djangoapps.discussion.rest_api.render import render_body +from lms.djangoapps.discussion.views import get_divided_discussions +from lms.djangoapps.discussion.django_comment_client.utils import ( + course_discussion_division_enabled, get_group_names_by_id, +) from student.models import get_user_by_username_or_email @@ -75,6 +81,7 @@ def validate_not_blank(value): class _ContentSerializer(serializers.Serializer): + # pylint: disable=abstract-method """ A base class for thread and comment serializers. """ @@ -368,6 +375,7 @@ def get_children(self, obj): ] def to_representation(self, data): + # pylint: disable=arguments-differ data = super(CommentSerializer, self).to_representation(data) # Django Rest Framework v3 no longer includes None values @@ -529,7 +537,7 @@ def validate_user_id(self, user_id): self.user = get_user_by_username_or_email(user_id) return user_id except DjangoUser.DoesNotExist: - raise ValidationError("'{}' is not a valid student identifier".format(user_id)) + raise ValidationError(u"'{}' is not a valid student identifier".format(user_id)) def validate(self, attrs): """Validate the data at an object level.""" diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index 2a974a661c9f..fe8fdd83ec28 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -31,7 +31,9 @@ update_comment, update_thread ) -from lms.djangoapps.discussion.rest_api.exceptions import CommentNotFoundError, DiscussionDisabledError, ThreadNotFoundError +from lms.djangoapps.discussion.rest_api.exceptions import ( + CommentNotFoundError, DiscussionDisabledError, ThreadNotFoundError, +) from lms.djangoapps.discussion.rest_api.tests.utils import ( CommentsServiceMockMixin, make_minimal_cs_comment, @@ -1549,7 +1551,7 @@ def test_title_truncation(self, mock_emit): }) self.register_post_thread_response(cs_thread) with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)): - actual = create_thread(self.request, data) + create_thread(self.request, data) event_name, event_data = mock_emit.call_args[0] self.assertEqual(event_name, "edx.forum.thread.created") self.assertEqual( diff --git a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py index aab26b6a08e9..c6dca7f414ee 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py @@ -10,7 +10,9 @@ from django.test.client import RequestFactory from lms.djangoapps.discussion.rest_api.serializers import CommentSerializer, ThreadSerializer, get_context -from lms.djangoapps.discussion.rest_api.tests.utils import CommentsServiceMockMixin, make_minimal_cs_comment, make_minimal_cs_thread +from lms.djangoapps.discussion.rest_api.tests.utils import ( + CommentsServiceMockMixin, make_minimal_cs_comment, make_minimal_cs_thread, +) from lms.djangoapps.discussion.django_comment_client.tests.utils import ForumsEnableMixin from django_comment_common.comment_client.comment import Comment from django_comment_common.comment_client.thread import Thread @@ -737,7 +739,7 @@ def test_create_parent_id_wrong_thread(self): @ddt.data(None, -1, 0, 2, 5) def test_create_parent_id_too_deep(self, max_depth): - with mock.patch("django_comment_client.utils.MAX_COMMENT_DEPTH", max_depth): + with mock.patch("lms.djangoapps.discussion.django_comment_client.utils.MAX_COMMENT_DEPTH", max_depth): data = self.minimal_data.copy() context = get_context(self.course, self.request, make_minimal_cs_thread()) if max_depth is None or max_depth >= 0: diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index eb6de94a825a..a1e7cd3f740e 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -29,7 +29,9 @@ make_minimal_cs_thread, make_paginated_api_response ) -from lms.djangoapps.discussion.django_comment_client.tests.utils import ForumsEnableMixin, config_course_discussions, topic_name_to_id +from lms.djangoapps.discussion.django_comment_client.tests.utils import ( + ForumsEnableMixin, config_course_discussions, topic_name_to_id, +) from django_comment_common.models import CourseDiscussionSettings, Role from django_comment_common.utils import seed_permissions_roles from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index 4641ccc80721..23a230311959 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -521,7 +521,7 @@ def check_images(self, user, storage, exist=True): self.assertTrue(storage.exists(name)) with closing(Image.open(storage.path(name))) as img: self.assertEqual(img.size, (size, size)) - self.assertEqual(img.format, 'JPEG') + self.assertEqual(img.format, 'JPEG') # pylint: disable=no-member else: self.assertFalse(storage.exists(name)) diff --git a/lms/djangoapps/discussion/rest_api/urls.py b/lms/djangoapps/discussion/rest_api/urls.py index 8b7d6326398e..10332275d8f9 100644 --- a/lms/djangoapps/discussion/rest_api/urls.py +++ b/lms/djangoapps/discussion/rest_api/urls.py @@ -1,3 +1,4 @@ +# pylint: skip-file """ Discussion API URLs """ @@ -29,7 +30,7 @@ name="discussion_course_settings", ), url( - r'^v1/courses/{}/roles/(?P[A-Za-z0-9+ _-]+)/?$'.format( + r"^v1/courses/{}/roles/(?P[A-Za-z0-9+ _-]+)/?$".format( settings.COURSE_ID_PATTERN ), CourseDiscussionRolesAPIView.as_view(), diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 416aba79d9db..a6889465a4aa 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -17,13 +17,13 @@ from rest_framework.viewsets import ViewSet from six import text_type -from lms.djangoapps.discussion.django_comment_client.utils import available_division_schemes from django_comment_common import comment_client from django_comment_common.models import Role from django_comment_common.utils import get_course_discussion_settings, set_course_discussion_settings from instructor.access import update_forum_role from discussion.views import get_divided_discussions +from lms.djangoapps.discussion.django_comment_client.utils import available_division_schemes from lms.djangoapps.discussion.rest_api.api import ( create_comment, create_thread, @@ -930,7 +930,7 @@ def post(self, request, course_id, rolename): try: update_forum_role(course_id, user, rolename, action) except Role.DoesNotExist: - raise ValidationError("Role '{}' does not exist".format(rolename)) + raise ValidationError(u"Role '{}' does not exist".format(rolename)) role = form.cleaned_data['role'] data = {'course_id': course_id, 'users': role.users.all()} diff --git a/lms/djangoapps/discussion/tasks.py b/lms/djangoapps/discussion/tasks.py index 38b0f72bd6ed..28598b0d7061 100644 --- a/lms/djangoapps/discussion/tasks.py +++ b/lms/djangoapps/discussion/tasks.py @@ -17,7 +17,9 @@ from edx_ace.utils import date from edx_ace.recipient import Recipient from eventtracking import tracker -from lms.djangoapps.django_comment_client.utils import permalink, get_accessible_discussion_xblocks_by_course_id +from lms.djangoapps.discussion.django_comment_client.utils import ( + permalink, get_accessible_discussion_xblocks_by_course_id, +) from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index 4d5eaad55c87..5cef3fd0fa0b 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -23,12 +23,13 @@ from rest_framework import status from web_fragments.fragment import Fragment -import lms.djangoapps.discussion.django_comment_client.utils as utils -import django_comment_common.comment_client as cc -from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context from courseware.access import has_access from courseware.courses import get_course_with_access from courseware.views.views import CourseTabView +import django_comment_common.comment_client as cc +from django_comment_common.models import CourseDiscussionSettings +from django_comment_common.utils import ThreadContext, get_course_discussion_settings, set_course_discussion_settings +import lms.djangoapps.discussion.django_comment_client.utils as utils from lms.djangoapps.discussion.django_comment_client.base.views import track_thread_viewed_event from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY from lms.djangoapps.discussion.django_comment_client.permissions import get_team, has_permission @@ -43,8 +44,7 @@ is_commentable_divided, strip_none ) -from django_comment_common.models import CourseDiscussionSettings -from django_comment_common.utils import ThreadContext, get_course_discussion_settings, set_course_discussion_settings +from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context from openedx.core.djangoapps.plugin_api.views import EdxFragmentView from openedx.features.course_duration_limits.access import generate_course_expired_fragment from student.models import CourseEnrollment diff --git a/lms/templates/discussion/_filter_dropdown.html b/lms/templates/discussion/_filter_dropdown.html index 03fcfffc4c24..fa04ac8eb889 100644 --- a/lms/templates/discussion/_filter_dropdown.html +++ b/lms/templates/discussion/_filter_dropdown.html @@ -1,7 +1,7 @@ <%page expression_filter="h"/> <%! from django.utils.translation import ugettext as _ -from lms.djangoapps.django_comment_client.constants import TYPE_ENTRY +from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY from openedx.core.djangolib.markup import HTML %>