refactor

NDLANO · Oct 28, 2024 · 8805ec3 · 8805ec3
1 parent 493746e
commit 8805ec3
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 29 deletions.
diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/ReadService.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/ReadService.scala
@@ -18,7 +18,7 @@ import no.ndla.learningpathapi.model.domain.UserInfo.LearningpathCombinedUser
 import no.ndla.learningpathapi.model.domain.{InvalidLpStatusException, StepStatus, LearningPathStatus as _}
 import no.ndla.learningpathapi.repository.LearningPathRepositoryComponent
 import no.ndla.network.clients.{FeideApiClient, MyNDLAApiClient, RedisClient}
-import no.ndla.network.model.CombinedUser
+import no.ndla.network.model.{CombinedUser, CombinedUserRequired}
 
 import scala.math.max
 import scala.util.{Failure, Success, Try}
@@ -37,9 +37,9 @@ trait ReadService {
       learningPathRepository.allPublishedContributors.map(author => commonApi.Author(author.`type`, author.name))
     }
 
-    def withOwnerV2(user: CombinedUser): List[LearningPathSummaryV2] = {
+    def withOwnerV2(user: CombinedUserRequired): List[LearningPathSummaryV2] = {
       learningPathRepository
-        .withOwner(user.id.get) // TODO: Make this cooler than .get please
+        .withOwner(user.id)
         .flatMap(value => converterService.asApiLearningpathSummaryV2(value, user).toOption)
     }
 

diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/UpdateService.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/UpdateService.scala
@@ -21,7 +21,7 @@ import no.ndla.learningpathapi.repository.LearningPathRepositoryComponent
 import no.ndla.learningpathapi.service.search.SearchIndexService
 import no.ndla.learningpathapi.validation.{LearningPathValidator, LearningStepValidator}
 import no.ndla.network.clients.{FeideApiClient, RedisClient}
-import no.ndla.network.model.CombinedUser
+import no.ndla.network.model.{CombinedUser, CombinedUserRequired}
 import no.ndla.network.tapir.auth.TokenUser
 
 import scala.annotation.tailrec
@@ -149,23 +149,21 @@ trait UpdateService {
     def updateLearningPathStatusV2(
         learningPathId: Long,
         status: LearningPathStatus.Value,
-        owner: CombinedUser,
+        owner: CombinedUserRequired,
         language: String,
         message: Option[String] = None
     ): Try[LearningPathV2] =
       writeDuringWriteRestrictionOrAccessDenied(owner) {
-        withId(learningPathId, includeDeleted = true).flatMap(_.canSetStatus(status, owner)) match {
-          case Failure(ex) => Failure(ex)
-          case Success(existing) =>
+        withId(learningPathId, includeDeleted = true)
+          .flatMap(_.canSetStatus(status, owner))
+          .flatMap { existing =>
             val validatedLearningPath =
               if (status == domain.LearningPathStatus.PUBLISHED) existing.validateForPublishing() else Success(existing)
 
             validatedLearningPath.flatMap(valid => {
               val newMessage = message match {
-                case Some(msg) if owner.isAdmin =>
-                  // TODO: Maybe enforce the id with some typings?
-                  Some(domain.Message(msg, owner.id.getOrElse("Invalid user id"), clock.now()))
-                case _ => valid.message
+                case Some(msg) if owner.isAdmin => Some(domain.Message(msg, owner.id, clock.now()))
+                case _                          => valid.message
               }
 
               val updatedLearningPath = learningPathRepository.update(
@@ -183,7 +181,7 @@ trait UpdateService {
                 )
 
             })
-        }
+          }
       }
 
     private[service] def deleteIsBasedOnReference(updatedLearningPath: domain.LearningPath): Unit = {

diff --git a/network/src/main/scala/no/ndla/network/model/CombinedUser.scala b/network/src/main/scala/no/ndla/network/model/CombinedUser.scala
@@ -11,7 +11,29 @@ package no.ndla.network.model
 import no.ndla.common.model.api.myndla.MyNDLAUser
 import no.ndla.network.tapir.auth.TokenUser
 
-case class CombinedUser(
-    tokenUser: Option[TokenUser],
-    myndlaUser: Option[MyNDLAUser]
-)
+sealed trait CombinedUser {
+  val tokenUser: Option[TokenUser]
+  val myndlaUser: Option[MyNDLAUser]
+}
+
+case class OptionalCombinedUser(tokenUser: Option[TokenUser], myndlaUser: Option[MyNDLAUser]) extends CombinedUser
+
+trait CombinedUserRequired extends CombinedUser {
+  def id: String
+}
+
+case class CombinedUserWithTokenUser(user: TokenUser, myndlaUser: Option[MyNDLAUser]) extends CombinedUserRequired {
+  override def id: FeideID         = user.id
+  val tokenUser: Option[TokenUser] = Some(user)
+}
+
+case class CombinedUserWithMyNDLAUser(tokenUser: Option[TokenUser], user: MyNDLAUser) extends CombinedUserRequired {
+  override def id: String            = user.feideId
+  val myndlaUser: Option[MyNDLAUser] = Some(user)
+}
+
+case class CombinedUserWithBoth(user: TokenUser, ndlaUser: MyNDLAUser) extends CombinedUserRequired {
+  override def id: String            = user.id
+  val tokenUser: Option[TokenUser]   = Some(user)
+  val myndlaUser: Option[MyNDLAUser] = Some(ndlaUser)
+}
diff --git a/network/src/main/scala/no/ndla/network/tapir/TapirController.scala b/network/src/main/scala/no/ndla/network/tapir/TapirController.scala
@@ -14,7 +14,14 @@ import no.ndla.common.configuration.HasBaseProps
 import no.ndla.common.model.api.myndla.MyNDLAUser
 import no.ndla.common.model.domain.myndla.auth.AuthUtility
 import no.ndla.network.clients.MyNDLAApiClient
-import no.ndla.network.model.{CombinedUser, HttpRequestException}
+import no.ndla.network.model.{
+  CombinedUser,
+  CombinedUserRequired,
+  CombinedUserWithBoth,
+  CombinedUserWithMyNDLAUser,
+  HttpRequestException,
+  OptionalCombinedUser
+}
 import no.ndla.network.tapir.auth.{Permission, TokenUser}
 import sttp.client3.Identity
 import sttp.model.StatusCode
@@ -119,21 +126,20 @@ trait TapirController extends TapirErrorHandling {
               }
             }
 
-            val combinedUser = CombinedUser(maybeUser, myndlaUser)
+            val combinedUser = OptionalCombinedUser(maybeUser, myndlaUser)
             Right(combinedUser)
           }
         val securityLogic = (m: MonadError[F]) => (a: (Option[TokenUser], Option[String])) => m.unit(authFunc(a))
         PartialServerEndpoint(newEndpoint, securityLogic)
       }
 
-      // TODO: Can we type this up somehow?
       def withRequiredMyNDLAUserOrTokenUser[F[_]]
-          : PartialServerEndpoint[(Option[TokenUser], Option[String]), CombinedUser, I, AllErrors, O, R, F] = {
+          : PartialServerEndpoint[(Option[TokenUser], Option[String]), CombinedUserRequired, I, AllErrors, O, R, F] = {
         val newEndpoint = self
           .securityIn(TokenUser.oauth2Input(Seq.empty))
           .securityIn(AuthUtility.feideOauth())
 
-        val authFunc: ((Option[TokenUser], Option[String])) => Either[AllErrors, CombinedUser] =
+        val authFunc: ((Option[TokenUser], Option[String])) => Either[AllErrors, CombinedUserRequired] =
           (userInputOptions: (Option[TokenUser], Option[String])) => {
             val maybeUser  = userInputOptions._1
             val maybeToken = userInputOptions._2
@@ -150,11 +156,11 @@ trait TapirController extends TapirErrorHandling {
               }
             }
 
-            val combinedUser = CombinedUser(maybeUser, myndlaUser)
-            if (combinedUser.tokenUser.isEmpty && combinedUser.myndlaUser.isEmpty) {
-              ErrorHelpers.unauthorized.asLeft
-            } else {
-              Right(combinedUser)
+            (maybeUser, myndlaUser) match {
+              case (Some(tokenUser), Some(ndlaUser)) => CombinedUserWithBoth(tokenUser, ndlaUser).asRight
+              case (Some(tokenUser), None)           => tokenUser.toCombined.asRight
+              case (None, Some(ndlaUser))            => CombinedUserWithMyNDLAUser(None, ndlaUser).asRight
+              case _                                 => ErrorHelpers.unauthorized.asLeft
             }
           }
         val securityLogic = (m: MonadError[F]) => (a: (Option[TokenUser], Option[String])) => m.unit(authFunc(a))

diff --git a/network/src/main/scala/no/ndla/network/tapir/auth/TokenUser.scala b/network/src/main/scala/no/ndla/network/tapir/auth/TokenUser.scala
@@ -9,7 +9,7 @@ package no.ndla.network.tapir.auth
 
 import cats.implicits.*
 import no.ndla.network.jwt.JWTExtractor
-import no.ndla.network.model.{CombinedUser, JWTClaims}
+import no.ndla.network.model.{CombinedUserWithTokenUser, JWTClaims}
 import sttp.model.HeaderNames
 import sttp.model.headers.{AuthenticationScheme, WWWAuthenticateChallenge}
 import sttp.tapir.CodecFormat.TextPlain
@@ -27,7 +27,7 @@ case class TokenUser(
 ) {
   def hasPermission(permission: Permission): Boolean             = permissions.contains(permission)
   def hasPermissions(permissions: Iterable[Permission]): Boolean = permissions.forall(hasPermission)
-  def toCombined: CombinedUser                                   = CombinedUser(Some(this), None)
+  def toCombined: CombinedUserWithTokenUser                      = CombinedUserWithTokenUser(this, None)
 }
 
 object TokenUser {