Skip to content

Commit

Permalink
fix: collection is not always searchable (#304)
Browse files Browse the repository at this point in the history
  • Loading branch information
jbarreau authored Jan 29, 2025
1 parent 9f8392f commit fa45ab7
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 21 deletions.
1 change: 0 additions & 1 deletion .github/workflows/generic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ jobs:
- uses: actions/checkout@v4
- id: changes
uses: ./.github/actions/changes

lint:
name: Linting
needs: [ changes ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,22 @@ async def _replace_search():
self.stack.queue_customization(_replace_search)
return self

def disable_search(self) -> Self:
"""Disable the search bar
Documentation:
https://docs.forestadmin.com/developer-guide-agents-python/agent-customization/search
Example:
.disable_search()
"""

async def _disable_search():
cast(SearchCollectionDecorator, self.stack.search.get_collection(self.collection_name)).disable_search()

self.stack.queue_customization(_disable_search)
return self

def add_chart(self, name: str, definition: CollectionChartDefinition) -> Self:
"""Create a new API chart
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,22 @@ class SearchCollectionDecorator(CollectionDecorator):
def __init__(self, collection: Collection, datasource: Datasource[BoundCollection]):
super().__init__(collection, datasource)
self._replacer: SearchDefinition = None
self._searchable = len(self._get_searchable_fields(self.child_collection, False)) > 0

def disable_search(self):
self._searchable = False
self.mark_schema_as_dirty()

def replace_search(self, replacer: SearchDefinition):
self._replacer = replacer
self._searchable = True
self.mark_schema_as_dirty()

def _refine_schema(self, sub_schema: CollectionSchema) -> CollectionSchema:
return {**sub_schema, "searchable": True}
return {
**sub_schema,
"searchable": self._searchable,
}

def _default_replacer(self, search: str, extended: bool) -> ConditionTree:
searchable_fields = self._get_searchable_fields(self.child_collection, extended)
Expand Down Expand Up @@ -77,18 +87,14 @@ async def _refine_filter(
return _filter

def _build_condition(self, field: str, schema: Column, search: str) -> Union[ConditionTree, None]:
if (
schema["column_type"] == PrimitiveType.NUMBER
and search.isnumeric()
and Operator.EQUAL in schema.get("filter_operators", [])
):
if schema["column_type"] == PrimitiveType.NUMBER and search.isnumeric():
try:
value = int(search)
except ValueError:
value = float(search)
return ConditionTreeLeaf(field, Operator.EQUAL, value)

if schema["column_type"] == PrimitiveType.ENUM and Operator.EQUAL in schema.get("filter_operators", []):
if schema["column_type"] == PrimitiveType.ENUM:
search_value = self.lenient_find(schema["enum_values"], search)
if search_value is not None:
return ConditionTreeLeaf(field, Operator.EQUAL, search_value)
Expand All @@ -103,17 +109,11 @@ def _build_condition(self, field: str, schema: Column, search: str) -> Union[Con
operator = Operator.CONTAINS
elif support_equal:
operator = Operator.EQUAL
else:
operator = None

if operator:
return ConditionTreeLeaf(field, operator, search)

if (
schema["column_type"] == PrimitiveType.UUID
and is_valid_uuid(search)
and Operator.EQUAL in schema.get("filter_operators", [])
):
if schema["column_type"] == PrimitiveType.UUID and is_valid_uuid(search):
return ConditionTreeLeaf(field, Operator.EQUAL, search)

def lenient_find(self, haystack: List[str], needle: str) -> Union[str, None]:
Expand All @@ -126,14 +126,14 @@ def _get_searchable_fields(self, collection: Collection, extended: bool) -> List
fields: List[Tuple[str, ColumnAlias]] = []

for name, field in collection.schema["fields"].items():
if is_column(field):
if is_column(field) and self._is_searchable_field(field):
fields.append((name, field))

if extended and (is_many_to_one(field) or is_one_to_one(field) or is_polymorphic_one_to_one(field)):
related = collection.datasource.get_collection(field["foreign_collection"])

for sub_name, sub_field in related.schema["fields"].items():
if is_column(sub_field):
if is_column(sub_field) and self._is_searchable_field(sub_field):
fields.append((f"{name}:{sub_name}", sub_field))

if extended and is_polymorphic_many_to_one(field):
Expand All @@ -145,3 +145,18 @@ def _get_searchable_fields(self, collection: Collection, extended: bool) -> List
)

return fields

def _is_searchable_field(self, field: Column) -> bool:
operators = field.get("filter_operators", [])

if field["column_type"] == PrimitiveType.STRING and (
Operator.CONTAINS in operators or Operator.EQUAL in operators
):
return True

if field["column_type"] in [PrimitiveType.NUMBER, PrimitiveType.UUID, PrimitiveType.ENUM] and (
Operator.EQUAL in operators
):
return True

return False
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,39 @@ def setUpClass(cls) -> None:
def setUp(self) -> None:
self.datasource: Datasource = Datasource()
self.collection_person = Collection("Person", self.datasource)
self.collection_person.add_fields(
{
"id": {
"is_primary_key": True,
"type": FieldType.COLUMN,
"column_type": PrimitiveType.NUMBER,
"filter_operators": set([Operator.EQUAL, Operator.IN]),
}
}
)

self.no_searchable_fields_collection = Collection("NotSearchable", self.datasource)
self.no_searchable_fields_collection.add_fields(
{
"id": {
"is_primary_key": True,
"type": FieldType.COLUMN,
"column_type": PrimitiveType.NUMBER,
"filter_operators": set(),
}
}
)
self.datasource.add_collection(self.collection_person)
self.datasource.add_collection(self.no_searchable_fields_collection)

self.datasource_decorator = DatasourceDecorator(self.datasource, SearchCollectionDecorator)
self.decorated_collection_person = self.datasource_decorator.get_collection("Person")
self.decorated_collection_person: SearchCollectionDecorator = self.datasource_decorator.get_collection(
"Person"
) # type:ignore

self.decorated_not_searchable_collection: SearchCollectionDecorator = self.datasource_decorator.get_collection(
"NotSearchable"
) # type:ignore

def test_replace_search_should_work(self):
def replacer(search: Any, search_extended: bool, context: CollectionCustomizationContext):
Expand All @@ -64,9 +93,28 @@ def replacer(search: Any, search_extended: bool, context: CollectionCustomizatio
self.decorated_collection_person.replace_search(replacer)
assert self.decorated_collection_person._replacer == replacer

def test_schema_is_searchable_should_be_true(self):
def test_schema_is_searchable_should_be_true_by_default_when_fields_can_be_searched(self):
assert self.decorated_collection_person.schema["searchable"] is True

def test_schema_is_searchable_should_be_false_when_no_fields_can_be_searched(self):
assert self.decorated_not_searchable_collection.schema["searchable"] is False

def test_schema_conflict_on_replace_and_disable_apply_the_latest_one(self):
self.decorated_collection_person.mark_schema_as_dirty()
assert self.decorated_collection_person.schema["searchable"] is True

self.decorated_collection_person.disable_search()
self.decorated_collection_person.mark_schema_as_dirty()
assert self.decorated_collection_person.schema["searchable"] is False

self.decorated_collection_person.replace_search(None)
self.decorated_collection_person.mark_schema_as_dirty()
assert self.decorated_collection_person.schema["searchable"] is True

def test_schema_is_searchable_should_be_false_when_disabling_search(self):
self.decorated_collection_person.disable_search()
assert self.decorated_collection_person.schema["searchable"] is False

def test_refine_filter_should_return_the_given_filter_for_empty_filter(self):
filter_ = Filter({"search": None})

Expand Down Expand Up @@ -169,6 +217,7 @@ def test_search_must_be_applied_on_all_fields(self):
"condition_tree": ConditionTreeBranch(
Aggregator.OR,
conditions=[
ConditionTreeLeaf("id", Operator.EQUAL, 1584),
ConditionTreeLeaf("number", Operator.EQUAL, 1584),
ConditionTreeLeaf("label", Operator.CONTAINS, "1584"),
],
Expand Down Expand Up @@ -206,11 +255,11 @@ def test_for_enum_value(self):
def test_search_number_in_all_field(self):
self.collection_person.add_field(
"field1",
Column(column_type=PrimitiveType.NUMBER, filter_operators=[Operator.EQUAL], type=FieldType.COLUMN),
Column(column_type=PrimitiveType.NUMBER, filter_operators=set([Operator.EQUAL]), type=FieldType.COLUMN),
)
self.collection_person.add_field(
"field2",
Column(column_type=PrimitiveType.NUMBER, filter_operators=[Operator.EQUAL], type=FieldType.COLUMN),
Column(column_type=PrimitiveType.NUMBER, filter_operators=set([Operator.EQUAL]), type=FieldType.COLUMN),
)

self.collection_person.add_field(
Expand All @@ -230,6 +279,7 @@ def test_search_number_in_all_field(self):
"condition_tree": ConditionTreeBranch(
Aggregator.OR,
conditions=[
ConditionTreeLeaf("id", Operator.EQUAL, 1584),
ConditionTreeLeaf("field1", Operator.EQUAL, 1584),
ConditionTreeLeaf("field2", Operator.EQUAL, 1584),
],
Expand Down Expand Up @@ -572,3 +622,13 @@ async def replacer_fn(value, extended, context):
filter_ = Filter({"search": "something", "search_extended": True})
self.loop.run_until_complete(self.decorated_collection_person._refine_filter(self.mocked_caller, filter_))
spy_replacer_fn.assert_awaited_with("something", True, ANY)

def test_disable_search_should_mark_schema_as_dirty(self):
with patch.object(self.decorated_collection_person, "mark_schema_as_dirty") as mark_schema_as_dirty:
self.decorated_collection_person.disable_search()
mark_schema_as_dirty.assert_called_once()

def test_replace_search_should_mark_schema_as_dirty(self):
with patch.object(self.decorated_collection_person, "mark_schema_as_dirty") as mark_schema_as_dirty:
self.decorated_collection_person.replace_search(None)
mark_schema_as_dirty.assert_called_once()

0 comments on commit fa45ab7

Please sign in to comment.