diff --git a/search/elastic.py b/search/elastic.py index e7e4145b..76565672 100644 --- a/search/elastic.py +++ b/search/elastic.py @@ -127,7 +127,11 @@ def filter_item(field): def _process_exclude_dictionary(exclude_dictionary): - """ We have a list of ids to exclude from the resultset """ + """ + Based on values in the exclude_dictionary generate a list of term queries that + will filter out unwanted results. + """ + # not_properties will hold the generated term queries. not_properties = [] for exclude_property in exclude_dictionary: exclude_values = exclude_dictionary[exclude_property] @@ -135,6 +139,11 @@ def _process_exclude_dictionary(exclude_dictionary): exclude_values = [exclude_values] not_properties.extend([{"term": {exclude_property: exclude_value}} for exclude_value in exclude_values]) + # Returning a query segment with an empty list freaks out ElasticSearch, + # so just return an empty segment. + if not not_properties: + return {} + return { "not": { "filter": { diff --git a/search/tests/tests.py b/search/tests/tests.py index a4e446fd..7cefa8bf 100644 --- a/search/tests/tests.py +++ b/search/tests/tests.py @@ -744,6 +744,35 @@ def test_exclude_filter_multiple(self): self.assertNotIn("FAKE_ID_4", result_ids) self.assertIn("FAKE_ID_5", result_ids) + def test_exclude_filter_empty(self): + """ Test that search works when exclude filter is an empty list """ + self.searcher.index("test_doc", {"course": "ABC", "org": "edX", "id": "FAKE_ID_1"}) + self.searcher.index("test_doc", {"course": "XYZ", "org": "edX", "id": "FAKE_ID_2"}) + self.searcher.index("test_doc", {"course": "DEF", "org": "MITX", "id": "FAKE_ID_3"}) + self.searcher.index("test_doc", {"course": "GHI", "org": "HarvardX", "id": "FAKE_ID_4"}) + self.searcher.index("test_doc", {"course": "LMN", "org": "edX", "id": "FAKE_ID_5"}) + + response = self.searcher.search(exclude_dictionary={"org": []}) + self.assertEqual(response["total"], 5) + result_ids = [r["data"]["id"] for r in response["results"]] + self.assertIn("FAKE_ID_1", result_ids) + self.assertIn("FAKE_ID_2", result_ids) + self.assertIn("FAKE_ID_3", result_ids) + self.assertIn("FAKE_ID_4", result_ids) + self.assertIn("FAKE_ID_5", result_ids) + + response = self.searcher.search( + field_dictionary={"course": ["XYZ", "LMN", "DEF"]}, + exclude_dictionary={"org": []} + ) + self.assertEqual(response["total"], 3) + result_ids = [r["data"]["id"] for r in response["results"]] + self.assertNotIn("FAKE_ID_1", result_ids) + self.assertIn("FAKE_ID_2", result_ids) + self.assertIn("FAKE_ID_3", result_ids) + self.assertNotIn("FAKE_ID_4", result_ids) + self.assertIn("FAKE_ID_5", result_ids) + @override_settings(SEARCH_ENGINE="search.tests.utils.ForceRefreshElasticSearchEngine") class ElasticSearchTests(MockSearchTests):