Skip to content

Commit

Permalink
fix: do not decorate ELLIPSIS with <b> tags (#123)
Browse files Browse the repository at this point in the history
* fix: do not decorate `ELLIPSIS` with `<b>` tags

Multiple matches are merged with `ELLIPSIS` as a separator.
This reverses the order of operations so that the matches are decorated before
they are merged. This way, the content of `ELLIPSIS` is not decorated with HTML
`<b>` tags.

* fix: use correct module for Iterable

Collections Abstract Base Classes are deprecated in `collections` since
Python 3.3. They will be moved to `collections.abc` after Python 3.8.
https://docs.python.org/3.8/library/collections.html
  • Loading branch information
Agrendalath authored Apr 8, 2022
1 parent decb566 commit 3cef5ef
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 8 deletions.
2 changes: 1 addition & 1 deletion edxsearch/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
""" Container module for testing / demoing search """

__version__ = '3.2.0'
__version__ = '3.3.0'
8 changes: 4 additions & 4 deletions search/result_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ def excerpt(self):
match_phrases,
DESIRED_EXCERPT_LENGTH
)
excerpt_text = ELLIPSIS.join(matches)

for match_word in match_phrases:
excerpt_text = SearchResultProcessor.decorate_matches(excerpt_text, match_word)
for i, _ in enumerate(matches):
for match_word in match_phrases:
matches[i] = SearchResultProcessor.decorate_matches(matches[i], match_word)

return excerpt_text
return ELLIPSIS.join(matches)
19 changes: 18 additions & 1 deletion search/tests/test_search_result_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from django.test import TestCase
from django.test.utils import override_settings
from search.result_processor import SearchResultProcessor
from search.result_processor import SearchResultProcessor, ELLIPSIS


# Any class that inherits from TestCase will cause too-many-public-methods pylint error
Expand Down Expand Up @@ -250,6 +250,23 @@ def test_excerpt_quoted(self, search_phrase, expected_excerpt):
srp = SearchResultProcessor(test_result, search_phrase)
self.assertIn(expected_excerpt, srp.excerpt)

def test_excerpt_ellipsis_undecorated(self):
"""
Multiple matches are joined with `ELLIPSIS` as a separator.
This verifies that the `ELLIPSIS` is not decorated with the HTML `<b>` tag when it matches the search phrase.
E.g. when `a` was in the search phrases before, it was resulting in text like `<sp<b>a</b>n`, which is not valid
HTML.
"""
test_result = {
"content": {
"a": "Just a line of text.",
"b": "Just a line of different text.",
}
}
srp = SearchResultProcessor(test_result, 'Just a line')
self.assertIn(ELLIPSIS, srp.excerpt)


class TestSearchResultProcessor(SearchResultProcessor):
"""
Expand Down
4 changes: 2 additions & 2 deletions search/utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
""" Utility classes to support others """

import importlib
import collections
from collections.abc import Iterable


def _load_class(class_path, default):
Expand All @@ -21,7 +21,7 @@ def _load_class(class_path, default):

def _is_iterable(item):
""" Checks if an item is iterable (list, tuple, generator), but not string """
return isinstance(item, collections.Iterable) and not isinstance(item, str)
return isinstance(item, Iterable) and not isinstance(item, str)


class ValueRange:
Expand Down

0 comments on commit 3cef5ef

Please sign in to comment.