Skip to content

fix(teams): accept username in edit member endpoint#5852

Merged
aecsocket merged 7 commits intomodrinth:mainfrom
Sychic:fix/edit-teams-username
Apr 21, 2026
Merged

fix(teams): accept username in edit member endpoint#5852
aecsocket merged 7 commits intomodrinth:mainfrom
Sychic:fix/edit-teams-username

Conversation

@Sychic
Copy link
Copy Markdown
Contributor

@Sychic Sychic commented Apr 18, 2026

Previously, all inputs to the id/username argument were treated as ids regardless of whether they were or not. Now the input is resolved to a user to obtain a user id before proceeding.

Closes #2568

@modrinth-bot
Copy link
Copy Markdown
Member

Pull request changelog

App

Added

Changed

Deprecated

Removed

Fixed

Security

Website

Added

Changed

Deprecated

Removed

Fixed

Security

Hosting

Added

Changed

Deprecated

Removed

Fixed

Security

@Prospector Prospector requested a review from a team April 19, 2026 05:40
@Sychic Sychic force-pushed the fix/edit-teams-username branch from 7ca62ac to 6b7a9f5 Compare April 19, 2026 11:13
Comment thread apps/labrinth/src/routes/v3/teams.rs Outdated
Comment on lines +703 to +708
.ok_or_else(|| {
ApiError::InvalidInput(
"The user specified does not exist".to_string(),
)
})?
.id;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use .wrap_request_err() from crate::util::error::Context

Suggested change
.ok_or_else(|| {
ApiError::InvalidInput(
"The user specified does not exist".to_string(),
)
})?
.id;
.wrap_request_err("specified user does not exist")?
.id;

@Sychic Sychic force-pushed the fix/edit-teams-username branch from 6b7a9f5 to 1a83de2 Compare April 19, 2026 17:18
@Sychic Sychic requested a review from aecsocket April 19, 2026 17:37
Comment thread apps/labrinth/src/routes/v3/teams.rs Outdated
let user_id = ids.1.into();

let user_id = DBUser::get(&ids.1, &**pool, &redis)
.await?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: database ops should have a .wrap_internal_err("failed to fetch ...")

Comment thread apps/labrinth/src/routes/v3/teams.rs Outdated
)
})?;
let team_association = DBTeam::get_association(id, &**pool)
.await?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: database ops should have a .wrap_internal_err("failed to fetch ...")

Comment thread apps/labrinth/src/routes/v3/teams.rs Outdated
.await?;
let edit_member_db =
DBTeamMember::get_from_user_id_pending(id, user_id, &**pool)
.await?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: database ops should have a .wrap_internal_err("failed to fetch ...")

@aecsocket aecsocket enabled auto-merge April 21, 2026 10:43
auto-merge was automatically disabled April 21, 2026 13:07

Head branch was pushed to by a user without write access

@aecsocket aecsocket enabled auto-merge April 21, 2026 13:29
@aecsocket aecsocket added this pull request to the merge queue Apr 21, 2026
Merged via the queue into modrinth:main with commit 77e4c41 Apr 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

PATCH to /team/:id/members/:user fails when used with username rather than ID

3 participants