-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
673 enhanced admin management #701
Conversation
}, | ||
}); | ||
const messages = await messagesRes.json(); | ||
const messages = await oasstApiClient.fetch_user_messages(user as string); |
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.
string array? or Message[]
?
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.
Added the explicit type.
const local_user_map = local_users.reduce((result, user) => { | ||
result.set(user.id, user.role); | ||
return result; | ||
}, new Map()); | ||
|
||
const users = all_users.map((user) => { | ||
const role = local_user_map.get(user.id) || "general"; | ||
return { | ||
...user, | ||
role, | ||
}; |
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.
will this happen every time you open the users dashboard?
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.
Yes, it's not ideal and not something I want to keep for long. I think the real fix is to push the role information to the backend but that would require a variety of new routes to make that fetching and setting easier.
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.
Front-end change approved!
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.
lgtm, thank you
Merging now that I see approval from all required people. |
Applies to #673
Updates the admin views to user the backend's User API routes:
What's not supported yet: