Conversation
For å få brukt riktig grunnbeløp i beregningen av refusjoner og korreksjoner må vi sørge for at grunnbeløpene som hentes via GrunnbelopService ender opp i beregningskoden. Derfor lager vi en Beregningskontekst, som kan utvides på sikt til å inkludere eventuelle andre ting som er nødvendig for å utføre beregninger.
a1eac9b to
6c5876d
Compare
There was a problem hiding this comment.
Pull request overview
Introduserer en Beregningskontekst for å føre grunnbeløp (G) fra GrunnbelopService inn i beregningskoden, slik at refusjoner/korreksjoner kan beregnes med riktig grunnbeløp for perioden og lagre hvilket grunnbeløp som faktisk ble brukt.
Changes:
- Legger til
Beregningskontekstog trer den gjennom beregningsfunksjonene (beregnRefusjonsbeløp,beregnRefusjon,beregnKorreksjon). - Lagrer grunnbeløp brukt i beregning i
Beregningog migrerer DB-skjema for nye felter. - Oppdaterer tjenester/kontrollere og tester til nye signaturer og tilførsel av grunnbeløp.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Beregningskontekst.kt | Ny kontekst som holder grunnbeløp-historikk for beregning. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Refusjonsberegner.kt | Trer inn beregningskontekst i beregning og forsøker å bruke periodens grunnbeløp i 5G-sjekk + lagrer brukt grunnbeløp. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Beregning.kt | Utvider beregningsresultat med grunnbeløp brukt + dato. |
| src/main/resources/db/migration/V62__grunnbelop_brukt_i_beregning.sql | Legger til kolonner for å persistere grunnbeløp brukt i beregning. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/grunnbelop/GrunnbelopService.kt | Endrer API til å returnere hele grunnbeløp-historikken (TreeMap) i stedet for enkeltoppslag. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/RefusjonService.kt | Injiserer GrunnbelopService og bygger Beregningskontekst ved beregning. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/AdminController.kt | Bygger Beregningskontekst i admin-reberegning endepunkter. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetSaksbehandler.kt | Bruker GrunnbelopService for å konstruere kontekst ved dry-run reberegning. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetBrukerService.kt | Trer GrunnbelopService inn i opprettelse av innlogget-bruker-objekter. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetArbeidsgiver.kt | Utvider constructor-signatur med GrunnbelopService. |
| src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/utils/5GSjekkerUtil.kt | Fjerner gammel hjelpefunksjon (5G-sjekk) fra util. |
| src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/RefusjonsberegningSteps.kt | Oppdaterer Cucumber-steps til å sende Beregningskontekst. |
| src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/RefusjonTest.kt | Oppdaterer enhetstester til å sende Beregningskontekst. |
| src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/KorreksjonTest.kt | Oppdaterer tester til ny signatur for korreksjonsberegning. |
| src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/inntekt/RefusjonsberegnerTest.kt | Oppdaterer beregningstester til å bruke Beregningskontekst. |
| src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/inntekt/RefusjonberegnerFratrekkFerieTest.kt | Trer GrunnbelopService inn i test-wiring der beregning kjøres. |
| src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetSaksbehandlerTest.kt | Oppdaterer test-wiring for ny dependency. |
| src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetArbeidsgiverTest.kt | Oppdaterer test-wiring for ny dependency. |
| src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/TestData.kt | Oppdaterer testdata-helper til å kalle ny beregnRefusjon-signatur. |
Comments suppressed due to low confidence (1)
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Refusjonsberegner.kt:140
gjenståendeEtterMaks5Gignores thegrunnbelopparameter and always uses the hardcodedåretsGconstant. This makes the 5G-cap calculation independent of the fetched grunnbeløp map and will be incorrect for other years/periods; compute5 * grunnbelop - sumUtbetalt(and consider removing theåretsGdependency once the map is the source of truth).
if (!refusjon.refusjonsgrunnlag.harTilstrekkeligInformasjonForBeregning()) {
return null
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Refusjonsberegner.kt
Show resolved
Hide resolved
src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetArbeidsgiverTest.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetArbeidsgiver.kt
Show resolved
Hide resolved
src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/inntekt/RefusjonberegnerFratrekkFerieTest.kt
Show resolved
Hide resolved
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Refusjonsberegner.kt
Show resolved
Hide resolved
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Refusjonsberegner.kt
Outdated
Show resolved
Hide resolved
ef65dfb to
ac8f9c8
Compare
ac8f9c8 to
b24a9ab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Refusjonsberegner.kt
Show resolved
Hide resolved
src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetArbeidsgiverTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/RefusjonsberegningSteps.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/KorreksjonTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/inntekt/RefusjonsberegnerTest.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/Refusjonsberegner.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/RefusjonService.kt
Show resolved
Hide resolved
src/main/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/grunnbelop/GrunnbelopService.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/autorisering/InnloggetArbeidsgiverTest.kt
Show resolved
Hide resolved
| tidligereRefundertBeløp = fratrekkRefunderbarBeløp, | ||
| overFemGrunnbeløp = overFemGrunnbeløp, | ||
| sumUtgifterFratrukketRefundertBeløp = sumUtgifterFratrukketRefundertBeløp.roundToInt() | ||
| sumUtgifterFratrukketRefundertBeløp = sumUtgifterFratrukketRefundertBeløp.roundToInt(), | ||
| grunnbelopBrukt = grunnbelopForPerioden.value, | ||
| grunnbelopDato = grunnbelopForPerioden.key, |
There was a problem hiding this comment.
New persisted fields grunnbelopBrukt/grunnbelopDato are now set on Beregning, but the existing beregning tests don’t assert these values. Adding assertions in an existing test (e.g. around the 5G-capping cases) would verify that the correct grunnbeløp entry is selected for tilskuddFom (including the boundary where tilskuddFom equals an effective-date).
01c7d85 to
a8debd8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/test/kotlin/no/nav/arbeidsgiver/tiltakrefusjon/refusjon/RefusjonsberegningSteps.kt
Show resolved
Hide resolved
| fun alleGrunnbelop(): TreeMap<LocalDate, Int> { | ||
| val grunnbelopMap: TreeMap<LocalDate, Int>? = cacheManager.getCache(FEM_G)?.get(cacheKey, { | ||
| logger.info("Cache miss for grunnbeløp, henter fra g.nav") | ||
| grunnbelopClient.alleGrunnbelop() | ||
| }) | ||
| return grunnbelopMap?.lowerEntry(dato)?.value ?: throw IllegalStateException("Fant ikke grunnbeløp for dato $dato") | ||
| // Lag kopi av cachet map for å unngå mutasjon | ||
| return grunnbelopMap?.toMap(TreeMap()) | ||
| ?: throw IllegalStateException("Kunne ikke hente grunnbeløp fra cache eller g.nav") |
There was a problem hiding this comment.
alleGrunnbelop() lager en ny TreeMap-kopi hver gang den kalles. Siden Beregningskontekst(grunnbelopService) brukes i beregningene, kan dette gi unødvendige allokeringer/kopiering ved høy trafikk. Vurder å returnere en uforanderlig view av den cachede mappen (eller cache en ferdigkopiert/immutabel map én gang), evt. la Beregningskontekst holde en referanse til en read-only map uten å kopiere per kall.
| data class Beregningskontekst( | ||
| val alleGrunnbelop: TreeMap<LocalDate, Int> | ||
| ) { | ||
| constructor(grunnbelopService: GrunnbelopService): this(grunnbelopService.alleGrunnbelop()) |
There was a problem hiding this comment.
Beregningskontekst er en ren datakontekst, men får en sekundær-konstruktør som tar inn GrunnbelopService. Det kobler domenekode til service-laget og gjør konteksten vanskeligere å bruke/teste uten Spring. Vurder å fjerne service-avhengigheten fra data-klassen (f.eks. en factory/mapper i service-laget som bygger Beregningskontekst), og la Beregningskontekst kun ta inn data (map) som primær input.
There was a problem hiding this comment.
Det er jo forsåvidt et godt poeng. Hvorfor ikke bare sende inn alle grunnbeløp istedenfor servic'en? 🤔
For å få brukt riktig grunnbeløp i beregningen
av refusjoner og korreksjoner må vi sørge for
at grunnbeløpene som hentes via GrunnbelopService
ender opp i beregningskoden.
Derfor lager vi en Beregningskontekst, som kan
utvides på sikt til å inkludere eventuelle andre
ting som er nødvendig for å utføre beregninger.