Merge pull request #3986 from edx/jsa/search-spell-correction

Add support for search spell corrections to Forums UX.
2014-06-05 15:14:00 -04:00
parent da6c337702 58c5066e66
commit f5cfd1ef96
15 changed files with 416 additions and 531 deletions
--- a/common/djangoapps/terrain/stubs/comments.py
+++ b/common/djangoapps/terrain/stubs/comments.py
@@ -17,6 +17,7 @@ class StubCommentsServiceHandler(StubHttpRequestHandler):
        pattern_handlers = {
            "/api/v1/users/(?P<user_id>\\d+)/active_threads$": self.do_user_profile,
            "/api/v1/users/(?P<user_id>\\d+)$": self.do_user,
+            "/api/v1/search/threads$": self.do_search_threads,
            "/api/v1/threads$": self.do_threads,
            "/api/v1/threads/(?P<thread_id>\\w+)$": self.do_thread,
            "/api/v1/comments/(?P<comment_id>\\w+)$": self.do_comment,
@@ -86,6 +87,9 @@ class StubCommentsServiceHandler(StubHttpRequestHandler):
    def do_threads(self):
        self.send_json_response({"collection": [], "page": 1, "num_pages": 1})

+    def do_search_threads(self):
+        self.send_json_response(self.server.config.get('search_result', {}))
+
    def do_comment(self, comment_id):
        # django_comment_client calls GET comment before doing a DELETE, so that's what this is here to support.
        if comment_id in self.server.config.get('comments', {}):
--- 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
@@ -0,0 +1,100 @@
+describe "DiscussionThreadListView", ->
+
+    beforeEach ->
+
+        setFixtures """
+        <script type="text/template" id="thread-list-template">
+            <div class="browse-search">
+                <div class="home"></div>
+                <div class="browse is-open"></div>
+                <div class="search">
+                    <form class="post-search">
+                        <label class="sr" for="search-discussions">Search</label>
+                        <input type="text" id="search-discussions" placeholder="Search all discussions" class="post-search-field">
+                    </form>
+                </div>
+            </div>
+            <div class="sort-bar"></div>
+            <div class="search-alerts"></div>
+            <div class="post-list-wrapper">
+                <ul class="post-list"></ul>
+            </div>
+        </script>
+        <script aria-hidden="true" type="text/template" id="search-alert-template">
+            <div class="search-alert" id="search-alert-<%- cid %>">
+                <div class="search-alert-content">
+                  <p class="message"><%- message %></p>
+                </div>
+
+                <div class="search-alert-controls">
+                  <a href="#" class="dismiss control control-dismiss"><i class="icon icon-remove"></i></a>
+                </div>
+            </div>
+        </script>
+        <div class="sidebar"></div>
+        """
+        window.$$course_id = "TestOrg/TestCourse/TestRun"
+        window.user = new DiscussionUser({id: "567", upvoted_ids: []})
+
+        spyOn($, "ajax")
+
+        @discussion = new Discussion([])
+        @view = new DiscussionThreadListView({collection: @discussion, el: $(".sidebar")})
+        @view.render()
+
+    testAlertMessages = (expectedMessages) ->
+        expect($(".search-alert .message").map( ->
+          $(@).html()
+        ).get()).toEqual(expectedMessages)
+
+    it "renders and removes search alerts", ->
+        testAlertMessages []
+        foo = @view.addSearchAlert("foo")
+        testAlertMessages ["foo"]
+        bar = @view.addSearchAlert("bar")
+        testAlertMessages ["foo", "bar"]
+        @view.removeSearchAlert(foo.cid)
+        testAlertMessages ["bar"]
+        @view.removeSearchAlert(bar.cid)
+        testAlertMessages []
+
+    it "clears all search alerts", ->
+        @view.addSearchAlert("foo")
+        @view.addSearchAlert("bar")
+        @view.addSearchAlert("baz")
+        testAlertMessages ["foo", "bar", "baz"]
+        @view.clearSearchAlerts()
+        testAlertMessages []
+
+    testCorrection = (view, correctedText) ->
+        spyOn(view, "addSearchAlert")
+        $.ajax.andCallFake(
+            (params) =>
+                params.success(
+                    {discussion_data: [], page: 42, num_pages: 99, corrected_text: correctedText}, 'success'
+                )
+                {always: ->}
+        )
+        view.searchFor("dummy")
+        expect($.ajax).toHaveBeenCalled()
+
+    it "adds a search alert when an alternate term was searched", ->
+        testCorrection(@view, "foo")
+        expect(@view.addSearchAlert).toHaveBeenCalled()
+        expect(@view.addSearchAlert.mostRecentCall.args[0]).toMatch(/foo/)
+
+    it "does not add a search alert when no alternate term was searched", ->
+        testCorrection(@view, null)
+        expect(@view.addSearchAlert).not.toHaveBeenCalled()
+
+    it "clears search alerts when a new search is performed", ->
+        spyOn(@view, "clearSearchAlerts")
+        spyOn(DiscussionUtil, "safeAjax")
+        @view.searchFor("dummy")
+        expect(@view.clearSearchAlerts).toHaveBeenCalled()
+
+    it "clears search alerts when the underlying collection changes", ->
+        spyOn(@view, "clearSearchAlerts")
+        spyOn(@view, "renderThread")
+        @view.collection.trigger("change", new Thread({id: 1}))
+        expect(@view.clearSearchAlerts).toHaveBeenCalled()
--- a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee
+++ b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee
@@ -36,7 +36,36 @@ if Backbone?
      @current_search = ""
      @mode = 'all'

+      @searchAlertCollection = new Backbone.Collection([], {model: Backbone.Model})
+
+      @searchAlertCollection.on "add", (searchAlert) =>
+        content = _.template(
+          $("#search-alert-template").html(),
+          {'message': searchAlert.attributes.message, 'cid': searchAlert.cid}
+          )
+        @$(".search-alerts").append(content)
+        @$("#search-alert-" + searchAlert.cid + " a.dismiss").bind "click", searchAlert, (event) =>
+          @removeSearchAlert(event.data.cid)
+
+      @searchAlertCollection.on "remove", (searchAlert) =>
+        @$("#search-alert-" + searchAlert.cid).remove()
+
+      @searchAlertCollection.on "reset", =>
+        @$(".search-alerts").empty()
+
+    addSearchAlert: (message) =>
+      m = new Backbone.Model({"message": message})
+      @searchAlertCollection.add(m)
+      m
+
+    removeSearchAlert: (searchAlert) =>
+      @searchAlertCollection.remove(searchAlert)
+
+    clearSearchAlerts: =>
+      @searchAlertCollection.reset()
+
    reloadDisplayedCollection: (thread) =>
+      @clearSearchAlerts()
      thread_id = thread.get('id')
      content = @renderThread(thread)
      current_el = @$("a[data-id=#{thread_id}]")
@@ -405,6 +434,7 @@ if Backbone?
        @searchFor(text)

    searchFor: (text, callback, value) ->
+      @clearSearchAlerts()
      @mode = 'search'
      @current_search = text
      url = DiscussionUtil.urlFor("search")
@@ -429,6 +459,8 @@ if Backbone?
            Content.loadContentInfos(response.annotated_content_info)
            @collection.current_page = response.page
            @collection.pages = response.num_pages
+            if !_.isNull response.corrected_text
+              @addSearchAlert('Showing results for "' + response.corrected_text + '"');
            # TODO: Perhaps reload user info so that votes can be updated.
            # In the future we might not load all of a user's votes at once
            # so this would probably be necessary anyway
--- a/common/test/acceptance/fixtures/discussion.py
+++ b/common/test/acceptance/fixtures/discussion.py
@@ -39,7 +39,6 @@ class Thread(ContentFactory):
    pinned = False
    read = False

-
 class Comment(ContentFactory):
    thread_id = None
    depth = 0
@@ -52,7 +51,34 @@ class Response(Comment):
    body = "dummy response body"


-class SingleThreadViewFixture(object):
+class SearchResult(factory.Factory):
+    FACTORY_FOR = dict
+    discussion_data = []
+    annotated_content_info = {}
+    num_pages = 1
+    page = 1
+    corrected_text = None
+
+
+class DiscussionContentFixture(object):
+
+    def push(self):
+        """
+        Push the data to the stub comments service.
+        """
+        requests.put(
+            '{}/set_config'.format(COMMENTS_STUB_URL),
+            data=self.get_config_data()
+        )
+
+    def get_config_data(self):
+        """
+        return a dictionary with the fixture's data serialized for PUTting to the stub server's config endpoint.
+        """
+        raise NotImplementedError()
+
+
+class SingleThreadViewFixture(DiscussionContentFixture):

    def __init__(self, thread):
        self.thread = thread
@@ -76,30 +102,27 @@ class SingleThreadViewFixture(object):
            return res
        return dict(_visit(self.thread))

-    def push(self):
-        """
-        Push the data to the stub comments service.
-        """
-        requests.put(
-            '{}/set_config'.format(COMMENTS_STUB_URL),
-            data={
-                "threads": json.dumps({self.thread['id']: self.thread}),
-                "comments": json.dumps(self._get_comment_map())
-            }
-        )
+    def get_config_data(self):
+        return {
+            "threads": json.dumps({self.thread['id']: self.thread}),
+            "comments": json.dumps(self._get_comment_map())
+        }

-class UserProfileViewFixture(object):
+
+class UserProfileViewFixture(DiscussionContentFixture):

    def __init__(self, threads):
        self.threads = threads

-    def push(self):
-        """
-        Push the data to the stub comments service.
-        """
-        requests.put(
-            '{}/set_config'.format(COMMENTS_STUB_URL),
-            data={
-                "active_threads": json.dumps(self.threads),
-            }
-        )
+    def get_config_data(self):
+        return {"active_threads": json.dumps(self.threads)}
+
+
+class SearchResultFixture(DiscussionContentFixture):
+
+    def __init__(self, result):
+        self.result = result
+
+    def get_config_data(self):
+        return {"search_result": json.dumps(self.result)}
+
--- a/common/test/acceptance/pages/lms/discussion.py
+++ b/common/test/acceptance/pages/lms/discussion.py
@@ -4,7 +4,13 @@ from bok_choy.promise import EmptyPromise
 from .course_page import CoursePage


-class DiscussionThreadPage(PageObject):
+class DiscussionPageMixin(object):
+
+    def is_ajax_finished(self):
+        return self.browser.execute_script("return jQuery.active") == 0
+
+
+class DiscussionThreadPage(PageObject, DiscussionPageMixin):
    url = None

    def __init__(self, browser, thread_selector):
@@ -53,11 +59,8 @@ class DiscussionThreadPage(PageObject):
        """Clicks the load more responses button and waits for responses to load"""
        self._find_within(".load-response-button").click()

-        def _is_ajax_finished():
-            return self.browser.execute_script("return jQuery.active") == 0
-
        EmptyPromise(
-            _is_ajax_finished,
+            self.is_ajax_finished,
            "Loading more Responses"
        ).fulfill()

@@ -304,3 +307,44 @@ class DiscussionUserProfilePage(CoursePage):
    def click_on_page(self, page_number):
        self._click_pager_with_text(unicode(page_number), page_number)

+
+class DiscussionTabHomePage(CoursePage, DiscussionPageMixin):
+
+    ALERT_SELECTOR = ".discussion-body .sidebar .search-alert"
+
+    def __init__(self, browser, course_id):
+        super(DiscussionTabHomePage, self).__init__(browser, course_id)
+        self.url_path = "discussion/forum/"
+
+    def is_browser_on_page(self):
+        return self.q(css=".discussion-body section.home-header").present
+
+    def perform_search(self):
+        self.q(css=".discussion-body .sidebar .search").first.click()
+        EmptyPromise(
+            lambda: self.q(css=".discussion-body .sidebar .search.is-open").present,
+            "waiting for search input to be available"
+        ).fulfill()
+        self.q(css="#search-discussions").fill("dummy" + chr(10))
+        EmptyPromise(
+            self.is_ajax_finished,
+            "waiting for server to return result"
+        ).fulfill()
+
+    def get_search_alert_messages(self):
+        return self.q(css=self.ALERT_SELECTOR + " .message").text
+
+    def dismiss_alert_message(self, text):
+        """
+        dismiss any search alert message containing the specified text.
+        """
+        def _match_messages(text):
+            return self.q(css=".search-alert").filter(lambda elem: text in elem.text)
+
+        for alert_id in _match_messages(text).attrs("id"):
+            self.q(css="{}#{} a.dismiss".format(self.ALERT_SELECTOR, alert_id)).click()
+        EmptyPromise(
+            lambda: _match_messages(text).results == [],
+            "waiting for dismissed alerts to disappear"
+        ).fulfill()
+
--- a/common/test/acceptance/tests/test_discussion.py
+++ b/common/test/acceptance/tests/test_discussion.py
@@ -11,10 +11,19 @@ from ..pages.lms.discussion import (
    DiscussionTabSingleThreadPage,
    InlineDiscussionPage,
    InlineDiscussionThreadPage,
-    DiscussionUserProfilePage
+    DiscussionUserProfilePage,
+    DiscussionTabHomePage
 )
 from ..fixtures.course import CourseFixture, XBlockFixtureDesc
-from ..fixtures.discussion import SingleThreadViewFixture, UserProfileViewFixture, Thread, Response, Comment
+from ..fixtures.discussion import (
+    SingleThreadViewFixture,
+    UserProfileViewFixture,
+    SearchResultFixture,
+    Thread,
+    Response,
+    Comment,
+    SearchResult
+)


 class DiscussionResponsePaginationTestMixin(object):
@@ -405,3 +414,47 @@ class DiscussionUserProfileTest(UniqueCourseTest):

    def test_151_threads(self):
        self.check_pages(151)
+
+class DiscussionSearchAlertTest(UniqueCourseTest):
+    """
+    Tests for spawning and dismissing alerts related to user search actions and their results.
+    """
+
+    def setUp(self):
+        super(DiscussionSearchAlertTest, self).setUp()
+        CourseFixture(**self.course_info).install()
+        AutoAuthPage(self.browser, course_id=self.course_id).visit()
+        self.page = DiscussionTabHomePage(self.browser, self.course_id)
+        self.page.visit()
+
+    def setup_corrected_text(self, text):
+        SearchResultFixture(SearchResult(corrected_text=text)).push()
+
+    def check_search_alert_messages(self, expected):
+        actual = self.page.get_search_alert_messages()
+        self.assertTrue(all(map(lambda msg, sub: msg.find(sub) >= 0, actual, expected)))
+
+    def test_no_rewrite(self):
+        self.setup_corrected_text(None)
+        self.page.perform_search()
+        self.check_search_alert_messages([])
+
+    def test_rewrite_dismiss(self):
+        self.setup_corrected_text("foo")
+        self.page.perform_search()
+        self.check_search_alert_messages(["foo"])
+        self.page.dismiss_alert_message("foo")
+        self.check_search_alert_messages([])
+
+    def test_new_search(self):
+        self.setup_corrected_text("foo")
+        self.page.perform_search()
+        self.check_search_alert_messages(["foo"])
+
+        self.setup_corrected_text("bar")
+        self.page.perform_search()
+        self.check_search_alert_messages(["bar"])
+
+        self.setup_corrected_text(None)
+        self.page.perform_search()
+        self.check_search_alert_messages([])