prevent duplicate notifications of challenge ccompletion (#1021)

* prevent duplicate notifications of challenge ccompletion * run scalafmt
maproulette · Mar 1, 2023 · f543e29 · f543e29
1 parent 4293b33
commit f543e29
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 14 deletions.
diff --git a/app/org/maproulette/framework/service/CommentService.scala b/app/org/maproulette/framework/service/CommentService.scala
@@ -163,7 +163,12 @@ class CommentService @Inject() (
     * @param comment The actual comment being added
     * @return The newly created comment object
     */
-  def createChallengeComment(user: User, challengeId: Long, comment: String): ChallengeComment = {
+  def createChallengeComment(
+      user: User,
+      challengeId: Long,
+      comment: String,
+      notify: Boolean = true
+  ): ChallengeComment = {
     val challenge = challengeDAL.retrieveById(challengeId) match {
       case Some(t) => t
       case None =>
@@ -176,15 +181,19 @@ class CommentService @Inject() (
     }
     val newComment =
       this.challengeCommentRepository.create(user, challenge.id, comment, challenge.general.parent);
-    this.serviceManager.notification
-      .createChallengeMentionNotifications(user, newComment, challenge)
+
+    if (notify == true) {
+      this.serviceManager.notification
+        .createChallengeMentionNotifications(user, newComment, challenge)
+    }
+
     newComment
   }
 
   /**
-    * Finds a challenge comments for a challenge
+    * Finds comments for a challenge
     *
-    * @param challengeId The id of the task that the user is adding the comment too
+    * @param challengeId The id of the challenge being searched
     * @return a list of comments
     */
   def findChallengeComments(challengeId: Long): List[ChallengeComment] = {

diff --git a/app/org/maproulette/models/dal/ChallengeDAL.scala b/app/org/maproulette/models/dal/ChallengeDAL.scala
@@ -1501,7 +1501,8 @@ class ChallengeDAL @Inject() (
     *                      still be marked finished. Defaults to false.
     */
   def updateFinishedStatus(
-      finishOnEmpty: Boolean = false
+      finishOnEmpty: Boolean = false,
+      user: User
   )(implicit id: Long, c: Option[Connection] = None): Unit = {
     this.withMRConnection { implicit c =>
       this.retrieveById(id) match {
@@ -1527,11 +1528,30 @@ class ChallengeDAL @Inject() (
               SQL(updateStatusQuery).as(this.parser.*).headOption match {
                 case Some(updatedChallenge) =>
                   if (updatedChallenge.status.getOrElse(Challenge.STATUS_NA) == Challenge.STATUS_FINISHED) {
-                    Future {
-                      this.serviceManager.achievement
-                        .awardChallengeCompletionAchievements(updatedChallenge)
-                      this.serviceManager.notification
-                        .createChallengeCompletionNotification(updatedChallenge)
+                    val challengeComments =
+                      this.serviceManager.comment.findChallengeComments(updatedChallenge.id)
+                    var challengeAlreadyCompleted = false
+
+                    challengeComments.foreach(c => {
+                      if (c.comment == "Challenge Completed") {
+                        challengeAlreadyCompleted = true
+                      }
+                    })
+
+                    if (!challengeAlreadyCompleted) {
+                      Future {
+                        this.serviceManager.achievement
+                          .awardChallengeCompletionAchievements(updatedChallenge)
+                        this.serviceManager.notification
+                          .createChallengeCompletionNotification(updatedChallenge)
+                      }
+
+                      this.serviceManager.comment.createChallengeComment(
+                        user,
+                        updatedChallenge.id,
+                        "Challenge Completed",
+                        false
+                      )
                     }
                   }
                   Some(updatedChallenge)

diff --git a/app/org/maproulette/models/dal/TaskDAL.scala b/app/org/maproulette/models/dal/TaskDAL.scala
@@ -266,7 +266,7 @@ class TaskDAL @Inject() (
       if (status == Task.STATUS_CREATED || status == Task.STATUS_SKIPPED) {
         this.manager.challenge.updateReadyStatus()(parentId)
       } else {
-        this.manager.challenge.updateFinishedStatus()(parentId)
+        this.manager.challenge.updateFinishedStatus(user = user)(parentId)
       }
 
       if (status == Task.STATUS_CREATED) {
@@ -829,7 +829,7 @@ class TaskDAL @Inject() (
       }
     }
 
-    this.manager.challenge.updateFinishedStatus()(primaryTask.parent)
+    this.manager.challenge.updateFinishedStatus(user = user)(primaryTask.parent)
 
     Future {
       this.serviceManager.achievement.awardTaskCompletionAchievements(user, primaryTask, status)

diff --git a/app/org/maproulette/provider/ChallengeProvider.scala b/app/org/maproulette/provider/ChallengeProvider.scala
@@ -566,7 +566,7 @@ class ChallengeProvider @Inject() (
                       this.challengeDAL.markTasksRefreshed(true)(challenge.id)
                       // If no tasks were created by this overpass query or all tasks are
                       // fixed, then we need to update the status to finished.
-                      this.challengeDAL.updateFinishedStatus(true)(challenge.id)
+                      this.challengeDAL.updateFinishedStatus(true, user = user)(challenge.id)
                   }
                 } catch {
                   case e: Exception =>