From 2d14e9aae48fb872ec6a1bb81bc45a233fdea66d Mon Sep 17 00:00:00 2001 From: Jonas Natten Date: Mon, 5 Jan 2026 14:50:54 +0100 Subject: [PATCH 1/3] refactor(learningpath-api): move learning steps into learningpath document JSON Migrate learning step persistence from the separate `learningsteps` table into the `learningpaths.document` JSONB payload, and update read/write flows, indexing, and tests to operate on embedded steps. - Add Flyway migration `V60__MoveLearningStepsToLearningPathDocument` to: - read existing path + step rows - enrich step documents with `id`, `revision`, `learningPathId`, `externalId` - embed sorted steps into each learning path document - delete migrated rows from `learningsteps` - Add migration `V61__AddStepIdIndex.sql` with a GIN index over embedded step IDs to keep step lookup performant. - Refactor `LearningPathRepository` to use `learningpaths.document` as the source of truth for steps: - fetch steps from embedded JSON - update step operations by rewriting the path document - assign path IDs + step IDs from existing sequences on insert - support imported step IDs while advancing sequence safely - add raw/non-filtered read helpers where deleted steps must be preserved. - Update `UpdateService` to: - operate on embedded steps for create/update/delete/seqNo/status actions - enforce step-level optimistic locking via revision checks - increment step revisions when mutating embedded steps - preserve deleted steps in persistence while exposing active-only views - index/search using active steps only. - Update `ConverterService` and search converters: - treat `learningsteps` as a required sequence - keep API conversion behavior compatible - filter to active steps for search indexing payloads. - Adjust internal endpoints and insert flow to return `Try`-based results for unified Tapir error handling. - Update model contract in `common`: - `LearningPath.learningsteps` is now `Seq[LearningStep]` (not `Option`) - JSON encoder/decoder now include `learningsteps` - add `withOnlyActiveSteps` helper on `LearningPath`. - Refresh integration/unit/e2e tests and test data across modules to match embedded-step persistence and new revision semantics. wip --- .../domain/learningpath/LearningPath.scala | 25 +- .../LearningpathApiClientTest.scala | 1 + .../controller/InternController.scala | 4 +- .../V39__MadeAvailableForThePublished.scala | 21 +- ...eLearningStepsToLearningPathDocument.scala | 131 +++++++ .../model/domain/DBLearningStep.scala | 47 --- .../model/domain/ImplicitLearningPath.scala | 5 +- .../repository/LearningPathRepository.scala | 308 +++++++--------- .../service/ConverterService.scala | 61 ++-- .../service/UpdateService.scala | 167 +++++---- .../SearchConverterServiceComponent.scala | 2 +- .../no/ndla/learningpathapi/TestData.scala | 2 +- ...rningStepsToLearningPathDocumentTest.scala | 80 +++++ .../LearningPathAndStepCreationTests.scala | 5 +- ...earningPathRepositoryIntegrationTest.scala | 330 ++++++++++------- .../service/ConverterServiceTest.scala | 15 +- .../service/ReadServiceTest.scala | 2 + .../service/UpdateServiceTest.scala | 340 +++++++++--------- .../service/search/SearchServiceTest.scala | 7 +- .../LearningPathValidatorTest.scala | 1 + .../LearningStepValidatorTest.scala | 1 + .../search/SearchConverterService.scala | 2 +- .../scala/no/ndla/searchapi/TestData.scala | 1 + .../search/LearningPathIndexServiceTest.scala | 38 +- .../tapirtesting/TapirControllerTest.scala | 2 - .../no/ndla/testbase/UnitTestSuiteBase.scala | 4 +- 26 files changed, 879 insertions(+), 723 deletions(-) create mode 100644 learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocument.scala delete mode 100644 learningpath-api/src/main/scala/no/ndla/learningpathapi/model/domain/DBLearningStep.scala create mode 100644 learningpath-api/src/test/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocumentTest.scala 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..26860f32a9 --- /dev/null +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocument.scala @@ -0,0 +1,131 @@ +/* + * 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 deleteSteps(learningPathId: Long)(using session: DBSession): Unit = { + val deleted = sql"delete from learningsteps where learning_path_id = $learningPathId".update() + logger.info(s"Deleted $deleted learning steps for learning path $learningPathId") + } + + 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) + deleteSteps(lpData.learningPathId)(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..cd3093cfd3 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 @@ -22,7 +22,6 @@ import org.mockito.invocation.InvocationOnMock import org.mockito.quality.Strictness import org.testcontainers.postgresql.PostgreSQLContainer import sttp.client3.quick.* - import java.util.concurrent.Executors import scala.concurrent.duration.DurationInt import scala.concurrent.{ExecutionContext, ExecutionContextExecutorService, Future} @@ -50,7 +49,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)) @@ -67,7 +66,7 @@ class LearningPathAndStepCreationTests } } - 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" 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) From 03d6085df9e9e135897dcf7238f1fd3969fa8ea7 Mon Sep 17 00:00:00 2001 From: Jonas Natten Date: Wed, 18 Feb 2026 12:01:26 +0100 Subject: [PATCH 2/3] chore(learningpath-api): improve learningpath crud e2e tests --- .../LearningPathAndStepCreationTests.scala | 394 +++++++++++++++--- 1 file changed, 335 insertions(+), 59 deletions(-) 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 cd3093cfd3..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,20 +8,24 @@ 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 import scala.concurrent.duration.DurationInt import scala.concurrent.{ExecutionContext, ExecutionContextExecutorService, Future} @@ -61,6 +65,7 @@ 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 } } @@ -98,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, @@ -115,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") { @@ -197,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)) From 5d87a79679618e3f42cdfab68f69827bb4f5bc9f Mon Sep 17 00:00:00 2001 From: Jonas Natten Date: Fri, 20 Feb 2026 10:05:07 +0100 Subject: [PATCH 3/3] refactor: avoid deleting learningstep rows when moving them to learningpath table this should make running the migration more robust, as it will not fail if the learningstep rows have already been deleted by a previous failed migration attempt. --- .../V60__MoveLearningStepsToLearningPathDocument.scala | 6 ------ 1 file changed, 6 deletions(-) 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 index 26860f32a9..bd5f40be94 100644 --- 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 @@ -87,11 +87,6 @@ class V60__MoveLearningStepsToLearningPathDocument extends BaseJavaMigration wit oldLp.mapObject(_.remove("learningsteps").add("learningsteps", Json.fromValues(updatedSteps))).noSpaces } - private def deleteSteps(learningPathId: Long)(using session: DBSession): Unit = { - val deleted = sql"delete from learningsteps where learning_path_id = $learningPathId".update() - logger.info(s"Deleted $deleted learning steps for learning path $learningPathId") - } - private def enrichStep(step: StepDocumentRowWithMeta): Json = { val json = CirceUtil.tryParse(step.learningStepDocument).get json.mapObject { obj => @@ -121,7 +116,6 @@ class V60__MoveLearningStepsToLearningPathDocument extends BaseJavaMigration wit allLearningPaths(offset * chunkSize).foreach { lpData => val steps = getStepDatas(lpData.learningPathId)(using session) updateLp(lpData, steps)(using session) - deleteSteps(lpData.learningPathId)(using session) } numPagesLeft -= 1 offset += 1