Skip to content

E
edx-platform
Commit e1c7bb83 by Andy Armstrong
Options

Merge pull request #5046 from edx/andya/cohort-visibility-labels-2 

Implement forum cohort visibility labels correctly
parents c98ffa77 8349dbfa
Showing with 218 additions and 138 deletions
common/static/coffee/spec/discussion/discussion_spec_helper.coffee
......@@ -10,6 +10,10 @@ class @DiscussionSpecHelper
DiscussionUtil.roleIds["Moderator"].push(parseInt(window.user.id))
@setUnderscoreFixtures = ->
for templateName in ['thread-show']
templateFixture = readFixtures('templates/discussion/' + templateName + '.underscore')
appendSetFixtures($('<script>', { id: templateName + '-template', type: 'text/template' })
.text(templateFixture))
appendSetFixtures("""
<div id="fixture-element"></div>
......@@ -56,52 +60,6 @@ browser and pasting the output. When that file changes, this one should be rege
</article>
</script>
<script aria-hidden="true" type="text/template" id="thread-show-template">
<div class="discussion-post">
<header>
<% if (obj.group_id) { %>
<div class="group-visibility-label"><%- obj.group_string%></div>
<% } %>
<div class="post-header-content">
<h1><%- title %></h1>
<p class="posted-details">
<%- thread_type %> posted <span class='timeago' title='<%- created_at %>'><%- created_at %></span> by <%= author_display %>
</p>
<div class="post-labels">
<span class="post-label-pinned"><i class="icon icon-pushpin"></i>Pinned</span>
<span class="post-label-reported"><i class="icon icon-flag"></i>Reported</span>
<span class="post-label-closed"><i class="icon icon-lock"></i>Closed</span>
</div>
</div>
<div class="post-header-actions post-extended-content">
<%=
_.template(
$('#forum-actions').html(),
{
contentId: cid,
contentType: 'post',
primaryActions: ['vote', 'follow'],
secondaryActions: ['pin', 'edit', 'delete', 'report', 'close']
}
)
%>
</div>
</header>
<div class="post-body"><%- body %></div>
<% if (mode == "tab" && obj.courseware_url) { %>
<div class="post-context"><%
var courseware_link = interpolate('<a href="%s">%s</a>', [courseware_url, _.escape(courseware_title)]);
print(interpolate('(this post is about %(courseware_title_linked)s)', {'courseware_title_linked': courseware_link}, true));
%></div>
<% } %>
</div>
</script>
<script aria-hidden="true" type="text/template" id="thread-edit-template">
<div class="discussion-post edit-post-form">
<h1>Editing post</h1>
......
common/static/coffee/spec/discussion/view/discussion_thread_show_view_spec.coffee
......@@ -142,3 +142,18 @@ describe "DiscussionThreadShowView", ->
$el = $('#fixture-element').html(@view.getAuthorDisplay())
expect($el.find('a.username').length).toEqual(0)
expect($el.text()).toMatch(/^(\s*)anonymous(\s*)$/)
describe "cohorting", ->
it "renders correctly for an uncohorted thread", ->
@view.render()
expect(@view.$('.group-visibility-label').text().trim()).toEqual(
'This post is visible to everyone.'
)
it "renders correctly for a cohorted thread", ->
@thread.set('group_id', '1')
@thread.set('group_name', 'Mock Cohort')
@view.render()
expect(@view.$('.group-visibility-label').text().trim()).toEqual(
'This post is visible only to Mock Cohort.'
)
common/static/js_test.yml
......@@ -79,4 +79,4 @@ spec_paths:
fixture_paths:
- js/fixtures
- js/capa/fixtures
- templates/discussion
common/static/templates/discussion 0 → 120000
../../templates/js/discussion
\ No newline at end of file
common/templates/js/discussion/thread-show.underscore 0 → 100644
<div class="discussion-post">
<header>
<div class="group-visibility-label">
<% if (obj.group_name) { %>
<%-
interpolate(
gettext('This post is visible only to %(group_name)s.'),
{group_name: obj.group_name},
true
)
%>
<% } else { %>
<%- gettext('This post is visible to everyone.') %>
<% } %>
</div>
<div class="post-header-content">
<h1><%- title %></h1>
<p class="posted-details">
<%
var timeAgoHtml = interpolate(
'<span class="timeago" title="%(created_at)s">%(created_at)s</span>',
{created_at: created_at},
true
);
%>
<%=
interpolate(
// Translators: post_type describes the kind of post this is (e.g. "question" or "discussion");
// time_ago is how much time has passed since the post was created (e.g. "4 hours ago")
_.escape(gettext('%(post_type)s posted %(time_ago)s by %(author)s')),
{post_type: thread_type, time_ago: timeAgoHtml, author: author_display},
true
)
%>
</p>
<div class="post-labels">
<span class="post-label-pinned"><i class="icon icon-pushpin"></i><%- gettext("Pinned") %></span>
<span class="post-label-reported"><i class="icon icon-flag"></i><%- gettext("Reported") %></span>
<span class="post-label-closed"><i class="icon icon-lock"></i>$<%- gettext("Closed") %></span>
</div>
</div>
<div class="post-header-actions post-extended-content">
<%=
_.template(
$('#forum-actions').html(),
{
contentId: cid,
contentType: 'post',
primaryActions: ['vote', 'follow'],
secondaryActions: ['pin', 'edit', 'delete', 'report', 'close']
}
)
%>
</div>
</header>
<div class="post-body"><%- body %></div>
<% if (mode == "tab" && obj.courseware_url) { %>
<%
var courseware_title_linked = interpolate(
'<a href="%(courseware_url)s">%(courseware_title)s</a>',
{courseware_url: courseware_url, courseware_title: _.escape(courseware_title)},
true
);
%>
<div class="post-context">
<%=
interpolate(
_.escape(gettext('(this post is about %(courseware_title_linked)s)')),
{courseware_title_linked: courseware_title_linked},
true
)
%>
</div>
<% } %>
</div>
lms/djangoapps/django_comment_client/base/views.py
......@@ -27,8 +27,8 @@ from django_comment_client.utils import (
get_ability,
JsonError,
JsonResponse,
safe_content
)
safe_content,
add_thread_group_name)
from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
import lms.lib.comment_client as cc
......@@ -139,6 +139,7 @@ def create_thread(request, course_id, commentable_id):
user = cc.User.from_django_user(request.user)
user.follow(thread)
data = thread.to_dict()
add_thread_group_name(data, course_key)
add_courseware_context([data], course)
if request.is_ajax():
return ajax_content_response(request, course_key, data)
......
lms/djangoapps/django_comment_client/forum/tests.py
import json
import logging
from django.http import Http404
from django.test.utils import override_settings
from django.test.client import Client, RequestFactory
......@@ -15,7 +17,7 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from nose.tools import assert_true # pylint: disable=E0611
from mock import patch, Mock, ANY, call
import logging
from course_groups.models import CourseUserGroup
log = logging.getLogger(__name__)
......@@ -87,7 +89,7 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):
self.assertEqual(self.response.status_code, 404)
def make_mock_thread_data(text, thread_id, include_children):
def make_mock_thread_data(text, thread_id, include_children, group_id=None, group_name=None):
thread_data = {
"id": thread_id,
"type": "thread",
......@@ -98,6 +100,9 @@ def make_mock_thread_data(text, thread_id, include_children):
"resp_skip": 25,
"resp_limit": 5,
}
if group_id is not None:
thread_data['group_id'] = group_id
thread_data['group_name'] = group_name
if include_children:
thread_data["children"] = [{
"id": "dummy_comment_id",
......@@ -107,16 +112,16 @@ def make_mock_thread_data(text, thread_id, include_children):
return thread_data
def make_mock_request_impl(text, thread_id="dummy_thread_id"):
def make_mock_request_impl(text, thread_id="dummy_thread_id", group_id=None):
def mock_request_impl(*args, **kwargs):
url = args[1]
data = None
if url.endswith("threads"):
data = {
"collection": [make_mock_thread_data(text, thread_id, False)]
"collection": [make_mock_thread_data(text, thread_id, False, group_id=group_id)]
}
elif thread_id and url.endswith(thread_id):
data = make_mock_thread_data(text, thread_id, True)
data = make_mock_thread_data(text, thread_id, True, group_id=group_id)
elif "/users/" in url:
data = {
"default_sort_key": "date",
......@@ -262,6 +267,74 @@ class SingleThreadTestCase(ModuleStoreTestCase):
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@patch('requests.request')
class SingleCohortedThreadTestCase(ModuleStoreTestCase):
def setUp(self):
self.course = CourseFactory.create()
self.student = UserFactory.create()
CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
self.student_cohort = CourseUserGroup.objects.create(
name="student_cohort",
course_id=self.course.id,
group_type=CourseUserGroup.COHORT
)
def _create_mock_cohorted_thread(self, mock_request):
self.mock_text = "dummy content"
self.mock_thread_id = "test_thread_id"
mock_request.side_effect = make_mock_request_impl(
self.mock_text, self.mock_thread_id,
group_id=self.student_cohort.id
)
def test_ajax(self, mock_request):
self._create_mock_cohorted_thread(mock_request)
request = RequestFactory().get(
"dummy_url",
HTTP_X_REQUESTED_WITH="XMLHttpRequest"
)
request.user = self.student
response = views.single_thread(
request,
self.course.id.to_deprecated_string(),
"dummy_discussion_id",
self.mock_thread_id
)
self.assertEquals(response.status_code, 200)
response_data = json.loads(response.content)
self.assertEquals(
response_data["content"],
make_mock_thread_data(
self.mock_text, self.mock_thread_id, True,
group_id=self.student_cohort.id,
group_name=self.student_cohort.name,
)
)
def test_html(self, mock_request):
self._create_mock_cohorted_thread(mock_request)
request = RequestFactory().get("dummy_url")
request.user = self.student
mako_middleware_process_request(request)
response = views.single_thread(
request,
self.course.id.to_deprecated_string(),
"dummy_discussion_id",
self.mock_thread_id
)
self.assertEquals(response.status_code, 200)
self.assertEqual(response['Content-Type'], 'text/html; charset=utf-8')
html = response.content
# Verify that the group name is correctly included in the HTML
self.assertRegexpMatches(html, r'&quot;group_name&quot;: &quot;student_cohort&quot;')
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@patch('requests.request')
class UserProfileTestCase(ModuleStoreTestCase):
TEST_THREAD_TEXT = 'userprofile-test-text'
......
lms/djangoapps/django_comment_client/forum/views.py
......@@ -17,7 +17,7 @@ from course_groups.cohorts import (is_course_cohorted, get_cohort_id, is_comment
from courseware.access import has_access
from django_comment_client.permissions import cached_has_permission
from django_comment_client.utils import (merge_dict, extract, strip_none, add_courseware_context)
from django_comment_client.utils import (merge_dict, extract, strip_none, add_courseware_context, add_thread_group_name)
import django_comment_client.utils as utils
import lms.lib.comment_client as cc
......@@ -55,7 +55,7 @@ def make_course_settings(course, include_category_map=False):
return obj
@newrelic.agent.function_trace()
def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAGE):
def get_threads(request, course_key, discussion_id=None, per_page=THREADS_PER_PAGE):
"""
This may raise an appropriate subclass of cc.utils.CommentClientError
if something goes wrong.
......@@ -67,7 +67,7 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
'sort_order': 'desc',
'text': '',
'commentable_id': discussion_id,
'course_id': course_id.to_deprecated_string(),
'course_id': course_key.to_deprecated_string(),
'user_id': request.user.id,
}
......@@ -87,7 +87,7 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
#is user a moderator
#did the user request a group
#if the user requested a group explicitly, give them that group, othewrise, if mod, show all, else if student, use cohort
#if the user requested a group explicitly, give them that group, otherwise, if mod, show all, else if student, use cohort
group_id = request.GET.get('group_id')
......@@ -95,8 +95,8 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
group_id = None
if not group_id:
if not cached_has_permission(request.user, "see_all_cohorts", course_id):
group_id = get_cohort_id(request.user, course_id)
if not cached_has_permission(request.user, "see_all_cohorts", course_key):
group_id = get_cohort_id(request.user, course_key)
if group_id:
default_query_params["group_id"] = group_id
......@@ -126,13 +126,7 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
#now add the group name if the thread has a group id
for thread in threads:
if thread.get('group_id'):
thread['group_name'] = get_cohort_by_id(course_id, thread.get('group_id')).name
thread['group_string'] = "This post visible only to Group %s." % (thread['group_name'])
else:
thread['group_name'] = ""
thread['group_string'] = "This post visible to everyone."
add_thread_group_name(thread, course_key)
#patch for backward compatibility to comments service
if not 'pinned' in thread:
......@@ -239,10 +233,10 @@ def forum_form_discussion(request, course_id):
@require_GET
@login_required
def single_thread(request, course_id, discussion_id, thread_id):
course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
nr_transaction = newrelic.agent.current_transaction()
course = get_course_with_access(request.user, 'load_forum', course_id)
course = get_course_with_access(request.user, 'load_forum', course_key)
course_settings = make_course_settings(course, include_category_map=True)
cc_user = cc.User.from_django_user(request.user)
user_info = cc_user.to_dict()
......@@ -265,8 +259,9 @@ def single_thread(request, course_id, discussion_id, thread_id):
is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)
if request.is_ajax():
with newrelic.agent.FunctionTrace(nr_transaction, "get_annotated_content_infos"):
annotated_content_info = utils.get_annotated_content_infos(course_id, thread, request.user, user_info=user_info)
content = utils.safe_content(thread.to_dict(), course_id, is_staff)
annotated_content_info = utils.get_annotated_content_infos(course_key, thread, request.user, user_info=user_info)
content = utils.safe_content(thread.to_dict(), course_key, is_staff)
add_thread_group_name(content, course_key)
with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
add_courseware_context([content], course)
return utils.JsonResponse({
......@@ -275,27 +270,26 @@ def single_thread(request, course_id, discussion_id, thread_id):
})
else:
threads, query_params = get_threads(request, course_id)
threads, query_params = get_threads(request, course_key)
threads.append(thread.to_dict())
with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
add_courseware_context(threads, course)
for thread in threads:
if thread.get('group_id') and not thread.get('group_name'):
thread['group_name'] = get_cohort_by_id(course_id, thread.get('group_id')).name
add_thread_group_name(thread, course_key)
#patch for backward compatibility with comments service
if not "pinned" in thread:
thread["pinned"] = False
threads = [utils.safe_content(thread, course_id, is_staff) for thread in threads]
threads = [utils.safe_content(thread, course_key, is_staff) for thread in threads]
with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"):
annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_info)
annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info)
with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"):
user_cohort = get_cohort_id(request.user, course_id)
user_cohort = get_cohort_id(request.user, course_key)
context = {
'discussion_id': discussion_id,
......@@ -308,10 +302,10 @@ def single_thread(request, course_id, discussion_id, thread_id):
'course_id': course.id.to_deprecated_string(), # TODO: Why pass both course and course.id to template?
'thread_id': thread_id,
'threads': _attr_safe_json(threads),
'roles': _attr_safe_json(utils.get_role_ids(course_id)),
'is_moderator': cached_has_permission(request.user, "see_all_cohorts", course_id),
'roles': _attr_safe_json(utils.get_role_ids(course_key)),
'is_moderator': cached_has_permission(request.user, "see_all_cohorts", course_key),
'thread_pages': query_params['num_pages'],
'is_course_cohorted': is_course_cohorted(course_id),
'is_course_cohorted': is_course_cohorted(course_key),
'flag_moderator': cached_has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, 'staff', course),
'cohorts': course_settings["cohorts"],
'user_cohort': user_cohort,
......
lms/djangoapps/django_comment_client/utils.py
......@@ -3,6 +3,7 @@ from collections import defaultdict
import logging
from datetime import datetime
from course_groups.cohorts import get_cohort_by_id
from django.contrib.auth.models import User
from django.core.urlresolvers import reverse
from django.db import connection
......@@ -371,7 +372,7 @@ def safe_content(content, course_id, is_staff=False):
'updated_at', 'depth', 'type', 'commentable_id', 'comments_count',
'at_position_list', 'children', 'highlighted_title', 'highlighted_body',
'courseware_title', 'courseware_url', 'unread_comments_count',
'read', 'group_id', 'group_name', 'group_string', 'pinned', 'abuse_flaggers',
'read', 'group_id', 'group_name', 'pinned', 'abuse_flaggers',
'stats', 'resp_skip', 'resp_limit', 'resp_total', 'thread_type',
'endorsed_responses', 'non_endorsed_responses', 'non_endorsed_resp_total',
'endorsement',
......@@ -411,3 +412,11 @@ def safe_content(content, course_id, is_staff=False):
content[child_content_key] = safe_children
return content
def add_thread_group_name(thread_info, course_key):
"""
Augment the specified thread info to include the group name if a group id is present.
"""
if thread_info.get('group_id') is not None:
thread_info['group_name'] = get_cohort_by_id(course_key, thread_info.get('group_id')).name
lms/templates/discussion/_underscore_templates.html
<%! from django.utils.translation import ugettext as _ %>
<%! from django.template.defaultfilters import escapejs %>
<%! from django_comment_client.permissions import has_permission %>
<%namespace name='static' file='../static_content.html'/>
## IMPORTANT: In order to keep js tests valid and relevant, please be sure to update the appropriate HTML in
## common/static/coffee/spec/discussion_spec_helper.coffee is changed and regenerated, whenever this one changes.
<script aria-hidden="true" type="text/template" id="thread-template">
......@@ -44,64 +47,11 @@
</article>
</script>
<script aria-hidden="true" type="text/template" id="thread-show-template">
<div class="discussion-post">
<header>
${"<% if (obj.group_id) { %>"}
<div class="group-visibility-label">${"<%- obj.group_string%>"}</div>
${"<% } %>"}
<div class="post-header-content">
<h1>${'<%- title %>'}</h1>
<p class="posted-details">
## This part is incredibly gross but necessary to combine i18n in
## mako with logic in underscore.
## Translators: post_type describes the kind of post this is
## (e.g. "question" or "discussion"); time_ago is how much time
## has passed since the post was created (e.g. "4 hours ago")
${_("{post_type} posted {time_ago} by {author}").format(
post_type="<%- thread_type %>",
time_ago="<span class='timeago' title='<%- created_at %>'><%- created_at %></span>",
author="<%= author_display %>"
)}
</p>
<div class="post-labels">
<span class="post-label-pinned"><i class="icon icon-pushpin"></i>${_("Pinned")}</span>
<span class="post-label-reported"><i class="icon icon-flag"></i>${_("Reported")}</span>
<span class="post-label-closed"><i class="icon icon-lock"></i>${_("Closed")}</span>
</div>
</div>
<div class="post-header-actions post-extended-content">
${"""<%=
_.template(
$('#forum-actions').html(),
{
contentId: cid,
contentType: 'post',
primaryActions: ['vote', 'follow'],
secondaryActions: ['pin', 'edit', 'delete', 'report', 'close']
}
)
%>"""}
</div>
</header>
<div class="post-body">${'<%- body %>'}</div>
<% js_block = u"""
var courseware_link = interpolate('<a href="%s">%s</a>', [courseware_url, _.escape(courseware_title)]);
print(interpolate('{}', {{'courseware_title_linked': courseware_link}}, true));
""".format(
## Translators: 'courseware_title_linked' is a placeholder for the title of the courseware unit referenced by this discussion thread.
escapejs(_("(this post is about %(courseware_title_linked)s)"))
)
%>
${'<% if (mode == "tab" && obj.courseware_url) { %>'}
<div class="post-context">${'<%'}${js_block}${'%>'}</div>
${'<% } %>'}
</div>
% for template_name in ['thread-show']:
<script aria-hidden="true" type="text/template" id="${template_name}-template">
<%static:include path="js/discussion/${template_name}.underscore" />
</script>
% endfor
<script aria-hidden="true" type="text/template" id="thread-edit-template">
<div class="discussion-post edit-post-form">
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment