2nd attempt: Merge "Shared Annotations" with "My annotations" (#6287)

* Revert "Revert "Merge "Shared Annotations" with "My annotations" (#6230)" (#6286)" This reverts commit de22b53. * broaden access context and add error chains to annotation public writes * llist access query * remove time logging * naming things Co-authored-by: Florian M <[email protected]> Co-authored-by: Florian M <[email protected]>
scalableminds · Jul 7, 2022 · fca7919 · fca7919
1 parent e7b938d
commit fca7919
Show file tree

Hide file tree

Showing 48 changed files with 756 additions and 550 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -130,15 +130,15 @@ jobs:
           command: docker-compose build --pull webknossos-tracingstore
 
       - run:
-          name: Lint frontend code
+          name: Lint frontend code and check formatting
           command: |
             .circleci/not-on-master.sh docker-compose run base bash -c "yarn run lint && yarn run am-i-pretty"
       - run:
           name: Run frontend tests
           command: |
             .circleci/not-on-master.sh docker-compose run base yarn test-verbose
       - run:
-          name: Lint backend code
+          name: Lint backend code and check formatting
           command: |
             .circleci/not-on-master.sh docker-compose run backend-lint-format
       - run:

diff --git a/.eslintrc.json b/.eslintrc.json
@@ -123,7 +123,7 @@
     "react/no-unstable-nested-components": "warn", // probably should do something about it
     "react/no-array-index-key": "error", // maybe sensible, maybe not
     "prefer-exponentiation-operator": "off", // I find Math.pow() more readble than **
-    "eqeqeq": "warn", // == vs ===
+    "eqeqeq": "error", // == vs ===
     "react/jsx-no-bind": "off", // no opinion
     "react/function-component-definition": "warn", // probably should do this. can be autofixed with eslint --fix
     "@typescript-eslint/no-throw-literal": "warn" // probably should do this.

diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -17,6 +17,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - Add new backend API routes for working with annotations without having to provide a 'type' argument. Note that these support stored annotations (Task and Explorational), but do not work for CompoundTask/CompoundProject/CompoundTaskType annotations. For the latter, please use the original route variants with explicit type. [#6285](https://github.com/scalableminds/webknossos/pull/6285)
 
 ### Changed
+- Merged the "Shared Annotations" tab into the "Annotations" tab in the user's dashboard. If annotations are shared with you, you can see them in your dashboard. The table can be filtered by owner if you prefer to see only your own annotations. [#6230](https://github.com/scalableminds/webknossos/pull/6230)
 
 ### Fixed
 - Fixed that zooming out for datasets with very large scale was not possible until the coarsest level. [#6304](https://github.com/scalableminds/webknossos/pull/6304)

diff --git a/app/controllers/AnnotationController.scala b/app/controllers/AnnotationController.scala
@@ -459,12 +459,37 @@ class AnnotationController @Inject()(
     } yield JsonOk(json)
   }
 
+  // Note that this lists both the user’s own explorationals and those shared with the user’s teams
+  @ApiOperation(hidden = true, value = "")
+  def listExplorationals(isFinished: Option[Boolean],
+                         limit: Option[Int],
+                         pageNumber: Option[Int] = None,
+                         includeTotalCount: Option[Boolean] = None): Action[AnyContent] =
+    sil.SecuredAction.async { implicit request =>
+      for {
+        readableAnnotations <- annotationDAO.findAllListableExplorationals(
+          isFinished,
+          limit.getOrElse(annotationService.DefaultAnnotationListLimit),
+          pageNumber.getOrElse(0))
+        annotationCount <- Fox.runIf(includeTotalCount.getOrElse(false))(
+          annotationDAO.countAllListableExplorationals(isFinished)) ?~> "annotation.countReadable.failed"
+        jsonList <- Fox.serialCombined(readableAnnotations)(annotationService.compactWrites) ?~> "annotation.compactWrites.failed"
+        _ = userDAO.updateLastActivity(request.identity._id)(GlobalAccessContext)
+      } yield {
+        val result = Ok(Json.toJson(jsonList))
+        annotationCount match {
+          case Some(count) => result.withHeaders("X-Total-Count" -> count.toString)
+          case None        => result
+        }
+      }
+    }
+
   @ApiOperation(hidden = true, value = "")
   def sharedAnnotations: Action[AnyContent] = sil.SecuredAction.async { implicit request =>
     for {
       userTeams <- userService.teamIdsFor(request.identity._id)
       sharedAnnotations <- annotationService.sharedAnnotationsFor(userTeams)
-      json <- Fox.serialCombined(sharedAnnotations)(annotationService.compactWrites(_))
+      json <- Fox.serialCombined(sharedAnnotations)(annotationService.compactWrites)
     } yield Ok(Json.toJson(json))
   }
 
@@ -473,7 +498,7 @@ class AnnotationController @Inject()(
     for {
       annotation <- provider.provideAnnotation(typ, id, request.identity)
       _ <- bool2Fox(annotation._user == request.identity._id) ?~> "notAllowed" ~> FORBIDDEN
-      teams <- annotationService.sharedTeamsFor(annotation._id)
+      teams <- teamDAO.findSharedTeamsForAnnotation(annotation._id)
       json <- Fox.serialCombined(teams)(teamService.publicWrites(_))
     } yield Ok(Json.toJson(json))
   }

diff --git a/app/controllers/UserController.scala b/app/controllers/UserController.scala
@@ -35,8 +35,6 @@ class UserController @Inject()(userService: UserService,
     extends Controller
     with FoxImplicits {
 
-  private val DefaultAnnotationListLimit = 1000
-
   @ApiOperation(value = "Returns a json with information about the requesting user", nickname = "currentUserInfo")
   def current: Action[AnyContent] = sil.SecuredAction.async { implicit request =>
     log() {
@@ -71,9 +69,9 @@ class UserController @Inject()(userService: UserService,
         annotations <- annotationDAO.findAllFor(request.identity._id,
                                                 isFinished,
                                                 AnnotationType.Explorational,
-                                                limit.getOrElse(DefaultAnnotationListLimit),
+                                                limit.getOrElse(annotationService.DefaultAnnotationListLimit),
                                                 pageNumber.getOrElse(0))
-        annotationCount <- Fox.runOptional(includeTotalCount.flatMap(BoolToOption.convert))(_ =>
+        annotationCount: Option[Int] <- Fox.runIf(includeTotalCount.getOrElse(false))(
           annotationDAO.countAllFor(request.identity._id, isFinished, AnnotationType.Explorational))
         jsonList <- Fox.serialCombined(annotations)(a => annotationService.compactWrites(a))
         _ = userDAO.updateLastActivity(request.identity._id)(GlobalAccessContext)
@@ -96,7 +94,7 @@ class UserController @Inject()(userService: UserService,
         annotations <- annotationDAO.findAllFor(request.identity._id,
                                                 isFinished,
                                                 AnnotationType.Task,
-                                                limit.getOrElse(DefaultAnnotationListLimit),
+                                                limit.getOrElse(annotationService.DefaultAnnotationListLimit),
                                                 pageNumber.getOrElse(0))
         annotationCount <- Fox.runOptional(includeTotalCount.flatMap(BoolToOption.convert))(_ =>
           annotationDAO.countAllFor(request.identity._id, isFinished, AnnotationType.Task))
@@ -183,11 +181,11 @@ class UserController @Inject()(userService: UserService,
         annotations <- annotationDAO.findAllFor(userIdValidated,
                                                 isFinished,
                                                 AnnotationType.Explorational,
-                                                limit.getOrElse(DefaultAnnotationListLimit),
+                                                limit.getOrElse(annotationService.DefaultAnnotationListLimit),
                                                 pageNumber.getOrElse(0))
         annotationCount <- Fox.runOptional(includeTotalCount.flatMap(BoolToOption.convert))(_ =>
           annotationDAO.countAllFor(userIdValidated, isFinished, AnnotationType.Explorational))
-        jsonList <- Fox.serialCombined(annotations)(a => annotationService.compactWrites(a))
+        jsonList <- Fox.serialCombined(annotations)(annotationService.compactWrites)
       } yield {
         val result = Ok(Json.toJson(jsonList))
         annotationCount match {
@@ -211,7 +209,7 @@ class UserController @Inject()(userService: UserService,
         annotations <- annotationDAO.findAllFor(userIdValidated,
                                                 isFinished,
                                                 AnnotationType.Task,
-                                                limit.getOrElse(DefaultAnnotationListLimit),
+                                                limit.getOrElse(annotationService.DefaultAnnotationListLimit),
                                                 pageNumber.getOrElse(0))
         annotationCount <- Fox.runOptional(includeTotalCount.flatMap(BoolToOption.convert))(_ =>
           annotationDAO.countAllFor(userIdValidated, isFinished, AnnotationType.Task))

diff --git a/app/models/annotation/Annotation.scala b/app/models/annotation/Annotation.scala
@@ -175,14 +175,19 @@ class AnnotationDAO @Inject()(sqlClient: SQLClient, annotationLayerDAO: Annotati
     }
 
   override def anonymousReadAccessQ(sharingToken: Option[String]) = s"visibility = '${AnnotationVisibility.Public}'"
+
+  private def listAccessQ(requestingUserId: ObjectId): String =
+    s"""
+       |(visibility = '${AnnotationVisibility.Internal}' and (select _organization from webknossos.teams where webknossos.teams._id = _team)
+       |            in (select _organization from webknossos.users_ where _id = '${requestingUserId.id}'))
+       |          or _team in (select _team from webknossos.user_team_roles where _user = '${requestingUserId.id}' and isTeamManager)
+       |          or _user = '${requestingUserId.id}'
+       |          or (select _organization from webknossos.teams where webknossos.teams._id = _team)
+       |            in (select _organization from webknossos.users_ where _id = '${requestingUserId.id}' and isAdmin)
+       |""".stripMargin
+
   override def readAccessQ(requestingUserId: ObjectId) =
-    s"""(visibility = '${AnnotationVisibility.Public}'
-          or (visibility = '${AnnotationVisibility.Internal}' and (select _organization from webknossos.teams where webknossos.teams._id = _team)
-            in (select _organization from webknossos.users_ where _id = '${requestingUserId.id}'))
-          or _team in (select _team from webknossos.user_team_roles where _user = '${requestingUserId.id}' and isTeamManager)
-          or _user = '${requestingUserId.id}'
-          or (select _organization from webknossos.teams where webknossos.teams._id = _team)
-            in (select _organization from webknossos.users_ where _id = '${requestingUserId.id}' and isAdmin))"""
+    s"""(visibility = '${AnnotationVisibility.Public}' or ${listAccessQ(requestingUserId)})"""
 
   override def deleteAccessQ(requestingUserId: ObjectId) =
     s"""(_team in (select _team from webknossos.user_team_roles where isTeamManager and _user = '${requestingUserId.id}') or _user = '${requestingUserId.id}'
@@ -224,6 +229,31 @@ class AnnotationDAO @Inject()(sqlClient: SQLClient, annotationLayerDAO: Annotati
     } yield parsed
   }
 
+  def findAllListableExplorationals(isFinished: Option[Boolean], limit: Int, pageNumber: Int = 0)(
+      implicit ctx: DBAccessContext): Fox[List[Annotation]] = {
+    val stateQuery = getStateQuery(isFinished)
+    for {
+      accessQuery <- accessQueryFromAccessQ(listAccessQ)
+      r <- run(sql"""select #$columns from #$existingCollectionName
+                     where typ = '#${AnnotationType.Explorational.toString}' and #$stateQuery and #$accessQuery
+                     order by _id desc limit $limit offset ${pageNumber * limit}""".as[AnnotationsRow])
+      parsed <- parseAll(r)
+    } yield parsed
+  }
+
+  def countAllListableExplorationals(isFinished: Option[Boolean])(
+      implicit ctx: DBAccessContext): Fox[List[Annotation]] = {
+    val stateQuery = getStateQuery(isFinished)
+    for {
+      accessQuery <- accessQueryFromAccessQ(listAccessQ)
+      r <- run(
+        sql"""select count(_id) from #$existingCollectionName
+                     typ = '#${AnnotationType.Explorational.toString}' and #$stateQuery and #$accessQuery"""
+          .as[AnnotationsRow])
+      parsed <- parseAll(r)
+    } yield parsed
+  }
+
   def findActiveTaskIdsForUser(userId: ObjectId): Fox[List[ObjectId]] = {
 
     val stateQuery = getStateQuery(isFinished = Some(false))

diff --git a/app/models/annotation/AnnotationService.scala b/app/models/annotation/AnnotationService.scala
@@ -41,7 +41,7 @@ import models.mesh.{MeshDAO, MeshService}
 import models.organization.OrganizationDAO
 import models.project.ProjectDAO
 import models.task.{Task, TaskDAO, TaskService, TaskTypeDAO}
-import models.team.{Team, TeamDAO}
+import models.team.{TeamDAO, TeamService}
 import models.user.{User, UserDAO, UserService}
 import net.liftweb.common.{Box, Full}
 import play.api.i18n.{Messages, MessagesProvider}
@@ -89,6 +89,7 @@ class AnnotationService @Inject()(
     taskDAO: TaskDAO,
     teamDAO: TeamDAO,
     userService: UserService,
+    teamService: TeamService,
     dataStoreDAO: DataStoreDAO,
     projectDAO: ProjectDAO,
     organizationDAO: OrganizationDAO,
@@ -104,6 +105,8 @@ class AnnotationService @Inject()(
     with LazyLogging {
   implicit val actorSystem: ActorSystem = ActorSystem()
 
+  val DefaultAnnotationListLimit = 1000
+
   private def selectSuitableTeam(user: User, dataSet: DataSet): Fox[ObjectId] =
     (for {
       userTeamIds <- userService.teamIdsFor(user._id)
@@ -556,13 +559,6 @@ class AnnotationService @Inject()(
       implicit ctx: DBAccessContext): Fox[Unit] =
     sharedAnnotationsDAO.updateTeamsForSharedAnnotation(annotationId, teams)
 
-  def sharedTeamsFor(annotationId: ObjectId)(implicit ctx: DBAccessContext): Fox[List[Team]] =
-    for {
-      teamIds <- sharedAnnotationsDAO.sharedTeamsFor(annotationId)
-      teamIdsValidated <- Fox.serialCombined(teamIds)(ObjectId.parse(_))
-      teams <- Fox.serialCombined(teamIdsValidated)(teamDAO.findOne(_))
-    } yield teams
-
   def zipAnnotations(annotations: List[Annotation], zipFileName: String, skipVolumeData: Boolean)(
       implicit
       ctx: DBAccessContext): Fox[TemporaryFile] =
@@ -788,6 +784,8 @@ class AnnotationService @Inject()(
       dataStoreJs <- dataStoreService.publicWrites(dataStore)
       meshes <- meshDAO.findAllWithAnnotation(annotation._id)
       meshesJs <- Fox.serialCombined(meshes)(meshService.publicWrites)
+      teams <- teamDAO.findSharedTeamsForAnnotation(annotation._id)
+      teamsJson <- Fox.serialCombined(teams)(teamService.publicWrites(_))
       tracingStore <- tracingStoreDAO.findFirst
       tracingStoreJs <- tracingStoreService.publicWrites(tracingStore)
     } yield {
@@ -811,6 +809,7 @@ class AnnotationService @Inject()(
         "visibility" -> annotation.visibility,
         "settings" -> settings,
         "tracingTime" -> annotation.tracingTime,
+        "teams" -> teamsJson,
         "tags" -> (annotation.tags ++ Set(dataSet.name, annotation.tracingType.toString)),
         "user" -> userJson,
         "owner" -> userJson,
@@ -833,11 +832,14 @@ class AnnotationService @Inject()(
     }
 
   //for Explorative Annotations list
-  def compactWrites(annotation: Annotation): Fox[JsObject] =
+  def compactWrites(annotation: Annotation): Fox[JsObject] = {
+    implicit val ctx: DBAccessContext = GlobalAccessContext
     for {
-      dataSet <- dataSetDAO.findOne(annotation._dataSet)(GlobalAccessContext) ?~> "dataSet.notFoundForAnnotation"
-      organization <- organizationDAO.findOne(dataSet._organization)(GlobalAccessContext) ?~> "organization.notFound"
-      user <- userDAO.findOne(annotation._user)(GlobalAccessContext)
+      dataSet <- dataSetDAO.findOne(annotation._dataSet) ?~> "dataSet.notFoundForAnnotation"
+      organization <- organizationDAO.findOne(dataSet._organization) ?~> "organization.notFound"
+      teams <- teamDAO.findSharedTeamsForAnnotation(annotation._id) ?~> s"fetching sharedTeams for annotation ${annotation._id} failed"
+      teamsJson <- Fox.serialCombined(teams)(teamService.publicWrites(_, Some(organization))) ?~> s"serializing sharedTeams for annotation ${annotation._id} failed"
+      user <- userDAO.findOne(annotation._user) ?~> s"fetching owner info for annotation ${annotation._id} failed"
       userJson = Json.obj(
         "id" -> user._id.toString,
         "firstName" -> user.firstName,
@@ -858,8 +860,10 @@ class AnnotationService @Inject()(
         "organization" -> organization.name,
         "visibility" -> annotation.visibility,
         "tracingTime" -> annotation.tracingTime,
+        "teams" -> teamsJson,
         "tags" -> (annotation.tags ++ Set(dataSet.name, annotation.tracingType.toString)),
         "owner" -> userJson
       )
     }
+  }
 }
diff --git a/app/models/team/Team.scala b/app/models/team/Team.scala
@@ -146,6 +146,15 @@ class TeamDAO @Inject()(sqlClient: SQLClient)(implicit ec: ExecutionContext)
       parsed <- parseAll(r)
     } yield parsed
 
+  def findSharedTeamsForAnnotation(annotationId: ObjectId)(implicit ctx: DBAccessContext): Fox[List[Team]] =
+    for {
+      accessQuery <- readAccessQuery
+      r <- run(sql"""select #$columns from #$existingCollectionName
+              where _id in (select _team from webknossos.annotation_sharedTeams where _annotation = $annotationId)
+              and #$accessQuery""".as[TeamsRow])
+      parsed <- parseAll(r)
+    } yield parsed
+
   def insertOne(t: Team): Fox[Unit] =
     for {
       _ <- run(sqlu"""insert into webknossos.teams(_id, _organization, name, created, isOrganizationTeam, isDeleted)

diff --git a/conf/webknossos.latest.routes b/conf/webknossos.latest.routes
@@ -136,6 +136,7 @@ GET           /annotations/:typ/:id/download
 GET           /annotations/:typ/:id/loggedTime                                          controllers.AnnotationController.loggedTime(typ: String, id: String)
 
 GET           /annotations/shared                                                       controllers.AnnotationController.sharedAnnotations()
+GET           /annotations/readable                                                     controllers.AnnotationController.listExplorationals(isFinished: Option[Boolean], limit: Option[Int], pageNumber: Option[Int], includeTotalCount: Option[Boolean])
 GET           /annotations/:typ/:id/sharedTeams                                         controllers.AnnotationController.getSharedTeams(typ: String, id: String)
 PATCH         /annotations/:typ/:id/sharedTeams                                         controllers.AnnotationController.updateSharedTeams(typ: String, id: String)