feat: add owner/name filter to list clients #2637
Conversation
client/client.go
Outdated
| @@ -200,6 +200,13 @@ type Client struct { | |||
| Metadata sqlxx.JSONRawMessage `json:"metadata,omitempty" db:"metadata"` | |||
| } | |||
|
|
|||
| type ClientFilters struct { | |||
There was a problem hiding this comment.
This needs to be added to the swagger spec:
Line 90 in b597c88
You can also probably re-use / re-purpose that struct as ClientFilter
There was a problem hiding this comment.
I added the name/owner params to that struct, is that all that needs done?
There was a problem hiding this comment.
Yes, that's all that's needed - I think this can be removed then!
There was a problem hiding this comment.
The ClientFilters struct is used internally to get passed to the client manager, so new filers could be easily added...
| if filters.Name != "" { | ||
| query.Where("client_name = ?", filters.Name) | ||
| } | ||
| if filters.Owner != "" { | ||
| query.Where("owner = ?", filters.Owner) | ||
| } |
There was a problem hiding this comment.
This probably means that we need to index these fields? Otherwise this will get very slow when you have lots of clients. Then again, if we just have like 1000 clients, it probably wouldn't matter too much (maybe 5ms with index, 200ms without). What do you think?
There was a problem hiding this comment.
For our use case, the index won't make much of a difference
There was a problem hiding this comment.
Ok, we can keep the filter omitted for now and add it later.
Codecov Report
@@ Coverage Diff @@
## master #2637 +/- ##
==========================================
+ Coverage 52.47% 52.54% +0.07%
==========================================
Files 234 234
Lines 13905 13924 +19
==========================================
+ Hits 7296 7317 +21
+ Misses 5986 5985 -1
+ Partials 623 622 -1
Continue to review full report at Codecov.
|
| if filters.Name != "" { | ||
| query.Where("client_name = ?", filters.Name) | ||
| } | ||
| if filters.Owner != "" { | ||
| query.Where("owner = ?", filters.Owner) | ||
| } |
There was a problem hiding this comment.
Ok, we can keep the filter omitted for now and add it later.
client/client.go
Outdated
| @@ -200,6 +200,13 @@ type Client struct { | |||
| Metadata sqlxx.JSONRawMessage `json:"metadata,omitempty" db:"metadata"` | |||
| } | |||
|
|
|||
| type ClientFilters struct { | |||
There was a problem hiding this comment.
Yes, that's all that's needed - I think this can be removed then!
|
This is what I had in mind :) |
This adds client name and owner filters to the client list endpoint.
Related issue(s)
#1485
Checklist
contributing code guidelines.
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
Further Comments
Still need to update documentation, looking for any initial feedback first