refactor(learningpath-api): move learningsteps into learningpath document by jnatten · Pull Request #877 · NDLANO/backend

jnatten · 2026-02-18T09:40:30Z

Denne ble større enn jeg så for meg, så hadde satt pris på om dere også testet denne litt! Hadde ikke blitt overrasket om det ikke var noe jeg ikke har tenkt på.

This is part 1 of 3 in a stack made with GitButler:

…ument 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

gunnarvelle · 2026-02-19T07:34:49Z

~~Tester i lokal k3d, og sletting av steg oppdaterer ikkje forhåndsvisninga umiddelbart.~~

~~Opprett eller oppdater en sti med x antall steg~~
~~Sjekk at alle steg er i forhåndsvisning~~
~~Gå tilbake og slett et av stega~~
~~Sjekk forhåndsvisning at steget fremdeles ligger der. F5 fjerner ikkje steget.~~

Det må være cache for eg ser ikkje at stien hentes på nytt fra backend ved henting av sida på nytt og heller ikkje med disableSSR=true.

jnatten · 2026-02-19T07:50:59Z

Det må være cache for eg ser ikkje at stien hentes på nytt fra backend ved henting av sida på nytt og heller ikkje med disableSSR=true.

Hmm, testa om det oppfører seg annerledes i test?

gunnarvelle · 2026-02-19T07:53:38Z

Oppførte seg visst likt der.

gunnarvelle · 2026-02-19T13:27:24Z

...cala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocument.scala

+    oldLp.mapObject(_.remove("learningsteps").add("learningsteps", Json.fromValues(updatedSteps))).noSpaces
+  }
+
+  private def deleteSteps(learningPathId: Long)(using session: DBSession): Unit = {


Dette gjør at migreringa ikkje kan kjøres om igjen dersom den skulle feile. Kanskje heller ha en egen migrering i etterkant som berre dropper tabellen?

Vil ikke en feil rulle tilbake migreringa?

Ja det kan vi godt gjøre 😄

Vil ikke en feil rulle tilbake migreringa?

Om det er en feil som kastes så, men om det er en logisk feil e.l som vi ikke har tenkt på så vil det jo potensielt commites.

gunnarvelle

Har testa dette både fra min ndla og ed i lokalt cluster, så funksjonelt fant eg ingen problemer. @amatho er ofte nøyere enn meg på kode, så fint om du også approver.

gunnarvelle

Glømte sjølve approve

learningpath-api/src/main/scala/no/ndla/learningpathapi/repository/LearningPathRepository.scala

amatho · 2026-02-20T07:17:55Z

...cala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocument.scala

+    oldLp.mapObject(_.remove("learningsteps").add("learningsteps", Json.fromValues(updatedSteps))).noSpaces
+  }
+
+  private def deleteSteps(learningPathId: Long)(using session: DBSession): Unit = {


Vil ikke en feil rulle tilbake migreringa?

learningpath-api/src/main/scala/no/ndla/learningpathapi/repository/LearningPathRepository.scala

...cala/no/ndla/learningpathapi/db/migration/V60__MoveLearningStepsToLearningPathDocument.scala

learningpath-api/src/main/scala/no/ndla/learningpathapi/service/UpdateService.scala

amatho · 2026-02-20T09:05:24Z

learningpath-api/src/main/scala/no/ndla/learningpathapi/service/UpdateService.scala

+        val stepWithStatus =
+          if (isChangedStep) curr.copy(status = newStatus, lastUpdated = now)
+          else curr
+        val updatedStep = incrementStepRevision(stepWithStatus.copy(seqNo = seqNo, lastUpdated = now))


Skal revision inkrementeres for alle steps, selv om de ikke er oppdatert status?

For å være ærlig så er jeg ikke helt sikker på hvordan denne burde fungere jeg.
Denne kan jo potensielt endre på sequence number og. Hva tenker du?

Hmm blir kanskje noe som det her hvis vi bare vil endre revision for steps som får endring i enten status eller seqNo?

val updatedStep = if (isChangedStep || stepWithStatus.seqNo != seqNo) incrementStepRevision(stepWithStatus.copy(seqNo = seqNo, lastUpdated = now)) else stepWithStatus

Joda mer om det er det vi faktisk har lyst til.
Jeg tenker egentlig at enten så må alle revisjoner oppdateres (ellers vil jo låsingen ikke låse så bra om noen oppdaterer et annet steg?).

Evt så må vi kanskje droppe hele learningstep revisjonen og ta i bruk learningpath for hele greia alltid?

Åja tenkte ikke så nøye på at det med låsing er en greie, da er jeg enig med at det er best å bare øke revision på alle

learningpath-api/src/main/scala/no/ndla/learningpathapi/service/UpdateService.scala

…ngpath 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.

jnatten force-pushed the refactor/move-learningsteps-into-path-table branch from 82aa663 to b375bc9 Compare February 18, 2026 11:11

jnatten force-pushed the refactor/move-learningsteps-into-path-table branch from b375bc9 to 6b50cc7 Compare February 19, 2026 09:40

jnatten requested a review from a team February 19, 2026 10:35

jnatten mentioned this pull request Feb 19, 2026

refactor: specify schemas for SQLSyntaxSupport implementation #879

Open

gunnarvelle reviewed Feb 19, 2026

View reviewed changes

jnatten force-pushed the refactor/move-learningsteps-into-path-table branch from 6b50cc7 to a02369a Compare February 20, 2026 09:11

jnatten mentioned this pull request Feb 20, 2026

chore: delete learningsteps table #880

Open

gunnarvelle reviewed Feb 20, 2026

View reviewed changes

gunnarvelle approved these changes Feb 20, 2026

View reviewed changes

amatho reviewed Feb 20, 2026

View reviewed changes

jnatten force-pushed the refactor/move-learningsteps-into-path-table branch 3 times, most recently from f8f4ffa to b3a0393 Compare February 23, 2026 09:13

jnatten requested a review from amatho February 23, 2026 12:45

jnatten added 2 commits February 24, 2026 08:38

chore(learningpath-api): improve learningpath crud e2e tests

03d6085

refactor: avoid deleting learningstep rows when moving them to learni…

5d87a79

…ngpath 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.

jnatten force-pushed the refactor/move-learningsteps-into-path-table branch from 7ba587c to 5d87a79 Compare February 24, 2026 07:39

amatho approved these changes Feb 24, 2026

View reviewed changes

jnatten merged commit a3ca96c into master Feb 26, 2026
46 checks passed

jnatten deleted the refactor/move-learningsteps-into-path-table branch February 26, 2026 09:12

Conversation

jnatten commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gunnarvelle commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnatten commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gunnarvelle commented Feb 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gunnarvelle left a comment

Choose a reason for hiding this comment

Uh oh!

gunnarvelle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jnatten commented Feb 18, 2026 •

edited

Loading

gunnarvelle commented Feb 19, 2026 •

edited

Loading

jnatten commented Feb 19, 2026 •

edited

Loading