Commit e6871af2 by jsa

forums: add UX for username search results.

JIRA: FOR-627
parent 7cc85b43
...@@ -84,63 +84,6 @@ describe "DiscussionThreadListView", -> ...@@ -84,63 +84,6 @@ describe "DiscussionThreadListView", ->
@view = new DiscussionThreadListView({collection: @discussion, el: $(".sidebar")}) @view = new DiscussionThreadListView({collection: @discussion, el: $(".sidebar")})
@view.render() @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()
makeView = (discussion) -> makeView = (discussion) ->
return new DiscussionThreadListView( return new DiscussionThreadListView(
el: $(".sidebar"), el: $(".sidebar"),
...@@ -202,3 +145,129 @@ describe "DiscussionThreadListView", -> ...@@ -202,3 +145,129 @@ describe "DiscussionThreadListView", ->
it "with sort preference comments", -> it "with sort preference comments", ->
changeSorting(@threads, "votes", "comments", ["Thread3", "Thread2", "Thread1"]) changeSorting(@threads, "votes", "comments", ["Thread3", "Thread2", "Thread1"])
describe "search alerts", ->
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 []
describe "search spell correction", ->
beforeEach ->
spyOn(@view, "searchForUser")
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.callCount).toEqual(1)
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.callCount).toEqual(1)
expect(@view.addSearchAlert.mostRecentCall.args[0]).toMatch(/no threads matched/i)
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()
describe "username search", ->
it "makes correct ajax calls", ->
$.ajax.andCallFake(
(params) =>
expect(params.data.username).toEqual("testing-username")
expect(params.url.path()).toEqual(DiscussionUtil.urlFor("users"))
params.success(
{users: []}, 'success'
)
{always: ->}
)
@view.searchForUser("testing-username")
expect($.ajax).toHaveBeenCalled()
setAjaxResults = (threadSuccess, userResult) ->
# threadSuccess is a boolean indicating whether the thread search ajax call should succeed
# userResult is the value that should be returned as data from the username search ajax call
$.ajax.andCallFake(
(params) =>
if params.data.text and threadSuccess
params.success(
{discussion_data: [], page: 42, num_pages: 99, corrected_text: "dummy"},
"success"
)
else if params.data.username
params.success(
{users: userResult},
"success"
)
{always: ->}
)
it "gets called after a thread search succeeds", ->
spyOn(@view, "searchForUser").andCallThrough()
setAjaxResults(true, [])
@view.searchFor("gizmo")
expect(@view.searchForUser).toHaveBeenCalled()
expect($.ajax.mostRecentCall.args[0].data.username).toEqual("gizmo")
it "does not get called after a thread search fails", ->
spyOn(@view, "searchForUser").andCallThrough()
setAjaxResults(false, [])
@view.searchFor("gizmo")
expect(@view.searchForUser).not.toHaveBeenCalled()
it "adds a search alert when an username was matched", ->
spyOn(@view, "addSearchAlert")
setAjaxResults(true, [{username: "gizmo", id: "1"}])
@view.searchForUser("dummy")
expect($.ajax).toHaveBeenCalled()
expect(@view.addSearchAlert).toHaveBeenCalled()
expect(@view.addSearchAlert.mostRecentCall.args[0]).toMatch(/gizmo/)
it "does not add a search alert when no username was matched", ->
spyOn(@view, "addSearchAlert")
setAjaxResults(true, [])
@view.searchForUser("dummy")
expect($.ajax).toHaveBeenCalled()
expect(@view.addSearchAlert).not.toHaveBeenCalled()
...@@ -73,6 +73,7 @@ class @DiscussionUtil ...@@ -73,6 +73,7 @@ class @DiscussionUtil
downvote_comment : "/courses/#{$$course_id}/discussion/comments/#{param}/downvote" downvote_comment : "/courses/#{$$course_id}/discussion/comments/#{param}/downvote"
undo_vote_for_comment : "/courses/#{$$course_id}/discussion/comments/#{param}/unvote" undo_vote_for_comment : "/courses/#{$$course_id}/discussion/comments/#{param}/unvote"
upload : "/courses/#{$$course_id}/discussion/upload" upload : "/courses/#{$$course_id}/discussion/upload"
users : "/courses/#{$$course_id}/discussion/users"
search : "/courses/#{$$course_id}/discussion/forum/search" search : "/courses/#{$$course_id}/discussion/forum/search"
retrieve_discussion : "/courses/#{$$course_id}/discussion/forum/#{param}/inline" retrieve_discussion : "/courses/#{$$course_id}/discussion/forum/#{param}/inline"
retrieve_single_thread : "/courses/#{$$course_id}/discussion/forum/#{param}/threads/#{param1}" retrieve_single_thread : "/courses/#{$$course_id}/discussion/forum/#{param}/threads/#{param1}"
......
...@@ -445,6 +445,7 @@ if Backbone? ...@@ -445,6 +445,7 @@ if Backbone?
data: { text: text } data: { text: text }
url: url url: url
type: "GET" type: "GET"
dataType: 'json'
$loading: $ $loading: $
loadingCallback: => loadingCallback: =>
@$(".post-list").html('<li class="loading"><div class="loading-animation"><span class="sr">' + gettext('Loading thread list') + '</span></div></li>') @$(".post-list").html('<li class="loading"><div class="loading-animation"><span class="sr">' + gettext('Loading thread list') + '</span></div></li>')
...@@ -465,10 +466,36 @@ if Backbone? ...@@ -465,10 +466,36 @@ if Backbone?
true true
) )
@addSearchAlert(message) @addSearchAlert(message)
else if response.discussion_data.length == 0
@addSearchAlert(gettext('No threads matched your query.'))
# TODO: Perhaps reload user info so that votes can be updated. # 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 # In the future we might not load all of a user's votes at once
# so this would probably be necessary anyway # so this would probably be necessary anyway
@displayedCollection.reset(@collection.models) # Don't think this is necessary @displayedCollection.reset(@collection.models) # Don't think this is necessary
@searchForUser(text) if text
searchForUser: (text) ->
DiscussionUtil.safeAjax
data: { username: text }
url: DiscussionUtil.urlFor("users")
type: "GET"
dataType: 'json'
error: =>
return
success: (response) =>
if response.users.length > 0
message = interpolate(
_.escape(gettext('Show posts by %(username)s.')),
{"username":
_.template('<a class="link-jump" href="<%= url %>"><%- username %></a>', {
url: DiscussionUtil.urlFor("user_profile", response.users[0].id),
username: response.users[0].username
})
},
true
)
@addSearchAlert(message)
clearSearch: (callback, value) -> clearSearch: (callback, value) ->
@$(".post-search-field").val("") @$(".post-search-field").val("")
......
...@@ -351,13 +351,13 @@ class DiscussionTabHomePage(CoursePage, DiscussionPageMixin): ...@@ -351,13 +351,13 @@ class DiscussionTabHomePage(CoursePage, DiscussionPageMixin):
def is_browser_on_page(self): def is_browser_on_page(self):
return self.q(css=".discussion-body section.home-header").present return self.q(css=".discussion-body section.home-header").present
def perform_search(self): def perform_search(self, text="dummy"):
self.q(css=".discussion-body .sidebar .search").first.click() self.q(css=".discussion-body .sidebar .search").first.click()
EmptyPromise( EmptyPromise(
lambda: self.q(css=".discussion-body .sidebar .search.is-open").present, lambda: self.q(css=".discussion-body .sidebar .search.is-open").present,
"waiting for search input to be available" "waiting for search input to be available"
).fulfill() ).fulfill()
self.q(css="#search-discussions").fill("dummy" + chr(10)) self.q(css="#search-discussions").fill(text + chr(10))
EmptyPromise( EmptyPromise(
self.is_ajax_finished, self.is_ajax_finished,
"waiting for server to return result" "waiting for server to return result"
...@@ -366,6 +366,9 @@ class DiscussionTabHomePage(CoursePage, DiscussionPageMixin): ...@@ -366,6 +366,9 @@ class DiscussionTabHomePage(CoursePage, DiscussionPageMixin):
def get_search_alert_messages(self): def get_search_alert_messages(self):
return self.q(css=self.ALERT_SELECTOR + " .message").text return self.q(css=self.ALERT_SELECTOR + " .message").text
def get_search_alert_links(self):
return self.q(css=self.ALERT_SELECTOR + " .link-jump")
def dismiss_alert_message(self, text): def dismiss_alert_message(self, text):
""" """
dismiss any search alert message containing the specified text. dismiss any search alert message containing the specified text.
......
...@@ -421,9 +421,18 @@ class DiscussionSearchAlertTest(UniqueCourseTest): ...@@ -421,9 +421,18 @@ class DiscussionSearchAlertTest(UniqueCourseTest):
Tests for spawning and dismissing alerts related to user search actions and their results. Tests for spawning and dismissing alerts related to user search actions and their results.
""" """
SEARCHED_USERNAME = "gizmo"
def setUp(self): def setUp(self):
super(DiscussionSearchAlertTest, self).setUp() super(DiscussionSearchAlertTest, self).setUp()
CourseFixture(**self.course_info).install() CourseFixture(**self.course_info).install()
# first auto auth call sets up a user that we will search for in some tests
self.searched_user_id = AutoAuthPage(
self.browser,
username=self.SEARCHED_USERNAME,
course_id=self.course_id
).visit().get_user_id()
# this auto auth call creates the actual session user
AutoAuthPage(self.browser, course_id=self.course_id).visit() AutoAuthPage(self.browser, course_id=self.course_id).visit()
self.page = DiscussionTabHomePage(self.browser, self.course_id) self.page = DiscussionTabHomePage(self.browser, self.course_id)
self.page.visit() self.page.visit()
...@@ -433,12 +442,12 @@ class DiscussionSearchAlertTest(UniqueCourseTest): ...@@ -433,12 +442,12 @@ class DiscussionSearchAlertTest(UniqueCourseTest):
def check_search_alert_messages(self, expected): def check_search_alert_messages(self, expected):
actual = self.page.get_search_alert_messages() actual = self.page.get_search_alert_messages()
self.assertTrue(all(map(lambda msg, sub: msg.find(sub) >= 0, actual, expected))) self.assertTrue(all(map(lambda msg, sub: msg.lower().find(sub.lower()) >= 0, actual, expected)))
def test_no_rewrite(self): def test_no_rewrite(self):
self.setup_corrected_text(None) self.setup_corrected_text(None)
self.page.perform_search() self.page.perform_search()
self.check_search_alert_messages([]) self.check_search_alert_messages(["no threads"])
def test_rewrite_dismiss(self): def test_rewrite_dismiss(self):
self.setup_corrected_text("foo") self.setup_corrected_text("foo")
...@@ -458,7 +467,26 @@ class DiscussionSearchAlertTest(UniqueCourseTest): ...@@ -458,7 +467,26 @@ class DiscussionSearchAlertTest(UniqueCourseTest):
self.setup_corrected_text(None) self.setup_corrected_text(None)
self.page.perform_search() self.page.perform_search()
self.check_search_alert_messages([]) self.check_search_alert_messages(["no threads"])
def test_rewrite_and_user(self):
self.setup_corrected_text("foo")
self.page.perform_search(self.SEARCHED_USERNAME)
self.check_search_alert_messages(["foo", self.SEARCHED_USERNAME])
def test_user_only(self):
self.setup_corrected_text(None)
self.page.perform_search(self.SEARCHED_USERNAME)
self.check_search_alert_messages(["no threads", self.SEARCHED_USERNAME])
# make sure clicking the link leads to the user profile page
UserProfileViewFixture([]).push()
self.page.get_search_alert_links().first.click()
DiscussionUserProfilePage(
self.browser,
self.course_id,
self.searched_user_id,
self.SEARCHED_USERNAME
).wait_for_page()
class DiscussionSortPreferenceTest(UniqueCourseTest): class DiscussionSortPreferenceTest(UniqueCourseTest):
......
...@@ -70,11 +70,12 @@ body.discussion { ...@@ -70,11 +70,12 @@ body.discussion {
@include transition(none); @include transition(none);
@extend %t-weight5; @extend %t-weight5;
padding: ($baseline/4) ($baseline/2); padding: ($baseline/4) ($baseline/2);
color: $white;
// reseting poorly globally scoped hover/focus state for this control // reseting poorly globally scoped hover/focus state for this control
&:hover, &:focus { &:hover, &:focus {
color: $link-color; color: $white;
text-decoration: underline; text-decoration: none;
} }
} }
} }
......
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