Skip to content

oppdater aksjonspunktdto #7456

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

oppdater aksjonspunktdto #7456

wants to merge 1 commit into from

Conversation

johannbm
Copy link
Contributor

No description provided.

@johannbm johannbm requested a review from a team as a code owner July 29, 2025 08:48
Copy link

Copy link
Collaborator

@jolarsen jolarsen left a comment

Choose a reason for hiding this comment

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

Synes vi skal ta praten i teamet om hvor vi vil gå her, ellers blir det mange annoteringer som ikke vedlikeholdes med mindre alle er med på tanken.
Så kan vi se på hva Jostein i K9 har gjort - de genererer frontend-kode uten disse annteringene backend.
Selv om din navnebror Brodwall brenner for kodegenerering, så har jeg vært lunken siden starten av arbeidslivet etter møtet med RPCgen (og en del historier).
Vi har pt holdt oss til NotNull, Size og Pattern for Dtos fra frontend til backend, mens denne er fra backend til frontend.

Det vi imidlertid bør gjøre er å gå gjennom Dto for Dto front/backend: Forenkle (Søknad, Famhendelse), fjerne ubrukt, få på riktig Validation, fornorske, fjerne unødvendig Json-annoteringer. Dessuten skille frontend og formidling....
Mao en aktivitet.

@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private Boolean toTrinnsBehandlingGodkjent;
@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private Set<VurderÅrsak> vurderPaNyttArsaker;
@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private String besluttersBegrunnelse;
@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private AksjonspunktType aksjonspunktType;
Copy link
Collaborator

Choose a reason for hiding this comment

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

aksjonspunktType er @NotNull - men om den brukes frontend er en annen sak.

@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private Set<VurderÅrsak> vurderPaNyttArsaker;
@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private String besluttersBegrunnelse;
@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private AksjonspunktType aksjonspunktType;
@NotNull private Boolean kanLoses;
Copy link
Collaborator

Choose a reason for hiding this comment

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

boolean ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Var slik. Kun lagt på annotasjoner

@NotNull private Boolean erAktivt;
@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private LocalDateTime fristTid;
@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private LocalDateTime endretTidspunkt;
@Schema(nullable = true, requiredMode = Schema.RequiredMode.REQUIRED) private String endretAv;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hvorfor alle REQUIRED? Kan man ikke ha en default for alt / klassen og markere der det er avvik.
Går på lesbarhet

@johannbm
Copy link
Contributor Author

johannbm commented Jul 29, 2025

Yes, tanken her er jo å få igang diskusjon før det ev. gjøres over hele.

Hvorfor alle REQUIRED? Kan man ikke ha en default for alt / klassen og markere der det er avvik.
Går på lesbarhet

Hvis det gjelder for alle DTOer kan vi sikkert sette globalt et sted at alt alltid er required. Da blir det mye penere ved at @NotNull er eneste annotasjon som trengs

aksjonspunktType er NotNull - men om den brukes frontend er en annen sak.

Tenker det er en annen fordel ved å annotere typene, da viser man tydelig hva man tenker kan være null og ikke. Nå når jeg skal skrive TS typer må jeg tolke backend koden for å utlede om noe er null eller ikke.

Så kan vi se på hva Jostein i K9 har gjort - de genererer frontend-kode uten disse annteringene backend.
Selv om din navnebror Brodwall brenner for kodegenerering, så har jeg vært lunken siden starten av arbeidslivet etter møtet med RPCgen (og en del historier).

Har snakket litt med Jostein, og han har delt for oss i frontend-på-tvers oppsettet deres. Når vi på frontend har diskutert dette har vi landet på at vi ikke vil gå like langt som dem - de generer fulle kodeklienter, mens vi vil kun generere typer. Altså generer vi ingen kode. Derfor ser jeg ingen nedside (annet enn tid og effort) av å generere typer. Det vil gi en løs kontrakt mellom frontend og backend, og man kan enkelt generere typer fra en backend PR for å se om det vil være breaking change for frontend eller ikke.

@jolarsen
Copy link
Collaborator

Tenker det er en annen fordel ved å annotere typene, da viser man tydelig hva man tenker kan være null og ikke. Nå når jeg skal skrive TS typer må jeg tolke backend koden for å utlede om noe er null eller ikke.

Enig i at NotNull er nyttig for lesbarheten, selv om de ikke valideres (som i jakarta.Validation).
Men det er litt redundant for boolean, int, long - de må alle ha en verdi, mens Boolean, Integer, Long kan være null
Bør også se på Collecction-constraints - Size med min=0/1 (evt NonEmpty dersom finnes(
Ser ikke at vi innfører Pattern for String overalt - kun der det skal lagres i fpsak - eller tallverdi-valideringer

Har snakket litt med Jostein, og han har delt for oss i frontend-på-tvers oppsettet deres. Når vi på frontend har diskutert dette har vi landet på at vi ikke vil gå like langt som dem - de generer fulle kodeklienter, mens vi vil kun generere typer. Altså generer vi ingen kode. Derfor ser jeg ingen nedside (annet enn tid og effort) av å generere typer. Det vil gi en løs kontrakt mellom frontend og backend, og man kan enkelt generere typer fra en backend PR for å se om det vil være breaking change for frontend eller ikke.

Bra. Men vi trenger få med hele den tradisjonelle backend-gjengen så vi får felles kjøreregler.
Dessuten gå i retning fullstack - så vi gjør expand/contract ved endringer front/backend og rydder løpende.

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.

2 participants