Skip to content

E
edx-platform
Commit 2f7fa6fd by Peter Fogg Committed by Andy Armstrong
Options

Enforce discussions permissions in teams UI. 

Adds a read-only mode to the discussion module which disables any
controls for updating the discussion (votes, follows, replies,
etc). This mode is enabled for users who are not a member of the team,
and are also not moderators, admins, or community TAs.
parent f5840489
Showing with 177 additions and 43 deletions
common/static/coffee/spec/discussion/discussion_spec_helper.coffee
......@@ -67,5 +67,6 @@ class @DiscussionSpecHelper
data-course-name="Fake Course"
data-user-create-comment="true"
data-user-create-subcomment="true"
data-read-only="false"
></div>
""")
common/static/coffee/src/discussion/views/discussion_thread_show_view.coffee
......@@ -13,7 +13,8 @@ if Backbone?
mode: @mode,
flagged: @model.isFlagged(),
author_display: @getAuthorDisplay(),
cid: @model.cid
cid: @model.cid,
readOnly: $('.discussion-module').data('read-only')
},
@model.attributes,
)
......
common/static/coffee/src/discussion/views/discussion_thread_view.coffee
......@@ -23,6 +23,8 @@ if Backbone?
if @mode not in ["tab", "inline"]
throw new Error("invalid mode: " + @mode)
@readOnly = $(".discussion-module").data('read-only')
# Quick fix to have an actual model when we're receiving new models from
# the server.
@model.collection.on "reset", (collection) =>
......@@ -51,12 +53,15 @@ if Backbone?
renderTemplate: ->
@template = _.template($("#thread-template").html())
templateData = @model.toJSON()
container = $("#discussion-container")
if !container.length
# inline discussion
container = $(".discussion-module")
templateData.can_create_comment = container.data("user-create-comment")
templateData = _.extend(
@model.toJSON(),
readOnly: @readOnly,
can_create_comment: container.data("user-create-comment")
)
@template(templateData)
render: ->
......
common/static/coffee/src/discussion/views/response_comment_show_view.coffee
......@@ -9,7 +9,8 @@ if Backbone?
_.extend(
{
cid: @model.cid,
author_display: @getAuthorDisplay()
author_display: @getAuthorDisplay(),
readOnly: $('.discussion-module').data('read-only')
},
@model.attributes
)
......
common/static/coffee/src/discussion/views/thread_response_show_view.coffee
......@@ -10,7 +10,8 @@ if Backbone?
{
cid: @model.cid,
author_display: @getAuthorDisplay(),
endorser_display: @getEndorserDisplay()
endorser_display: @getEndorserDisplay(),
readOnly: $('.discussion-module').data('read-only')
},
@model.attributes
)
......
common/static/coffee/src/discussion/views/thread_response_view.coffee
......@@ -13,17 +13,21 @@ if Backbone?
initialize: (options) ->
@collapseComments = options.collapseComments
@createShowView()
@readOnly = $('.discussion-module').data('read-only')
renderTemplate: ->
@template = _.template($("#thread-response-template").html())
templateData = @model.toJSON()
templateData.wmdId = @model.id ? (new Date()).getTime()
container = $("#discussion-container")
if !container.length
# inline discussion
container = $(".discussion-module")
templateData.create_sub_comment = container.data("user-create-subcomment")
templateData = _.extend(
@model.toJSON(),
wmdId: @model.id ? (new Date()).getTime(),
create_sub_comment: container.data("user-create-subcomment"),
readOnly: @readOnly
)
@template(templateData)
render: ->
......@@ -88,6 +92,9 @@ if Backbone?
comment.set('thread', @model.get('thread'))
view = new ResponseCommentView(model: comment)
view.render()
if @readOnly
@$el.find('.comments').append(view.el)
else
@$el.find(".comments .new-comment").before(view.el)
view.bind "comment:edit", (event) =>
@cancelEdit(event) if @editView?
......
common/static/common/templates/discussion/forum-actions.underscore
<ul class="<%= contentType %>-actions-list">
<% if (!readOnly) { %>
<ul class="<%= contentType %>-actions-list">
<% _.each(primaryActions, function(action) { print(_.template($('#forum-action-' + action).html(), {})) }) %>
<li class="actions-item is-visible">
<div class="more-wrapper">
......@@ -13,4 +14,5 @@
</div>
</div>
</li>
</ul>
</ul>
<% } %>
common/static/common/templates/discussion/response-comment-show.underscore
......@@ -7,7 +7,8 @@
contentId: cid,
contentType: 'comment',
primaryActions: [],
secondaryActions: ['edit', 'delete', 'report']
secondaryActions: ['edit', 'delete', 'report'],
readOnly: readOnly
}
)
%>
......
common/static/common/templates/discussion/thread-response-show.underscore
......@@ -49,7 +49,8 @@
contentId: cid,
contentType: 'response',
primaryActions: ['vote', thread.get('thread_type') == 'question' ? 'answer' : 'endorse'],
secondaryActions: ['edit', 'delete', 'report']
secondaryActions: ['edit', 'delete', 'report'],
readOnly: readOnly
}
)
%>
......
common/static/common/templates/discussion/thread-response.underscore
......@@ -12,7 +12,7 @@
</a>
<ol class="comments">
<li class="new-comment">
<% if (create_sub_comment) { %>
<% if (create_sub_comment && !readOnly) { %>
<form class="comment-form" data-id="<%- wmdId %>">
<ul class="discussion-errors"></ul>
<label class="sr" for="add-new-comment"><%- gettext("Add a comment") %></label>
......
common/static/common/templates/discussion/thread-show.underscore
......@@ -40,6 +40,7 @@
<span class="post-label-closed"><i class="icon fa fa-lock"></i><%- gettext("Closed") %></span>
</div>
</div>
<% if (!readOnly) { %>
<div class="post-header-actions post-extended-content">
<%=
_.template(
......@@ -48,11 +49,13 @@
contentId: cid,
contentType: 'post',
primaryActions: ['vote', 'follow'],
secondaryActions: ['pin', 'edit', 'delete', 'report', 'close']
secondaryActions: ['pin', 'edit', 'delete', 'report', 'close'],
readOnly: readOnly
}
)
%>
</div>
<% } %>
</header>
<div class="post-body"><%- body %></div>
......
common/static/common/templates/discussion/thread.underscore
......@@ -8,18 +8,20 @@
</div>
<div class="post-extended-content">
<div class="response-count"/>
<% if (!readOnly) { %>
<div class="add-response">
<button class="button add-response-btn">
<i class="icon fa fa-reply"></i>
<span class="add-response-btn-text"><%- gettext("Add a Response") %></span>
</button>
</div>
<% } %>
<ol class="responses js-response-list"/>
<div class="response-pagination"/>
<div class="post-status-closed bottom-post-status" style="display: none">
<%- gettext("This thread is closed.") %>
</div>
<% if (can_create_comment) { %>
<% if (can_create_comment && !readOnly) { %>
<form class="discussion-reply-new" data-id="<%- id %>">
<h4><%- gettext("Post a response:") %></h4>
<ul class="discussion-errors"></ul>
......
common/test/acceptance/tests/lms/test_teams.py
......@@ -3,6 +3,7 @@ Acceptance tests for the teams feature.
"""
import json
import ddt
from nose.plugins.attrib import attr
from uuid import uuid4
......@@ -652,6 +653,7 @@ class CreateTeamTest(TeamsTabBase):
@attr('shard_5')
@ddt.ddt
class TeamPageTest(TeamsTabBase):
"""Tests for viewing a specific team"""
def setUp(self):
......@@ -659,12 +661,11 @@ class TeamPageTest(TeamsTabBase):
self.topic = {u"name": u"Example Topic", u"id": "example_topic", u"description": "Description"}
self.set_team_configuration({'course_id': self.course_id, 'max_team_size': 10, 'topics': [self.topic]})
self.team = self.create_teams(self.topic, 1)[0]
self.create_membership(self.user_info['username'], self.team['id'])
self.team_page = TeamPage(self.browser, self.course_id, self.team)
def setup_thread(self):
"""
Set up the discussion thread for the team.
Create and return a thread for this test's discussion topic.
"""
thread = Thread(
id="test_thread_{}".format(uuid4().hex),
......@@ -675,15 +676,25 @@ class TeamPageTest(TeamsTabBase):
thread_fixture.push()
return thread
def test_discussion_on_team_page(self):
def setup_discussion_user(self, role=None, staff=False):
"""Set this test's user to have the given role in its
discussions. Role is one of 'Community TA', 'Moderator',
'Administrator', or 'Student'.
"""
Scenario: Team Page renders a team discussion.
Given I am enrolled in a course with a team configuration, a topic,
and a team belonging to that topic
When a thread exists in the team's discussion
And I visit the Team page for that team
Then I should see a discussion with the correct discussion_id
And I should see the existing thread
kwargs = {
'course_id': self.course_id,
'staff': staff
}
if role is not None:
kwargs['roles'] = role
#pylint: disable=attribute-defined-outside-init
self.user_info = AutoAuthPage(self.browser, **kwargs).visit().user_info
def verify_teams_discussion_permissions(self, should_have_permission):
"""Verify that the teams discussion component is in the correct state
for the test user. If `should_have_permission` is True, assert that
the user can see controls for posting replies, voting, editing, and
deleting. Otherwise, assert that those controls are hidden.
"""
thread = self.setup_thread()
self.team_page.visit()
......@@ -693,3 +704,42 @@ class TeamPageTest(TeamsTabBase):
self.assertTrue(discussion.is_discussion_expanded())
self.assertEqual(discussion.get_num_displayed_threads(), 1)
self.assertTrue(discussion.has_thread(thread['id']))
assertion = self.assertTrue if should_have_permission else self.assertFalse
assertion(discussion.q(css='.post-header-actions').present)
assertion(discussion.q(css='.add-response').present)
assertion(discussion.q(css='.new-post-btn').present)
def test_discussion_on_my_team_page(self):
"""
Scenario: Team Page renders a discussion for a team to which I belong.
Given I am enrolled in a course with a team configuration, a topic,
and a team belonging to that topic of which I am a member
When the team has a discussion with a thread
And I visit the Team page for that team
Then I should see a discussion with the correct discussion_id
And I should see the existing thread
And I should see controls to change the state of the discussion
"""
self.create_membership(self.user_info['username'], self.team['id'])
self.verify_teams_discussion_permissions(True)
@ddt.data(True, False)
def test_discussion_on_other_team_page(self, is_staff):
"""
Scenario: Team Page renders a team discussion for a team to which I do
not belong.
Given I am enrolled in a course with a team configuration, a topic,
and a team belonging to that topic of which I am not a member
When the team has a discussion with a thread
And I visit the Team page for that team
Then I should see a discussion with the correct discussion_id
And I should see the team's thread
And I should not see controls to change the state of the discussion
"""
self.setup_discussion_user(staff=is_staff)
self.verify_teams_discussion_permissions(False)
@ddt.data('Moderator', 'Community TA', 'Administrator')
def test_discussion_privileged(self, role):
self.setup_discussion_user(role=role)
self.verify_teams_discussion_permissions(True)
lms/djangoapps/django_comment_client/tests/test_utils.py
......@@ -22,6 +22,7 @@ from openedx.core.djangoapps.course_groups.tests.helpers import config_course_co
from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory
from openedx.core.djangoapps.content.course_structures.models import CourseStructure
from openedx.core.djangoapps.util.testing import ContentGroupTestCase
from student.roles import CourseStaffRole
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE
from xmodule.modulestore.django import modulestore
......@@ -80,6 +81,8 @@ class AccessUtilsTestCase(ModuleStoreTestCase):
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)
self.course_staff = UserFactory(username='course_staff', email='course_staff@edx.org')
CourseStaffRole(self.course_id).add_users(self.course_staff)
def test_get_role_ids(self):
ret = utils.get_role_ids(self.course_id)
......@@ -89,6 +92,7 @@ class AccessUtilsTestCase(ModuleStoreTestCase):
def test_has_discussion_privileges(self):
self.assertFalse(utils.has_discussion_privileges(self.student1, self.course_id))
self.assertFalse(utils.has_discussion_privileges(self.student2, self.course_id))
self.assertFalse(utils.has_discussion_privileges(self.course_staff, self.course_id))
self.assertTrue(utils.has_discussion_privileges(self.moderator, self.course_id))
self.assertTrue(utils.has_discussion_privileges(self.community_ta1, self.course_id))
self.assertTrue(utils.has_discussion_privileges(self.community_ta2, self.course_id))
......
lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js
......@@ -88,5 +88,35 @@ define([
expectFocus(teamsTabView.$('.warning'));
});
});
describe('Discussion privileges', function () {
it('allows privileged access to any team', function () {
teamsTabView.$el.data('privileged', true);
// Note: using `undefined` here to ensure that we
// don't even look at the team when the user is
// privileged
expect(teamsTabView.readOnlyDiscussion(undefined)).toBe(false);
});
it('allows access to a team which an unprivileged user is a member of', function () {
teamsTabView.$el.data('privileged', false).data('username', 'test-user');
expect(teamsTabView.readOnlyDiscussion({
attributes: {
membership: [{
user: {
username: 'test-user'
}
}]
}
})).toBe(false);
});
it('does not allow access if the user is neither privileged nor a team member', function () {
teamsTabView.$el.data('privileged', false).data('username', 'test-user');
expect(teamsTabView.readOnlyDiscussion({
attributes: { membership: [] }
})).toBe(true);
});
});
});
});
lms/djangoapps/teams/static/teams/js/views/team_profile.js
......@@ -10,15 +10,14 @@
initialize: function (options) {
this.courseID = options.courseID;
this.discussionTopicID = this.model.get('discussion_topic_id');
this.readOnly = options.readOnly;
},
render: function () {
var canPostToTeam = true; // TODO: determine this permission correctly!
this.$el.html(_.template(teamTemplate, {
courseID: this.courseID,
discussionTopicID: this.discussionTopicID,
canCreateComment: canPostToTeam,
canCreateSubComment: canPostToTeam
readOnly: this.readOnly
}));
this.discussionView = new TeamDiscussionView({
el: this.$('.discussion-module')
......
lms/djangoapps/teams/static/teams/js/views/teams_tab.js
......@@ -209,9 +209,11 @@
courseID = this.courseID;
self.getTopic(topicID).done(function(topic) {
self.getTeam(teamID).done(function(team) {
var view = new TeamProfileView({
var readOnly = self.readOnlyDiscussion(team),
view = new TeamProfileView({
courseID: courseID,
model: team
model: team,
readOnly: readOnly
});
deferred.resolve(self.createViewWithHeader(view, team, topic));
});
......@@ -377,6 +379,23 @@
hideWarning: function () {
this.$('.warning').toggleClass('is-hidden', true);
},
/**
* Returns true if the discussion thread belonging to
* `team` is accessible to the user. This is the case
* if the user is privileged (i.e., a community TA,
* moderator, or administrator), or if the user
* belongs to the team.
*/
readOnlyDiscussion: function (team) {
var self = this;
return !(
this.$el.data('privileged') ||
_.any(team.attributes.membership, function (membership) {
return membership.user.username === self.$el.data('username');
})
);
}
});
......
lms/djangoapps/teams/static/teams/templates/team-profile.underscore
<div class="team-profile">
<div class="discussion-module" data-course-id="<%= courseID %>" data-discussion-id="<%= discussionTopicID %>"
data-user-create-comment="<%= canCreateComment %>"
data-user-create-subcomment="<%= canCreateSubComment %>">
data-read-only="<%= readOnly %>"
data-user-create-comment="<%= !readOnly %>"
data-user-create-subcomment="<%= !readOnly %>">
<% if ( !readOnly) { %>
<a href="#" class="new-post-btn" role="button"><span class="icon fa fa-edit new-post-icon"></span><%= gettext("New Post") %></a>
<% } %>
</div>
</div>
lms/djangoapps/teams/templates/teams/teams.html
......@@ -18,7 +18,7 @@
<div class="container">
<div class="teams-wrapper">
<section class="teams-content">
<section class="teams-content" data-username=${json.dumps(username, cls=EscapedEdxJSONEncoder)} data-privileged="${json.dumps(privileged)}">
</section>
</div>
</div>
......
lms/djangoapps/teams/tests/test_views.py
......@@ -467,7 +467,7 @@ class TestCreateTeamAPI(TeamAPITestCase):
# Verify that the creating user gets added to the team.
self.assertEqual(len(team_membership), 1)
member = team_membership[0]['user']
self.assertEqual(member['id'], creator)
self.assertEqual(member['username'], creator)
self.assertEqual(team, {
'name': 'Fully specified team',
......@@ -688,7 +688,7 @@ class TestListMembershipAPI(TeamAPITestCase):
membership = self.get_membership_list(status, {'team_id': self.test_team_1.team_id}, user=user)
if status == 200:
self.assertEqual(membership['count'], 1)
self.assertEqual(membership['results'][0]['user']['id'], self.users['student_enrolled'].username)
self.assertEqual(membership['results'][0]['user']['username'], self.users['student_enrolled'].username)
@ddt.data(
(None, 401, False),
......@@ -705,7 +705,7 @@ class TestListMembershipAPI(TeamAPITestCase):
if status == 200:
if has_content:
self.assertEqual(membership['count'], 1)
self.assertEqual(membership['results'][0]['team']['id'], self.test_team_1.team_id)
self.assertEqual(membership['results'][0]['team']['team_id'], self.test_team_1.team_id)
else:
self.assertEqual(membership['count'], 0)
......@@ -754,8 +754,8 @@ class TestCreateMembershipAPI(TeamAPITestCase):
user=user
)
if status == 200:
self.assertEqual(membership['user']['id'], self.users['student_enrolled_not_on_team'].username)
self.assertEqual(membership['team']['id'], self.test_team_1.team_id)
self.assertEqual(membership['user']['username'], self.users['student_enrolled_not_on_team'].username)
self.assertEqual(membership['team']['team_id'], self.test_team_1.team_id)
memberships = self.get_membership_list(200, {'team_id': self.test_team_1.team_id})
self.assertEqual(memberships['count'], 2)
......
lms/djangoapps/teams/views.py
......@@ -90,6 +90,7 @@ class TeamsDashboardView(View):
instance=topics_page,
context={'course_id': course.id, 'sort_order': sort_order}
)
user = request.user
context = {
"course": course,
"topics": topics_serializer.data,
......@@ -100,6 +101,8 @@ class TeamsDashboardView(View):
"teams_url": reverse('teams_list', request=request),
"languages": settings.ALL_LANGUAGES,
"countries": list(countries),
"username": user.username,
"privileged": has_discussion_privileges(user, course_key)
}
return render_to_response("teams/teams.html", context)
......
lms/templates/discussion/_discussion_module.html
......@@ -3,7 +3,7 @@
from django.utils.translation import ugettext as _
%>
<div class="discussion-module" data-discussion-id="${discussion_id | h}" data-user-create-comment="${can_create_comment}" data-user-create-subcomment="${can_create_subcomment}">
<div class="discussion-module" data-discussion-id="${discussion_id | h}" data-user-create-comment="${can_create_comment}" data-user-create-subcomment="${can_create_subcomment}" data-read-only="false">
<a class="discussion-show control-button" href="javascript:void(0)" data-discussion-id="${discussion_id | h}" role="button"><span class="show-hide-discussion-icon"></span><span class="button-text">${_("Show Discussion")}</span></a>
% if can_create_thread:
<a href="#" class="new-post-btn" role="button"><span class="icon fa fa-edit new-post-icon"></span>${_("New Post")}</a>
......
lms/templates/discussion/index.html
......@@ -31,6 +31,7 @@ from django.core.urlresolvers import reverse
data-user-info="${user_info}"
data-user-create-comment="${can_create_comment}"
data-user-create-subcomment="${can_create_subcomment}"
data-read-only="false"
data-threads="${threads}"
data-thread-pages="${thread_pages}"
data-content-info="${annotated_content_info}"
......
openedx/core/lib/api/serializers.py
......@@ -31,7 +31,6 @@ class PaginationSerializer(pagination.PaginationSerializer):
class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer):
"""Serializes arbitrary models in a collapsed format, with just an id and url."""
id = serializers.CharField(read_only=True) # pylint: disable=invalid-name
url = serializers.HyperlinkedIdentityField(view_name='')
def __init__(self, model_class, view_name, id_source='id', lookup_field=None, *args, **kwargs):
......@@ -42,7 +41,8 @@ class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer):
view_name (string): Name of the Django view used to lookup the
model.
id_source (string): Optional name of the id field on the model.
Defaults to 'id'.
Defaults to 'id'. Also used as the property name of the field
in the serialized representation.
lookup_field (string): Optional name of the model field used to
lookup the model in the view. Defaults to the value of
id_source.
......@@ -54,7 +54,7 @@ class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer):
super(CollapsedReferenceSerializer, self).__init__(*args, **kwargs)
self.fields['id'].source = id_source
self.fields[id_source] = serializers.CharField(read_only=True, source=id_source)
self.fields['url'].view_name = view_name
self.fields['url'].lookup_field = lookup_field
......@@ -63,4 +63,4 @@ class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer):
model is set dynamically in __init__.
"""
fields = ("id", "url")
fields = ("url",)
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