From 3af90ef256523f580267ee6564bb1e238532829e Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 1 Apr 2014 16:07:59 -0400 Subject: [PATCH 1/2] Add repsonse pagination to inline discussions --- .../discussion_thread_view_inline_spec.coffee | 89 ++++++++++++++++++ .../view/discussion_thread_view_spec.coffee | 16 +--- .../view/discussion_view_spec_helper.coffee | 8 +- .../views/discussion_thread_view.coffee | 15 ++- .../discussion_thread_view_inline.coffee | 94 +++---------------- common/static/js_test.yml | 1 + lms/static/sass/_discussion.scss | 4 - .../mustache/_inline_thread.mustache | 34 +++---- .../mustache/_inline_thread_cohorted.mustache | 34 +++---- 9 files changed, 162 insertions(+), 133 deletions(-) create mode 100644 common/static/coffee/spec/discussion/view/discussion_thread_view_inline_spec.coffee diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_view_inline_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_thread_view_inline_spec.coffee new file mode 100644 index 0000000000..88304ae82b --- /dev/null +++ b/common/static/coffee/spec/discussion/view/discussion_thread_view_inline_spec.coffee @@ -0,0 +1,89 @@ +describe "DiscussionThreadInlineView", -> + beforeEach -> + setFixtures( + """ + + +
+ """ + ) + + @threadData = { + id: "dummy", + body: "dummy body", + abuse_flaggers: [], + votes: {up_count: "42"} + } + @thread = new Thread(@threadData) + @view = new DiscussionThreadInlineView({ model: @thread }) + @view.setElement($(".thread-fixture")) + spyOn($, "ajax") + # Avoid unnecessary boilerplate + spyOn(@view.showView, "render") + spyOn(@view, "makeWmdEditor") + spyOn(DiscussionThreadView.prototype, "renderResponse") + + assertContentVisible = (view, selector, visible) -> + content = view.$el.find(selector) + expect(content.length).toEqual(1) + expect(content.is(":visible")).toEqual(visible) + + assertExpandedContentVisible = (view, expanded) -> + expect(view.$el.hasClass("expanded")).toEqual(expanded) + assertContentVisible(view, ".post-extended-content", expanded) + assertContentVisible(view, ".expand-post", not expanded) + assertContentVisible(view, ".collapse-post", expanded) + + describe "render", -> + it "uses the cohorted template if cohorted", -> + @view.model.set({group_id: 1}) + @view.render() + expect(@view.$el.find(".cohorted-indicator").length).toEqual(1) + + it "uses the non-cohorted template if not cohorted", -> + @view.render() + expect(@view.$el.find(".non-cohorted-indicator").length).toEqual(1) + + it "shows content that should be visible when collapsed", -> + @view.render() + assertExpandedContentVisible(@view, false) + + it "does not render any responses by default", -> + @view.render() + expect($.ajax).not.toHaveBeenCalled() + expect(@view.$el.find(".responses li").length).toEqual(0) + + describe "expand/collapse", -> + it "shows/hides appropriate content", -> + DiscussionViewSpecHelper.setNextResponseContent({resp_total: 0, children: []}) + @view.render() + @view.expandPost() + assertExpandedContentVisible(@view, true) + @view.collapsePost() + assertExpandedContentVisible(@view, false) 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 bd251de39f..31a883cb31 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 @@ -27,16 +27,8 @@ describe "DiscussionThreadView", -> spyOn(DiscussionThreadView.prototype, "renderResponse") describe "response count and pagination", -> - - setNextResponseContent = (content) -> - $.ajax.andCallFake( - (params) => - params.success({"content": content}) - {always: ->} - ) - renderWithContent = (view, content) -> - setNextResponseContent(content) + DiscussionViewSpecHelper.setNextResponseContent(content) view.render() jasmine.Clock.tick(100) @@ -73,16 +65,16 @@ describe "DiscussionThreadView", -> assertRenderedCorrectly(@view, "5 responses", "Showing first response", "Load all responses") it "correctly re-render when all threads have loaded", -> - setNextResponseContent({resp_total: 5, children: [{}, {}, {}, {}]}) + DiscussionViewSpecHelper.setNextResponseContent({resp_total: 5, children: [{}, {}, {}, {}]}) @view.$el.find(".load-response-button").click() assertRenderedCorrectly(@view, "5 responses", "Showing all responses", null) it "correctly re-render when one page remains", -> - setNextResponseContent({resp_total: 42, children: [{}, {}]}) + DiscussionViewSpecHelper.setNextResponseContent({resp_total: 42, children: [{}, {}]}) @view.$el.find(".load-response-button").click() assertRenderedCorrectly(@view, "42 responses", "Showing first 3 responses", "Load all responses") it "correctly re-render when multiple pages remain", -> - setNextResponseContent({resp_total: 111, children: [{}, {}]}) + DiscussionViewSpecHelper.setNextResponseContent({resp_total: 111, children: [{}, {}]}) @view.$el.find(".load-response-button").click() assertRenderedCorrectly(@view, "111 responses", "Showing first 3 responses", "Load next 100 responses") diff --git a/common/static/coffee/spec/discussion/view/discussion_view_spec_helper.coffee b/common/static/coffee/spec/discussion/view/discussion_view_spec_helper.coffee index c6c69990fa..45a97f3175 100644 --- a/common/static/coffee/spec/discussion/view/discussion_view_spec_helper.coffee +++ b/common/static/coffee/spec/discussion/view/discussion_view_spec_helper.coffee @@ -110,6 +110,12 @@ class @DiscussionViewSpecHelper button.trigger($.Event("keydown", {which: 32})) expect(spy).toHaveBeenCalled() - @checkVoteButtonEvents = (view) -> @checkButtonEvents(view, "toggleVote", ".vote-btn") + + @setNextResponseContent = (content) -> + $.ajax.andCallFake( + (params) => + params.success({"content": content}) + {always: ->} + ) 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 57297a1a1e..c753d15f3c 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee @@ -22,6 +22,7 @@ if Backbone? render: -> @$el.html(@renderTemplate()) + @initLocal() @delegateEvents() @renderShowView() @@ -33,10 +34,7 @@ if Backbone? @responses.on("add", @renderResponse) # Without a delay, jQuery doesn't add the loading extension defined in # utils.coffee before safeAjax is invoked, which results in an error - setTimeout( - => @loadResponses(INITIAL_RESPONSE_PAGE_SIZE, @$el.find(".responses"), true), - 100 - ) + setTimeout((=> @loadInitialResponses()), 100) @ cleanup: -> @@ -71,6 +69,9 @@ if Backbone? gettext("We had some trouble loading more responses. Please try again.") ) + loadInitialResponses: () -> + @loadResponses(INITIAL_RESPONSE_PAGE_SIZE, @$el.find(".responses"), true) + renderResponseCountAndPagination: (responseTotal) => @$el.find(".response-count").html( interpolate( @@ -226,6 +227,9 @@ if Backbone? renderEditView: () -> @renderSubView(@editView) + getShowViewClass: () -> + return DiscussionThreadShowView + createShowView: () -> if @editView? @@ -233,7 +237,8 @@ if Backbone? @editView.$el.empty() @editView = null - @showView = new DiscussionThreadShowView(model: @model) + showViewClass = @getShowViewClass() + @showView = new showViewClass(model: @model) @showView.bind "thread:_delete", @_delete @showView.bind "thread:edit", @edit diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view_inline.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view_inline.coffee index 0282313bff..b15f361472 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view_inline.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view_inline.coffee @@ -16,7 +16,7 @@ if Backbone? @$local = @$el @$delegateElement = @$local - render: -> + renderTemplate: () -> if @model.has('group_id') @template = DiscussionUtil.getTemplate("_inline_thread_cohorted") else @@ -25,107 +25,43 @@ if Backbone? if not @model.has('abbreviatedBody') @abbreviateBody() params = @model.toJSON() - @$el.html(Mustache.render(@template, params)) - #@createShowView() + Mustache.render(@template, params) - @initLocal() - @delegateEvents() - @renderShowView() - @renderAttrs() - - @$("span.timeago").timeago() + render: () -> + super() @$el.find('.post-extended-content').hide() + @$el.find('.collapse-post').hide() + + getShowViewClass: () -> + return DiscussionThreadInlineShowView + + loadInitialResponses: () -> if @expanded - @makeWmdEditor "reply-body" - @renderAddResponseButton() - @renderResponses() - @ - createShowView: () -> - - if @editView? - @editView.undelegateEvents() - @editView.$el.empty() - @editView = null - @showView = new DiscussionThreadInlineShowView(model: @model) - @showView.bind "thread:_delete", @_delete - @showView.bind "thread:edit", @edit - - renderResponses: -> - #TODO: threadview - DiscussionUtil.safeAjax - url: "/courses/#{$$course_id}/discussion/forum/#{@model.get('commentable_id')}/threads/#{@model.id}" - $loading: @$el - success: (data, textStatus, xhr) => - # @$el.find(".loading").remove() - Content.loadContentInfos(data['annotated_content_info']) - comments = new Comments(data['content']['children']) - comments.each @renderResponse - @trigger "thread:responses:rendered" - @$('.loading').remove() - - - toggleClosed: (event) -> - #TODO: showview - $elem = $(event.target) - url = @model.urlFor('close') - closed = @model.get('closed') - data = { closed: not closed } - DiscussionUtil.safeAjax - $elem: $elem - url: url - data: data - type: "POST" - success: (response, textStatus) => - @model.set('closed', not closed) - @model.set('ability', response.ability) - - toggleEndorse: (event) -> - #TODO: showview - $elem = $(event.target) - url = @model.urlFor('endorse') - endorsed = @model.get('endorsed') - data = { endorsed: not endorsed } - DiscussionUtil.safeAjax - $elem: $elem - url: url - data: data - type: "POST" - success: (response, textStatus) => - @model.set('endorsed', not endorsed) + super() abbreviateBody: -> abbreviated = DiscussionUtil.abbreviateString @model.get('body'), 140 @model.set('abbreviatedBody', abbreviated) expandPost: (event) => - @expanded = true @$el.addClass('expanded') - @$el.find('.post-body').html(@model.get('body')) - @showView.convertMath() @$el.find('.expand-post').css('display', 'none') @$el.find('.collapse-post').css('display', 'block') @$el.find('.post-extended-content').show() - @makeWmdEditor "reply-body" - @renderAttrs() - if @$el.find('.loading').length - @renderAddResponseButton() - @renderResponses() + if not @expanded + @expanded = true + @loadInitialResponses() collapsePost: (event) -> curScroll = $(window).scrollTop() postTop = @$el.offset().top if postTop < curScroll $('html, body').animate({scrollTop: postTop}) - @expanded = false @$el.removeClass('expanded') - @$el.find('.post-body').html(@model.get('abbreviatedBody')) - @showView.convertMath() + @$el.find('.expand-post').css('display', 'block') @$el.find('.collapse-post').css('display', 'none') @$el.find('.post-extended-content').hide() - @$el.find('.expand-post').css('display', 'block') createEditView: () -> super() - @editView.bind "thread:update", @expandPost @editView.bind "thread:update", @abbreviateBody - @editView.bind "thread:cancel_edit", @expandPost diff --git a/common/static/js_test.yml b/common/static/js_test.yml index a52c7a08e8..ec4b061a7b 100644 --- a/common/static/js_test.yml +++ b/common/static/js_test.yml @@ -31,6 +31,7 @@ lib_paths: - js/vendor/jquery.min.js - js/vendor/jasmine-jquery.js - js/vendor/jasmine-imagediff.js + - js/vendor/mustache.js - js/vendor/underscore-min.js - js/vendor/backbone-min.js - js/vendor/jquery.timeago.js diff --git a/lms/static/sass/_discussion.scss b/lms/static/sass/_discussion.scss index ea5d53d9b6..0b26fcac63 100644 --- a/lms/static/sass/_discussion.scss +++ b/lms/static/sass/_discussion.scss @@ -2069,10 +2069,6 @@ body.discussion { font-size: 12px; line-height: 30px; - &.collapse-post { - display: none; - } - .icon { color: $link-color; margin-right: ($baseline*0.25); diff --git a/lms/templates/discussion/mustache/_inline_thread.mustache b/lms/templates/discussion/mustache/_inline_thread.mustache index 4802e2795a..8345b0cfec 100644 --- a/lms/templates/discussion/mustache/_inline_thread.mustache +++ b/lms/templates/discussion/mustache/_inline_thread.mustache @@ -3,23 +3,25 @@
-
- +
+
+
+ +
+
    +
    +
    +

    ${_("Post a response:")}

    +
      +
      +
      + ${_("Submit")} +
      +
      -
        -
      1. ${_("Loading content")}
        2. -
      -
      -

      ${_("Post a response:")}

      -
        -
        -
        - ${_("Submit")} -
        -
      diff --git a/lms/templates/discussion/mustache/_inline_thread_cohorted.mustache b/lms/templates/discussion/mustache/_inline_thread_cohorted.mustache index e33a1f9926..4c6fac4cd9 100644 --- a/lms/templates/discussion/mustache/_inline_thread_cohorted.mustache +++ b/lms/templates/discussion/mustache/_inline_thread_cohorted.mustache @@ -4,23 +4,25 @@
      {{group_string}}
      -
      - +
      +
      +
      + +
      +
        +
        +
        +

        ${_("Post a response:")}

        +
          +
          +
          + ${_("Submit")} +
          +
          -
            -
          1. ${_("Loading content")}
            2. -
          -
          -

          ${_("Post a response:")}

          -
            -
            -
            - ${_("Submit")} -
            -
          From 16847d85a91e7c706b8a7f407a9dcb945e1758f4 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 4 Apr 2014 15:46:30 -0400 Subject: [PATCH 2/2] Add bok-choy tests for inline response pagination --- common/djangoapps/terrain/stubs/comments.py | 12 + .../test/acceptance/pages/lms/courseware.py | 16 ++ ...cussion_single_thread.py => discussion.py} | 131 +++++++--- .../test/acceptance/tests/test_discussion.py | 239 ++++++++++++------ 4 files changed, 286 insertions(+), 112 deletions(-) create mode 100644 common/test/acceptance/pages/lms/courseware.py rename common/test/acceptance/pages/lms/{discussion_single_thread.py => discussion.py} (58%) diff --git a/common/djangoapps/terrain/stubs/comments.py b/common/djangoapps/terrain/stubs/comments.py index 28f3729b6e..eaca957935 100644 --- a/common/djangoapps/terrain/stubs/comments.py +++ b/common/djangoapps/terrain/stubs/comments.py @@ -14,6 +14,7 @@ class StubCommentsServiceHandler(StubHttpRequestHandler): "/api/v1/threads$": self.do_threads, "/api/v1/threads/(?P\\w+)$": self.do_thread, "/api/v1/comments/(?P\\w+)$": self.do_comment, + "/api/v1/(?P\\w+)/threads$": self.do_commentable, } path = urlparse.urlparse(self.path).path for pattern in pattern_handlers: @@ -63,6 +64,17 @@ class StubCommentsServiceHandler(StubHttpRequestHandler): comment = self.server.config['comments'][comment_id] self.send_json_response(comment) + def do_commentable(self, commentable_id): + self.send_json_response({ + "collection": [ + thread + for thread in self.server.config.get('threads', {}).values() + if thread.get('commentable_id') == commentable_id + ], + "page": 1, + "num_pages": 1, + }) + class StubCommentsService(StubHttpService): HANDLER_CLASS = StubCommentsServiceHandler diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py new file mode 100644 index 0000000000..44e5b8d594 --- /dev/null +++ b/common/test/acceptance/pages/lms/courseware.py @@ -0,0 +1,16 @@ +""" +Courseware page. +""" + +from .course_page import CoursePage + + +class CoursewarePage(CoursePage): + """ + Course info. + """ + + url_path = "courseware" + + def is_browser_on_page(self): + return self.q(css='body.courseware').present diff --git a/common/test/acceptance/pages/lms/discussion_single_thread.py b/common/test/acceptance/pages/lms/discussion.py similarity index 58% rename from common/test/acceptance/pages/lms/discussion_single_thread.py rename to common/test/acceptance/pages/lms/discussion.py index 7ca23de40c..5652ccfcca 100644 --- a/common/test/acceptance/pages/lms/discussion_single_thread.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -1,39 +1,45 @@ -from bok_choy.page_object import unguarded +from bok_choy.page_object import PageObject from bok_choy.promise import EmptyPromise from .course_page import CoursePage -class DiscussionSingleThreadPage(CoursePage): - def __init__(self, browser, course_id, thread_id): - super(DiscussionSingleThreadPage, self).__init__(browser, course_id) - self.thread_id = thread_id +class DiscussionThreadPage(PageObject): + url = None + + def __init__(self, browser, thread_selector): + super(DiscussionThreadPage, self).__init__(browser) + self.thread_selector = thread_selector + + def _find_within(self, selector): + """ + Returns a query corresponding to the given CSS selector within the scope + of this thread page + """ + return self.q(css=self.thread_selector + " " + selector) def is_browser_on_page(self): - return self.q( - css="body.discussion .discussion-article[data-id='{thread_id}']".format(thread_id=self.thread_id) - ).present - - @property - @unguarded - def url_path(self): - return "discussion/forum/dummy/threads/" + self.thread_id + return self.q(css=self.thread_selector).present def _get_element_text(self, selector): """ Returns the text of the first element matching the given selector, or None if no such element exists """ - text_list = self.q(css=selector).text + text_list = self._find_within(selector).text return text_list[0] if text_list else None + def _is_element_visible(self, selector): + query = self._find_within(selector) + return query.present and query.visible + def get_response_total_text(self): """Returns the response count text, or None if not present""" return self._get_element_text(".response-count") def get_num_displayed_responses(self): """Returns the number of responses actually rendered""" - return len(self.q(css=".discussion-response").results) + return len(self._find_within(".discussion-response")) def get_shown_responses_text(self): """Returns the shown response count text, or None if not present""" @@ -45,7 +51,7 @@ class DiscussionSingleThreadPage(CoursePage): def load_more_responses(self): """Clicks the load more responses button and waits for responses to load""" - self.q(css=".load-response-button").first.click() + self._find_within(".load-response-button").click() def _is_ajax_finished(): return self.browser.execute_script("return jQuery.active") == 0 @@ -64,25 +70,19 @@ class DiscussionSingleThreadPage(CoursePage): Clicks the add response button and ensures that the response text field receives focus """ - self.q(css=".add-response-btn").first.click() + self._find_within(".add-response-btn").first.click() EmptyPromise( - lambda: self.q(css="#wmd-input-reply-body-{thread_id}:focus".format(thread_id=self.thread_id)), + lambda: self._find_within(".discussion-reply-new textarea:focus").present, "Response field received focus" ).fulfill() - def _is_element_visible(self, selector): - return ( - self.q(css=selector).present and - self.q(css=selector).visible - ) - def is_response_editor_visible(self, response_id): """Returns true if the response editor is present, false otherwise""" return self._is_element_visible(".response_{} .edit-post-body".format(response_id)) def start_response_edit(self, response_id): """Click the edit button for the response, loading the editing view""" - self.q(css=".response_{} .discussion-response .action-edit".format(response_id)).first.click() + self._find_within(".response_{} .discussion-response .action-edit".format(response_id)).first.click() EmptyPromise( lambda: self.is_response_editor_visible(response_id), "Response edit started" @@ -105,7 +105,7 @@ class DiscussionSingleThreadPage(CoursePage): def delete_comment(self, comment_id): with self.handle_alert(): - self.q(css="#comment_{} div.action-delete".format(comment_id)).first.click() + self._find_within("#comment_{} div.action-delete".format(comment_id)).first.click() EmptyPromise( lambda: not self.is_comment_visible(comment_id), "Deleted comment was removed" @@ -120,12 +120,12 @@ class DiscussionSingleThreadPage(CoursePage): return self._is_element_visible(".edit-comment-body[data-id='{}']".format(comment_id)) def _get_comment_editor_value(self, comment_id): - return self.q(css="#wmd-input-edit-comment-body-{}".format(comment_id)).text[0] + return self._find_within("#wmd-input-edit-comment-body-{}".format(comment_id)).text[0] def start_comment_edit(self, comment_id): """Click the edit button for the comment, loading the editing view""" old_body = self.get_comment_body(comment_id) - self.q(css="#comment_{} .action-edit".format(comment_id)).first.click() + self._find_within("#comment_{} .action-edit".format(comment_id)).first.click() EmptyPromise( lambda: ( self.is_comment_editor_visible(comment_id) and @@ -137,11 +137,11 @@ class DiscussionSingleThreadPage(CoursePage): def set_comment_editor_value(self, comment_id, new_body): """Replace the contents of the comment editor""" - self.q(css="#comment_{} .wmd-input".format(comment_id)).fill(new_body) + self._find_within("#comment_{} .wmd-input".format(comment_id)).fill(new_body) def submit_comment_edit(self, comment_id, new_comment_body): """Click the submit button on the comment editor""" - self.q(css="#comment_{} .post-update".format(comment_id)).first.click() + self._find_within("#comment_{} .post-update".format(comment_id)).first.click() EmptyPromise( lambda: ( not self.is_comment_editor_visible(comment_id) and @@ -153,7 +153,7 @@ class DiscussionSingleThreadPage(CoursePage): def cancel_comment_edit(self, comment_id, original_body): """Click the cancel button on the comment editor""" - self.q(css="#comment_{} .post-cancel".format(comment_id)).first.click() + self._find_within("#comment_{} .post-cancel".format(comment_id)).first.click() EmptyPromise( lambda: ( not self.is_comment_editor_visible(comment_id) and @@ -162,3 +162,72 @@ class DiscussionSingleThreadPage(CoursePage): ), "Comment edit was canceled" ).fulfill() + + +class DiscussionTabSingleThreadPage(CoursePage): + def __init__(self, browser, course_id, thread_id): + super(DiscussionTabSingleThreadPage, self).__init__(browser, course_id) + self.thread_page = DiscussionThreadPage( + browser, + "body.discussion .discussion-article[data-id='{thread_id}']".format(thread_id=thread_id) + ) + self.url_path = "discussion/forum/dummy/threads/" + thread_id + + def is_browser_on_page(self): + return self.thread_page.is_browser_on_page() + + def __getattr__(self, name): + return getattr(self.thread_page, name) + + +class InlineDiscussionPage(PageObject): + url = None + + def __init__(self, browser, discussion_id): + super(InlineDiscussionPage, self).__init__(browser) + self._discussion_selector = ( + "body.courseware .discussion-module[data-discussion-id='{discussion_id}'] ".format( + discussion_id=discussion_id + ) + ) + + def _find_within(self, selector): + """ + Returns a query corresponding to the given CSS selector within the scope + of this discussion page + """ + return self.q(css=self._discussion_selector + " " + selector) + + def is_browser_on_page(self): + return self.q(css=self._discussion_selector).present + + def is_discussion_expanded(self): + return self._find_within(".discussion").present + + def expand_discussion(self): + """Click the link to expand the discussion""" + self._find_within(".discussion-show").first.click() + EmptyPromise( + self.is_discussion_expanded, + "Discussion expanded" + ).fulfill() + + def get_num_displayed_threads(self): + return len(self._find_within(".discussion-thread")) + + +class InlineDiscussionThreadPage(DiscussionThreadPage): + def __init__(self, browser, thread_id): + super(InlineDiscussionThreadPage, self).__init__( + browser, + "body.courseware .discussion-module #thread_{thread_id}".format(thread_id=thread_id) + ) + + def expand(self): + """Clicks the link to expand the thread""" + self._find_within(".expand-post").first.click() + EmptyPromise( + lambda: bool(self.get_response_total_text()), + "Thread expanded" + ).fulfill() + diff --git a/common/test/acceptance/tests/test_discussion.py b/common/test/acceptance/tests/test_discussion.py index f52029dfd2..cad4779bb2 100644 --- a/common/test/acceptance/tests/test_discussion.py +++ b/common/test/acceptance/tests/test_discussion.py @@ -2,96 +2,132 @@ Tests for discussion pages """ +from uuid import uuid4 + from .helpers import UniqueCourseTest from ..pages.studio.auto_auth import AutoAuthPage -from ..pages.lms.discussion_single_thread import DiscussionSingleThreadPage -from ..fixtures.course import CourseFixture +from ..pages.lms.courseware import CoursewarePage +from ..pages.lms.discussion import ( + DiscussionTabSingleThreadPage, + InlineDiscussionPage, + InlineDiscussionThreadPage +) +from ..fixtures.course import CourseFixture, XBlockFixtureDesc from ..fixtures.discussion import SingleThreadViewFixture, Thread, Response, Comment -class DiscussionSingleThreadTest(UniqueCourseTest): +class DiscussionResponsePaginationTestMixin(object): + """ + A mixin containing tests for response pagination for use by both inline + discussion and the discussion tab + """ + + def setup_thread(self, num_responses, **thread_kwargs): + """ + Create a test thread with the given number of responses, passing all + keyword arguments through to the Thread fixture, then invoke + setup_thread_page. + """ + thread_id = "test_thread_{}".format(uuid4().hex) + thread_fixture = SingleThreadViewFixture( + Thread(id=thread_id, commentable_id=self.discussion_id, **thread_kwargs) + ) + for i in range(num_responses): + thread_fixture.addResponse(Response(id=str(i), body=str(i))) + thread_fixture.push() + self.setup_thread_page(thread_id) + + def assert_response_display_correct(self, response_total, displayed_responses): + """ + Assert that various aspects of the display of responses are all correct: + * Text indicating total number of responses + * Presence of "Add a response" button + * Number of responses actually displayed + * Presence and text of indicator of how many responses are shown + * Presence and text of button to load more responses + """ + self.assertEqual( + self.thread_page.get_response_total_text(), + str(response_total) + " responses" + ) + self.assertEqual(self.thread_page.has_add_response_button(), response_total != 0) + self.assertEqual(self.thread_page.get_num_displayed_responses(), displayed_responses) + self.assertEqual( + self.thread_page.get_shown_responses_text(), + ( + None if response_total == 0 else + "Showing all responses" if response_total == displayed_responses else + "Showing first {} responses".format(displayed_responses) + ) + ) + self.assertEqual( + self.thread_page.get_load_responses_button_text(), + ( + None if response_total == displayed_responses else + "Load all responses" if response_total - displayed_responses < 100 else + "Load next 100 responses" + ) + ) + + def test_pagination_no_responses(self): + self.setup_thread(0) + self.assert_response_display_correct(0, 0) + + def test_pagination_few_responses(self): + self.setup_thread(5) + self.assert_response_display_correct(5, 5) + + def test_pagination_two_response_pages(self): + self.setup_thread(50) + self.assert_response_display_correct(50, 25) + + self.thread_page.load_more_responses() + self.assert_response_display_correct(50, 50) + + def test_pagination_exactly_two_response_pages(self): + self.setup_thread(125) + self.assert_response_display_correct(125, 25) + + self.thread_page.load_more_responses() + self.assert_response_display_correct(125, 125) + + def test_pagination_three_response_pages(self): + self.setup_thread(150) + self.assert_response_display_correct(150, 25) + + self.thread_page.load_more_responses() + self.assert_response_display_correct(150, 125) + + self.thread_page.load_more_responses() + self.assert_response_display_correct(150, 150) + + def test_add_response_button(self): + self.setup_thread(5) + self.assertTrue(self.thread_page.has_add_response_button()) + self.thread_page.click_add_response_button() + + def test_add_response_button_closed_thread(self): + self.setup_thread(5, closed=True) + self.assertFalse(self.thread_page.has_add_response_button()) + + +class DiscussionTabSingleThreadTest(UniqueCourseTest, DiscussionResponsePaginationTestMixin): """ Tests for the discussion page displaying a single thread """ def setUp(self): - super(DiscussionSingleThreadTest, self).setUp() + super(DiscussionTabSingleThreadTest, self).setUp() + self.discussion_id = "test_discussion_{}".format(uuid4().hex) # Create a course to register for CourseFixture(**self.course_info).install() - self.user_id = AutoAuthPage(self.browser, course_id=self.course_id).visit().get_user_id() + AutoAuthPage(self.browser, course_id=self.course_id).visit() - def setup_thread(self, thread, num_responses): - view = SingleThreadViewFixture(thread=thread) - for i in range(num_responses): - view.addResponse(Response(id=str(i), body=str(i))) - view.push() - - def test_no_responses(self): - self.setup_thread(Thread(id="0_responses"), 0) - page = DiscussionSingleThreadPage(self.browser, self.course_id, "0_responses") - page.visit() - self.assertEqual(page.get_response_total_text(), "0 responses") - self.assertFalse(page.has_add_response_button()) - self.assertEqual(page.get_num_displayed_responses(), 0) - self.assertEqual(page.get_shown_responses_text(), None) - self.assertIsNone(page.get_load_responses_button_text()) - - def test_few_responses(self): - self.setup_thread(Thread(id="5_responses"), 5) - page = DiscussionSingleThreadPage(self.browser, self.course_id, "5_responses") - page.visit() - self.assertEqual(page.get_response_total_text(), "5 responses") - self.assertEqual(page.get_num_displayed_responses(), 5) - self.assertEqual(page.get_shown_responses_text(), "Showing all responses") - self.assertIsNone(page.get_load_responses_button_text()) - - def test_two_response_pages(self): - self.setup_thread(Thread(id="50_responses"), 50) - page = DiscussionSingleThreadPage(self.browser, self.course_id, "50_responses") - page.visit() - self.assertEqual(page.get_response_total_text(), "50 responses") - self.assertEqual(page.get_num_displayed_responses(), 25) - self.assertEqual(page.get_shown_responses_text(), "Showing first 25 responses") - self.assertEqual(page.get_load_responses_button_text(), "Load all responses") - - page.load_more_responses() - self.assertEqual(page.get_num_displayed_responses(), 50) - self.assertEqual(page.get_shown_responses_text(), "Showing all responses") - self.assertEqual(page.get_load_responses_button_text(), None) - - def test_three_response_pages(self): - self.setup_thread(Thread(id="150_responses"), 150) - page = DiscussionSingleThreadPage(self.browser, self.course_id, "150_responses") - page.visit() - self.assertEqual(page.get_response_total_text(), "150 responses") - self.assertEqual(page.get_num_displayed_responses(), 25) - self.assertEqual(page.get_shown_responses_text(), "Showing first 25 responses") - self.assertEqual(page.get_load_responses_button_text(), "Load next 100 responses") - - page.load_more_responses() - self.assertEqual(page.get_num_displayed_responses(), 125) - self.assertEqual(page.get_shown_responses_text(), "Showing first 125 responses") - self.assertEqual(page.get_load_responses_button_text(), "Load all responses") - - page.load_more_responses() - self.assertEqual(page.get_num_displayed_responses(), 150) - self.assertEqual(page.get_shown_responses_text(), "Showing all responses") - self.assertEqual(page.get_load_responses_button_text(), None) - - def test_add_response_button(self): - self.setup_thread(Thread(id="5_responses"), 5) - page = DiscussionSingleThreadPage(self.browser, self.course_id, "5_responses") - page.visit() - self.assertTrue(page.has_add_response_button()) - page.click_add_response_button() - - def test_add_response_button_closed_thread(self): - self.setup_thread(Thread(id="5_responses_closed", closed=True), 5) - page = DiscussionSingleThreadPage(self.browser, self.course_id, "5_responses_closed") - page.visit() - self.assertFalse(page.has_add_response_button()) + def setup_thread_page(self, thread_id): + self.thread_page = DiscussionTabSingleThreadPage(self.browser, self.course_id, thread_id) # pylint:disable=W0201 + self.thread_page.visit() class DiscussionCommentDeletionTest(UniqueCourseTest): @@ -119,7 +155,7 @@ class DiscussionCommentDeletionTest(UniqueCourseTest): def test_comment_deletion_as_student(self): self.setup_user() self.setup_view() - page = DiscussionSingleThreadPage(self.browser, self.course_id, "comment_deletion_test_thread") + page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_deletion_test_thread") page.visit() self.assertTrue(page.is_comment_deletable("comment_self_author")) self.assertTrue(page.is_comment_visible("comment_other_author")) @@ -129,7 +165,7 @@ class DiscussionCommentDeletionTest(UniqueCourseTest): def test_comment_deletion_as_moderator(self): self.setup_user(roles=['Moderator']) self.setup_view() - page = DiscussionSingleThreadPage(self.browser, self.course_id, "comment_deletion_test_thread") + page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_deletion_test_thread") page.visit() self.assertTrue(page.is_comment_deletable("comment_self_author")) self.assertTrue(page.is_comment_deletable("comment_other_author")) @@ -168,7 +204,7 @@ class DiscussionCommentEditTest(UniqueCourseTest): def test_edit_comment_as_student(self): self.setup_user() self.setup_view() - page = DiscussionSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread") + page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread") page.visit() self.assertTrue(page.is_comment_editable("comment_self_author")) self.assertTrue(page.is_comment_visible("comment_other_author")) @@ -178,7 +214,7 @@ class DiscussionCommentEditTest(UniqueCourseTest): def test_edit_comment_as_moderator(self): self.setup_user(roles=["Moderator"]) self.setup_view() - page = DiscussionSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread") + page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread") page.visit() self.assertTrue(page.is_comment_editable("comment_self_author")) self.assertTrue(page.is_comment_editable("comment_other_author")) @@ -188,7 +224,7 @@ class DiscussionCommentEditTest(UniqueCourseTest): def test_cancel_comment_edit(self): self.setup_user() self.setup_view() - page = DiscussionSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread") + page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread") page.visit() self.assertTrue(page.is_comment_editable("comment_self_author")) original_body = page.get_comment_body("comment_self_author") @@ -200,7 +236,7 @@ class DiscussionCommentEditTest(UniqueCourseTest): """Only one editor should be visible at a time within a single response""" self.setup_user(roles=["Moderator"]) self.setup_view() - page = DiscussionSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread") + page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread") page.visit() self.assertTrue(page.is_comment_editable("comment_self_author")) self.assertTrue(page.is_comment_editable("comment_other_author")) @@ -224,3 +260,44 @@ class DiscussionCommentEditTest(UniqueCourseTest): page.cancel_comment_edit("comment_self_author", original_body) self.assertFalse(page.is_comment_editor_visible("comment_self_author")) self.assertTrue(page.is_add_comment_visible("response1")) + + +class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMixin): + """ + Tests for inline discussions + """ + + def setUp(self): + super(InlineDiscussionTest, self).setUp() + self.discussion_id = "test_discussion_{}".format(uuid4().hex) + CourseFixture(**self.course_info).add_children( + XBlockFixtureDesc("chapter", "Test Section").add_children( + XBlockFixtureDesc("sequential", "Test Subsection").add_children( + XBlockFixtureDesc("vertical", "Test Unit").add_children( + XBlockFixtureDesc( + "discussion", + "Test Discussion", + metadata={"discussion_id": self.discussion_id} + ) + ) + ) + ) + ).install() + + AutoAuthPage(self.browser, course_id=self.course_id).visit() + + CoursewarePage(self.browser, self.course_id).visit() + self.discussion_page = InlineDiscussionPage(self.browser, self.discussion_id) + + def setup_thread_page(self, thread_id): + self.discussion_page.expand_discussion() + self.assertEqual(self.discussion_page.get_num_displayed_threads(), 1) + self.thread_page = InlineDiscussionThreadPage(self.browser, thread_id) # pylint:disable=W0201 + self.thread_page.expand() + + def test_initial_render(self): + self.assertFalse(self.discussion_page.is_discussion_expanded()) + + def test_expand_discussion_empty(self): + self.discussion_page.expand_discussion() + self.assertEqual(self.discussion_page.get_num_displayed_threads(), 0)