-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/ny sykemelding frontend #3957
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
Conversation
…syfosmregler and tsm-input-dolly - Changed the setup of the proxy in minor ways #deploy-proxy-sykemelding
…; update tests and configuration files accordingly. #deploy-proxy-sykemelding
…ute. #deploy-proxy-sykemelding #deploy-sykemelding-api #deploy-test-sykemelding-api
#deploy-proxy-sykemelding
…melding commands and configurations.
…ort for new TSM Sykemelding processing and configurations. Fix mapping strategies for consistency. #deploy-test-dolly-backend
…kemeldingClient. Refactor TsmSykemeldingConsumer to support delete operations. #deploy-test-dolly-backend
#deploy-test-dolly-backend
#deploy-proxy-sykemelding
…g files #deploy-test-dolly-backend
…refixes. #deploy-test-dolly-backend
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.
Startet så vidt et review ...
...ain/java/no/nav/dolly/bestilling/sykemelding/command/SyfosmreglerSykemeldingPostCommand.java
Outdated
Show resolved
Hide resolved
...d/src/main/java/no/nav/dolly/bestilling/sykemelding/command/TsmSykemeldingDeleteCommand.java
Outdated
Show resolved
Hide resolved
...end/src/main/java/no/nav/dolly/bestilling/sykemelding/command/TsmSykemeldingPostCommand.java
Outdated
Show resolved
Hide resolved
...-backend/src/main/java/no/nav/dolly/bestilling/sykemelding/domain/TsmSykemeldingRequest.java
Outdated
Show resolved
Hide resolved
...lly-backend/src/main/java/no/nav/dolly/bestilling/sykemelding/dto/NySykemeldingResponse.java
Outdated
Show resolved
Hide resolved
… `NySykemelding`, update DTO naming conventions, and simplify related methods. #deploy-test-dolly-backend
…gration. - Implement new attributes and validation logic for "Ny Sykemelding". - Refactor existing Sykemelding logic to account for "Ny Sykemelding". - Add dynamic proxy configuration for `testnav-sykemelding-proxy`. - Update SWR hooks and components to handle new sykemelding-related data. - Modify environment-specific configuration files. #deploy-test-frontend
…gration. - Implement new attributes and validation logic for "Ny Sykemelding". - Refactor existing Sykemelding logic to account for "Ny Sykemelding". - Add dynamic proxy configuration for `testnav-sykemelding-proxy`. - Update SWR hooks and components to handle new sykemelding-related data. - Modify environment-specific configuration files. #deploy-test-frontend
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.
Pull Request Overview
This PR introduces a new "ny sykemelding" (new sick leave) frontend feature, adding a simplified sykemelding interface alongside the existing detailed sykemelding functionality.
- Adds a new
nySykemelding
form option with activity periods instead of detailed medical information - Integrates with a new TSM (Team Sykmelding) backend service via proxy
- Updates the sykemelding proxy to support routing to both existing syfosmregler and new tsm-input-dolly services
Reviewed Changes
Copilot reviewed 52 out of 55 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
proxies/sykemelding-proxy/src/main/java/no/nav/testnav/proxies/sykemeldingproxy/RouteLocatorConfig.java | Updates proxy routing to handle both syfosmregler and TSM services with path-based routing |
apps/dolly-frontend/src/main/js/src/components/fagsystem/sykdom/form/Form.tsx | Adds support for new sykemelding form alongside existing detailed form |
apps/dolly-frontend/src/main/js/src/components/fagsystem/sykdom/form/partials/NySykemelding.tsx | Implements the new simplified sykemelding form with activity periods |
apps/dolly-backend/src/main/java/no/nav/dolly/bestilling/sykemelding/SykemeldingClient.java | Adds backend integration for new sykemelding service |
apps/dolly-frontend/src/main/js/src/components/fagsystem/sykdom/visning/Visning.tsx | Updates display logic to show new sykemelding data when available |
Files not reviewed (1)
- apps/dolly-frontend/src/main/js/package-lock.json: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
fromDate: minDate, | ||
toDate: maxDate, | ||
disabled: excludeDates, | ||
defaultSelected: convertInputToDate(existingValue)?.toDate?.() || new Date(), |
Copilot
AI
Sep 25, 2025
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.
The defaultSelected
property is being set to a default value of new Date()
when existingValue
is undefined or conversion fails. This could cause the datepicker to always show today's date as selected by default, which may not be the intended behavior. Consider using undefined
or null
instead to allow the datepicker to have no default selection.
defaultSelected: convertInputToDate(existingValue)?.toDate?.() || new Date(), | |
defaultSelected: convertInputToDate(existingValue)?.toDate?.() ?? undefined, |
Copilot uses AI. Check for mistakes.
header, | ||
newEntry, | ||
hjelpetekst = null, | ||
hjelpetekst = null as unknown as string, |
Copilot
AI
Sep 25, 2025
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.
Using as unknown as string
type assertion is unsafe. Consider using proper optional typing like hjelpetekst?: string | null
in the interface definition instead of forcing type conversion.
hjelpetekst = null as unknown as string, | |
hjelpetekst = null, |
Copilot uses AI. Check for mistakes.
tag = null, | ||
isOrganisasjon = false, | ||
handleNewEntry = null, | ||
handleNewEntry = null as any, |
Copilot
AI
Sep 25, 2025
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.
Using as any
defeats the purpose of TypeScript's type checking. Consider defining a proper type for handleNewEntry
such as (() => void) | null
or making it optional with proper typing.
handleNewEntry = null as any, | |
handleNewEntry = null as (() => void) | null, |
Copilot uses AI. Check for mistakes.
|
||
const mergeData = () => { | ||
const mergeMiljo = [] | ||
const mergeMiljo: Array<{ data: any[]; miljo: string }> = [] |
Copilot
AI
Sep 25, 2025
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.
The any[]
type for data
property reduces type safety. Consider defining a proper interface or type for the data structure to improve maintainability and catch potential runtime errors.
Copilot uses AI. Check for mistakes.
forsteMiljo={forsteMiljo} | ||
data={filteredData ?? mergetData} | ||
> | ||
<Visning data={filteredData} /> |
Copilot
AI
Sep 25, 2025
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.
The Visning
component is being passed filteredData
but the component expects data
to be from mergetData
when filteredData
is null/undefined. This could cause the component to receive unexpected data structure. Should use filteredData ?? mergetData
instead.
<Visning data={filteredData} /> | |
<Visning data={filteredData ?? mergetData} /> |
Copilot uses AI. Check for mistakes.
import no.nav.dolly.service.TransactionHelperService; | ||
import no.nav.dolly.service.TransaksjonMappingService; |
Copilot
AI
Sep 25, 2025
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.
[nitpick] Import statements are not in alphabetical order. Consider placing imports in consistent alphabetical order for better code organization.
import no.nav.dolly.service.TransactionHelperService; | |
import no.nav.dolly.service.TransaksjonMappingService; | |
import no.nav.dolly.service.TransaksjonMappingService; | |
import no.nav.dolly.service.TransactionHelperService; |
Copilot uses AI. Check for mistakes.
.retrieve() | ||
.bodyToMono(NySykemeldingResponseDTO.class) | ||
.doOnError(WebClientError.logTo(log)) | ||
.onErrorResume(error -> Mono.just(new NySykemeldingResponseDTO(error.getMessage(), "NA", null, request.getIdent()))); |
Copilot
AI
Sep 25, 2025
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.
Using 'NA' as a hardcoded sykmeldingId in error cases is unclear. Consider using a more descriptive value like 'ERROR' or null, or define a constant for this default value.
Copilot uses AI. Check for mistakes.
# Conflicts: # apps/dolly-frontend/src/main/java/no/nav/dolly/web/DollyFrontendApplicationStarter.java # apps/dolly-frontend/src/main/java/no/nav/dolly/web/config/Consumers.java
…th `undefined` instead of `new Date()` for improved input handling. #deploy-test-frontend
#deploy-test-frontend
…component. #deploy-test-frontend
#deploy-test-frontend
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.
Lagt til noen kommentarer 👍
apps/dolly-backend/src/main/java/no/nav/dolly/bestilling/sykemelding/SykemeldingClient.java
Outdated
Show resolved
Hide resolved
...nd/src/main/java/no/nav/dolly/bestilling/sykemelding/command/NySykemeldingDeleteCommand.java
Show resolved
Hide resolved
...kend/src/main/java/no/nav/dolly/bestilling/sykemelding/command/NySykemeldingPostCommand.java
Outdated
Show resolved
Hide resolved
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.
Dette ble... sykt bra! 🤩
…ture, and enhance response handling in NySykemelding classes. #deploy-test-dolly-backend
No description provided.