Skip to content

Commit

Permalink
Discussion consolidation: quality
Browse files Browse the repository at this point in the history
  • Loading branch information
nasthagiri committed May 3, 2019
1 parent 7b4b6c3 commit c606aae
Show file tree
Hide file tree
Showing 41 changed files with 139 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: skip-file
"""
Transformers for Discussion-related events.
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: skip-file
# -*- coding: utf-8 -*-
"""Tests for django comment client views."""
import json
Expand Down
42 changes: 21 additions & 21 deletions lms/djangoapps/discussion/django_comment_client/base/views.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring,unused-argument
"""Views for discussion forums."""

from __future__ import print_function
Expand All @@ -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,
Expand All @@ -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__)
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
import json
import logging

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
"""
Module for checking permissions with the comment_client backend
"""
Expand Down
6 changes: 5 additions & 1 deletion lms/djangoapps/discussion/django_comment_client/settings.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
from django.conf import settings

MAX_COMMENT_DEPTH = None
Expand All @@ -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
)
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
from factory.django import DjangoModelFactory

from django_comment_common.models import Permission, Role
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
import json
import re

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: skip-file
import json
from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer
from logging import getLogger
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
import json
import threading
import unittest
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
import json

import django.http
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: skip-file
# -*- coding: utf-8 -*-
import datetime
import json
Expand All @@ -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,
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
# coding=utf-8


Expand All @@ -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}")
13 changes: 10 additions & 3 deletions lms/djangoapps/discussion/django_comment_client/utils.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: skip-file
import json
import logging
from collections import defaultdict
Expand All @@ -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,
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
from __future__ import print_function

from django.contrib.auth.models import User
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
"""
This must be run only after seed_permissions_roles.py!
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring,broad-except
"""
Reload forum (comment client) users from existing users.
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
"""
Management command to seed default permissions and roles.
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring,too-many-format-args
from __future__ import print_function

from django.contrib.auth.models import User
Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/discussion/notification_prefs/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
# pylint: disable=missing-docstring
NOTIFICATION_PREF_KEY = "notification_pref"
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring,unused-argument,no-member
from django.contrib.auth.models import User
from lettuce import step, world

Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/discussion/notification_prefs/tests.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring,consider-iterating-dictionary
import json

from django.contrib.auth.models import AnonymousUser
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/discussion/notification_prefs/views.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
"""
Views to support notification preferences.
"""
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/discussion/notifier_api/serializers.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/discussion/notifier_api/tests.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=missing-docstring
from __future__ import absolute_import
import itertools

Expand Down
3 changes: 3 additions & 0 deletions lms/djangoapps/discussion/notifier_api/views.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
37 changes: 23 additions & 14 deletions lms/djangoapps/discussion/rest_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions lms/djangoapps/discussion/rest_api/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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
Loading

0 comments on commit c606aae

Please sign in to comment.