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

Trans for søknadsbehandling #661

Merged
merged 3 commits into from
Jan 27, 2022
Merged

Trans for søknadsbehandling #661

merged 3 commits into from
Jan 27, 2022

Conversation

simejo
Copy link
Contributor

@simejo simejo commented Jan 26, 2022

Transaksjon i søknadsbehandling.

@simejo simejo requested a review from a team January 26, 2022 13:05
@@ -113,6 +115,30 @@ internal class UtbetalingServiceImpl(
}
}

override fun genererUtbetalingsRequest(
Copy link
Contributor

Choose a reason for hiding this comment

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

Kunne ikke denne ha vært privat? også kan det være lagreUtbetaling og publiserUtbetaling sitt ansvar å kalle denne?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Problemet er at vi trenger resultatet (mer bestemt den genererte id-en) for å kunne lagre vedtak. Men kan sjekke opp om jeg får vridd det litt

verify(utbetalingPublisherMock).publish(any())
verify(utbetalingRepoMock).opprettUtbetaling(any())
}
verify(sakServiceMock, times(2)).hentSak(
Copy link
Contributor

Choose a reason for hiding this comment

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

times(2) 😢 (burde hatt en todo på å gjøre om denne til at den kun henter sak en gang også endrer saken via modellen og ikke bare via basen)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ja, helt enig. Kan se om jeg får gjort det i en egen commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

simulering = argThat { it shouldBe simulering },
uføregrunnlag = argThat { it shouldBe emptyList() },
)
serviceAndMocks.verifyNoMoreInteractions()
}

@Test
fun `svarer med feil dersom vi ikke kunne utbetale`() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Uten å lese implementasjonen så sier navnet på denne testen meg at man egentlig ikke trenger verify, den trenger bare å sjekke resultatet (og det ser ut som det er flere tester i samme situasjon).

Men kan jo evt. være (som du nevnte på Slack) en test i tillegg som sjekker at X ikke skjer i feil-situasjonene.

@simejo simejo merged commit 15d3d3e into master Jan 27, 2022
@simejo simejo deleted the trans-for-søknadsbehandling branch January 27, 2022 11:14
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