refactor: Move GroupStatus update from responder to services (DEV-3292)…

… (#3195)
dasch-swiss · Apr 19, 2024 · 3e08df7 · 3e08df7
1 parent d5f2294
commit 3e08df7
Show file tree

Hide file tree

Showing 12 changed files with 161 additions and 59 deletions.
diff --git a/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala b/integration/src/test/scala/org/knora/webapi/responders/admin/GroupsResponderADMSpec.scala
@@ -7,8 +7,6 @@ package org.knora.webapi.responders.admin
 
 import zio._
 
-import java.util.UUID
-
 import dsp.errors._
 import org.knora.webapi._
 import org.knora.webapi.messages.admin.responder.usersmessages._
@@ -154,7 +152,7 @@ class GroupsResponderADMSpec extends CoreSpec {
         group.selfjoin should equal(false)
       }
 
-      "return 'NotFound' if a not-existing group IRI is submitted during update" in {
+      "return 'NotFoundException' if a not-existing group IRI is submitted during update" in {
         val groupIri = "http://rdfh.ch/groups/0000/notexisting"
         val exit = UnsafeZioRun.run(
           groupRestService(
@@ -204,7 +202,7 @@ class GroupsResponderADMSpec extends CoreSpec {
         )
       }
 
-      "return 'BadRequest' if nothing would be changed during the update" in {
+      "return 'BadRequestException' if nothing would be changed during the update" in {
         val exit = UnsafeZioRun.run(
           groupRestService(
             _.putGroup(
@@ -245,11 +243,11 @@ class GroupsResponderADMSpec extends CoreSpec {
         group.members.size shouldBe 2
 
         val statusChangeResponse = UnsafeZioRun.runOrThrow(
-          ZIO.serviceWithZIO[GroupsResponderADM](
-            _.updateGroupStatus(
+          groupRestService(
+            _.putGroupStatus(
               GroupIri.unsafeFrom(imagesReviewerGroup.id),
               GroupStatusUpdateRequest(GroupStatus.inactive),
-              UUID.randomUUID(),
+              rootUser,
             ),
           ),
         )

diff --git a/webapi/src/main/scala/org/knora/webapi/responders/admin/GroupsResponderADM.scala b/webapi/src/main/scala/org/knora/webapi/responders/admin/GroupsResponderADM.scala
@@ -28,7 +28,6 @@ import org.knora.webapi.messages.util.KnoraSystemInstances
 import org.knora.webapi.responders.IriLocker
 import org.knora.webapi.responders.IriService
 import org.knora.webapi.responders.Responder
-import org.knora.webapi.slice.admin.api.GroupsRequests.GroupStatusUpdateRequest
 import org.knora.webapi.slice.admin.api.GroupsRequests.GroupUpdateRequest
 import org.knora.webapi.slice.admin.domain.model
 import org.knora.webapi.slice.admin.domain.model.Group
@@ -181,29 +180,6 @@ final case class GroupsResponderADM(
   def groupMembersGetRequest(iri: GroupIri, user: User): Task[GroupMembersGetResponseADM] =
     groupMembersGetADM(iri.value, user).map(GroupMembersGetResponseADM.apply)
 
-  /**
-   * Change group's status.
-   *
-   * @param groupIri             the IRI of the group we want to change.
-   * @param request              the change request.
-   * @param apiRequestID         the unique request ID.
-   * @return a [[GroupGetResponseADM]].
-   */
-  def updateGroupStatus(
-    groupIri: GroupIri,
-    request: GroupStatusUpdateRequest,
-    apiRequestID: UUID,
-  ): Task[GroupGetResponseADM] = {
-    val task = for {
-      // update group status
-      updated <- updateGroupHelper(groupIri, GroupUpdateRequest(None, None, Some(request.status), None))
-
-      // remove all members from group if status is false
-      result <- removeGroupMembersIfNecessary(updated.group)
-    } yield result
-    IriLocker.runWithIriLock(apiRequestID, groupIri.value, task)
-  }
-
   def deleteGroup(
     iri: GroupIri,
     apiRequestID: UUID,

diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/GroupRestService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/GroupRestService.scala
@@ -75,9 +75,11 @@ final case class GroupRestService(
 
   def putGroupStatus(iri: GroupIri, request: GroupStatusUpdateRequest, user: User): Task[GroupGetResponseADM] =
     for {
-      _        <- auth.ensureSystemAdminOrProjectAdminOfGroup(user, iri)
-      uuid     <- Random.nextUUID
-      internal <- responder.updateGroupStatus(iri, request, uuid)
+      _ <- auth.ensureSystemAdminOrProjectAdminOfGroup(user, iri)
+      groupToUpdate <- groupService
+                         .findById(iri)
+                         .someOrFail(NotFoundException(s"Group <${iri.value}> not found."))
+      internal <- groupService.updateGroupStatus(groupToUpdate, request.status).map(GroupGetResponseADM.apply)
       external <- format.toExternalADM(internal)
     } yield external
 

diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/AdminDomainModule.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/AdminDomainModule.scala
@@ -10,7 +10,7 @@ import zio.ZLayer
 import org.knora.webapi.config.AppConfig
 import org.knora.webapi.responders.IriService
 import org.knora.webapi.slice.admin.domain.service.GroupService
-import org.knora.webapi.slice.admin.domain.service.KnoraGroupService.KnoraGroupService
+import org.knora.webapi.slice.admin.domain.service.KnoraGroupService
 import org.knora.webapi.slice.admin.domain.service.KnoraProjectService
 import org.knora.webapi.slice.admin.domain.service.PasswordService
 import org.knora.webapi.slice.admin.domain.service.ProjectService
@@ -24,10 +24,23 @@ import org.knora.webapi.store.triplestore.api.TriplestoreService
 object AdminDomainModule {
 
   type Dependencies =
-    AppConfig & AdminRepoModule.Provided & CacheService & IriService & OntologyRepo & PredicateObjectMapper & TriplestoreService
+    AppConfig &
+      AdminRepoModule.Provided &
+      CacheService &
+      IriService &
+      OntologyRepo &
+      PredicateObjectMapper &
+      TriplestoreService
 
   type Provided =
-    AdministrativePermissionService & GroupService & KnoraGroupService & KnoraProjectService & KnoraUserService & MaintenanceService & PasswordService & ProjectService
+    AdministrativePermissionService &
+      GroupService &
+      KnoraGroupService &
+      KnoraProjectService &
+      KnoraUserService &
+      MaintenanceService &
+      PasswordService &
+      ProjectService
 
   val layer = ZLayer.makeSome[Dependencies, Provided](
     AdministrativePermissionService.layer,

diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/GroupService.scala
@@ -33,7 +33,7 @@ final case class GroupService(
 
   private def toGroups(knoraGroups: Chunk[KnoraGroup]): Task[Chunk[Group]] = ZIO.foreach(knoraGroups)(toGroup)
 
-  private def toGroup(knoraGroup: KnoraGroup): Task[Group] =
+  private[service] def toGroup(knoraGroup: KnoraGroup): Task[Group] =
     for {
       project <- knoraGroup.belongsToProject.map(projectService.findById).getOrElse(ZIO.none)
     } yield Group(
@@ -45,7 +45,7 @@ final case class GroupService(
       selfjoin = knoraGroup.hasSelfJoinEnabled.value,
     )
 
-  private def toKnoraGroup(group: Group): KnoraGroup =
+  private[service] def toKnoraGroup(group: Group): KnoraGroup =
     KnoraGroup(
       id = GroupIri.unsafeFrom(group.id),
       groupName = GroupName.unsafeFrom(group.name),
@@ -60,6 +60,9 @@ final case class GroupService(
 
   def updateGroup(groupToUpdate: Group, request: GroupUpdateRequest): Task[Group] =
     knoraGroupService.updateGroup(toKnoraGroup(groupToUpdate), request).flatMap(toGroup)
+
+  def updateGroupStatus(groupToUpdate: Group, status: GroupStatus): Task[Group] =
+    knoraGroupService.updateGroupStatus(toKnoraGroup(groupToUpdate), status).flatMap(toGroup)
 }
 
 object GroupService {

diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraGroupService.scala
@@ -16,11 +16,13 @@ import org.knora.webapi.slice.admin.api.GroupsRequests.GroupCreateRequest
 import org.knora.webapi.slice.admin.api.GroupsRequests.GroupUpdateRequest
 import org.knora.webapi.slice.admin.domain.model.GroupIri
 import org.knora.webapi.slice.admin.domain.model.GroupName
+import org.knora.webapi.slice.admin.domain.model.GroupStatus
 import org.knora.webapi.slice.admin.domain.model.KnoraGroup
 import org.knora.webapi.slice.admin.domain.model.KnoraProject
 
 case class KnoraGroupService(
   knoraGroupRepo: KnoraGroupRepo,
+  knoraUserService: KnoraUserService,
   iriService: IriService,
 ) {
 
@@ -50,10 +52,7 @@ case class KnoraGroupService(
 
   def updateGroup(groupToUpdate: KnoraGroup, request: GroupUpdateRequest): Task[KnoraGroup] =
     for {
-      _ <- request.name match {
-             case Some(value) => ensureGroupNameIsUnique(value)
-             case None        => ZIO.unit
-           }
+      _ <- ZIO.foreachDiscard(request.name)(ensureGroupNameIsUnique)
 
       updatedGroup <-
         knoraGroupRepo.save(
@@ -66,14 +65,20 @@ case class KnoraGroupService(
         )
     } yield updatedGroup
 
+  def updateGroupStatus(groupToUpdate: KnoraGroup, status: GroupStatus): Task[KnoraGroup] =
+    for {
+      group <- knoraGroupRepo.save(groupToUpdate.copy(status = status))
+      _ <- ZIO.unless(group.status.value)(knoraUserService.findByGroupMembership(group.id).flatMap { members =>
+             ZIO.foreachDiscard(members)(user => knoraUserService.removeUserFromKnoraGroup(user, group.id))
+           })
+    } yield group
+
   private def ensureGroupNameIsUnique(name: GroupName) =
     ZIO.whenZIO(knoraGroupRepo.existsByName(name)) {
       ZIO.fail(DuplicateValueException(s"Group with name: '${name.value}' already exists."))
     }
 }
 
 object KnoraGroupService {
-  object KnoraGroupService {
-    val layer = ZLayer.derive[KnoraGroupService]
-  }
+  val layer = ZLayer.derive[KnoraGroupService]
 }
diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserRepo.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserRepo.scala
@@ -50,6 +50,8 @@ trait KnoraUserRepo extends Repository[KnoraUser, UserIri] {
 
   def findByProjectAdminMembership(projectIri: ProjectIri): Task[Chunk[KnoraUser]]
 
+  def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]]
+
   /**
    * Saves a given user. Use the returned instance for further operations as the save operation might have changed the entity instance completely.
    *

diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala
@@ -51,6 +51,9 @@ case class KnoraUserService(
   def findByProjectAdminMembership(project: KnoraProject): Task[Chunk[KnoraUser]] =
     userRepo.findByProjectAdminMembership(project.id)
 
+  def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]] =
+    userRepo.findByGroupMembership(groupIri)
+
   def updateSystemAdminStatus(knoraUser: KnoraUser, status: SystemAdmin): Task[KnoraUser] =
     updateUser(knoraUser, UserChangeRequest(systemAdmin = Some(status)))
 
@@ -138,10 +141,13 @@ case class KnoraUserService(
   def removeUserFromGroup(user: KnoraUser, group: Group): IO[UserServiceError, KnoraUser] = for {
     _ <- ZIO
            .fail(UserServiceError(s"User ${user.id.value} is not member of group ${group.groupIri.value}."))
-           .when(!user.isInGroup.contains(group.groupIri))
+           .unless(user.isInGroup.contains(group.groupIri))
     user <- updateUser(user, UserChangeRequest(groups = Some(user.isInGroup.filterNot(_ == group.groupIri)))).orDie
   } yield user
 
+  def removeUserFromKnoraGroup(user: KnoraUser, groupIri: GroupIri): UIO[KnoraUser] =
+    userRepo.save(user.copy(isInGroup = user.isInGroup.filterNot(_ == groupIri))).orDie
+
   def addUserToProject(user: KnoraUser, project: Project): IO[UserServiceError, KnoraUser] = for {
     _ <- ZIO
            .fail(UserServiceError(s"User ${user.id.value} is already member of project ${project.projectIri.value}."))
@@ -163,7 +169,7 @@ case class KnoraUserService(
   ): IO[UserServiceError, KnoraUser] = for {
     _ <- ZIO
            .fail(UserServiceError(s"User ${user.id.value} is not member of project ${project.projectIri.value}."))
-           .when(!user.isInProject.contains(project.projectIri))
+           .unless(user.isInProject.contains(project.projectIri))
     projectIri               = project.projectIri
     newIsInProject           = user.isInProject.filterNot(_ == projectIri)
     newIsInProjectAdminGroup = user.isInProjectAdminGroup.filterNot(_ == projectIri)
@@ -194,7 +200,7 @@ case class KnoraUserService(
         val msg =
           s"User ${user.id.value} is not a member of project ${project.projectIri.value}. A user needs to be a member of the project to be added as project admin."
         UserServiceError(msg)
-      }.when(!user.isInProject.contains(project.projectIri))
+      }.unless(user.isInProject.contains(project.projectIri))
     theChange = UserChangeRequest(projectsAdmin = Some(user.isInProjectAdminGroup :+ project.projectIri))
     user     <- updateUser(user, theChange).orDie
   } yield user
@@ -213,7 +219,7 @@ case class KnoraUserService(
   ): IO[UserServiceError, KnoraUser] = for {
     _ <- ZIO
            .fail(UserServiceError(s"User ${user.id.value} is not admin member of project ${project.projectIri.value}."))
-           .when(!user.isInProjectAdminGroup.contains(project.projectIri))
+           .unless(user.isInProjectAdminGroup.contains(project.projectIri))
     theChange = UserChangeRequest(projectsAdmin = Some(user.isInProjectAdminGroup.filterNot(_ == project.projectIri)))
     user     <- updateUser(user, theChange).orDie
   } yield user

diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/repo/service/KnoraUserRepoLive.scala
@@ -67,6 +67,10 @@ final case class KnoraUserRepoLive(
     findAllByTriplePattern(_.has(isInProject, Rdf.iri(projectIri.value)))
       .map(_ ++ KnoraUserRepo.builtIn.findAllBy(_.isInProject.contains(projectIri)))
 
+  override def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]] =
+    findAllByTriplePattern(_.has(isInGroup, Rdf.iri(groupIri.value)))
+      .map(_ ++ KnoraUserRepo.builtIn.findAllBy(_.isInGroup.contains(groupIri)))
+
   override def findByEmail(mail: Email): Task[Option[KnoraUser]] =
     findOneByTriplePattern(_.has(email, Rdf.literalOf(mail.value)))
       .map(_.orElse(KnoraUserRepo.builtIn.findOneBy(_.email == mail)))

diff --git a/...rc/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala b/...rc/test/scala/org/knora/webapi/slice/admin/api/service/AuthorizationRestServiceSpec.scala
@@ -19,10 +19,13 @@ import org.knora.webapi.responders.IriService
 import org.knora.webapi.slice.admin.domain.model.User
 import org.knora.webapi.slice.admin.domain.repo.KnoraProjectRepoInMemory
 import org.knora.webapi.slice.admin.domain.service.KnoraGroupRepo
-import org.knora.webapi.slice.admin.domain.service.KnoraGroupService.KnoraGroupService
+import org.knora.webapi.slice.admin.domain.service.KnoraGroupService
 import org.knora.webapi.slice.admin.domain.service.KnoraProjectRepo.builtIn.SystemProject
 import org.knora.webapi.slice.admin.domain.service.KnoraProjectService
+import org.knora.webapi.slice.admin.domain.service.KnoraUserService
+import org.knora.webapi.slice.admin.domain.service.PasswordService
 import org.knora.webapi.slice.admin.repo.service.KnoraGroupRepoInMemory
+import org.knora.webapi.slice.admin.repo.service.KnoraUserRepoLive
 import org.knora.webapi.slice.common.api.AuthorizationRestService
 import org.knora.webapi.slice.resourceinfo.domain.IriConverter
 import org.knora.webapi.store.cache.CacheService
@@ -131,16 +134,19 @@ object AuthorizationRestServiceSpec extends ZIOSpecDefault {
       },
     ),
   ).provide(
+    AppConfig.layer,
     AuthorizationRestService.layer,
     CacheService.layer,
-    KnoraProjectService.layer,
-    KnoraProjectRepoInMemory.layer,
-    KnoraGroupService.layer,
-    KnoraGroupRepoInMemory.layer,
-    IriService.layer,
     IriConverter.layer,
-    TriplestoreServiceLive.layer,
+    IriService.layer,
+    KnoraGroupRepoInMemory.layer,
+    KnoraGroupService.layer,
+    KnoraProjectRepoInMemory.layer,
+    KnoraProjectService.layer,
+    KnoraUserRepoLive.layer,
+    KnoraUserService.layer,
+    PasswordService.layer,
     StringFormatter.live,
-    AppConfig.layer,
+    TriplestoreServiceLive.layer,
   )
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -50,6 +50,8 @@ trait KnoraUserRepo extends Repository[KnoraUser, UserIri] { @@
       def findByProjectAdminMembership(projectIri: ProjectIri): Task[Chunk[KnoraUser]]
+      def findByGroupMembership(groupIri: GroupIri): Task[Chunk[KnoraUser]]
       /**
        * Saves a given user. Use the returned instance for further operations as the save operation might have changed the entity instance completely.
        *
@@ Expand Down @@