From c735f9868b6722a1af1d6ed4a6bedba2169e6f76 Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Sat, 1 Jan 2022 12:29:50 -0800 Subject: [PATCH] change(routers.packages): delete_package -> pkgbase_delete_instance `delete_package` was processing package deletions through `Package` instances. This doesn't make sense; if we delete a package, we want to target its package base. This new function vastly simplifies the previous. Signed-off-by: Kevin Morris --- aurweb/routers/packages.py | 69 +++++++++++------------------------- test/test_packages_routes.py | 4 +-- 2 files changed, 23 insertions(+), 50 deletions(-) diff --git a/aurweb/routers/packages.py b/aurweb/routers/packages.py index 9152ef9a..585c62ee 100644 --- a/aurweb/routers/packages.py +++ b/aurweb/routers/packages.py @@ -130,39 +130,18 @@ async def packages(request: Request) -> Response: return await packages_get(request, context) -def delete_package(request: Request, package: models.Package, - merge_into: models.PackageBase = None, - comments: str = str()): - bases_to_delete = [] +def pkgbase_delete_instance(request: Request, pkgbase: models.PackageBase, + comments: str = str()) \ + -> List[notify.Notification]: + notifs = handle_request(request, DELETION_ID, pkgbase) + [ + notify.DeleteNotification(request.user.ID, pkgbase.ID) + ] - target = db.query(models.PackageBase).filter( - models.PackageBase.Name == merge_into - ).first() - - notifs = [] - # In all cases, though, just delete the Package in question. - if package.PackageBase.packages.count() == 1: - notifs = handle_request(request, DELETION_ID, package.PackageBase, - target=target) - - bases_to_delete.append(package.PackageBase) - - with db.begin(): - update_closure_comment(package.PackageBase, DELETION_ID, comments, - target=target) - - # Prepare DeleteNotification. - notifs.append( - notify.DeleteNotification(request.user.ID, package.PackageBase.ID) - ) - - # Perform all the deletions. with db.begin(): - db.delete(package) - db.delete_all(bases_to_delete) + update_closure_comment(pkgbase, DELETION_ID, comments) + db.delete(pkgbase) - # Send out all the notifications. - util.apply_all(notifs, lambda n: n.send()) + return notifs async def make_single_context(request: Request, @@ -675,9 +654,8 @@ async def pkgbase_request_post(request: Request, name: str, logger.debug(f"New request #{pkgreq.ID} is marked for auto-orphan.") elif type == "deletion" and is_maintainer and outdated: # This request should be auto-accepted. - packages = pkgbase.packages.all() - for package in packages: - delete_package(request, package) + notifs = pkgbase_delete_instance(request, pkgbase, comments=comments) + util.apply_all(notifs, lambda n: n.send()) logger.debug(f"New request #{pkgreq.ID} is marked for auto-deletion.") # Redirect the submitting user to /packages. @@ -1054,11 +1032,8 @@ async def pkgbase_delete_post(request: Request, name: str, for pkgreq in requests: pkgreq.ClosureComment = comments - # Obtain deletion locks and delete the packages. - packages = pkgbase.packages.all() - for package in packages: - delete_package(request, package, comments=comments) - + notifs = pkgbase_delete_instance(request, pkgbase, comments=comments) + util.apply_all(notifs, lambda n: n.send()) return RedirectResponse("/packages", status_code=HTTPStatus.SEE_OTHER) @@ -1255,10 +1230,6 @@ async def packages_delete(request: Request, package_ids: List[int] = [], if not request.user.has_credential(creds.PKGBASE_DELETE): return (False, ["You do not have permission to delete packages."]) - # A "memo" used to store names of packages that we delete. - # We'll use this to log out a message about the deletions that occurred. - deleted_pkgs = [] - # set-ify package_ids and query the database for related records. package_ids = set(package_ids) packages = db.query(models.Package).filter( @@ -1270,16 +1241,18 @@ async def packages_delete(request: Request, package_ids: List[int] = [], # TODO: This error has not yet been translated. return (False, ["One of the packages you selected does not exist."]) - # Now let's actually walk through and delete all of the packages, - # using the same method we use in our /pkgbase/{name}/delete route. - for pkg in packages: - deleted_pkgs.append(pkg.Name) - delete_package(request, pkg) + # Make a set out of all package bases related to `packages`. + bases = {pkg.PackageBase for pkg in packages} + deleted_bases, notifs = [], [] + for pkgbase in bases: + deleted_bases.append(pkgbase.Name) + notifs += pkgbase_delete_instance(request, pkgbase) # Log out the fact that this happened for accountability. logger.info(f"Privileged user '{request.user.Username}' deleted the " - f"following packages: {str(deleted_pkgs)}.") + f"following package bases: {str(deleted_bases)}.") + util.apply_all(notifs, lambda n: n.send()) return (True, ["The selected packages have been deleted."]) # A mapping of action string -> callback functions used within the diff --git a/test/test_packages_routes.py b/test/test_packages_routes.py index 3a3415c5..ba724f19 100644 --- a/test/test_packages_routes.py +++ b/test/test_packages_routes.py @@ -2483,9 +2483,9 @@ def test_packages_post_delete(caplog: pytest.fixture, client: TestClient, assert successes[0].text.strip() == expected # Expect that the package deletion was logged. - packages = [package.Name] + pkgbases = [package.PackageBase.Name] expected = (f"Privileged user '{tu_user.Username}' deleted the " - f"following packages: {str(packages)}.") + f"following package bases: {str(pkgbases)}.") assert expected in caplog.text