Skip to content

E
edx-platform
Commit 8d1e10bd by E. Kolpakov
Options

Fixes and improvements from solutions fork: 

* Removing jQuery.ajax.beforeSend - it might be set globally and this code overrides it (part of 62fa253a)
* Avoid resetting Backbone history if already started (part of 9591b526)
* goHome no longer triggers thread:removed event - already triggered up the call stack (part of 9591b526)
* Avoid using beforeSend as it might be set globally (adapted from 2dfe104f)
* Refactor triggering navigation to use constant/method instead of magic strings (part of 4843facd)
* Switching to using jQueyr.prop instead of jQuery.attr
* Fixed email notifications enable/disable (adapted from 8a7f0224)
* Load thread if requested thread isn't loaded yet (adapted from e28dde32)
* Added ability to set custom css classes on search messages (adapted from b2e4c189)
* Actually rerendering view with new model data when it is loaded/changed
parent e1dbfe3a
Showing with 195 additions and 96 deletions
common/static/common/js/discussion/discussion_router.js
/* globals DiscussionThreadListView, DiscussionThreadView, DiscussionUtil, NewPostView */
/* globals DiscussionThreadListView, DiscussionThreadView, DiscussionUtil, NewPostView, Thread */
(function() {
'use strict';
var __hasProp = {}.hasOwnProperty,
......@@ -18,9 +18,20 @@
return child;
};
function getSingleThreadRoute(commentable_id, thread_id) {
return commentable_id + "/threads/" + thread_id;
}
if (typeof Backbone !== "undefined" && Backbone !== null) {
this.DiscussionRouter = (function(_super) {
var allThreadsRoute = "",
singleThreadRoute = getSingleThreadRoute(":forum_name", ":thread_id"), // :forum_name/threads/:thread_id
routes = {};
routes[allThreadsRoute] = "allThreads";
routes[singleThreadRoute] = "showThread";
__extends(DiscussionRouter, _super);
function DiscussionRouter() {
......@@ -40,16 +51,16 @@
this.showMain = function() {
return DiscussionRouter.prototype.showMain.apply(self, arguments);
};
this.renderThreadView = function() {
return DiscussionRouter.prototype.renderThreadView.apply(self, arguments);
};
this.setActiveThread = function() {
return DiscussionRouter.prototype.setActiveThread.apply(self, arguments);
};
return DiscussionRouter.__super__.constructor.apply(this, arguments);
}
DiscussionRouter.prototype.routes = {
"": "allThreads",
":forum_name/threads/:thread_id": "showThread"
};
DiscussionRouter.prototype.routes = routes;
DiscussionRouter.prototype.initialize = function(options) {
var self = this;
......@@ -89,16 +100,44 @@
if (this.thread) {
return this.nav.setActiveThread(this.thread.get("id"));
} else {
return this.nav.goHome;
return this.nav.goHome();
}
};
DiscussionRouter.prototype.showThread = function(forum_name, thread_id) {
var self;
self = this;
this.thread = this.discussion.get(thread_id);
if (this.thread) {
this.renderThreadView();
return;
}
// if thread is not loaded yet for some reason - try loading it
DiscussionUtil.safeAjax({
url: DiscussionUtil.urlFor('retrieve_single_thread', forum_name, thread_id)
}).done(function(data) {
// if succeded - proceed normally
self.thread = new Thread(data.content);
self.discussion.add(self.thread);
self.renderThreadView();
}).fail(function(xhr) {
// otherwise display error message and navigate to all threads view
var errorMessage = (xhr.status === 404) ?
gettext("The thread you selected has been deleted. Please select another thread.") :
gettext("We had some trouble loading more responses. Please try again.");
DiscussionUtil.discussionAlert(gettext("Sorry"), errorMessage);
this.allThreads();
});
};
DiscussionRouter.prototype.renderThreadView = function() {
this.thread.set("unread_comments_count", 0);
this.thread.set("read", true);
this.setActiveThread();
return this.showMain();
this.showMain();
};
DiscussionRouter.prototype.showMain = function() {
......@@ -127,17 +166,14 @@
};
DiscussionRouter.prototype.navigateToThread = function(thread_id) {
var thread;
var thread, targetThreadRoute;
thread = this.discussion.get(thread_id);
return this.navigate("" + (thread.get("commentable_id")) + "/threads/" + thread_id, {
trigger: true
});
targetThreadRoute = getSingleThreadRoute(thread.get("commentable_id"), thread_id);
this.navigate(targetThreadRoute, { trigger: true});
};
DiscussionRouter.prototype.navigateToAllThreads = function() {
return this.navigate("", {
trigger: true
});
this.navigate(allThreadsRoute, { trigger: true });
};
DiscussionRouter.prototype.showNewPost = function() {
......
common/static/common/js/discussion/main.js
......@@ -34,10 +34,11 @@
course_settings: course_settings
});
/* jshint +W031*/
return Backbone.history.start({
pushState: true,
root: "/courses/" + $$course_id + "/discussion/forum/"
});
if (!Backbone.History.started) {
Backbone.history.start({pushState: true, root: "/courses/" + $$course_id + "/discussion/forum/"});
} else {
Backbone.history.loadUrl(window.location.pathname);
}
}
};
DiscussionProfileApp = {
......
common/static/common/js/discussion/utils.js
......@@ -184,9 +184,9 @@
DiscussionUtil.safeAjax = function(params) {
var $elem, deferred, request,
self = this;
self = this, deferred;
$elem = params.$elem;
if ($elem && $elem.attr("disabled")) {
if ($elem && $elem.prop("disabled")) {
deferred = $.Deferred();
deferred.reject();
return deferred.promise();
......@@ -194,18 +194,6 @@
params.url = URI(params.url).addSearch({
ajax: 1
});
params.beforeSend = function() {
if ($elem) {
$elem.attr("disabled", "disabled");
}
if (params.$loading) {
if (params.loadingCallback) {
return params.loadingCallback.apply(params.$loading);
} else {
return self.showLoadingIndicator($(params.$loading), params.takeFocus);
}
}
};
if (!params.error) {
params.error = function() {
self.discussionAlert(
......@@ -216,9 +204,21 @@
);
};
}
if ($elem) {
$elem.prop("disabled", true);
}
if (params.$loading) {
if (params.loadingCallback) {
params.loadingCallback.apply(params.$loading);
} else {
self.showLoadingIndicator($(params.$loading), params.takeFocus);
}
}
request = $.ajax(params).always(function() {
if ($elem) {
$elem.removeAttr("disabled");
$elem.prop("disabled", false);
}
if (params.$loading) {
if (params.loadedCallback) {
......@@ -231,7 +231,7 @@
return request;
};
DiscussionUtil.updateWithUndo = function(model, updates, safeAjaxParams, errorMsg) {
DiscussionUtil.updateWithUndo = function(model, updates, safeAjaxParams, errorMsg, beforeSend) {
var undo,
self = this;
if (errorMsg) {
......@@ -241,6 +241,9 @@
}
undo = _.pick(model.attributes, _.keys(updates));
model.set(updates);
if (typeof beforeSend === 'function') {
beforeSend();
}
return this.safeAjax(safeAjaxParams).fail(function() {
return model.set(undo);
});
......
common/static/common/js/discussion/views/discussion_content_view.js
......@@ -389,18 +389,18 @@
msg = gettext("We had some trouble removing this endorsement. Please try again.");
}
}
beforeFunc = function() {
return self.trigger("comment:endorse");
};
return DiscussionUtil.updateWithUndo(this.model, updates, {
url: url,
type: "POST",
data: {
endorsed: is_endorsing
return DiscussionUtil.updateWithUndo(
this.model,
updates,
{
url: url,
type: "POST",
data: { endorsed: is_endorsing },
$elem: $(event.currentTarget)
},
beforeSend: beforeFunc,
$elem: $(event.currentTarget)
}, msg).always(this.trigger("comment:endorse"));
msg,
function() { return self.trigger("comment:endorse"); }
).always(this.trigger("comment:endorse"));
};
DiscussionContentShowView.prototype.toggleVote = function(event) {
......
common/static/common/js/discussion/views/discussion_thread_list_view.js
......@@ -123,6 +123,7 @@
return self.displayedCollection.reset(discussion.models);
});
this.collection.on("add", this.addAndSelectThread);
this.collection.on("thread:remove", this.threadRemoved);
this.sidebar_padding = 10;
this.boardName = null;
this.template = _.template($("#thread-list-template").html());
......@@ -135,7 +136,8 @@
var content;
content = _.template($("#search-alert-template").html())({
'message': searchAlert.attributes.message,
'cid': searchAlert.cid
'cid': searchAlert.cid,
'css_class': searchAlert.attributes.css_class
});
self.$(".search-alerts").append(content);
return self.$("#search-alert-" + searchAlert.cid + " a.dismiss")
......@@ -151,11 +153,19 @@
});
};
DiscussionThreadListView.prototype.addSearchAlert = function(message) {
/**
* Creates search alert model and adds it to collection
* @param message - alert message
* @param css_class - Allows setting custom css class for a message. This can be used to style messages
* of different types differently (i.e. other background, completely hide, etc.)
* @returns {Backbone.Model}
*/
DiscussionThreadListView.prototype.addSearchAlert = function(message, css_class) {
var m;
m = new Backbone.Model({
"message": message
});
if (typeof css_class === 'undefined' || css_class === null) {
css_class = "";
}
m = new Backbone.Model({"message": message, "css_class": css_class});
this.searchAlertCollection.add(m);
return m;
};
......@@ -392,8 +402,8 @@
return false;
};
DiscussionThreadListView.prototype.threadRemoved = function(thread_id) {
return this.trigger("thread:removed", thread_id);
DiscussionThreadListView.prototype.threadRemoved = function(thread) {
this.trigger("thread:removed", thread);
};
DiscussionThreadListView.prototype.setActiveThread = function(thread_id) {
......@@ -406,7 +416,7 @@
};
DiscussionThreadListView.prototype.goHome = function() {
var thread_id, url;
var url;
this.template = _.template($("#discussion-home-template").html());
$(".forum-content").html(this.template);
$(".forum-nav-thread-list a").removeClass("is-active").find(".sr").remove();
......@@ -416,19 +426,9 @@
url: url,
type: "GET",
success: function(response) {
if (response.status) {
return $('input.email-setting').attr('checked', 'checked');
} else {
return $('input.email-setting').removeAttr('checked');
}
$('input.email-setting').prop('checked', response.status);
}
});
thread_id = null;
return this.trigger("thread:removed");
/*
select all threads
*/
};
DiscussionThreadListView.prototype.isBrowseMenuVisible = function() {
......@@ -731,8 +731,7 @@
url: DiscussionUtil.urlFor("users"),
type: "GET",
dataType: 'json',
error: function() {
},
error: function() {},
success: function(response) {
var message;
if (response.users.length > 0) {
......@@ -742,7 +741,7 @@
username: response.users[0].username
})
}, true);
return self.addSearchAlert(message);
return self.addSearchAlert(message, 'search-by-user');
}
}
});
......@@ -765,23 +764,17 @@
};
DiscussionThreadListView.prototype.updateEmailNotifications = function() {
if ($('input.email-setting').attr('checked')) {
return DiscussionUtil.safeAjax({
url: DiscussionUtil.urlFor("enable_notifications"),
type: "POST",
error: function() {
return $('input.email-setting').removeAttr('checked');
}
});
} else {
return DiscussionUtil.safeAjax({
url: DiscussionUtil.urlFor("disable_notifications"),
type: "POST",
error: function() {
return $('input.email-setting').attr('checked', 'checked');
}
});
}
var checkbox, checked, urlName;
checkbox = $('input.email-setting');
checked = checkbox.prop('checked');
urlName = (checked) ? "enable_notifications" : "disable_notifications";
DiscussionUtil.safeAjax({
url: DiscussionUtil.urlFor(urlName),
type: "POST",
error: function() {
checkbox.prop('checked', !checked);
}
});
};
return DiscussionThreadListView;
......
common/static/common/js/discussion/views/discussion_thread_view.js
......@@ -91,6 +91,7 @@
id = self.model.get("id");
if (collection.get(id)) {
self.model = collection.get(id);
self.rerender();
}
});
this.createShowView();
......
common/static/common/js/spec/discussion/utils_spec.js
......@@ -3,9 +3,10 @@
'use strict';
describe('DiscussionUtil', function() {
beforeEach(function() {
return DiscussionSpecHelper.setUpGlobals();
DiscussionSpecHelper.setUpGlobals();
});
return describe("updateWithUndo", function() {
describe("updateWithUndo", function() {
it("calls through to safeAjax with correct params, and reverts the model in case of failure", function() {
var deferred, model, res, updates;
deferred = $.Deferred();
......@@ -45,13 +46,13 @@
updates = {
hello: "world"
};
$elem = jasmine.createSpyObj('$elem', ['attr']);
$elem.attr.and.returnValue(true);
$elem = jasmine.createSpyObj('$elem', ['prop']);
$elem.prop.and.returnValue(true);
res = DiscussionUtil.updateWithUndo(model, updates, {
foo: "bar",
$elem: $elem
}, "error message");
expect($elem.attr).toHaveBeenCalledWith("disabled");
expect($elem.prop).toHaveBeenCalledWith("disabled");
expect(DiscussionUtil.safeAjax).toHaveBeenCalled();
expect(model.attributes).toEqual({
hello: false,
......@@ -64,6 +65,30 @@
return expect(failed).toBe(true);
});
});
describe('safeAjax', function() {
function dismissAlert() {
$(".modal#discussion-alert").remove();
}
it('respects global beforeSend', function() {
var beforeSendSpy = jasmine.createSpy();
$.ajaxSetup({beforeSend: beforeSendSpy});
var $elem = jasmine.createSpyObj('$elem', ['prop']);
DiscussionUtil.safeAjax({
$elem: $elem,
url: "/",
type: "GET",
dataType: "json"
}).always(function() {
dismissAlert();
});
expect($elem.prop).toHaveBeenCalledWith("disabled", true);
expect(beforeSendSpy).toHaveBeenCalled();
});
});
});
}).call(this);
common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js
......@@ -261,7 +261,7 @@
});
});
describe("search alerts", function() {
var testAlertMessages;
var testAlertMessages, getAlertMessagesAndClasses;
testAlertMessages = function(expectedMessages) {
return expect($(".search-alert .message").map(function() {
......@@ -269,6 +269,15 @@
}).get()).toEqual(expectedMessages);
};
getAlertMessagesAndClasses = function () {
return $(".search-alert").map(function() {
return {
text: $('.message', this).html(),
css_class: $(this).attr('class')
};
}).get();
};
it("renders and removes search alerts", function() {
var bar, foo;
testAlertMessages([]);
......@@ -282,6 +291,27 @@
return testAlertMessages([]);
});
it("renders search alert with custom class", function() {
var foo, messages;
testAlertMessages([]);
this.view.addSearchAlert("foo", "custom-class");
messages = getAlertMessagesAndClasses();
expect(messages.length).toEqual(1);
expect(messages[0].text).toEqual("foo");
expect(messages[0].css_class).toEqual("search-alert custom-class");
foo = this.view.addSearchAlert("bar", "other-class");
messages = getAlertMessagesAndClasses();
expect(messages.length).toEqual(2);
expect(messages[0].text).toEqual("foo");
expect(messages[0].css_class).toEqual("search-alert custom-class");
expect(messages[1].text).toEqual("bar");
expect(messages[1].css_class).toEqual("search-alert other-class");
});
it("clears all search alerts", function() {
this.view.addSearchAlert("foo");
this.view.addSearchAlert("bar");
......
common/static/common/js/spec/discussion/view/new_post_view_spec.js
......@@ -94,7 +94,7 @@
this.view.render();
expectedGroupId = null;
DiscussionSpecHelper.makeAjaxSpy(function(params) {
return expect(params.data.group_id).toEqual(expectedGroupId);
expect(params.data.group_id).toEqual(expectedGroupId);
});
return _.each(["1", "2", ""], function(groupIdStr) {
expectedGroupId = groupIdStr;
......@@ -103,6 +103,7 @@
self.view.$(".js-post-body textarea").val("dummy body");
self.view.$(".forum-new-post-form").submit();
expect($.ajax).toHaveBeenCalled();
self.view.$(".forum-new-post-form").prop("disabled", false);
return $.ajax.calls.reset();
});
});
......
common/static/common/js/spec/discussion/view/response_comment_view_spec.js
......@@ -33,8 +33,10 @@
}
});
this.event = DiscussionSpecHelper.makeEventSpy();
this.event.target = $("body");
spyOn(this.comment, "remove");
return spyOn(this.view.$el, "remove");
spyOn(this.view.$el, "remove");
$(this.event.target).prop("disabled", false);
});
setAjaxResult = function(isSuccess) {
return spyOn($, "ajax").and.callFake(function(params) {
......@@ -151,7 +153,7 @@
this.view.$el.find(".edit-comment-body").html($("<textarea></textarea>"));
this.view.$el.find(".edit-comment-body textarea").val(this.updatedBody);
spyOn(this.view, 'cancelEdit');
return spyOn($, "ajax").and.callFake(function(params) {
spyOn($, "ajax").and.callFake(function(params) {
if (self.ajaxSucceed) {
params.success();
} else {
......@@ -164,10 +166,17 @@
}
};
});
this.event = DiscussionSpecHelper.makeEventSpy();
// All the way down in discussion/utils.js there's this line
// element.after(...);
// element is event.target in this case. This causes a JS exception, so we override the target
this.event.target = $("body");
$(this.event.target).prop("disabled", false);
});
it('calls the update endpoint correctly and displays the show view on success', function() {
this.ajaxSucceed = true;
this.view.update(DiscussionSpecHelper.makeEventSpy());
this.view.update(this.event);
expect($.ajax).toHaveBeenCalled();
expect($.ajax.calls.mostRecent().args[0].url._parts.path)
.toEqual('/courses/edX/999/test/discussion/comments/01234567/update');
......@@ -179,7 +188,7 @@
var originalBody;
originalBody = this.comment.get("body");
this.ajaxSucceed = false;
this.view.update(DiscussionSpecHelper.makeEventSpy());
this.view.update(this.event);
expect($.ajax).toHaveBeenCalled();
expect($.ajax.calls.mostRecent().args[0].url._parts.path)
.toEqual('/courses/edX/999/test/discussion/comments/01234567/update');
......
common/static/common/templates/discussion/search-alert.underscore
<div class="search-alert" id="search-alert-<%- cid %>">
<div class="search-alert <%= css_class %>" id="search-alert-<%- cid %>">
<div class="search-alert-content">
<p class="message"><%= message %></p>
</div>
......
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