-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
allow filtering with array of contract states #920
Conversation
- register a custome Encoder/Decoder for in the schema pkg for []string param - change the type of ContractFilter to be []string - modify the sql query/ mock tests to check the existing of the state in an array instead of exact equal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some questions
grid-proxy/pkg/client/grid_client.go
Outdated
if value.Kind() == reflect.Slice && value.Type().Elem().Kind() == reflect.String { | ||
slice := value.Interface().([]string) | ||
return strings.Join(slice, ",") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious why we are checking value type and its elements type, doesn't the first value in RegisterEncoder
guarantee only []string will be passed?
Also, wouldn't something like slice, ok := slice := value.Interface().([]string)
work instead of the multiple reflect calls?
@@ -96,7 +97,7 @@ func (c *RentContract) satisfies(f types.ContractFilter) bool { | |||
return false | |||
} | |||
|
|||
if f.State != nil && *f.State != c.State { | |||
if f.State != nil && len(f.State) != 0 && !containsState(f.State, c.State) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think neither f.State != nil
or len(f.State) != 0
is needed, containsState
should be enough
@@ -783,8 +783,13 @@ func (d *PostgresDatabase) GetContracts(ctx context.Context, filter types.Contra | |||
if filter.Type != nil { | |||
q = q.Where("type = ?", *filter.Type) | |||
} | |||
if filter.State != nil { | |||
q = q.Where("state ILIKE ?", *filter.State) | |||
if filter.State != nil && len(filter.State) != 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to check for nil len
works fine with nil
…-sdk-go into development_proxy_array_filtering
Description
allow filtering with array of contract states
Changes
Related Issues
Checklist