melsk-r / HC-BRK-bevragen-issues

0 stars 0 forks source link

Verduidelijking reason bij opvragen niet-uniek field of expand #274

Closed melsk-r closed 3 days ago

melsk-r commented 3 days ago

Originally created by rhengeveld (https://github.com/VNG-Realisatie/Haal-Centraal-BRK-bevragen/issues/604):

Naar aanleiding van #418 ervaren ik en een Kadaster tester dat de volgende melding ~erg~ te weinig informatie geeft: image

Wat wij hierin missen is welke properties ertoe leiden dat de gevraagde property als niet-uniek wordt gezien. Dit sluit ook aan op de initiële onduidelijkheid met postbusnummer van #587.

Ondertussen heb ik logging aan de applicatie toegevoegd, zodat we dit in ieder geval intern makkelijker kunnen nazoeken:

Found at least 2 results for property 'dag': [geboorte.datum.dag, heeftpartnerschap.datumontbinding.dag]
Found at least 2 results for property 'jaar': [geboorte.datum.jaar, heeftpartnerschap.datumontbinding.jaar]

Een gebruiker heeft deze feedback echter niet.

Is het een idee om dergelijke feedback ook terug te geven aan de gebruiker mits het om 1 parameterwaarde gaat? Bijvoorbeeld:

melsk-r commented 3 days ago

This comment originally might have been created by someone else.

Volgens mij is er niet echt een maximale lengte voor details en reason, dus wat mij betreft kan je dit toevoegen, ook wanneer er meerdere niet-unieke fields velden zijn.

2+ parameterwaardes met meerdere resultaten: "reason": "Deel van de parameterwaarde niet uniek: dag, maand. Er zijn meerdere resultaten gevonden voor dag: geboorte.datum.dag, heeftpartnerschap.datumontbinding.dag. Er zijn meerdere resultaten gevonden voor maand: geboorte.datum. maand, heeftpartnerschap.datumontbinding.maand, +3 andere."

De foutafhandeling feature beschrijft wat in de invalidParams.reason komt met als doel ten minste bepaalde informatie te leveren aan gebruikers. Dit mag nooit een reden zijn niet meer informatie te geven. Vooral niet in een veld als reason, want die is echt bedoeld als voor mensen leesbare tekst, niet computer leesbaar. Toevoegen voor meer informatie is dus zeker niet erg (zolang die wel juist is, duidelijk en enigszins bondig), is juist goed. Hoe beter we de gebruiker helpen, des te beter is het.

melsk-r commented 3 days ago

This comment originally might have been created by someone else.

Fijn, goed dat te weten 😄

In mijn voorbeelden probeer ik de reason nog enigszins kort en bondig te houden; met de limiet op 2 properties (en daarna "+X andere") is het bedoeld als leidraad/voorbeeld, niet als uitputtende lijst van alle gevonden properties. Dit kunnen we uiteraard voor elke parameterwaarde doen.

@fsamwel aansluitend op je voorbeeld lijkt dit mij een goede tussenvorm:

"reason": "Deel van de parameterwaarde niet uniek: dag, maand, datum.jaar. Er zijn meerdere resultaten gevonden voor dag: geboorte.datum.dag, heeftpartnerschap.datumontbinding.dag. Voor maand: geboorte.datum.maand, heeftpartnerschap.datumontbinding.maand, +3 andere. Voor datum.jaar: geboorte.datum.jaar, overlijden.datum.jaar."

melsk-r commented 3 days ago

This comment originally might have been created by someone else.

Lijkt mij prima zo! Wordt er veel duidelijker op, perfect!

melsk-r commented 3 days ago

This comment originally might have been created by someone else.

Top, dan ga ik het zo implementeren

melsk-r commented 3 days ago

This comment originally might have been created by someone else.

Geïmplementeerd ✔️ en beschikbaar op de testomgeving.

Voorbeeldje van de testomgeving: image

Extremer voorbeeldje, gecombineerd met andere fields en expand:

{
  "type": "https://docs.microsoft.com/en-us/dotnet/api/system.net.httpstatuscode?#System_Net_HttpStatusCode_BadRequest",
  "title": "Een of meerdere parameters zijn niet correct.",
  "status": 400,
  "detail": "The request could not be understood by the server due to malformed syntax. The client SHOULD NOT repeat the request without modification.",
  "instance": "/kadastraalonroerendezaken?persoon__identificatie=1233582&zakelijkGerechtigde__type=EIGENAAR&fields=self,ident,coordinates&expand=bestaatNiet,teller,href",
  "code": "paramsValidation",
  "invalidParams": [
    {
      "name": "fields",
      "code": "fields",
      "reason": "Deel van de parameterwaarde niet uniek: coordinates. Er zijn meerdere resultaten gevonden voor coordinates: begrenzingperceel.coordinates, plaatscoordinaten.coordinates."
    },
    {
      "name": "expand",
      "code": "expand",
      "reason": "Deel van de parameterwaarde niet uniek: teller, href. Er zijn meerdere resultaten gevonden voor teller: embedded.zakelijkgerechtigden.tenaamstelling.aandeel.teller, embedded.zakelijkgerechtigden.tenaamstelling.gezamenlijkaandeel.teller. Voor href: embedded.stukken.links.self.href, embedded.stukken.links.stukdelen.href, +11 andere."
    },
    {
      "name": "fields",
      "code": "fields",
      "reason": "Deel van de parameterwaarde niet correct: ident."
    },
    {
      "name": "expand",
      "code": "expand",
      "reason": "Deel van de parameterwaarde niet correct: bestaatNiet."
    }
  ]
}