feat: Support Or and nested queries #13
Conversation
CeeEffEff
requested review from
gonzaatcroud,
marian-stykhun and
paul-watkins-croud
marian-stykhun
left a comment
marian-stykhun
left a comment
There was a problem hiding this comment.
Thank you, Few minor changes
| op: Optional[str] = None, | ||
| value: Optional[Any] = None, | ||
| filter: Optional[FieldFilter] = None, | ||
| filter: Union[FieldFilter, And, Or, None] = None, |
There was a problem hiding this comment.
Not critical
May we use OptionalUnion[FieldFilter, And, Or] ?
As None is not a type. and using Union[type, None] is outdated
There was a problem hiding this comment.
Optional is just shorthand for Union[x, None].
In this case it wouldn't make things much shorter as you end up with Optional[Union[x]].
It's not deprecated either to use Union
There was a problem hiding this comment.
It wouldn't
Thats why it isnt critical.
But even python's creators decided that it is bad design :)
| if filter is not None: | ||
| field, op, value = filter.field_path, filter.op_string, filter.value | ||
| if field is None or op is None or value is None: | ||
| if filter is None and (field is None or op is None or value is None): |
There was a problem hiding this comment.
filter is None and not any([field, op, value])?
Or we need i direct check is None ?
There was a problem hiding this comment.
Yes as we cannot rely on the truthiness of value.
We are explicitly checking for if the params have not been provided.
2 participants
In our tools we are requiring more complex queries than single FieldFilters, or a group joined in a kind of "And" behaviour.
Because Firestore supports more complex queries than this, and we are using them, this PR updates mockfirestore so that we can effortlessly use these nice features without having to craft convoluted tests.
Of note is that I am supporting the native Firestore objects, not creating any new kinds of query objects.
I've added a test to show the kind of usage now supported.