Open challenge reports to readers, and spare reported challenges from auto-archiving

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

@@ -69,6 +69,22 @@ class ChallengeReportController @Inject() (
       }
   }
 
+  /**
+    * Lists every report filed against a challenge, resolved ones included, so a
+    * reader can see what has been raised about it and where each report stands.
+    * Open to anyone, like the challenge comments that filing a report posts:
+    * the reporter's identity and words are already public through those. The
+    * reporter's email and the admin side of the triage record are stripped.
+    *
+    * @param challengeId The challenge in question
+    * @return The reports against that challenge, newest first
+    */
+  def listForChallenge(challengeId: Long): Action[AnyContent] = Action.async { implicit request =>
+    this.sessionManager.userAwareRequest { _ =>
+      Ok(Json.toJson(this.challengeReportService.retrieveReportsForChallenge(challengeId)))
+    }
+  }
+
   /**
     * Lists reports, newest first. Superusers only.
     *

app/org/maproulette/framework/model/ChallengeReport.scala

@@ -11,9 +11,11 @@ import play.api.libs.json._
 /**
   * A report against a challenge's design -- "this challenge is poorly designed
   * and is causing people to make incorrect edits" -- rather than a bug or a
-  * feature request. Any authenticated user can file one; only superusers can
-  * read them, because a report names the reporter and may carry the email
-  * address they volunteered for follow-up.
+  * feature request. Any authenticated user can file one, and anyone can read
+  * what has been reported about a challenge, since filing also posts a public
+  * challenge comment naming the reporter and quoting the report. The triage
+  * queue is superusers only: it carries the email address the reporter may have
+  * volunteered for follow-up, and the notes admins leave on a decision.
   *
   * Reports are resolved, never deleted: an admin marks one actioned (say, after
   * archiving the challenge) or dismissed, so the history of what was reported

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

@@ -171,8 +171,8 @@ class ChallengeReportRepository @Inject() (override val db: Database) extends Re
 
   /**
     * Retrieves a reporter's still-open report against one challenge, if they
-    * have one. This is the only report-reading path open to a non-superuser:
-    * it returns nothing but the caller's own report.
+    * have one. It returns nothing but the caller's own report, so a
+    * non-superuser may read through it.
     */
   def retrieveOpenForReporter(challengeId: Long, reporterId: Long): Option[ChallengeReport] = {
     this.withMRConnection { implicit c =>
@@ -193,6 +193,37 @@ class ChallengeReportRepository @Inject() (override val db: Database) extends Re
     }
   }
 
+  /**
+    * Lists every report filed against one challenge, newest first, resolved
+    * ones included. Rows come back whole; it is the service that decides which
+    * parts of them a given caller may see.
+    */
+  def listForChallenge(challengeId: Long): List[ChallengeReport] = {
+    this.withMRConnection { implicit c =>
+      SQL(
+        s"""SELECT COUNT(*) OVER() AS full_count, $selectColumns
+            $fromClause
+            WHERE cr.challenge_id = {challengeId}
+            ORDER BY cr.reported_at DESC"""
+      ).on(Symbol("challengeId") -> challengeId)
+        .as(this.parser.*)
+    }
+  }
+
+  /**
+    * The ids of every challenge carrying at least one open report. The archive
+    * scheduler uses this to leave reported challenges alone: archiving one
+    * would drop it out of the admin triage queue, which defaults to challenges
+    * that are still active, before anyone had ruled on the report.
+    */
+  def challengeIdsWithOpenReports(): List[Long] = {
+    this.withMRConnection { implicit c =>
+      SQL"""SELECT DISTINCT challenge_id FROM challenge_reports
+            WHERE status = ${ChallengeReport.STATUS_OPEN}"""
+        .as(get[Long]("challenge_id").*)
+    }
+  }
+
   /**
     * Counts a reporter's still-open reports against one challenge, so the same
     * person cannot file the same complaint repeatedly.

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

@@ -16,8 +16,10 @@ import org.slf4j.LoggerFactory
 
 /**
   * Handles reports filed against a challenge's design. Filing is open to any
-  * authenticated user; reading and triaging is restricted to superusers, since
-  * a report carries the reporter's identity and possibly their email address.
+  * authenticated user, and what a report says is readable by anyone, since
+  * filing posts a public challenge comment saying the same thing. The triage
+  * queue is restricted to superusers: it carries the email address a reporter
+  * may have volunteered, and the notes admins leave for each other.
   *
   * Filing a report also posts a challenge comment naming the reporter and
   * quoting the report, which is what tells the challenge owner that something
@@ -140,6 +142,47 @@ class ChallengeReportService @Inject() (
   def retrieveOwnOpenReport(user: User, challengeId: Long): Option[ChallengeReport] =
     this.repository.retrieveOpenForReporter(challengeId, user.id)
 
+  /**
+    * Retrieves every report filed against a challenge, newest first and
+    * resolved ones included, so anyone weighing up the challenge can see what
+    * has been raised about it and where each report stands.
+    *
+    * Open to anyone, signed in or not, because filing a report already posts a
+    * challenge comment naming the reporter and quoting what they wrote -- the
+    * reporter's identity and words are public either way. What is not public is
+    * stripped: see [[publicView]].
+    *
+    * @param challengeId The challenge in question
+    * @return The reports against that challenge
+    */
+  def retrieveReportsForChallenge(challengeId: Long): List[ChallengeReport] =
+    this.repository.listForChallenge(challengeId).map(this.publicView)
+
+  /**
+    * Reduces a report to the parts that are already public knowledge. Out go
+    * the email address the reporter volunteered for follow-up, which the
+    * accompanying challenge comment deliberately omits, and the admin side of
+    * the triage record -- who ruled on the report and the note they left for
+    * their own purposes. The outcome and its date survive, so a reader can see
+    * that a report was dealt with.
+    */
+  private def publicView(report: ChallengeReport): ChallengeReport =
+    report.copy(
+      reporterEmail = None,
+      reviewedBy = None,
+      reviewedByName = None,
+      reviewComment = None
+    )
+
+  /**
+    * The ids of every challenge carrying at least one open report, so the
+    * archive scheduler can leave those challenges alone until an admin has
+    * ruled. Internal to the scheduler -- it names no reporter, but it is not
+    * exposed over the API either.
+    */
+  def challengeIdsWithOpenReports(): Set[Long] =
+    this.repository.challengeIdsWithOpenReports().toSet
+
   /**
     * Lists reports for the admin dashboard.
     *

app/org/maproulette/jobs/SchedulerActor.scala

@@ -756,9 +756,20 @@ class SchedulerActor @Inject() (
 
     logger.info(action + " - Stale Date: " + staleDate);
 
+    // A challenge someone has reported is left alone until an admin has ruled
+    // on the report: archiving it would drop it out of the triage queue, which
+    // defaults to challenges that are still active, before anyone looked at it.
+    val reportedChallengeIds = this.serviceManager.challengeReport.challengeIdsWithOpenReports()
+    if (reportedChallengeIds.nonEmpty) {
+      logger.info(
+        action + " - Skipping " + reportedChallengeIds.size + " challenge(s) with an open report"
+      )
+    }
+
     this.serviceManager.challenge
       .activeChallenges()
       .filter(challenge => challenge.created.toString("yyyy-MM-dd") < staleDate)
+      .filterNot(challenge => reportedChallengeIds.contains(challenge.id))
       .foreach(challenge => {
 
         val tasks         = this.serviceManager.challenge.getTasksByParentId(challenge.id);

conf/v2_route/challengereport.api

@@ -66,6 +66,29 @@ POST    /challenge/:challengeId/report               @org.maproulette.framework.
 GET     /challenge/:challengeId/report/mine          @org.maproulette.framework.controller.ChallengeReportController.retrieveOwnOpenReport(challengeId:Long)
 ###
 # tags: [ Challenge Report ]
+# operationId: challenge_report_list_for_challenge
+# summary: List the Reports Against a Challenge
+# description: Returns every report filed against a challenge, newest first, resolved ones included, so a reader can see what has been raised about it and where each report stands. Open to anyone, like the challenge comments that filing a report posts -- the reporter's identity and words are already public through those. The email a reporter volunteered for follow-up, and the admin side of the triage record (who ruled on a report and the note they left), are stripped; the outcome and its date remain.
+# responses:
+#   '200':
+#     description: The reports against this challenge
+#     content:
+#       application/json:
+#         schema:
+#           type: array
+#           items:
+#             $ref: '#/components/schemas/org.maproulette.framework.model.ChallengeReport'
+# parameters:
+#   - name: challengeId
+#     in: path
+#     description: The id of the challenge in question
+#     required: true
+#     schema:
+#       type: integer
+###
+GET     /challenge/:challengeId/reports              @org.maproulette.framework.controller.ChallengeReportController.listForChallenge(challengeId:Long)
+###
+# tags: [ Challenge Report ]
 # operationId: challenge_report_list
 # summary: List Challenge Reports
 # description: Lists reports filed against challenges, newest first, for the admin triage dashboard. Restricted to superusers, because a report carries the reporter's identity and any email address they volunteered. Each returned report includes the total number of matches in fullCount.

test/org/maproulette/framework/FrameworkMasterSuite.scala

@@ -24,6 +24,7 @@ class FrameworkMasterSuite extends Suites with BeforeAndAfterAll with TestDataba
     new ChallengeServiceSpec,
     new ChallengeRepositorySpec,
     new ChallengeReportRepositorySpec,
+    new ChallengeReportServiceSpec,
     new TeamImageRepositorySpec,
     new TeamAvatarRepositorySpec,
     new ChallengeListingServiceSpec,

test/org/maproulette/framework/repository/ChallengeReportRepositorySpec.scala

@@ -99,6 +99,41 @@ class ChallengeReportRepositorySpec(implicit val application: Application) exten
       this.repository.retrieveOpenForReporter(challenge.id, this.defaultUser.id) mustEqual None
     }
 
+    "return every report on a challenge, newest first, resolved ones included" taggedAs ChallengeReportRepoTag in {
+      val challenge =
+        this.createChallengeStructure("report_challenge_history", this.defaultProject.id, 1)
+      val other = this.createChallengeStructure("report_other_history", this.defaultProject.id, 1)
+
+      this.repository.listForChallenge(challenge.id) mustEqual List()
+
+      val first = report(challenge)
+      this.repository
+        .updateStatus(first.id, ChallengeReport.STATUS_DISMISSED, this.defaultUser, None)
+      val second = report(challenge)
+
+      // A report on a different challenge stays out of this challenge's history.
+      report(other)
+
+      val history = this.repository.listForChallenge(challenge.id)
+      history.map(_.id) mustEqual List(second.id, first.id)
+      history.map(_.status) mustEqual
+        List(ChallengeReport.STATUS_OPEN, ChallengeReport.STATUS_DISMISSED)
+      history.map(_.reporterId).distinct mustEqual List(Some(this.defaultUser.id))
+    }
+
+    "name only the challenges carrying an open report" taggedAs ChallengeReportRepoTag in {
+      val challenge = this.createChallengeStructure("report_open_ids", this.defaultProject.id, 1)
+
+      this.repository.challengeIdsWithOpenReports() must not contain challenge.id
+
+      val open = report(challenge)
+      this.repository.challengeIdsWithOpenReports() must contain(challenge.id)
+
+      this.repository
+        .updateStatus(open.id, ChallengeReport.STATUS_ACTIONED, this.defaultUser, None)
+      this.repository.challengeIdsWithOpenReports() must not contain challenge.id
+    }
+
     "filter a listing by status and by challenge, newest first" taggedAs ChallengeReportRepoTag in {
       val challenge = this.createChallengeStructure("report_listing", this.defaultProject.id, 1)
 

test/org/maproulette/framework/service/ChallengeReportServiceSpec.scala

@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2020 MapRoulette contributors (see CONTRIBUTORS.md).
+ * Licensed under the Apache License, Version 2.0 (see LICENSE).
+ */
+
+package org.maproulette.framework.service
+
+import org.maproulette.framework.model.{ChallengeReport, User}
+import org.maproulette.framework.util.{ChallengeReportTag, FrameworkHelper}
+import play.api.Application
+
+class ChallengeReportServiceSpec(implicit val application: Application) extends FrameworkHelper {
+  val service: ChallengeReportService =
+    this.application.injector.instanceOf(classOf[ChallengeReportService])
+
+  private val COMMENT = "This challenge asks mappers to delete valid buildings." * 2
+
+  "ChallengeReportService" should {
+    "list everyone's reports on a challenge, without the reporter's email" taggedAs ChallengeReportTag in {
+      val challenge =
+        this.createChallengeStructure("report_service_list", this.defaultProject.id, 1)
+
+      this.service.retrieveReportsForChallenge(challenge.id) mustEqual List()
+
+      val filed =
+        this.service.create(this.defaultUser, challenge.id, COMMENT, Some("mapper@example.com"))
+      filed.reporterEmail mustEqual Some("mapper@example.com")
+
+      val visible = this.service.retrieveReportsForChallenge(challenge.id)
+      visible.map(_.id) mustEqual List(filed.id)
+      // The reporter is named -- filing already posted a challenge comment
+      // saying so -- but the address they left for follow-up is not.
+      visible.head.reporterName mustEqual Some(this.defaultUser.name)
+      visible.head.reporterEmail mustEqual None
+      visible.head.comment mustEqual COMMENT
+    }
+
+    "show that a report was resolved without exposing who ruled on it, or their note" taggedAs ChallengeReportTag in {
+      val challenge =
+        this.createChallengeStructure("report_service_resolved", this.defaultProject.id, 1)
+      val filed = this.service.create(this.defaultUser, challenge.id, COMMENT, None)
+
+      this.service.updateStatus(
+        User.superUser,
+        filed.id,
+        ChallengeReport.STATUS_ACTIONED,
+        Some("archived the challenge")
+      )
+
+      val visible = this.service.retrieveReportsForChallenge(challenge.id).head
+      visible.status mustEqual ChallengeReport.STATUS_ACTIONED
+      visible.reviewedAt mustBe defined
+      visible.reviewedBy mustEqual None
+      visible.reviewedByName mustEqual None
+      visible.reviewComment mustEqual None
+
+      // The admin dashboard still sees the whole record.
+      val forAdmin = this.service.retrieve(User.superUser, filed.id).get
+      forAdmin.reviewedBy mustEqual Some(User.superUser.id)
+      forAdmin.reviewComment mustEqual Some("archived the challenge")
+    }
+
+    "name the challenges carrying an open report, so the archiver can skip them" taggedAs ChallengeReportTag in {
+      val challenge =
+        this.createChallengeStructure("report_service_open_ids", this.defaultProject.id, 1)
+
+      this.service.challengeIdsWithOpenReports() must not contain challenge.id
+
+      val filed = this.service.create(this.defaultUser, challenge.id, COMMENT, None)
+      this.service.challengeIdsWithOpenReports() must contain(challenge.id)
+
+      this.service
+        .updateStatus(User.superUser, filed.id, ChallengeReport.STATUS_DISMISSED, None)
+      this.service.challengeIdsWithOpenReports() must not contain challenge.id
+    }
+  }
+
+  override implicit val projectTestName: String = "ChallengeReportServiceSpecProject"
+}

test/org/maproulette/framework/util/FrameworkHelper.scala

@@ -167,6 +167,7 @@ object ChallengeListingTag     extends Tag("challengelisting")
 object ChallengeListingRepoTag extends Tag("challengelistingrepo")
 object ChallengeSnapshotTag    extends Tag("challengesnapshot")
 object TeamImageRepoTag        extends Tag("teamimagerepo")
+object ChallengeReportTag      extends Tag("challengereport")
 object ChallengeReportRepoTag  extends Tag("challengereportrepo")
 object TeamAvatarRepoTag       extends Tag("teamavatarrepo")
 object ProjectTag              extends Tag("project")