From 003b14d44c74c915e6b834dea149520bf6b269e8 Mon Sep 17 00:00:00 2001 From: Florian M Date: Mon, 28 Nov 2022 13:03:36 +0100 Subject: [PATCH] Only allow taskTypeId as parameter in task creation (#6640) * Only allow taskTypeId as parameter in task creation * fix sample orga task type id * changelog, migration guide --- CHANGELOG.unreleased.md | 1 + MIGRATIONS.unreleased.md | 2 ++ app/controllers/InitialDataController.scala | 2 +- app/controllers/TaskController.scala | 11 +++---- app/models/task/TaskCreationParameters.scala | 31 ++++++++----------- app/models/task/TaskCreationService.scala | 19 +++++------- .../admin/task/task_create_bulk_view.tsx | 16 +++++----- .../admin/task/task_create_form_view.tsx | 4 +-- .../test/backend-snapshot-tests/tasks.e2e.ts | 4 +-- 9 files changed, 40 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index ca9fbe78342..a4e1b4aa9b8 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -13,6 +13,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released ### Added ### Changed +- Bulk task creation now needs the taskTypeId, the task type summary will no longer be accepted. [#6640](https://github.com/scalableminds/webknossos/pull/6640) ### Fixed diff --git a/MIGRATIONS.unreleased.md b/MIGRATIONS.unreleased.md index 21b79d4ce6f..a4b433692f6 100644 --- a/MIGRATIONS.unreleased.md +++ b/MIGRATIONS.unreleased.md @@ -8,4 +8,6 @@ User-facing changes are documented in the [changelog](CHANGELOG.released.md). ## Unreleased [Commits](https://github.com/scalableminds/webknossos/compare/22.12.0...HEAD) +- Bulk task creation now needs the taskTypeId, the task type summary will no longer be accepted. If you have scripts generating CSVs for bulk task creation, they should not output task type summaries. [#6640](https://github.com/scalableminds/webknossos/pull/6640) + ### Postgres Evolutions: diff --git a/app/controllers/InitialDataController.scala b/app/controllers/InitialDataController.scala index d7307e8239f..f6daa959a5c 100644 --- a/app/controllers/InitialDataController.scala +++ b/app/controllers/InitialDataController.scala @@ -190,7 +190,7 @@ Samplecountry taskTypeDAO.findAll.flatMap { types => if (types.isEmpty) { val taskType = TaskType( - ObjectId.generate, + ObjectId("63721e2cef0100470266c485"), organizationTeam._id, "sampleTaskType", "Check those cells out!" diff --git a/app/controllers/TaskController.scala b/app/controllers/TaskController.scala index 98fc3b0b442..3e502fb45f9 100755 --- a/app/controllers/TaskController.scala +++ b/app/controllers/TaskController.scala @@ -62,11 +62,8 @@ class TaskController @Inject()(taskCreationService: TaskCreationService, def create: Action[List[TaskParameters]] = sil.SecuredAction.async(validateJson[List[TaskParameters]]) { implicit request => for { - taskParameters <- Fox.serialCombined(request.body) { tp => - taskCreationService.normalizeTaskTypeId(tp, request.identity._organization) - } - _ <- taskCreationService.assertBatchLimit(request.body.length, taskParameters.map(_.taskTypeIdOrSummary)) - taskParameters <- taskCreationService.createTracingsFromBaseAnnotations(taskParameters, + _ <- taskCreationService.assertBatchLimit(request.body.length, request.body.map(_.taskTypeId)) + taskParameters <- taskCreationService.createTracingsFromBaseAnnotations(request.body, request.identity._organization) skeletonBaseOpts: List[Option[SkeletonTracing]] <- taskCreationService.createTaskSkeletonTracingBases( taskParameters) @@ -109,8 +106,8 @@ Expects: _ <- bool2Fox(inputFiles.nonEmpty) ?~> "nml.file.notFound" jsonString <- body.dataParts.get("formJSON").flatMap(_.headOption) ?~> "format.json.missing" params <- JsonHelper.parseJsonToFox[NmlTaskParameters](jsonString) ?~> "task.create.failed" - _ <- taskCreationService.assertBatchLimit(inputFiles.length, List(params.taskTypeIdOrSummary)) - taskTypeIdValidated <- ObjectId.fromString(params.taskTypeIdOrSummary) ?~> "taskType.id.invalid" + _ <- taskCreationService.assertBatchLimit(inputFiles.length, List(params.taskTypeId)) + taskTypeIdValidated <- ObjectId.fromString(params.taskTypeId) ?~> "taskType.id.invalid" taskType <- taskTypeDAO.findOne(taskTypeIdValidated) ?~> "taskType.notFound" ~> NOT_FOUND project <- projectDAO .findOneByNameAndOrganization(params.projectName, request.identity._organization) ?~> "project.notFound" ~> NOT_FOUND diff --git a/app/models/task/TaskCreationParameters.scala b/app/models/task/TaskCreationParameters.scala index fa781f4b134..c7fa691ba8d 100644 --- a/app/models/task/TaskCreationParameters.scala +++ b/app/models/task/TaskCreationParameters.scala @@ -4,29 +4,24 @@ import com.scalableminds.util.geometry.{BoundingBox, Vec3Int, Vec3Double} import models.user.Experience import play.api.libs.json.{Format, Json} -case class TaskParameters( - taskTypeIdOrSummary: String, - // The taskTypeIdOrSummary parameter may be an id or a summary. At the beginning of TaskController>create - // this is normalized so only the task id is stored in this field. - neededExperience: Experience, - openInstances: Int, - projectName: String, - scriptId: Option[String], - boundingBox: Option[BoundingBox], - dataSet: String, - editPosition: Vec3Int, - editRotation: Vec3Double, - creationInfo: Option[String], - description: Option[String], - baseAnnotation: Option[BaseAnnotation] -) +case class TaskParameters(taskTypeId: String, + neededExperience: Experience, + openInstances: Int, + projectName: String, + scriptId: Option[String], + boundingBox: Option[BoundingBox], + dataSet: String, + editPosition: Vec3Int, + editRotation: Vec3Double, + creationInfo: Option[String], + description: Option[String], + baseAnnotation: Option[BaseAnnotation]) object TaskParameters { implicit val taskParametersFormat: Format[TaskParameters] = Json.format[TaskParameters] } -case class NmlTaskParameters(taskTypeIdOrSummary: String, - // taskTypeIdOrSummary named like this for compatibility, note that in NML case only ids are valid. +case class NmlTaskParameters(taskTypeId: String, neededExperience: Experience, openInstances: Int, projectName: String, diff --git a/app/models/task/TaskCreationService.scala b/app/models/task/TaskCreationService.scala index a09b559e7ae..a183e1d327c 100644 --- a/app/models/task/TaskCreationService.scala +++ b/app/models/task/TaskCreationService.scala @@ -71,7 +71,7 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService, taskParameters: TaskParameters, organizationId: ObjectId)(implicit ctx: DBAccessContext, m: MessagesProvider): Fox[BaseAnnotation] = for { - taskTypeIdValidated <- ObjectId.fromString(taskParameters.taskTypeIdOrSummary) ?~> "taskType.id.invalid" + taskTypeIdValidated <- ObjectId.fromString(taskParameters.taskTypeId) ?~> "taskType.id.invalid" taskType <- taskTypeDAO.findOne(taskTypeIdValidated) ?~> "taskType.notFound" dataSet <- dataSetDAO.findOneByNameAndOrganization(taskParameters.dataSet, organizationId) baseAnnotationIdValidated <- ObjectId.fromString(baseAnnotation.baseId) @@ -169,7 +169,7 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService, implicit ctx: DBAccessContext): Fox[List[Option[SkeletonTracing]]] = Fox.serialCombined(paramsList) { params => for { - taskTypeIdValidated <- ObjectId.fromString(params.taskTypeIdOrSummary) ?~> "taskType.id.invalid" + taskTypeIdValidated <- ObjectId.fromString(params.taskTypeId) ?~> "taskType.id.invalid" taskType <- taskTypeDAO.findOne(taskTypeIdValidated) ?~> "taskType.notFound" skeletonTracingOpt = if ((taskType.tracingType == TracingType.skeleton || taskType.tracingType == TracingType.hybrid) && params.baseAnnotation.isEmpty) { Some( @@ -189,7 +189,7 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService, m: MessagesProvider): Fox[List[Option[(VolumeTracing, Option[File])]]] = Fox.serialCombined(paramsList) { params => for { - taskTypeIdValidated <- ObjectId.fromString(params.taskTypeIdOrSummary) ?~> "taskType.id.invalid" + taskTypeIdValidated <- ObjectId.fromString(params.taskTypeId) ?~> "taskType.id.invalid" taskType <- taskTypeDAO.findOne(taskTypeIdValidated) ?~> "taskType.notFound" volumeTracingOpt <- if ((taskType.tracingType == TracingType.volume || taskType.tracingType == TracingType.hybrid) && params.baseAnnotation.isEmpty) { annotationService @@ -261,7 +261,7 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService, val parsedNmlTracingBoundingBox = params._1.map(b => BoundingBox(b.topLeft, b.width, b.height, b.depth)) val bbox = if (nmlFormParams.boundingBox.isDefined) nmlFormParams.boundingBox else parsedNmlTracingBoundingBox TaskParameters( - nmlFormParams.taskTypeIdOrSummary, + nmlFormParams.taskTypeId, nmlFormParams.neededExperience, nmlFormParams.openInstances, nmlFormParams.projectName, @@ -390,7 +390,7 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService, "dataSet.notFound", firstDatasetName) _ = if (fullTasks.exists(task => task._1.baseAnnotation.isDefined)) - slackNotificationService.noticeBaseAnnotationTaskCreation(fullTasks.map(_._1.taskTypeIdOrSummary).distinct, + slackNotificationService.noticeBaseAnnotationTaskCreation(fullTasks.map(_._1.taskTypeId).distinct, fullTasks.count(_._1.baseAnnotation.isDefined)) tracingStoreClient <- tracingStoreService.clientFor(dataSet) savedSkeletonTracingIds: List[Box[Option[String]]] <- tracingStoreClient.saveSkeletonTracings( @@ -475,7 +475,7 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService, } match { case Full((params: TaskParameters, Some((tracing, initialFile)))) => for { - taskTypeIdValidated <- ObjectId.fromString(params.taskTypeIdOrSummary) ?~> "taskType.id.invalid" + taskTypeIdValidated <- ObjectId.fromString(params.taskTypeId) ?~> "taskType.id.invalid" taskType <- taskTypeDAO.findOne(taskTypeIdValidated) ?~> "taskType.notFound" saveResult <- tracingStoreClient .saveVolumeTracing(tracing, initialFile, resolutionRestrictions = taskType.settings.resolutionRestrictions) @@ -528,7 +528,7 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService, skeletonIdOpt <- skeletonTracingIdBox.toFox volumeIdOpt <- volumeTracingIdBox.toFox _ <- bool2Fox(skeletonIdOpt.isDefined || volumeIdOpt.isDefined) ?~> "task.create.needsEitherSkeletonOrVolume" - taskTypeIdValidated <- ObjectId.fromString(params.taskTypeIdOrSummary) + taskTypeIdValidated <- ObjectId.fromString(params.taskTypeId) project <- projectDAO.findOneByNameAndOrganization(params.projectName, requestingUser._organization) ?~> "project.notFound" _ <- validateScript(params.scriptId) ?~> "script.invalid" _ <- Fox.assertTrue(userService.isTeamManagerOrAdminOf(requestingUser, project._team)) @@ -561,9 +561,4 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService, js <- taskService.publicWrites(task) } yield js - def normalizeTaskTypeId(taskParameters: TaskParameters, organization: ObjectId)( - implicit ctx: DBAccessContext): Fox[TaskParameters] = - for { - taskTypeIdString <- taskTypeService.idOrSummaryToId(taskParameters.taskTypeIdOrSummary, organization) - } yield taskParameters.copy(taskTypeIdOrSummary = taskTypeIdString) } diff --git a/frontend/javascripts/admin/task/task_create_bulk_view.tsx b/frontend/javascripts/admin/task/task_create_bulk_view.tsx index ecda5562930..126008f3899 100644 --- a/frontend/javascripts/admin/task/task_create_bulk_view.tsx +++ b/frontend/javascripts/admin/task/task_create_bulk_view.tsx @@ -24,7 +24,7 @@ export type NewTask = { readonly projectName: string; readonly scriptId: string | null | undefined; readonly openInstances: number; - readonly taskTypeIdOrSummary: string; + readonly taskTypeId: string; readonly csvFile?: File; readonly nmlFiles?: File; readonly baseAnnotation?: @@ -69,7 +69,7 @@ function TaskCreateBulkView() { if ( !_.isString(task.neededExperience.domain) || !_.isString(task.dataSet) || - !_.isString(task.taskTypeIdOrSummary) || + !_.isString(task.taskTypeId) || !_.isString(task.projectName) || task.editPosition.some(Number.isNaN) || task.editRotation.some(Number.isNaN) || @@ -111,7 +111,7 @@ function TaskCreateBulkView() { function parseLine(line: string): NewTask { const words = splitToWords(line); const dataSet = words[0]; - const taskTypeIdOrSummary = words[1]; + const taskTypeId = words[1]; const experienceDomain = words[2]; const minExperience = parseInt(words[3]); const x = parseInt(words[4]); @@ -150,7 +150,7 @@ function TaskCreateBulkView() { }; return { dataSet, - taskTypeIdOrSummary, + taskTypeId, scriptId, openInstances, boundingBox, @@ -257,9 +257,9 @@ function TaskCreateBulkView() { Specify each new task on a separate line as comma seperated values (CSV) in the following format:
- dataSet, taskType (ID or summary), - experienceDomain, minExperience, x, y, z, rotX, rotY, rotZ, instances, minX, minY, minZ, - width, height, depth, project [, scriptId + dataSet, taskTypeId, experienceDomain, + minExperience, x, y, z, rotX, rotY, rotZ, instances, minX, minY, minZ, width, height, + depth, project [, scriptId , baseAnnotationId]
If you want to define some (but not all) of the optional values, please list all @@ -299,7 +299,7 @@ function TaskCreateBulkView() { >