refactor(projects): cleaner value objects usage in addProject route (DEV-119) #1920

subotic · 2021-10-08T17:59:22Z

webapi/src/main/scala/org/knora/webapi/routing/admin/ProjectsRouteADM.scala

irinaschubert

It's basically just one comment (just added it on several locations). Apart from that I think it looks good.

irinaschubert · 2021-10-11T15:39:58Z

webapi/src/main/scala/org/knora/webapi/routing/KnoraRoute.scala

+  /**
+   * Helper method converting an [[Either]] to a [[Future]].
+   */
+  def toFuture[A](either: Either[Throwable, A]): Future[A] = either.fold(Future.failed, Future.successful)


Do we need this (i.e. do we still want to use Either in the future and not Validation everywhere)?

irinaschubert · 2021-10-11T15:42:18Z

webapi/src/main/scala/org/knora/webapi/routing/admin/UsersRouteADM.scala

@@ -145,7 +145,7 @@ class UsersRouteADM(routeData: KnoraRouteData) extends KnoraRoute(routeData) wit
            givenName = GivenName.create(apiRequest.givenName).fold(error => throw error, value => value),
            familyName = FamilyName.create(apiRequest.familyName).fold(error => throw error, value => value),
            password = Password.create(apiRequest.password).fold(error => throw error, value => value),
-            status = Status.create(apiRequest.status).fold(error => throw error, value => value),
+            status = Status.make(apiRequest.status).fold(error => throw error.head, value => value),
            lang = LanguageCode.create(apiRequest.lang).fold(error => throw error, value => value),


Is fold still needed? Can't it be done with Validation.validateWith as in the ProjectsRouteADM.scala?

Validation.validateWith needs all inputs to be of Validation type. The change can be done in another PR.

irinaschubert · 2021-10-11T15:42:32Z

webapi/src/main/scala/org/knora/webapi/routing/admin/UsersRouteADM.scala

@@ -414,7 +414,7 @@ class UsersRouteADM(routeData: KnoraRouteData) extends KnoraRoute(routeData) wit
          }

          val newStatus = apiRequest.status match {
-            case Some(status) => Status.create(status).fold(error => throw error, value => value)
+            case Some(status) => Status.make(status).fold(error => throw error.head, value => value)
            case None         => throw BadRequestException("The status is missing.")


See above, is fold needed?

irinaschubert · 2021-10-11T15:42:43Z

webapi/src/main/scala/org/knora/webapi/routing/admin/UsersRouteADM.scala

@@ -464,7 +464,7 @@ class UsersRouteADM(routeData: KnoraRouteData) extends KnoraRoute(routeData) wit
        }

        /* update existing user's status to false */
-        val status = Status.create(false).fold(error => throw error, value => value)
+        val status = Status.make(false).fold(error => throw error.head, value => value)



See above, is fold needed?

irinaschubert · 2021-10-11T15:46:29Z

.../test/scala/org/knora/webapi/messages/admin/responder/valueObjects/ValueObjectsADMSpec.scala

@@ -55,7 +55,7 @@ class ValueObjectsADMSpec extends UnitSpec(ValueObjectsADMSpec.config) {
      givenName = GivenName.create(createUserApiRequestADM.givenName).fold(error => throw error, value => value),
      familyName = FamilyName.create(createUserApiRequestADM.familyName).fold(error => throw error, value => value),
      password = Password.create(createUserApiRequestADM.password).fold(error => throw error, value => value),
-      status = Status.create(createUserApiRequestADM.status).fold(error => throw error, value => value),
+      status = Status.make(createUserApiRequestADM.status).fold(error => throw error.head, value => value),
      lang = LanguageCode.create(createUserApiRequestADM.lang).fold(error => throw error, value => value),


See above, is fold needed?

irinaschubert · 2021-10-11T15:49:35Z

webapi/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala

@@ -469,7 +469,7 @@ class UsersResponderADMSpec extends CoreSpec(UsersResponderADMSpec.config) with
      "UPDATE the user's status, (deleting) making him inactive " in {
        responderManager ! UserChangeStatusRequestADM(
          userIri = SharedTestDataADM.normalUser.id,
-          status = Status.create(false).fold(error => throw error, value => value),
+          status = Status.make(false).fold(error => throw error.head, value => value),
          featureFactoryConfig = defaultFeatureFactoryConfig,
