diff --git a/common/static/coffee/spec/discussion/discussion_spec_helper.coffee b/common/static/coffee/spec/discussion/discussion_spec_helper.coffee index 73f6eecb32..d1a3de530a 100644 --- a/common/static/coffee/spec/discussion/discussion_spec_helper.coffee +++ b/common/static/coffee/spec/discussion/discussion_spec_helper.coffee @@ -19,6 +19,23 @@ class @DiscussionSpecHelper {always: ->} ) + @makeEventSpy = () -> + jasmine.createSpyObj('event', ['preventDefault', 'target']) + + @makeCourseSettings = (is_cohorted=true) -> + new DiscussionCourseSettings( + category_map: + children: ['Test Topic', 'Other Topic'] + entries: + 'Test Topic': + is_cohorted: is_cohorted + id: 'test_topic' + 'Other Topic': + is_cohorted: is_cohorted + id: 'other_topic' + is_cohorted: is_cohorted + ) + @setUnderscoreFixtures = -> for templateName in ['thread-show'] templateFixture = readFixtures('templates/discussion/' + templateName + '.underscore') diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_edit_view_spec.js b/common/static/coffee/spec/discussion/view/discussion_thread_edit_view_spec.js index 34bba0e813..2f1e07b8d2 100644 --- a/common/static/coffee/spec/discussion/view/discussion_thread_edit_view_spec.js +++ b/common/static/coffee/spec/discussion/view/discussion_thread_edit_view_spec.js @@ -1,32 +1,24 @@ (function() { 'use strict'; describe('DiscussionThreadEditView', function() { + var testUpdate, testCancel; + beforeEach(function() { DiscussionSpecHelper.setUpGlobals(); DiscussionSpecHelper.setUnderscoreFixtures(); spyOn(DiscussionUtil, 'makeWmdEditor'); - this.threadData = DiscussionViewSpecHelper.makeThreadWithProps(); - this.thread = new Thread(this.threadData); - this.course_settings = new DiscussionCourseSettings({ - 'category_map': { - 'children': ['Topic'], - 'entries': { - 'Topic': { - 'is_cohorted': true, - 'id': 'topic' - } - } - }, - 'is_cohorted': true + this.threadData = DiscussionViewSpecHelper.makeThreadWithProps({ + 'commentable_id': 'test_topic', + 'title': 'test thread title' }); + this.thread = new Thread(this.threadData); + this.course_settings = DiscussionSpecHelper.makeCourseSettings(); this.createEditView = function (options) { - options = _.extend({ + options = _.extend({ container: $('#fixture-element'), model: this.thread, mode: 'tab', - topicId: 'dummy_id', - threadType: 'question', course_settings: this.course_settings }, options); this.view = new DiscussionThreadEditView(options); @@ -34,36 +26,58 @@ }; }); - it('can save new data correctly', function() { - var view; + testUpdate = function(view, thread) { spyOn($, 'ajax').andCallFake(function(params) { expect(params.url.path()).toEqual(DiscussionUtil.urlFor('update_thread', 'dummy_id')); - expect(params.data.thread_type).toBe('discussion'); - expect(params.data.commentable_id).toBe('topic'); - expect(params.data.title).toBe('new_title'); + if (view.isTabMode()) { + // TODO remove the tabMode condition, depends on #5554 / TNL-606 + expect(params.data.thread_type).toBe('discussion'); + } + expect(params.data.commentable_id).toBe('other_topic'); + expect(params.data.title).toBe('changed thread title'); params.success(); return {always: function() {}}; }); - this.createEditView(); - this.view.$el.find('a.topic-title').first().click(); // set new topic - this.view.$('.edit-post-title').val('new_title'); // set new title - this.view.$("label[for$='post-type-discussion']").click(); // set new thread type - this.view.$('.post-update').click(); + view.$el.find('a.topic-title')[1].click(); // set new topic + view.$('.edit-post-title').val('changed thread title'); // set new title + view.$("label[for$='post-type-discussion']").click(); // set new thread type + view.$('.post-update').click(); expect($.ajax).toHaveBeenCalled(); - expect(this.thread.get('title')).toBe('new_title'); - expect(this.thread.get('commentable_id')).toBe('topic'); - expect(this.thread.get('thread_type')).toBe('discussion'); - expect(this.thread.get('courseware_title')).toBe('Topic'); + expect(thread.get('title')).toBe('changed thread title'); + if (view.isTabMode()) { + // TODO remove the tabMode condition, depends on #5554 / TNL-606 + expect(thread.get('thread_type')).toBe('discussion'); + } + expect(thread.get('commentable_id')).toBe('other_topic'); + expect(thread.get('courseware_title')).toBe('Other Topic'); + expect(view.$('.edit-post-title')).toHaveValue(''); + expect(view.$('.wmd-preview p')).toHaveText(''); + } - expect(this.view.$('.edit-post-title')).toHaveValue(''); - expect(this.view.$('.wmd-preview p')).toHaveText(''); + it('can save new data correctly in tab mode', function() { + this.createEditView(); + testUpdate(this.view, this.thread); }); - it('can close the view', function() { - this.createEditView(); - this.view.$('.post-cancel').click(); + it('can save new data correctly in inline mode', function() { + this.createEditView({"mode": "inline"}); + testUpdate(this.view, this.thread); + }); + + testCancel = function(view) { + view.$('.post-cancel').click(); expect($('.edit-post-form')).not.toExist(); + } + + it('can close the view in tab mode', function() { + this.createEditView(); + testCancel(this.view); + }); + + it('can close the view in inline mode', function() { + this.createEditView({"mode": "inline"}); + testCancel(this.view); }); }); }).call(this); diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee index 06a435e6a9..42b5b97551 100644 --- a/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee +++ b/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee @@ -11,6 +11,7 @@ describe "DiscussionThreadView", -> # Avoid unnecessary boilerplate spyOn(DiscussionThreadShowView.prototype, "convertMath") spyOn(DiscussionContentView.prototype, "makeWmdEditor") + spyOn(DiscussionUtil, "makeWmdEditor") spyOn(ThreadResponseView.prototype, "renderShowView") renderWithContent = (view, content) -> @@ -46,7 +47,12 @@ describe "DiscussionThreadView", -> threadData = DiscussionViewSpecHelper.makeThreadWithProps({closed: originallyClosed}) thread = new Thread(threadData) discussion = new Discussion(thread) - view = new DiscussionThreadView({ model: thread, el: $("#fixture-element"), mode: mode}) + view = new DiscussionThreadView( + model: thread + el: $("#fixture-element") + mode: mode + course_settings: DiscussionSpecHelper.makeCourseSettings() + ) renderWithContent(view, {resp_total: 1, children: [{}]}) if mode == "inline" view.expand() @@ -70,7 +76,12 @@ describe "DiscussionThreadView", -> describe "tab mode", -> beforeEach -> - @view = new DiscussionThreadView({ model: @thread, el: $("#fixture-element"), mode: "tab"}) + @view = new DiscussionThreadView( + model: @thread + el: $("#fixture-element") + mode: "tab" + course_settings: DiscussionSpecHelper.makeCourseSettings() + ) describe "response count and pagination", -> it "correctly render for a thread with no responses", -> @@ -111,7 +122,12 @@ describe "DiscussionThreadView", -> describe "inline mode", -> beforeEach -> - @view = new DiscussionThreadView({ model: @thread, el: $("#fixture-element"), mode: "inline"}) + @view = new DiscussionThreadView( + model: @thread + el: $("#fixture-element") + mode: "inline" + course_settings: DiscussionSpecHelper.makeCourseSettings() + ) describe "render", -> it "shows content that should be visible when collapsed", -> @@ -178,11 +194,26 @@ describe "DiscussionThreadView", -> expect($(".post-body").text()).toEqual(maliciousAbbreviation) expect($(".post-body").html()).not.toContain(" + DiscussionViewSpecHelper.setNextResponseContent({resp_total: 0, children: []}) + @view.render() + @view.expand() + assertExpandedContentVisible(@view, true) + @view.edit() + assertContentVisible(@view, ".edit-post-body", true) + expect(@view.$el.find(".post-actions-list").length).toBe(0) + @view.closeEditView(DiscussionSpecHelper.makeEventSpy()) + expect(@view.$el.find(".edit-post-body").length).toBe(0) + assertContentVisible(@view, ".post-actions-list", true) + describe "for question threads", -> beforeEach -> @thread.set("thread_type", "question") @view = new DiscussionThreadView( - {model: @thread, el: $("#fixture-element"), mode: "tab"} + model: @thread + el: $("#fixture-element") + mode: "tab" + course_settings: DiscussionSpecHelper.makeCourseSettings() ) renderTestCase = (view, numEndorsed, numNonEndorsed) -> 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 index 7c1b744d8e..9f641a8b16 100644 --- a/common/static/coffee/spec/discussion/view/response_comment_view_spec.coffee +++ b/common/static/coffee/spec/discussion/view/response_comment_view_spec.coffee @@ -17,12 +17,10 @@ describe 'ResponseCommentView', -> spyOn(DiscussionUtil, "makeWmdEditor") @view.render() - makeEventSpy = () -> jasmine.createSpyObj('event', ['preventDefault', 'target']) - describe '_delete', -> beforeEach -> @comment.updateInfo {ability: {can_delete: true}} - @event = makeEventSpy() + @event = DiscussionSpecHelper.makeEventSpy() spyOn(@comment, "remove") spyOn(@view.$el, "remove") @@ -81,9 +79,9 @@ describe 'ResponseCommentView', -> # Without calling renderEditView first, renderShowView is a no-op @view.renderEditView() @view.renderShowView() - @view.showView.trigger "comment:_delete", makeEventSpy() + @view.showView.trigger "comment:_delete", DiscussionSpecHelper.makeEventSpy() expect(@view._delete).toHaveBeenCalled() - @view.showView.trigger "comment:edit", makeEventSpy() + @view.showView.trigger "comment:edit", DiscussionSpecHelper.makeEventSpy() expect(@view.edit).toHaveBeenCalled() expect(@view.$(".edit-post-form#comment_#{@comment.id}")).not.toHaveClass("edit-post-form") @@ -92,9 +90,9 @@ describe 'ResponseCommentView', -> spyOn(@view, "update") spyOn(@view, "cancelEdit") @view.renderEditView() - @view.editView.trigger "comment:update", makeEventSpy() + @view.editView.trigger "comment:update", DiscussionSpecHelper.makeEventSpy() expect(@view.update).toHaveBeenCalled() - @view.editView.trigger "comment:cancel_edit", makeEventSpy() + @view.editView.trigger "comment:cancel_edit", DiscussionSpecHelper.makeEventSpy() expect(@view.cancelEdit).toHaveBeenCalled() expect(@view.$(".edit-post-form#comment_#{@comment.id}")).toHaveClass("edit-post-form") @@ -138,7 +136,7 @@ describe 'ResponseCommentView', -> it 'calls the update endpoint correctly and displays the show view on success', -> @ajaxSucceed = true - @view.update(makeEventSpy()) + @view.update(DiscussionSpecHelper.makeEventSpy()) expect($.ajax).toHaveBeenCalled() expect($.ajax.mostRecentCall.args[0].url._parts.path).toEqual('/courses/edX/999/test/discussion/comments/01234567/update') expect($.ajax.mostRecentCall.args[0].data.body).toEqual(@updatedBody) @@ -148,7 +146,7 @@ describe 'ResponseCommentView', -> it 'handles AJAX errors', -> originalBody = @comment.get("body") @ajaxSucceed = false - @view.update(makeEventSpy()) + @view.update(DiscussionSpecHelper.makeEventSpy()) expect($.ajax).toHaveBeenCalled() expect($.ajax.mostRecentCall.args[0].url._parts.path).toEqual('/courses/edX/999/test/discussion/comments/01234567/update') expect($.ajax.mostRecentCall.args[0].data.body).toEqual(@updatedBody) diff --git a/common/static/coffee/src/discussion/discussion_module_view.coffee b/common/static/coffee/src/discussion/discussion_module_view.coffee index ed5c3c914e..6bff86635f 100644 --- a/common/static/coffee/src/discussion/discussion_module_view.coffee +++ b/common/static/coffee/src/discussion/discussion_module_view.coffee @@ -98,7 +98,7 @@ if Backbone? @$el.append($discussion) @newPostForm = $('.new-post-article') - @threadviews = @discussion.map (thread) -> + @threadviews = @discussion.map (thread) => new DiscussionThreadView( el: @$("article#thread_#{thread.id}"), model: thread, diff --git a/common/static/coffee/src/discussion/views/discussion_thread_edit_view.js b/common/static/coffee/src/discussion/views/discussion_thread_edit_view.js index 3a93732548..13eb8bc57a 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_edit_view.js +++ b/common/static/coffee/src/discussion/views/discussion_thread_edit_view.js @@ -17,7 +17,7 @@ this.mode = options.mode || 'inline'; this.course_settings = options.course_settings; this.threadType = this.model.get('thread_type'); - this.topicId = options.topicId; + this.topicId = this.model.get('commentable_id'); _.bindAll(this); return this; }, @@ -32,12 +32,12 @@ threadTypeTemplate = _.template($("#thread-type-template").html()); this.addField(threadTypeTemplate({form_id: formId})); this.$("#" + formId + "-post-type-" + this.threadType).attr('checked', true); - this.topicView = new DiscussionTopicMenuView({ - topicId: this.topicId, - course_settings: this.course_settings - }); - this.addField(this.topicView.render()); } + this.topicView = new DiscussionTopicMenuView({ + topicId: this.topicId, + course_settings: this.course_settings + }); + this.addField(this.topicView.render()); DiscussionUtil.makeWmdEditor(this.$el, $.proxy(this.$, this), 'edit-post-body'); return this; }, @@ -55,7 +55,7 @@ var title = this.$('.edit-post-title').val(), threadType = this.$(".post-type-input:checked").val(), body = this.$('.edit-post-body textarea').val(), - commentableId = this.isTabMode() ? this.topicView.getCurrentTopicId() : null; + commentableId = this.topicView.getCurrentTopicId(); return DiscussionUtil.safeAjax({ $elem: this.submitBtn, @@ -74,19 +74,15 @@ success: function() { var newAttrs = { title: title, - body: body + body: body, + thread_type: threadType, + commentable_id: commentableId, + courseware_title: this.topicView.getFullTopicName() }; // @TODO: Move this out of the callback, this makes it feel sluggish this.$('.edit-post-title').val('').attr('prev-text', ''); this.$('.edit-post-body textarea').val('').attr('prev-text', ''); this.$('.wmd-preview p').html(''); - if (this.isTabMode()) { - _.extend(newAttrs, { - thread_type: threadType, - commentable_id: commentableId, - courseware_title: this.topicView.getFullTopicName() - }); - } this.model.set(newAttrs).unset('abbreviatedBody'); this.trigger('thread:updated'); if (this.threadType !== threadType) { diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee index 7d09242128..1fac688a1e 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee @@ -272,7 +272,6 @@ if Backbone? model: @model mode: @mode course_settings: @options.course_settings - topicId: @model.get('commentable_id') ) @editView.bind "thread:updated thread:cancel_edit", @closeEditView @editView.bind "comment:endorse", @endorseThread @@ -296,6 +295,9 @@ if Backbone? closeEditView: (event) => @createShowView() @renderShowView() + # next call is necessary to re-render the post action controls after + # submitting or cancelling a thread edit in inline mode. + @$el.find(".post-extended-content").show() # If you use "delete" here, it will compile down into JS that includes the # use of DiscussionThreadView.prototype.delete, and that will break IE8 diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 962a7f1f0b..5e0999923e 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -40,7 +40,7 @@ def _attr_safe_json(obj): return saxutils.escape(json.dumps(obj), {'"': '"'}) @newrelic.agent.function_trace() -def make_course_settings(course, include_category_map=False): +def make_course_settings(course): """ Generate a JSON-serializable model for course settings, which will be used to initialize a DiscussionCourseSettings object on the client. @@ -51,11 +51,9 @@ def make_course_settings(course, include_category_map=False): 'allow_anonymous': course.allow_anonymous, 'allow_anonymous_to_peers': course.allow_anonymous_to_peers, 'cohorts': [{"id": str(g.id), "name": g.name} for g in get_course_cohorts(course)], + 'category_map': utils.get_discussion_category_map(course) } - if include_category_map: - obj['category_map'] = utils.get_discussion_category_map(course) - return obj @newrelic.agent.function_trace() @@ -167,7 +165,7 @@ def forum_form_discussion(request, course_id): nr_transaction = newrelic.agent.current_transaction() course = get_course_with_access(request.user, 'load_forum', course_key, check_if_enrolled=True) - course_settings = make_course_settings(course, include_category_map=True) + course_settings = make_course_settings(course) user = cc.User.from_django_user(request.user) user_info = user.to_dict() @@ -231,7 +229,7 @@ def single_thread(request, course_id, discussion_id, thread_id): nr_transaction = newrelic.agent.current_transaction() course = get_course_with_access(request.user, 'load_forum', course_key) - course_settings = make_course_settings(course, include_category_map=True) + course_settings = make_course_settings(course) cc_user = cc.User.from_django_user(request.user) user_info = cc_user.to_dict() is_moderator = cached_has_permission(request.user, "see_all_cohorts", course_key) diff --git a/lms/static/sass/_shame.scss b/lms/static/sass/_shame.scss index 984f9dd3a2..86ac923028 100644 --- a/lms/static/sass/_shame.scss +++ b/lms/static/sass/_shame.scss @@ -262,3 +262,19 @@ footer .references { .dashboard { padding-top: 60px; } + +// ==================== + +// poor definition/scope on ul elements inside .vert-mod element in courseware - override needed for inline discussion editing +.course-content .discussion-post.edit-post-form .topic-menu { + padding-left: 0; + list-style: none; + + .topic-menu-item { + margin-bottom: 0; + } +} + +.course-content .discussion-post.edit-post-form .topic-submenu { + list-style: none; +}