Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix duplicate calls to search API

Browse Source 
TNL-3198

Also fixed a regression with the search breadcrumbs
where it wasn't possible to navigate back to the
full list.
This commit is contained in:
Andy Armstrong
2015-09-14 12:18:11 -04:00
parent 0a434e4388
commit f80b6221d9
8 changed files with 108 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
common/static/common/js/components/collections/paging_collection.js
View File

@@ -90,16 +90,20 @@
*/
setPage: function (page) {
var oldPage = this.currentPage,
self = this;
return this.goTo(page - (this.isZeroIndexed ? 1 : 0), {reset: true}).then(
self = this,
deferred = $.Deferred();
this.goTo(page - (this.isZeroIndexed ? 1 : 0), {reset: true}).then(
function () {
self.isStale = false;
self.trigger('page_changed');
deferred.resolve();
},
function () {
self.currentPage = oldPage;
deferred.fail();
}
);
return deferred.promise();
},

36
common/test/acceptance/tests/helpers.py
View File

@@ -334,7 +334,7 @@ class EventsTestMixin(TestCase):
captured_events.append(event)
@contextmanager
def assert_events_match_during(self, event_filter=None, expected_events=None):
def assert_events_match_during(self, event_filter=None, expected_events=None, in_order=True):
"""
Context manager that ensures that events matching the `event_filter` and `expected_events` are emitted.
@@ -351,7 +351,7 @@ class EventsTestMixin(TestCase):
with self.capture_events(event_filter, len(expected_events), captured_events):
yield
self.assert_events_match(expected_events, captured_events)
self.assert_events_match(expected_events, captured_events, in_order=in_order)
def wait_for_events(self, start_time=None, event_filter=None, number_of_matches=1, timeout=None):
"""
@@ -477,17 +477,29 @@ class EventsTestMixin(TestCase):
self.assertEquals(len(matching_events), 0, description)
def assert_events_match(self, expected_events, actual_events):
def assert_events_match(self, expected_events, actual_events, in_order=True):
"""Assert that each actual event matches one of the expected events.
Args:
expected_events (List): a list of dicts representing the expected events.
actual_events (List): a list of dicts that were actually recorded.
in_order (bool): if True then the events must be in the same order (defaults to True).
"""
Assert that each item in the expected events sequence matches its counterpart at the same index in the actual
events sequence.
"""
for expected_event, actual_event in zip(expected_events, actual_events):
assert_event_matches(
expected_event,
actual_event,
tolerate=EventMatchTolerates.lenient()
)
if in_order:
for expected_event, actual_event in zip(expected_events, actual_events):
assert_event_matches(
expected_event,
actual_event,
tolerate=EventMatchTolerates.lenient()
)
else:
for expected_event in expected_events:
actual_event = next(event for event in actual_events if is_matching_event(expected_event, event))
assert_event_matches(
expected_event,
actual_event or {},
tolerate=EventMatchTolerates.lenient()
)
def relative_path_to_absolute_uri(self, relative_path):
"""Return an aboslute URI given a relative path taking into account the test context."""

19
common/test/acceptance/tests/lms/test_teams.py
View File

@@ -9,7 +9,6 @@ from dateutil.parser import parse
import ddt
from nose.plugins.attrib import attr
from uuid import uuid4
from unittest import skip
from ..helpers import EventsTestMixin, UniqueCourseTest
from ...fixtures import LMS_BASE_URL
@@ -783,7 +782,6 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase):
self.browse_teams_page.click_browse_all_teams_link()
self.assertTrue(self.topics_page.is_browser_on_page())
@skip("Skip until TNL-3198 (searching teams makes two AJAX requests) is resolved")
def test_search(self):
"""
Scenario: User should be able to search for a team
@@ -794,6 +792,7 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase):
And the search header should be shown
And 0 results should be shown
And my browser should fire a page viewed event for the search page
And a searched event should have been fired
"""
# Note: all searches will return 0 results with the mock search server
# used by Bok Choy.
@@ -801,21 +800,21 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase):
self.create_teams(self.topic, 5)
self.browse_teams_page.visit()
events = [{
'event_type': 'edx.team.searched',
'event': {
'search_text': search_text,
'topic_id': self.topic['id'],
'number_of_results': 0
}
}, {
'event_type': 'edx.team.page_viewed',
'event': {
'page_name': 'search-teams',
'topic_id': self.topic['id'],
'team_id': None
}
}, {
'event_type': 'edx.team.searched',
'event': {
'search_text': search_text,
'topic_id': self.topic['id'],
'number_of_results': 0
}
}]
with self.assert_events_match_during(self.only_team_events, expected_events=events):
with self.assert_events_match_during(self.only_team_events, expected_events=events, in_order=False):
search_results_page = self.browse_teams_page.search(search_text)
self.verify_search_header(search_results_page, search_text)
self.assertTrue(search_results_page.get_pagination_header_text().startswith('Showing 0 out of 0 total'))

4
lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js
View File

@@ -24,7 +24,9 @@ define([
it('can render itself', function () {
var testTeamData = TeamSpecHelpers.createMockTeamData(1, 5),
teamsView = createTeamsView({
teams: TeamSpecHelpers.createMockTeams(testTeamData)
teams: TeamSpecHelpers.createMockTeams({
results: testTeamData
})
});
expect(teamsView.$('.teams-paging-header').text()).toMatch('Showing 1-5 out of 6 total');

77
lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js
View File

@@ -29,19 +29,6 @@ define([
return teamsTabView;
};
/**
* Filters out all team events from a list of requests.
*/
var removeTeamEvents = function (requests) {
return requests.filter(function (request) {
if (request.requestBody && request.requestBody.startsWith('event_type=edx.team')) {
return false;
} else {
return true;
}
});
};
beforeEach(function () {
setFixtures('<div class="teams-content"></div>');
spyOn($.fn, 'focus');
@@ -233,25 +220,33 @@ define([
options
));
};
it('can search teams', function () {
var requests = AjaxHelpers.requests(this),
teamsTabView = createTeamsTabView();
teamsTabView.browseTopic(TeamSpecHelpers.testTopicID);
verifyTeamsRequest(requests, {
order_by: 'last_activity_at',
text_search: ''
});
AjaxHelpers.respondWithJson(requests, {});
var performSearch = function(requests, teamsTabView) {
teamsTabView.$('.search-field').val('foo');
teamsTabView.$('.action-search').click();
verifyTeamsRequest(requests, {
order_by: '',
text_search: 'foo'
});
AjaxHelpers.respondWithJson(requests, TeamSpecHelpers.createMockTeamsResponse({results: []}));
};
it('can search teams', function () {
var requests = AjaxHelpers.requests(this),
teamsTabView = createTeamsTabView(),
requestCountBeforeSearch;
teamsTabView.browseTopic(TeamSpecHelpers.testTopicID);
verifyTeamsRequest(requests, {
order_by: 'last_activity_at',
text_search: ''
});
AjaxHelpers.respondWithJson(requests, {});
requestCountBeforeSearch = requests.length;
performSearch(requests, teamsTabView);
expect(teamsTabView.$('.page-title').text()).toBe('Team Search');
expect(teamsTabView.$('.page-description').text()).toBe('Showing results for "foo"');
// Expect exactly one search request to be fired
expect(requests.length).toBe(requestCountBeforeSearch + 1);
});
it('can clear a search', function () {
@@ -259,17 +254,10 @@ define([
teamsTabView = createTeamsTabView();
teamsTabView.browseTopic(TeamSpecHelpers.testTopicID);
AjaxHelpers.respondWithJson(requests, {});
performSearch(requests, teamsTabView);
// Perform a search
teamsTabView.$('.search-field').val('foo');
teamsTabView.$('.action-search').click();
// Note: this is a bit of a hack -- without it the URL
// fragment won't be what it would be in the real
// app. This line sets the fragment without triggering
// callbacks, allowing teams_tab.js to detect the
// fragment correctly.
Backbone.history.navigate('topics/' + TeamSpecHelpers.testTopicID + '/search', {trigger: false});
AjaxHelpers.respondWithJson(requests, {});
performSearch(requests, teamsTabView);
// Clear the search and submit it again
teamsTabView.$('.search-field').val('');
@@ -283,16 +271,39 @@ define([
expect(teamsTabView.$('.page-description').text()).toBe('Test description 1');
});
it('can navigate back to all teams from a search', function () {
var requests = AjaxHelpers.requests(this),
teamsTabView = createTeamsTabView();
teamsTabView.browseTopic(TeamSpecHelpers.testTopicID);
AjaxHelpers.respondWithJson(requests, {});
// Perform a search
performSearch(requests, teamsTabView);
// Verify the breadcrumbs have a link back to the teams list, and click on it
expect(teamsTabView.$('.breadcrumbs a').length).toBe(2);
teamsTabView.$('.breadcrumbs a').last().click();
verifyTeamsRequest(requests, {
order_by: 'last_activity_at',
text_search: ''
});
AjaxHelpers.respondWithJson(requests, {});
expect(teamsTabView.$('.page-title').text()).toBe('Test Topic 1');
expect(teamsTabView.$('.page-description').text()).toBe('Test description 1');
});
it('does not switch to showing results when the search returns an error', function () {
var requests = AjaxHelpers.requests(this),
teamsTabView = createTeamsTabView();
teamsTabView.browseTopic(TeamSpecHelpers.testTopicID);
AjaxHelpers.respondWithJson(requests, {});
// Perform a search
// Perform a search but respond with a 500
teamsTabView.$('.search-field').val('foo');
teamsTabView.$('.action-search').click();
AjaxHelpers.respondWithError(requests);
// Verify that the team list is still shown
expect(teamsTabView.$('.page-title').text()).toBe('Test Topic 1');
expect(teamsTabView.$('.page-description').text()).toBe('Test description 1');
expect(teamsTabView.$('.search-field').val(), 'foo');

4
lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js
View File

@@ -44,7 +44,9 @@ define([
it('can render itself', function () {
var testTeamData = TeamSpecHelpers.createMockTeamData(1, 5),
teamsView = createTopicTeamsView({
teams: TeamSpecHelpers.createMockTeams(testTeamData),
teams: TeamSpecHelpers.createMockTeams({
results: testTeamData
}),
teamMemberships: TeamSpecHelpers.createMockTeamMemberships([])
});

17
lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js
View File

@@ -43,18 +43,22 @@ define([
});
};
var createMockTeams = function(teamData) {
if (!teamData) {
teamData = createMockTeamData(1, 5);
}
return new TeamCollection(
var createMockTeamsResponse = function(options) {
return _.extend(
{
count: 6,
num_pages: 2,
current_page: 1,
start: 0,
results: teamData
results: createMockTeamData(1, 5)
},
options
);
};
var createMockTeams = function(options) {
return new TeamCollection(
createMockTeamsResponse(options),
{
teamEvents: teamEvents,
course_id: testCourseID,
@@ -325,6 +329,7 @@ define([
testTeamDiscussionID: testTeamDiscussionID,
testContext: testContext,
createMockTeamData: createMockTeamData,
createMockTeamsResponse: createMockTeamsResponse,
createMockTeams: createMockTeams,
createMockTeamMembershipsData: createMockTeamMembershipsData,
createMockTeamMemberships: createMockTeamMemberships,

9
lms/djangoapps/teams/static/teams/js/views/teams_tab.js
View File

@@ -214,6 +214,7 @@
view.mainView = view.createTeamsListView({
topic: topic,
collection: view.teamsCollection,
breadcrumbs: view.createBreadcrumbs(topic),
title: gettext('Team Search'),
description: interpolate(
gettext('Showing results for "%(searchString)s"'),
@@ -337,6 +338,7 @@
var teamsView = view.createTeamsListView({
topic: topic,
collection: collection,
breadcrumbs: view.createBreadcrumbs(),
showSortControls: true
});
deferred.resolve(teamsView);
@@ -368,7 +370,7 @@
headerActionsView: searchFieldView,
title: options.title,
description: options.description,
breadcrumbs: this.createBreadcrumbs()
breadcrumbs: options.breadcrumbs
}),
searchUrl = 'topics/' + topic.get('id') + '/search';
// Listen to requests to sync the collection and redirect it as follows:
@@ -378,6 +380,11 @@
// 3. Otherwise, do nothing and remain on the current page.
// Note: Backbone makes this a no-op if redirecting to the current page.
this.listenTo(collection, 'sync', function() {
// Clear the stale flag here as by definition the collection is up-to-date,
// and the flag itself isn't guaranteed to be cleared yet. This is to ensure
// that the collection doesn't unnecessarily get refreshed again.
collection.isStale = false;
if (collection.searchString) {
Backbone.history.navigate(searchUrl, {trigger: true});
} else if (Backbone.history.getFragment() === searchUrl) {