From eec2bed0c7e7f60ba79fbab07d3aee1b574dca40 Mon Sep 17 00:00:00 2001 From: Bill DeRusha Date: Thu, 10 Sep 2015 12:24:31 -0400 Subject: [PATCH] Add edx-search configuration to lms remove "defensive code" until we can figure out how to actually catch the errors, --- cms/envs/common.py | 3 - cms/envs/test.py | 3 - .../test/acceptance/tests/lms/test_teams.py | 2 +- lms/djangoapps/teams/errors.py | 5 -- .../commands/reindex_course_team.py | 4 +- .../tests/test_reindex_course_team.py | 4 +- lms/djangoapps/teams/search_indexes.py | 17 +----- .../teams/js/spec/views/teams_tab_spec.js | 58 +++++-------------- .../teams/js/spec/views/topic_teams_spec.js | 2 +- .../teams/static/teams/js/views/teams_tab.js | 2 +- .../static/teams/js/views/topic_teams.js | 15 ++--- lms/djangoapps/teams/views.py | 10 +--- lms/envs/aws.py | 4 +- lms/envs/test.py | 3 - 14 files changed, 36 insertions(+), 96 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index eb7980eabe..f1d34953a1 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -170,9 +170,6 @@ FEATURES = { # Teams feature 'ENABLE_TEAMS': True, - # Teams search feature - 'ENABLE_TEAMS_SEARCH': False, - # Show video bumper in Studio 'ENABLE_VIDEO_BUMPER': False, diff --git a/cms/envs/test.py b/cms/envs/test.py index 1539c28fb7..16133ac8ef 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -281,8 +281,5 @@ SEARCH_ENGINE = "search.tests.mock_search_engine.MockSearchEngine" # teams feature FEATURES['ENABLE_TEAMS'] = True -# teams search -FEATURES['ENABLE_TEAMS_SEARCH'] = True - # Dummy secret key for dev/test SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' diff --git a/common/test/acceptance/tests/lms/test_teams.py b/common/test/acceptance/tests/lms/test_teams.py index 6bc7f9a385..22ad5f7f7d 100644 --- a/common/test/acceptance/tests/lms/test_teams.py +++ b/common/test/acceptance/tests/lms/test_teams.py @@ -783,7 +783,7 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase): self.browse_teams_page.click_browse_all_teams_link() self.assertTrue(self.topics_page.is_browser_on_page()) - @skip('Disabled until search connectivity issues are resolved, see TNL-3206') + @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 diff --git a/lms/djangoapps/teams/errors.py b/lms/djangoapps/teams/errors.py index a011ea6780..0f718d93a6 100644 --- a/lms/djangoapps/teams/errors.py +++ b/lms/djangoapps/teams/errors.py @@ -16,11 +16,6 @@ class AlreadyOnTeamInCourse(TeamAPIRequestError): pass -class ElasticSearchConnectionError(TeamAPIRequestError): - """System was unable to connect to the configured elasticsearch instance""" - pass - - class ImmutableMembershipFieldException(Exception): """An attempt was made to change an immutable field on a CourseTeamMembership model""" pass diff --git a/lms/djangoapps/teams/management/commands/reindex_course_team.py b/lms/djangoapps/teams/management/commands/reindex_course_team.py index f857736a64..901789180f 100644 --- a/lms/djangoapps/teams/management/commands/reindex_course_team.py +++ b/lms/djangoapps/teams/management/commands/reindex_course_team.py @@ -53,8 +53,8 @@ class Command(BaseCommand): if len(args) == 0 and not options.get('all', False): raise CommandError(u"reindex_course_team requires one or more arguments: ") - elif not settings.FEATURES.get('ENABLE_TEAMS_SEARCH', False): - raise CommandError(u"ENABLE_TEAMS_SEARCH must be enabled to use course team indexing") + elif not settings.FEATURES.get('ENABLE_TEAMS', False): + raise CommandError(u"ENABLE_TEAMS must be enabled to use course team indexing") if options.get('all', False): course_teams = CourseTeam.objects.all() diff --git a/lms/djangoapps/teams/management/commands/tests/test_reindex_course_team.py b/lms/djangoapps/teams/management/commands/tests/test_reindex_course_team.py index 885bf48188..63b7593bcd 100644 --- a/lms/djangoapps/teams/management/commands/tests/test_reindex_course_team.py +++ b/lms/djangoapps/teams/management/commands/tests/test_reindex_course_team.py @@ -39,9 +39,9 @@ class ReindexCourseTeamTest(SharedModuleStoreTestCase): def test_teams_search_flag_disabled_raises_command_error(self): """ Test that raises CommandError for disabled feature flag. """ with mock.patch('django.conf.settings.FEATURES') as features: - features.return_value = {"ENABLE_TEAMS_SEARCH": False} + features.return_value = {"ENABLE_TEAMS": False} with self.assertRaises(SystemExit), nostderr(): - with self.assertRaisesRegexp(CommandError, ".* ENABLE_TEAMS_SEARCH must be enabled .*"): + with self.assertRaisesRegexp(CommandError, ".* ENABLE_TEAMS must be enabled .*"): call_command('reindex_course_team') def test_given_invalid_team_id_raises_command_error(self): diff --git a/lms/djangoapps/teams/search_indexes.py b/lms/djangoapps/teams/search_indexes.py index 6350f2bf91..c1acc2e3bd 100644 --- a/lms/djangoapps/teams/search_indexes.py +++ b/lms/djangoapps/teams/search_indexes.py @@ -1,8 +1,5 @@ """ Search index used to load data into elasticsearch""" -import logging -from requests import ConnectionError - from django.conf import settings from django.db.models.signals import post_delete, post_save from django.dispatch import receiver @@ -11,7 +8,6 @@ from functools import wraps from search.search_engine_base import SearchEngine -from .errors import ElasticSearchConnectionError from .serializers import CourseTeamSerializer, CourseTeam @@ -34,7 +30,7 @@ class CourseTeamIndexer(object): """ INDEX_NAME = "course_team_index" DOCUMENT_TYPE_NAME = "course_team" - ENABLE_SEARCH_KEY = "ENABLE_TEAMS_SEARCH" + ENABLE_SEARCH_KEY = "ENABLE_TEAMS" def __init__(self, course_team): self.course_team = course_team @@ -108,11 +104,7 @@ class CourseTeamIndexer(object): Return course team search engine (if feature is enabled). """ if cls.search_is_enabled(): - try: - return SearchEngine.get_search_engine(index=cls.INDEX_NAME) - except ConnectionError as err: - logging.error("Error connecting to elasticsearch: %s", err) - raise ElasticSearchConnectionError + return SearchEngine.get_search_engine(index=cls.INDEX_NAME) @classmethod def search_is_enabled(cls): @@ -127,10 +119,7 @@ def course_team_post_save_callback(**kwargs): """ Reindex object after save. """ - try: - CourseTeamIndexer.index(kwargs['instance']) - except ElasticSearchConnectionError: - pass + CourseTeamIndexer.index(kwargs['instance']) @receiver(post_delete, sender=CourseTeam, dispatch_uid='teams.signals.course_team_post_delete_callback') 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 194eb73029..afe02aad49 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 @@ -9,7 +9,7 @@ define([ ], function ($, Backbone, Logger, AjaxHelpers, SpecHelpers, TeamsTabView, TeamSpecHelpers) { 'use strict'; - describe('TeamsTab', function () { + describe('TeamsTab', function() { var expectError = function (teamsTabView, text) { expect(teamsTabView.$('.warning').text()).toContain(text); }; @@ -19,19 +19,12 @@ define([ }; var createTeamsTabView = function(options) { - var defaultTopics = { - count: 5, - num_pages: 1, - current_page: 1, - start: 0, - results: TeamSpecHelpers.createMockTopicData(1, 5) - }, - teamsTabView = new TeamsTabView( - { - el: $('.teams-content'), - context: TeamSpecHelpers.createMockContext(options) - } - ); + var teamsTabView = new TeamsTabView( + { + el: $('.teams-content'), + context: TeamSpecHelpers.createMockContext(options) + } + ); teamsTabView.start(); return teamsTabView; }; @@ -115,6 +108,7 @@ define([ AjaxHelpers.respondWithError(requests, 500); expectError(teamsTabView, "Your request could not be completed due to a server problem. Reload the page and try again. If the issue persists, click the Help tab to report the problem."); expectFocus(teamsTabView.$('.warning')); + }); it('does not navigate to the topics page when syncing its collection if not on the search page', function () { var teamsTabView = createTeamsTabView(), @@ -174,7 +168,7 @@ define([ }, function (url, expectedEvent) { var requests = AjaxHelpers.requests(this), teamsTabView = createTeamsTabView({ - userInfo: TeamSpecHelpers.createMockUserInfo({ staff: true }) + userInfo: TeamSpecHelpers.createMockUserInfo({staff: true}) }); teamsTabView.router.navigate(url, {trigger: true}); if (requests.length) { @@ -187,7 +181,7 @@ define([ describe('Discussion privileges', function () { it('allows privileged access to any team', function () { var teamsTabView = createTeamsTabView({ - userInfo: TeamSpecHelpers.createMockUserInfo({ privileged: true }) + userInfo: TeamSpecHelpers.createMockUserInfo({privileged: true}) }); // Note: using `undefined` here to ensure that we // don't even look at the team when the user is @@ -215,10 +209,10 @@ define([ it('does not allow access if the user is neither privileged nor a team member', function () { var teamsTabView = createTeamsTabView({ - userInfo: TeamSpecHelpers.createMockUserInfo({ privileged: false, staff: true }) + userInfo: TeamSpecHelpers.createMockUserInfo({privileged: false, staff: true}) }); expect(teamsTabView.readOnlyDiscussion({ - attributes: { membership: [] } + attributes: {membership: []} })).toBe(true); }); }); @@ -240,7 +234,7 @@ define([ )); }; - xit('can search teams', function () { + it('can search teams', function () { var requests = AjaxHelpers.requests(this), teamsTabView = createTeamsTabView(); teamsTabView.browseTopic(TeamSpecHelpers.testTopicID); @@ -260,7 +254,7 @@ define([ expect(teamsTabView.$('.page-description').text()).toBe('Showing results for "foo"'); }); - xit('can clear a search', function () { + it('can clear a search', function () { var requests = AjaxHelpers.requests(this), teamsTabView = createTeamsTabView(); teamsTabView.browseTopic(TeamSpecHelpers.testTopicID); @@ -289,29 +283,7 @@ define([ expect(teamsTabView.$('.page-description').text()).toBe('Test description 1'); }); - xit('clears the search when navigating away and then back', function () { - var requests = AjaxHelpers.requests(this), - teamsTabView = createTeamsTabView(); - teamsTabView.browseTopic(TeamSpecHelpers.testTopicID); - AjaxHelpers.respondWithJson(requests, {}); - - // Perform a search - teamsTabView.$('.search-field').val('foo'); - teamsTabView.$('.action-search').click(); - AjaxHelpers.respondWithJson(requests, {}); - - // Navigate back to the teams list - teamsTabView.$('.breadcrumbs a').last().click(); - verifyTeamsRequest(removeTeamEvents(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'); - }); - - xit('does not switch to showing results when the search returns an error', function () { + 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); 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 902fb01208..a67a5298dc 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 @@ -66,7 +66,7 @@ define([ expect(Backbone.history.navigate.calls[0].args).toContain('browse'); }); - xit('gives the search field focus when clicking on the search teams link', function () { + it('gives the search field focus when clicking on the search teams link', function () { var emptyMembership = TeamSpecHelpers.createMockTeamMemberships([]), teamsView = createTopicTeamsView({ teamMemberships: emptyMembership }); spyOn($.fn, 'focus').andCallThrough(); 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 04b2ec1221..e83abeb0ed 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -365,7 +365,7 @@ viewWithHeader = this.createViewWithHeader({ subject: topic, mainView: teamsView, - headerActionsView: null, // TODO: add back SearchFieldView when search is enabled + headerActionsView: searchFieldView, title: options.title, description: options.description, breadcrumbs: this.createBreadcrumbs() diff --git a/lms/djangoapps/teams/static/teams/js/views/topic_teams.js b/lms/djangoapps/teams/static/teams/js/views/topic_teams.js index 2d23304bd4..b545483d5d 100644 --- a/lms/djangoapps/teams/static/teams/js/views/topic_teams.js +++ b/lms/djangoapps/teams/static/teams/js/views/topic_teams.js @@ -57,16 +57,13 @@ }, searchTeams: function (event) { - //var searchField = $('.page-header-search .search-field'); + var searchField = $('.page-header-search .search-field'); event.preventDefault(); - //searchField.focus(); - //searchField.select(); - //$('html, body').animate({ - // scrollTop: 0 - //}, 500); - - // TODO! Will navigate to correct place once required functionality is available - Backbone.history.navigate('browse', {trigger: true}); + searchField.focus(); + searchField.select(); + $('html, body').animate({ + scrollTop: 0 + }, 500); }, showCreateTeamForm: function (event) { diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index e1d49f8fdc..eea9e58400 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -58,7 +58,7 @@ from .serializers import ( add_team_count ) from .search_indexes import CourseTeamIndexer -from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam, ElasticSearchConnectionError +from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam TEAM_MEMBERSHIPS_PER_PAGE = 2 TOPICS_PER_PAGE = 12 @@ -366,13 +366,7 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): return Response(error, status=status.HTTP_400_BAD_REQUEST) result_filter.update({'topic_id': topic_id}) if text_search and CourseTeamIndexer.search_is_enabled(): - try: - search_engine = CourseTeamIndexer.engine() - except ElasticSearchConnectionError: - return Response( - build_api_error(ugettext_noop('Error connecting to elasticsearch')), - status=status.HTTP_400_BAD_REQUEST - ) + search_engine = CourseTeamIndexer.engine() result_filter.update({'course_id': course_id_string}) search_results = search_engine.search( diff --git a/lms/envs/aws.py b/lms/envs/aws.py index aae22b3f82..3566c13970 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -633,10 +633,12 @@ PDF_RECEIPT_COBRAND_LOGO_HEIGHT_MM = ENV_TOKENS.get( if FEATURES.get('ENABLE_COURSEWARE_SEARCH') or \ FEATURES.get('ENABLE_DASHBOARD_SEARCH') or \ FEATURES.get('ENABLE_COURSE_DISCOVERY') or \ - FEATURES.get('ENABLE_TEAMS_SEARCH'): + FEATURES.get('ENABLE_TEAMS'): # Use ElasticSearch as the search engine herein SEARCH_ENGINE = "search.elastic.ElasticSearchEngine" +ELASTIC_SEARCH_CONFIG = ENV_TOKENS.get('ELASTIC_SEARCH_CONFIG', [{}]) + # Facebook app FACEBOOK_API_VERSION = AUTH_TOKENS.get("FACEBOOK_API_VERSION") FACEBOOK_APP_SECRET = AUTH_TOKENS.get("FACEBOOK_APP_SECRET") diff --git a/lms/envs/test.py b/lms/envs/test.py index b26c03b4de..1e52d4297c 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -486,9 +486,6 @@ FEATURES['ENABLE_EDXNOTES'] = True # Enable teams feature for tests. FEATURES['ENABLE_TEAMS'] = True -# Enable teams search for tests. -FEATURES['ENABLE_TEAMS_SEARCH'] = True - # Add milestones to Installed apps for testing INSTALLED_APPS += ('milestones', 'openedx.core.djangoapps.call_stack_manager')