From 0e59038c720de8de1ade5a901678b60873dcfc19 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 16 Apr 2014 11:11:41 -0400 Subject: [PATCH] Add pagination to forum user profile page --- .../discussion_user_profile_view_spec.coffee | 274 ++++++++++++++++++ .../discussion/discussion_module_view.coffee | 17 +- .../static/coffee/src/discussion/main.coffee | 4 +- .../static/coffee/src/discussion/utils.coffee | 15 + .../views/discussion_user_profile_view.coffee | 61 ++-- .../django_comment_client/forum/tests.py | 4 +- .../django_comment_client/forum/views.py | 2 + lms/static/sass/_discussion.scss | 24 +- .../discussion/mustache/_pagination.mustache | 2 +- .../mustache/_user_profile.mustache | 17 +- lms/templates/discussion/user_profile.html | 8 +- 11 files changed, 362 insertions(+), 66 deletions(-) create mode 100644 common/static/coffee/spec/discussion/view/discussion_user_profile_view_spec.coffee diff --git a/common/static/coffee/spec/discussion/view/discussion_user_profile_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_user_profile_view_spec.coffee new file mode 100644 index 0000000000..d07480183b --- /dev/null +++ b/common/static/coffee/spec/discussion/view/discussion_user_profile_view_spec.coffee @@ -0,0 +1,274 @@ +describe "DiscussionUserProfileView", -> + beforeEach -> + setFixtures( + """ + + + +
+ """ + ) + window.$$course_id = "dummy_course_id" + spyOn(DiscussionThreadProfileView.prototype, "render") + + makeView = (threads, page, numPages) -> + return new DiscussionUserProfileView( + el: $(".user-profile-fixture") + collection: threads + page: page + numPages: numPages + ) + + describe "thread rendering should be correct", -> + checkRender = (numThreads) -> + threads = _.map(_.range(numThreads), (i) -> {id: i.toString(), body: "dummy body"}) + view = makeView(threads, 1, 1) + expect(view.$(".discussion").children().length).toEqual(numThreads) + _.each(threads, (thread) -> expect(view.$("#thread_#{thread.id}").length).toEqual(1)) + + it "with no threads", -> + checkRender(0) + + it "with one thread", -> + checkRender(1) + + it "with several threads", -> + checkRender(5) + + describe "pagination rendering should be correct", -> + baseUri = URI(window.location) + + pageInfo = (page) -> {url: baseUri.clone().addSearch("page", page).toString(), number: page} + + checkRender = (params) -> + view = makeView([], params.page, params.numPages) + paramsQuery = view.$(".pagination-params") + expect(paramsQuery.length).toEqual(1) + _.each( + ["page", "leftdots", "rightdots"], + (param) -> + expect(paramsQuery.data(param)).toEqual(params[param]) + ) + _.each( + ["previous", "first", "last", "next"], + (param) -> + expected = params[param] + expect(paramsQuery.find("." + param).data()).toEqual( + if expected then pageInfo(expected) else null + ) + ) + _.each( + ["lowPages", "highPages"] + (param) -> + expect(paramsQuery.find("." + param).map(-> $(this).data()).get()).toEqual( + _.map(params[param], pageInfo) + ) + ) + + it "for one page", -> + checkRender( + page: 1 + numPages: 1 + previous: null + first: null + leftdots: false + lowPages: [] + highPages: [] + rightdots: false + last: null + next: null + ) + + it "for first page of three (max with no last)", -> + checkRender( + page: 1 + numPages: 3 + previous: null + first: null + leftdots: false + lowPages: [] + highPages: [2, 3] + rightdots: false + last: null + next: 2 + ) + + it "for first page of four (has last but no dots)", -> + checkRender( + page: 1 + numPages: 4 + previous: null + first: null + leftdots: false + lowPages: [] + highPages: [2, 3] + rightdots: false + last: 4 + next: 2 + ) + + it "for first page of five (has dots)", -> + checkRender( + page: 1 + numPages: 5 + previous: null + first: null + leftdots: false + lowPages: [] + highPages: [2, 3] + rightdots: true + last: 5 + next: 2 + ) + + it "for last page of three (max with no first)", -> + checkRender( + page: 3 + numPages: 3 + previous: 2 + first: null + leftdots: false + lowPages: [1, 2] + highPages: [] + rightdots: false + last: null + next: null + ) + + it "for last page of four (has first but no dots)", -> + checkRender( + page: 4 + numPages: 4 + previous: 3 + first: 1 + leftdots: false + lowPages: [2, 3] + highPages: [] + rightdots: false + last: null + next: null + ) + + it "for last page of five (has dots)", -> + checkRender( + page: 5 + numPages: 5 + previous: 4 + first: 1 + leftdots: true + lowPages: [3, 4] + highPages: [] + rightdots: false + last: null + next: null + ) + + it "for middle page of five (max with no first/last)", -> + checkRender( + page: 3 + numPages: 5 + previous: 2 + first: null + leftdots: false + lowPages: [1, 2] + highPages: [4, 5] + rightdots: false + last: null + next: 4 + ) + + it "for middle page of seven (has first/last but no dots)", -> + checkRender( + page: 4 + numPages: 7 + previous: 3 + first: 1 + leftdots: false + lowPages: [2, 3] + highPages: [5, 6] + rightdots: false + last: 7 + next: 5 + ) + + it "for middle page of nine (has dots)", -> + checkRender( + page: 5 + numPages: 9 + previous: 4 + first: 1 + leftdots: true + lowPages: [3, 4] + highPages: [6, 7] + rightdots: true + last: 9 + next: 6 + ) + + describe "pagination interaction", -> + beforeEach -> + @view = makeView([], 1, 1) + spyOn($, "ajax") + + it "causes updated rendering", -> + $.ajax.andCallFake( + (params) => + params.success( + discussion_data: [{id: "on_page_42", body: "dummy body"}] + page: 42 + num_pages: 99 + ) + {always: ->} + ) + @view.$(".pagination a").first().click() + expect(@view.$("#thread_on_page_42").length).toEqual(1) + expect(@view.$(".pagination-params").data("page")).toEqual(42) + expect(@view.$(".pagination-params .last").data("number")).toEqual(99) + + it "handles AJAX errors", -> + spyOn(DiscussionUtil, "discussionAlert") + $.ajax.andCallFake( + (params) => + params.error() + {always: ->} + ) + @view.$(".pagination a").first().click() + expect(DiscussionUtil.discussionAlert).toHaveBeenCalled() diff --git a/common/static/coffee/src/discussion/discussion_module_view.coffee b/common/static/coffee/src/discussion/discussion_module_view.coffee index 3fe40e84b6..64d1dd9a93 100644 --- a/common/static/coffee/src/discussion/discussion_module_view.coffee +++ b/common/static/coffee/src/discussion/discussion_module_view.coffee @@ -116,7 +116,7 @@ if Backbone? @discussion.on "add", @addThread @retrieved = true @showed = true - @renderPagination(2, response.num_pages) + @renderPagination(response.num_pages) if @isWaitingOnNewPost @newPostForm.show() @@ -128,21 +128,10 @@ if Backbone? threadView.render() @threadviews.unshift threadView - renderPagination: (delta, numPages) => - minPage = Math.max(@page - delta, 1) - maxPage = Math.min(@page + delta, numPages) + renderPagination: (numPages) => pageUrl = (number) -> "?discussion_page=#{number}" - params = - page: @page - lowPages: _.range(minPage, @page).map (n) -> {number: n, url: pageUrl(n)} - highPages: _.range(@page+1, maxPage+1).map (n) -> {number: n, url: pageUrl(n)} - previous: if @page-1 >= 1 then {url: pageUrl(@page-1), number: @page-1} else false - next: if @page+1 <= numPages then {url: pageUrl(@page+1), number: @page+1} else false - leftdots: minPage > 2 - rightdots: maxPage < numPages-1 - first: if minPage > 1 then {url: pageUrl(1)} else false - last: if maxPage < numPages then {number: numPages, url: pageUrl(numPages)} else false + params = DiscussionUtil.getPaginationParams(@page, numPages, pageUrl) thing = Mustache.render @paginationTemplate(), params @$('section.pagination').html(thing) diff --git a/common/static/coffee/src/discussion/main.coffee b/common/static/coffee/src/discussion/main.coffee index c0ca520113..cb43cb9572 100644 --- a/common/static/coffee/src/discussion/main.coffee +++ b/common/static/coffee/src/discussion/main.coffee @@ -21,7 +21,9 @@ if Backbone? threads = element.data("threads") user_info = element.data("user-info") window.user = new DiscussionUser(user_info) - new DiscussionUserProfileView(el: element, collection: threads) + page = element.data("page") + numPages = element.data("num-pages") + new DiscussionUserProfileView(el: element, collection: threads, page: page, numPages: numPages) $ -> $("section.discussion").each (index, elem) -> DiscussionApp.start(elem) diff --git a/common/static/coffee/src/discussion/utils.coffee b/common/static/coffee/src/discussion/utils.coffee index 8b90f04662..6346d1230a 100644 --- a/common/static/coffee/src/discussion/utils.coffee +++ b/common/static/coffee/src/discussion/utils.coffee @@ -299,3 +299,18 @@ class @DiscussionUtil minLength++ return text.substr(0, minLength) + gettext('…') + @getPaginationParams: (curPage, numPages, pageUrlFunc) => + delta = 2 + minPage = Math.max(curPage - delta, 1) + maxPage = Math.min(curPage + delta, numPages) + pageInfo = (pageNum) -> {number: pageNum, url: pageUrlFunc(pageNum)} + params = + page: curPage + lowPages: _.range(minPage, curPage).map(pageInfo) + highPages: _.range(curPage+1, maxPage+1).map(pageInfo) + previous: if curPage > 1 then pageInfo(curPage - 1) else null + next: if curPage < numPages then pageInfo(curPage + 1) else null + leftdots: minPage > 2 + rightdots: maxPage < numPages-1 + first: if minPage > 1 then pageInfo(1) else null + last: if maxPage < numPages then pageInfo(numPages) else null diff --git a/common/static/coffee/src/discussion/views/discussion_user_profile_view.coffee b/common/static/coffee/src/discussion/views/discussion_user_profile_view.coffee index c3415fecf9..1f192e12e2 100644 --- a/common/static/coffee/src/discussion/views/discussion_user_profile_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_user_profile_view.coffee @@ -1,23 +1,44 @@ if Backbone? class @DiscussionUserProfileView extends Backbone.View -# events: -# "":"" - initialize: (options) -> - @renderThreads @$el, @collection - renderThreads: ($elem, threads) => - #Content.loadContentInfos(response.annotated_content_info) - @discussion = new Discussion() - @discussion.reset(threads, {silent: false}) - $discussion = $(Mustache.render $("script#_user_profile").html(), {'threads':threads}) - $elem.append($discussion) - @threadviews = @discussion.map (thread) -> - new DiscussionThreadProfileView el: @$("article#thread_#{thread.id}"), model: thread - _.each @threadviews, (dtv) -> dtv.render() + events: + "click .discussion-paginator a": "changePage" - addThread: (thread, collection, options) => - # TODO: When doing pagination, this will need to repaginate. Perhaps just reload page 1? - article = $("
") - @$('section.discussion > .threads').prepend(article) - threadView = new DiscussionThreadInlineView el: article, model: thread - threadView.render() - @threadviews.unshift threadView + initialize: (options) -> + super() + @page = options.page + @numPages = options.numPages + @discussion = new Discussion() + @discussion.on("reset", @render) + @discussion.reset(@collection, {silent: false}) + + render: () => + profileTemplate = $("script#_user_profile").html() + @$el.html(Mustache.render(profileTemplate, {threads: @discussion.models})) + @discussion.map (thread) -> + new DiscussionThreadProfileView(el: @$("article#thread_#{thread.id}"), model: thread).render() + baseUri = URI(window.location).removeSearch("page") + pageUrlFunc = (page) -> baseUri.clone().addSearch("page", page) + paginationParams = DiscussionUtil.getPaginationParams(@page, @numPages, pageUrlFunc) + paginationTemplate = $("script#_pagination").html() + @$el.find(".pagination").html(Mustache.render(paginationTemplate, paginationParams)) + + changePage: (event) -> + event.preventDefault() + url = $(event.target).attr("href") + DiscussionUtil.safeAjax + $elem: @$el + $loading: $(event.target) + takeFocus: true + url: url + type: "GET" + dataType: "json" + success: (response, textStatus, xhr) => + @page = response.page + @numPages = response.num_pages + @discussion.reset(response.discussion_data, {silent: false}) + history.pushState({}, "", url) + error: => + DiscussionUtil.discussionAlert( + gettext("Sorry"), + gettext("We had some trouble loading the page you requested. Please try again.") + ) diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index b4a3eb7d00..fb32c94118 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -299,8 +299,8 @@ class UserProfileTestCase(ModuleStoreTestCase): self.assertEqual(response.status_code, 200) self.assertEqual(response['Content-Type'], 'text/html; charset=utf-8') html = response.content - self.assertRegexpMatches(html, r'var \$\$course_id = \"{}\"'.format(self.course.id)) - self.assertRegexpMatches(html, r'var \$\$profiled_user_id = \"{}\"'.format(self.profiled_user.id)) + self.assertRegexpMatches(html, r'data-page="1"') + self.assertRegexpMatches(html, r'data-num-pages="1"') self.assertRegexpMatches(html, r'1 discussion started') self.assertRegexpMatches(html, r'2 comments') self.assertRegexpMatches(html, r'"id": "{}"'.format(self.TEST_THREAD_ID)) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index cbbd4b0205..4a29d8e5b9 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -353,6 +353,8 @@ def user_profile(request, course_id, user_id): 'threads': saxutils.escape(json.dumps(threads), escapedict), 'user_info': saxutils.escape(json.dumps(user_info), escapedict), 'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict), + 'page': query_params['page'], + 'num_pages': query_params['num_pages'], # 'content': content, } diff --git a/lms/static/sass/_discussion.scss b/lms/static/sass/_discussion.scss index 0b26fcac63..71b4518693 100644 --- a/lms/static/sass/_discussion.scss +++ b/lms/static/sass/_discussion.scss @@ -2226,18 +2226,18 @@ body.discussion { a { @include white-button; } - } - - li.current-page{ - height: 35px; - padding: 0 15px; - border: 1px solid #ccc; - border-radius: 3px; - font-size: 13px; - font-weight: 700; - line-height: 32px; - color: #333; - text-shadow: 0 1px 0 rgba(255, 255, 255, 0.6); + &.current-page span { + display: inline-block; + height: 35px; + padding: 0 15px; + border: 1px solid #ccc; + border-radius: 3px; + font-size: 13px; + font-weight: 700; + line-height: 32px; + color: #333; + text-shadow: 0 1px 0 rgba(255, 255, 255, 0.6); + } } } } diff --git a/lms/templates/discussion/mustache/_pagination.mustache b/lms/templates/discussion/mustache/_pagination.mustache index 71f7c52754..f984e8d38d 100644 --- a/lms/templates/discussion/mustache/_pagination.mustache +++ b/lms/templates/discussion/mustache/_pagination.mustache @@ -15,7 +15,7 @@ {{#lowPages}}
  • {{number}}
    • {{/lowPages}} -
  • {{page}}
    • +
  • {{page}}
    • {{#highPages}}
  • {{number}}
    • {{/highPages}} diff --git a/lms/templates/discussion/mustache/_user_profile.mustache b/lms/templates/discussion/mustache/_user_profile.mustache index be9f3791ca..3e0fcd516d 100644 --- a/lms/templates/discussion/mustache/_user_profile.mustache +++ b/lms/templates/discussion/mustache/_user_profile.mustache @@ -1,10 +1,9 @@ -
    -
    - {{#threads}} -
    -
    - {{/threads}} -
    -
    -
    +<%! from django.utils.translation import ugettext as _ %> + +

    ${_("Active Threads")}

    +
    + {{#threads}} +
    + {{/threads}}
    +
    diff --git a/lms/templates/discussion/user_profile.html b/lms/templates/discussion/user_profile.html index 7f5cee42d6..03b3aa71f3 100644 --- a/lms/templates/discussion/user_profile.html +++ b/lms/templates/discussion/user_profile.html @@ -32,14 +32,8 @@
    -
    -

    ${_("Active Threads")}

    -
    +
    - <%include file="_underscore_templates.html" />