Utled historikk for markeringer#2868
Conversation
🔍 Detekt Code Quality ReportTotal Issues: 30
|
martinaa
force-pushed
the
utled-markeringer
branch
from
2feb4bd to
cdbcaa2
Compare
| // TODO: Hent fra oppgave-lib | ||
| data class MarkeringNyDto( | ||
| val markeringType: MarkeringForBehandling, | ||
| val begrunnelse: String? = null, | ||
| val opprettetAv: String? = null, | ||
| val opprettetTidspunkt: LocalDateTime, | ||
| val opprettetAvNavn: String? = null, | ||
| val hendelseType: MarkeringHendelseType? = null, | ||
| ) | ||
|
|
||
| // TODO: Hent fra oppgave-lib | ||
| enum class MarkeringHendelseType { | ||
| OPPRETTET, | ||
| FJERNET | ||
| } |
There was a problem hiding this comment.
Sikkert ok her, men kan også merge halve PR i oppgave først, så slippes dette :)
martinaa
force-pushed
the
utled-markeringer
branch
from
47877b5 to
958d276
Compare
| // TODO: Hent fra oppgave-lib | ||
| data class MarkeringNyDto( | ||
| val markeringType: MarkeringForBehandling, | ||
| val begrunnelse: String? = null, | ||
| val opprettetAv: String? = null, | ||
| val opprettetTidspunkt: LocalDateTime, | ||
| val opprettetAvNavn: String? = null, | ||
| val hendelseType: MarkeringHendelseType? = null, | ||
| ) | ||
|
|
||
| // TODO: Hent fra oppgave-lib | ||
| enum class MarkeringHendelseType { | ||
| OPPRETTET, | ||
| FJERNET | ||
| } |
There was a problem hiding this comment.
Jeg synes det blir mer riktig å duplisere klassene i behandlingsflyt (som du har gjort) enn å importere dem fra oppgave-kontrakt. Dette skapte krøll for meg sist jeg kjørte gentypes for behandlingsflyt, for da dukket plutselig en oppgave-type (MarkeringDto) opp i schema, samtidig som oppgavetypene egentlig genereres fra oppgave-backend på en annen måte. Da får man to varianter av samme type som kan være forskjellige.
There was a problem hiding this comment.
Om de dukket opp i skjema, er ikke det fordi de har blitt eksponert i skjemaet på et vis?
Egentlig tenker jeg begge ting burde skje: gateway bruker kontrakt, men api burde ikke gjøre det om vi eksponerer dette i behandlingsflyt-apiet. Der burde det dupliseres.
There was a problem hiding this comment.
Men blir det ikke litt rart å både duplisere og importere samme klasse ulike steder i behandlingsflyt? Hvis MarkeringDto oppdateres i oppgave så må man både huske på å bumpe oppgave-versjon og manuelt oppdatere den kopierte klassen? 🤔
There was a problem hiding this comment.
Mulig jeg ikke sjekket hvordan det blir brukt. Men i gateway er det jo oppgave som bestemmer - er det viktig at den som eksponereres i apiet holdes i synk?
There was a problem hiding this comment.
Gir det ikke mening å kun ha én MarkeringDto som bor i aap-oppgave og som både behandlingsflyt og saksbehandling bruker for å være i sync med typene? Internt i aap-oppgave bruker vi BehandlingMarkering (som har en .tilDto())
There was a problem hiding this comment.
Det som er "skummelt" nå er jo at vi får en to-veis-avhengighet mellom behandlingsflyt og oppgave:
Oppgave er avhengig av behandlingsflyt sin kontrakt for å fungere nå, som er litt synd, men greit nok. Det er ganske mange "tjukke" kontraktsobjekter som flyter den veien.
Behandlingsflyt har dratt inn oppgave sin kontrakt, men ikke brukt i noen særlig grad. For min del tenker jeg at vi ikke burde dra inn oppgave sin kontrakt ettersom vi risikerer at begge blir avhengige av hverandre for å komme videre. Med tanke på at vi gjør små og enkle kall mot oppgave kan godt de dto-objektene lages i behandlingsflyt og være frikoblet fra oppgave sin kontrakt.
Vi bruker nå:
- OppgaveEnhetDto og OppgaveEnhetResponse som enkelt kunne vært data class i behandlingsflyt
- MarkeringDto i løseren for totrinnsvurdering (!) som er veldig skummelt, siden endring i Oppgave vil endre kontrakten for hva som skal sendes inn i saksbehandlingsløseren - her vil jeg helst at vi bytter til en lokal data klasse
Hadde vi fjernet bruken av disse tre dto-ene kan vi klippe snora for "oppgave-kontrakten" fra behandlingsflyt.
There was a problem hiding this comment.
Kommer an på hvem menes med i synk :P. Jeg tenker at det er rart om vi skal forvente at api til behandlingsflyt skal forvente/levere ut samme typer som i oppgave. Da blir det fort veldig kronglete å endre på typen.
Virker litt kronglete å gjøre modellendringer om vi skal bruke type fra oppgave begge steder.
martinaa
force-pushed
the
utled-markeringer
branch
from
958d276 to
4194adf
Compare
5 participants
No description provided.