From 7d32a90ae2fe2f4420cf7469f3cedc42a5bafb09 Mon Sep 17 00:00:00 2001
From: jsa <jsa@edx.org>
Date: Fri, 14 Feb 2014 11:44:20 -0500
Subject: [PATCH] add ability to delete comments on responses in discussions.
---
common/static/coffee/spec/discussion/content_spec.coffee | 6 +++---
common/static/coffee/spec/discussion/view/discussion_content_view_spec.coffee | 2 +-
common/static/coffee/spec/discussion/view/response_comment_show_view_spec.coffee | 35 +++++++++++++++++++++++------------
common/static/coffee/spec/discussion/view/response_comment_view_spec.coffee | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
common/static/coffee/src/discussion/views/response_comment_show_view.coffee | 10 ++++++++++
common/static/coffee/src/discussion/views/response_comment_view.coffee | 22 ++++++++++++++++++++++
common/static/coffee/src/discussion/views/thread_response_view.coffee | 1 +
lms/static/sass/_discussion.scss | 4 ++--
lms/templates/discussion/_underscore_templates.html | 2 ++
9 files changed, 134 insertions(+), 18 deletions(-)
create mode 100644 common/static/coffee/spec/discussion/view/response_comment_view_spec.coffee
diff --git a/common/static/coffee/spec/discussion/content_spec.coffee b/common/static/coffee/spec/discussion/content_spec.coffee
index 3a7cc35..b6075fe 100644
--- a/common/static/coffee/spec/discussion/content_spec.coffee
+++ b/common/static/coffee/spec/discussion/content_spec.coffee
@@ -2,7 +2,7 @@ describe 'All Content', ->
beforeEach ->
# TODO: figure out a better way of handling this
# It is set up in main.coffee DiscussionApp.start
- window.$$course_id = 'mitX/999/test'
+ window.$$course_id = 'edX/999/test'
window.user = new DiscussionUser {id: '567'}
describe 'Content', ->
@@ -10,7 +10,7 @@ describe 'All Content', ->
@content = new Content {
id: '01234567',
user_id: '567',
- course_id: 'mitX/999/test',
+ course_id: 'edX/999/test',
body: 'this is some content',
abuse_flaggers: ['123']
}
@@ -22,7 +22,7 @@ describe 'All Content', ->
@content.initialize
expect(Content.contents['01234567']).toEqual @content
expect(@content.get 'id').toEqual '01234567'
- expect(@content.get 'user_url').toEqual '/courses/mitX/999/test/discussion/forum/users/567'
+ expect(@content.get 'user_url').toEqual '/courses/edX/999/test/discussion/forum/users/567'
expect(@content.get 'children').toEqual []
expect(@content.get 'comments').toEqual(jasmine.any(Comments))
diff --git a/common/static/coffee/spec/discussion/view/discussion_content_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_content_view_spec.coffee
index 71495ad..d0ff555 100644
--- a/common/static/coffee/spec/discussion/view/discussion_content_view_spec.coffee
+++ b/common/static/coffee/spec/discussion/view/discussion_content_view_spec.coffee
@@ -25,7 +25,7 @@ describe "DiscussionContentView", ->
@threadData = {
id: '01234567',
user_id: '567',
- course_id: 'mitX/999/test',
+ course_id: 'edX/999/test',
body: 'this is a thread',
created_at: '2013-04-03T20:08:39Z',
abuse_flaggers: ['123'],
diff --git a/common/static/coffee/spec/discussion/view/response_comment_show_view_spec.coffee b/common/static/coffee/spec/discussion/view/response_comment_show_view_spec.coffee
index f43a880..2b918fb 100644
--- a/common/static/coffee/spec/discussion/view/response_comment_show_view_spec.coffee
+++ b/common/static/coffee/spec/discussion/view/response_comment_show_view_spec.coffee
@@ -8,6 +8,8 @@ describe 'ResponseCommentShowView', ->
<div class="response-body"><%- body %></div>
<div class="discussion-flag-abuse notflagged" data-role="thread-flag" data-tooltip="report misuse">
<i class="icon"></i><span class="flag-label"></span></div>
+ <div style="display:none" class="discussion-delete-comment action-delete" data-role="comment-delete" data-tooltip="Delete Comment" role="button" aria-pressed="false" tabindex="0">
+ <i class="icon icon-remove"></i><span class="sr delete-label">Delete Comment</span></div>
<p class="posted-details">–posted <span class="timeago" title="<%- created_at %>"><%- created_at %></span> by
<% if (obj.username) { %>
<a href="<%- user_url %>" class="profile-link"><%- username %></a>
@@ -18,18 +20,17 @@ describe 'ResponseCommentShowView', ->
"""
# set up a model for a new Comment
- @response = new Comment {
+ @comment = new Comment {
id: '01234567',
user_id: '567',
- course_id: 'mitX/999/test',
+ course_id: 'edX/999/test',
body: 'this is a response',
created_at: '2013-04-03T20:08:39Z',
abuse_flaggers: ['123']
roles: []
}
- @view = new ResponseCommentShowView({ model: @response })
-
- # spyOn(DiscussionUtil, 'loadRoles').andReturn []
+ @view = new ResponseCommentShowView({ model: @comment })
+ spyOn(@view, "convertMath")
it 'defines the tag', ->
expect($('#jasmine-fixtures')).toExist
@@ -37,26 +38,36 @@ describe 'ResponseCommentShowView', ->
expect(@view.el.tagName.toLowerCase()).toBe 'li'
it 'is tied to the model', ->
- expect(@view.model).toBeDefined();
+ expect(@view.model).toBeDefined()
describe 'rendering', ->
beforeEach ->
spyOn(@view, 'renderAttrs')
spyOn(@view, 'markAsStaff')
- spyOn(@view, 'convertMath')
it 'produces the correct HTML', ->
@view.render()
expect(@view.el.innerHTML).toContain('"discussion-flag-abuse notflagged"')
it 'can be flagged for abuse', ->
- @response.flagAbuse()
- expect(@response.get 'abuse_flaggers').toEqual ['123', '567']
+ @comment.flagAbuse()
+ expect(@comment.get 'abuse_flaggers').toEqual ['123', '567']
it 'can be unflagged for abuse', ->
temp_array = []
temp_array.push(window.user.get('id'))
- @response.set("abuse_flaggers",temp_array)
- @response.unflagAbuse()
- expect(@response.get 'abuse_flaggers').toEqual []
+ @comment.set("abuse_flaggers",temp_array)
+ @comment.unflagAbuse()
+ expect(@comment.get 'abuse_flaggers').toEqual []
+
+ describe 'comment deletion', ->
+
+ it 'triggers the delete event when the delete icon is clicked', ->
+ DiscussionUtil.loadRoles []
+ @comment.updateInfo {ability: {'can_delete': true}}
+ triggerTarget = jasmine.createSpy()
+ @view.bind "comment:_delete", triggerTarget
+ @view.render()
+ @view.$el.find('.action-delete').click()
+ expect(triggerTarget).toHaveBeenCalled()
diff --git a/common/static/coffee/spec/discussion/view/response_comment_view_spec.coffee b/common/static/coffee/spec/discussion/view/response_comment_view_spec.coffee
new file mode 100644
index 0000000..9a0bc03
--- /dev/null
+++ b/common/static/coffee/spec/discussion/view/response_comment_view_spec.coffee
@@ -0,0 +1,70 @@
+describe 'ResponseCommentView', ->
+ beforeEach ->
+
+ @comment = new Comment {
+ id: '01234567',
+ user_id: '567',
+ course_id: 'edX/999/test',
+ body: 'this is a response',
+ created_at: '2013-04-03T20:08:39Z',
+ abuse_flaggers: ['123']
+ roles: ['Student']
+ }
+ @view = new ResponseCommentView({ model: @comment })
+ spyOn(@view, "render")
+
+ describe '_delete', ->
+ beforeEach ->
+ @comment.updateInfo {ability: {can_delete: true}}
+ @event = jasmine.createSpyObj('event', ['preventDefault', 'target'])
+ spyOn(@comment, "remove")
+ spyOn(@view.$el, "remove")
+
+ setAjaxResult = (isSuccess) ->
+ spyOn($, "ajax").andCallFake(
+ (params) =>
+ (if isSuccess then params.success else params.error) {}
+ {always: ->}
+ )
+
+ it 'requires confirmation before deleting', ->
+ spyOn(window, "confirm").andReturn(false)
+ setAjaxResult(true)
+ @view._delete(@event)
+ expect(window.confirm).toHaveBeenCalled()
+ expect($.ajax).not.toHaveBeenCalled()
+ expect(@comment.remove).not.toHaveBeenCalled()
+
+ it 'removes the deleted comment object', ->
+ setAjaxResult(true)
+ @view._delete(@event)
+ expect(@comment.remove).toHaveBeenCalled()
+ expect(@view.$el.remove).toHaveBeenCalled()
+
+ it 'calls the ajax comment deletion endpoint', ->
+ setAjaxResult(true)
+ @view._delete(@event)
+ expect(@event.preventDefault).toHaveBeenCalled()
+ expect($.ajax).toHaveBeenCalled()
+ expect($.ajax.mostRecentCall.args[0].url._parts.path).toEqual('/courses/edX/999/test/discussion/comments/01234567/delete')
+
+ it 'handles ajax errors', ->
+ spyOn(DiscussionUtil, "discussionAlert")
+ setAjaxResult(false)
+ @view._delete(@event)
+ expect(@event.preventDefault).toHaveBeenCalled()
+ expect($.ajax).toHaveBeenCalled()
+ expect(@comment.remove).not.toHaveBeenCalled()
+ expect(@view.$el.remove).not.toHaveBeenCalled()
+ expect(DiscussionUtil.discussionAlert).toHaveBeenCalled()
+
+ it 'does not delete a comment if the permission is false', ->
+ @comment.updateInfo {ability: {'can_delete': false}}
+ spyOn(window, "confirm")
+ setAjaxResult(true)
+ @view._delete(@event)
+ expect(window.confirm).not.toHaveBeenCalled()
+ expect($.ajax).not.toHaveBeenCalled()
+ expect(@comment.remove).not.toHaveBeenCalled()
+ expect(@view.$el.remove).not.toHaveBeenCalled()
+
diff --git a/common/static/coffee/src/discussion/views/response_comment_show_view.coffee b/common/static/coffee/src/discussion/views/response_comment_show_view.coffee
index c1d974f..35f03a8 100644
--- a/common/static/coffee/src/discussion/views/response_comment_show_view.coffee
+++ b/common/static/coffee/src/discussion/views/response_comment_show_view.coffee
@@ -1,12 +1,20 @@
if Backbone?
class @ResponseCommentShowView extends DiscussionContentView
+ events:
+ "click .action-delete": "_delete"
+
tagName: "li"
initialize: ->
super()
@model.on "change", @updateModelDetails
+ abilityRenderer:
+ can_delete:
+ enable: -> @$(".action-delete").show()
+ disable: -> @$(".action-delete").hide()
+
render: ->
@template = _.template($("#response-comment-show-template").html())
params = @model.toJSON()
@@ -40,6 +48,8 @@ if Backbone?
else if DiscussionUtil.isTA(@model.get("user_id"))
@$el.find("a.profile-link").after('<span class="community-ta-label">' + gettext('Community TA') + '</span>')
+ _delete: (event) ->
+ @trigger "comment:_delete", event
renderFlagged: =>
if window.user.id in @model.get("abuse_flaggers") or (DiscussionUtil.isFlagModerator and @model.get("abuse_flaggers").length > 0)
diff --git a/common/static/coffee/src/discussion/views/response_comment_view.coffee b/common/static/coffee/src/discussion/views/response_comment_view.coffee
index f0bb975..dbf6a3d 100644
--- a/common/static/coffee/src/discussion/views/response_comment_view.coffee
+++ b/common/static/coffee/src/discussion/views/response_comment_view.coffee
@@ -21,6 +21,7 @@ if Backbone?
@editView = null
@showView = new ResponseCommentShowView(model: @model)
+ @showView.bind "comment:_delete", @_delete
renderSubView: (view) ->
view.setElement(@$el)
@@ -29,3 +30,24 @@ if Backbone?
renderShowView: () ->
@renderSubView(@showView)
+
+ _delete: (event) =>
+ event.preventDefault()
+ if not @model.can('can_delete')
+ return
+ if not confirm gettext("Are you sure you want to delete this comment?")
+ return
+ url = @model.urlFor('_delete')
+ $elem = $(event.target)
+ DiscussionUtil.safeAjax
+ $elem: $elem
+ url: url
+ type: "POST"
+ success: (response, textStatus) =>
+ @model.remove()
+ @$el.remove()
+ error: =>
+ DiscussionUtil.discussionAlert(
+ gettext("Sorry"),
+ gettext("We had some trouble deleting this comment. Please try again.")
+ )
diff --git a/common/static/coffee/src/discussion/views/thread_response_view.coffee b/common/static/coffee/src/discussion/views/thread_response_view.coffee
index 3f15791..1403e8d 100644
--- a/common/static/coffee/src/discussion/views/thread_response_view.coffee
+++ b/common/static/coffee/src/discussion/views/thread_response_view.coffee
@@ -91,6 +91,7 @@ if Backbone?
body: body
success: (response, textStatus) ->
comment.set(response.content)
+ comment.updateInfo(response.annotated_content_info)
view.render() # This is just to update the id for the most part, but might be useful in general
_delete: (event) =>
diff --git a/lms/static/sass/_discussion.scss b/lms/static/sass/_discussion.scss
index b7539b9..76234c7 100644
--- a/lms/static/sass/_discussion.scss
+++ b/lms/static/sass/_discussion.scss
@@ -2529,7 +2529,7 @@ body.discussion {
display:none;
}
-.discussion-flag-abuse {
+.discussion-flag-abuse, .discussion-delete-comment {
font-size: 12px;
float:right;
padding-right: 5px;
@@ -2542,7 +2542,7 @@ display:none;
opacity: 1.0;
}
}
-
+
.notflagged .icon {
display: block;
color: #333;
diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html
index 1505eba..aef9682 100644
--- a/lms/templates/discussion/_underscore_templates.html
+++ b/lms/templates/discussion/_underscore_templates.html
@@ -166,6 +166,8 @@
<div class="response-body">${'<%- body %>'}</div>
<div class="discussion-flag-abuse notflagged" data-role="thread-flag" data-tooltip="${_('Report Misuse') | h}" role="button" aria-pressed="false" tabindex="0">
<i class="icon icon-flag"></i><span class="sr flag-label">${_("Report Misuse")}</span></div>
+ <div style="display: none" class="discussion-delete-comment action-delete" data-tooltip="${_('Delete Comment') | h}" role="button" tabindex="0">
+ <i class="icon icon-remove"></i><span class="sr">${_("Delete Comment")}</span></div>
<%
js_block = u"""
interpolate(
--
libgit2 0.26.0