From c4cdb457de1b8827a31fc9cdc9cdf4a810c060cc Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 1 Nov 2013 13:59:22 -0400 Subject: [PATCH] Remove label from forum posts by global staff The motivation for this change is performance. The forums UI code gets the list of users for each role and renders the staff label based on those lists. The list for the staff role is expensive to compute because there is no index on the is_staff attribute, and we cannot create one because the User model is built into django. Users with is_staff=True are still assigned the Moderator role upon enrolling in a course, so this change will have no practical effect except that a user who is granted staff privileges after enrolling in a course will have to be made a Moderator in order for their posts to be labeled. Additionally, the UI did not use the list of users with the Student role, so that list has been removed as well. --- CHANGELOG.rst | 3 +++ common/static/coffee/src/discussion/utils.coffee | 2 +- .../django_comment_client/tests/test_utils.py | 7 ++++++- lms/djangoapps/django_comment_client/utils.py | 10 +++------- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9ba3178ac5..a29ca70a91 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,9 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +LMS: Users with is_staff=True no longer have the STAFF label appear on +their forum posts. + Blades: Video start and end times now function the same for both YouTube and HTML5 videos. If end time is set, the video can still play until the end, after it pauses on the end time. diff --git a/common/static/coffee/src/discussion/utils.coffee b/common/static/coffee/src/discussion/utils.coffee index dd7caeb033..193b6d1a6d 100644 --- a/common/static/coffee/src/discussion/utils.coffee +++ b/common/static/coffee/src/discussion/utils.coffee @@ -26,7 +26,7 @@ class @DiscussionUtil @loadFlagModerator($("#discussion-container").data("flag-moderator")) @isStaff: (user_id) -> - staff = _.union(@roleIds['Staff'], @roleIds['Moderator'], @roleIds['Administrator']) + staff = _.union(@roleIds['Moderator'], @roleIds['Administrator']) _.include(staff, parseInt(user_id)) @isTA: (user_id) -> diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index c275036319..4ba950ace6 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -39,6 +39,7 @@ class AccessUtilsTestCase(TestCase): self.course_id = 'edX/toy/2012_Fall' 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.student1 = UserFactory(username='student', email='student@edx.org') self.student1_enrollment = CourseEnrollmentFactory(user=self.student1) self.student_role.users.add(self.student1) @@ -47,10 +48,14 @@ class AccessUtilsTestCase(TestCase): 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_ta1 = UserFactory(username='community_ta1', email='community_ta1@edx.org') + self.community_ta_role.users.add(self.community_ta1) + self.community_ta2 = UserFactory(username='community_ta2', email='community_ta2@edx.org') + self.community_ta_role.users.add(self.community_ta2) def test_get_role_ids(self): ret = utils.get_role_ids(self.course_id) - expected = {u'Moderator': [3], u'Student': [1, 2], 'Staff': [3]} + expected = {u'Moderator': [3], u'Community TA': [4, 5]} self.assertEqual(ret, expected) def test_has_forum_access(self): diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 31d675c8cf..527a7d0860 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -9,7 +9,7 @@ from django.core.urlresolvers import reverse from django.db import connection from django.http import HttpResponse from django.utils import simplejson -from django_comment_common.models import Role +from django_comment_common.models import Role, FORUM_ROLE_STUDENT from django_comment_client.permissions import check_permissions_by_view import mitxmako @@ -42,12 +42,8 @@ def merge_dict(dic1, dic2): def get_role_ids(course_id): - roles = Role.objects.filter(course_id=course_id) - staff = list(User.objects.filter(is_staff=True).values_list('id', flat=True)) - roles_with_ids = {'Staff': staff} - for role in roles: - roles_with_ids[role.name] = list(role.users.values_list('id', flat=True)) - return roles_with_ids + roles = Role.objects.filter(course_id=course_id).exclude(name=FORUM_ROLE_STUDENT) + return dict([(role.name, list(role.users.values_list('id', flat=True))) for role in roles]) def has_forum_access(uname, course_id, rolename):