Allow to filter user by display_name
#821
Conversation
notmd
requested review from
fozziethebeat,
k-nearest-neighbor,
AbdBarho,
yk and
andreaskoepf
as code owners
|
I like the look of the new table! |
| @@ -161,10 +161,10 @@ def query_users( | |||
| users = users.order_by(User.display_name) | |||
|
|
|||
| if gt: | |||
| users = users.filter(User.display_name > gt) | |||
| users = users.filter(User.id > gt) | |||
There was a problem hiding this comment.
Not sure what the intention here is, but it doesn't look right.
There was a problem hiding this comment.
I thought we want to cursor based on the user id?
There was a problem hiding this comment.
Is the user id column monotonically increasing?
There was a problem hiding this comment.
UUID V4 isn't guaranteed to be monotonically increasing. I think only UUID V6 or V7 have that property. V4 is random in it's ordering.
| {header.isPlaceholder ? null : flexRender(header.column.columnDef.header, header.getContext())} | ||
| {(header.column.columnDef as DataTableColumnDef<T>).filterable && ( | ||
| <FilterModal | ||
| value={filterValues.find((value) => value.id === header.id)?.value ?? ""} |
There was a problem hiding this comment.
If filterValues was an object and not an array could we avoid using find and findIndex?
There was a problem hiding this comment.
The filter API is temporary for now. It will change significantly in the next PR. I want to have something like MUI Grid. It will be an array to support multiple filters in 1 column.
There was a problem hiding this comment.
Thank you,
I was mainly referring to a suggestion to use object for items and not an array which would enable memorable look up of filters.
const filters = {
linkOperator: 'is',
items: {
1: {
id: 1,
columnField: 'rating',
operatorValue: '>',
value: '4',
},
2: {
id: 2,
columnField: 'isAdmin',
operatorValue: 'is',
value: 'true',
},
},
}
const getFilterById = (id) => filters.items[id]
| @@ -207,6 +207,7 @@ def query_users_ordered_by_display_name( | |||
| limit: Optional[int] = 100, | |||
| desc: bool = False, | |||
| ) -> list[User]: | |||
|
|
|||
There was a problem hiding this comment.
this newline triggers my review as code owner. it's a very beautiful newline. I approve ;)
close #766
DO NOT MERGE, depend on #851It's ready now.DataTablecomponent. I implement our ownfilterandpaginatelogic cause the built-in doesn't really useful when fetching data from server side and is inflexible.react-tablev8. I plan to migrate other components currently using v7 to v8 when I complete Admin Enhancement: Make it easier to find specific users #766.display_name.Use UUID based cursor.It now provided by the backend.Fix a bug when querying the user in the backward direction.Note that this PR doesn't support filtering users by prefix and theapi/v1/frontend_users/by_display_nameendpoint doesn't support pagination yet. I'm going to address this in the next PR.Here is the new UI