-
Notifications
You must be signed in to change notification settings - Fork 1
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
IS-3142: Add consumer for updating oppfolgingsenhet changes #527
IS-3142: Add consumer for updating oppfolgingsenhet changes #527
Conversation
personBehandlendeEnhetService.updateBehandlendeEnhet( | ||
PersonIdent(record.value().personident) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ingrid og jeg begynte på denne oppgaven, men ser jeg glemte å pushe det opp etter at vi begynte med feilsøking av varslene forrige uke. Vi tenkte å bruke akkurat samme funksjon i servicen, men også å sende med tildeltEnhet
-parameteret basert på hva som lå inne i databasen på den personen fra før. Det er ikke sikkert den siste biten er vits, egentlig.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tror det er lurt! Ellers kan det bli sånn at man nuller tildeltVeileder selv om det egentlig ikke er noen endring i tildeltEnhet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Litt rart at vi sender inn tildeltEnhet
her 🤔 Kanskje vi heller burde hentet eksisterende tildelt enhet i updateBehandlendeEnhet
funksjonen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ja, det hadde antakelig vært en bedre løsning.
src/test/kotlin/no/nav/syfo/personstatus/application/PersonBehandlendeEnhetServiceSpek.kt
Show resolved
Hide resolved
Litt urelatert til kodeendringene, men ser til at det er feil referanse til lapp i trello. |
override val pollDurationInMillis: Long = 1000 | ||
|
||
override suspend fun pollAndProcessRecords(consumer: KafkaConsumer<String, BehandlendeEnhetUpdateRecord>) { | ||
val records = consumer.poll(Duration.ofMillis(pollDurationInMillis)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Som i de andre consumer'ene er det nok lurt å filtrere bort records med value == null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gjøres i processRecords
funksjonen 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er du sikker på det? requireNoNulls()
filtrerer bort de som er null, men ikke records med value == null
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha! Skal dobbelsjekke. Var slik det var satt opp i en annen consumer nemlig, men trenger jo ikke å være riktig for det 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Endret nå! Bra sett 🕵️♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, da bør vi sikkert ta en gjennomgang på der vi bruker requireNoNulls()
. Det har antagelig ikke skjedd at record.value()
er null
fordi da tror jeg vi skulle fått en nullpointer 🤔
Woops, fikser i commitmelding 😊 |
89523a7
to
b840485
Compare
callId = UUID.randomUUID().toString(), | ||
personIdent = personIdent, | ||
) | ||
if (maybeNewBehandlendeEnhet != null && maybeNewBehandlendeEnhet.enhetId != tildeltEnhet) { | ||
val tildeltEnhet = personoversiktStatusRepository.getPersonOversiktStatus(personIdent)?.enhet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flyttet henting av eksisterende tildelt enhet inn her. Tanker om det? 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fint!
b840485
to
0168654
Compare
fun start(applicationState: ApplicationState, kafkaEnvironment: KafkaEnvironment) { | ||
val consumerProperties = Properties().apply { | ||
putAll(kafkaAivenConsumerConfig(kafkaEnvironment = kafkaEnvironment)) | ||
this[ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG] = BehandlendeEnhetUpdateRecordDeserializer::class.java.canonicalName | ||
this[ConsumerConfig.AUTO_OFFSET_RESET_CONFIG] = "latest" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vil dette fungere? 🤔 Setter at vi consumer latest her. Inne i kafkaAivenConsumerConfig
er denne satt til "earliest"
, men regner med at denne overskriver(?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ja, dette blir riktig 👍🏼 Og viktig at det kom på plass 🙃
@@ -237,15 +237,10 @@ class PersonOversiktStatusRepository(private val database: DatabaseInterface) : | |||
} | |||
} | |||
|
|||
override fun getPersonerWithOppgaveAndOldEnhet(): List<Pair<PersonIdent, String?>> = | |||
override fun getPersonerWithOppgaveAndOldEnhet(): List<PersonIdent> = | |||
database.connection.use { connection -> | |||
connection.prepareStatement(GET_PERSONER_WITH_OPPGAVE_AND_OLD_ENHET).use { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kanskje best å endre query'et også? (trenger ikke lengre selecte tildelt_enhet)
0168654
to
bfd9163
Compare
Hva har blitt lagt til✨🌈
teamsykefravr.behandlendeenhet
topicet som det blir produsert til når oppfølgingsenhet blir endret isyfobehandlendeenhet
.Se tilhørende PR i
syfobehandlendeenhet