diff --git a/common/src/main/scala/no/ndla/common/model/domain/learningpath/LearningPath.scala b/common/src/main/scala/no/ndla/common/model/domain/learningpath/LearningPath.scala index e88801be9f..545594b37c 100644 --- a/common/src/main/scala/no/ndla/common/model/domain/learningpath/LearningPath.scala +++ b/common/src/main/scala/no/ndla/common/model/domain/learningpath/LearningPath.scala @@ -17,6 +17,7 @@ import no.ndla.common.model.domain.Priority import sttp.tapir.Schema import no.ndla.common.DeriveHelpers import no.ndla.common.model.domain.RevisionMeta +import no.ndla.common.model.domain.learningpath.StepStatus.ACTIVE case class LearningPath( id: Option[Long], @@ -35,7 +36,7 @@ case class LearningPath( owner: String, copyright: LearningpathCopyright, isMyNDLAOwner: Boolean, - learningsteps: Option[Seq[LearningStep]] = None, + learningsteps: Seq[LearningStep], message: Option[Message] = None, madeAvailable: Option[NDLADate] = None, responsible: Option[Responsible], @@ -47,27 +48,25 @@ case class LearningPath( ) extends Content { def supportedLanguages: Seq[String] = { - val stepLanguages = learningsteps.getOrElse(Seq.empty).flatMap(_.supportedLanguages) - - ( - getSupportedLanguages(title, description, tags) ++ stepLanguages - ).distinct + val stepLanguages = learningsteps.flatMap(_.supportedLanguages) + val allSupportedLanguages = getSupportedLanguages(title, description, tags) ++ stepLanguages + allSupportedLanguages.distinct } def isPrivate: Boolean = Seq(LearningPathStatus.PRIVATE, LearningPathStatus.READY_FOR_SHARING).contains(status) def isPublished: Boolean = status == LearningPathStatus.PUBLISHED def isDeleted: Boolean = status == LearningPathStatus.DELETED + def withOnlyActiveSteps: LearningPath = { + val activeSteps = learningsteps.filter(_.status == ACTIVE) + this.copy(learningsteps = activeSteps) + } + } object LearningPath { - // NOTE: We remove learningsteps from the JSON object before decoding it since it is stored in a separate table - implicit val encoder: Encoder[LearningPath] = deriveEncoder[LearningPath].mapJsonObject(_.remove("learningsteps")) - implicit val decoder: Decoder[LearningPath] = deriveDecoder[LearningPath].prepare { obj => - val learningsteps = obj.downField("learningsteps") - if (learningsteps.succeeded) learningsteps.delete - else obj - } + implicit val encoder: Encoder[LearningPath] = deriveEncoder[LearningPath] + implicit val decoder: Decoder[LearningPath] = deriveDecoder[LearningPath] import sttp.tapir.generic.auto.* implicit def schema: Schema[LearningPath] = DeriveHelpers.getSchema diff --git a/integration-tests/src/test/scala/no/ndla/integrationtests/searchapi/learningpathapi/LearningpathApiClientTest.scala b/integration-tests/src/test/scala/no/ndla/integrationtests/searchapi/learningpathapi/LearningpathApiClientTest.scala index f3a4930808..49ab39fed8 100644 --- a/integration-tests/src/test/scala/no/ndla/integrationtests/searchapi/learningpathapi/LearningpathApiClientTest.scala +++ b/integration-tests/src/test/scala/no/ndla/integrationtests/searchapi/learningpathapi/LearningpathApiClientTest.scala @@ -83,6 +83,7 @@ class LearningpathApiClientTest .insert( learningpathapi.TestData.sampleDomainLearningPath.copy(id = Some(id), lastUpdated = NDLADate.fromUnixTime(0)) ) + .get }) } diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/controller/InternController.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/controller/InternController.scala index a2e2b22bb8..c965a8dfd2 100644 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/controller/InternController.scala +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/controller/InternController.scala @@ -144,9 +144,7 @@ class InternController(using .in(jsonBody[commonDomain.LearningPath]) .out(jsonBody[commonDomain.LearningPath]) .errorOut(errorOutputsFor(404)) - .serverLogicPure { dumpToInsert => - updateService.insertDump(dumpToInsert).asRight - } + .serverLogicPure(dumpToInsert => updateService.insertDump(dumpToInsert)) private def learningPathStats: ServerEndpoint[Any, Eff] = endpoint .get diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V39__MadeAvailableForThePublished.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V39__MadeAvailableForThePublished.scala index c0a8ee81b6..407e54e5eb 100644 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V39__MadeAvailableForThePublished.scala +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V39__MadeAvailableForThePublished.scala @@ -8,23 +8,6 @@ package no.ndla.learningpathapi.db.migration -import no.ndla.common.CirceUtil -import no.ndla.common.model.domain.learningpath.LearningPath -import no.ndla.common.model.domain.learningpath.LearningPathStatus.{PUBLISHED, UNLISTED} -import no.ndla.database.DocumentMigration +import no.ndla.database.FinishedMigration -class V39__MadeAvailableForThePublished extends DocumentMigration { - override val columnName: String = "document" - override val tableName: String = "learningpaths" - - def convertColumn(document: String): String = { - val oldLp = CirceUtil.unsafeParseAs[LearningPath](document) - val madeAvailable = oldLp.status match { - case UNLISTED | PUBLISHED => Some(oldLp.lastUpdated) - case _ => None - } - - val newLearningPath = oldLp.copy(madeAvailable = madeAvailable) - CirceUtil.toJsonString(newLearningPath) - } -} +class V39__MadeAvailableForThePublished extends FinishedMigration diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocument.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocument.scala new file mode 100644 index 0000000000..bd5f40be94 --- /dev/null +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocument.scala @@ -0,0 +1,125 @@ +/* + * Part of NDLA learningpath-api + * Copyright (C) 2025 NDLA + * + * See LICENSE + * + */ + +package no.ndla.learningpathapi.db.migration + +import com.typesafe.scalalogging.StrictLogging +import io.circe.Json +import no.ndla.common.CirceUtil +import org.flywaydb.core.api.migration.{BaseJavaMigration, Context} +import org.postgresql.util.PGobject +import org.slf4j.MDC +import scalikejdbc.* + +case class LpDocumentRowWithId(learningPathId: Long, learningPathDocument: String) +case class StepDocumentRowWithMeta( + learningStepId: Long, + learningPathId: Long, + revision: Int, + externalId: Option[String], + learningStepDocument: String, +) + +class V60__MoveLearningStepsToLearningPathDocument extends BaseJavaMigration with StrictLogging { + private val chunkSize = 1000 + + override def migrate(context: Context): Unit = DB(context.getConnection) + .autoClose(false) + .withinTx { session => + migrateRows(using session) + } + + private def countAllRows(implicit session: DBSession): Option[Long] = { + sql"select count(*) from learningpaths where document is not null".map(rs => rs.long("count")).single() + } + + private def allLearningPaths(offset: Long)(implicit session: DBSession): List[LpDocumentRowWithId] = { + sql"select id, document from learningpaths where document is not null order by id limit $chunkSize offset $offset" + .map(rs => LpDocumentRowWithId(rs.long("id"), rs.string("document"))) + .list() + } + + private def getStepDatas(learningPathId: Long)(using session: DBSession): List[StepDocumentRowWithMeta] = { + sql""" + select id, learning_path_id, revision, external_id, document + from learningsteps + where learning_path_id = $learningPathId and document is not null + order by id + """ + .map { rs => + StepDocumentRowWithMeta( + learningStepId = rs.long("id"), + learningPathId = rs.long("learning_path_id"), + revision = rs.int("revision"), + externalId = rs.stringOpt("external_id"), + learningStepDocument = rs.string("document"), + ) + } + .list() + } + + private def updateLp( + row: LpDocumentRowWithId, + steps: List[StepDocumentRowWithMeta], + )(using session: DBSession): Unit = { + val updatedLpJson = CirceUtil.tryParse(mergeLearningSteps(row.learningPathDocument, steps)).get + + val dataObject = new PGobject() + dataObject.setType("jsonb") + dataObject.setValue(updatedLpJson.noSpaces) + + val updated = sql"update learningpaths set document = $dataObject where id = ${row.learningPathId}".update() + if (updated != 1) + throw new RuntimeException(s"Failed to update learning path document for id ${row.learningPathId}") + } + + private[migration] def mergeLearningSteps( + learningPathDocument: String, + steps: List[StepDocumentRowWithMeta], + ): String = { + val oldLp = CirceUtil.tryParse(learningPathDocument).get + val updatedSteps = steps.sortBy(stepSeqNo).map(enrichStep) + oldLp.mapObject(_.remove("learningsteps").add("learningsteps", Json.fromValues(updatedSteps))).noSpaces + } + + private def enrichStep(step: StepDocumentRowWithMeta): Json = { + val json = CirceUtil.tryParse(step.learningStepDocument).get + json.mapObject { obj => + val withIds = obj + .add("id", Json.fromLong(step.learningStepId)) + .add("revision", Json.fromInt(step.revision)) + .add("learningPathId", Json.fromLong(step.learningPathId)) + step.externalId match { + case Some(value) => withIds.add("externalId", Json.fromString(value)) + case None => withIds.remove("externalId") + } + } + } + + private def stepSeqNo(step: StepDocumentRowWithMeta): Int = { + val json = CirceUtil.tryParse(step.learningStepDocument).get + json.hcursor.get[Int]("seqNo").toOption.getOrElse(Int.MaxValue) + } + + private def migrateRows(using session: DBSession): Unit = { + val count = countAllRows.get + var numPagesLeft = (count / chunkSize) + 1 + var offset = 0L + + MDC.put("migrationName", this.getClass.getSimpleName): Unit + while (numPagesLeft > 0) { + allLearningPaths(offset * chunkSize).foreach { lpData => + val steps = getStepDatas(lpData.learningPathId)(using session) + updateLp(lpData, steps)(using session) + } + numPagesLeft -= 1 + offset += 1 + } + MDC.remove("migrationName"): Unit + } +} diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/model/domain/DBLearningStep.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/model/domain/DBLearningStep.scala deleted file mode 100644 index 2b6bf9fd8a..0000000000 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/model/domain/DBLearningStep.scala +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Part of NDLA learningpath-api - * Copyright (C) 2016 NDLA - * - * See LICENSE - * - */ - -package no.ndla.learningpathapi.model.domain - -import no.ndla.common.CirceUtil -import no.ndla.common.model.domain.learningpath.LearningStep -import scalikejdbc.* - -object DBLearningStep extends SQLSyntaxSupport[LearningStep] { - override val tableName = "learningsteps" - def fromResultSet(ls: SyntaxProvider[LearningStep])(rs: WrappedResultSet): LearningStep = - fromResultSet(ls.resultName)(rs) - - def fromResultSet(ls: ResultName[LearningStep])(rs: WrappedResultSet): LearningStep = { - val jsonStr = rs.string(ls.c("document")) - val meta = CirceUtil.unsafeParseAs[LearningStep](jsonStr) - LearningStep( - Some(rs.long(ls.c("id"))), - Some(rs.int(ls.c("revision"))), - rs.stringOpt(ls.c("external_id")), - Some(rs.long(ls.c("learning_path_id"))), - meta.seqNo, - meta.title, - meta.introduction, - meta.description, - meta.embedUrl, - meta.articleId, - meta.`type`, - meta.copyright, - meta.created, - meta.lastUpdated, - meta.owner, - meta.showTitle, - meta.status, - ) - } - - def opt(ls: ResultName[LearningStep])(rs: WrappedResultSet): Option[LearningStep] = rs - .longOpt(ls.c("id")) - .map(_ => fromResultSet(ls)(rs)) -} diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/model/domain/ImplicitLearningPath.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/model/domain/ImplicitLearningPath.scala index 2efa892cf9..c81632f053 100644 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/model/domain/ImplicitLearningPath.scala +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/model/domain/ImplicitLearningPath.scala @@ -47,11 +47,10 @@ extension (learningPath: LearningPath) { def canEditPath(userInfo: CombinedUser): Boolean = canEditLearningPath(userInfo).isSuccess - private def lsLength: Int = learningPath.learningsteps.map(_.length).getOrElse(0) def validateSeqNo(seqNo: Int): Unit = { - if (seqNo < 0 || seqNo > lsLength - 1) { + if (seqNo < 0 || seqNo > learningPath.learningsteps.length - 1) { throw new ValidationException(errors = - List(ValidationMessage("seqNo", s"seqNo must be between 0 and ${lsLength - 1}")) + List(ValidationMessage("seqNo", s"seqNo must be between 0 and ${learningPath.learningsteps.length - 1}")) ) } } diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/repository/LearningPathRepository.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/repository/LearningPathRepository.scala index 965eb2ed23..7ea0ad1c02 100644 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/repository/LearningPathRepository.scala +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/repository/LearningPathRepository.scala @@ -12,20 +12,14 @@ import no.ndla.database.DBUtility import com.typesafe.scalalogging.StrictLogging import no.ndla.common.CirceUtil import no.ndla.common.model.domain.{Author, Tag} -import no.ndla.common.model.domain.learningpath.{ - LearningPath, - LearningPathStatus, - LearningStep, - LearningpathCopyright, - StepStatus, -} +import no.ndla.common.model.domain.learningpath.{LearningPath, LearningPathStatus, LearningStep, LearningpathCopyright} import no.ndla.learningpathapi.model.domain.* import org.postgresql.util.PGobject import scalikejdbc.* import no.ndla.database.implicits.* import java.util.UUID -import scala.util.Try +import scala.util.{Failure, Success, Try} class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { @@ -64,98 +58,43 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { learningPathsWhere(sqls"lp.document->>'isBasedOn' = ${isBasedOnId.toString}") } + def learningPathsWithIsBasedOnRaw(isBasedOnId: Long): List[LearningPath] = { + learningPathsWhereWithInactive(sqls"lp.document->>'isBasedOn' = ${isBasedOnId.toString}") + } + def learningStepsFor( learningPathId: Long )(implicit session: DBSession = dbUtility.readOnlySession): Seq[LearningStep] = { - val ls = DBLearningStep.syntax("ls") - tsql"select ${ls.result.*} from ${DBLearningStep.as(ls)} where ${ls.learningPathId} = $learningPathId" - .map(DBLearningStep.fromResultSet(ls.resultName)) - .runList() - .get - } - - def learningStepWithId(learningPathId: Long, learningStepId: Long)(implicit - session: DBSession = dbUtility.readOnlySession - ): Option[LearningStep] = { - val ls = DBLearningStep.syntax("ls") - tsql"select ${ls.result.*} from ${DBLearningStep.as(ls)} where ${ls.learningPathId} = $learningPathId and ${ls.id} = $learningStepId" - .map(DBLearningStep.fromResultSet(ls.resultName)) + val lp = DBLearningPath.syntax("lp") + tsql"select ${lp.result.*} from ${DBLearningPath.as(lp)} where ${lp.id} = $learningPathId" + .map(rs => DBLearningPath.fromResultSet(lp.resultName)(rs).learningsteps) .runSingle() .get + .getOrElse(Seq.empty) } - def learningStepWithExternalIdAndForLearningPath(externalId: Option[String], learningPathId: Option[Long])(implicit + def learningStepWithId(learningPathId: Long, learningStepId: Long)(implicit session: DBSession = dbUtility.readOnlySession ): Option[LearningStep] = { - if (externalId.isEmpty || learningPathId.isEmpty) { - None - } else { - val ls = DBLearningStep.syntax("ls") - tsql"select ${ls.result.*} from ${DBLearningStep.as(ls)} where ${ls.externalId} = ${externalId.get} and ${ls.learningPathId} = ${learningPathId.get}" - .map(DBLearningStep.fromResultSet(ls.resultName)) - .runSingle() - .get - } - } - - def insert(learningpath: LearningPath)(implicit session: DBSession = dbUtility.autoSession): LearningPath = { - val startRevision = 1 - val dataObject = new PGobject() - dataObject.setType("jsonb") - dataObject.setValue(CirceUtil.toJsonString(learningpath)) - - val learningPathId: Long = - tsql"insert into learningpaths(external_id, document, revision) values(${learningpath.externalId}, $dataObject, $startRevision)" - .updateAndReturnGeneratedKey() - .get - - val learningSteps = learningpath - .learningsteps - .map(lsteps => - lsteps.map(learningStep => { - insertLearningStep(learningStep.copy(learningPathId = Some(learningPathId))) - }) - ) - - logger.info(s"Inserted learningpath with id $learningPathId") - learningpath.copy(id = Some(learningPathId), revision = Some(startRevision), learningsteps = learningSteps) + learningStepsFor(learningPathId).find(_.id.contains(learningStepId)) } - def insertWithImportId(learningpath: LearningPath, importId: String)(implicit - session: DBSession = dbUtility.autoSession - ): LearningPath = { - val startRevision = 1 - val dataObject = new PGobject() + def insert(learningpath: LearningPath)(implicit session: DBSession = dbUtility.autoSession): Try[LearningPath] = { + val startRevision = 1 + val learningPathId = generateLearningPathId() + val toInsert = withStepIdsForInsert(learningpath, learningPathId, startRevision) + val dataObject = new PGobject() dataObject.setType("jsonb") - dataObject.setValue(CirceUtil.toJsonString(learningpath)) - - val importIdUUID = Try(UUID.fromString(importId)).toOption - val learningPathId: Long = - tsql"insert into learningpaths(external_id, document, revision, import_id) values(${learningpath.externalId}, $dataObject, $startRevision, $importIdUUID)" - .updateAndReturnGeneratedKey() - .get - val learningSteps = learningpath - .learningsteps - .map(lsteps => - lsteps.map(learningStep => { - insertLearningStep(learningStep.copy(learningPathId = Some(learningPathId))) - }) - ) - - logger.info(s"Inserted learningpath with id $learningPathId") - learningpath.copy(id = Some(learningPathId), revision = Some(startRevision), learningsteps = learningSteps) - } - - def idAndimportIdOfLearningpath( - externalId: String - )(implicit session: DBSession = dbUtility.autoSession): Option[(Long, Option[String])] = { - val lp = DBLearningPath.syntax("lp") - tsql"""select id, import_id - from ${DBLearningPath.as(lp)} - where lp.document is not NULL and lp.external_id = $externalId""" - .map(rs => (rs.long("id"), rs.stringOpt("import_id"))) - .runSingle() - .get + dataObject.setValue(CirceUtil.toJsonString(toInsert)) + tsql"""insert into learningpaths(id, external_id, document, revision) + values($learningPathId, ${learningpath.externalId}, $dataObject, $startRevision) + """.update() match { + case Success(1) => + logger.info(s"Inserted learningpath with id $learningPathId") + Success(toInsert) + case Success(_) => Failure(new RuntimeException(s"Failed to insert learningpath with id $learningPathId")) + case Failure(ex) => Failure(ex) + } } def insertLearningStep( @@ -272,21 +211,14 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { def learningPathsWithIdBetween(min: Long, max: Long)(implicit session: DBSession = dbUtility.readOnlySession ): List[LearningPath] = { - val (lp, ls) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) - val status = LearningPathStatus.PUBLISHED.toString + val lp = DBLearningPath.syntax("lp") + val status = LearningPathStatus.PUBLISHED.toString - tsql"""select ${lp.result.*}, ${ls.result.*} + tsql"""select ${lp.result.*} from ${DBLearningPath.as(lp)} - left join ${DBLearningStep.as(ls)} on ${lp.id} = ${ls.learningPathId} where lp.document->>'status' = $status - and lp.id between $min and $max""" - .one(DBLearningPath.fromResultSet(lp.resultName)) - .toMany(DBLearningStep.opt(ls.resultName)) - .map { (learningpath, learningsteps) => - learningpath.copy(learningsteps = Some(learningsteps.toSeq)) - } - .runList() - .get + and lp.id between $min and $max + """.map(DBLearningPath.fromResultSet(lp.resultName)).runList().get } def minMaxId(implicit session: DBSession = dbUtility.readOnlySession): (Long, Long) = { @@ -336,13 +268,19 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { private def learningPathsWhere( whereClause: SQLSyntax )(implicit session: DBSession = dbUtility.readOnlySession): List[LearningPath] = { - val (lp, ls) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) - tsql"select ${lp.result.*}, ${ls.result.*} from ${DBLearningPath.as(lp)} left join ${DBLearningStep.as(ls)} on ${lp.id} = ${ls.learningPathId} where $whereClause" - .one(DBLearningPath.fromResultSet(lp.resultName)) - .toMany(DBLearningStep.opt(ls.resultName)) - .map { (learningpath, learningsteps) => - learningpath.copy(learningsteps = Some(learningsteps.filter(_.status == StepStatus.ACTIVE).toSeq)) - } + val lp = DBLearningPath.syntax("lp") + tsql"select ${lp.result.*} from ${DBLearningPath.as(lp)} where $whereClause" + .map(rs => DBLearningPath.fromResultSet(lp.resultName)(rs).withOnlyActiveSteps) + .runList() + .get + } + + private def learningPathsWhereWithInactive( + whereClause: SQLSyntax + )(implicit session: DBSession = dbUtility.readOnlySession): List[LearningPath] = { + val lp = DBLearningPath.syntax("lp") + tsql"select ${lp.result.*} from ${DBLearningPath.as(lp)} where $whereClause" + .map(rs => DBLearningPath.fromResultSet(lp.resultName)(rs)) .runList() .get } @@ -350,13 +288,9 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { private def learningPathWhere( whereClause: SQLSyntax )(implicit session: DBSession = dbUtility.readOnlySession): Option[LearningPath] = { - val (lp, ls) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) - tsql"select ${lp.result.*}, ${ls.result.*} from ${DBLearningPath.as(lp)} left join ${DBLearningStep.as(ls)} on ${lp.id} = ${ls.learningPathId} where $whereClause" - .one(DBLearningPath.fromResultSet(lp.resultName)) - .toMany(DBLearningStep.opt(ls.resultName)) - .map { (learningpath, learningsteps) => - learningpath.copy(learningsteps = Some(learningsteps.filter(_.status == StepStatus.ACTIVE).toSeq)) - } + val lp = DBLearningPath.syntax("lp") + tsql"select ${lp.result.*} from ${DBLearningPath.as(lp)} where $whereClause" + .map(rs => DBLearningPath.fromResultSet(lp.resultName)(rs).withOnlyActiveSteps) .runSingle() .get } @@ -364,62 +298,44 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { def pageWithIds(ids: Seq[Long], pageSize: Int, offset: Int)(implicit session: DBSession = dbUtility.readOnlySession ): List[LearningPath] = { - val (lp, ls) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) - val lps = SubQuery.syntax("lps").include(lp) + val lp = DBLearningPath.syntax("lp") tsql""" - select ${lps.resultAll}, ${ls.resultAll} from (select ${lp.resultAll} - from ${DBLearningPath.as(lp)} - where ${lp.c("id")} in ($ids) - limit $pageSize - offset $offset) lps - left join ${DBLearningStep.as(ls)} on ${lps(lp).id} = ${ls.learningPathId} - """ - .one(DBLearningPath.fromResultSet(lps(lp).resultName)) - .toMany(DBLearningStep.opt(ls.resultName)) - .map { (learningpath, learningsteps) => - learningpath.copy(learningsteps = Some(learningsteps.filter(_.status == StepStatus.ACTIVE).toSeq)) - } - .runList() - .get + select ${lp.resultAll} + from ${DBLearningPath.as(lp)} + where ${lp.c("id")} in ($ids) + order by ${lp.id} + limit $pageSize + offset $offset + """.map(rs => DBLearningPath.fromResultSet(lp.resultName)(rs).withOnlyActiveSteps).runList().get } def getAllLearningPathsByPage(pageSize: Int, offset: Int)(implicit session: DBSession = dbUtility.readOnlySession ): List[LearningPath] = { - val (lp, ls) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) - val lps = SubQuery.syntax("lps").include(lp) + val lp = DBLearningPath.syntax("lp") tsql""" - select ${lps.resultAll}, ${ls.resultAll} from (select ${lp.resultAll}, ${lp.id} as row_id - from ${DBLearningPath.as(lp)} - limit $pageSize - offset $offset) lps - left join ${DBLearningStep.as(ls)} on ${lps(lp).id} = ${ls.learningPathId} + select ${lp.resultAll}, ${lp.id} as row_id + from ${DBLearningPath.as(lp)} order by row_id - """ - .one(DBLearningPath.fromResultSet(lps(lp).resultName)) - .toMany(DBLearningStep.opt(ls.resultName)) - .map { (learningpath, learningsteps) => - learningpath.copy(learningsteps = Some(learningsteps.filter(_.status == StepStatus.ACTIVE).toSeq)) - } - .runList() - .get + limit $pageSize + offset $offset + """.map(rs => DBLearningPath.fromResultSet(lp.resultName)(rs).withOnlyActiveSteps).runList().get } def getExternalLinkStepSamples()(implicit session: DBSession = dbUtility.readOnlySession): List[LearningPath] = { - val (lp, ls) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) + val lp = DBLearningPath.syntax("lp") tsql""" WITH candidates AS ( SELECT DISTINCT clp.id FROM learningpaths clp - WHERE + WHERE clp."document"->>'isMyNDLAOwner' = 'true' AND clp."document"->>'status' = 'UNLISTED' AND EXISTS ( SELECT 1 - FROM learningsteps lss - WHERE lss.learning_path_id = clp.id - AND jsonb_array_length(lss."document"->'embedUrl') > 0 - AND lss."document"->>'status' = 'ACTIVE' + FROM jsonb_array_elements(clp.document->'learningsteps') AS step + WHERE step->>'status' = 'ACTIVE' + AND jsonb_array_length(step->'embedUrl') > 0 ) ), matched_ids AS ( @@ -428,42 +344,27 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { ORDER BY random() LIMIT 5 ) - SELECT ${lp.result.*}, ${ls.result.*} + SELECT ${lp.result.*} FROM matched_ids ids JOIN ${DBLearningPath.as(lp)} ON ${lp.id} = ids.id - LEFT JOIN ${DBLearningStep.as(ls)} ON ${lp.id} = ${ls.learningPathId} - """ - .one(DBLearningPath.fromResultSet(lp.resultName)) - .toMany(DBLearningStep.opt(ls.resultName)) - .map { (learningpath, learningsteps) => - learningpath.copy(learningsteps = Some(learningsteps.filter(_.status == StepStatus.ACTIVE).toSeq)) - } - .runList() - .get + """.map(rs => DBLearningPath.fromResultSet(lp.resultName)(rs).withOnlyActiveSteps).runList().get } def getPublishedLearningPathByPage(pageSize: Int, offset: Int)(implicit session: DBSession = dbUtility.readOnlySession ): List[LearningPath] = { - val (lp, ls) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) - val lps = SubQuery.syntax("lps").include(lp) + val lp = DBLearningPath.syntax("lp") + val lps = SubQuery.syntax("lps").include(lp) tsql""" - select ${lps.resultAll}, ${ls.resultAll} from (select ${lp.resultAll}, ${lp.id} as row_id - from ${DBLearningPath.as(lp)} - where document#>>'{status}' = ${LearningPathStatus.PUBLISHED.toString} - limit $pageSize - offset $offset) lps - left join ${DBLearningStep.as(ls)} on ${lps(lp).id} = ${ls.learningPathId} + select ${lps.resultAll} from (select ${lp.resultAll}, ${lp.id} as row_id + from ${DBLearningPath.as(lp)} + where document#>>'{status}' = ${LearningPathStatus.PUBLISHED.toString} + order by ${lp.id} + limit $pageSize + offset $offset) lps order by row_id - """ - .one(DBLearningPath.fromResultSet(lps(lp).resultName)) - .toMany(DBLearningStep.opt(ls.resultName)) - .map { (learningpath, learningsteps) => - learningpath.copy(learningsteps = Some(learningsteps.filter(_.status == StepStatus.ACTIVE).toSeq)) - } - .runList() - .get + """.map(rs => DBLearningPath.fromResultSet(lps(lp).resultName)(rs).withOnlyActiveSteps).runList().get } def learningPathsWithStatus( @@ -473,7 +374,7 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { } def publishedLearningPathCount(implicit session: DBSession = dbUtility.readOnlySession): Long = { - val (lp, _) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) + val lp = DBLearningPath.syntax("lp") tsql"select count(*) from ${DBLearningPath.as(lp)} where document#>>'{status}' = ${LearningPathStatus.PUBLISHED.toString}" .map(rs => rs.long("count")) .runSingle() @@ -482,7 +383,7 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { } def learningPathCount(implicit session: DBSession = dbUtility.readOnlySession): Long = { - val (lp, _) = (DBLearningPath.syntax("lp"), DBLearningStep.syntax("ls")) + val lp = DBLearningPath.syntax("lp") tsql"select count(*) from ${DBLearningPath.as(lp)}".map(rs => rs.long("count")).runSingle().get.getOrElse(0) } @@ -501,4 +402,53 @@ class LearningPathRepository(using dbUtility: DBUtility) extends StrictLogging { where document@>'{"isMyNDLAOwner": true}' and document->>'status' != ${LearningPathStatus.DELETED.toString} """.map(rs => rs.long("count")).runSingle().get.getOrElse(0) } + + def generateStepId()(implicit session: DBSession): Long = { + sql"select nextval('learningsteps_id_seq')" + .map(rs => rs.long(1)) + .single() + .getOrElse(throw new RuntimeException("Could not generate learning step id.")) + } + + def generateLearningPathId()(implicit session: DBSession): Long = { + sql"select nextval('learningpaths_id_seq')" + .map(rs => rs.long(1)) + .single() + .getOrElse(throw new RuntimeException("Could not generate learning path id.")) + } + + private def withStepIdsForInsert(learningpath: LearningPath, learningPathId: Long, startRevision: Int)(implicit + session: DBSession + ): LearningPath = { + val updatedSteps = learningpath + .learningsteps + .map { step => + val stepId = generateStepId() + val stepRevision = step.revision.orElse(Some(startRevision)) + step.copy(id = Some(stepId), revision = stepRevision, learningPathId = Some(learningPathId)) + } + learningpath.copy(id = Some(learningPathId), revision = Some(startRevision), learningsteps = updatedSteps) + } + + def withIdWithInactiveSteps(id: Long, includeDeleted: Boolean = false)(implicit + session: DBSession = AutoSession + ): Option[LearningPath] = { + if (includeDeleted) { + learningPathWhereWithInactiveSteps(sqls"lp.id = $id") + } else { + learningPathWhereWithInactiveSteps( + sqls"lp.id = $id AND lp.document->>'status' <> ${LearningPathStatus.DELETED.toString}" + ) + } + } + + private[repository] def learningPathWhereWithInactiveSteps( + whereClause: SQLSyntax + )(implicit session: DBSession = ReadOnlyAutoSession): Option[LearningPath] = { + val lp = DBLearningPath.syntax("lp") + tsql"select ${lp.result.*} from ${DBLearningPath.as(lp)} where $whereClause" + .map(rs => DBLearningPath.fromResultSet(lp.resultName)(rs)) + .runSingle() + .get + } } diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/ConverterService.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/ConverterService.scala index 5076c9bb99..0f9201d7c3 100644 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/ConverterService.scala +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/ConverterService.scala @@ -137,13 +137,9 @@ class ConverterService(using .getOrElse(api.LearningPathTagsDTO(Seq(), DefaultLanguage)) val learningSteps = lp .learningsteps - .map(lsteps => { - lsteps - .flatMap(ls => asApiLearningStepV2(ls, lp, searchLanguage, fallback, userInfo).toOption) - .toList - .sortBy(_.seqNo) - }) - .getOrElse(Seq.empty) + .flatMap(ls => asApiLearningStepV2(ls, lp, searchLanguage, fallback, userInfo).toOption) + .toList + .sortBy(_.seqNo) val message = lp.message.filter(_ => lp.canEditPath(userInfo)).map(asApiMessage) val owner = Some(lp.owner).filter(_ => userInfo.isAdmin) @@ -302,7 +298,7 @@ class ConverterService(using .map(t => t.map(embed => Seq(embed))) .getOrElse(Success(Seq.empty)) - val listOfLearningSteps = learningPath.learningsteps.getOrElse(Seq.empty) + val listOfLearningSteps = learningPath.learningsteps val newSeqNo = if (listOfLearningSteps.isEmpty) 0 @@ -344,12 +340,9 @@ class ConverterService(using } def insertLearningStep(learningPath: LearningPath, updatedStep: LearningStep): LearningPath = { - val existingLearningSteps = learningPath.learningsteps.getOrElse(Seq.empty).filterNot(_.id == updatedStep.id) - val steps = - if (StepStatus.ACTIVE == updatedStep.status) existingLearningSteps :+ updatedStep - else existingLearningSteps - - learningPath.copy(learningsteps = Some(steps), lastUpdated = clock.now()) + val existingLearningSteps = learningPath.learningsteps.filterNot(_.id == updatedStep.id) + val steps = existingLearningSteps :+ updatedStep + learningPath.copy(learningsteps = steps, lastUpdated = clock.now()) } def deleteLearningStepLanguage(step: LearningStep, language: String): Try[LearningStep] = { @@ -429,7 +422,6 @@ class ConverterService(using embedUrlsT.map(embedUrls => existing.copy( - revision = Some(updated.revision), title = titles, introduction = introductions, description = descriptions, @@ -481,23 +473,21 @@ class ConverterService(using val steps = existing .learningsteps - .map(ls => - ls.map { step => - val stepCopyright = step.`type` match { - case StepType.TEXT => step.copyright.orElse(existingPathCopyright) - case _ => step.copyright - } - - step.copy( - id = None, - revision = None, - externalId = None, - learningPathId = None, - copyright = stepCopyright, - lastUpdated = clock.now(), - ) + .map { step => + val stepCopyright = step.`type` match { + case StepType.TEXT => step.copyright.orElse(existingPathCopyright) + case _ => step.copyright } - ) + + step.copy( + id = None, + revision = None, + externalId = None, + learningPathId = None, + copyright = stepCopyright, + lastUpdated = clock.now(), + ) + } existing.copy( id = None, @@ -561,7 +551,7 @@ class ConverterService(using owner = ownerId, copyright = asCopyright(copyright), isMyNDLAOwner = user.isMyNDLAUser, - learningsteps = Some(Seq.empty), + learningsteps = Seq.empty, message = None, madeAvailable = None, responsible = newLearningPath @@ -614,8 +604,9 @@ class ConverterService(using .getOrElse(api.LearningPathTagsDTO(Seq(), DefaultLanguage)) val introduction = findByLanguageOrBestEffort(learningpath.introduction.map(asApiIntroduction), AllLanguages) .getOrElse( - findByLanguageOrBestEffort(getApiIntroduction(learningpath.learningsteps.getOrElse(Seq.empty)), AllLanguages) - .getOrElse(api.IntroductionDTO("", DefaultLanguage)) + findByLanguageOrBestEffort(getApiIntroduction(learningpath.learningsteps), AllLanguages).getOrElse( + api.IntroductionDTO("", DefaultLanguage) + ) ) val message = learningpath.message.filter(_ => learningpath.canEditPath(user)).map(_.message) @@ -671,7 +662,7 @@ class ConverterService(using Success( api.LearningStepV2DTO( id = ls.id.get, - revision = ls.revision.get, + revision = ls.revision.getOrElse(1), seqNo = ls.seqNo, title = title, introduction = introduction, 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 index 9e9844582f..e17a532d48 100644 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/UpdateService.scala +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/UpdateService.scala @@ -30,7 +30,6 @@ import scala.annotation.tailrec import scala.util.{Failure, Success, Try, boundary} import no.ndla.language.Language import no.ndla.database.DBUtility -import scalikejdbc.DBSession class UpdateService(using learningPathRepository: LearningPathRepository, @@ -63,7 +62,7 @@ class UpdateService(using } } - def insertDump(dump: learningpath.LearningPath): learningpath.LearningPath = learningPathRepository.insert(dump) + def insertDump(dump: learningpath.LearningPath): Try[learningpath.LearningPath] = learningPathRepository.insert(dump) private[service] def writeOrAccessDenied[T]( willExecute: Boolean, @@ -83,7 +82,7 @@ class UpdateService(using case Some(Success(existing)) => for { toInsert <- converterService.newFromExistingLearningPath(existing, newLearningPath, owner) validated <- learningPathValidator.validate(toInsert, allowUnknownLanguage = true) - inserted <- Try(learningPathRepository.insert(validated)) + inserted <- learningPathRepository.insert(validated) converted <- converterService.asApiLearningpathV2(inserted, newLearningPath.language, fallback = true, owner) } yield converted } @@ -94,7 +93,7 @@ class UpdateService(using for { learningPath <- converterService.newLearningPath(newLearningPath, owner) validated <- learningPathValidator.validate(learningPath) - inserted <- Try(learningPathRepository.insert(validated)) + inserted <- learningPathRepository.insert(validated) converted <- converterService.asApiLearningpathV2(inserted, newLearningPath.language, fallback = true, owner) } yield converted } @@ -113,8 +112,12 @@ class UpdateService(using validatedMergedPath <- learningPathValidator.validate(mergedPath, allowUnknownLanguage = true) updatedLearningPath <- Try(learningPathRepository.update(validatedMergedPath)) _ <- updateSearchAndTaxonomy(updatedLearningPath, owner.tokenUser) - converted <- - converterService.asApiLearningpathV2(updatedLearningPath, learningPathToUpdate.language, fallback = true, owner) + converted <- converterService.asApiLearningpathV2( + updatedLearningPath.withOnlyActiveSteps, + learningPathToUpdate.language, + fallback = true, + owner, + ) } yield converted } @@ -128,14 +131,14 @@ class UpdateService(using learningPath <- withId(learningPathId).flatMap(_.canEditLearningPath(owner)) updatedSteps <- learningPath .learningsteps - .getOrElse(Seq.empty) + .filter(_.status == StepStatus.ACTIVE) .traverse(step => deleteLanguageFromStep(step, language, learningPath)) withUpdatedSteps <- Try(converterService.insertLearningSteps(learningPath, updatedSteps)) withDeletedLanguage <- converterService.deleteLearningPathLanguage(withUpdatedSteps, language) updatedPath <- Try(learningPathRepository.update(withDeletedLanguage)) _ <- updateSearchAndTaxonomy(updatedPath, owner.tokenUser) converted <- converterService.asApiLearningpathV2( - withDeletedLanguage, + withDeletedLanguage.withOnlyActiveSteps, language = Language.DefaultLanguage, fallback = true, owner, @@ -171,24 +174,26 @@ class UpdateService(using } } - private def deleteLanguageFromStep(learningStep: LearningStep, language: String, learningPath: LearningPath)(implicit - session: DBSession + private def deleteLanguageFromStep( + learningStep: LearningStep, + language: String, + learningPath: LearningPath, ): Try[LearningStep] = { for { withDeletedLanguage <- converterService.deleteLearningStepLanguage(learningStep, language) validated <- learningStepValidator.validate(withDeletedLanguage, learningPath, allowUnknownLanguage = true) - updatedStep <- Try(learningPathRepository.updateLearningStep(validated)) - } yield updatedStep + } yield incrementStepRevision(validated) } private def updateSearchAndTaxonomy(learningPath: LearningPath, user: Option[TokenUser]) = { - val sRes = searchIndexService.indexDocument(learningPath) + val indexPath = learningPath.withOnlyActiveSteps + val sRes = searchIndexService.indexDocument(indexPath) if (learningPath.isDeleted) { deleteIsBasedOnReference(learningPath): Unit searchApiClient.deleteLearningPathDocument(learningPath.id.get, user): Unit } else { - searchApiClient.indexLearningPathDocument(learningPath, user): Unit + searchApiClient.indexLearningPathDocument(indexPath, user): Unit } sRes.flatMap(lp => taxonomyApiClient.updateTaxonomyForLearningPath(lp, createResourceIfMissing = false, user)) @@ -229,7 +234,12 @@ class UpdateService(using val updatedLearningPath = learningPathRepository.update(toUpdateWith) updateSearchAndTaxonomy(updatedLearningPath, owner.tokenUser).flatMap(_ => - converterService.asApiLearningpathV2(updatedLearningPath, language, fallback = true, owner) + converterService.asApiLearningpathV2( + updatedLearningPath.withOnlyActiveSteps, + language, + fallback = true, + owner, + ) ) }) @@ -238,7 +248,7 @@ class UpdateService(using private[service] def deleteIsBasedOnReference(updatedLearningPath: LearningPath): Unit = { learningPathRepository - .learningPathsWithIsBasedOn(updatedLearningPath.id.get) + .learningPathsWithIsBasedOnRaw(updatedLearningPath.id.get) .foreach(lp => { learningPathRepository.update(lp.copy(lastUpdated = clock.now(), isBasedOn = None)) }) @@ -253,25 +263,28 @@ class UpdateService(using withId(learningPathId).flatMap(_.canEditLearningPath(owner)) match { case Failure(ex) => Failure(ex) case Success(learningPath) => - val validated = for { - newStep <- converterService.asDomainLearningStep(newLearningStep, learningPath, owner.id) - validated <- learningStepValidator.validate(newStep, learningPath) + val activeLearningPath = learningPath.withOnlyActiveSteps + val validated = for { + newStep <- converterService.asDomainLearningStep(newLearningStep, activeLearningPath, owner.id) + validated <- learningStepValidator.validate(newStep, activeLearningPath) } yield validated validated match { case Failure(ex) => Failure(ex) case Success(newStep) => - val (insertedStep, updatedPath) = learningPathRepository.inTransaction { implicit session => - val insertedStep = learningPathRepository.insertLearningStep(newStep) - val toUpdate = converterService.insertLearningStep(learningPath, insertedStep) - val updatedPath = learningPathRepository.update(toUpdate) - - (insertedStep, updatedPath) + val (newStepWithIdAndRevision, updatedPath) = learningPathRepository.inTransaction { implicit session => + val generatedId = Some(learningPathRepository.generateStepId()) + val newStepWithIdAndRevision = + newStep.copy(id = generatedId, learningPathId = learningPath.id, revision = Some(1)) + val toUpdate = converterService.insertLearningStep(learningPath, newStepWithIdAndRevision) + val updatedPath = learningPathRepository.update(toUpdate) + + (newStepWithIdAndRevision, updatedPath) } updateSearchAndTaxonomy(updatedPath, owner.tokenUser).flatMap(_ => converterService.asApiLearningStepV2( - insertedStep, + newStepWithIdAndRevision, updatedPath, newLearningStep.language, fallback = true, @@ -306,24 +319,26 @@ class UpdateService(using case _ => // continue } val validated = for { + _ <- validateStepRevision(existing, learningStepToUpdate.revision) toUpdate <- converterService.mergeLearningSteps(existing, learningStepToUpdate) - validated <- learningStepValidator.validate(toUpdate, learningPath, allowUnknownLanguage = true) + validated <- learningStepValidator.validate( + toUpdate.copy(revision = Some(learningStepToUpdate.revision + 1)), + learningPath, + allowUnknownLanguage = true, + ) } yield validated validated match { case Failure(ex) => Failure(ex) case Success(toUpdate) => - val (updatedStep, updatedPath) = learningPathRepository.inTransaction { implicit session => - val updatedStep = learningPathRepository.updateLearningStep(toUpdate) - val pathToUpdate = converterService.insertLearningStep(learningPath, updatedStep) - val updatedPath = learningPathRepository.update(pathToUpdate) - - (updatedStep, updatedPath) + val updatedPath = learningPathRepository.inTransaction { implicit session => + val pathToUpdate = converterService.insertLearningStep(learningPath, toUpdate) + learningPathRepository.update(pathToUpdate) } updateSearchAndTaxonomy(updatedPath, owner.tokenUser).flatMap(_ => converterService.asApiLearningStepV2( - updatedStep, + toUpdate, updatedPath, learningStepToUpdate.language, fallback = true, @@ -345,27 +360,28 @@ class UpdateService(using stepToUpdate: LearningStep, stepsToChange: Seq[LearningStep], ): (LearningPath, LearningStep) = learningPathRepository.inTransaction { implicit session => - val (_, updatedStep, newLearningSteps) = stepsToChange + val (_, maybeUpdatedStep, newLearningSteps) = stepsToChange .sortBy(_.seqNo) - .foldLeft((0, stepToUpdate, Seq.empty[LearningStep])) { case ((seqNo, foundStep, steps), curr) => - val now = clock.now() - val isChangedStep = curr.id.contains(learningStepId) - val (mainStep, stepToAdd) = - if (isChangedStep) - (curr.copy(status = newStatus, lastUpdated = now), curr.copy(status = newStatus, lastUpdated = now)) - else (foundStep, curr) - val updatedMainStep = mainStep.copy(seqNo = seqNo, lastUpdated = now) - val updatedSteps = steps :+ stepToAdd.copy(seqNo = seqNo, lastUpdated = now) - val nextSeqNo = - if (stepToAdd.status == DELETED) seqNo + .foldLeft((0, Option.empty[LearningStep], Seq.empty[LearningStep])) { case ((seqNo, foundStep, steps), curr) => + val now = clock.now() + val isChangedStep = curr.id.contains(learningStepId) + val stepWithStatus = + if (isChangedStep) curr.copy(status = newStatus, lastUpdated = now) + else curr + val updatedStep = incrementStepRevision(stepWithStatus.copy(seqNo = seqNo, lastUpdated = now)) + val nextSeqNo = + if (updatedStep.status == DELETED) seqNo else seqNo + 1 + val updatedMainStep = + if (isChangedStep) Some(updatedStep) + else foundStep - (nextSeqNo, updatedMainStep, updatedSteps) + (nextSeqNo, updatedMainStep, steps :+ updatedStep) } - val updated = newLearningSteps.map(learningPathRepository.updateLearningStep) - val lp = converterService.insertLearningSteps(learningPath, updated) - val updatedPath = learningPathRepository.update(lp.copy(learningsteps = None)) + val updatedStep = maybeUpdatedStep.getOrElse(stepToUpdate) + val lp = converterService.insertLearningSteps(learningPath, newLearningSteps) + val updatedPath = learningPathRepository.update(lp) (updatedPath, updatedStep) } @@ -376,7 +392,6 @@ class UpdateService(using owner: CombinedUserRequired, ): Try[LearningStepV2DTO] = writeOrAccessDenied(owner.canWrite) { boundary { - withId(learningPathId).flatMap(_.canEditLearningPath(owner)) match { case Failure(ex) => Failure(ex) case Success(learningPath) => @@ -423,13 +438,13 @@ class UpdateService(using NotFoundException(s"LearningStep with id $learningStepId in learningPath $learningPathId not found") ) case Some(learningStep) => - learningPath.validateSeqNo(seqNo) + val activeLearningPath = learningPath.withOnlyActiveSteps + activeLearningPath.validateSeqNo(seqNo) val from = learningStep.seqNo val to = seqNo - val toUpdate = learningPath + val toUpdate = activeLearningPath .learningsteps - .getOrElse(Seq.empty) .filter(step => rangeToUpdate(from, to).contains(step.seqNo)) def addOrSubtract(seqNo: Int): Int = @@ -437,13 +452,20 @@ class UpdateService(using else seqNo - 1 val now = clock.now() - learningPathRepository.inTransaction { implicit session => - val _ = learningPathRepository.updateLearningStep(learningStep.copy(seqNo = seqNo, lastUpdated = now)) - toUpdate.foreach(step => { - learningPathRepository.updateLearningStep( - step.copy(seqNo = addOrSubtract(step.seqNo), lastUpdated = now) - ) - }) + val updatedSteps = learningPath + .learningsteps + .map { step => + if (step.id == learningStep.id) { + incrementStepRevision(step.copy(seqNo = seqNo, lastUpdated = now)) + } else if (toUpdate.exists(_.id == step.id)) { + incrementStepRevision(step.copy(seqNo = addOrSubtract(step.seqNo), lastUpdated = now)) + } else { + step + } + } + + val _ = learningPathRepository.inTransaction { implicit session => + learningPathRepository.update(learningPath.copy(learningsteps = updatedSteps, lastUpdated = now)) } Success(LearningStepSeqNoDTO(seqNo)) @@ -459,11 +481,8 @@ class UpdateService(using private def withId(learningPathId: Long, includeDeleted: Boolean = false): Try[LearningPath] = { val lpOpt = - if (includeDeleted) { - learningPathRepository.withIdIncludingDeleted(learningPathId) - } else { - learningPathRepository.withId(learningPathId) - } + if (includeDeleted) learningPathRepository.withIdWithInactiveSteps(learningPathId, includeDeleted = true) + else learningPathRepository.withIdWithInactiveSteps(learningPathId) lpOpt match { case Some(learningPath) => Success(learningPath) @@ -471,6 +490,22 @@ class UpdateService(using } } + private def currentStepRevision(step: LearningStep): Int = step.revision.getOrElse(1) + + private def incrementStepRevision(step: LearningStep): LearningStep = { + step.copy(revision = Some(currentStepRevision(step) + 1)) + } + + private def validateStepRevision(existing: LearningStep, expectedRevision: Int): Try[Unit] = { + val currentRevision = currentStepRevision(existing) + if (currentRevision == expectedRevision) Success(()) + else { + val msg = + s"Conflicting revision is detected for learningStep with id = ${existing.id} and revision = $expectedRevision" + Failure(OptimisticLockException(msg)) + } + } + @tailrec private def optimisticLockRetries[T](n: Int)(fn: => T): T = { try { diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/search/SearchConverterServiceComponent.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/search/SearchConverterServiceComponent.scala index 2c142ace00..c29de05f94 100644 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/search/SearchConverterServiceComponent.scala +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/service/search/SearchConverterServiceComponent.scala @@ -87,7 +87,7 @@ class SearchConverterServiceComponent(using converterService: ConverterService, learningPath.lastUpdated, defaultTitle.map(_.title), SearchableLanguageList(learningPath.tags.map(tags => LanguageValue(tags.language, tags.tags))), - learningPath.learningsteps.getOrElse(Seq.empty).map(asSearchableLearningStep).toList, + learningPath.withOnlyActiveSteps.learningsteps.map(asSearchableLearningStep).toList, converterService.asApiCopyright(learningPath.copyright), learningPath.isBasedOn, learningPath.grepCodes, diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/TestData.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/TestData.scala index de797086e1..89648f13b1 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/TestData.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/TestData.scala @@ -97,7 +97,7 @@ object TestData { owner = "me", copyright = LearningpathCopyright(CC_BY.toString, List.empty), isMyNDLAOwner = false, - learningsteps = Some(List(domainLearningStep1, domainLearningStep2)), + learningsteps = Seq(domainLearningStep1, domainLearningStep2), responsible = None, comments = Seq.empty, priority = common.Priority.Unspecified, diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocumentTest.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocumentTest.scala new file mode 100644 index 0000000000..4d77ce2357 --- /dev/null +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocumentTest.scala @@ -0,0 +1,80 @@ +/* + * Part of NDLA learningpath-api + * Copyright (C) 2025 NDLA + * + * See LICENSE + * + */ + +package no.ndla.learningpathapi.db.migration + +import no.ndla.common.CirceUtil +import no.ndla.learningpathapi.{TestEnvironment, UnitSuite} + +class V60__MoveLearningStepsToLearningPathDocumentTest extends UnitSuite with TestEnvironment { + test("that learningsteps are embedded in learningpath document with metadata and ordering") { + val migration = new V60__MoveLearningStepsToLearningPathDocument + + val learningPathDocument = """ + |{ + | "title": "Test path" + |} + |""".stripMargin + + val stepZero = StepDocumentRowWithMeta( + learningStepId = 1, + learningPathId = 10, + revision = 1, + externalId = Some("ext-1"), + learningStepDocument = """ + |{ + | "seqNo": 0, + | "title": "Step 0" + |} + |""".stripMargin, + ) + + val stepTwo = StepDocumentRowWithMeta( + learningStepId = 2, + learningPathId = 10, + revision = 3, + externalId = None, + learningStepDocument = """ + |{ + | "seqNo": 2, + | "title": "Step 2" + |} + |""".stripMargin, + ) + + val updatedDocument = migration.mergeLearningSteps(learningPathDocument, List(stepTwo, stepZero)) + + val expectedDocument = """ + |{ + | "title": "Test path", + | "learningsteps": [ + | { + | "seqNo": 0, + | "title": "Step 0", + | "id": 1, + | "revision": 1, + | "learningPathId": 10, + | "externalId": "ext-1" + | }, + | { + | "seqNo": 2, + | "title": "Step 2", + | "id": 2, + | "revision": 3, + | "learningPathId": 10 + | } + | ] + |} + |""".stripMargin + + val resultJson = CirceUtil.unsafeParse(updatedDocument) + val expectedJson = CirceUtil.unsafeParse(expectedDocument) + + resultJson should be(expectedJson) + } +} diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/e2e/LearningPathAndStepCreationTests.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/e2e/LearningPathAndStepCreationTests.scala index 92ea0624dc..08ef569b32 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/e2e/LearningPathAndStepCreationTests.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/e2e/LearningPathAndStepCreationTests.scala @@ -8,19 +8,22 @@ package no.ndla.learningpathapi.e2e +import io.circe.Decoder import no.ndla.common.{CirceUtil, Clock} import no.ndla.common.configuration.Prop import no.ndla.common.model.NDLADate +import no.ndla.common.model.api.{AuthorDTO, LicenseDTO} import no.ndla.common.model.domain.learningpath.{EmbedType, LearningPath, StepType} import no.ndla.learningpathapi.model.api.* import no.ndla.learningpathapi.* -import no.ndla.learningpathapi.integration.TaxonomyApiClient +import no.ndla.learningpathapi.integration.{Node, TaxonomyApiClient} import no.ndla.scalatestsuite.{DatabaseIntegrationSuite, ElasticsearchIntegrationSuite} import org.mockito.ArgumentMatchers.any import org.mockito.Mockito.{when, withSettings} import org.mockito.invocation.InvocationOnMock import org.mockito.quality.Strictness import org.testcontainers.postgresql.PostgreSQLContainer +import sttp.client3.{Identity, RequestT, Response} import sttp.client3.quick.* import java.util.concurrent.Executors @@ -50,7 +53,7 @@ class LearningPathAndStepCreationTests val someDate: NDLADate = NDLADate.of(2017, 1, 1, 1, 59) - val learningpathApi: MainClass = new MainClass(learningpathApiProperties) { + lazy val learningpathApi: MainClass = new MainClass(learningpathApiProperties) { override val componentRegistry: ComponentRegistry = new ComponentRegistry(learningpathApiProperties) { override implicit lazy val clock: Clock = { val mockClock = mock[Clock](withSettings.strictness(Strictness.LENIENT)) @@ -62,12 +65,13 @@ class LearningPathAndStepCreationTests when(client.updateTaxonomyForLearningPath(any, any, any)).thenAnswer { (i: InvocationOnMock) => Success(i.getArgument[LearningPath](0)) } + when(client.queryNodes(any[Long])).thenReturn(Success(List.empty[Node])) client } } } - val testClock: Clock = learningpathApi.componentRegistry.clock + lazy val testClock: Clock = learningpathApi.componentRegistry.clock val learningpathApiBaseUrl: String = s"http://localhost:$learningpathApiPort" val learningpathApiLPUrl: String = s"$learningpathApiBaseUrl/learningpath-api/v2/learningpaths" @@ -99,15 +103,33 @@ class LearningPathAndStepCreationTests val fakeToken = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiIsImtpZCI6Ik9FSTFNVVU0T0RrNU56TTVNekkyTXpaRE9EazFOMFl3UXpkRE1EUXlPRFZDUXpRM1FUSTBNQSJ9.eyJodHRwczovL25kbGEubm8vbmRsYV9pZCI6Inh4eHl5eSIsImlzcyI6Imh0dHBzOi8vbmRsYS5ldS5hdXRoMC5jb20vIiwic3ViIjoieHh4eXl5QGNsaWVudHMiLCJhdWQiOiJuZGxhX3N5c3RlbSIsImlhdCI6MTUxMDMwNTc3MywiZXhwIjoxNTEwMzkyMTczLCJwZXJtaXNzaW9ucyI6WyJhcnRpY2xlczpwdWJsaXNoIiwiZHJhZnRzOndyaXRlIiwiZHJhZnRzOnNldF90b19wdWJsaXNoIiwiYXJ0aWNsZXM6d3JpdGUiLCJsZWFybmluZ3BhdGg6d3JpdGUiLCJsZWFybmluZ3BhdGg6cHVibGlzaCIsImxlYXJuaW5ncGF0aDphZG1pbiJdLCJndHkiOiJjbGllbnQtY3JlZGVudGlhbHMifQ.XnP0ywYk-A0j9bGZJBCDNA5fZ4OuGRLkXFBBr3IYD50" - def createLearningpath(title: String, shouldSucceed: Boolean = true): LearningPathV2DTO = { + private def sendAuthed(request: RequestT[Identity, String, Any]): Response[String] = { + simpleHttpClient.send( + request + .header("Content-type", "application/json") + .header("Authorization", s"Bearer $fakeToken") + .readTimeout(10.seconds) + ) + } + + private def parseAs[T: Decoder](res: Response[String]): T = { + CirceUtil.unsafeParseAs[T](res.body) + } + + def createLearningpath( + title: String, + tags: Option[Seq[String]] = None, + language: String = "nb", + duration: Option[Int] = None, + ): LearningPathV2DTO = { val dto = NewLearningPathV2DTO( title = title, description = None, introduction = None, coverPhotoMetaUrl = None, - duration = None, - tags = None, - language = "nb", + duration = duration, + tags = tags, + language = language, copyright = None, responsibleId = None, comments = None, @@ -116,76 +138,329 @@ class LearningPathAndStepCreationTests grepCodes = None, ) - val x = CirceUtil.toJsonString(dto) + val res = sendAuthed(quickRequest.post(uri"$learningpathApiLPUrl").body(CirceUtil.toJsonString(dto))) + res.code.code should be(201) + parseAs[LearningPathV2DTO](res) + } - val res = simpleHttpClient.send( - quickRequest - .post(uri"$learningpathApiLPUrl") - .body(x) - .header("Content-type", "application/json") - .header("Authorization", s"Bearer $fakeToken") - .readTimeout(10.seconds) + def copyLearningpath(pathId: Long, title: String): LearningPathV2DTO = { + val dto = NewCopyLearningPathV2DTO( + title = title, + introduction = None, + description = None, + language = "nb", + coverPhotoMetaUrl = None, + duration = None, + tags = None, + copyright = None, ) - if (shouldSucceed) { - res.code.code should be(201) - } - CirceUtil.unsafeParseAs[LearningPathV2DTO](res.body) + val res = sendAuthed(quickRequest.post(uri"$learningpathApiLPUrl/$pathId/copy").body(CirceUtil.toJsonString(dto))) + res.code.code should be(201) + parseAs[LearningPathV2DTO](res) } - def createLearningStep(pathId: Long, title: String, shouldSucceed: Boolean = true): LearningStepV2DTO = { + def createLearningStep( + pathId: Long, + title: String, + language: String = "nb", + embedUrl: Option[EmbedUrlV2DTO] = + Some(EmbedUrlV2DTO(url = "https://www.example.com/", embedType = EmbedType.External.entryName)), + articleId: Option[Long] = None, + ): LearningStepV2DTO = { val dto = NewLearningStepV2DTO( title = title, introduction = None, description = None, - language = "nb", - embedUrl = Some(EmbedUrlV2DTO(url = "https://www.example.com/", embedType = EmbedType.External.entryName)), - articleId = None, + language = language, + embedUrl = embedUrl, + articleId = articleId, showTitle = false, `type` = StepType.TEXT.toString, license = None, copyright = None, ) - val x = CirceUtil.toJsonString(dto) - val res = simpleHttpClient.send( + val res = + sendAuthed(quickRequest.post(uri"$learningpathApiLPUrl/$pathId/learningsteps").body(CirceUtil.toJsonString(dto))) + res.code.code should be(201) + parseAs[LearningStepV2DTO](res) + } + + def getLearningPathResponse(pathId: Long): Response[String] = { + sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/$pathId")) + } + + def getLearningPath(pathId: Long): LearningPathV2DTO = { + val res = getLearningPathResponse(pathId) + res.code.code should be(200) + parseAs[LearningPathV2DTO](res) + } + + def getLearningStepResponse(pathId: Long, stepId: Long): Response[String] = { + sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/$pathId/learningsteps/$stepId")) + } + + def getLearningStep(pathId: Long, stepId: Long): LearningStepV2DTO = { + val res = getLearningStepResponse(pathId, stepId) + res.code.code should be(200) + parseAs[LearningStepV2DTO](res) + } + + def updateLearningPathTitle(pathId: Long, revision: Int, language: String, title: String): LearningPathV2DTO = { + val body = s"""{"revision":$revision,"title":"$title","language":"$language"}""" + val res = sendAuthed(quickRequest.patch(uri"$learningpathApiLPUrl/$pathId").body(body)) + res.code.code should be(200) + parseAs[LearningPathV2DTO](res) + } + + def updateLearningStepTitle( + pathId: Long, + stepId: Long, + revision: Int, + language: String, + title: String, + ): LearningStepV2DTO = { + val body = s"""{"revision":$revision,"title":"$title","language":"$language"}""" + val res = sendAuthed(quickRequest.patch(uri"$learningpathApiLPUrl/$pathId/learningsteps/$stepId").body(body)) + res.code.code should be(200) + parseAs[LearningStepV2DTO](res) + } + + def getLearningPathStatus(pathId: Long): LearningPathStatusDTO = { + val res = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/$pathId/status")) + res.code.code should be(200) + parseAs[LearningPathStatusDTO](res) + } + + def getLearningStepStatus(pathId: Long, stepId: Long): LearningStepStatusDTO = { + val res = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/$pathId/learningsteps/$stepId/status")) + res.code.code should be(200) + parseAs[LearningStepStatusDTO](res) + } + + def updateLearningPathStatus(pathId: Long, status: String, message: Option[String] = None): LearningPathV2DTO = { + val dto = UpdateLearningPathStatusDTO(status = status, message = message) + val res = sendAuthed(quickRequest.put(uri"$learningpathApiLPUrl/$pathId/status").body(CirceUtil.toJsonString(dto))) + res.code.code should be(200) + parseAs[LearningPathV2DTO](res) + } + + def updateLearningStepStatus(pathId: Long, stepId: Long, status: String): LearningStepV2DTO = { + val dto = LearningStepStatusDTO(status = status) + val res = sendAuthed( quickRequest - .post(uri"$learningpathApiLPUrl/$pathId/learningsteps") - .body(x) - .header("Content-type", "application/json") - .header("Authorization", s"Bearer $fakeToken") - .readTimeout(10.seconds) + .put(uri"$learningpathApiLPUrl/$pathId/learningsteps/$stepId/status") + .body(CirceUtil.toJsonString(dto)) ) - if (shouldSucceed) { - res.code.code should be(201) - } - CirceUtil.unsafeParseAs[LearningStepV2DTO](res.body) + res.code.code should be(200) + parseAs[LearningStepV2DTO](res) } - def getLearningPath(pathId: Long, shouldSucceed: Boolean = true): LearningPathV2DTO = { - val res = simpleHttpClient.send( - quickRequest - .get(uri"$learningpathApiLPUrl/$pathId") - .header("Content-type", "application/json") - .header("Authorization", s"Bearer $fakeToken") - .readTimeout(10.seconds) + def updateLearningStepSeqNo(pathId: Long, stepId: Long, seqNo: Int): LearningStepSeqNoDTO = { + val dto = LearningStepSeqNoDTO(seqNo) + val res = sendAuthed( + quickRequest.put(uri"$learningpathApiLPUrl/$pathId/learningsteps/$stepId/seqNo").body(CirceUtil.toJsonString(dto)) ) - if (shouldSucceed) { - res.code.code should be(200) - } - CirceUtil.unsafeParseAs[LearningPathV2DTO](res.body) + res.code.code should be(200) + parseAs[LearningStepSeqNoDTO](res) } - def deleteStep(pathId: Long, stepId: Long, maybeExpectedCode: Option[Int] = Some(204)): Unit = { - val res = simpleHttpClient.send( - quickRequest - .delete(uri"$learningpathApiLPUrl/$pathId/learningsteps/$stepId") - .header("Content-type", "application/json") - .header("Authorization", s"Bearer $fakeToken") - .readTimeout(10.seconds) + def deleteLearningPathLanguage(pathId: Long, language: String): LearningPathV2DTO = { + val res = sendAuthed(quickRequest.delete(uri"$learningpathApiLPUrl/$pathId/language/$language")) + res.code.code should be(200) + parseAs[LearningPathV2DTO](res) + } + + def deleteLearningStepLanguage(pathId: Long, stepId: Long, language: String): LearningStepV2DTO = { + val res = + sendAuthed(quickRequest.delete(uri"$learningpathApiLPUrl/$pathId/learningsteps/$stepId/language/$language")) + res.code.code should be(200) + parseAs[LearningStepV2DTO](res) + } + + def updateTaxonomyForLearningPath(pathId: Long): LearningPathV2DTO = { + val res = sendAuthed( + quickRequest.post( + uri"$learningpathApiLPUrl/$pathId/update-taxonomy?language=nb&fallback=true&create-if-missing=true" + ) ) - maybeExpectedCode match { - case None => - case Some(expectedCode) => res.code.code should be(expectedCode) - } + res.code.code should be(200) + parseAs[LearningPathV2DTO](res) + } + + def deleteLearningPath(pathId: Long): Response[String] = { + sendAuthed(quickRequest.delete(uri"$learningpathApiLPUrl/$pathId")) + } + + def deleteLearningStep(pathId: Long, stepId: Long): Response[String] = { + sendAuthed(quickRequest.delete(uri"$learningpathApiLPUrl/$pathId/learningsteps/$stepId")) + } + + test("Learningpath endpoints support full CRUD and management operations") { + val created = createLearningpath(title = "LearningPath CRUD", tags = Some(Seq("ndla-tag"))) + created.title.title should be("LearningPath CRUD") + + val fetched = getLearningPath(created.id) + fetched.id should be(created.id) + + val updated = updateLearningPathTitle(created.id, fetched.revision, "nb", "LearningPath Updated") + updated.title.title should be("LearningPath Updated") + + val statusBeforeUpdate = getLearningPathStatus(created.id) + statusBeforeUpdate.status should be("PRIVATE") + + val unlistedPath = updateLearningPathStatus(created.id, "UNLISTED", Some("Ready for sharing")) + unlistedPath.status should be("UNLISTED") + + val withStatusRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/status/UNLISTED")) + withStatusRes.code.code should be(200) + val withStatusPaths = parseAs[List[LearningPathV2DTO]](withStatusRes) + withStatusPaths.map(_.id) should contain(created.id) + + val englishPathVersion = updateLearningPathTitle(created.id, unlistedPath.revision, "en", "LearningPath English") + englishPathVersion.supportedLanguages should contain("en") + + val afterLanguageDelete = deleteLearningPathLanguage(created.id, "en") + afterLanguageDelete.supportedLanguages should not contain "en" + + val taxonomyUpdated = updateTaxonomyForLearningPath(created.id) + taxonomyUpdated.id should be(created.id) + + val copied = copyLearningpath(created.id, "LearningPath Copy") + copied.isBasedOn should be(Some(created.id)) + + val idsQuery = s"${created.id},${copied.id}" + val byIdsRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/ids?ids=$idsQuery&language=nb&fallback=true")) + byIdsRes.code.code should be(200) + val byIds = parseAs[List[LearningPathV2DTO]](byIdsRes) + byIds.map(_.id).toSet should be(Set(created.id, copied.id)) + + val mineRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/mine")) + mineRes.code.code should be(200) + val mine = parseAs[List[LearningPathV2DTO]](mineRes) + mine.map(_.id) should contain(created.id) + mine.map(_.id) should contain(copied.id) + + val deleteCopyRes = deleteLearningPath(copied.id) + deleteCopyRes.code.code should be(204) + + val getDeletedCopyRes = getLearningPathResponse(copied.id) + getDeletedCopyRes.code.code should be(404) + } + + test("Learningstep endpoints support full CRUD and management operations") { + val learningPath = createLearningpath("Path for step CRUD") + val step1 = createLearningStep(learningPath.id, "Step One") + val step2 = createLearningStep(learningPath.id, "Step Two") + + val stepsRes = sendAuthed( + quickRequest.get(uri"$learningpathApiLPUrl/${learningPath.id}/learningsteps?language=nb&fallback=true") + ) + stepsRes.code.code should be(200) + val stepContainer = parseAs[LearningStepContainerSummaryDTO](stepsRes) + stepContainer.learningsteps.map(_.id).toSet should be(Set(step1.id, step2.id)) + + val fetchedStep = getLearningStep(learningPath.id, step1.id) + fetchedStep.title.title should be("Step One") + + val updatedStep = updateLearningStepTitle( + pathId = learningPath.id, + stepId = step1.id, + revision = fetchedStep.revision, + language = "nb", + title = "Step One Updated", + ) + updatedStep.title.title should be("Step One Updated") + + val initialStepStatus = getLearningStepStatus(learningPath.id, step1.id) + initialStepStatus.status should be("ACTIVE") + + val movedStepSeq = updateLearningStepSeqNo(learningPath.id, step2.id, 0) + movedStepSeq.seqNo should be(0) + + val pathAfterSeqUpdate = getLearningPath(learningPath.id) + val seqMap = pathAfterSeqUpdate.learningsteps.map(step => step.id -> step.seqNo).toMap + seqMap(step2.id) should be(0) + seqMap(step1.id) should be(1) + + val deletedStep = updateLearningStepStatus(learningPath.id, step1.id, "DELETED") + deletedStep.status should be("DELETED") + + val trashRes = sendAuthed( + quickRequest.get(uri"$learningpathApiLPUrl/${learningPath.id}/learningsteps/trash?language=nb&fallback=true") + ) + trashRes.code.code should be(200) + val trashContainer = parseAs[LearningStepContainerSummaryDTO](trashRes) + trashContainer.learningsteps.map(_.id) should contain(step1.id) + + val reactivatedStep = updateLearningStepStatus(learningPath.id, step1.id, "ACTIVE") + reactivatedStep.status should be("ACTIVE") + + val englishStepVersion = updateLearningStepTitle( + pathId = learningPath.id, + stepId = step1.id, + revision = reactivatedStep.revision, + language = "en", + title = "Step One English", + ) + englishStepVersion.supportedLanguages should contain("en") + + val stepAfterLanguageDelete = deleteLearningStepLanguage(learningPath.id, step1.id, "en") + stepAfterLanguageDelete.supportedLanguages should not contain "en" + + val deleteStepRes = deleteLearningStep(learningPath.id, step1.id) + deleteStepRes.code.code should be(204) + + val deleteStepAgainRes = deleteLearningStep(learningPath.id, step1.id) + deleteStepAgainRes.code.code should be(404) + + val getDeletedStepRes = getLearningStepResponse(learningPath.id, step1.id) + getDeletedStepRes.code.code should be(200) + parseAs[LearningStepV2DTO](getDeletedStepRes).status should be("DELETED") + } + + test("Search and metadata endpoints return valid payloads") { + val lpWithExternalAndArticleStep = createLearningpath("Path for metadata endpoints") + val publishedTaggedPath = + createLearningpath(title = "Published tags path", tags = Some(Seq("published-tag")), duration = Some(10)) + updateLearningPathStatus(publishedTaggedPath.id, "PUBLISHED") + createLearningStep(lpWithExternalAndArticleStep.id, "External step") + createLearningStep( + pathId = lpWithExternalAndArticleStep.id, + title = "Article step", + embedUrl = None, + articleId = Some(424242L), + ) + updateLearningPathStatus(lpWithExternalAndArticleStep.id, "UNLISTED") + + val listRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl")) + listRes.code.code should be(200) + parseAs[SearchResultV2DTO](listRes) + + val postSearchRes = sendAuthed(quickRequest.post(uri"$learningpathApiLPUrl/search").body("{}")) + postSearchRes.code.code should be(200) + parseAs[SearchResultV2DTO](postSearchRes) + + val tagsRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/tags?language=nb&fallback=true")) + tagsRes.code.code should be(200) + parseAs[LearningPathTagsSummaryDTO](tagsRes) + + val licensesRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/licenses")) + licensesRes.code.code should be(200) + val licenses = parseAs[Seq[LicenseDTO]](licensesRes) + licenses should not be empty + + val contributorsRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/contributors")) + contributorsRes.code.code should be(200) + parseAs[List[AuthorDTO]](contributorsRes) + + val containsArticleRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/contains-article/424242")) + containsArticleRes.code.code should be(200) + parseAs[Seq[LearningPathSummaryV2DTO]](containsArticleRes) + + val externalSamplesRes = sendAuthed(quickRequest.get(uri"$learningpathApiLPUrl/external-samples")) + externalSamplesRes.code.code should be(200) + val samples = parseAs[List[LearningPathV2DTO]](externalSamplesRes) + samples.size should be <= 5 } test("That sequence numbers of learningsteps are updated correctly") { @@ -198,11 +473,11 @@ class LearningPathAndStepCreationTests val pathBeforeDelete = getLearningPath(x.id) pathBeforeDelete.learningsteps.map(_.seqNo) should be(Seq(0, 1, 2, 3, 4)) - deleteStep(x.id, s1.id) - deleteStep(x.id, s1.id, Some(404)) - deleteStep(x.id, s1.id, Some(404)) - deleteStep(x.id, s1.id, Some(404)) - deleteStep(x.id, s1.id, Some(404)) + deleteLearningStep(x.id, s1.id).code.code should be(204) + deleteLearningStep(x.id, s1.id).code.code should be(404) + deleteLearningStep(x.id, s1.id).code.code should be(404) + deleteLearningStep(x.id, s1.id).code.code should be(404) + deleteLearningStep(x.id, s1.id).code.code should be(404) val path = getLearningPath(x.id) path.learningsteps.map(_.seqNo) should be(Seq(0, 1, 2, 3)) diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/repository/LearningPathRepositoryIntegrationTest.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/repository/LearningPathRepositoryIntegrationTest.scala index 682aeae184..6994ab29e7 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/repository/LearningPathRepositoryIntegrationTest.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/repository/LearningPathRepositoryIntegrationTest.scala @@ -67,6 +67,7 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit owner = "UNIT-TEST", copyright = copyright, isMyNDLAOwner = false, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = Priority.Unspecified, @@ -128,7 +129,7 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit test("That insert, fetch and delete works happy-day") { repository.inTransaction { implicit session => - val inserted = repository.insert(DefaultLearningPath) + val inserted = repository.insert(DefaultLearningPath).get inserted.id.isDefined should be(true) val fetched = repository.withId(inserted.id.get) @@ -145,7 +146,7 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit try { repository.inTransaction { implicit session => - repository.insert(DefaultLearningPath.copy(owner = owner)) + repository.insert(DefaultLearningPath.copy(owner = owner)).get throw new RuntimeException("Provoking exception inside transaction") } fail("Exception should prevent normal execution") @@ -155,7 +156,7 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That updating several times is not throwing optimistic locking exception") { - val inserted = repository.insert(DefaultLearningPath) + val inserted = repository.insert(DefaultLearningPath).get val firstUpdate = repository.update(inserted.copy(title = List(Title("First change", "unknown")))) val secondUpdate = repository.update(firstUpdate.copy(title = List(Title("Second change", "unknown")))) val thirdUpdate = repository.update(secondUpdate.copy(title = List(Title("Third change", "unknown")))) @@ -169,7 +170,7 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That trying to update a learningPath with old revision number throws optimistic locking exception") { - val inserted = repository.insert(DefaultLearningPath) + val inserted = repository.insert(DefaultLearningPath).get repository.update(inserted.copy(title = List(Title("First change", "unknown")))) assertResult( @@ -183,16 +184,23 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit repository.deletePath(inserted.id.get) } - test("That trying to update a learningStep with old revision throws optimistic locking exception") { - val learningPath = repository.insert(DefaultLearningPath) - val insertedStep = repository.insertLearningStep(DefaultLearningStep.copy(learningPathId = learningPath.id)) - repository.updateLearningStep(insertedStep.copy(title = List(Title("First change", "unknown")))) + test( + "That trying to update a learningPath with old revision while modifying steps throws optimistic locking exception" + ) { + val learningPath = repository.insert(DefaultLearningPath.copy(learningsteps = Seq(DefaultLearningStep))).get + repository.update( + learningPath.copy(learningsteps = Seq(DefaultLearningStep.copy(title = List(Title("First change", "unknown"))))) + ) assertResult( - s"Conflicting revision is detected for learningStep with id = ${insertedStep.id} and revision = ${insertedStep.revision}" + s"Conflicting revision is detected for learningPath with id = ${learningPath.id} and revision = ${learningPath.revision}" ) { intercept[OptimisticLockException] { - repository.updateLearningStep(insertedStep.copy(title = List(Title("First change", "unknown")))) + repository.update( + learningPath.copy(learningsteps = + Seq(DefaultLearningStep.copy(title = List(Title("Second change", "unknown")))) + ) + ) }.getMessage } @@ -200,11 +208,11 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That learningPathsWithIsBasedOn returns all learningpaths that has one is based on id") { - val learningPath1 = repository.insert(DefaultLearningPath) - val learningPath2 = repository.insert(DefaultLearningPath) + val learningPath1 = repository.insert(DefaultLearningPath).get + val learningPath2 = repository.insert(DefaultLearningPath).get - val copiedLearningPath1 = repository.insert(learningPath1.copy(id = None, isBasedOn = learningPath1.id)) - val copiedLearningPath2 = repository.insert(learningPath1.copy(id = None, isBasedOn = learningPath1.id)) + val copiedLearningPath1 = repository.insert(learningPath1.copy(id = None, isBasedOn = learningPath1.id)).get + val copiedLearningPath2 = repository.insert(learningPath1.copy(id = None, isBasedOn = learningPath1.id)).get val learningPaths = repository.learningPathsWithIsBasedOn(learningPath1.id.get) @@ -222,14 +230,16 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That allPublishedTags returns only published tags") { - val publicPath = repository.insert( - DefaultLearningPath.copy( - status = LearningPathStatus.PUBLISHED, - tags = List(Tag(Seq("aaa"), "nb"), Tag(Seq("bbb"), "nn"), Tag(Seq("ccc"), "en")), + val publicPath = repository + .insert( + DefaultLearningPath.copy( + status = LearningPathStatus.PUBLISHED, + tags = List(Tag(Seq("aaa"), "nb"), Tag(Seq("bbb"), "nn"), Tag(Seq("ccc"), "en")), + ) ) - ) + .get - val privatePath = repository.insert(DefaultLearningPath.copy(tags = List(Tag(Seq("ddd"), "nb")))) + val privatePath = repository.insert(DefaultLearningPath.copy(tags = List(Tag(Seq("ddd"), "nb")))).get val publicTags = repository.allPublishedTags publicTags should contain(Tag(Seq("aaa"), "nb")) @@ -242,15 +252,19 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That allPublishedTags removes duplicates") { - val publicPath1 = repository.insert( - DefaultLearningPath.copy( - status = LearningPathStatus.PUBLISHED, - tags = List(Tag(Seq("aaa"), "nb"), Tag(Seq("aaa"), "nn")), + val publicPath1 = repository + .insert( + DefaultLearningPath.copy( + status = LearningPathStatus.PUBLISHED, + tags = List(Tag(Seq("aaa"), "nb"), Tag(Seq("aaa"), "nn")), + ) ) - ) - val publicPath2 = repository.insert( - DefaultLearningPath.copy(status = LearningPathStatus.PUBLISHED, tags = List(Tag(Seq("aaa", "bbb"), "nb"))) - ) + .get + val publicPath2 = repository + .insert( + DefaultLearningPath.copy(status = LearningPathStatus.PUBLISHED, tags = List(Tag(Seq("aaa", "bbb"), "nb"))) + ) + .get val publicTags = repository.allPublishedTags publicTags should contain(Tag(Seq("aaa", "bbb"), "nb")) @@ -263,25 +277,29 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That allPublishedContributors returns only published contributors") { - val publicPath = repository.insert( - DefaultLearningPath.copy( - status = LearningPathStatus.PUBLISHED, - copyright = LearningpathCopyright( - "by", - List( - Author(ContributorType.Writer, "James Bond"), - Author(ContributorType.Writer, "Christian Bond"), - Author(ContributorType.Writer, "Jens Petrius"), + val publicPath = repository + .insert( + DefaultLearningPath.copy( + status = LearningPathStatus.PUBLISHED, + copyright = LearningpathCopyright( + "by", + List( + Author(ContributorType.Writer, "James Bond"), + Author(ContributorType.Writer, "Christian Bond"), + Author(ContributorType.Writer, "Jens Petrius"), + ), ), - ), + ) ) - ) + .get - val privatePath = repository.insert( - DefaultLearningPath.copy(copyright = - LearningpathCopyright("by", List(Author(ContributorType.Writer, "Test testesen"))) + val privatePath = repository + .insert( + DefaultLearningPath.copy(copyright = + LearningpathCopyright("by", List(Author(ContributorType.Writer, "Test testesen"))) + ) ) - ) + .get val publicContributors = repository.allPublishedContributors publicContributors should contain(Author(ContributorType.Writer, "James Bond")) @@ -294,28 +312,32 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That allPublishedContributors removes duplicates") { - val publicPath1 = repository.insert( - DefaultLearningPath.copy( - status = LearningPathStatus.PUBLISHED, - copyright = LearningpathCopyright( - "by", - List( - Author(ContributorType.Writer, "James Bond"), - Author(ContributorType.Writer, "Christian Bond"), - Author(ContributorType.Writer, "Jens Petrius"), + val publicPath1 = repository + .insert( + DefaultLearningPath.copy( + status = LearningPathStatus.PUBLISHED, + copyright = LearningpathCopyright( + "by", + List( + Author(ContributorType.Writer, "James Bond"), + Author(ContributorType.Writer, "Christian Bond"), + Author(ContributorType.Writer, "Jens Petrius"), + ), ), - ), + ) ) - ) - val publicPath2 = repository.insert( - DefaultLearningPath.copy( - status = LearningPathStatus.PUBLISHED, - copyright = LearningpathCopyright( - "by", - List(Author(ContributorType.Writer, "James Bond"), Author(ContributorType.Writer, "Test testesen")), - ), + .get + val publicPath2 = repository + .insert( + DefaultLearningPath.copy( + status = LearningPathStatus.PUBLISHED, + copyright = LearningpathCopyright( + "by", + List(Author(ContributorType.Writer, "James Bond"), Author(ContributorType.Writer, "Test testesen")), + ), + ) ) - ) + .get val publicContributors = repository.allPublishedContributors publicContributors should contain(Author(ContributorType.Writer, "James Bond")) @@ -330,18 +352,34 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That only learningsteps with status ACTIVE are returned together with a learningpath") { - val learningPath = repository.insert(DefaultLearningPath) - repository.insertLearningStep(DefaultLearningStep.copy(learningPathId = learningPath.id)) - repository.insertLearningStep(DefaultLearningStep.copy(learningPathId = learningPath.id)) - repository.insertLearningStep( - DefaultLearningStep.copy(learningPathId = learningPath.id, status = StepStatus.DELETED) - ) + val learningPath = repository + .insert( + DefaultLearningPath.copy(learningsteps = + Seq(DefaultLearningStep, DefaultLearningStep, DefaultLearningStep.copy(status = StepStatus.DELETED)) + ) + ) + .get learningPath.id.isDefined should be(true) val savedLearningPath = repository.withId(learningPath.id.get) savedLearningPath.isDefined should be(true) - savedLearningPath.get.learningsteps.get.size should be(2) - savedLearningPath.get.learningsteps.get.forall(_.status == StepStatus.ACTIVE) should be(true) + savedLearningPath.get.learningsteps.size should be(2) + savedLearningPath.get.learningsteps.forall(_.status == StepStatus.ACTIVE) should be(true) + + repository.deletePath(learningPath.id.get) + } + + test("That insert assigns ids to embedded steps and learningStepWithId reads from the document") { + val steps = Seq(DefaultLearningStep, DefaultLearningStep.copy(seqNo = 1)) + val learningPath = repository.insert(DefaultLearningPath.copy(learningsteps = steps)).get + + learningPath.learningsteps.forall(_.id.isDefined) should be(true) + learningPath.learningsteps.forall(_.learningPathId.contains(learningPath.id.get)) should be(true) + + val stepId = learningPath.learningsteps.head.id.get + val found = repository.learningStepWithId(learningPath.id.get, stepId) + found.isDefined should be(true) + found.get.id should be(Some(stepId)) repository.deletePath(learningPath.id.get) } @@ -350,8 +388,9 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit when(clock.now()).thenReturn(NDLADate.fromUnixTime(0)) val steps = List(DefaultLearningStep, DefaultLearningStep, DefaultLearningStep) - val learningPath = - repository.insert(DefaultLearningPath.copy(learningsteps = Some(steps), status = LearningPathStatus.PUBLISHED)) + val learningPath = repository + .insert(DefaultLearningPath.copy(learningsteps = steps, status = LearningPathStatus.PUBLISHED)) + .get val page1 = repository.getPublishedLearningPathByPage(2, 0) val page2 = repository.getPublishedLearningPathByPage(2, 2) @@ -365,12 +404,15 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit test("That getLearningPathByPage returns only published results") { val steps = List(DefaultLearningStep, DefaultLearningStep, DefaultLearningStep) - val learningPath1 = - repository.insert(DefaultLearningPath.copy(learningsteps = Some(steps), status = LearningPathStatus.PRIVATE)) - val learningPath2 = - repository.insert(DefaultLearningPath.copy(learningsteps = Some(steps), status = LearningPathStatus.PRIVATE)) - val learningPath3 = - repository.insert(DefaultLearningPath.copy(learningsteps = Some(steps), status = LearningPathStatus.PUBLISHED)) + val learningPath1 = repository + .insert(DefaultLearningPath.copy(learningsteps = steps, status = LearningPathStatus.PRIVATE)) + .get + val learningPath2 = repository + .insert(DefaultLearningPath.copy(learningsteps = steps, status = LearningPathStatus.PRIVATE)) + .get + val learningPath3 = repository + .insert(DefaultLearningPath.copy(learningsteps = steps, status = LearningPathStatus.PUBLISHED)) + .get val page1 = repository.getPublishedLearningPathByPage(2, 0) val page2 = repository.getPublishedLearningPathByPage(2, 2) @@ -388,13 +430,13 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit val steps = Vector(DefaultLearningStep, DefaultLearningStep, DefaultLearningStep) val path = DefaultLearningPath.copy( - learningsteps = Some(steps), + learningsteps = steps, status = LearningPathStatus.PRIVATE, owner = "123", message = Some(Message("this is message", "kwawk", clock.now())), ) - val inserted = repository.insert(path) + val inserted = repository.insert(path).get val fetched = repository.withId(inserted.id.get) inserted should be(fetched.get) @@ -402,10 +444,10 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That get by ids gets all results") { - val learningPath1 = repository.insert(DefaultLearningPath) - val learningPath2 = repository.insert(DefaultLearningPath) - val learningPath3 = repository.insert(DefaultLearningPath) - val learningPath4 = repository.insert(DefaultLearningPath) + val learningPath1 = repository.insert(DefaultLearningPath).get + val learningPath2 = repository.insert(DefaultLearningPath).get + val learningPath3 = repository.insert(DefaultLearningPath).get + val learningPath4 = repository.insert(DefaultLearningPath).get val learningpaths = repository.pageWithIds( Seq(learningPath1.id.get, learningPath2.id.get, learningPath3.id.get, learningPath4.id.get), @@ -416,32 +458,36 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That learning step sample only returns learningpaths containing a learningstep with an external link") { - repository.insert( - DefaultLearningPath.copy( - learningsteps = Some(List(DefaultLearningStep, DefaultLearningStep.copy(embedUrl = List.empty))), - status = LearningPathStatus.UNLISTED, - isMyNDLAOwner = true, + repository + .insert( + DefaultLearningPath.copy( + learningsteps = List(DefaultLearningStep, DefaultLearningStep.copy(embedUrl = List.empty)), + status = LearningPathStatus.UNLISTED, + isMyNDLAOwner = true, + ) ) - ); - repository.insert( - DefaultLearningPath.copy( - learningsteps = Some( - List( + .get + repository + .insert( + DefaultLearningPath.copy( + learningsteps = List( DefaultLearningStep.copy(embedUrl = List.empty, articleId = Some(1)), DefaultLearningStep.copy(embedUrl = List.empty, articleId = Some(2)), - ) - ), - status = LearningPathStatus.UNLISTED, - isMyNDLAOwner = true, + ), + status = LearningPathStatus.UNLISTED, + isMyNDLAOwner = true, + ) ) - ); - val lp3 = repository.insert( - DefaultLearningPath.copy( - learningsteps = Some(List(DefaultLearningStep)), - status = LearningPathStatus.UNLISTED, - isMyNDLAOwner = true, + .get + val lp3 = repository + .insert( + DefaultLearningPath.copy( + learningsteps = List(DefaultLearningStep), + status = LearningPathStatus.UNLISTED, + isMyNDLAOwner = true, + ) ) - ); + .get val learningpaths = repository.getExternalLinkStepSamples() learningpaths.length should be(2) @@ -449,15 +495,16 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That learning step sample only returns published learningpaths containing an external link") { - - repository.insert(DefaultLearningPath.copy(learningsteps = Some(List(DefaultLearningStep)), isMyNDLAOwner = true)) - val lp2 = repository.insert( - DefaultLearningPath.copy( - learningsteps = Some(List(DefaultLearningStep)), - status = LearningPathStatus.UNLISTED, - isMyNDLAOwner = true, + repository.insert(DefaultLearningPath.copy(learningsteps = List(DefaultLearningStep), isMyNDLAOwner = true)).get + val lp2 = repository + .insert( + DefaultLearningPath.copy( + learningsteps = List(DefaultLearningStep), + status = LearningPathStatus.UNLISTED, + isMyNDLAOwner = true, + ) ) - ) + .get val learningpaths = repository.getExternalLinkStepSamples() learningpaths.length should be(1) @@ -465,20 +512,24 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That learning step sample only returns learningpaths owned by MyNDLA") { - repository.insert( - DefaultLearningPath.copy( - learningsteps = Some(List(DefaultLearningStep)), - status = LearningPathStatus.UNLISTED, - isMyNDLAOwner = false, + repository + .insert( + DefaultLearningPath.copy( + learningsteps = List(DefaultLearningStep), + status = LearningPathStatus.UNLISTED, + isMyNDLAOwner = false, + ) ) - ) - val lp2 = repository.insert( - DefaultLearningPath.copy( - learningsteps = Some(List(DefaultLearningStep)), - status = LearningPathStatus.UNLISTED, - isMyNDLAOwner = true, + .get + val lp2 = repository + .insert( + DefaultLearningPath.copy( + learningsteps = List(DefaultLearningStep), + status = LearningPathStatus.UNLISTED, + isMyNDLAOwner = true, + ) ) - ) + .get val learningpaths = repository.getExternalLinkStepSamples() learningpaths.length should be(1) @@ -486,20 +537,24 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit } test("That learning step sample only returns learningpaths with an active step with an external link") { - repository.insert( - DefaultLearningPath.copy( - learningsteps = Some(List(DefaultLearningStep.copy(status = StepStatus.DELETED))), - status = LearningPathStatus.UNLISTED, - isMyNDLAOwner = true, + repository + .insert( + DefaultLearningPath.copy( + learningsteps = List(DefaultLearningStep.copy(status = StepStatus.DELETED)), + status = LearningPathStatus.UNLISTED, + isMyNDLAOwner = true, + ) ) - ) - val lp2 = repository.insert( - DefaultLearningPath.copy( - learningsteps = Some(List(DefaultLearningStep)), - status = LearningPathStatus.UNLISTED, - isMyNDLAOwner = true, + .get + val lp2 = repository + .insert( + DefaultLearningPath.copy( + learningsteps = List(DefaultLearningStep), + status = LearningPathStatus.UNLISTED, + isMyNDLAOwner = true, + ) ) - ) + .get val learningpaths = repository.getExternalLinkStepSamples() learningpaths.length should be(1) @@ -509,7 +564,6 @@ class LearningPathRepositoryIntegrationTest extends DatabaseIntegrationSuite wit def emptyTestDatabase: Boolean = { DBUtil.writeSession(implicit session => { sql"delete from learningpaths;".execute()(using session) - sql"delete from learningsteps;".execute()(using session) }) } diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/ConverterServiceTest.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/ConverterServiceTest.scala index 1baf59f553..24b1026a3d 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/ConverterServiceTest.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/ConverterServiceTest.scala @@ -146,7 +146,7 @@ class ConverterServiceTest extends UnitSuite with UnitTestEnvironment { owner = "me", copyright = LearningpathCopyright(CC_BY.toString, List.empty), isMyNDLAOwner = false, - learningsteps = None, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = Priority.Unspecified, @@ -605,14 +605,13 @@ class ConverterServiceTest extends UnitSuite with UnitTestEnvironment { None, ) val lpId = 5591L - val lp1 = TestData.sampleDomainLearningPath.copy(id = Some(lpId), learningsteps = None) - val lp2 = TestData.sampleDomainLearningPath.copy(id = Some(lpId), learningsteps = Some(Seq.empty)) + val lp1 = TestData.sampleDomainLearningPath.copy(id = Some(lpId), learningsteps = Seq.empty) + val lp2 = TestData.sampleDomainLearningPath.copy(id = Some(lpId), learningsteps = Seq.empty) val lp3 = TestData .sampleDomainLearningPath .copy( id = Some(lpId), - learningsteps = - Some(Seq(TestData.domainLearningStep1.copy(seqNo = 0), TestData.domainLearningStep2.copy(seqNo = 1))), + learningsteps = Seq(TestData.domainLearningStep1.copy(seqNo = 0), TestData.domainLearningStep2.copy(seqNo = 1)), ) service.asDomainLearningStep(newLs, lp1, owner.id).get.seqNo should be(0) @@ -622,7 +621,7 @@ class ConverterServiceTest extends UnitSuite with UnitTestEnvironment { test("mergeLearningSteps correctly retains nullable fields") { val updatedStep = api.UpdatedLearningStepV2DTO( - 2, + 1, commonApi.Missing, commonApi.Missing, "nb", @@ -642,7 +641,7 @@ class ConverterServiceTest extends UnitSuite with UnitTestEnvironment { test("mergeLearningSteps correctly deletes correct language version of nullable fields") { val updatedStep = api.UpdatedLearningStepV2DTO( - 2, + 1, commonApi.Delete, commonApi.Delete, "nn", @@ -663,7 +662,7 @@ class ConverterServiceTest extends UnitSuite with UnitTestEnvironment { test("mergeLearningSteps correctly updates language fields") { val updatedStep = api.UpdatedLearningStepV2DTO( - 2, + 1, commonApi.UpdateWith("Tittel på bokmål oppdatert"), commonApi.UpdateWith("Introduksjon på bokmål oppdatert"), "nb", diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/ReadServiceTest.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/ReadServiceTest.scala index e1600113f8..0f8ac82ade 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/ReadServiceTest.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/ReadServiceTest.scala @@ -63,6 +63,7 @@ class ReadServiceTest extends UnitSuite with UnitTestEnvironment { owner = PUBLISHED_OWNER.id, copyright = copyright, isMyNDLAOwner = false, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = Priority.Unspecified, @@ -88,6 +89,7 @@ class ReadServiceTest extends UnitSuite with UnitTestEnvironment { owner = PRIVATE_OWNER.id, copyright = copyright, isMyNDLAOwner = false, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = Priority.Unspecified, diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/UpdateServiceTest.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/UpdateServiceTest.scala index 2db30e58b4..7e498c8f41 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/UpdateServiceTest.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/UpdateServiceTest.scala @@ -22,12 +22,13 @@ import no.ndla.common.model.{NDLADate, api as commonApi, domain as common} import no.ndla.learningpathapi.* import no.ndla.learningpathapi.model.* import no.ndla.learningpathapi.model.api.* +import no.ndla.learningpathapi.model.domain.OptimisticLockException import no.ndla.mapping.License import no.ndla.network.model.CombinedUserWithMyNDLAUser import no.ndla.network.tapir.auth.Permission.{LEARNINGPATH_API_ADMIN, LEARNINGPATH_API_PUBLISH, LEARNINGPATH_API_WRITE} import no.ndla.network.tapir.auth.TokenUser import org.mockito.ArgumentMatchers.{any, eq as eqTo} -import org.mockito.Mockito.{doAnswer, never, times, verify, when} +import org.mockito.Mockito.{doAnswer, doReturn, never, times, verify, when} import org.mockito.invocation.InvocationOnMock import scalikejdbc.DBSession @@ -234,7 +235,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { owner = PUBLISHED_OWNER.id, copyright = copyright, isMyNDLAOwner = false, - learningsteps = Some(STEP1 :: STEP2 :: STEP3 :: STEP4 :: STEP5 :: STEP6 :: Nil), + learningsteps = STEP1 :: STEP2 :: STEP3 :: STEP4 :: STEP5 :: STEP6 :: Nil, responsible = None, comments = Seq.empty, priority = common.Priority.Unspecified, @@ -260,7 +261,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { owner = PUBLISHED_OWNER.id, copyright = copyright, isMyNDLAOwner = false, - learningsteps = None, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = common.Priority.Unspecified, @@ -286,7 +287,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { owner = PRIVATE_OWNER.id, copyright = copyright, isMyNDLAOwner = false, - learningsteps = Some(STEP1 :: STEP2 :: STEP3 :: STEP4 :: STEP5 :: STEP6 :: Nil), + learningsteps = STEP1 :: STEP2 :: STEP3 :: STEP4 :: STEP5 :: STEP6 :: Nil, responsible = None, comments = Seq.empty, priority = common.Priority.Unspecified, @@ -312,7 +313,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { owner = PRIVATE_OWNER.id, copyright = copyright, isMyNDLAOwner = false, - learningsteps = None, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = common.Priority.Unspecified, @@ -338,7 +339,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { owner = PRIVATE_OWNER.id, copyright = copyright, isMyNDLAOwner = false, - learningsteps = Some(STEP1 :: STEP2 :: STEP3 :: STEP4 :: STEP5 :: STEP6 :: Nil), + learningsteps = STEP1 :: STEP2 :: STEP3 :: STEP4 :: STEP5 :: STEP6 :: Nil, responsible = None, comments = Seq.empty, priority = common.Priority.Unspecified, @@ -426,6 +427,18 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningStepValidator.validate(any[LearningStep], any[LearningPath], any[Boolean])).thenAnswer( (i: InvocationOnMock) => Success(i.getArgument[LearningStep](0)) ) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) + when(learningPathRepository.withIdWithInactiveSteps(any[Long], any[Boolean])(using any[DBSession])).thenAnswer( + (i: InvocationOnMock) => { + val id = i.getArgument[Long](0) + val includeDeleted = i.getArgument[Boolean](1) + val session = i.getArgument[DBSession](2) + val fetched = + if (includeDeleted) learningPathRepository.withIdIncludingDeleted(id)(using session) + else learningPathRepository.withId(id)(using session) + Option(fetched).flatten + } + ) doAnswer((i: InvocationOnMock) => { val x = i.getArgument[DBSession => Try[?]](0) x(mock[DBSession]) @@ -433,7 +446,9 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { } test("That addLearningPathV2 inserts the given LearningPathV2") { - when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn(PRIVATE_LEARNINGPATH) + when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn( + Success(PRIVATE_LEARNINGPATH) + ) val saved = service.addLearningPathV2(NEW_PRIVATE_LEARNINGPATHV2, PRIVATE_OWNER.toCombined) assert(saved.get.id == PRIVATE_LEARNINGPATH.id.get) @@ -452,7 +467,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { test("That updateLearningPathV2 updates the learningpath when the given user is the owner if the status is PRIVATE") { when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(PRIVATE_LEARNINGPATH)) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(PRIVATE_LEARNINGPATH) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) assertResult(PRIVATE_LEARNINGPATH.id.get) { service.updateLearningPathV2(PRIVATE_ID, UPDATED_PRIVATE_LEARNINGPATHV2, PRIVATE_OWNER.toCombined).get.id @@ -468,7 +483,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { val unlistedLp = PRIVATE_LEARNINGPATH.copy(status = LearningPathStatus.UNLISTED) when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(unlistedLp)) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(unlistedLp) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) assertResult(PRIVATE_LEARNINGPATH.id.get) { service.updateLearningPathV2(PRIVATE_ID, UPDATED_PRIVATE_LEARNINGPATHV2, PRIVATE_OWNER.toCombined).get.id @@ -490,6 +505,29 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { } } + test("That updateLearningPathV2 preserves deleted steps on update") { + val deletedStep = STEP1.copy(status = StepStatus.DELETED) + val learningPathWithDeleted = PRIVATE_LEARNINGPATH.copy(learningsteps = Seq(STEP2, deletedStep)) + + // Simulate repository filtering for withId, and ensure update uses raw steps. + when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn( + Some(learningPathWithDeleted.withOnlyActiveSteps) + ) + doReturn(Some(learningPathWithDeleted)) + .when(learningPathRepository) + .withIdWithInactiveSteps(eqTo(PRIVATE_ID), eqTo(false))(using any[DBSession]) + when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer(_.getArgument(0)) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) + + val pathCaptor: ArgumentCaptor[LearningPath] = ArgumentCaptor.forClass(classOf[LearningPath]) + service.updateLearningPathV2(PRIVATE_ID, UPDATED_PRIVATE_LEARNINGPATHV2, PRIVATE_OWNER.toCombined).get + verify(learningPathRepository, times(1)).withIdWithInactiveSteps(eqTo(PRIVATE_ID), eqTo(false))(using + any[DBSession] + ) + verify(learningPathRepository).update(pathCaptor.capture())(using any) + pathCaptor.getValue.learningsteps.exists(_.status == StepStatus.DELETED) should be(true) + } + test("That updateLearningPathV2 returns Failure if user is not the owner") { when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(PRIVATE_LEARNINGPATH)) @@ -508,7 +546,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn( PUBLISHED_LEARNINGPATH.copy(status = LearningPathStatus.UNLISTED) ) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) val result = service .updateLearningPathV2(PUBLISHED_ID, UPDATED_PUBLISHED_LEARNINGPATHV2, PUBLISHED_OWNER.toCombined) @@ -520,7 +558,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { test("That updateLearningPathV2 status PRIVATE remains PRIVATE if not publisher") { when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(PRIVATE_LEARNINGPATH)) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(PRIVATE_LEARNINGPATH) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) val result = service.updateLearningPathV2(PRIVATE_ID, UPDATED_PRIVATE_LEARNINGPATHV2, PRIVATE_OWNER.toCombined).get result.id should be(PRIVATE_LEARNINGPATH.id.get) @@ -545,7 +583,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn( PUBLISHED_LEARNINGPATH.copy(status = learningpath.LearningPathStatus.PRIVATE) ) - when(learningPathRepository.learningPathsWithIsBasedOn(PUBLISHED_ID)).thenReturn(List()) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(PUBLISHED_ID)).thenReturn(List()) assertResult("PRIVATE") { service @@ -569,7 +607,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn( PUBLISHED_LEARNINGPATH.copy(status = learningpath.LearningPathStatus.PRIVATE) ) - when(learningPathRepository.learningPathsWithIsBasedOn(PUBLISHED_ID)).thenReturn(List()) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(PUBLISHED_ID)).thenReturn(List()) val nowDate = NDLADate.fromUnixTime(1337) when(clock.now()).thenReturn(nowDate) val user = PRIVATE_OWNER.copy(permissions = Set(LEARNINGPATH_API_ADMIN)).toCombined @@ -593,7 +631,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn( PUBLISHED_LEARNINGPATH.copy(status = learningpath.LearningPathStatus.PRIVATE) ) - when(learningPathRepository.learningPathsWithIsBasedOn(PUBLISHED_ID)).thenReturn(List()) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(PUBLISHED_ID)).thenReturn(List()) assertResult("PRIVATE") { service @@ -620,7 +658,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn( PRIVATE_LEARNINGPATH.copy(status = learningpath.LearningPathStatus.DELETED) ) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) assertResult("DELETED") { service @@ -638,7 +676,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn( DELETED_LEARNINGPATH.copy(status = learningpath.LearningPathStatus.UNLISTED) ) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) assertResult("UNLISTED") { service @@ -682,7 +720,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn( PUBLISHED_LEARNINGPATH.copy(status = learningpath.LearningPathStatus.DELETED) ) - when(learningPathRepository.learningPathsWithIsBasedOn(PUBLISHED_ID)).thenReturn( + when(learningPathRepository.learningPathsWithIsBasedOnRaw(PUBLISHED_ID)).thenReturn( List( DELETED_LEARNINGPATH.copy(id = Some(9), isBasedOn = Some(PUBLISHED_ID)), DELETED_LEARNINGPATH.copy(id = Some(8), isBasedOn = Some(PUBLISHED_ID)), @@ -702,7 +740,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { } verify(learningPathRepository, times(3)).update(any[LearningPath])(using any) - verify(learningPathRepository, times(1)).learningPathsWithIsBasedOn(any[Long]) + verify(learningPathRepository, times(1)).learningPathsWithIsBasedOnRaw(any[Long]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) } @@ -720,7 +758,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { } test("That updateLearningPathStatusV2 allows owner to edit PUBLISHED to PRIVATE") { - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) when(learningPathRepository.withIdIncludingDeleted(eqTo(PUBLISHED_ID))(using any[DBSession])).thenReturn( Some(PUBLISHED_LEARNINGPATH) ) @@ -739,7 +777,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { } test("That updateLearningPathStatusV2 allows owner to edit PUBLISHED to UNLISTED") { - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) when(learningPathRepository.withIdIncludingDeleted(eqTo(PUBLISHED_ID))(using any[DBSession])).thenReturn( Some(PUBLISHED_LEARNINGPATH) ) @@ -777,7 +815,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => i.getArgument[LearningPath](0) ) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) when(clock.now()).thenReturn(NDLADate.fromUnixTime(0)) val expected = @@ -800,7 +838,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => i.getArgument[LearningPath](0) ) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) when(clock.now()).thenReturn(NDLADate.MIN) service.updateLearningPathStatusV2( @@ -823,7 +861,6 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(None) val Failure(ex) = service.addLearningStepV2(PRIVATE_ID, NEW_STEPV2, PRIVATE_OWNER.toCombined): @unchecked ex.isInstanceOf[NotFoundException] should be(true) - verify(learningPathRepository, never).insertLearningStep(any[LearningStep])(using any) verify(learningPathRepository, never).update(any[LearningPath])(using any) } @@ -831,34 +868,52 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { "That addLearningStepV2 inserts the learningstepV2 and update lastUpdated on the learningpath when the given user is the owner and status is PRIVATE" ) { when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(PRIVATE_LEARNINGPATH)) - when(learningPathRepository.insertLearningStep(any[LearningStep])(using any[DBSession])).thenReturn(STEP1) + when(learningPathRepository.generateStepId()(using any[DBSession])).thenReturn(STEP1.id.get) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(PRIVATE_LEARNINGPATH) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) + + val result = service.addLearningStepV2(PRIVATE_ID, NEW_STEPV2, PRIVATE_OWNER.toCombined).get + result.id should be(STEP1.id.get) - assertResult(STEP1.id.get) { - service.addLearningStepV2(PRIVATE_ID, NEW_STEPV2, PRIVATE_OWNER.toCombined).get.id - } - verify(learningPathRepository, times(1)).insertLearningStep(any[LearningStep])(using any[DBSession]) verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) verify(searchIndexService, times(0)).deleteDocument(any[LearningPath], any) } + test("That addLearningStepV2 computes seqNo from active steps when raw path includes deleted steps") { + val insertedStepId = 99L + val deletedWithHighSeqNo = STEP3.copy(id = Some(30L), seqNo = 7, status = StepStatus.DELETED) + val learningPathWithSteps = PRIVATE_LEARNINGPATH.copy(learningsteps = Seq(STEP1, STEP2, deletedWithHighSeqNo)) + + doReturn(Some(learningPathWithSteps)) + .when(learningPathRepository) + .withIdWithInactiveSteps(eqTo(PRIVATE_ID), eqTo(false))(using any[DBSession]) + when(learningPathRepository.generateStepId()(using any[DBSession])).thenReturn(insertedStepId) + when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer(_.getArgument(0)) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) + + service.addLearningStepV2(PRIVATE_ID, NEW_STEPV2, PRIVATE_OWNER.toCombined).isSuccess should be(true) + + val pathCaptor: ArgumentCaptor[LearningPath] = ArgumentCaptor.forClass(classOf[LearningPath]) + verify(learningPathRepository).update(pathCaptor.capture())(using any[DBSession]) + val insertedStep = pathCaptor.getValue.learningsteps.find(_.id.contains(insertedStepId)) + insertedStep.get.seqNo should be(2) + } + test( "That addLearningStep inserts the learningstep and update lastUpdated on the learningpath when the given user is the owner and status is PUBLISHED" ) { when(learningPathRepository.withId(eqTo(PUBLISHED_ID))(using any[DBSession])).thenReturn( Some(PUBLISHED_LEARNINGPATH) ) - when(learningPathRepository.insertLearningStep(any[LearningStep])(using any[DBSession])).thenReturn(STEP2) + when(learningPathRepository.generateStepId()(using any[DBSession])).thenReturn(STEP2.id.get) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => i.getArgument[LearningPath](0) ) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) assertResult(STEP2.id.get) { service.addLearningStepV2(PUBLISHED_ID, NEW_STEPV2, PUBLISHED_OWNER.toCombined).get.id } - verify(learningPathRepository, times(1)).insertLearningStep(any[LearningStep])(using any[DBSession]) verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) verify(searchIndexService, times(0)).deleteDocument(any[LearningPath], any) @@ -879,7 +934,6 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { service.updateLearningStepV2(PUBLISHED_ID, STEP1.id.get, UPDATED_STEPV2, PUBLISHED_OWNER.toCombined): @unchecked ex.isInstanceOf[NotFoundException] should be(true) - verify(learningPathRepository, never).updateLearningStep(any[LearningStep])(using any) verify(learningPathRepository, never).update(any[LearningPath])(using any) } @@ -892,7 +946,21 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { val Failure(ex) = service.updateLearningStepV2(PUBLISHED_ID, STEP1.id.get, UPDATED_STEPV2, PUBLISHED_OWNER.toCombined): @unchecked ex.isInstanceOf[NotFoundException] should be(true) - verify(learningPathRepository, never).updateLearningStep(any[LearningStep])(using any[DBSession]) + verify(learningPathRepository, never).update(any[LearningPath])(using any[DBSession]) + } + + test("That updateLearningStepV2 returns optimistic lock error when revision is stale") { + when(learningPathRepository.withId(eqTo(PUBLISHED_ID))(using any[DBSession])).thenReturn( + Some(PUBLISHED_LEARNINGPATH) + ) + when(learningPathRepository.learningStepWithId(eqTo(PUBLISHED_ID), eqTo(STEP1.id.get))(using any[DBSession])) + .thenReturn(Some(STEP1)) + + val staleUpdate = UPDATED_STEPV2.copy(revision = UPDATED_STEPV2.revision - 1) + val Failure(ex) = + service.updateLearningStepV2(PUBLISHED_ID, STEP1.id.get, staleUpdate, PUBLISHED_OWNER.toCombined): @unchecked + + ex.isInstanceOf[OptimisticLockException] should be(true) verify(learningPathRepository, never).update(any[LearningPath])(using any[DBSession]) } @@ -904,7 +972,6 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { ) when(learningPathRepository.learningStepWithId(eqTo(PUBLISHED_ID), eqTo(STEP1.id.get))(using any[DBSession])) .thenReturn(Some(STEP1)) - when(learningPathRepository.updateLearningStep(any[LearningStep])(using any[DBSession])).thenReturn(STEP1) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(PUBLISHED_LEARNINGPATH) assertResult(STEP1.id.get) { @@ -918,7 +985,6 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { .get .id } - verify(learningPathRepository, times(1)).updateLearningStep(any[LearningStep])(using any[DBSession]) verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) } @@ -929,14 +995,12 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(PRIVATE_LEARNINGPATH)) when(learningPathRepository.learningStepWithId(eqTo(PRIVATE_ID), eqTo(STEP1.id.get))(using any[DBSession])) .thenReturn(Some(STEP1)) - when(learningPathRepository.updateLearningStep(any[LearningStep])(using any[DBSession])).thenReturn(STEP1) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(PRIVATE_LEARNINGPATH) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) assertResult(STEP1.id.get) { service.updateLearningStepV2(PRIVATE_ID, STEP1.id.get, UPDATED_STEPV2, PRIVATE_OWNER.toCombined).get.id } - verify(learningPathRepository, times(1)).updateLearningStep(any[LearningStep])(using any[DBSession]) verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) } @@ -994,22 +1058,16 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.learningStepWithId(eqTo(PRIVATE_ID), eqTo(STEP1.id.get))(using any[DBSession])) .thenReturn(Some(STEP1)) when(learningPathRepository.learningStepsFor(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(List(STEP1)) - when( - learningPathRepository.updateLearningStep(eqTo(STEP1.copy(status = StepStatus.DELETED, lastUpdated = nowDate)))( - using any[DBSession] - ) - ).thenReturn(STEP1.copy(status = StepStatus.DELETED, lastUpdated = nowDate)) - when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(PRIVATE_LEARNINGPATH) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => + i.getArgument[LearningPath](0) + ) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) val updatedStep = service.updateLearningStepStatusV2(PRIVATE_ID, STEP1.id.get, StepStatus.DELETED, PRIVATE_OWNER.toCombined) updatedStep.isSuccess should be(true) updatedStep.get.status should equal(StepStatus.DELETED.entryName) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP1.copy(status = StepStatus.DELETED, lastUpdated = nowDate)) - )(using any[DBSession]) verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) } @@ -1023,29 +1081,22 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.learningStepWithId(eqTo(PUBLISHED_ID), eqTo(STEP2.id.get))(using any[DBSession])) .thenReturn(Some(STEP2)) when(learningPathRepository.learningStepsFor(eqTo(PUBLISHED_ID))(using any[DBSession])).thenReturn( - PUBLISHED_LEARNINGPATH.learningsteps.get - ) - when(learningPathRepository.updateLearningStep(any)(using any)).thenAnswer((i: InvocationOnMock) => - i.getArgument[LearningStep](0) + PUBLISHED_LEARNINGPATH.learningsteps ) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => i.getArgument[LearningPath](0) ) val updatedDate = NDLADate.fromUnixTime(0) when(clock.now()).thenReturn(updatedDate) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) val updatedStep = service.updateLearningStepStatusV2(PUBLISHED_ID, STEP2.id.get, StepStatus.DELETED, PUBLISHED_OWNER.toCombined) updatedStep.isSuccess should be(true) updatedStep.get.status should equal(StepStatus.DELETED.entryName) + updatedStep.get.revision should equal(2) - val expectedUpdatePath = PUBLISHED_LEARNINGPATH.copy(learningsteps = None, lastUpdated = updatedDate) - - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP2.copy(status = StepStatus.DELETED, lastUpdated = updatedDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).update(eqTo(expectedUpdatePath))(using any[DBSession]) + verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) verify(searchIndexService, times(0)).deleteDocument(any[LearningPath], any) } @@ -1060,26 +1111,16 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.learningStepsFor(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn( List(STEP1, STEP2, STEP3) ) - when(learningPathRepository.updateLearningStep(any)(using any[DBSession])).thenAnswer((i: InvocationOnMock) => - i.getArgument[LearningStep](0) + when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => + i.getArgument[LearningPath](0) ) - when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(PRIVATE_LEARNINGPATH) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) val updatedStep = service.updateLearningStepStatusV2(PRIVATE_ID, STEP1.id.get, StepStatus.DELETED, PRIVATE_OWNER.toCombined) updatedStep.isSuccess should be(true) updatedStep.get.status should equal(StepStatus.DELETED.entryName) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP1.copy(status = StepStatus.DELETED, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP2.copy(seqNo = STEP2.seqNo - 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP3.copy(seqNo = STEP3.seqNo - 1, lastUpdated = nowDate)) - )(using any[DBSession]) verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) } @@ -1094,26 +1135,16 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.learningStepsFor(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn( List(STEP1, STEP2, STEP3) ) - when(learningPathRepository.updateLearningStep(any)(using any[DBSession])).thenAnswer((i: InvocationOnMock) => - i.getArgument[LearningStep](0) + when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => + i.getArgument[LearningPath](0) ) - when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenReturn(PRIVATE_LEARNINGPATH) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) val updatedStep = service.updateLearningStepStatusV2(PRIVATE_ID, STEP1.id.get, StepStatus.ACTIVE, PRIVATE_OWNER.toCombined) updatedStep.isSuccess should be(true) updatedStep.get.status should equal(StepStatus.ACTIVE.entryName) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP1.copy(status = StepStatus.ACTIVE, seqNo = 0, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep(eqTo(STEP2.copy(seqNo = 1, lastUpdated = nowDate)))( - using any[DBSession] - ) - verify(learningPathRepository, times(1)).updateLearningStep(eqTo(STEP3.copy(seqNo = 2, lastUpdated = nowDate)))( - using any[DBSession] - ) verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(any[LearningPath]) } @@ -1155,24 +1186,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { val updatedStep = service.updateSeqNo(PRIVATE_ID, STEP1.id.get, STEP6.seqNo, PRIVATE_OWNER.toCombined) updatedStep.get.seqNo should equal(STEP6.seqNo) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP2.copy(seqNo = STEP2.seqNo - 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP3.copy(seqNo = STEP3.seqNo - 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP4.copy(seqNo = STEP4.seqNo - 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP5.copy(seqNo = STEP5.seqNo - 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP6.copy(seqNo = STEP6.seqNo - 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP1.copy(seqNo = STEP6.seqNo, lastUpdated = nowDate)) - )(using any[DBSession]) + verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) } test("That updateSeqNo from last to 0 updates all learningsteps in between") { @@ -1186,24 +1200,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { val updatedStep = service.updateSeqNo(PRIVATE_ID, STEP6.id.get, STEP1.seqNo, PRIVATE_OWNER.toCombined) updatedStep.get.seqNo should equal(STEP1.seqNo) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP6.copy(seqNo = STEP1.seqNo, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP1.copy(seqNo = STEP1.seqNo + 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP2.copy(seqNo = STEP2.seqNo + 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP3.copy(seqNo = STEP3.seqNo + 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP4.copy(seqNo = STEP4.seqNo + 1, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP5.copy(seqNo = STEP5.seqNo + 1, lastUpdated = nowDate)) - )(using any[DBSession]) + verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) } test("That updateSeqNo between two middle steps only updates the two middle steps") { @@ -1217,12 +1214,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { val updatedStep = service.updateSeqNo(PRIVATE_ID, STEP2.id.get, STEP3.seqNo, PRIVATE_OWNER.toCombined) updatedStep.get.seqNo should equal(STEP3.seqNo) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP2.copy(seqNo = STEP3.seqNo, lastUpdated = nowDate)) - )(using any[DBSession]) - verify(learningPathRepository, times(1)).updateLearningStep( - eqTo(STEP3.copy(seqNo = STEP2.seqNo, lastUpdated = nowDate)) - )(using any[DBSession]) + verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) } test("That updateSeqNo also update seqNo for all affected steps") { @@ -1233,7 +1225,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { val updatedStep = service.updateSeqNo(PRIVATE_ID, STEP1.id.get, 1, PRIVATE_OWNER.toCombined) updatedStep.get.seqNo should equal(1) - verify(learningPathRepository, times(2)).updateLearningStep(any[LearningStep])(using any[DBSession]) + verify(learningPathRepository, times(1)).update(any[LearningPath])(using any[DBSession]) } test("new fromExisting2 should allow language fields set to unknown") { @@ -1242,7 +1234,9 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.withId(eqTo(learningpathWithUnknownLang.id.get))(using any[DBSession])).thenReturn( Some(learningpathWithUnknownLang) ) - when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn(learningpathWithUnknownLang) + when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn( + Success(learningpathWithUnknownLang) + ) val newCopy = NewCopyLearningPathV2DTO("hehe", None, None, "nb", None, None, None, None) service @@ -1264,20 +1258,18 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { test("owner updates step private should not update status") { val newDate = NDLADate.fromUnixTime(648000000) - val stepWithBadTitle = STEP1.copy(title = Seq(common.Title("Dårlig tittel", "nb")), lastUpdated = newDate) + val stepWithBadTitle = + STEP1.copy(title = Seq(common.Title("Dårlig tittel", "nb")), revision = Some(2), lastUpdated = newDate) when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(PRIVATE_LEARNINGPATH)) when(learningPathRepository.learningStepWithId(eqTo(PRIVATE_ID), eqTo(STEP1.id.get))(using any[DBSession])) .thenReturn(Some(STEP1)) when(learningPathRepository.learningStepsFor(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(List()) - when(learningPathRepository.updateLearningStep(eqTo(stepWithBadTitle))(using any[DBSession])).thenReturn( - stepWithBadTitle - ) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => i.getArgument[LearningPath](0) ) when(clock.now()).thenReturn(newDate) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) val updatedLs = UpdatedLearningStepV2DTO( 1, @@ -1295,10 +1287,9 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { service.updateLearningStepV2(PRIVATE_ID, STEP1.id.get, updatedLs, PRIVATE_OWNER.toCombined) val updatedPath = PRIVATE_LEARNINGPATH.copy( lastUpdated = newDate, - learningsteps = Some(PRIVATE_LEARNINGPATH.learningsteps.get.tail ++ List(stepWithBadTitle)), + learningsteps = PRIVATE_LEARNINGPATH.learningsteps.tail ++ List(stepWithBadTitle), ) - verify(learningPathRepository, times(1)).updateLearningStep(eqTo(stepWithBadTitle))(using any[DBSession]) verify(learningPathRepository, times(1)).update(eqTo(updatedPath))(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(updatedPath) verify(searchIndexService, times(0)).deleteDocument(eqTo(updatedPath), any) @@ -1306,7 +1297,8 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { test("admin updates step should not update status") { val newDate = NDLADate.fromUnixTime(648000000) - val stepWithBadTitle = STEP1.copy(title = Seq(common.Title("Dårlig tittel", "nb")), lastUpdated = newDate) + val stepWithBadTitle = + STEP1.copy(title = Seq(common.Title("Dårlig tittel", "nb")), revision = Some(2), lastUpdated = newDate) when(learningPathRepository.withId(eqTo(PUBLISHED_ID))(using any[DBSession])).thenReturn( Some(PUBLISHED_LEARNINGPATH) @@ -1314,9 +1306,6 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.learningStepWithId(eqTo(PUBLISHED_ID), eqTo(STEP1.id.get))(using any[DBSession])) .thenReturn(Some(STEP1)) when(learningPathRepository.learningStepsFor(eqTo(PUBLISHED_ID))(using any[DBSession])).thenReturn(List()) - when(learningPathRepository.updateLearningStep(eqTo(stepWithBadTitle))(using any[DBSession])).thenReturn( - stepWithBadTitle - ) when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer((i: InvocationOnMock) => i.getArgument[LearningPath](0) ) @@ -1343,10 +1332,9 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { ) val updatedPath = PUBLISHED_LEARNINGPATH.copy( lastUpdated = newDate, - learningsteps = Some(PUBLISHED_LEARNINGPATH.learningsteps.get.tail ++ List(stepWithBadTitle)), + learningsteps = PUBLISHED_LEARNINGPATH.learningsteps.tail ++ List(stepWithBadTitle), ) - verify(learningPathRepository, times(1)).updateLearningStep(eqTo(stepWithBadTitle))(using any[DBSession]) verify(learningPathRepository, times(1)).update(eqTo(updatedPath))(using any[DBSession]) verify(searchIndexService, times(1)).indexDocument(updatedPath) } @@ -1366,7 +1354,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { Some(PUBLISHED_LEARNINGPATH_NO_STEPS) ) when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn( - PUBLISHED_LEARNINGPATH_NO_STEPS + Success(PUBLISHED_LEARNINGPATH_NO_STEPS) ) service.newFromExistingV2(PUBLISHED_ID, NEW_COPIED_LEARNINGPATHV2, PRIVATE_OWNER.toCombined) @@ -1392,7 +1380,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { Some(PRIVATE_LEARNINGPATH_NO_STEPS) ) when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn( - PRIVATE_LEARNINGPATH_NO_STEPS + Success(PRIVATE_LEARNINGPATH_NO_STEPS) ) service.newFromExistingV2(PRIVATE_ID, NEW_COPIED_LEARNINGPATHV2, PRIVATE_OWNER.toCombined) @@ -1418,7 +1406,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { Some(PUBLISHED_LEARNINGPATH_NO_STEPS) ) when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn( - PUBLISHED_LEARNINGPATH_NO_STEPS + Success(PUBLISHED_LEARNINGPATH_NO_STEPS) ) service.newFromExistingV2(PUBLISHED_ID, NEW_COPIED_LEARNINGPATHV2, MYNDLA_USER) @@ -1446,7 +1434,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { Some(PUBLISHED_LEARNINGPATH_NO_STEPS) ) when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn( - PUBLISHED_LEARNINGPATH_NO_STEPS + Success(PUBLISHED_LEARNINGPATH_NO_STEPS) ) val titlesToOverride = "Overridden title" @@ -1496,7 +1484,9 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { when(learningPathRepository.withId(eqTo(PUBLISHED_ID))(using any[DBSession])).thenReturn( Some(PUBLISHED_LEARNINGPATH) ) - when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn(PUBLISHED_LEARNINGPATH) + when(learningPathRepository.insert(any[LearningPath])(using any[DBSession])).thenReturn( + Success(PUBLISHED_LEARNINGPATH) + ) service.newFromExistingV2(PUBLISHED_ID, NEW_COPIED_LEARNINGPATHV2, PRIVATE_OWNER.toCombined) @@ -1511,24 +1501,21 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { lastUpdated = now, learningsteps = PUBLISHED_LEARNINGPATH .learningsteps - .map( - _.map { step => - val stepCopyright = step.`type` match { - case StepType.TEXT if step.copyright.isEmpty => Some(PUBLISHED_LEARNINGPATH.copyright) - case StepType.TEXT => step.copyright - case _ => None - - } - step.copy( - id = None, - revision = None, - externalId = None, - learningPathId = None, - copyright = stepCopyright, - lastUpdated = now, - ) + .map { step => + val stepCopyright = step.`type` match { + case StepType.TEXT if step.copyright.isEmpty => Some(PUBLISHED_LEARNINGPATH.copyright) + case StepType.TEXT => step.copyright + case _ => None } - ), + step.copy( + id = None, + revision = None, + externalId = None, + learningPathId = None, + copyright = stepCopyright, + lastUpdated = now, + ) + }, ) verify(learningPathRepository, times(1)).insert(eqTo(expectedNewLearningPath))(using any) @@ -1547,7 +1534,7 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { i.getArgument[LearningPath](0) ) when(clock.now()).thenReturn(newDate) - when(learningPathRepository.learningPathsWithIsBasedOn(any[Long])).thenReturn(List.empty) + when(learningPathRepository.learningPathsWithIsBasedOnRaw(any[Long])).thenReturn(List.empty) val lpToUpdate = UpdatedLearningPathV2DTO( 1, @@ -1605,15 +1592,16 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { test("That delete learning step removes language from all language fields") { val step = STEP1.copy(title = Seq(Title("Tittel", "nb"), Title("Title", "en"))) - val lp = PRIVATE_LEARNINGPATH.copy(learningsteps = Some(Seq(step))) + val lp = PRIVATE_LEARNINGPATH.copy(learningsteps = Seq(step)) - val stepCaptor: ArgumentCaptor[LearningStep] = ArgumentCaptor.forClass(classOf[LearningStep]) + val pathCaptor: ArgumentCaptor[LearningPath] = ArgumentCaptor.forClass(classOf[LearningPath]) when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(lp)) when(learningPathRepository.learningStepWithId(eqTo(PRIVATE_ID), eqTo(step.id.get))(using any[DBSession])) .thenReturn(Some(step)) + when(learningPathRepository.update(any[LearningPath])(using any[DBSession])).thenAnswer(_.getArgument(0)) service.deleteLearningStepLanguage(lp.id.get, step.id.get, "en", PRIVATE_OWNER.toCombined) - verify(learningPathRepository).updateLearningStep(stepCaptor.capture())(using any) - stepCaptor.getValue.title.length should be(1) + verify(learningPathRepository).update(pathCaptor.capture())(using any) + pathCaptor.getValue.learningsteps.head.title.length should be(1) } test("That delete learning path language should fail when only one language") { @@ -1631,16 +1619,13 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { title = PRIVATE_LEARNINGPATH.title :+ Title("Tittel", "nn"), description = PRIVATE_LEARNINGPATH.description :+ Description("Beskrivelse", "nn"), learningsteps = - Some(PRIVATE_LEARNINGPATH.learningsteps.get.map(step => step.copy(title = step.title :+ Title("Tittel", "nn")))), + PRIVATE_LEARNINGPATH.learningsteps.map(step => step.copy(title = step.title :+ Title("Tittel", "nn"))), ) when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(learningPath)) - when(learningPathRepository.updateLearningStep(any)(using any[DBSession])).thenAnswer(_.getArgument(0)) when(learningPathRepository.update(any)(using any[DBSession])).thenAnswer(_.getArgument(0)) val res = service.deleteLearningPathLanguage(PRIVATE_ID, "nb", PRIVATE_OWNER.toCombined).failIfFailure - verify(learningPathRepository, times(PRIVATE_LEARNINGPATH.learningsteps.get.size)).updateLearningStep(any)(using - any - ) + verify(learningPathRepository, times(1)).update(any[LearningPath])(using any) res.supportedLanguages should be(Seq("nn")) } @@ -1650,13 +1635,10 @@ class UpdateServiceTest extends UnitSuite with UnitTestEnvironment { description = PRIVATE_LEARNINGPATH.description :+ Description("Beskrivelse", "nn"), ) when(learningPathRepository.withId(eqTo(PRIVATE_ID))(using any[DBSession])).thenReturn(Some(learningPath)) - when(learningPathRepository.updateLearningStep(any)(using any[DBSession])).thenAnswer(_.getArgument(0)) when(learningPathRepository.update(any)(using any[DBSession])).thenAnswer(_.getArgument(0)) val res = service.deleteLearningPathLanguage(PRIVATE_ID, "nn", PRIVATE_OWNER.toCombined).failIfFailure - verify(learningPathRepository, times(PRIVATE_LEARNINGPATH.learningsteps.get.size)).updateLearningStep(any)(using - any - ) + verify(learningPathRepository, times(1)).update(any[LearningPath])(using any) res.supportedLanguages should be(Seq("nb")) } diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/search/SearchServiceTest.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/search/SearchServiceTest.scala index 7acec0f776..f6f872787f 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/search/SearchServiceTest.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/service/search/SearchServiceTest.scala @@ -70,6 +70,7 @@ class SearchServiceTest extends ElasticsearchIntegrationSuite with UnitSuite wit owner = "owner", copyright = copyright, isMyNDLAOwner = false, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = Priority.Unspecified, @@ -143,7 +144,7 @@ class SearchServiceTest extends ElasticsearchIntegrationSuite with UnitSuite wit created = yesterday, lastUpdated = yesterday, tags = List(Tag(Seq("superhelt", "kanikkefly"), "nb")), - learningsteps = Some(List(activeStep)), + learningsteps = List(activeStep), grepCodes = Seq("KM123", "KM456"), ) @@ -155,7 +156,7 @@ class SearchServiceTest extends ElasticsearchIntegrationSuite with UnitSuite wit created = yesterday, lastUpdated = today, tags = List(Tag(Seq("superhelt", "kanfly"), "nb")), - learningsteps = Some(List(activeStep, deletedStep)), + learningsteps = List(activeStep, deletedStep), grepCodes = Seq("KM456", "KM789"), ) @@ -167,7 +168,7 @@ class SearchServiceTest extends ElasticsearchIntegrationSuite with UnitSuite wit created = yesterday, lastUpdated = tomorrow, tags = List(Tag(Seq("disney", "kanfly"), "nb")), - learningsteps = Some(List(deletedStep)), + learningsteps = List(deletedStep), verificationStatus = LearningPathVerificationStatus.CREATED_BY_NDLA, grepCodes = Seq("KM123", "KM456", "KM789"), ) diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/validation/LearningPathValidatorTest.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/validation/LearningPathValidatorTest.scala index 112c35bc6e..e5157a9eb6 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/validation/LearningPathValidatorTest.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/validation/LearningPathValidatorTest.scala @@ -61,6 +61,7 @@ class LearningPathValidatorTest extends UnitSuite with TestEnvironment { owner = "", copyright = copyright, isMyNDLAOwner = false, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = Priority.Unspecified, diff --git a/learningpath-api/src/test/scala/no/ndla/learningpathapi/validation/LearningStepValidatorTest.scala b/learningpath-api/src/test/scala/no/ndla/learningpathapi/validation/LearningStepValidatorTest.scala index a859ebee8f..2bbc92c0a0 100644 --- a/learningpath-api/src/test/scala/no/ndla/learningpathapi/validation/LearningStepValidatorTest.scala +++ b/learningpath-api/src/test/scala/no/ndla/learningpathapi/validation/LearningStepValidatorTest.scala @@ -76,6 +76,7 @@ class LearningStepValidatorTest extends UnitSuite with TestEnvironment { owner = "owner", copyright = copyright, isMyNDLAOwner = false, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = Priority.Unspecified, diff --git a/search-api/src/main/scala/no/ndla/searchapi/service/search/SearchConverterService.scala b/search-api/src/main/scala/no/ndla/searchapi/service/search/SearchConverterService.scala index 78c941f06b..60a79b00fa 100644 --- a/search-api/src/main/scala/no/ndla/searchapi/service/search/SearchConverterService.scala +++ b/search-api/src/main/scala/no/ndla/searchapi/service/search/SearchConverterService.scala @@ -361,7 +361,7 @@ class SearchConverterService(using lastUpdated = lp.lastUpdated, defaultTitle = defaultTitle.map(_.title), tags = SearchableLanguageList(lp.tags.map(tag => LanguageValue(tag.language, tag.tags))), - learningsteps = lp.learningsteps.getOrElse(Seq.empty).map(asSearchableLearningStep).toList, + learningsteps = lp.withOnlyActiveSteps.learningsteps.map(asSearchableLearningStep).toList, license = lp.copyright.license, copyright = license, isBasedOn = lp.isBasedOn, diff --git a/search-api/src/test/scala/no/ndla/searchapi/TestData.scala b/search-api/src/test/scala/no/ndla/searchapi/TestData.scala index 50bd5db63a..347f9fc0d3 100644 --- a/search-api/src/test/scala/no/ndla/searchapi/TestData.scala +++ b/search-api/src/test/scala/no/ndla/searchapi/TestData.scala @@ -1035,6 +1035,7 @@ object TestData { owner = "owner", copyright = copyright, isMyNDLAOwner = false, + learningsteps = Seq.empty, responsible = None, comments = Seq.empty, priority = Priority.Unspecified, diff --git a/search-api/src/test/scala/no/ndla/searchapi/service/search/LearningPathIndexServiceTest.scala b/search-api/src/test/scala/no/ndla/searchapi/service/search/LearningPathIndexServiceTest.scala index 2a2c151e18..042dcd308e 100644 --- a/search-api/src/test/scala/no/ndla/searchapi/service/search/LearningPathIndexServiceTest.scala +++ b/search-api/src/test/scala/no/ndla/searchapi/service/search/LearningPathIndexServiceTest.scala @@ -59,26 +59,24 @@ class LearningPathIndexServiceTest extends ElasticsearchIntegrationSuite with Un val domainLearningPath = TestData .learningPath1 .copy( - learningsteps = Some( - List( - LearningStep( - id = Some(1L), - revision = Some(1), - externalId = Some("hei"), - learningPathId = Some(1L), - seqNo = 1, - title = Seq(Title("hei", "nb")), - introduction = Seq(), - description = Seq(LPDescription("hei", "nb")), - embedUrl = Seq(EmbedUrl("hei", "nb", EmbedType.OEmbed)), - articleId = None, - `type` = StepType.TEXT, - copyright = Some(LearningpathCopyright(license = "hei", contributors = Seq.empty)), - status = StepStatus.ACTIVE, - created = NDLADate.now(), - lastUpdated = NDLADate.now(), - owner = "yolo", - ) + learningsteps = List( + LearningStep( + id = Some(1L), + revision = Some(1), + externalId = Some("hei"), + learningPathId = Some(1L), + seqNo = 1, + title = Seq(Title("hei", "nb")), + introduction = Seq(), + description = Seq(LPDescription("hei", "nb")), + embedUrl = Seq(EmbedUrl("hei", "nb", EmbedType.OEmbed)), + articleId = None, + `type` = StepType.TEXT, + copyright = Some(LearningpathCopyright(license = "hei", contributors = Seq.empty)), + status = StepStatus.ACTIVE, + created = NDLADate.now(), + lastUpdated = NDLADate.now(), + owner = "yolo", ) ), responsible = Some(Responsible("yolo", NDLADate.now())), diff --git a/tapirtesting/src/main/scala/no/ndla/tapirtesting/TapirControllerTest.scala b/tapirtesting/src/main/scala/no/ndla/tapirtesting/TapirControllerTest.scala index 1c5994dd1a..6f4400430f 100644 --- a/tapirtesting/src/main/scala/no/ndla/tapirtesting/TapirControllerTest.scala +++ b/tapirtesting/src/main/scala/no/ndla/tapirtesting/TapirControllerTest.scala @@ -35,7 +35,6 @@ trait TapirControllerTest extends UnitTestSuite { override def beforeAll(): Unit = { super.beforeAll() - Thread .ofVirtual() .start(() => { @@ -52,7 +51,6 @@ trait TapirControllerTest extends UnitTestSuite { } override def afterAll(): Unit = server.foreach(_.stop()) - test("That no endpoints are shadowed") { import sttp.tapir.testing.EndpointVerifier val errors = EndpointVerifier(controller.endpoints.map(_.endpoint)) diff --git a/testbase/src/main/scala/no/ndla/testbase/UnitTestSuiteBase.scala b/testbase/src/main/scala/no/ndla/testbase/UnitTestSuiteBase.scala index 6ed12c595d..57ffcb4870 100644 --- a/testbase/src/main/scala/no/ndla/testbase/UnitTestSuiteBase.scala +++ b/testbase/src/main/scala/no/ndla/testbase/UnitTestSuiteBase.scala @@ -44,9 +44,9 @@ trait UnitTestSuiteBase socket.setReuseAddress(true) val port = socket.getLocalPort closeQuietly(socket) - return port; + return port } catch { - case e: IOException => System.err.println(("Failed to open socket", e)); + case e: IOException => System.err.println(("Failed to open socket", e)) } finally { if (socket != null) { closeQuietly(socket)