Altinn / app-frontend-react

Altinn application React frontend
BSD 3-Clause "New" or "Revised" License
16 stars 24 forks source link

Validate and warn about invalid application configuration #648

Open olemartinorg opened 1 year ago

olemartinorg commented 1 year ago

Description

This need has been thought of and discussed a lot before, and while we would like to produce warnings/errors for invalid configuration in Altinn Studio, rapid feedback on changes is needed, so having these warnings in app-frontend-react as well might lead to a better developer experience for those working on their app locally. The code should preferably be written in a way that makes it possible to extract the code to also show warnings in Studio.

Suggested validations

Relevant issue(s)

ivarne commented 1 year ago

Jobber med dette i app-lib-dotnet https://github.com/Altinn/app-lib-dotnet/compare/feature/backend-dynamics...ivarne:app-lib-dotnet:app-validator

olemartinorg commented 1 year ago

@RonnyB71 @mijohansen Dette ble såvidt nevnt i møtet i går, så jeg tenkte å pinge dere her. Jeg har laget et forslag med ting jeg ønsker at vi validerer i en app, og at vi produserer en liste med potensielle feilmeldinger/advarsler om en gitt app.

Som nevnt over er det allerede påbegynt en implementasjon av dette i app-lib-dotnet, men jeg vil påstå at app-frontend-react er riktig repo å ha dette overordnede verktøyet i. Vi har såvidt jeg har skjønt flere ulike behov/stadier av app-utvikling hvor vi ønsker denne formen for validering:

Jeg tenker det er viktig å ha kode som validerer layout lokal, det vil si i nærheten av hvor layout blir brukt. Hvis vi legger til en mulighet i layout via en ny feature i app-frontend er det mest naturlig at vi også skriver koden som validerer om denne blir brukt riktig her i dette repoet - og til enhver tid vil det jo være app-frontend som leder an i forventningen om hva som er "riktig" layout. Layout blir i langt mindre grad lest og brukt på backend, dermed tenker jeg det er unaturlig å legge valideringen av layout der. Samme med Studio - det verktøyet produserer layout, men konsumerer det ikke, så det hjelper lite å bygge valideringslogikk for layout et sted som ikke har god kjennskap til hvordan layout blir brukt.

Det er riktignok en god del konfigurasjon på backend også, og det er gjerne mest naturlig at nuget-pakkene selv sørger for validering for å sjekke om den konfigurasjonen er riktig i appen.

Mitt forslag:

bjosttveit commented 7 months ago

Dette punktet:

[ ] As long as app-frontend-react puts the repeating group indexes into the component ID, we should show warnings when component IDs could be misunderstood as having repeating group indexes (i.e. matching -)

Er vel på sett og vis løst av #1512 siden det ligger vel i regexen for id? Den gir i alle fall feil om at component-ider som summary-1 ikke matcher regexen. Nå er ikke den feilmeldingen spesielt brukervennlig da men.

Image

ivarne commented 7 months ago

Regex regelen treffer mye mer en den bør. Kombinasjonen av masse falske positiver og en melding jeg knapt forstår noen måneder etter at jeg var med å feilsøke problemet gjør at jeg er uenig i at den er løst.

bjosttveit commented 7 months ago

Vi kan eventuelt velge å suppresse den warningen hvis vi legger inn en "custom" validering av komponent-ider

ivarne commented 7 months ago

Hvis du får en feilmelding som bare treffer skjema med kollisjon på repeterende grupper, kan jo bare regex fjerneres fra json schema

olemartinorg commented 7 months ago

Regexen er kanskje ikke optimal, men det har ikke kommet forslag om en som er mer optimal heller. Uansett - jeg synes det gir verdi å vise en feil om dette i vscode også, så jeg synes regexen skal bestå uavhengig av hva vi lager av verktøy i frontend.

Synes forsåvidt også vi kan "fiffe opp" akkurat den meldingen slik at den gir mer informasjon om hva man faktisk må gjøre for å fikse problemet, så det punktet kan godt ligge uløst her fortsatt.

olemartinorg commented 4 months ago

Ref denne diskusjonen:

Jeg tenker vi kanskje burde bygge dette slik at det kan brukes som regler i eslint. Men i tillegg også et API som kan brukes av Studio og hentes inn som en egen pakke der, og da lokal støtte (gjerne via noe som eslint). Jeg ser for meg at vi kan eksportere en pakke fra app-frontend-react til npm, slik at vårt app-cli-verktøy også kan hente inn og kjøre eslint. Da kan vi også få automatisk verktøystøtte rett i editoren dersom man f.eks. bruker vscode - samt implementere automatiske fikser for problemer vi vet hvordan kan løses med kode - f.eks. å bytte ut $schema-stien i alle layout-filer og diverse med riktig sti på CDN.

eslint gir oss et mye kraftigere verktøy enn vi får med ren JsonSchema, og vi kan nok også laste inn andre filer (som applicationmetadata.json) for å gjøre mye dypere analyser av konfigurasjon enn det som da gjøres i dag.

Et neste alternativ er kanskje å lage en skikkelig "Altinn language server" som da også gir deg skikkelig god integrasjon i en editor, med forslag til refaktoreringer osv. Jeg begynte såvidt å se på det en gang i forbindelse med et stunt jeg tenkte å prøve meg på (i løpet av et litt kjedelig møte), men det var åpenbart ikke noe som stod på noen plan, så det ble ikke jobbet videre med den gang.