feat: advanced filters for feedbacks and chats admin api #525
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@sszgwdk is attempting to deploy a commit to the pingcap Team on Vercel. A member of the Team first needs to authorize it. |
backend/app/api/admin_routes/user.py
Outdated
| router = APIRouter() | ||
|
|
||
|
|
||
| @router.get("/users") |
There was a problem hiding this comment.
This API is used only for admin users.
Move to /api/v1/admin/stats/chats/users
There was a problem hiding this comment.
I think /api/v1/admin/users is reasonable, the API is design for filter option list.
There was a problem hiding this comment.
Do I need to roll back the code?
There was a problem hiding this comment.
@Mini256 Only rollback this API is ok to me, this is the standard get users list api. The rest are stats api.
backend/app/api/routes/chat.py
Outdated
| class ChatOrigin(UUIDBaseModel): | ||
| origin: str | ||
|
|
||
| @router.get("/chats/origins") |
There was a problem hiding this comment.
This API is used only for admin users.
Move to /api/v1/admin/stats/chats/origins
| params=params, | ||
| ) | ||
|
|
||
| @router.get("/admin/feedbacks/origins") |
There was a problem hiding this comment.
Move to /api/v1/admin/stats/feedbacks/origins
634750802
changed the title
| ) -> Page[ChatOrigin]: | ||
| return paginate( | ||
| session, | ||
| select(Chat.origin, Chat.id).order_by(Chat.created_at.desc()), |
There was a problem hiding this comment.
I think we only need to return distinct Chat.origin values
| ) -> Page[User]: | ||
| return paginate( | ||
| session, | ||
| select(User).order_by(User.id), |
There was a problem hiding this comment.
How about:
/admin/users->/admin/users/searchlist_users->search_users
Maybe we need a search parameter for user search. The user list may be very long, and we need to consider how to help users find the user_id they need.
For the return type, you can using UserDescriptor instead to avoid return unnecessary or sensitive information to the frontend.
Add email: str to UserDescriptor:
| user: CurrentSuperuserDep, | ||
| params: Params = Depends(), | ||
| ) -> Page[ChatOrigin]: | ||
| return paginate( |
There was a problem hiding this comment.
BTW, I perfer to using ChatRepo to handle all of DB access operaction of Chat model, so that we can reuse it in other place.
| class ChatOrigin(UUIDBaseModel): | ||
| origin: str | ||
|
|
||
| @router.get("/admin/stats/chats/origins") |
There was a problem hiding this comment.
For the route path naming, you can share your points on the two naming methods /admin/stats/chats/origins vs /admin/chats/origins
design first, then code.
part of #389