-
Notifications
You must be signed in to change notification settings - Fork 131
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
feat(admin): allow deleting package versions #1011
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
alter table package_version_dependencies | ||
drop constraint package_version_dependencies_package_scope_package_name_fkey; | ||
|
||
alter table package_version_dependencies | ||
add foreign key (package_scope, package_name) references packages ON UPDATE CASCADE ON DELETE CASCADE; | ||
|
||
|
||
alter table package_version_dependencies | ||
drop constraint package_version_dependencies_package_scope_package_name_pa_fkey; | ||
|
||
alter table package_version_dependencies | ||
add constraint package_version_dependencies_package_scope_package_name_pa_fkey | ||
foreign key (package_scope, package_name, package_version) references package_versions ON UPDATE CASCADE ON DELETE CASCADE; | ||
|
||
alter table package_files | ||
drop constraint package_files_scope_name_version_fkey; | ||
|
||
alter table package_files | ||
add foreign key (scope, name, version) references package_versions | ||
ON UPDATE CASCADE ON DELETE CASCADE; | ||
|
||
alter table npm_tarballs | ||
drop constraint npm_tarballs_scope_name_version_fkey; | ||
|
||
alter table npm_tarballs | ||
add foreign key (scope, name, version) references package_versions | ||
on UPDATE CASCADE ON DELETE CASCADE; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1649,6 +1649,26 @@ impl Database { | |
.await | ||
} | ||
|
||
#[instrument(name = "Database::yank_package_version", skip(self), err)] | ||
pub async fn delete_package_version( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/yank_package_version/delete_package_version There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think you should log or instrument the package name here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thats already done in the route handler function that calls this (same setup we have for all other db calls & endpoints) |
||
&self, | ||
scope: &ScopeName, | ||
name: &PackageName, | ||
version: &Version, | ||
) -> Result<()> { | ||
sqlx::query_as!( | ||
PackageVersion, | ||
r#"DELETE FROM package_versions WHERE scope = $1 AND name = $2 AND version = $3"#, | ||
scope as _, | ||
name as _, | ||
version as _ | ||
) | ||
.execute(&self.pool) | ||
.await?; | ||
|
||
Ok(()) | ||
} | ||
|
||
#[instrument(name = "Database::get_package_file", skip(self), err)] | ||
pub async fn get_package_file( | ||
&self, | ||
|
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.
this check is not correct, as it doesnt use the version as a requirement, and as such this check applies to all versions and is stricter than should be. This is good enough for now, however ideally this would build a graph and check if there are any other versions that fit the constraints used by dependents, and only if there is no such constraint would it error.