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

pdfconfig #1203

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

pdfconfig #1203

wants to merge 6 commits into from

Conversation

marcuasc
Copy link
Contributor

@marcuasc marcuasc commented Jan 7, 2025

Mulighet til å sende pdfConfig til familie pdf.
Hvis man ikke sender inn en pdfConfig i feltmap, vil defaulverdiene i familipdf være:

  • pdfen har innholdsfortegnelse
  • språk i metadataen er satt til norsk

Brukes for å teste familiepdf branch i preprod

@marcuasc marcuasc requested a review from a team as a code owner January 7, 2025 09:57
Copy link
Contributor

@SandraLekve SandraLekve left a comment

Choose a reason for hiding this comment

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

Nice! tror ikke det burde være nødvendig å endre felter deres løsning også ☺️

Copy link
Contributor

@fredrikmork fredrikmork left a comment

Choose a reason for hiding this comment

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

Jeg godkjenner hvis @ViktorGSolberg godkjenner 💯

@@ -48,7 +50,7 @@ object SøknadTilFeltMap {
): FeltMap {
val finnFelter = finnFelter(søknad)
val vedlegg = mapTilVedlegg(vedleggTitler)
return FeltMap("Søknad om overgangsstønad (NAV 15-00.01)", finnFelter + vedlegg)
return FeltMap("Søknad om overgangsstønad (NAV 15-00.01)", finnFelter + vedlegg, PdfConfig(harInnholdsfortegnelse = false, språk = Språk.NB))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return FeltMap("Søknad om overgangsstønad (NAV 15-00.01)", finnFelter + vedlegg, PdfConfig(harInnholdsfortegnelse = false, språk = Språk.NB))
return FeltMap("Søknad om overgangsstønad (NAV 15-00.01)", finnFelter + vedlegg)

@@ -19,5 +19,6 @@
"label" : "Vedlegg",
"verdi" : "Lærlingkontrakt\n\nUtgifter til pass av barn"
} ]
} ]
} ],
Copy link
Contributor

Choose a reason for hiding this comment

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

Sjekk om det fungerer med dagens løsning

@@ -8,6 +8,7 @@ data class FeltMap(
val label: String,
@field:NotNull(message = "Verdiliste kan ikke være null")
val verdiliste: List<VerdilisteElement>,
val pdfConfig: PdfConfig? = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Tror du også kan:
Sette en defaultverdi, istedet for å la pdfConfig være null, kan du sette en standardverdi som opprettes automatisk når parameteren ikke er spesifisert.
Fordeler:Ingen risiko for NullPointerException.
val pdfConfig: PdfConfig = PdfConfig() // Standardverdi som ikke tillater null

Da skal du heller ikke trenge så sende den med i deres testfiler ("gammel flyt test filer")

Copy link
Contributor

Choose a reason for hiding this comment

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

Er det noen grunner til at PdfConfig er nullable i utganspunktet? Er det ikke slik at man alltid vet om man ønsker innholdsfortegnelse eller ikke og om spårket skal være på norsk eller engelsk?

Copy link
Contributor Author

@marcuasc marcuasc Jan 10, 2025

Choose a reason for hiding this comment

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

Nå kan ikke jeg kotlin så godt, men i typescript ville jeg kanskje hatt en optional type. pdfConfig? : PdfConfig. Om den skal være null eller ikke vet jeg ikke hva som er best practise, det håndteres uansett i familie-pdf.

Burde det heller være sånn her med standard verdi?
val pdfConfig: PdfConfig = PdfConfig(harInnholdsfortegnelse = false, språk = Språk.NB)

Hvis man ikke sender inn pdfconfig sånn som det er nå, er defaultverdiene i familie pdf, harInnholdsfortegnelse = false, språk = Språk.NB.
@ViktorGSolberg

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