Remove deleted challenges from uniqueness validation for challenge names (#1204)

* remove deleted challenges from uniqueness validation for challenge names

* run formatting

* remove incorrect down

Author: CollinBeczak

app/org/maproulette/models/dal/ChallengeDAL.scala

@@ -527,6 +527,24 @@ class ChallengeDAL @Inject() (
     }
 
     this.permission.hasObjectWriteAccess(challenge, user)
+
+    // Check for existing non-deleted challenge with same name in same project
+    this.withMRConnection { implicit c =>
+      val existingChallenge = SQL"""
+        SELECT id FROM challenges 
+        WHERE parent_id = ${challenge.general.parent} 
+        AND LOWER(name) = LOWER(${challenge.name})
+        AND (deleted = false OR deleted IS NULL)
+        LIMIT 1
+      """.as(SqlParser.long("id").singleOpt)
+
+      if (existingChallenge.isDefined) {
+        throw new UniqueViolationException(
+          s"Challenge with name ${challenge.name} already exists in the database"
+        )
+      }
+    }
+
     this.cacheManager.withOptionCaching { () =>
       val insertedChallenge =
         this.withMRTransaction { implicit c =>
@@ -553,7 +571,7 @@ class ChallengeDAL @Inject() (
                       ${challenge.extra.limitReviewTags}, ${challenge.extra.taskStyles}, ${challenge.general.requiresLocal}, ${challenge.extra.isArchived},
                       ${challenge.extra.reviewSetting}, ${challenge.extra.datasetUrl}, ${challenge.requireConfirmation}, ${challenge.requireRejectReason},
                       ${asJson(challenge.extra.taskWidgetLayout.getOrElse(Json.parse("{}")))}
-                      ) ON CONFLICT(parent_id, LOWER(name)) DO NOTHING RETURNING #${this.retrieveColumns}"""
+                      ) RETURNING #${this.retrieveColumns}"""
             .as(this.parser.*)
             .headOption
         }
@@ -656,6 +674,25 @@ class ChallengeDAL @Inject() (
           val name     = (updates \ "name").asOpt[String].getOrElse(cachedItem.name)
           val ownerId  = (updates \ "ownerId").asOpt[Long].getOrElse(cachedItem.general.owner)
           val parentId = (updates \ "parentId").asOpt[Long].getOrElse(cachedItem.general.parent)
+
+          // Check if name or parent changed and if so, validate uniqueness
+          if (name != cachedItem.name || parentId != cachedItem.general.parent) {
+            val existingChallenge = SQL"""
+              SELECT id FROM challenges 
+              WHERE parent_id = $parentId 
+              AND LOWER(name) = LOWER($name)
+              AND (deleted = false OR deleted IS NULL)
+              AND id != $id
+              LIMIT 1
+            """.as(SqlParser.long("id").singleOpt)
+
+            if (existingChallenge.isDefined) {
+              throw new UniqueViolationException(
+                s"Challenge with name $name already exists in the database"
+              )
+            }
+          }
+
           val difficulty =
             (updates \ "difficulty").asOpt[Int].getOrElse(cachedItem.general.difficulty)
           val description =

app/org/maproulette/models/dal/ParentDAL.scala

@@ -118,6 +118,7 @@ trait ParentDAL[Key, T <: BaseObject[Key], C <: BaseObject[Key]]
       val query =
         s"""SELECT ${this.childColumns} FROM ${this.childTable}
                       WHERE parent_id = {id} ${this.enabled(onlyEnabled)}
+                      AND (deleted = false OR deleted IS NULL)
                       ${this.searchField("name")}
                       ${this.order(Some(orderColumn), orderDirection)}
                       LIMIT ${this.sqlLimit(limit)} OFFSET {offset}"""
@@ -155,6 +156,7 @@ trait ParentDAL[Key, T <: BaseObject[Key], C <: BaseObject[Key]]
         s"""SELECT ${this.childColumns} FROM ${this.childTable} c
                       INNER JOIN ${this.tableName} p ON p.id = c.parent_id
                       WHERE p.name = LOWER({name}) ${this.enabled(onlyEnabled, "p")}
+                      AND (c.deleted = false OR c.deleted IS NULL)
                       ${this.searchField("c.name")}
                       ${this.order(Some(orderColumn), orderDirection, "c")}
                       LIMIT ${this.sqlLimit(limit)} OFFSET {offset}"""
@@ -183,6 +185,7 @@ trait ParentDAL[Key, T <: BaseObject[Key], C <: BaseObject[Key]]
       val query =
         s"""SELECT COUNT(*) as TotalChildren FROM ${this.childTable}
            |WHERE parent_id = {id} ${this.searchField("name")}
+           |AND (deleted = false OR deleted IS NULL)
            |${this.enabled(onlyEnabled)}""".stripMargin
       SQL(query)
         .on(

conf/evolutions/default/105.sql

@@ -0,0 +1,12 @@
+# --- MapRoulette Scheme
+
+# --- !Ups
+-- Modify unique constraint on (parent_id, name) to exclude deleted challenges
+-- This allows challenge names to be reused after deletion
+DROP INDEX IF EXISTS idx_challenges_parent_id_name;;
+
+CREATE UNIQUE INDEX idx_challenges_parent_id_name ON challenges (parent_id, lower(name)) 
+WHERE (deleted = false OR deleted IS NULL);;
+
+# --- !Downs
+DROP INDEX IF EXISTS idx_challenges_parent_id_name;;