diff --git a/common/static/common/js/components/collections/paging_collection.js b/common/static/common/js/components/collections/paging_collection.js index 4bf4ff3066..a8af909897 100644 --- a/common/static/common/js/components/collections/paging_collection.js +++ b/common/static/common/js/components/collections/paging_collection.js @@ -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(); }, diff --git a/common/test/acceptance/tests/helpers.py b/common/test/acceptance/tests/helpers.py index 16d74902ce..c89b37cdc2 100644 --- a/common/test/acceptance/tests/helpers.py +++ b/common/test/acceptance/tests/helpers.py @@ -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.""" diff --git a/common/test/acceptance/tests/lms/test_teams.py b/common/test/acceptance/tests/lms/test_teams.py index 22ad5f7f7d..db7ad3de81 100644 --- a/common/test/acceptance/tests/lms/test_teams.py +++ b/common/test/acceptance/tests/lms/test_teams.py @@ -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')) diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js index 6809e4adba..e392de7185 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js @@ -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'); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js index afe02aad49..c43da45b7d 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js @@ -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('
'); 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'); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js index a67a5298dc..ec08acfe1b 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js @@ -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([]) }); diff --git a/lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js b/lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js index 2d74fbcde6..94090f9fc8 100644 --- a/lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js +++ b/lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js @@ -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, diff --git a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js index 87f4d4ab54..d95cc7b428 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -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) {