-
Notifications
You must be signed in to change notification settings - Fork 649
Add an API route for admins to assess crates #11528
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
base: main
Are you sure you want to change the base?
Conversation
Connects to #10387. This API route, under `/private/` rather than `v1`, will make it a lot easier to assess whether crates contain useful functionality or are squatting. This is only the backend; some of the functionality described in the issue will be frontend-only, but I wanted to get the API in first. The API route will only return data if the currently authenticated user is an admin. This is important because the crate owner's verified email address is part of the returned data so that the admin can contact the owner if necessary. Another reason this route is limited to admins is that some of the queries may be slow.
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, just a few minor comments :)
Path(username): Path<String>, | ||
req: Parts, | ||
) -> AppResult<Json<AdminListResponse>> { | ||
let mut conn = state.db_write().await?; |
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.
do we need the write access for this endpoint? I think it's read-only or am I missing something?
AdminCrateInfo { | ||
name, | ||
updated_at, | ||
num_rev_deps, | ||
num_versions: versions.map(|v| v.len()).unwrap_or(0), | ||
crate_size: last_version.map(|v| v.crate_size).unwrap_or(0), | ||
bin_names: last_version | ||
.map(|v| v.bin_names.clone()) | ||
.unwrap_or_default(), | ||
} |
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.
based on the SQL query I'm usually running in these situations, I recommend to also include:
- crate description
- default version (could probably be included in the
let crates
query) - crate yanked or not (same as default_version yanked or not)
- total crate downloads
}) | ||
.collect(); | ||
Ok(Json(AdminListResponse { | ||
user_email: verified.then_some(email).flatten(), |
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.
maybe we should have user_email
and user_email_verified
fields?
let versions: Vec<Version> = versions::table | ||
.filter(versions::crate_id.eq_any(crate_ids)) | ||
.select(Version::as_select()) | ||
.load(&mut conn) | ||
.await?; | ||
let mut versions_by_crate_id: HashMap<i32, Vec<Version>> = HashMap::new(); | ||
for version in versions { | ||
let crate_versions = versions_by_crate_id.entry(version.crate_id).or_default(); | ||
crate_versions.push(version); | ||
} |
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.
if we use the default version instead of the last version then we only need the versions list for the "number of versions" field and that information is also available in the default_versions
table. in other words it could be folded in the crates query above.
Connects to #10387.
This API route, under
/private/
rather thanv1
, will make it a lot easier to assess whether crates contain useful functionality or are squatting.This is only the backend; some of the functionality described in the issue will be frontend-only, but I wanted to get the API in first.
The API route will only return data if the currently authenticated user is an admin. This is important because the crate owner's verified email address is part of the returned data so that the admin can contact the owner if necessary.
Another reason this route is limited to admins is that some of the queries may be slow.