Skip to content

TFP-6069: V2 API for brev #7219

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

mrsladek
Copy link
Collaborator

@mrsladek mrsladek commented Mar 28, 2025

Først og fremst behandling, fagsak, resultat og verge - noe som alltid hentes for alle type brev.
Vi kjører expand kontrakt i formidling og sammenligner svar.
På sikt kan vi frikoble oss fra APIet som deles med frontend og fptilbake og starte med forenkling.

@mrsladek mrsladek requested a review from a team as a code owner March 28, 2025 13:02
@mrsladek mrsladek changed the title TFP-6069: Utkast av en v1 API for formidling slik at vi på sikt kan fjerne bruk av frontend lenkene TFP-6069: Utkast av en V2 API for brev slik at vi på sikt kan fjerne bruk av frontend lenkene Mar 28, 2025
@jolarsen
Copy link
Collaborator

Hovedproblemet er at lenker + Dtos brukes av både frontend og formidling (og tilbake)
Ressurslenkene er i seg selv ikke et problem - det gjelder bare å ha disjoint links til frontend, formidling og tilbake

Copy link
Collaborator

@jolarsen jolarsen left a comment

Choose a reason for hiding this comment

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

Hva med å flytte nye ting til no.nav.foreldrepenger.web.app.tjenester.formidling ?
Hvis du beholder under behandling/dto - så er det fint om du kaller det FormidlingFagsak eller BrevFagsak - samme med de andre V2 (behandlingresultat, verge, ..). Det bør være lett å lese formålet med klassene ut fra klassenavn / package

@mrsladek mrsladek marked this pull request as draft March 31, 2025 11:50
@mrsladek
Copy link
Collaborator Author

Ja, dette er en god idé å samle alt på ett sted. Dette er fortsatt et pågående arbeid, så det vil komme flere endringer underveis.

Jeg gir beskjed når alt er ferdig utviklet men takk for innspill så langt :)

@mrsladek mrsladek force-pushed the feat/TFP-6069_dedisert_brev_gen_api branch from de455b0 to 7d765a9 Compare April 8, 2025 12:06
@mrsladek mrsladek force-pushed the feat/TFP-6069_dedisert_brev_gen_api branch from 4110d7c to e86bf08 Compare May 6, 2025 09:34
@mrsladek mrsladek marked this pull request as ready for review May 8, 2025 07:33
@mrsladek mrsladek force-pushed the feat/TFP-6069_dedisert_brev_gen_api branch from 3b5b761 to bdb14d1 Compare May 8, 2025 13:43
@mrsladek mrsladek changed the title TFP-6069: Utkast av en V2 API for brev slik at vi på sikt kan fjerne bruk av frontend lenkene TFP-6069: V2 API for brev May 14, 2025
@mrsladek mrsladek force-pushed the feat/TFP-6069_dedisert_brev_gen_api branch from 408d81f to 88318bf Compare May 14, 2025 09:20
@mrsladek mrsladek requested a review from jolarsen May 14, 2025 09:22
Copy link
Collaborator

@jolarsen jolarsen left a comment

Choose a reason for hiding this comment

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

Ikke helt med på å duplisere kodeverkene - uten ref som tvinger fram vedlikehold.

  • Mange er statiske
  • Behandlingsårsaker - krever at man er bevisst på at den er duplisert. Vil gi feil i løpet av et års tid.

Enn så lenge så blir de serialisert som pg JsonValue.

Kan vurdere om det er verdt innsatsen med en expand-contract i retning plain enum - men tror kanskje vi skal se på noe felles kode.

Ellers bra tiltak !

@mrsladek
Copy link
Collaborator Author

Tanken med duplisert kodeverk var at man kunne flytte hele kontrakten ut til fp-kontrakter og så bruke på tvers mellom sak og formidling. Vi kan godt ha enumene duplisert og forenkle APiet først til noe som blir enklere å sentralisere.

@jolarsen
Copy link
Collaborator

Tanken med duplisert kodeverk var at man kunne flytte hele kontrakten ut til fp-kontrakter og så bruke på tvers mellom sak og formidling. Vi kan godt ha enumene duplisert og forenkle APiet først til noe som blir enklere å sentralisere.

Da ville jeg tenkt expand / contract: lagd plain enums med en gang og latt Dto ha to verdier/felter - en for kodeverdi-enums og en for de nye plain enumene. Når formidling er konvertert så kan kodeverdi-feltene fjernes i Dto.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
36.9% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

Copy link
Collaborator

@jolarsen jolarsen left a comment

Choose a reason for hiding this comment

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

Jeg ville gått for plain enum ganske fort - eller med en gang egentlig.
En expand/contract vil uansett kreve GammelEnum ytelseType + NyEnum yt1 i en overgangsfase.

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.

2 participants