search

melsk-r / HC-common-issues

0 stars 0 forks source link

vullen links bij paginering #2

Open melsk-r opened 4 months ago

melsk-r commented 4 months ago

Originally created by fsamwel (https://github.com/VNG-Realisatie/Haal-Centraal-common/issues/48):

Er is discussie over:

n.a.v. opmerking @strijm in https://github.com/VNG-Realisatie/Haal-Centraal-common/pull/40#discussion_r439472183:

Hoe weten clients in dit geval om welke pagina het hier gaat? _links.self is er wel maar zonder page en pageSize, _links.first is er niet, _links.prev is er niet en _links.next is er wel. Dat betekent dat een client dus niet uit de self link kan bepalen om welk pagina het hier gaat en dan uit het feit dat er geen _links.first of _links.prev en wel een _links.next moet halen dat het om de eerste pagina gaat? Wat een omslachtige manier van werken. Ik ben er echt voor om gewoon in de self page en pageSize op te nemen, dan is er één gegeven waar een client altijd aan kan zien om welke pagina het gaat. De andere gegevens kunnen dan gebruikt worden om of helemaal naar het begin of helemaal naar het einde van een result set te gaan (altijd) of één pagina terug of verder te gaan (indien aanwezig), supersimpel! Ik vind het argument dat dit consistent moet zijn met het gedrag van fields en expand niet helemaal kloppen. Ik ben het er wel mee eens dat als deze parameters een default zouden hebben, deze ook gebruikt moeten worden, omdat dat eigenlijk impliciet is wat er door een client is opgegeven, maar fields en expand hebben nu geen default in de specificatie, dus is het gedrag per definitie anders dan van parameters die wel een default hebben.

Als een result set leeg is, kun je erover twisten of er wel of geen pagina is. Geen pagina omdat er geen resultaten zijn of is er één pagina zonder resultaten. M.i. de laatste. En natuurlijk geven we geen prev en next links terug als een result set leeg is. Prev en next links kunnen immers niet ingevuld worden omdat er geen (andere) pagina's zijn met resultaten. Als er geen pagina is zonder resultaten, dan kunnen ook first en last links niet worden geretourneerd. Als er wel één pagina zonder resultaten is, dan zijn de eerste (en dus ook de laatste pagina) hetzelfde (net als self overigens) en bevatten geen resultaten. We kunnen dus wel altijd een first, last en self link invullen. Dat die soms hetzelfde zijn lijkt me geen probleem, als clients altijd een self link krijgen met page en pageSize weten ze altijd waar ze zijn en met de links kunnen ze bepalen welke mogelijkheden ze hebben qua navigatie.

Reactie @MelvLee:

Ik zou de self link niet wijzigen omdat dit een heel ander doel heeft dan navigatie, maar ik begreep uit een eerdere reactie dat dit het gedrag is van een gebruikte framework. Mijn voorstel is daarom om dan de page en pageSize parameter verplicht te maken omdat deze door de framework toch in alle paging links worden toegevoegd.

Verder vind ik het prima om de first en prev links altijd toe te voegen.

Reactie @fsamwel daarop:

Mijn voorstel is daarom om dan de page en pageSize parameter verplicht te maken omdat deze door de framework toch in alle paging links worden toegevoegd.

We hebben deze specificaties nu in de Haal-Centraal-common zitten. Dat betekent dat deze specificaties ook gelden voor andere API's die paginering gebruiken, zoals voor het Handelsregister. Ik ben er niet voor om ook voor bijvoorbeeld het HR dingen gedrag te beschrijven (verplichten) omdat voor het BAG een framework is gebruikt die blijkbaar niet voldoende configureerbaar is om het eigenlijk gewenste gedrag te vertonen. Dan moeten we de paginering (parameters) specifiek in de BAG specs opnemen en niet hergebruiken van de common.

Ook zou ik het gek vinden dat een request "/adressen?zoekresultaatidentificatie=adr-5c01945883b0f0390d7a52f8462b0686" een foutmelding zou geven, omdat geen page en pageSize is opgegeven. Bij deze vraag zal er immers maar één adres terugkomen.

Wat bedoel je met "Verder vind ik het prima om de first en prev links altijd toe te voegen."? Dat je ook wanneer er maar één resultaat is (of zelfs geen resultaat) toch first en previous links worden geleverd? Waarom?

melsk-r commented 4 months ago

This comment originally might have been created by someone else.

een ander punt dat we moeten bespreken: in de specs (in haal centraal common) staat een default voor pageSize. Dit betekent dat in de BAG, maar ook elke andere HC APi die paginering gebruikt, de default paginagrootte 20 is, en het wijzigen daarvan is een breaking change. Willen we dat wel in de specificaties vastgelegd zien?

melsk-r commented 4 months ago

This comment originally might have been created by someone else.

een ander punt dat we moeten bespreken: in de specs (in haal centraal common) staat een default voor pageSize. Dit betekent dat in de BAG, maar ook elke andere HC APi die paginering gebruikt, de default paginagrootte 20 is, en het wijzigen daarvan is een breaking change. Willen we dat wel in de specificaties vastgelegd zien?

We kiezen voor de grote gemene deler als het gaat om de definitie van componenten in common.yaml. Net zoals verplicht geldt ook voor de default dan dat als je wilt afwijken van die 20 je dan deze parameter lokaal moet definieren en geen hergebruik maakt van de common.yaml.

melsk-r commented 4 months ago

This comment originally might have been created by someone else.

We hebben dit besproken in overleg vandaag en zijn tot de conclusie gekomen dat:

  1. De self link altijd identiek moet zijn aan de url van de request
  2. Dat first en last alleen geleverd moeten worden wanneer die ongelijk zijn aan de huidige pagina (first≠self en last≠self)
  3. Dat deze wijzigingen niet de hoogste prioriteit hebben. Beide punten kunnen (zo nodig) in een nazorg-sprint na livegang worden opgelost
melsk-r commented 4 months ago

This comment originally might have been created by someone else.

1e punt (self link) wordt opgelost via issue https://github.com/VNG-Realisatie/Haal-Centraal-BAG-bevragen/issues/244

melsk-r commented 4 months ago

This comment originally might have been created by someone else.

2e punt wordt opgelost in https://github.com/VNG-Realisatie/Haal-Centraal-BAG-bevragen/issues/254

Er zijn geen wijzigingen in de features of specificaties nodig hiervoor.