Skip to content

Commit

Permalink
Admin interface: Change display name (LAION-AI#2849) (LAION-AI#2908)
Browse files Browse the repository at this point in the history
### Hello 馃憢,

I hope you're doing great! I'm delighted to submit this pull request for
enhancing the admin interface LAION-AI#2849 by adding the ability to change the
display name of users.

## 馃専 Changes Made
Below is a summary of the changes I've made in this PR:

### FRONTEND:
---
- ```[id].tsx``` - remove ```isDisabled``` from FormControl.
- ```oasst_api_client.ts``` - change API PUT url in
```set_user_status``` method to accept ```display_name``` parameter.
- ```update_user.tsx``` add ```display_name``` parameter.

### BACKEND:
---
- ```user.py``` -> update ```update_user``` function signature *(added
```display_name: Optional[str] = None```)
- ```user_repository.py``` update ```update_user``` function signature
*(added ```display_name: Optional[str] = None```) and add check in if
statment.


## 馃 Your Feedback is Valued
I appreciate your time in reviewing this PR, and I'm looking forward to
any feedback or suggestions you might have. If you have any questions or
need further clarification, please don't hesitate to reach out. I'll be
more than happy to assist.

Thank you once again for considering this contribution to the project!
Let's continue working together to make our admin interface even better!
馃槉

Best regards,

datatab.
  • Loading branch information
DeanChugall authored and grgau committed May 8, 2023
1 parent f8146df commit b156b28
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 7 deletions.
3 changes: 2 additions & 1 deletion backend/oasst_backend/api/v1/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ def get_user(
@router.put("/{user_id}", status_code=HTTP_204_NO_CONTENT)
def update_user(
user_id: UUID,
display_name: Optional[str] = None,
enabled: Optional[bool] = None,
notes: Optional[str] = None,
show_on_leaderboard: Optional[bool] = None,
Expand All @@ -199,7 +200,7 @@ def update_user(
Update a user by global user ID. Only trusted clients can update users.
"""
ur = UserRepository(db, api_client)
ur.update_user(user_id, enabled, notes, show_on_leaderboard, tos_acceptance)
ur.update_user(user_id, display_name, enabled, notes, show_on_leaderboard, tos_acceptance)


@router.delete("/{user_id}", status_code=HTTP_204_NO_CONTENT)
Expand Down
5 changes: 4 additions & 1 deletion backend/oasst_backend/user_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def query_frontend_user(
def update_user(
self,
id: UUID,
display_name: Optional[str] = None,
enabled: Optional[bool] = None,
notes: Optional[str] = None,
show_on_leaderboard: Optional[bool] = None,
Expand All @@ -86,7 +87,6 @@ def update_user(
raise OasstError("Forbidden", OasstErrorCode.API_CLIENT_NOT_AUTHORIZED, HTTP_403_FORBIDDEN)

user: User = self.db.query(User).filter(User.id == id).first()

if user is None:
raise OasstError("User not found", OasstErrorCode.USER_NOT_FOUND, HTTP_404_NOT_FOUND)

Expand All @@ -98,8 +98,11 @@ def update_user(
user.show_on_leaderboard = show_on_leaderboard
if tos_acceptance:
user.tos_acceptance_date = utcnow()
if display_name is not None:
user.display_name = display_name

self.db.add(user)

return user

@managed_tx_method(CommitMode.COMMIT)
Expand Down
5 changes: 3 additions & 2 deletions website/src/lib/oasst_api_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,11 @@ export class OasstApiClient {
user_id: string,
is_enabled: boolean,
notes: string,
show_on_leaderboard: boolean
show_on_leaderboard: boolean,
display_name: string
): Promise<void> {
await this.put(
`/api/v1/users/${user_id}?enabled=${is_enabled}&notes=${notes}&show_on_leaderboard=${show_on_leaderboard}`
`/api/v1/users/${user_id}?enabled=${is_enabled}&notes=${notes}&show_on_leaderboard=${show_on_leaderboard}&display_name=${display_name}`
);
}

Expand Down
2 changes: 1 addition & 1 deletion website/src/pages/admin/manage_user/[id].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const ManageUser = ({ user }: InferGetServerSidePropsType<typeof getServerSidePr
<input type="hidden" {...register("auth_method")}></input>
<FormControl>
<FormLabel>Display Name</FormLabel>
<Input {...register("display_name")} isDisabled />
<Input {...register("display_name")} />
</FormControl>
<FormControl mt="2">
<FormLabel>Role</FormLabel>
Expand Down
4 changes: 2 additions & 2 deletions website/src/pages/api/admin/update_user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { getFrontendUserIdForUser } from "src/lib/users";
*/
const handler = withAnyRole(["admin", "moderator"], async (req, res, token) => {
// id is the 'username' from python backend, user_id is 'id' from the backend
const { id, user_id, notes, role, show_on_leaderboard, auth_method } = req.body;
const { id, user_id, notes, role, show_on_leaderboard, display_name, auth_method } = req.body;

// mod can't update user role to mod or admin
if (token.role === ROLES.MODERATOR && (role === ROLES.MODERATOR || role === ROLES.ADMIN)) {
Expand All @@ -28,7 +28,7 @@ const handler = withAnyRole(["admin", "moderator"], async (req, res, token) => {
});

// Tell the backend the user's enabled or not enabled status.
await oasstApiClient.set_user_status(user_id, role !== "banned", notes, show_on_leaderboard);
await oasstApiClient.set_user_status(user_id, role !== "banned", notes, show_on_leaderboard, display_name);

res.status(200).json({});
});
Expand Down

0 comments on commit b156b28

Please sign in to comment.