From 31babbe67e7d96974a8ea9844f34095022b84b8a Mon Sep 17 00:00:00 2001 From: frcroth Date: Wed, 30 Oct 2024 09:55:58 +0100 Subject: [PATCH 1/8] Allow for the deletion of workflow reports --- app/controllers/VoxelyticsController.scala | 13 +++++++++++ app/models/voxelytics/VoxelyticsDAO.scala | 13 +++++++++++ conf/webknossos.latest.routes | 1 + frontend/javascripts/admin/admin_rest_api.ts | 6 +++++ .../admin/voxelytics/task_list_view.tsx | 22 ++++++++++++++++++- 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/app/controllers/VoxelyticsController.scala b/app/controllers/VoxelyticsController.scala index 9cd27ab9230..deebcdda537 100644 --- a/app/controllers/VoxelyticsController.scala +++ b/app/controllers/VoxelyticsController.scala @@ -3,6 +3,7 @@ package controllers import com.scalableminds.util.time.Instant import com.scalableminds.util.tools.{Fox, FoxImplicits} import models.organization.OrganizationDAO +import models.user.UserService import models.voxelytics._ import play.api.libs.json._ import play.api.mvc._ @@ -19,6 +20,7 @@ class VoxelyticsController @Inject()( organizationDAO: OrganizationDAO, voxelyticsDAO: VoxelyticsDAO, voxelyticsService: VoxelyticsService, + userService: UserService, lokiClient: LokiClient, wkConf: WkConf, sil: Silhouette[WkEnv])(implicit ec: ExecutionContext, bodyParsers: PlayBodyParsers) @@ -158,6 +160,17 @@ class VoxelyticsController @Inject()( } yield JsonOk(result) } + def deleteWorkflow(workflowHash: String): Action[AnyContent] = + sil.SecuredAction.async { implicit request => + for { + _ <- bool2Fox(wkConf.Features.voxelyticsEnabled) ?~> "voxelytics.disabled" + _ <- userService.assertIsSuperUser(request.identity) + _ <- voxelyticsDAO.findWorkflowByHash(workflowHash) ?~> "voxelytics.workflowNotFound" ~> NOT_FOUND + _ = logger.info(s"Deleting workflow with hash $workflowHash") + _ <- voxelyticsDAO.deleteWorkflow(workflowHash) + } yield Ok + } + def storeWorkflowEvents(workflowHash: String, runName: String): Action[List[WorkflowEvent]] = sil.SecuredAction.async(validateJson[List[WorkflowEvent]]) { implicit request => def createWorkflowEvent(runId: ObjectId, events: List[WorkflowEvent]): Fox[Unit] = diff --git a/app/models/voxelytics/VoxelyticsDAO.scala b/app/models/voxelytics/VoxelyticsDAO.scala index 799d72391fd..19be6137441 100644 --- a/app/models/voxelytics/VoxelyticsDAO.scala +++ b/app/models/voxelytics/VoxelyticsDAO.scala @@ -1135,4 +1135,17 @@ class VoxelyticsDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContex """.asUpdate) } yield () + def deleteWorkflow(hash: String): Fox[Unit] = + for { + _ <- run(q""" + DELETE FROM webknossos.voxelytics_workflows + WHERE hash = $hash; + """.asUpdate) + _ <- run(q""" + UPDATE webknossos.jobs + SET _voxelytics_workflowhash = NULL + WHERE _voxelytics_workflowhash = $hash; + """.asUpdate) + } yield () + } diff --git a/conf/webknossos.latest.routes b/conf/webknossos.latest.routes index a3ffbbbb158..ba3401eeeed 100644 --- a/conf/webknossos.latest.routes +++ b/conf/webknossos.latest.routes @@ -308,6 +308,7 @@ POST /verifyEmail POST /voxelytics/workflows controllers.VoxelyticsController.storeWorkflow() GET /voxelytics/workflows controllers.VoxelyticsController.listWorkflows() GET /voxelytics/workflows/:workflowHash controllers.VoxelyticsController.getWorkflow(workflowHash: String, runId: Option[String]) +DELETE /voxelytics/workflows/:workflowHash controllers.VoxelyticsController.deleteWorkflow(workflowHash: String) POST /voxelytics/workflows/:workflowHash/events controllers.VoxelyticsController.storeWorkflowEvents(workflowHash: String, runName: String) GET /voxelytics/workflows/:workflowHash/chunkStatistics controllers.VoxelyticsController.getChunkStatistics(workflowHash: String, runId: Option[String], taskName: String) GET /voxelytics/workflows/:workflowHash/artifactChecksums controllers.VoxelyticsController.getArtifactChecksums(workflowHash: String, runId: Option[String], taskName: String, artifactName: Option[String]) diff --git a/frontend/javascripts/admin/admin_rest_api.ts b/frontend/javascripts/admin/admin_rest_api.ts index 6c3b4ef8e78..83adbbc3dae 100644 --- a/frontend/javascripts/admin/admin_rest_api.ts +++ b/frontend/javascripts/admin/admin_rest_api.ts @@ -2501,6 +2501,12 @@ export function getVoxelyticsArtifactChecksums( ); } +export function deleteWorkflow(workflowHash: string): Promise { + return Request.triggerRequest(`/api/voxelytics/workflows/${workflowHash}`, { + method: "DELETE", + }); +} + // ### Help / Feedback userEmail export function sendHelpEmail(message: string) { return Request.receiveJSON( diff --git a/frontend/javascripts/admin/voxelytics/task_list_view.tsx b/frontend/javascripts/admin/voxelytics/task_list_view.tsx index a22d909d125..44000948ac9 100644 --- a/frontend/javascripts/admin/voxelytics/task_list_view.tsx +++ b/frontend/javascripts/admin/voxelytics/task_list_view.tsx @@ -50,7 +50,7 @@ import TaskView from "./task_view"; import { formatLog } from "./log_tab"; import { addAfterPadding, addBeforePadding } from "./utils"; import { LOG_LEVELS } from "oxalis/constants"; -import { getVoxelyticsLogs } from "admin/admin_rest_api"; +import { getVoxelyticsLogs, deleteWorkflow } from "admin/admin_rest_api"; import ArtifactsDiskUsageList from "./artifacts_disk_usage_list"; import { notEmpty } from "libs/utils"; import type { ArrayElement } from "types/globals"; @@ -421,6 +421,25 @@ export default function TaskListView({ } } + async function deleteWorkflowReport() { + await modal.confirm({ + title: "Delete Workflow Report", + content: "Are you sure you want to delete this workflow report?", + okText: "Delete", + okButtonProps: { danger: true }, + onOk: async () => { + try { + await deleteWorkflow(report.workflow.hash); + history.push("/workflows"); + message.success("Workflow report deleted."); + } catch (error) { + console.error(error); + message.error("Could not delete workflow report."); + } + }, + }); + } + const overflowMenu: MenuProps = { items: [ { key: "1", onClick: copyAllArtifactPaths, label: "Copy All Artifact Paths" }, @@ -440,6 +459,7 @@ export default function TaskListView({ ), }, { key: "5", onClick: showArtifactsDiskUsageList, label: "Show Disk Usage of Artifacts" }, + { key: "6", onClick: deleteWorkflowReport, label: "Delete Workflow Report" }, ], }; From 5ad996b03bebf399eb485acad2e1b6baa5da53ea Mon Sep 17 00:00:00 2001 From: frcroth Date: Wed, 30 Oct 2024 10:01:06 +0100 Subject: [PATCH 2/8] Update Changelog --- CHANGELOG.unreleased.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index f285b180b9e..6b0846e649e 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -18,6 +18,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released - Increased loading speed for precomputed meshes. [#8110](https://github.com/scalableminds/webknossos/pull/8110) - Unified wording in UI and code: “Magnification”/“mag” is now used in place of “Resolution“ most of the time, compare [https://docs.webknossos.org/webknossos/terminology.html](terminology document). [#8111](https://github.com/scalableminds/webknossos/pull/8111) - Added support for adding remote OME-Zarr NGFF version 0.5 datasets. [#8122](https://github.com/scalableminds/webknossos/pull/8122) +- Workflow reports may be deleted. [#8156](https://github.com/scalableminds/webknossos/pull/8156) ### Changed - Some mesh-related actions were disabled in proofreading-mode when using meshfiles that were created for a mapping rather than an oversegmentation. [#8091](https://github.com/scalableminds/webknossos/pull/8091) From ba82913a9e6ea2fde7cc140e04fd9bf4c964f3fb Mon Sep 17 00:00:00 2001 From: frcroth Date: Mon, 4 Nov 2024 10:02:26 +0100 Subject: [PATCH 3/8] Add note on deletion --- frontend/javascripts/admin/voxelytics/task_list_view.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/admin/voxelytics/task_list_view.tsx b/frontend/javascripts/admin/voxelytics/task_list_view.tsx index 44000948ac9..7e1c3a51138 100644 --- a/frontend/javascripts/admin/voxelytics/task_list_view.tsx +++ b/frontend/javascripts/admin/voxelytics/task_list_view.tsx @@ -424,7 +424,7 @@ export default function TaskListView({ async function deleteWorkflowReport() { await modal.confirm({ title: "Delete Workflow Report", - content: "Are you sure you want to delete this workflow report?", + content: "Are you sure you want to delete this workflow report?\nNote that if the workflow is still running, this may cause it to fail.", okText: "Delete", okButtonProps: { danger: true }, onOk: async () => { @@ -459,7 +459,7 @@ export default function TaskListView({ ), }, { key: "5", onClick: showArtifactsDiskUsageList, label: "Show Disk Usage of Artifacts" }, - { key: "6", onClick: deleteWorkflowReport, label: "Delete Workflow Report" }, + { key: "6", onClick: deleteWorkflowReport, label: "Delete Workflow Report"}, ], }; From c26f05dc2994cc07817d61283fc6171667c26c4f Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Mon, 4 Nov 2024 13:06:25 +0100 Subject: [PATCH 4/8] not tested: only show delete workflow report entry for superusers --- .../admin/voxelytics/task_list_view.tsx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/admin/voxelytics/task_list_view.tsx b/frontend/javascripts/admin/voxelytics/task_list_view.tsx index 7e1c3a51138..596f177c6f2 100644 --- a/frontend/javascripts/admin/voxelytics/task_list_view.tsx +++ b/frontend/javascripts/admin/voxelytics/task_list_view.tsx @@ -54,6 +54,8 @@ import { getVoxelyticsLogs, deleteWorkflow } from "admin/admin_rest_api"; import ArtifactsDiskUsageList from "./artifacts_disk_usage_list"; import { notEmpty } from "libs/utils"; import type { ArrayElement } from "types/globals"; +import { useSelector } from "react-redux"; +import type { OxalisState } from "oxalis/store"; const { Search } = Input; @@ -272,6 +274,11 @@ export default function TaskListView({ const highlightedTask = params.highlightedTask || ""; const location = useLocation(); + const isCurrentUserSuperUser = useSelector((state: OxalisState) => { + const activeUser = state.activeUser; + return activeUser?.isSuperUser; + }); + const singleRunId = report.runs.length === 1 ? report.runs[0].id : runId; useEffect(() => { @@ -424,7 +431,8 @@ export default function TaskListView({ async function deleteWorkflowReport() { await modal.confirm({ title: "Delete Workflow Report", - content: "Are you sure you want to delete this workflow report?\nNote that if the workflow is still running, this may cause it to fail.", + content: + "Are you sure you want to delete this workflow report?\nNote that if the workflow is still running, this may cause it to fail.", okText: "Delete", okButtonProps: { danger: true }, onOk: async () => { @@ -459,9 +467,14 @@ export default function TaskListView({ ), }, { key: "5", onClick: showArtifactsDiskUsageList, label: "Show Disk Usage of Artifacts" }, - { key: "6", onClick: deleteWorkflowReport, label: "Delete Workflow Report"}, ], }; + if (isCurrentUserSuperUser) + overflowMenu.items?.push({ + key: "6", + onClick: deleteWorkflowReport, + label: "Delete Workflow Report", + }); type ItemType = ArrayElement; From 856f9c6c1068f626c96c321bb19871233826554f Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Mon, 4 Nov 2024 13:09:14 +0100 Subject: [PATCH 5/8] improve syntax --- frontend/javascripts/admin/voxelytics/task_list_view.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/frontend/javascripts/admin/voxelytics/task_list_view.tsx b/frontend/javascripts/admin/voxelytics/task_list_view.tsx index 596f177c6f2..d82171c72dd 100644 --- a/frontend/javascripts/admin/voxelytics/task_list_view.tsx +++ b/frontend/javascripts/admin/voxelytics/task_list_view.tsx @@ -125,7 +125,7 @@ function TaskStateTag({ taskInfo }: { taskInfo: VoxelyticsTaskInfo }) { (taskInfo.chunkCounts.complete + taskInfo.chunkCounts.failed + taskInfo.chunkCounts.cancelled)) * - (taskInfo.chunkCounts.total - taskInfo.chunkCounts.skipped) - + (taskInfo.chunkCounts.total - taskInfo.chunkCounts.skipped) - currentDuration; const estimatedEndTime = new Date(Date.now() + estimatedRemainingDuration); return ( @@ -274,10 +274,7 @@ export default function TaskListView({ const highlightedTask = params.highlightedTask || ""; const location = useLocation(); - const isCurrentUserSuperUser = useSelector((state: OxalisState) => { - const activeUser = state.activeUser; - return activeUser?.isSuperUser; - }); + const isCurrentUserSuperUser = useSelector((state: OxalisState) => state.activeUser?.isSuperUser); const singleRunId = report.runs.length === 1 ? report.runs[0].id : runId; From 9f65900fa6409750d0c18e33ebb050bd11a50dde Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Mon, 4 Nov 2024 13:15:51 +0100 Subject: [PATCH 6/8] lint --- frontend/javascripts/admin/voxelytics/task_list_view.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/javascripts/admin/voxelytics/task_list_view.tsx b/frontend/javascripts/admin/voxelytics/task_list_view.tsx index d82171c72dd..da6a0202e8c 100644 --- a/frontend/javascripts/admin/voxelytics/task_list_view.tsx +++ b/frontend/javascripts/admin/voxelytics/task_list_view.tsx @@ -125,7 +125,7 @@ function TaskStateTag({ taskInfo }: { taskInfo: VoxelyticsTaskInfo }) { (taskInfo.chunkCounts.complete + taskInfo.chunkCounts.failed + taskInfo.chunkCounts.cancelled)) * - (taskInfo.chunkCounts.total - taskInfo.chunkCounts.skipped) - + (taskInfo.chunkCounts.total - taskInfo.chunkCounts.skipped) - currentDuration; const estimatedEndTime = new Date(Date.now() + estimatedRemainingDuration); return ( From 54037c7b760b1003abbabe8271d4ea818d423fff Mon Sep 17 00:00:00 2001 From: frcroth Date: Mon, 4 Nov 2024 16:05:28 +0100 Subject: [PATCH 7/8] Apply suggestions from code review --- CHANGELOG.unreleased.md | 2 +- app/controllers/VoxelyticsController.scala | 4 ++-- app/models/voxelytics/VoxelyticsDAO.scala | 9 +++++---- frontend/javascripts/admin/voxelytics/task_list_view.tsx | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 20491726540..eddc43730c4 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -19,7 +19,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released - Added a button to the search popover in the skeleton and segment tab to select all matching non-group results. [#8123](https://github.com/scalableminds/webknossos/pull/8123) - Unified wording in UI and code: “Magnification”/“mag” is now used in place of “Resolution“ most of the time, compare [https://docs.webknossos.org/webknossos/terminology.html](terminology document). [#8111](https://github.com/scalableminds/webknossos/pull/8111) - Added support for adding remote OME-Zarr NGFF version 0.5 datasets. [#8122](https://github.com/scalableminds/webknossos/pull/8122) -- Workflow reports may be deleted. [#8156](https://github.com/scalableminds/webknossos/pull/8156) +- Workflow reports may be deleted by superusers. [#8156](https://github.com/scalableminds/webknossos/pull/8156) ### Changed - Some mesh-related actions were disabled in proofreading-mode when using meshfiles that were created for a mapping rather than an oversegmentation. [#8091](https://github.com/scalableminds/webknossos/pull/8091) diff --git a/app/controllers/VoxelyticsController.scala b/app/controllers/VoxelyticsController.scala index deebcdda537..0400c317771 100644 --- a/app/controllers/VoxelyticsController.scala +++ b/app/controllers/VoxelyticsController.scala @@ -166,8 +166,8 @@ class VoxelyticsController @Inject()( _ <- bool2Fox(wkConf.Features.voxelyticsEnabled) ?~> "voxelytics.disabled" _ <- userService.assertIsSuperUser(request.identity) _ <- voxelyticsDAO.findWorkflowByHash(workflowHash) ?~> "voxelytics.workflowNotFound" ~> NOT_FOUND - _ = logger.info(s"Deleting workflow with hash $workflowHash") - _ <- voxelyticsDAO.deleteWorkflow(workflowHash) + _ = logger.info(s"Deleting workflow with hash $workflowHash in organization ${request.identity._organization}") + _ <- voxelyticsDAO.deleteWorkflow(workflowHash, request.identity._organization) } yield Ok } diff --git a/app/models/voxelytics/VoxelyticsDAO.scala b/app/models/voxelytics/VoxelyticsDAO.scala index 19be6137441..18511c45f54 100644 --- a/app/models/voxelytics/VoxelyticsDAO.scala +++ b/app/models/voxelytics/VoxelyticsDAO.scala @@ -1135,16 +1135,17 @@ class VoxelyticsDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContex """.asUpdate) } yield () - def deleteWorkflow(hash: String): Fox[Unit] = + def deleteWorkflow(hash: String, organizationId: String): Fox[Unit] = for { _ <- run(q""" DELETE FROM webknossos.voxelytics_workflows - WHERE hash = $hash; + WHERE hash = $hash + AND _organization = $organizationId; """.asUpdate) _ <- run(q""" UPDATE webknossos.jobs - SET _voxelytics_workflowhash = NULL - WHERE _voxelytics_workflowhash = $hash; + SET _voxelytics_workflowHash = NULL + WHERE _voxelytics_workflowHash = $hash; """.asUpdate) } yield () diff --git a/frontend/javascripts/admin/voxelytics/task_list_view.tsx b/frontend/javascripts/admin/voxelytics/task_list_view.tsx index da6a0202e8c..feecafa1149 100644 --- a/frontend/javascripts/admin/voxelytics/task_list_view.tsx +++ b/frontend/javascripts/admin/voxelytics/task_list_view.tsx @@ -429,7 +429,7 @@ export default function TaskListView({ await modal.confirm({ title: "Delete Workflow Report", content: - "Are you sure you want to delete this workflow report?\nNote that if the workflow is still running, this may cause it to fail.", + "Are you sure you want to delete this workflow report? This can not be undone. Note that if the workflow is still running, this may cause it to fail.", okText: "Delete", okButtonProps: { danger: true }, onOk: async () => { From 443b71cb665b472180a16882657272d549774f35 Mon Sep 17 00:00:00 2001 From: frcroth Date: Mon, 4 Nov 2024 16:23:29 +0100 Subject: [PATCH 8/8] Ensure org for setting hash to null in jobs table --- app/models/voxelytics/VoxelyticsDAO.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/voxelytics/VoxelyticsDAO.scala b/app/models/voxelytics/VoxelyticsDAO.scala index 18511c45f54..beffc0b3010 100644 --- a/app/models/voxelytics/VoxelyticsDAO.scala +++ b/app/models/voxelytics/VoxelyticsDAO.scala @@ -1145,7 +1145,8 @@ class VoxelyticsDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContex _ <- run(q""" UPDATE webknossos.jobs SET _voxelytics_workflowHash = NULL - WHERE _voxelytics_workflowHash = $hash; + WHERE _voxelytics_workflowHash = $hash + AND (SELECT _organization FROM webknossos.users AS u WHERE u._id = _owner) = $organizationId; """.asUpdate) } yield ()