search

VNG-Realisatie / klantinteracties

Project repository tbv de ontwikkeling van de Klantinteracties API specificatie
https://vng-realisatie.github.io/klantinteracties/
1 stars 4 forks source link

Als consumer (project chatbot) wil ik de klantinteractie/contactmoment in markdown kunnen vastleggen en dat de API specificatie / documentatie beschrijft dat het attribuut tekst ook markdown ondersteunt zodat behandel componenten/applicaties hier rekening mee kunnen houden in de weergave #74

Closed harveyvandermeer closed 1 year ago

harveyvandermeer commented 4 years ago

...zodat het project chatbot contact tussen chatbot en inwoner in een dialoog opgemaakt kan opslaan in de klantinteractie en dat wij deze API kunnen gaan benutten voor het opslaan van klantcontacten ipv dialoog ...met aandacht voor dat dit aanvullend moet zijn en dat wij open staan voor alternatieve ideeën om dit vorm te geven.

Hugo-ter-Doest commented 4 years ago

Markdown is slechts een van de formaten die hiervoor zinvol zijn. HTML en RTF zijn ook voorbeelden. Zal dus generiek moeten worden ingericht zodat later andere formaten kunnen worden ondersteund.

sergei-maertens commented 4 years ago

In het begin van dit traject had ik ook een dergelijke vraag gesteld - ondersteuning van tekst-based formaten zoals markdown. Dat was toen "niet nodig" bevonden, maar ik ben er nog steeds voorstander van. Lieve wat rich-text met markdown dan dat je een word-document als informatieobject moet toevoegen. Dus als de oplossing kan doorgezet worden naar de toelichting/omschrijving velden bij zaken etc., dan zou dat mooi zijn.

harveyvandermeer commented 4 years ago

@Hugo-ter-Doest: Fijn dat dit serieus opgepakt wordt. Toch nog even mijn input; Markdown is wel meer een tussenformaat wat aanvullend is op platte tekst. Vanuit hier kun je verschillende formaten genereren. Markdown is leesbaar, is gelimiteerd en je kunt hier vanuit HTML genereren en eventueel ook RTF. Het generiek maken en daarmee verruimen van het aantal ondersteunende formaten lijkt mij een risico in de afstemming tussen consumers van de specificatie. Bij formaten als HTML moet je je gaan afvragen, welke features ga je ondersteunen, voor je het weet wordt het complex, Bij Markdown loop je dat risico niet, als consumers dit niet ondersteunen is dit nog steeds leesbaar. Keep It Smart & Simple.

michielverhoef commented 4 years ago

Wat Hugo volgens mij aangeeft is dat de oplossing zo moet worden ingericht dat later, indien nodig, andere formaten ook mogelijk zijn.

Mijn vraag hierbij is: Gaat deze userstory om het aanpassen van de beschrijving van het attribuut tekst of moet er ook echt iets aangepast worden in de validatie? Want uit de OAS kan ik niet halen dat het nu technisch niet mogelijk is om markdown of een ander tekstgebaseerd formaat aan te leveren.

sergei-maertens commented 4 years ago

Je kan nu wel markdown gebruiken, maar een andere consumer weet niet dat de tekst markdown is. Die zal het dan waarschijnlijk gewoon plat weergeven. Nu is dat met markdown geen probleem, maar het wordt wel een probleem als je andere formaten opneemt.

Stel dat consumer X markdown opvoert, en consumer Y gebruikt platte tekst, dan zal consumer X proberen markdown te renderen van consumer Y als dat weergegeven moet worden. Platte tekst valt nog mee, maar met HTML, RST... wordt het wel foute boel.

Je zal een mechanisme nodig hebben om te kunnen aangeven welk formaat een attribuut heeft, met een lijst van toegelaten formaten, zodat consumers niet moeten gaan gissen.

michielverhoef commented 4 years ago

Dan je een soort content-type attribuut nodig denk ik zo. Dan kan ik me voorstellen dat daar een enumeratie achter hangt en dan moet je wel vantevoren goed nadenken over de formaten die terug kunnen komen anders wordt het totale chaos inderdaad.

hdksi commented 2 years ago

Voorstel: volgen suggestie Michiel. Enumeratie formaat toevoegen waarmee kan worden aangegeven dat 'tekst' in markdown. Rekening houden met toevoegen van andere formaten in de toekomst.

hdksi commented 2 years ago

Kijken bij vormgeving bijgewerkte API-spec naar oplossing tabellen bij Haal Centraal.

HenriKorver commented 2 years ago

Kijken bij vormgeving bijgewerkte API-spec naar oplossing tabellen bij Haal Centraal.

@hdksi Wat bedoel je hiermee?

hdksi commented 2 years ago

@melsk-r deed die suggestie de vorige keer bij het bespreken van de oplossing voor het vastleggen van inhoud in verschillende formaten. Wellicht heeft hij een link naar een toepassing als voorbeeld?

HenriKorver commented 2 years ago

Bij deze aanpassing op de OAS zodat de toelichting op het contactmoment met de klant zowel in plain text als MarkDown formaat kan worden opgemaakt: https://github.com/HenriKorver/gemma-zaken/commit/627eb1dd3ba214f19a4bc267c471ccd0bd661834

HenriKorver commented 2 years ago

Dit is de link naar de volledige OAS. Hierbij een voorbeeld van het nieuwe tekst-veld opgemaakt in MarkDown:

{
  "vorigContactmoment": "http://example.com",
  "bronorganisatie": "string",
  "registratiedatum": "2019-08-24T14:15:22Z",
  "kanaal": "string",
  "voorkeurskanaal": "string",
  "voorkeurstaal": "str",
  "tekst": {
    "inhoud": "Tekst die in dit geval in **MarkDown** formaat is opgemaakt.",
    "formaat": "markdown"
  },
  "onderwerpLinks": [
    "http://example.com"
  ],
  "initiatiefnemer": "gemeente",
  "medewerker": "http://example.com",
  "medewerkerIdentificatie": {
    "identificatie": "string",
    "achternaam": "string",
    "voorletters": "string",
    "voorvoegselAchternaam": "string"
  }
}
harveyvandermeer commented 2 years ago

Dit voldoet prima! Ben erg benieuwd wanneer dit bij een van de gemeenten in gebruik genomen gaat worden.

sergei-maertens commented 2 years ago

Kunnen we dan ook niet gewoon internationale standaarden gebruiken en hiervoor media/content/mime types gebruiken in plaats van het wiel opnieuw uitvinden? https://datatracker.ietf.org/doc/html/rfc7763 lijkt hiervoor te voorzien in text/markdown als media type. Merk op dat ook de charset in principe nodig is, waarbij ik utf-8 zou adviseren wegens lekker compatibel met JSON.

harveyvandermeer commented 2 years ago

Prima overweging lijkt mij, eerder ook aangegeven door @michielverhoef om mime/content types te gebruiken. Is het niet zo dat charset op deze plek in het bericht overbodig is?

Dit omdat de encoding/charset al onderdeel is van de headers van het api bericht zelf. Charset binnen een charset kan alleen maar verwarrend werken lijkt mij. Hoe e.e.a. vervolgens opgeslagen wordt en met welke encoding in de database is aan de implementatie van de producer en database lijkt mij.

michielverhoef commented 2 years ago

Standaardiseren ben ik helemaalvoor maar waarom haken we dan niet aan bij het attribuut "formaat" uit RGBZ? https://www.gemmaonline.nl/index.php/Rgbz_2.0/doc/attribuutsoort/enkelvoudig_informatieobject.formaat

Formaat moet gevuld worden met waarden uit de IANA lijst: https://www.iana.org/assignments/media-types/media-types.xhtml

Dat is meteen het formaat uit de Documenten API en dan zijn we flexibel wanneer eventueel een volgend formaat gewenst is. NB in de IANA lijst staat markdown ook opgenomen als text/markdown (inclusief verwijzing naar RFC7763 ).

sergei-maertens commented 2 years ago

@harveyvandermeer

Is het niet zo dat charset op deze plek in het bericht overbodig is?

mja, daar heeft een RFC niet veel oren naar als die zegt dat het required is. Het punt van internationale standaarden en RFCs is nu net dat je geen dialecten erop na gaat houden zodat het goed uitwisselbaar blijft :) Ik snap de motivatie, maar mixed encodings in 1 enkel bericht zijn wel een ding. Niet zo courant in JSON-land, maar ook daar zie je bijvoorbeeld base64 encoding voor bestanden in een utf-8 encoded JSON payload.

Ik speel het persoonlijk dan liever veilig en volg IANA + RFC7763 dan dat je er een eigen draai aan geeft.

@michielverhoef

Standaardiseren ben ik helemaalvoor maar waarom haken we dan niet aan bij het attribuut "formaat" uit RGBZ? https://www.gemmaonline.nl/index.php/Rgbz_2.0/doc/attribuutsoort/enkelvoudig_informatieobject.formaat

Even vluchtig bekeken en RFC7763 en formaat met IANA-verwijzing sluiten op mekaar aan. Ik was even bang dat RGBZ een andere, "eigen" standaard voorschreef, maar dat is niet het geval. Dus +1 op dit voorstel, en consistentie met de Documenten API is mooi meegenomen!

harveyvandermeer commented 2 years ago

@sergei-maertens terecht punt, ik zie ook geen bezwaar om het expliciet op te nemen. Voor text gerelateerde mimetypes (text/*) zou echter vreemd zijn om een andere charset er op na te houden dan die van het api bericht zelf, dat is vragen om problemen bij implementatie van zowel consumer als de service. Uit eigen ervaring verwacht ik dat daar veel markt standaard toepassingen (bijv. soapui / postman) niet goed mee om gaan.

sergei-maertens commented 2 years ago

Begrijpelijk - nu doet Postman volgens mij sowieso niets met de inhoudelijke response body behalve dus renderen op basis van de Content-Type en Content-Encoding HTTP response headers.

In de praktijk gaan die inderdaad wel op mekaar afgestemd zijn - de wereld heeft ondertussen grotendeels door dat utf8 op dit moment het meest betrouwbaar is :stuck_out_tongue:

michielverhoef commented 2 years ago

Ik weet niet wat de discussie is geweest voordat dit issue opgepakt is maar voor er nu een soort Documenten API lite gaat ontstaan, is dit de goede oplossingsrichting? Ik snap ook wel dat je niet voor alles het documenten register wilt gebruiken maar het is ook niet handig om twee (of zelfs meer) soorten opslag voor rich text te hebben.

Zeker wanneer het in de toekomst mogeljik gaat zijn dat er nog meer verschillende formaten mogelijk gaan worden zou ik niet apart een mini versie van het informatieobject gaan verzinnen. Dan is een gewoon tekstveld voor platte tekst in de Contactmomenten API prima en bij rich text opslag in de Documenten API veel eenvoudiger.

sergei-maertens commented 2 years ago

dan begreep ik je verkeerd. Documenten API is toch een heel ander beestje dan gewoon inhoud wat intrinsiek hoort bij een API resource. Je hebt dan weer een informatieobjecttype nodig, en andere verplichte velden waar allerlei businessrules bij gelden.

Ik dacht dat de consensus nu was om:

veldnaam:
  inhoud: lorem ipsum...
  formaat: <IANA-mime type>

aan te houden. De nuttige data blijft dan in de API resource, want dit is precies bedoeld voor snelle weergave en hoort intrinsiek erbij, en het blijft consistent met wat de Documenten API doet met het formaat veld cfr. RGBZ wat een IANA verwijzing bevat. Eventueel kan je per API specifiek die lijst beperken met een enum zodat je initieel enkel text/plain en text/markdown ondersteunt.

michielverhoef commented 2 years ago

Je begreep me goed, alleen had ik last van voortschrijdend inzicht :-D

Waar het mij meer om gaat is dat het niet handig is om naast de Documenten API ook nog in andere API's iets dergelijks als een informatieobject lite te gaan maken. Voor erg beperkte varianten is dat prima maar het moet niet meer/groter worden.

Dus eventuele attachments bij een contactmoment moeten wel in de Documenten API opgeslagen worden. Dat soort dingen.

HenriKorver commented 2 years ago

Dank voor jullie review en input! Ik heb de enumeratie aangepast conform IANA-waarden, zie https://github.com/HenriKorver/gemma-zaken/commit/e2fa123a00426e88b23172598ec8d358281102ad. Is het nu naar wens?

{
  "vorigContactmoment": "string",
  "bronorganisatie": "string",
  "registratiedatum": "2022-02-23T11:03:23.608Z",
  "kanaal": "string",
  "voorkeurskanaal": "string",
  "voorkeurstaal": "str",
  "tekst": {
    "inhoud": "Tekst die in **MarkDown** formaat is opgemaakt.",
    "formaat": "text/markdown"
  },
  "onderwerpLinks": [
    "string"
  ],
  "initiatiefnemer": "gemeente",
  "medewerker": "string",
  "medewerkerIdentificatie": {
    "identificatie": "string",
    "achternaam": "string",
    "voorletters": "string",
    "voorvoegselAchternaam": "string"
  }
}
HenriKorver commented 2 years ago

afbeelding

sergei-maertens commented 2 years ago

Je begreep me goed, alleen had ik last van voortschrijdend inzicht :-D

Waar het mij meer om gaat is dat het niet handig is om naast de Documenten API ook nog in andere API's iets dergelijks als een informatieobject lite te gaan maken. Voor erg beperkte varianten is dat prima maar het moet niet meer/groter worden.

Dus eventuele attachments bij een contactmoment moeten wel in de Documenten API opgeslagen worden. Dat soort dingen.

check, dan zitten we op dezelfde lijn!

@HenriKorver bijna, ik denk dat het cfr. RFC7763 (https://datatracker.ietf.org/doc/html/rfc7763#section-2) text/markdown; charset=utf-8 moet zijn. Ik heb de vrijheid genomen om utf-8 te nemen in de context van JSON API's die ook utf-8 zijn.

HenriKorver commented 2 years ago

Wegens design rule DR2.4 mag de enumeratie geen spaties en andere speciale tekens bevatten. Daarom heb ik het nu in de documentatie van het veld opgelost waarin wordt uitgelegd dat de waarde markdown in de enumeratie verwijst naar het IANA media type text/markdown; charset=UTF-8.

HenriKorver commented 2 years ago

afbeelding

michielverhoef commented 2 years ago

Ik begrijp je redenering maar vraag me af of deze ontwerpregel wel echt van toepassing is/zou moeten zijn. Nu wordt een impliciete betekenis toegekend aan de waarden en moeten de eigenlijke waarden vertaald worden. Wanneer dit niet of niet goed gedaan wordt heb je eigenlijk onbruikbare waarden in het register staan.

Ik weet wel waar het vandaan komt maar eigenlijk vind ik dit typisch iets waar je wel af mag (moet) wijken van een ontwerpregel domweg omdat de inhoud van de enumeratie nu niet overeenkomt met wat je zou willen opslaan. De achtergrond van deze ontwerpregel is ook heel anders, die is nooit bedacht om waarden waarin een "/" wel degelijk nodig (en onderdeeel van een grotere standaard) is op te nemen.

melsk-r commented 2 years ago

Bij de Haal Centraal BRP API specificatie hebben we een aantal enumerations vervangen door een verwijzing naar een referentietabel. Juist in die situaties waarin in de toekomst waarden toegevoegd moeten kunnen worden wat me hier ook van toepassing lijkt. Dit maakt de specificatie meer evolvable omdat je daarvoor geen nieuwe versie van de specificatie hoeft te publiceren. Of dit ook het probleem met de DR2.4 (code-generatie) oplost weet ik echter niet.

sergei-maertens commented 2 years ago

Dit maakt de specificatie meer evolvable omdat je daarvoor geen nieuwe versie van de specificatie hoeft te publiceren.

in dit geval is dit enorm developer-unfriendly. Er leven dan namelijk waarden die je wel moet supporten in een externe tabel, en die wijzingen zijn niet duidelijk of gepaard met een nieuwe versie van de API specificatie, precies die specificatie die je contract moet zijn tussen provider en consumer. :-1:

Het renderen van text/plain vs. text/markdown resulteert in verschillende codepaden en waarschijnlijk zelfs libraries die als dependencies moeten toegevoegd worden. Als er zonder een API spec change plots andere dingen bijkomen, dan gaan consumers daar sowieso op stuk gaan zodra die waarde gebruikt wordt.

HenriKorver commented 2 years ago

Eens met @sergei-maertens om niet af te wijken van het gebruik van een enumeration. @sergei-maertens Kun je ermee leven dat we in de enumeration korte namen gebruiken zonder speciale tekens en in de beschrijving verwijzen naar de IANA-standaard zoals boven aangegeven?

sergei-maertens commented 2 years ago

ik kan ermee leven, maar het maakt me niet vrolijker :P

ik denk dat rekening houden met code generation inderdaad developer-friendly is, maar ik ben ook van mening dat code generation niet meer is dan een startpunt en dat je hier en daar het resultaat van die code-generation als developer maar nog even moet finetunen. Uiteindelijk is het de code-generator zelf die buggy is en invalide code genereert uit een API specificatie die wel standard-compliant is (!). Daarom vind ik het opleggen van extra constraints bovenop de OpenAPI specificatie symptoombestrijding in plaats van een echte oplossing. De echte oplossingen zou namelijk zijn om de code-generation te fixen, en als tussenoplossing manueel de fouten van de generator oplossen. Maar ik leg me erbij neer dat dit niet gaat gebeuren.

joeribekker commented 2 years ago

@HenriKorver schreef:

Eens met @sergei-maertens om niet af te wijken van het gebruik van een enumeration.

👍

We moeten ook oppassen dat je enumeraties niet "zomaar" of "altijd" vervangt met een externe tabel. Het is geen generieke toepasbare oplossing. Ik vind (1) als je van te voren bijvoorbeeld al weet dat er op relatief korte termijn meer/minder elementen in een enumeratie gaan voorkomen, en (2) clients ondervinden geen "breaking" last van wijzigingen, dat er een case te maken is om een externe tabel bij te houden. Hanteer altijd: enumeration, tenzij...

HenriKorver commented 2 years ago

Uiteindelijk is het de code-generator zelf die buggy is en invalide code genereert uit een API specificatie die wel standard-compliant is (!). Daarom vind ik het opleggen van extra constraints bovenop de OpenAPI specificatie symptoombestrijding in plaats van een echte oplossing.

@sergei-maertens Eens en wat dat betreft kunnen we best kritisch zijn ten opzichte van design rules die teveel leunen op het argument van code-generatie en die herzien waar nodig. Wat mij betreft zijn design rules nooit voor altijd in beton gegoten en moet er altijd mogelijkheid zijn om uit te leggen waarom je het niet hebt toegepast. Alleen in dit geval zou ik dezelfde keuze hebben gemaakt ook zonder het bestaan van DR2.4. In een enumeratie wil men doorgaans korte eenvoudige waarden, bijvoorbeeld ook bij de ZGW API's, zie daar de ontwerpkeuze voor enumeraties. Daar is bij de enumeratie een extra weergave veld toegevoegd zodat je de volledige beschrijving van de enumeratie-waarde kunt teruggeven als een automatisch gegenereerd read-only veld. Dat zou ik hier ook kunnen toevoegen als er behoefte aan is.

HenriKorver commented 2 years ago

Hierbij de uitbreiding met een formaatWeergave veld dat wordt gegenereerd en read-only is:

{
  "vorigContactmoment": "http://example.com",
  "bronorganisatie": "string",
  "registratiedatum": "2019-08-24T14:15:22Z",
  "kanaal": "string",
  "voorkeurskanaal": "string",
  "voorkeurstaal": "str",
  "tekst": {
    "inhoud": "Tekst die in dit geval in **MarkDown** formaat is opgemaakt.",
    "formaat": "markdown",
    "formaatWeergave": "text/markdown; charset=UTF-8"
  },
  "onderwerpLinks": [
    "http://example.com"
  ],
  "initiatiefnemer": "gemeente",
  "medewerker": "http://example.com",
  "medewerkerIdentificatie": {
    "identificatie": "string",
    "achternaam": "string",
    "voorletters": "string",
    "voorvoegselAchternaam": "string"
  }
}
HenriKorver commented 2 years ago

Hierbij ook de aanpassing in de OAS met de toevoeging van het formaatWeergave veld.

HenriKorver commented 2 years ago

N.a.v. interne review zal plain_text worden hernoemd naar plain. Ook zullen er links worden toegevoegd naar de onderliggende RFC's van de IANA-lijst

HenriKorver commented 2 years ago

N.a.v. interne review zal plain_text worden hernoemd naar plain. Ook zullen er links worden toegevoegd naar de onderliggende RFC's van de IANA-lijst

Done! (Zie OAS.)

hdksi commented 2 years ago

Mooi, akkoord wat mij betreft!

hdksi commented 2 years ago

@melsk-r wil je deze reviewen?

melsk-r commented 2 years ago

Akkoord wat mij betreft!