Commit ec482c0d by Greg Price

Allow authors of forum questions to mark answers

Co-authored-by: jsa <jsa@edx.org>
parent a3703fbf
...@@ -41,11 +41,11 @@ describe 'All Content', -> ...@@ -41,11 +41,11 @@ describe 'All Content', ->
it 'can update info', -> it 'can update info', ->
@content.updateInfo { @content.updateInfo {
ability: 'can_endorse', ability: {'can_edit': true},
voted: true, voted: true,
subscribed: true subscribed: true
} }
expect(@content.get 'ability').toEqual 'can_endorse' expect(@content.get 'ability').toEqual {'can_edit': true}
expect(@content.get 'voted').toEqual true expect(@content.get 'voted').toEqual true
expect(@content.get 'subscribed').toEqual true expect(@content.get 'subscribed').toEqual true
...@@ -77,3 +77,39 @@ describe 'All Content', -> ...@@ -77,3 +77,39 @@ describe 'All Content', ->
myComments = new Comments myComments = new Comments
myComments.add @comment1 myComments.add @comment1
expect(myComments.find('123')).toBe @comment1 expect(myComments.find('123')).toBe @comment1
it 'can be endorsed', ->
DiscussionUtil.loadRoles(
{"Moderator": [111], "Administrator": [222], "Community TA": [333]}
)
@discussionThread = new Thread({id: 1, thread_type: "discussion", user_id: 99})
@discussionResponse = new Comment({id: 1, thread: @discussionThread})
@questionThread = new Thread({id: 1, thread_type: "question", user_id: 99})
@questionResponse = new Comment({id: 1, thread: @questionThread})
# mod
window.user = new DiscussionUser({id: 111})
expect(@discussionResponse.canBeEndorsed()).toBe(true)
expect(@questionResponse.canBeEndorsed()).toBe(true)
# admin
window.user = new DiscussionUser({id: 222})
expect(@discussionResponse.canBeEndorsed()).toBe(true)
expect(@questionResponse.canBeEndorsed()).toBe(true)
# TA
window.user = new DiscussionUser({id: 333})
expect(@discussionResponse.canBeEndorsed()).toBe(true)
expect(@questionResponse.canBeEndorsed()).toBe(true)
# thread author
window.user = new DiscussionUser({id: 99})
expect(@discussionResponse.canBeEndorsed()).toBe(false)
expect(@questionResponse.canBeEndorsed()).toBe(true)
# anyone else
window.user = new DiscussionUser({id: 999})
expect(@discussionResponse.canBeEndorsed()).toBe(false)
expect(@questionResponse.canBeEndorsed()).toBe(false)
...@@ -41,6 +41,7 @@ describe "ThreadResponseShowView", -> ...@@ -41,6 +41,7 @@ describe "ThreadResponseShowView", ->
course_id: "TestOrg/TestCourse/TestRun", course_id: "TestOrg/TestCourse/TestRun",
body: "this is a comment", body: "this is a comment",
created_at: "2013-04-03T20:08:39Z", created_at: "2013-04-03T20:08:39Z",
endorsed: false,
abuse_flaggers: [], abuse_flaggers: [],
votes: {up_count: "42"} votes: {up_count: "42"}
} }
...@@ -99,8 +100,8 @@ describe "ThreadResponseShowView", -> ...@@ -99,8 +100,8 @@ describe "ThreadResponseShowView", ->
expect(@view.$(".posted-details").text()).not.toMatch(" by ") expect(@view.$(".posted-details").text()).not.toMatch(" by ")
it "re-renders correctly when endorsement changes", -> it "re-renders correctly when endorsement changes", ->
DiscussionUtil.loadRoles({"Moderator": [parseInt(window.user.id)]})
@thread.set("thread_type", "question") @thread.set("thread_type", "question")
@comment.updateInfo({"ability": {"can_endorse": true}})
expect(@view.$(".posted-details").text()).not.toMatch("marked as answer") expect(@view.$(".posted-details").text()).not.toMatch("marked as answer")
@view.$(".action-endorse").click() @view.$(".action-endorse").click()
expect(@view.$(".posted-details").text()).toMatch( expect(@view.$(".posted-details").text()).toMatch(
...@@ -108,3 +109,56 @@ describe "ThreadResponseShowView", -> ...@@ -108,3 +109,56 @@ describe "ThreadResponseShowView", ->
) )
@view.$(".action-endorse").click() @view.$(".action-endorse").click()
expect(@view.$(".posted-details").text()).not.toMatch("marked as answer") expect(@view.$(".posted-details").text()).not.toMatch("marked as answer")
it "allows a moderator to mark an answer in a question thread", ->
DiscussionUtil.loadRoles({"Moderator": parseInt(window.user.id)})
@thread.set({
"thread_type": "question",
"user_id": (parseInt(window.user.id) + 1).toString()
})
@view.render()
endorseButton = @view.$(".action-endorse")
expect(endorseButton.length).toEqual(1)
expect(endorseButton).not.toHaveCss({"display": "none"})
expect(endorseButton).toHaveClass("is-clickable")
endorseButton.click()
expect(endorseButton).toHaveClass("is-endorsed")
it "allows the author of a question thread to mark an answer", ->
@thread.set({
"thread_type": "question",
"user_id": window.user.id
})
@view.render()
endorseButton = @view.$(".action-endorse")
expect(endorseButton.length).toEqual(1)
expect(endorseButton).not.toHaveCss({"display": "none"})
expect(endorseButton).toHaveClass("is-clickable")
endorseButton.click()
expect(endorseButton).toHaveClass("is-endorsed")
it "does not allow the author of a discussion thread to endorse", ->
@thread.set({
"thread_type": "discussion",
"user_id": window.user.id
})
@view.render()
endorseButton = @view.$(".action-endorse")
expect(endorseButton.length).toEqual(1)
expect(endorseButton).toHaveCss({"display": "none"})
expect(endorseButton).not.toHaveClass("is-clickable")
endorseButton.click()
expect(endorseButton).not.toHaveClass("is-endorsed")
it "does not allow a student who is not the author of a question thread to mark an answer", ->
@thread.set({
"thread_type": "question",
"user_id": (parseInt(window.user.id) + 1).toString()
})
@view.render()
endorseButton = @view.$(".action-endorse")
expect(endorseButton.length).toEqual(1)
expect(endorseButton).toHaveCss({"display": "none"})
expect(endorseButton).not.toHaveClass("is-clickable")
endorseButton.click()
expect(endorseButton).not.toHaveClass("is-endorsed")
...@@ -9,7 +9,6 @@ if Backbone? ...@@ -9,7 +9,6 @@ if Backbone?
actions: actions:
editable: '.admin-edit' editable: '.admin-edit'
can_reply: '.discussion-reply' can_reply: '.discussion-reply'
can_endorse: '.admin-endorse'
can_delete: '.admin-delete' can_delete: '.admin-delete'
can_openclose: '.admin-openclose' can_openclose: '.admin-openclose'
...@@ -21,6 +20,9 @@ if Backbone? ...@@ -21,6 +20,9 @@ if Backbone?
can: (action) -> can: (action) ->
(@get('ability') || {})[action] (@get('ability') || {})[action]
# Default implementation
canBeEndorsed: -> false
updateInfo: (info) -> updateInfo: (info) ->
if info if info
@set('ability', info.ability) @set('ability', info.ability)
...@@ -187,6 +189,13 @@ if Backbone? ...@@ -187,6 +189,13 @@ if Backbone?
count += comment.getCommentsCount() + 1 count += comment.getCommentsCount() + 1
count count
canBeEndorsed: =>
user_id = window.user.get("id")
user_id && (
DiscussionUtil.isPrivilegedUser(user_id) ||
(@get('thread').get('thread_type') == 'question' && @get('thread').get('user_id') == user_id)
)
class @Comments extends Backbone.Collection class @Comments extends Backbone.Collection
model: Comment model: Comment
......
...@@ -41,6 +41,9 @@ class @DiscussionUtil ...@@ -41,6 +41,9 @@ class @DiscussionUtil
ta = _.union(@roleIds['Community TA']) ta = _.union(@roleIds['Community TA'])
_.include(ta, parseInt(user_id)) _.include(ta, parseInt(user_id))
@isPrivilegedUser: (user_id) ->
@isStaff(user_id) || @isTA(user_id)
@bulkUpdateContentInfo: (infos) -> @bulkUpdateContentInfo: (infos) ->
for id, info of infos for id, info of infos
Content.getContent(id).updateInfo(info) Content.getContent(id).updateInfo(info)
......
...@@ -46,15 +46,6 @@ if Backbone? ...@@ -46,15 +46,6 @@ if Backbone?
can_delete: can_delete:
enable: -> @$(".action-delete").closest("li").show() enable: -> @$(".action-delete").closest("li").show()
disable: -> @$(".action-delete").closest("li").hide() disable: -> @$(".action-delete").closest("li").hide()
can_endorse:
enable: ->
@$(".action-endorse").show().css("cursor", "auto")
disable: ->
@$(".action-endorse").css("cursor", "default")
if not @model.get('endorsed')
@$(".action-endorse").hide()
else
@$(".action-endorse").show()
can_openclose: can_openclose:
enable: -> @$(".action-openclose").closest("li").show() enable: -> @$(".action-openclose").closest("li").show()
disable: -> @$(".action-openclose").closest("li").hide() disable: -> @$(".action-openclose").closest("li").hide()
......
...@@ -14,14 +14,10 @@ if Backbone? ...@@ -14,14 +14,10 @@ if Backbone?
attrRenderer: $.extend({}, DiscussionContentView.prototype.attrRenderer, { attrRenderer: $.extend({}, DiscussionContentView.prototype.attrRenderer, {
endorsed: (endorsed) -> endorsed: (endorsed) ->
if endorsed $endorseButton = @$(".action-endorse")
@$(".action-endorse").show().addClass("is-endorsed") $endorseButton.toggleClass("is-clickable", @model.canBeEndorsed())
else $endorseButton.toggleClass("is-endorsed", endorsed)
if @model.get('ability')?.can_endorse $endorseButton.toggle(endorsed || @model.canBeEndorsed())
@$(".action-endorse").show()
else
@$(".action-endorse").hide()
@$(".action-endorse").removeClass("is-endorsed")
}) })
$: (selector) -> $: (selector) ->
...@@ -67,7 +63,7 @@ if Backbone? ...@@ -67,7 +63,7 @@ if Backbone?
toggleEndorse: (event) -> toggleEndorse: (event) ->
event.preventDefault() event.preventDefault()
if not @model.can('can_endorse') if not @model.canBeEndorsed()
return return
$elem = $(event.target) $elem = $(event.target)
url = @model.urlFor('endorse') url = @model.urlFor('endorse')
......
...@@ -6,7 +6,7 @@ from django.test.utils import override_settings ...@@ -6,7 +6,7 @@ from django.test.utils import override_settings
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.core.management import call_command from django.core.management import call_command
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from mock import patch, ANY from mock import patch, ANY, Mock
from nose.tools import assert_true, assert_equal # pylint: disable=E0611 from nose.tools import assert_true, assert_equal # pylint: disable=E0611
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
...@@ -26,9 +26,11 @@ CS_PREFIX = "http://localhost:4567/api/v1" ...@@ -26,9 +26,11 @@ CS_PREFIX = "http://localhost:4567/api/v1"
class MockRequestSetupMixin(object): class MockRequestSetupMixin(object):
def _create_repsonse_mock(self, data):
return Mock(text=json.dumps(data), json=Mock(return_value=data))\
def _set_mock_request_data(self, mock_request, data): def _set_mock_request_data(self, mock_request, data):
mock_request.return_value.text = json.dumps(data) mock_request.return_value = self._create_repsonse_mock(data)
mock_request.return_value.json.return_value = data
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
...@@ -620,6 +622,53 @@ class ViewPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSet ...@@ -620,6 +622,53 @@ class ViewPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSet
) )
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
def _set_mock_request_thread_and_comment(self, mock_request, thread_data, comment_data):
def handle_request(*args, **kwargs):
url = args[1]
if "/threads/" in url:
return self._create_repsonse_mock(thread_data)
elif "/comments/" in url:
return self._create_repsonse_mock(comment_data)
else:
raise ArgumentError("Bad url to mock request")
mock_request.side_effect = handle_request
def test_endorse_response_as_staff(self, mock_request):
self._set_mock_request_thread_and_comment(
mock_request,
{"type": "thread", "thread_type": "question", "user_id": str(self.student.id)},
{"type": "comment", "thread_id": "dummy"}
)
self.client.login(username=self.moderator.username, password=self.password)
response = self.client.post(
reverse("endorse_comment", kwargs={"course_id": self.course.id.to_deprecated_string(), "comment_id": "dummy"})
)
self.assertEqual(response.status_code, 200)
def test_endorse_response_as_student(self, mock_request):
self._set_mock_request_thread_and_comment(
mock_request,
{"type": "thread", "thread_type": "question", "user_id": str(self.moderator.id)},
{"type": "comment", "thread_id": "dummy"}
)
self.client.login(username=self.student.username, password=self.password)
response = self.client.post(
reverse("endorse_comment", kwargs={"course_id": self.course.id.to_deprecated_string(), "comment_id": "dummy"})
)
self.assertEqual(response.status_code, 401)
def test_endorse_response_as_student_question_author(self, mock_request):
self._set_mock_request_thread_and_comment(
mock_request,
{"type": "thread", "thread_type": "question", "user_id": str(self.student.id)},
{"type": "comment", "thread_id": "dummy"}
)
self.client.login(username=self.student.username, password=self.password)
response = self.client.post(
reverse("endorse_comment", kwargs={"course_id": self.course.id.to_deprecated_string(), "comment_id": "dummy"})
)
self.assertEqual(response.status_code, 200)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class CreateThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockRequestSetupMixin): class CreateThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockRequestSetupMixin):
......
...@@ -5,6 +5,7 @@ Module for checking permissions with the comment_client backend ...@@ -5,6 +5,7 @@ Module for checking permissions with the comment_client backend
import logging import logging
from types import NoneType from types import NoneType
from django.core import cache from django.core import cache
from lms.lib.comment_client import Thread
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
CACHE = cache.get_cache('default') CACHE = cache.get_cache('default')
...@@ -34,7 +35,7 @@ def has_permission(user, permission, course_id=None): ...@@ -34,7 +35,7 @@ def has_permission(user, permission, course_id=None):
return False return False
CONDITIONS = ['is_open', 'is_author'] CONDITIONS = ['is_open', 'is_author', 'is_question_author']
def _check_condition(user, condition, content): def _check_condition(user, condition, content):
...@@ -50,9 +51,22 @@ def _check_condition(user, condition, content): ...@@ -50,9 +51,22 @@ def _check_condition(user, condition, content):
except KeyError: except KeyError:
return False return False
def check_question_author(user, content):
if not content:
return False
try:
if content["type"] == "thread":
return content["thread_type"] == "question" and content["user_id"] == str(user.id)
else:
# N.B. This will trigger a comments service query
return check_question_author(user, Thread(id=content["thread_id"]).to_dict())
except KeyError:
return False
handlers = { handlers = {
'is_open': check_open, 'is_open': check_open,
'is_author': check_author, 'is_author': check_author,
'is_question_author': check_question_author,
} }
return handlers[condition](user, content) return handlers[condition](user, content)
...@@ -85,7 +99,7 @@ VIEW_PERMISSIONS = { ...@@ -85,7 +99,7 @@ VIEW_PERMISSIONS = {
'create_comment': [["create_comment", "is_open"]], 'create_comment': [["create_comment", "is_open"]],
'delete_thread': ['delete_thread', ['update_thread', 'is_author']], 'delete_thread': ['delete_thread', ['update_thread', 'is_author']],
'update_comment': ['edit_content', ['update_comment', 'is_open', 'is_author']], 'update_comment': ['edit_content', ['update_comment', 'is_open', 'is_author']],
'endorse_comment': ['endorse_comment'], 'endorse_comment': ['endorse_comment', 'is_question_author'],
'openclose_thread': ['openclose_thread'], 'openclose_thread': ['openclose_thread'],
'create_sub_comment': [['create_sub_comment', 'is_open']], 'create_sub_comment': [['create_sub_comment', 'is_open']],
'delete_comment': ['delete_comment', ['update_comment', 'is_open', 'is_author']], 'delete_comment': ['delete_comment', ['update_comment', 'is_open', 'is_author']],
......
...@@ -258,7 +258,6 @@ def get_ability(course_id, content, user): ...@@ -258,7 +258,6 @@ def get_ability(course_id, content, user):
return { return {
'editable': check_permissions_by_view(user, course_id, content, "update_thread" if content['type'] == 'thread' else "update_comment"), 'editable': check_permissions_by_view(user, course_id, content, "update_thread" if content['type'] == 'thread' else "update_comment"),
'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_endorse': check_permissions_by_view(user, course_id, content, "endorse_comment") if content['type'] == 'comment' else False,
'can_delete': check_permissions_by_view(user, course_id, content, "delete_thread" if content['type'] == 'thread' else "delete_comment"), 'can_delete': check_permissions_by_view(user, course_id, content, "delete_thread" if content['type'] == 'thread' else "delete_comment"),
'can_openclose': check_permissions_by_view(user, course_id, content, "openclose_thread") if content['type'] == 'thread' else False, 'can_openclose': check_permissions_by_view(user, course_id, content, "openclose_thread") if content['type'] == 'thread' else False,
'can_vote': check_permissions_by_view(user, course_id, content, "vote_for_thread" if content['type'] == 'thread' else "vote_for_comment"), 'can_vote': check_permissions_by_view(user, course_id, content, "vote_for_thread" if content['type'] == 'thread' else "vote_for_comment"),
......
...@@ -647,6 +647,11 @@ body.discussion { ...@@ -647,6 +647,11 @@ body.discussion {
border: 1px solid #a0a0a0; border: 1px solid #a0a0a0;
@include linear-gradient(top, $white 35%, $gray-l4); @include linear-gradient(top, $white 35%, $gray-l4);
box-shadow: 0 1px 1px $shadow-l1; box-shadow: 0 1px 1px $shadow-l1;
cursor: default;
&.is-clickable {
cursor: auto;
}
.check-icon { .check-icon {
display: block; display: block;
...@@ -654,6 +659,7 @@ body.discussion { ...@@ -654,6 +659,7 @@ body.discussion {
height: 12px; height: 12px;
margin: 8px auto; margin: 8px auto;
background: url(../images/endorse-icon.png) no-repeat; background: url(../images/endorse-icon.png) no-repeat;
pointer-events: none;
} }
&.mark-answer .check-icon { &.mark-answer .check-icon {
......
...@@ -162,10 +162,9 @@ ...@@ -162,10 +162,9 @@
<a <a
href="javascript:void(0)" href="javascript:void(0)"
class="endorse-btn action-endorse ${"<%= thread.get('thread_type') == 'question' ? 'mark-answer' : '' %>"}" class="endorse-btn action-endorse ${"<%= thread.get('thread_type') == 'question' ? 'mark-answer' : '' %>"}"
style="cursor: default; display: none;"
data-tooltip="${tooltip_expr}" data-tooltip="${tooltip_expr}"
> >
<span class="check-icon" style="pointer-events: none; "></span> <span class="check-icon"></span>
</a> </a>
${"<% if (obj.username) { %>"} ${"<% if (obj.username) { %>"}
<a href="${'<%- user_url %>'}" class="posted-by">${'<%- username %>'}</a> <a href="${'<%- user_url %>'}" class="posted-by">${'<%- username %>'}</a>
......
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