Update Bundle Naming Convention and Add Task Locking Endpoints (#1163)

* change 'reset -> update' task bundling naming convention

* fix task status issues

* remove task lock finct

* add bundle locking endpoints

* make bundling data more usefule

* simplify bundle creation

Author: CollinBeczak

app/org/maproulette/controllers/api/TaskController.scala

@@ -369,6 +369,90 @@ class TaskController @Inject() (
     }
   }
 
+  /**
+    * Locks a bundle of tasks based on the provided task IDs.
+    *
+    * @param taskIds The IDs of the tasks to lock
+    * @return
+    */
+  def lockTaskBundleByIds(taskIds: List[Long]): Action[AnyContent] = Action.async {
+    implicit request =>
+      this.sessionManager.authenticatedRequest { implicit user =>
+        val (lockedTasks, failedTasks) = taskIds.foldLeft((List.empty[Task], List.empty[Long])) {
+          case ((locked, failed), taskId) =>
+            this.dal.retrieveById(taskId) match {
+              case Some(task) =>
+                try {
+                  this.dal.lockItem(user, task)
+                  (task :: locked, failed)
+                } catch {
+                  case _: LockedException => (locked, taskId :: failed)
+                }
+              case None => (locked, failed)
+            }
+        }
+
+        if (failedTasks.nonEmpty) {
+          lockedTasks.foreach(task => this.dal.unlockItem(user, task))
+          Ok(Json.toJson(failedTasks, false))
+        } else {
+          Ok(Json.toJson(lockedTasks, true))
+        }
+      }
+  }
+
+  /**
+    * Unlocks a bundle of tasks based on the provided task IDs.
+    *
+    * @param taskIds The IDs of the tasks to unlock
+    * @return
+    */
+  def unlockTaskBundleByIds(taskIds: List[Long]): Action[AnyContent] = Action.async {
+    implicit request =>
+      this.sessionManager.authenticatedRequest { implicit user =>
+        val tasks = taskIds.flatMap { taskId =>
+          this.dal.retrieveById(taskId) match {
+            case Some(task) =>
+              try {
+                this.dal.unlockItem(user, task)
+                Some(taskId)
+              } catch {
+                case _: Exception => None
+              }
+            case None => None
+          }
+        }
+
+        Ok(Json.toJson(tasks))
+      }
+  }
+
+  /**
+    * Refreshes the locks on a bundle of tasks based on the provided task IDs.
+    *
+    * @param taskIds The IDs of the tasks to refresh locks
+    * @return
+    */
+  def refreshTaskLocksByIds(taskIds: List[Long]): Action[AnyContent] = Action.async {
+    implicit request =>
+      this.sessionManager.authenticatedRequest { implicit user =>
+        val result = taskIds.map { taskId =>
+          this.dal.retrieveById(taskId) match {
+            case Some(task) =>
+              try {
+                this.dal.refreshItemLock(user, task)
+                Some(taskId)
+              } catch {
+                case _: LockedException => None
+              }
+            case None => None
+          }
+        }.flatten
+
+        Ok(Json.toJson(result))
+      }
+  }
+
   /**
     * Gets a random task(s) given the provided tags.
     *

app/org/maproulette/framework/controller/TaskBundleController.scala

@@ -262,17 +262,17 @@ class TaskBundleController @Inject() (
   }
 
   /**
-    *  Resets the bundle to the tasks provided, and unlock all tasks removed from current bundle
+    *  Sets the bundle to the tasks provided, and unlock all tasks removed from current bundle
     *
     * @param bundleId The id of the bundle
     * @param taskIds The task ids the bundle will reset to
     */
-  def resetTaskBundle(
+  def updateTaskBundle(
       id: Long,
       taskIds: List[Long]
   ): Action[AnyContent] = Action.async { implicit request =>
     this.sessionManager.authenticatedRequest { implicit user =>
-      this.serviceManager.taskBundle.resetTaskBundle(user, id, taskIds)
+      this.serviceManager.taskBundle.updateTaskBundle(user, id, taskIds)
       Ok(Json.toJson(this.serviceManager.taskBundle.getTaskBundle(user, id)))
     }
   }
@@ -286,11 +286,10 @@ class TaskBundleController @Inject() (
     */
   def unbundleTasks(
       id: Long,
-      taskIds: List[Long],
-      preventTaskIdUnlocks: List[Long]
+      taskIds: List[Long]
   ): Action[AnyContent] = Action.async { implicit request =>
     this.sessionManager.authenticatedRequest { implicit user =>
-      this.serviceManager.taskBundle.unbundleTasks(user, id, taskIds, preventTaskIdUnlocks)
+      this.serviceManager.taskBundle.unbundleTasks(user, id, taskIds)
       Ok(Json.toJson(this.serviceManager.taskBundle.getTaskBundle(user, id)))
     }
   }

app/org/maproulette/framework/repository/TaskBundleRepository.scala

@@ -51,7 +51,8 @@ class TaskBundleRepository @Inject() (
       taskIds: List[Long],
       verifyTasks: (List[Task]) => Unit
   ): TaskBundle = {
-    this.withMRTransaction { implicit c =>
+    // First transaction: verify tasks and create bundle
+    val bundleId = this.withMRTransaction { implicit c =>
       val lockedTasks = this.withListLocking(user, Some(TaskType())) { () =>
         this.taskDAL.retrieveListById(-1, 0)(taskIds)
       }
@@ -70,54 +71,35 @@ class TaskBundleRepository @Inject() (
         SQL"""INSERT INTO bundles (owner_id, name) VALUES (${user.id}, ${name})""".executeInsert()
 
       rowId match {
-        case Some(bundleId: Long) =>
-          // Update the task object to bind it to the bundle
-          SQL(s"""UPDATE tasks SET bundle_id = $bundleId
-               WHERE id IN ({inList})""")
-            .on(
-              "inList" -> taskIds
-            )
-            .executeUpdate()
-
-          primaryId match {
-            case Some(id) =>
-              val sqlQuery = s"""UPDATE tasks SET is_bundle_primary = true WHERE id = $id"""
-              SQL(sqlQuery).executeUpdate()
-            case None => // Handle the case where primaryId is None
-          }
-
-          val sqlInsertTaskBundles =
-            s"""INSERT INTO task_bundles (task_id, bundle_id) VALUES ({taskId}, $bundleId)"""
-          val parameters = lockedTasks.map(task => Seq[NamedParameter]("taskId" -> task.id))
-          BatchSql(sqlInsertTaskBundles, parameters.head, parameters.tail: _*).execute()
-
-          // Lock each of the new tasks to indicate they are part of the bundle
-          for (task <- lockedTasks) {
-            try {
-              this.lockItem(user, task)
-            } catch {
-              case e: Exception => this.logger.warn(e.getMessage)
-            }
-            taskRepository.cacheManager.cache.remove(task.id)
+        case Some(id: Long) =>
+          // Set primary task if specified
+          primaryId.foreach { pid =>
+            SQL"""UPDATE tasks SET is_bundle_primary = true WHERE id = $pid""".executeUpdate()
           }
-
-          TaskBundle(bundleId, user.id, lockedTasks.map(task => {
-            task.id
-          }), Some(lockedTasks))
-
+          id
         case None =>
           throw new Exception("Bundle creation failed")
       }
     }
+
+    // Second transaction: add tasks to bundle
+    this.bundleTasks(user, bundleId, taskIds)
+
+    val lockedTasks = this.withListLocking(user, Some(TaskType())) { () =>
+      this.taskDAL.retrieveListById(-1, 0)(taskIds)
+    }
+
+    // Return the created bundle
+    TaskBundle(bundleId, user.id, taskIds, Some(lockedTasks))
   }
 
   /**
-    *  Resets the bundle to the tasks provided, and unlock all tasks removed from current bundle
+    *  Sets the bundle to the tasks provided, and unlock all tasks removed from current bundle
     *
     * @param bundleId The id of the bundle
     * @param taskIds The task ids the bundle will reset to
     */
-  def resetTaskBundle(
+  def updateTaskBundle(
       user: User,
       bundleId: Long,
       taskIds: List[Long]
@@ -134,7 +116,7 @@ class TaskBundleRepository @Inject() (
       val tasksToRemove = currentTaskIds.filter(taskId => !taskIds.contains(taskId))
 
       if (tasksToRemove.nonEmpty) {
-        this.unbundleTasks(user, bundleId, tasksToRemove, List.empty)
+        this.unbundleTasks(user, bundleId, tasksToRemove)
       }
 
       // Filter for tasks that need to be added back to the bundle.
@@ -207,12 +189,6 @@ class TaskBundleRepository @Inject() (
       }
 
       lockedTasks.foreach { task =>
-        try {
-          this.lockItem(user, task)
-        } catch {
-          case e: Exception =>
-            this.logger.warn(e.getMessage)
-        }
         taskRepository.cacheManager.cache.remove(task.id)
       }
     }
@@ -226,8 +202,7 @@ class TaskBundleRepository @Inject() (
   def unbundleTasks(
       user: User,
       bundleId: Long,
-      taskIds: List[Long],
-      preventTaskIdUnlocks: List[Long]
+      taskIds: List[Long]
   ): Unit = {
     this.withMRConnection { implicit c =>
       val tasks = this.retrieveTasks(
@@ -265,13 +240,6 @@ class TaskBundleRepository @Inject() (
                 )
                 .executeUpdate()
 
-              if (!preventTaskIdUnlocks.contains(task.id)) {
-                try {
-                  this.unlockItem(user, task)
-                } catch {
-                  case e: Exception => this.logger.warn(e.getMessage)
-                }
-              }
               taskRepository.cacheManager.cache.remove(task.id)
             case None => // do nothing
           }
@@ -306,13 +274,19 @@ class TaskBundleRepository @Inject() (
         .on("bundleId" -> bundleId)
         .executeUpdate()
 
+      // Update cache for each task
       tasks.foreach { task =>
         if (!task.isBundlePrimary.getOrElse(false)) {
-          try {
-            this.unlockItem(user, task)
-          } catch {
-            case e: Exception => this.logger.warn(e.getMessage)
-          }
+          SQL(
+            """UPDATE tasks 
+                  SET status = {status} 
+                  WHERE id = {taskId}
+              """
+          ).on(
+              "taskId" -> task.id,
+              "status" -> STATUS_CREATED
+            )
+            .executeUpdate()
         }
         taskRepository.cacheManager.cache.remove(task.id)
       }

app/org/maproulette/framework/service/TaskBundleService.scala

@@ -90,12 +90,12 @@ class TaskBundleService @Inject() (
   }
 
   /**
-    *  Resets the bundle to the tasks provided, and unlock all tasks removed from current bundle
+    *  Sets the bundle to the tasks provided, and unlock all tasks removed from current bundle
     *
     * @param bundleId The id of the bundle
     * @param taskIds The task ids the bundle will reset to
     */
-  def resetTaskBundle(
+  def updateTaskBundle(
       user: User,
       bundleId: Long,
       taskIds: List[Long]
@@ -108,7 +108,7 @@ class TaskBundleService @Inject() (
       )
     }
 
-    this.repository.resetTaskBundle(user, bundleId, taskIds)
+    this.repository.updateTaskBundle(user, bundleId, taskIds)
     this.getTaskBundle(user, bundleId)
   }
 
@@ -120,8 +120,7 @@ class TaskBundleService @Inject() (
   def unbundleTasks(
       user: User,
       bundleId: Long,
-      taskIds: List[Long],
-      preventTaskIdUnlocks: List[Long]
+      taskIds: List[Long]
   )(): TaskBundle = {
     val bundle = this.getTaskBundle(user, bundleId)
 
@@ -132,7 +131,7 @@ class TaskBundleService @Inject() (
       )
     }
 
-    this.repository.unbundleTasks(user, bundleId, taskIds, preventTaskIdUnlocks)
+    this.repository.unbundleTasks(user, bundleId, taskIds)
     this.getTaskBundle(user, bundleId)
   }
 

conf/v2_route/bundle.api

@@ -47,8 +47,8 @@ POST    /taskBundle                                 @org.maproulette.framework.c
 POST     /taskBundle/:id                             @org.maproulette.framework.controller.TaskBundleController.getTaskBundle(id:Long, lockTasks:Boolean ?= false)
 ###
 # tags: [ Bundle ]
-# summary: Resets a Task Bundle
-# description: Resets the bundle to the tasks provided, and unlock all tasks removed from current bundle
+# summary: Updates a Task Bundle
+# description: Sets the bundle to the tasks provided, and unlock all tasks removed from current bundle
 # responses:
 #   '200':
 #     description: Ok with empty body
@@ -63,7 +63,7 @@ POST     /taskBundle/:id                             @org.maproulette.framework.
 #     in: query
 #     description: The task ids the bundle will reset to
 ###
-POST     /taskBundle/:id/reset                          @org.maproulette.framework.controller.TaskBundleController.resetTaskBundle(id: Long, taskIds: List[Long])
+POST     /taskBundle/:id/update                          @org.maproulette.framework.controller.TaskBundleController.updateTaskBundle(id: Long, taskIds: List[Long])
 ###
 # tags: [ Bundle ]
 # summary: Deletes a Task Bundle
@@ -104,9 +104,5 @@ DELETE     /taskBundle/:id                          @org.maproulette.framework.c
 #     in: query
 #     description: The list of task ids to remove from the bundle
 #     required: true
-#   - name: preventTaskIdUnlocks
-#     in: query
-#     description: The list of task ids to keep locked when removed from bundle
-#     required: true
 ###
-POST       /taskBundle/:id/unbundle                 @org.maproulette.framework.controller.TaskBundleController.unbundleTasks(id:Long, taskIds:List[Long], preventTaskIdUnlocks:List[Long])
+POST       /taskBundle/:id/unbundle                 @org.maproulette.framework.controller.TaskBundleController.unbundleTasks(id:Long, taskIds:List[Long])