Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TB-140 Legg til nye endepunkter for arkivarisk historikk #214

Closed

Conversation

idagyl
Copy link
Contributor

@idagyl idagyl commented Jan 31, 2025

Bygger på: #207

Bakgrunn

Det må være mulig å hente ut tilstanden for egenregistrerte data for et gitt tidspunkt tilbake i tid. Dette er blant annet nødvendig for å støtte egenregistrert data inn i Historisk Matrikkelbrev (lovkrav).

Endringer 🚀

  • To nye interne endepunkter for uthenting av egenregistrert data basert på registreringstidspunkt, for henholdvis bygning og bruksenhet. Nytt tillegg på route oppbygningen: /arkiv?registreringstidspunkt=<datoher>
  • Gjenbrukt funksjonene i service laget ved å legge til et nytt parameter for registreringstidspunkt som defaulter til Instant.now() - etter samtale med William. I databasen, rent teknisk, henter vi ut materialiseringer frem til og med oppgitt registreringstidspunkt og bruker den nyeste av de.
  • Lagt til swagger for de nye endepunktene. Har litt problemer med genereringen av schema type for parameterne, men har en issue hos SMILEY4 her: Type not set for Path/Query parameters with CustomProcessor (Instant) SMILEY4/ktor-swagger-ui#180. Midlertidig tenkte jeg at vi kanskje kan leve med at det står "null (date-time)", ellers er det jo mulig å midlertidig sette den som <string> i swagger doc'en og heller legge inn date-time ellernoe i description🤷‍♀️ Ville ikke bruke for mye tid på det i denne oppgaven for å unngå blokking.

Har ikke lagt til noe ekstra tilgangsstyring for nå, det får bli en egen oppgave når vi har fått litt mer oversikt over scopet.

Tips til testing 🧪

  • Sjekk at du klarer å hente ut egenregistrert data på både bygning og bruksenhet via de nye endepunktene og med gyldig registreringstidspunkt.
  • Prøv å egenregistrere noe på en bygning eller bruksenhet. Hent ut langt tilbake i tid og sjekk at du ikke får med de nyeste endringene. Hent ut "nå" og sjekk at du får siste endring med.
  • Sjekk at du får 400 bad request dersom du oppgir et ugyldig datoformat.
  • Se over swagger doc og sjekk at det ser greit og forståelig ut. Kom gjerne med input på ordlyd som kan gjøre det mer beskrivende.

BTW! Jeg avventer å squshe til bedre commit melding osv til etter review så jeg slipper å gjøre det flere ganger 😄

@kvstrant
Copy link
Collaborator

Du kan se https://objektkatalog.geonorge.no/Objekttype/Index/EAID_1EBFCE48_A259_4966_A0DA_0505EBC2B31F for etablert navn på noen generelle felter i SOSI.

Copy link
Collaborator

@anderssonw anderssonw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nivået endepunktet skal ligge på, og paths
Jeg tenker at per nå er det greit nok at det ligger der det ligger nå, så får vi heller gå det opp ved behov for flere kilder til arkivarisk historikk. Det kjipe med å lage en egen "toppnivå" path er jo at vi må lage strukturen som allerede finnes på nytt, som jeg hvert fall tenker blir ganske masete med dagens innhold

Navnet "arkiv" og "fremTilDato"
Kan droppe Dato fra fremTil, for at det er et tidspunkt fremkommer fra typen. Jeg liker "arkiv" siden vi har brukt arkivarisk historikk som betegnelse, men jeg vet ikke egentlig om det betyr noe for andre folk i KV.

Gjenbruk av funksjoner i servicelaget
Gønne med gjenbruk, tenker det er så lite annerledes at det ikke er noe problem

@idagyl
Copy link
Contributor Author

idagyl commented Jan 31, 2025

Kan droppe Dato fra fremTil, for at det er et tidspunkt fremkommer fra typen (...)

Will fix 👍 Et annet spørsmål ref det feltet: gir begrepet "fremTil" fortsatt like mye mening nå som vi henter ut snapshots og ikke akkumulerer egenregistreringer frem til en viss dato on the fly? Vi henter vel bare ut étt objekt fra db nå 🤔 @anderssonw

@idagyl
Copy link
Contributor Author

idagyl commented Feb 3, 2025

Kan droppe Dato fra fremTil, for at det er et tidspunkt fremkommer fra typen (...)

Will fix 👍 Et annet spørsmål ref det feltet: gir begrepet "fremTil" fortsatt like mye mening nå som vi henter ut snapshots og ikke akkumulerer egenregistreringer frem til en viss dato on the fly? Vi henter vel bare ut étt objekt fra db nå 🤔 @anderssonw

Siste status fra diskusjon med William: registreringstidspunkt.

@idagyl idagyl marked this pull request as ready for review February 4, 2025 15:49
@idagyl idagyl requested a review from a team as a code owner February 4, 2025 15:49
@idagyl
Copy link
Contributor Author

idagyl commented Feb 4, 2025

Denne bygger som sagt på en annen PR som fortsatt jobbes med. Jeg løser opp i konflikter når PRen til @anderssonw er ferdig og jeg uansett må rebase etc 😄

Ble ganske mye swagger doc i én fil nå, som kanskje gjør det litt rotete å få oversikt over rutene 🤔 Snakket litt med William blant annet, og at det tidligere har vært trukket mer ut og at det også var litt uoversiktlig på sin egne måte. Så jeg så litt på om man i hvert fall kunne flytte på noe av det som er likt for de ulike nivåene innad i bygning routes for å unngå duplisering. Men usikker på om det ble så mye bedre altså. Fikk redusert litt gjentakende kode/boilerplate, men ble ikke akkurat veldig oversiktlig kan du si - om noe kanskje enda vanskeligere å få oversikt? 🤷‍♀️😅 Her er en kjapp versjon for å illustrere: https://github.com/kartverket/matrikkel-bygning-egenregistrering/compare/TB-140-nytt-endepunkt-for-arkiverte-egenregistreringer...testing-reuse-of-bygning-swagger-doc?expand=1

Sikkert en del andre ting man kan teste her også for å få det litt ryddigere swagger kode, men tenker det kanskje kan tas i en egen oppgave i så tilfelle så vi bare får ut disse endepunktene i første omgang 🚀

@anderssonw anderssonw force-pushed the cache-bygning-bruksenhet branch from 8330d92 to 8a3b7dc Compare February 6, 2025 08:48
@@ -128,6 +128,14 @@ class BygningRouteTest : TestApplicationWithDb() {
}
}
}

@Test
fun `gitt et ugyldig dato query parameter svarer bygning bad request`() = testApplication {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tenker vi bør ha en test som verifiserer at vi får tilbake korrekt data basert på registreringstidspunkt også (eventuelt om dette allerede er dekket av andre tester)

Base automatically changed from cache-bygning-bruksenhet to main February 6, 2025 11:13
@idagyl
Copy link
Contributor Author

idagyl commented Feb 6, 2025

For å gjøre det enklest mulig lukker jeg denne og åpnet en ny: #218

Ble alt for mye krøll med å bygge på andre branches når det er rebase/squashing og force pushing i flyten vår. Har ikke hatt noe problemer med det før pga man har valgt å beholde "all" historikk i tidligere team - men tenker jeg skal være litt mer forsiktig med å bygge på andre branches fremover 😅

@idagyl idagyl closed this Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants