Skip to content

E
edx-platform
Commit 182ae7ae by Greg Price
Options

Add endorsement info to marked answers in forum 

Co-authored-by: jsa <jsa@edx.org>
parent 10063a31
Showing with 178 additions and 56 deletions
common/static/coffee/spec/discussion/view/thread_response_show_view_spec.coffee
......@@ -3,14 +3,38 @@ describe "ThreadResponseShowView", ->
DiscussionSpecHelper.setUpGlobals()
setFixtures(
"""
<div class="discussion-post">
<a href="#" class="vote-btn" data-tooltip="vote" role="button" aria-pressed="false">
<span class="plus-icon"/><span class="votes-count-number">0</span> <span class="sr">votes (click to vote)</span>
<script type="text/template" id="thread-response-show-template">
<a href="#" class="vote-btn" data-tooltip="vote" role="button" aria-pressed="false"></a>
<a
href="javascript:void(0)"
class="endorse-btn action-endorse <%= thread.get('thread_type') == 'question' ? 'mark-answer' : '' %>"
style="cursor: default; display: none;"
data-tooltip="<%= thread.get('thread_type') == 'question' ? 'mark as answer' : 'endorse' %>"
>
<span class="check-icon" style="pointer-events: none; "></span>
</a>
</div>
<p class="posted-details">
<span class="timeago" title="<%= created_at %>"><%= created_at %></span>
<% if (thread.get('thread_type') == 'question' && obj.endorsement) { %> -
<%=
interpolate(
endorsement.username ? "marked as answer %(time_ago)s by %(user)s" : "marked as answer %(time_ago)s",
{
'time_ago': '<span class="timeago" title="' + endorsement.time + '">' + endorsement.time + '</span>',
'user': endorsement.username
},
true
)
%>
<% } %>
</p>
</script>
<div class="discussion-post"></div>
"""
)
@thread = new Thread({"thread_type": "discussion"})
@commentData = {
id: "dummy",
user_id: "567",
......@@ -21,9 +45,15 @@ describe "ThreadResponseShowView", ->
votes: {up_count: "42"}
}
@comment = new Comment(@commentData)
@comment.set("thread", @thread)
@view = new ThreadResponseShowView({ model: @comment })
@view.setElement($(".discussion-post"))
# Avoid unnecessary boilerplate
spyOn(ThreadResponseShowView.prototype, "convertMath")
@view.render()
it "renders the vote correctly", ->
DiscussionViewSpecHelper.checkRenderVote(@view, @comment)
......@@ -38,3 +68,32 @@ describe "ThreadResponseShowView", ->
it "vote button activates on appropriate events", ->
DiscussionViewSpecHelper.checkVoteButtonEvents(@view)
it "renders endorsement correctly for a marked answer in a question thread", ->
endorsement = {
"username": "test_endorser",
"time": new Date().toISOString()
}
@thread.set("thread_type", "question")
@comment.set({
"endorsed": true,
"endorsement": endorsement
})
@view.render()
expect(@view.$(".posted-details").text()).toMatch(
"marked as answer less than a minute ago by " + endorsement.username
)
it "renders anonymous endorsement correctly for a marked answer in a question thread", ->
endorsement = {
"username": null,
"time": new Date().toISOString()
}
@thread.set("thread_type", "question")
@comment.set({
"endorsed": true,
"endorsement": endorsement
})
@view.render()
expect(@view.$(".posted-details").text()).toMatch("marked as answer less than a minute ago")
expect(@view.$(".posted-details").text()).not.toMatch(" by ")
common/static/coffee/src/discussion/views/thread_response_show_view.coffee
......@@ -29,7 +29,7 @@ if Backbone?
@renderVote()
@renderAttrs()
@renderFlagged()
@$el.find(".posted-details").timeago()
@$el.find(".posted-details .timeago").timeago()
@convertMath()
@markAsStaff()
@
......
lms/djangoapps/django_comment_client/base/views.py
......@@ -53,11 +53,11 @@ def permitted(fn):
return wrapper
def ajax_content_response(request, course_id, content):
def ajax_content_response(request, course_key, content):
user_info = cc.User.from_django_user(request.user).to_dict()
annotated_content_info = get_annotated_content_info(course_id, content, request.user, user_info)
annotated_content_info = get_annotated_content_info(course_key, content, request.user, user_info)
return JsonResponse({
'content': safe_content(content),
'content': safe_content(content, course_key),
'annotated_content_info': annotated_content_info,
})
......@@ -71,8 +71,8 @@ def create_thread(request, course_id, commentable_id):
"""
log.debug("Creating new thread in %r, id %r", course_id, commentable_id)
course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
course = get_course_with_access(request.user, 'load', course_id)
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
course = get_course_with_access(request.user, 'load', course_key)
post = request.POST
if course.allow_anonymous:
......@@ -94,7 +94,7 @@ def create_thread(request, course_id, commentable_id):
anonymous=anonymous,
anonymous_to_peers=anonymous_to_peers,
commentable_id=commentable_id,
course_id=course_id.to_deprecated_string(),
course_id=course_key.to_deprecated_string(),
user_id=request.user.id,
body=post["body"],
title=post["title"]
......@@ -107,13 +107,13 @@ def create_thread(request, course_id, commentable_id):
#not anymore, only for admins
# Cohort the thread if the commentable is cohorted.
if is_commentable_cohorted(course_id, commentable_id):
user_group_id = get_cohort_id(user, course_id)
if is_commentable_cohorted(course_key, commentable_id):
user_group_id = get_cohort_id(user, course_key)
# TODO (vshnayder): once we have more than just cohorts, we'll want to
# change this to a single get_group_for_user_and_commentable function
# that can do different things depending on the commentable_id
if cached_has_permission(request.user, "see_all_cohorts", course_id):
if cached_has_permission(request.user, "see_all_cohorts", course_key):
# admins can optionally choose what group to post as
group_id = post.get('group_id', user_group_id)
else:
......@@ -135,9 +135,9 @@ def create_thread(request, course_id, commentable_id):
data = thread.to_dict()
add_courseware_context([data], course)
if request.is_ajax():
return ajax_content_response(request, course_id, data)
return ajax_content_response(request, course_key, data)
else:
return JsonResponse(safe_content(data))
return JsonResponse(safe_content(data, course_key))
@require_POST
......@@ -151,19 +151,20 @@ def update_thread(request, course_id, thread_id):
return JsonError(_("Title can't be empty"))
if 'body' not in request.POST or not request.POST['body'].strip():
return JsonError(_("Body can't be empty"))
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
thread = cc.Thread.find(thread_id)
thread.body = request.POST["body"]
thread.title = request.POST["title"]
thread.save()
if request.is_ajax():
return ajax_content_response(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), thread.to_dict())
return ajax_content_response(request, course_key, thread.to_dict())
else:
return JsonResponse(safe_content(thread.to_dict()))
return JsonResponse(safe_content(thread.to_dict(), course_key))
def _create_comment(request, course_key, thread_id=None, parent_id=None):
"""
given a course_id, thread_id, and parent_id, create a comment,
given a course_key, thread_id, and parent_id, create a comment,
called from create_comment to do the actual creation
"""
assert isinstance(course_key, CourseKey)
......@@ -199,7 +200,7 @@ def _create_comment(request, course_key, thread_id=None, parent_id=None):
if request.is_ajax():
return ajax_content_response(request, course_key, comment.to_dict())
else:
return JsonResponse(safe_content(comment.to_dict()))
return JsonResponse(safe_content(comment.to_dict(), course.id))
@require_POST
......@@ -224,9 +225,10 @@ def delete_thread(request, course_id, thread_id):
given a course_id and thread_id, delete this thread
this is ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
thread = cc.Thread.find(thread_id)
thread.delete()
return JsonResponse(safe_content(thread.to_dict()))
return JsonResponse(safe_content(thread.to_dict(), course_key))
@require_POST
......@@ -237,15 +239,16 @@ def update_comment(request, course_id, comment_id):
given a course_id and comment_id, update the comment with payload attributes
handles static and ajax submissions
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
comment = cc.Comment.find(comment_id)
if 'body' not in request.POST or not request.POST['body'].strip():
return JsonError(_("Body can't be empty"))
comment.body = request.POST["body"]
comment.save()
if request.is_ajax():
return ajax_content_response(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), comment.to_dict())
return ajax_content_response(request, course_key, comment.to_dict())
else:
return JsonResponse(safe_content(comment.to_dict()))
return JsonResponse(safe_content(comment.to_dict(), course_key))
@require_POST
......@@ -256,10 +259,12 @@ def endorse_comment(request, course_id, comment_id):
given a course_id and comment_id, toggle the endorsement of this comment,
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
comment = cc.Comment.find(comment_id)
comment.endorsed = request.POST.get('endorsed', 'false').lower() == 'true'
comment.endorsement_user_id = request.user.id
comment.save()
return JsonResponse(safe_content(comment.to_dict()))
return JsonResponse(safe_content(comment.to_dict(), course_key))
@require_POST
......@@ -270,13 +275,14 @@ def openclose_thread(request, course_id, thread_id):
given a course_id and thread_id, toggle the status of this thread
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
thread = cc.Thread.find(thread_id)
thread.closed = request.POST.get('closed', 'false').lower() == 'true'
thread.save()
thread = thread.to_dict()
return JsonResponse({
'content': safe_content(thread),
'ability': get_ability(SlashSeparatedCourseKey.from_deprecated_string(course_id), thread, request.user),
'content': safe_content(thread, course_key),
'ability': get_ability(course_key, thread, request.user),
})
......@@ -302,9 +308,10 @@ def delete_comment(request, course_id, comment_id):
given a course_id and comment_id delete this comment
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
comment = cc.Comment.find(comment_id)
comment.delete()
return JsonResponse(safe_content(comment.to_dict()))
return JsonResponse(safe_content(comment.to_dict(), course_key))
@require_POST
......@@ -314,10 +321,11 @@ def vote_for_comment(request, course_id, comment_id, value):
"""
given a course_id and comment_id,
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = cc.User.from_django_user(request.user)
comment = cc.Comment.find(comment_id)
user.vote(comment, value)
return JsonResponse(safe_content(comment.to_dict()))
return JsonResponse(safe_content(comment.to_dict(), course_key))
@require_POST
......@@ -328,10 +336,11 @@ def undo_vote_for_comment(request, course_id, comment_id):
given a course id and comment id, remove vote
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = cc.User.from_django_user(request.user)
comment = cc.Comment.find(comment_id)
user.unvote(comment)
return JsonResponse(safe_content(comment.to_dict()))
return JsonResponse(safe_content(comment.to_dict(), course_key))
@require_POST
......@@ -342,10 +351,11 @@ def vote_for_thread(request, course_id, thread_id, value):
given a course id and thread id vote for this thread
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
user.vote(thread, value)
return JsonResponse(safe_content(thread.to_dict()))
return JsonResponse(safe_content(thread.to_dict(), course_key))
@require_POST
......@@ -356,10 +366,11 @@ def flag_abuse_for_thread(request, course_id, thread_id):
given a course_id and thread_id flag this thread for abuse
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
thread.flagAbuse(user, thread)
return JsonResponse(safe_content(thread.to_dict()))
return JsonResponse(safe_content(thread.to_dict(), course_key))
@require_POST
......@@ -371,12 +382,12 @@ def un_flag_abuse_for_thread(request, course_id, thread_id):
ajax only
"""
user = cc.User.from_django_user(request.user)
course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
course = get_course_by_id(course_id)
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
course = get_course_by_id(course_key)
thread = cc.Thread.find(thread_id)
remove_all = cached_has_permission(request.user, 'openclose_thread', course_id) or has_access(request.user, 'staff', course)
remove_all = cached_has_permission(request.user, 'openclose_thread', course_key) or has_access(request.user, 'staff', course)
thread.unFlagAbuse(user, thread, remove_all)
return JsonResponse(safe_content(thread.to_dict()))
return JsonResponse(safe_content(thread.to_dict(), course_key))
@require_POST
......@@ -387,10 +398,11 @@ def flag_abuse_for_comment(request, course_id, comment_id):
given a course and comment id, flag comment for abuse
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = cc.User.from_django_user(request.user)
comment = cc.Comment.find(comment_id)
comment.flagAbuse(user, comment)
return JsonResponse(safe_content(comment.to_dict()))
return JsonResponse(safe_content(comment.to_dict(), course_key))
@require_POST
......@@ -407,7 +419,7 @@ def un_flag_abuse_for_comment(request, course_id, comment_id):
remove_all = cached_has_permission(request.user, 'openclose_thread', course_key) or has_access(request.user, 'staff', course)
comment = cc.Comment.find(comment_id)
comment.unFlagAbuse(user, comment, remove_all)
return JsonResponse(safe_content(comment.to_dict()))
return JsonResponse(safe_content(comment.to_dict(), course_key))
@require_POST
......@@ -418,10 +430,11 @@ def undo_vote_for_thread(request, course_id, thread_id):
given a course id and thread id, remove users vote for thread
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
user.unvote(thread)
return JsonResponse(safe_content(thread.to_dict()))
return JsonResponse(safe_content(thread.to_dict(), course_key))
@require_POST
......@@ -432,10 +445,11 @@ def pin_thread(request, course_id, thread_id):
given a course id and thread id, pin this thread
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
thread.pin(user, thread_id)
return JsonResponse(safe_content(thread.to_dict()))
return JsonResponse(safe_content(thread.to_dict(), course_key))
@require_POST
......@@ -446,10 +460,11 @@ def un_pin_thread(request, course_id, thread_id):
given a course id and thread id, remove pin from this thread
ajax only
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
thread.un_pin(user, thread_id)
return JsonResponse(safe_content(thread.to_dict()))
return JsonResponse(safe_content(thread.to_dict(), course_key))
@require_POST
......
lms/djangoapps/django_comment_client/forum/views.py
......@@ -150,7 +150,7 @@ def inline_discussion(request, course_id, discussion_id):
annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_info)
is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)
return utils.JsonResponse({
'discussion_data': [utils.safe_content(thread, is_staff) for thread in threads],
'discussion_data': [utils.safe_content(thread, course_id, is_staff) for thread in threads],
'user_info': user_info,
'annotated_content_info': annotated_content_info,
'page': query_params['page'],
......@@ -173,7 +173,7 @@ def forum_form_discussion(request, course_id):
try:
unsafethreads, query_params = get_threads(request, course_id) # This might process a search query
is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)
threads = [utils.safe_content(thread, is_staff) for thread in unsafethreads]
threads = [utils.safe_content(thread, course_id, is_staff) for thread in unsafethreads]
except cc.utils.CommentClientMaintenanceError:
log.warning("Forum is in maintenance mode")
return render_to_response('discussion/maintenance.html', {})
......@@ -253,7 +253,7 @@ def single_thread(request, course_id, discussion_id, thread_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(), is_staff)
content = utils.safe_content(thread.to_dict(), course_id, is_staff)
with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
add_courseware_context([content], course)
return utils.JsonResponse({
......@@ -276,7 +276,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
if not "pinned" in thread:
thread["pinned"] = False
threads = [utils.safe_content(thread, is_staff) for thread in threads]
threads = [utils.safe_content(thread, course_id, 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)
......@@ -335,7 +335,7 @@ def user_profile(request, course_id, user_id):
if request.is_ajax():
is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)
return utils.JsonResponse({
'discussion_data': [utils.safe_content(thread, is_staff) for thread in threads],
'discussion_data': [utils.safe_content(thread, course_id, is_staff) for thread in threads],
'page': query_params['page'],
'num_pages': query_params['num_pages'],
'annotated_content_info': _attr_safe_json(annotated_content_info),
......@@ -386,7 +386,7 @@ def followed_threads(request, course_id, user_id):
is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)
return utils.JsonResponse({
'annotated_content_info': annotated_content_info,
'discussion_data': [utils.safe_content(thread, is_staff) for thread in threads],
'discussion_data': [utils.safe_content(thread, course_id, is_staff) for thread in threads],
'page': query_params['page'],
'num_pages': query_params['num_pages'],
})
......
lms/djangoapps/django_comment_client/utils.py
......@@ -9,7 +9,7 @@ from django.db import connection
from django.http import HttpResponse
from django.utils import simplejson
from django_comment_common.models import Role, FORUM_ROLE_STUDENT
from django_comment_client.permissions import check_permissions_by_view
from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
from edxmako import lookup_template
import pystache_custom as pystache
......@@ -365,7 +365,7 @@ def add_courseware_context(content_list, course):
content.update({"courseware_url": url, "courseware_title": title})
def safe_content(content, is_staff=False):
def safe_content(content, course_id, is_staff=False):
fields = [
'id', 'title', 'body', 'course_id', 'anonymous', 'anonymous_to_peers',
'endorsed', 'parent_id', 'thread_id', 'votes', 'closed', 'created_at',
......@@ -375,14 +375,40 @@ def safe_content(content, is_staff=False):
'read', 'group_id', 'group_name', 'group_string', 'pinned', 'abuse_flaggers',
'stats', 'resp_skip', 'resp_limit', 'resp_total', 'thread_type',
'endorsed_responses', 'non_endorsed_responses', 'non_endorsed_resp_total',
'endorsement',
]
if (content.get('anonymous') is False) and ((content.get('anonymous_to_peers') is False) or is_staff):
fields += ['username', 'user_id']
content = strip_none(extract(content, fields))
if content.get("endorsement"):
endorsement = content["endorsement"]
endorser = None
if endorsement["user_id"]:
try:
endorser = User.objects.get(pk=endorsement["user_id"])
except User.DoesNotExist:
log.error("User ID {0} in endorsement for comment {1} but not in our DB.".format(
content.get('user_id'),
content.get('id'))
)
# Only reveal endorser if requester can see author or if endorser is staff
if (
endorser and
("username" in fields or cached_has_permission(endorser, "endorse_comment", course_id))
):
endorsement["username"] = endorser.username
else:
del endorsement["user_id"]
for child_content_key in ["children", "endorsed_responses", "non_endorsed_responses"]:
if child_content_key in content:
safe_children = [safe_content(child) for child in content[child_content_key]]
safe_children = [
safe_content(child, course_id, is_staff) for child in content[child_content_key]
]
content[child_content_key] = safe_children
return strip_none(extract(content, fields))
return content
lms/lib/comment_client/comment.py
......@@ -11,12 +11,12 @@ class Comment(models.Model):
'id', 'body', 'anonymous', 'anonymous_to_peers', 'course_id',
'endorsed', 'parent_id', 'thread_id', 'username', 'votes', 'user_id',
'closed', 'created_at', 'updated_at', 'depth', 'at_position_list',
'type', 'commentable_id', 'abuse_flaggers'
'type', 'commentable_id', 'abuse_flaggers', 'endorsement',
]
updatable_fields = [
'body', 'anonymous', 'anonymous_to_peers', 'course_id', 'closed',
'user_id', 'endorsed'
'user_id', 'endorsed', 'endorsement_user_id',
]
initializable_fields = updatable_fields
......
lms/lib/comment_client/models.py
......@@ -35,7 +35,7 @@ class Model(object):
return self.__getattr__(name)
def __setattr__(self, name, value):
if name == 'attributes' or name not in self.accessible_fields:
if name == 'attributes' or name not in (self.accessible_fields + self.updatable_fields):
super(Model, self).__setattr__(name, value)
else:
self.attributes[name] = value
......@@ -46,7 +46,7 @@ class Model(object):
return self.attributes.get(key)
def __setitem__(self, key, value):
if key not in self.accessible_fields:
if key not in (self.accessible_fields + self.updatable_fields):
raise KeyError("Field {0} does not exist".format(key))
self.attributes.__setitem__(key, value)
......
lms/static/sass/discussion/_discussion.scss
......@@ -441,7 +441,8 @@ body.discussion {
font-weight: 700;
}
span {
.timeago, .top-post-status {
color: inherit;
font-style: italic;
}
}
......
lms/templates/discussion/_underscore_templates.html
......@@ -159,7 +159,28 @@
${"<% } else { %>"}
<span class="anonymous"><em>${_('anonymous')}</em></span>
${"<% } %>"}
<p class="posted-details" title="${'<%- created_at %>'}">${'<%- created_at %>'}</p>
<p class="posted-details">
<span class="timeago" title="${'<%= created_at %>'}">${'<%= created_at %>'}</span>
<%
js_block = u"""
interpolate(
endorsement.username ? "{user_fmt_str}" : "{anon_fmt_str}",
{{
'time_ago': '<span class="timeago" title="' + endorsement.time + '">' + endorsement.time + '</span>',
'user': endorsement.username
}},
true
)""".format(
## Translators: time_ago is a placeholder for a fuzzy, relative timestamp
## like "4 hours ago" or "about a month ago"
user_fmt_str=escapejs(_("marked as answer %(time_ago)s by %(user)s")),
## Translators: time_ago is a placeholder for a fuzzy, relative timestamp
## like "4 hours ago" or "about a month ago"
anon_fmt_str=escapejs(_("marked as answer %(time_ago)s")),
)
%>
${"<% if (thread.get('thread_type') == 'question' && obj.endorsement) { %> - <%="}${js_block}${"%><% } %>"}
</p>
</header>
<div class="response-body">${"<%- body %>"}</div>
<div class="discussion-flag-abuse notflagged" data-role="thread-flag" role="button" aria-pressed="false" tabindex="0">
......
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