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

Legg til hjelpefunksjoner for tidslinje fra familie-felles #5073

Merged
merged 12 commits into from
Feb 14, 2025

Conversation

MagnusTonnessen
Copy link
Contributor

💰 Hva skal gjøres, og hvorfor?

Favro: NAV-23270

Vi ønsker å ta i bruk tidslinjebiblioteket fra familie-felles i ba-sak.
Starter migreringen med å importere biblioteket, og oversette hjelpefunksjonene fra det gamle tidlsinjebiblioteket til det nye.
Kopierer og oversetter testene av de gamle funksjonene, for å sjekke at funksjonaliteten er lik.

Sammenlikn gjerne nye og gamle funksjoner, for å sjekke at logikken er den samme.

@MagnusTonnessen MagnusTonnessen requested a review from a team as a code owner February 13, 2025 10:02
@MagnusTonnessen MagnusTonnessen force-pushed the tidslinje-bibliotek branch 2 times, most recently from 754ea50 to fb305b2 Compare February 13, 2025 10:53
* Extension-metode for å konvertere fra dag-basert tidslinje til måned-basert tidslinje
* mapper-funksjonen tar inn listen av alle dagverdiene i én måned, og returner verdien måneden skal ha
* Dagverdiene kommer i samme rekkefølge som dagene i måneden, og vil ha null-verdi hvis dagen ikke har en verdi
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Kunne vært fint om det kom frem i kommentaren at det er verdien til første dag i måneden som blir brukt?

import no.nav.familie.tidslinje.utvidelser.filtrer
import no.nav.familie.tidslinje.utvidelser.kombinerMed

fun <V> Tidslinje<V>.filtrerIkkeNull(filter: (V) -> Boolean): Tidslinje<V> = filtrer { it != null && filter(it) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Burde man ikke heller utvide den i felles? F.eks noe ala
fun Tidslinje.filtrerIkkeNull(predicate: (T?) -> Boolean): Tidslinje = filtrerIkkeNull().filtrer(predicate)

Copy link
Contributor

@fredrikpf fredrikpf left a comment

Choose a reason for hiding this comment

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

Dritbra jobba!!!! 🤩

@stigebil
Copy link
Contributor

Burde man flyttet en del av disse extensions-funksjonene til tidslinje felles?

@MagnusTonnessen
Copy link
Contributor Author

Burde man flyttet en del av disse extensions-funksjonene til tidslinje felles?

Tanken var å flytte noen av funksjonene over til felles etterhvert. Ettersom oppbyggningen av tidslinjer i de to bibliotekene er ganske forskjellig, er det ikke sikkert vi trenger alle funksjonene når vi er ferdige å ta i bruk det nye tidslinjebiblioteket

@MagnusTonnessen MagnusTonnessen added this pull request to the merge queue Feb 14, 2025
Merged via the queue into main with commit abfb509 Feb 14, 2025
8 checks passed
@MagnusTonnessen MagnusTonnessen deleted the tidslinje-bibliotek branch February 14, 2025 10:33
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.

3 participants