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

Chore: koderydding del 4 #4500

Merged
merged 17 commits into from
Apr 19, 2024
Merged

Chore: koderydding del 4 #4500

merged 17 commits into from
Apr 19, 2024

Conversation

idaame
Copy link
Contributor

@idaame idaame commented Apr 19, 2024

💰 Hva skal gjøres, og hvorfor?

Skriv 1 eller 2 setninger om hvilken funksjonell endring som blir implementert. Ta med en lenke til Favro og
skriv en kort beskrivelse av hvorfor endringen skal gjøres.

Favro: https://favro.com/organization/98c34fb974ce445eac854de0/1844bbac3b6605eacc8f5543?card=NAV-20565

Sletter enda mer ubrukt kode. Største endringene her er at jeg slette enum-verdier for BrevPeriodeType som er deprecated, men rimelig sikker på at det skal gå bra da det aldri er noe vi lagrer ned. Kan verifisere i preprod før merge:)

Burde leses commit for commit!

🔎️ Er det noe spesielt du ønsker tilbakemelding om?

Er det noe du er usikker på eller ønsker å diskutere? Beskriv det gjerne her eller kommenter koden det gjelder.
Nei

✅ Checklist

Har du husket alle punktene i listen?

  • Jeg har testet mine endringer i henhold til akseptansekriteriene 🕵️
  • Jeg har config- eller sql-endringer. I så fall, husk manuell deploy til miljø for å verifisere endringene.
  • Jeg har skrevet tester. Hvis du ikke har skrevet tester, beskriv hvorfor under 👇

Jeg har ikke skrevet tester fordi:
Finnes eksisterende tester, jeg bare sletter kode

💬 Ønsker du en muntlig gjennomgang?

  • Ja
  • Nei

@@ -637,6 +637,5 @@ fun ISanityBegrunnelse.erGjeldendeForBrevPeriodeType(
vedtaksperiode.fom,
erUtbetalingEllerDeltBostedIPeriode,
)
return this.periodeType == brevPeriodeType || this.periodeType == BrevPeriodeType.IKKE_RELEVANT ||
(this.periodeType == BrevPeriodeType.FORTSATT_INNVILGET && brevPeriodeType == BrevPeriodeType.FORTSATT_INNVILGET_NY)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Denne sjekken på fortsatt innvilget tolker jeg at kun var der fordi:

  • Brevperiodetypen som kommer fra sanity har verdi FORTSATT_INNVILGET
  • Vi har laget en ny brevperiodetype for fortsatt innvilget som har postfix _NY. For at disse skal verifisere som samme verdi må man være eksplisitt da this.periodeType == brevPeriodeType ikke fanger opp dette

@idaame idaame marked this pull request as ready for review April 19, 2024 13:44
@idaame idaame requested a review from a team as a code owner April 19, 2024 13:44
Copy link
Contributor

@halvorbmundal halvorbmundal left a comment

Choose a reason for hiding this comment

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

Ser veldig bra ut! Digg å få slettet kode 🚀

@idaame idaame added this pull request to the merge queue Apr 19, 2024
Merged via the queue into main with commit 67e7cc6 Apr 19, 2024
11 checks passed
@idaame idaame deleted the chore/koderydding-del4 branch April 19, 2024 14:11
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