Allow multiple values in GraphqlFilter for a logical OR

[Mark90]

The filterBy parameter is defined in graphql resolvers as filter_by: list[GraphqlFilter] | None = None with the following implementation

@strawberry.experimental.pydantic.input(model=Filter)
class GraphqlFilter:
    field: str = strawberry.field(description="Field to filter on")
    value: str = strawberry.field(description="Value to sort the field on")

For example: filterBy: [{field: "product", value: "FW"}, {field: "status": "active"}] filters FW products that have status active.

If we want to filter multiple products, i.e. we want to have FW OR IP products with the status active, this is done through filterBy: [{field: "product", value: "FW-IP"}, {field: "status": "active"}] where - acts as a value separator. I believe this was inherited from the REST endpoint; it isn't great design as leaks implementation details and it prevents searching on values with - in the name.

It would be nice if we can do filterBy: [{field: "product", value: ["FW", "IP"]}, {field: "status": "active"}].

Tasks:

• Make GraphqlFilter accept both a str and list[str]
• Passing values as a list[str] should do a logical-OR search
• Don't immediately remove the - value separator! Deprecate it and raise a warning. Create follow-up ticket to remove in a future minor release.

[dmarjenburgh]

Separating on - has a number of other problems. Dates, UUIDs and negative numbers naturally have - characters which makes it almost impossible to write generic filtering logic using this disjunction/OR.

I have already added splitting on |, which is a better option so - can already be deprecated. Using | is also consistent with the logical OR meaning in the query parameter.

If we change the type of filterBy, the "value" will become a union of str and list of str. I don't mind, but it makes handling the typechecker a bit of a pain. Maybe we should wait and see of the current splitting on | yields any practical issues?

[Mark90]

Yeah | might be sufficient indeed.

But since the query parameter is for the frontend to spew user search queries to, filterBy is specifically for backends which can programmatically create the filtering params - so it's a bit weird that they have to create a 'query-like' filtering string, only for it to be split again.

My str | list[str] union suggestion is indeed not appreciated by strawberry. :)

Another option: add a parameter values: list[str] on the Filter type, which can co-exist with the current value: str. It seems to work.. but do we like it?

# db/filters/filters.py
class Filter(BaseModel):
    field: str
    value: str
    values: list[str] | None = None

...

def _filter_to_node(filter_item: Filter) -> Node:
    ...
    values = filter_item.values or _re_split.split(filter_item.value) if should_split else filter_item.value.split("|")
    ...

# graphql/types.py
@strawberry.experimental.pydantic.input(model=Filter)
class GraphqlFilter:
    field: str = strawberry.field(description="Field to filter on")
    value: str = strawberry.field(description="Value to filter by")
    values: list[str] | None = strawberry.field(description="Values to filter by", default=None)