diff --git a/common/djangoapps/terrain/stubs/comments.py b/common/djangoapps/terrain/stubs/comments.py index c454177df3..af4648fa3f 100644 --- a/common/djangoapps/terrain/stubs/comments.py +++ b/common/djangoapps/terrain/stubs/comments.py @@ -6,7 +6,6 @@ import re import urlparse from .http import StubHttpRequestHandler, StubHttpService - class StubCommentsServiceHandler(StubHttpRequestHandler): @property @@ -23,26 +22,41 @@ class StubCommentsServiceHandler(StubHttpRequestHandler): "/api/v1/comments/(?P\\w+)$": self.do_comment, "/api/v1/(?P\\w+)/threads$": self.do_commentable, } + if self.match_pattern(pattern_handlers): + return + + self.send_response(404, content="404 Not Found") + + def match_pattern(self, pattern_handlers): path = urlparse.urlparse(self.path).path for pattern in pattern_handlers: match = re.match(pattern, path) if match: pattern_handlers[pattern](**match.groupdict()) - return - - self.send_response(404, content="404 Not Found") + return True + return None def do_PUT(self): if self.path.startswith('/set_config'): return StubHttpRequestHandler.do_PUT(self) + pattern_handlers = { + "/api/v1/users/(?P\\d+)$": self.do_put_user, + } + if self.match_pattern(pattern_handlers): + return self.send_response(204, "") + def do_put_user(self, user_id): + self.server.config['default_sort_key'] = self.post_dict.get("default_sort_key", "date") + self.send_json_response({'username': self.post_dict.get("username"), 'external_id': self.post_dict.get("external_id")}) + def do_DELETE(self): self.send_json_response({}) def do_user(self, user_id): response = { "id": user_id, + "default_sort_key": self.server.config.get("default_sort_key", "date"), "upvoted_ids": [], "downvoted_ids": [], "subscribed_thread_ids": [], diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee index 260fc07492..f2dd11b8cd 100644 --- a/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee +++ b/common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee @@ -3,6 +3,36 @@ describe "DiscussionThreadListView", -> beforeEach -> setFixtures """ +
""" + @threads = [ + {id: "1", title: "Thread1", body: "dummy body", votes: {up_count: '20'}, unread_comments_count:0, comments_count:1, created_at: '2013-04-03T20:08:39Z',}, + {id: "2", title: "Thread2", body: "dummy body", votes: {up_count: '42'}, unread_comments_count:0, comments_count:2, created_at: '2013-04-03T20:07:39Z',}, + {id: "3", title: "Thread3", body: "dummy body", votes: {up_count: '12'}, unread_comments_count:0, comments_count:3, created_at: '2013-04-03T20:06:39Z',}, + ] window.$$course_id = "TestOrg/TestCourse/TestRun" window.user = new DiscussionUser({id: "567", upvoted_ids: []}) @@ -98,3 +140,65 @@ describe "DiscussionThreadListView", -> spyOn(@view, "renderThread") @view.collection.trigger("change", new Thread({id: 1})) expect(@view.clearSearchAlerts).toHaveBeenCalled() + + makeView = (discussion) -> + return new DiscussionThreadListView( + el: $(".sidebar"), + collection: discussion + ) + + checkThreadsOrdering = (view, sort_order, type) -> + expect(view.$el.find(".post-list .list-item").children().length).toEqual(3) + expect(view.$el.find(".post-list .list-item:nth-child(1) .title").text()).toEqual(sort_order[0]) + expect(view.$el.find(".post-list .list-item:nth-child(2) .title").text()).toEqual(sort_order[1]) + expect(view.$el.find(".post-list .list-item:nth-child(3) .title").text()).toEqual(sort_order[2]) + expect(view.$el.find(".sort-bar a.active").text()).toEqual(type) + + describe "thread rendering should be correct", -> + checkRender = (threads, type, sort_order) -> + discussion = new Discussion(threads, {pages: 1, sort: type}) + view = makeView(discussion) + view.render() + checkThreadsOrdering(view, sort_order, type) + + it "with sort preference date", -> + checkRender(@threads, "date", [ "Thread1", "Thread2", "Thread3"]) + + it "with sort preference votes", -> + checkRender(@threads, "votes", [ "Thread2", "Thread1", "Thread3"]) + + it "with sort preference comments", -> + checkRender(@threads, "comments", [ "Thread3", "Thread2", "Thread1"]) + + describe "Sort click should be correct", -> + changeSorting = (threads, selected_type, new_type, sort_order) -> + discussion = new Discussion(threads, {pages: 1, sort: selected_type}) + view = makeView(discussion) + view.render() + expect(view.$el.find(".sort-bar a.active").text()).toEqual(selected_type) + sorted_threads = [] + if new_type == 'date' + sorted_threads = [threads[0], threads[1], threads[2]] + else if new_type == 'comments' + sorted_threads = [threads[2], threads[1], threads[0]] + else if new_type == 'votes' + sorted_threads = [threads[1], threads[0], threads[2]] + $.ajax.andCallFake((params) => + params.success( + {"discussion_data":sorted_threads, page:1, num_pages:1} + ) + {always: ->} + ) + view.$el.find(".sort-bar a[data-sort='"+new_type+"']").click() + expect($.ajax).toHaveBeenCalled() + expect(view.sortBy).toEqual(new_type) + checkThreadsOrdering(view, sort_order, new_type) + + it "with sort preference date", -> + changeSorting(@threads, "comments", "date", ["Thread1", "Thread2", "Thread3"]) + + it "with sort preference votes", -> + changeSorting(@threads, "date", "votes", ["Thread2", "Thread1", "Thread3"]) + + it "with sort preference comments", -> + changeSorting(@threads, "votes", "comments", ["Thread3", "Thread2", "Thread1"]) diff --git a/common/static/coffee/src/discussion/discussion.coffee b/common/static/coffee/src/discussion/discussion.coffee index 954dd80129..1e5fdd693b 100644 --- a/common/static/coffee/src/discussion/discussion.coffee +++ b/common/static/coffee/src/discussion/discussion.coffee @@ -5,9 +5,10 @@ if Backbone? initialize: (models, options={})-> @pages = options['pages'] || 1 @current_page = 1 + @sort_preference = options['sort'] @bind "add", (item) => item.discussion = @ - @comparator = @sortByDateRecentFirst + @setSortComparator(@sort_preference) @on "thread:remove", (thread) => @remove(thread) @@ -17,6 +18,12 @@ if Backbone? hasMorePages: -> @current_page < @pages + setSortComparator: (sortBy) -> + switch sortBy + when 'date' then @comparator = @sortByDateRecentFirst + when 'votes' then @comparator = @sortByVotes + when 'comments' then @comparator = @sortByComments + addThread: (thread, options) -> # TODO: Check for existing thread with same ID in a faster way if not @find(thread.id) diff --git a/common/static/coffee/src/discussion/main.coffee b/common/static/coffee/src/discussion/main.coffee index cb43cb9572..42fd2260ff 100644 --- a/common/static/coffee/src/discussion/main.coffee +++ b/common/static/coffee/src/discussion/main.coffee @@ -6,12 +6,13 @@ if Backbone? element = $(elem) window.$$course_id = element.data("course-id") user_info = element.data("user-info") + sort_preference = element.data("sort-preference") threads = element.data("threads") thread_pages = element.data("thread-pages") content_info = element.data("content-info") window.user = new DiscussionUser(user_info) Content.loadContentInfos(content_info) - discussion = new Discussion(threads, pages: thread_pages) + discussion = new Discussion(threads, {pages: thread_pages, sort: sort_preference}) new DiscussionRouter({discussion: discussion}) Backbone.history.start({pushState: true, root: "/courses/#{$$course_id}/discussion/forum/"}) DiscussionProfileApp = diff --git a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee index 8ef910a17e..96a6f2a419 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee @@ -132,6 +132,9 @@ if Backbone? @displayedCollection.on "reset", @renderThreads @displayedCollection.on "thread:remove", @renderThreads @renderThreads() + sort_element = @$('.sort-bar a[data-sort="' + this.collection.sort_preference + '"]') + sort_element.attr('aria-checked',true) + sort_element.addClass('active') @ renderThreads: => @@ -413,18 +416,14 @@ if Backbone? @loadMorePages(event) sortThreads: (event) -> - activeSort = @$(".sort-bar a[class='active']") + activeSort = @$(".sort-bar a.active") activeSort.removeClass("active") activeSort.attr("aria-checked", "false") newSort = $(event.target) newSort.addClass("active") newSort.attr("aria-checked", "true") @sortBy = newSort.data("sort") - - @displayedCollection.comparator = switch @sortBy - when 'date' then @displayedCollection.sortByDateRecentFirst - when 'votes' then @displayedCollection.sortByVotes - when 'comments' then @displayedCollection.sortByComments + @displayedCollection.setSortComparator(@sortBy) @retrieveFirstPage(event) performSearch: (event) -> diff --git a/common/test/acceptance/fixtures/discussion.py b/common/test/acceptance/fixtures/discussion.py index 38da67acd6..bf98134c71 100644 --- a/common/test/acceptance/fixtures/discussion.py +++ b/common/test/acceptance/fixtures/discussion.py @@ -125,4 +125,3 @@ class SearchResultFixture(DiscussionContentFixture): def get_config_data(self): return {"search_result": json.dumps(self.result)} - diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index bccc16a183..91886c0c29 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -167,6 +167,38 @@ class DiscussionThreadPage(PageObject, DiscussionPageMixin): ).fulfill() +class DiscussionSortPreferencePage(CoursePage): + """ + Page that contain the discussion board with sorting options + """ + def __init__(self, browser, course_id): + super(DiscussionSortPreferencePage, self).__init__(browser, course_id) + self.url_path = "discussion/forum" + + def is_browser_on_page(self): + """ + Return true if the browser is on the right page else false. + """ + return self.q(css="body.discussion .sort-bar").present + + def get_selected_sort_preference_text(self): + """ + Return the text of option that is selected for sorting. + """ + return self.q(css="body.discussion .sort-bar a.active").text[0].lower() + + def change_sort_preference(self, sort_by): + """ + Change the option of sorting by clicking on new option. + """ + self.q(css="body.discussion .sort-bar a[data-sort='{0}']".format(sort_by)).click() + + def refresh_page(self): + """ + Reload the page. + """ + self.browser.refresh() + class DiscussionTabSingleThreadPage(CoursePage): def __init__(self, browser, course_id, thread_id): super(DiscussionTabSingleThreadPage, self).__init__(browser, course_id) diff --git a/common/test/acceptance/tests/test_discussion.py b/common/test/acceptance/tests/test_discussion.py index f7dc37fcbb..d518a63e4b 100644 --- a/common/test/acceptance/tests/test_discussion.py +++ b/common/test/acceptance/tests/test_discussion.py @@ -12,7 +12,8 @@ from ..pages.lms.discussion import ( InlineDiscussionPage, InlineDiscussionThreadPage, DiscussionUserProfilePage, - DiscussionTabHomePage + DiscussionTabHomePage, + DiscussionSortPreferencePage, ) from ..fixtures.course import CourseFixture, XBlockFixtureDesc from ..fixtures.discussion import ( @@ -22,7 +23,7 @@ from ..fixtures.discussion import ( Thread, Response, Comment, - SearchResult + SearchResult, ) @@ -458,3 +459,52 @@ class DiscussionSearchAlertTest(UniqueCourseTest): self.setup_corrected_text(None) self.page.perform_search() self.check_search_alert_messages([]) + + +class DiscussionSortPreferenceTest(UniqueCourseTest): + """ + Tests for the discussion page displaying a single thread. + """ + + def setUp(self): + super(DiscussionSortPreferenceTest, self).setUp() + + # Create a course to register for. + CourseFixture(**self.course_info).install() + + AutoAuthPage(self.browser, course_id=self.course_id).visit() + + self.sort_page = DiscussionSortPreferencePage(self.browser, self.course_id) + self.sort_page.visit() + + def test_default_sort_preference(self): + """ + Test to check the default sorting preference of user. (Default = date ) + """ + selected_sort = self.sort_page.get_selected_sort_preference_text() + self.assertEqual(selected_sort, "date") + + def test_change_sort_preference(self): + """ + Test that if user sorting preference is changing properly. + """ + selected_sort = "" + for sort_type in ["votes", "comments", "date"]: + self.assertNotEqual(selected_sort, sort_type) + self.sort_page.change_sort_preference(sort_type) + selected_sort = self.sort_page.get_selected_sort_preference_text() + self.assertEqual(selected_sort, sort_type) + + def test_last_preference_saved(self): + """ + Test that user last preference is saved. + """ + selected_sort = "" + for sort_type in ["votes", "comments", "date"]: + self.assertNotEqual(selected_sort, sort_type) + self.sort_page.change_sort_preference(sort_type) + selected_sort = self.sort_page.get_selected_sort_preference_text() + self.assertEqual(selected_sort, sort_type) + self.sort_page.refresh_page() + selected_sort = self.sort_page.get_selected_sort_preference_text() + self.assertEqual(selected_sort, sort_type) diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 65199b5836..4ad9c72aa3 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -118,6 +118,7 @@ def make_mock_request_impl(text, thread_id="dummy_thread_id"): data = make_mock_thread_data(text, thread_id, True) elif "/users/" in url: data = { + "default_sort_key": "date", "upvoted_ids": [], "downvoted_ids": [], "subscribed_thread_ids": [], diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index d9cf8dc7d9..ce3bb42777 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -3,9 +3,9 @@ import logging import xml.sax.saxutils as saxutils from django.contrib.auth.decorators import login_required -from django.http import Http404 from django.core.context_processors import csrf from django.contrib.auth.models import User +from django.http import Http404 from django.utils.translation import ugettext as _ from django.views.decorators.http import require_GET import newrelic.agent @@ -225,7 +225,8 @@ def forum_form_discussion(request, course_id): 'cohorts': cohorts, 'user_cohort': user_cohort_id, 'cohorted_commentables': cohorted_commentables, - 'is_course_cohorted': is_course_cohorted(course_id) + 'is_course_cohorted': is_course_cohorted(course_id), + 'sort_preference': user.default_sort_key, } # print "start rendering.." return render_to_response('discussion/index.html', context) @@ -296,7 +297,6 @@ def single_thread(request, course_id, discussion_id, thread_id): cohorts = get_course_cohorts(course_id) cohorted_commentables = get_cohorted_commentables(course_id) user_cohort = get_cohort_id(request.user, course_id) - context = { 'discussion_id': discussion_id, 'csrf': csrf(request)['csrf_token'], @@ -316,9 +316,9 @@ def single_thread(request, course_id, discussion_id, thread_id): 'flag_moderator': cached_has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, 'staff', course), 'cohorts': cohorts, 'user_cohort': get_cohort_id(request.user, course_id), - 'cohorted_commentables': cohorted_commentables + 'cohorted_commentables': cohorted_commentables, + 'sort_preference': cc_user.default_sort_key, } - return render_to_response('discussion/index.html', context) @require_GET diff --git a/lms/templates/discussion/_thread_list_template.html b/lms/templates/discussion/_thread_list_template.html index c91fc9187f..67f2cd9237 100644 --- a/lms/templates/discussion/_thread_list_template.html +++ b/lms/templates/discussion/_thread_list_template.html @@ -29,7 +29,7 @@
${_("Sort by:")} diff --git a/lms/templates/discussion/index.html b/lms/templates/discussion/index.html index 87c24fe2f1..88bb49df6b 100644 --- a/lms/templates/discussion/index.html +++ b/lms/templates/discussion/index.html @@ -25,7 +25,7 @@ <%include file="_new_post.html" /> -
+