From: Michael Tremer <michael.tremer@ipfire.org>
Date: Fri, 24 Jan 2025 16:52:16 +0000 (+0000)
Subject: builds: Fix deleting builds
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e6716b4b3de3cb6f3138858ab381fdd9587be21;p=pbs.git

builds: Fix deleting builds

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---

diff --git a/src/buildservice/builds.py b/src/buildservice/builds.py
index ae343b17..0d15051a 100644
--- a/src/buildservice/builds.py
+++ b/src/buildservice/builds.py
@@ -383,26 +383,28 @@ class Build(database.Base, database.BackendMixin, database.SoftDeleteMixin):
 		# You can't touch this
 		return False
 
-	async def delete(self, user=None):
+	async def delete(self, deleted_by):
 		"""
 			Deletes this build including all jobs,
 			packages and the source package.
 		"""
+		# Check if we can actually delete
+		if not self.can_be_deleted(deleted_by):
+			raise PermissionError
+
 		# Delete all test builds
-		if self.test_builds:
-			await self.test_builds.delete(user=user)
+		#if self.test_builds:
+		#	await self.test_builds.delete(user=user)
 
 		# Delete all jobs
-		for job in self._jobs:
-			await job.delete(user=user)
+		for job in self.alljobs:
+			await job.delete(deleted_by=deleted_by)
 
 		# Mark as deleted
-		self._set_attribute_now("deleted_at")
-		if user:
-			await self._set_attribute("deleted_by", user)
+		await super().delete(deleted_by=deleted_by)
 
 		# Delete source package
-		await self.pkg.delete(user=user)
+		await self.pkg.delete(deleted_by=deleted_by)
 
 		# All repositories this build has been in have been changed
 		await self._update_repos(build=True)
diff --git a/src/buildservice/jobs.py b/src/buildservice/jobs.py
index dcfc357c..c343c012 100644
--- a/src/buildservice/jobs.py
+++ b/src/buildservice/jobs.py
@@ -865,18 +865,16 @@ class Job(database.Base, database.BackendMixin, database.SoftDeleteMixin):
 
 	message = Column(Text, nullable=False, default="")
 
-	async def delete(self, user=None):
+	async def delete(self, deleted_by=None):
 		"""
 			Deletes a job
 		"""
 		# Delete all binary packages
 		for pkg in self.packages:
-			await pkg.delete(user=user)
+			await pkg.delete(deleted_by=deleted_by)
 
 		# Mark as deleted
-		self._set_attribute_now("deleted_at")
-		if user:
-			self._set_attribute("deleted_by", user)
+		await super().delete(deleted_by=deleted_by)
 
 		# Delete the log
 		await self._delete_log()
diff --git a/src/buildservice/packages.py b/src/buildservice/packages.py
index 8add22a9..c8dae350 100644
--- a/src/buildservice/packages.py
+++ b/src/buildservice/packages.py
@@ -289,7 +289,7 @@ class Package(database.Base, database.BackendMixin, database.SoftDeleteMixin):
 		DateTime(timezone=False), nullable=False, server_default=sqlalchemy.func.current_timestamp(),
 	)
 
-	async def delete(self, *args, **kwargs):
+	async def delete(self, deleted_by=None):
 		"""
 			Deletes this package
 		"""
@@ -301,7 +301,7 @@ class Package(database.Base, database.BackendMixin, database.SoftDeleteMixin):
 		log.info("Deleting package %s" % self)
 
 		# Delete from the database
-		await super().delete(*args, **kwargs)
+		await super().delete(deleted_by=deleted_by)
 
 		# Unlink the payload
 		if self.path:
diff --git a/src/templates/builds/delete.html b/src/templates/builds/delete.html
index 5e359663..a4010b2b 100644
--- a/src/templates/builds/delete.html
+++ b/src/templates/builds/delete.html
@@ -1,6 +1,6 @@
-{% extends "../modal.html" %}
+{% extends "modal.html" %}
 
-{% block title %}{{ _("Delete Build") }} - {{ build }}{% end block %}
+{% block title %}{{ _("Delete Build") }} - {{ build }}{% endblock %}
 
 {% block breadcrumbs %}
 	<nav class="breadcrumb" aria-label="breadcrumbs">
@@ -19,16 +19,16 @@
 			</li>
 		</ul>
 	</nav>
-{% end block %}
+{% endblock %}
 
 {% block modal_title %}
 	<h4 class="title is-4">{{ _("Delete Build") }}</h4>
 	<h6 class="subtitle is-6">{{ build }}</h6>
-{% end block %}
+{% endblock %}
 
 {% block modal %}
 	<form method="POST" action="">
-		{% raw xsrf_form_html() %}
+		{{ xsrf_form_html() | safe }}
 
 		<div class="content">
 			<p>
@@ -43,4 +43,4 @@
 			</button>
 		</div>
 	</form>
-{% end block %}
+{% endblock %}
diff --git a/src/web/builds.py b/src/web/builds.py
index cc97feea..6b3991fe 100644
--- a/src/web/builds.py
+++ b/src/web/builds.py
@@ -166,8 +166,8 @@ class CloneHandler(base.BaseHandler):
 
 class DeleteHandler(base.BaseHandler):
 	@base.authenticated
-	def get(self, uuid):
-		build = self.backend.builds.get_by_uuid(uuid)
+	async def get(self, uuid):
+		build = await self.backend.builds.get_by_uuid(uuid)
 		if not build:
 			raise tornado.web.HTTPError(404, "Could not find build %s" % uuid)
 
@@ -176,22 +176,19 @@ class DeleteHandler(base.BaseHandler):
 			raise tornado.web.HTTPError(403, "%s cannot delete build %s" \
 				% (self.current_user, build))
 
-		self.render("builds/delete.html", build=build)
+		await self.render("builds/delete.html", build=build)
 
 	@base.authenticated
 	async def post(self, uuid):
-		build = self.backend.builds.get_by_uuid(uuid)
+		build = await self.backend.builds.get_by_uuid(uuid)
 		if not build:
 			raise tornado.web.HTTPError(404, "Could not find build %s" % uuid)
 
-		# Check permissions
-		if not build.can_be_deleted(self.current_user):
-			raise tornado.web.HTTPError(403, "%s cannot delete build %s" \
-				% (self.current_user, build))
-
-		# Perform Deletion
-		with self.db.transaction():
-			await build.delete(self.current_user)
+		# Delete the build
+		try:
+			await build.delete(deleted_by=self.current_user)
+		except PermissionError as e:
+			raise tornado.web.HTTPError(403) from e
 
 		self.redirect("/builds")