Skip to content
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

Update design_decisions.md #26

Merged
merged 2 commits into from
Apr 20, 2020
Merged

Update design_decisions.md #26

merged 2 commits into from
Apr 20, 2020

Conversation

melsk-r
Copy link
Contributor

@melsk-r melsk-r commented Apr 16, 2020

Design Decisions heb ik beter proberen te ordenen. Ze hebben nu ook een nummer toegewezen gekregen bestaande uit 'DD'[xx].[xx]. 'DD' staat natuurlijk voor 'Design Decision' en dat kunnen we evt. nog vervangen door 'HCDD' (HaalCentraal Design Decision) of iets anders.
Daar waar mogelijk is er een verwijzing opgenomen naar de gerelateerde sectie in de NL API strategie. Ook heb ik hier en daar de formuleringen aangescherpt.

Design Decisions heb ik beter proberen te ordenen. Ze hebben nu ook een nummer toegewezen gekregen en daar waar mogelijk is er een verwijzing opgenomen naar de gerelateerde sectie in de Nl API strategie. Ook hier en daar wat formuleringen aangescherpt.
@fsamwel
Copy link
Collaborator

fsamwel commented Apr 16, 2020

@melsk-r dit pull request gaat pull request #21 vreselijk in de weg lopen, of andersom (afhankelijk van welke eerst wordt gemerged).

Maar op zich is ordenen en nummeren een prima actie.

Als ik het document zo bekijk zou het denk ik beter ontwerprichtlijnen moeten heten in plaats van design decisions. Het zijn namelijk geen ontwerpbeslissingen die erin staan, maar richtlijnen of best practices voor het ontwerpen van een API

@melsk-r
Copy link
Contributor Author

melsk-r commented Apr 16, 2020

Eerst maar #21 mergen zou ik dus zeggen.

@fsamwel
Copy link
Collaborator

fsamwel commented Apr 16, 2020

Ik heb #21 gemerged, nu geeft deze dus conflicten aan. Ik denk dat het makkelijker is voor me om te reviewen, wanneer je die conflicten hebt opgelost.

@melsk-r
Copy link
Contributor Author

melsk-r commented Apr 16, 2020

Je kunt nu je gang gaan Frank, ik heb de conflicten weggewerkt. Je voorstel om het document ontwerprichtlijnen te noemen heb ik er nog niet in verwerkt. Dat heeft trouwens ook gevolgen voor de nummering want dan starten we de nummers natuurlijk niet met 'DD'.
Denk even goed na over deze nummering. In overleg met Johan ga ik op dezelfde wijze ook door het Design Decisions document van de BAG, BRK en BRP en het lijkt mij goed om ook die Design Decisions (of richtlijnen) te nummeren. De vraag is of we dezelfde nummering als hier hanteren of dat we in de nummering een indicatie voor BAG, BRK en BRP opnemen.

@JohanBoer
Copy link
Collaborator

Robert, de descriptions van Expand en Fields parameter in de common.yaml waren door Cathy opgesteld. Ik denk dat je haar even specifiek moet vragen of zijn het met jouw verbetervoorstel eens is.

@melsk-r
Copy link
Contributor Author

melsk-r commented Apr 17, 2020

@JohanBoer Ik neem aan dat je op een specifieke Design Decision doelt want ik ben me er nl. niet van bewust dat ik de descriptions van deze parameters in de common yaml heb aangepast. Waar doel je precies op?

Copy link
Collaborator

@fsamwel fsamwel left a comment

Choose a reason for hiding this comment

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

Robert, de indeling en nummering is prima. Mijn opmerkingen gaan over:

  • inhoud van punten
  • andere documentnaam/documenten zodat er onderscheid is tussen API ontwerp en API architectuur

Ik vind het prima als je eerst deze pull request merged (mijn inhoudelijke opmerkingen staan los van wat je met het pull request hebt proberen te bereiken), maar zou het wel fijn vinden als op korte termijn iets met mijn opmerkingen gedaan wordt.


Uitgangspunt voor de naamgeving van resourceproperties is altijd zo duidelijk mogelijk te benoemen wat iets is.
Hoofdregel is altijd:
Copy link
Collaborator

Choose a reason for hiding this comment

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

waarom maak je van "uitgangspunt" het veel hardere "hoofdregel"? in mijn visie is dit document geen standaard, maar zijn het suggesties (best practices, checklist, ...) die je (met gezond verstand) kunt gebruiken om een goede API te ontwerpen.


## Toepassen van Patterns
### DD1.11 Schema componentnamen voor domeinwaarden en enumeraties krijgen een vaste extensie
Schema componenten voor dynamische domeinwaarden (referentielijsten zoals "Tabel 32 Nationaliteitentabel") en enumeraties krijgen respectievelijk extensie "\_tabel" en "\_enum".
Copy link
Collaborator

Choose a reason for hiding this comment

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

is er ooit een schema component voor een tabel? (afgezien van de generieke in common.yaml)?


Bij een specifieke bevragingen-API is het toepassen van Patterns beperkt tot de parameters.
### DD1.12 Namen van gegevensgroepen worden ingekort
Copy link
Collaborator

Choose a reason for hiding this comment

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

dit geldt niet alleen voor namen van gegevensgroepen, toch? Als ik zo de voorbeelden zie gaat het over het weghalen van redundantie in propertynamen. Bijvoorbeeld de gegevensgroep overlijden heeft elementen overlijdensdatum, landoverlijden en plaatsoverzijden. Dat wordt dan datum, plaats en land, omdat het al in gegevensgroep overlijden zit. Of wordt hier iets anders bedoeld?

## Dynamische domeinwaarden worden in de query-parameters met de code opgenomen
Voor een query-parameter waarin een enty uit een waardelijst of een landelijke tabel als selectie-criterium wordt gebruikt wordt altijd de *code* van de entry gebruikt.
### DD2.1 Dynamische domeinwaarden worden in de response opgenomen met zowel de code als de omschrijving
Voor een element van een referentielijst-type, wordt in de response zowel de code als de omschrijving opgenomen. Dit betreft dynamische lijsten (tabellen) met een code en waarde, zoals "Tabel 32 Nationaliteitentabel" en "Tabel 36 Voorvoegselstabel".
Copy link
Collaborator

Choose a reason for hiding this comment

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

voorbeeld van de voorvoegseltabel is erg ongelukkig. Dat is nu net de enige BRP tabel die geen codes kent en alleen waardes/omschrijvingen.


## Enumeraties-waarden bevatten geen hoofdletters.
Enumeratiewaarden bevatten alleen kleine letters en underscores. Geen spaties, geen speciale tekenen en geen hoofletters.
Als landelijk beheerde dynamische domeinwaarden ook daadwerkelijk landelijk beschikbaar gesteld worden (zoals de common ground gedachte wel beoogd) dan worden deze als resource ontsloten en dus als link (uri) opgenomen.
Copy link
Collaborator

Choose a reason for hiding this comment

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

dit suggereert dat in dat geval de code en omschrijving niet meer worden meegeleverd en alleen een link. dat is erg onvriendelijk voor de gebruiker. het toevoegen van de link naast de code en omschrijving is dan wel een goed idee, hoewel de toegevoegde waarde erg beperkt is. het volgen van die link geeft namelijk nauwelijks extra informatie. hoogstens sinds wanneer de code in gebruik is.


Deze gegevens worden niet vastgelegd in een (voor alle gemeenten geldend) bronsysteem dat voor de bevraging geraadpleegd kan worden. Deze gegevens zijn dus (voorlopig) niet raadpleegbaar. Ook worden deze gegevens niet in alle gemeenten (op dezelfde manier) gebruikt.

### DD5.6 De API filtert terug te geven gegevens op autorisatie van de organisatie
Copy link
Collaborator

Choose a reason for hiding this comment

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

is het niet beter een document te maken met ontwerprichtlijnen voor API ontwerp, gericht op hoe de API specificaties eruit moeten zien, en een ander document met dit soort architectuurprincipes?


## Toepassen van HAL-Links
Er zijn grofweg twee categoriën Hal-links waar we gebruik van maken. Links naar resources binnen het eigen domein en links naar resources die in een ander domein beheerd worden. Om discoverability te bereiken, worden voor beide categorieën de Hal-link opgenomen naar de gerelateerde resource.
Deze design decision is al lang geleden gemaakt. Meer recent hebben we uit technische hergebruik overwegingen abstracte types geïntroduceerd. Het is het overwegen waard om die technische keuzes te vergelijken met de keuzes uit de informatiemodellen en te controleren of daar uniformering mogelijk en wenselijk is.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Er moet dus juist een ontwerprichtlijn zijn die zegt dat je zoveel mogelijk moet hergebruiken. Dus wanneer er twee componenten zijn met meerdere identieke eigenschappen, dat er een bovenliggende component wordt gemaakt voor overerving (hergebruik).

Voor developers die geen HAL links willen gebruiken wordt tevens de identificatie van de gerelateerde resource opgenomen.
### DD5.9 Actuele zoekresultaten worden niet gesorteerd
De API standaard schrijft niet voor hoe zoekresultaten in de API moeten worden gesorteerd. Wanneer de client behoefte heeft aan gesorteerde resultaten, moet zij de ontvangen resultaten zelf sorteren.
Dit betekent dat er in de berichtspecificaties geen gebruik gemaakt wordt van de _'sorteer'_ parameter.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Er wordt alleen een parameter sorteer opgenomen, wanneer daar behoefte aan is (die blijkt uit een user story). Maar dat geldt voor elke parameter.


## Attributen die geen waarde of een boolean waarde ‘false’ hebben, worden niet geretourneerd door de API.
### DD5.11 Attributen die geen waarde of een boolean waarde ‘false’ hebben, worden niet geretourneerd door de API
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wat is het verschil tussen DD05.10 en DD05.11?

Ik vind het gebruik van termen als indicator en attribuut verwarrend. Wat zijn dat?

In JSON responses spreken we over objecten en properties (Nederlandse vertaling "eigenschappen"). "Attributen" is denk ik een term uit informatiemodellen o.i.d.

## Gebruik van Booleans als indicatoren
In diverse situaties worden booleans opgenomen als er sprake is van indicatoren. Deze booleans worden alleen geretourneerd als de waarde van de boolean ook informatief is. De indicator wordt dus alleen opgenomen als de waarde van de Boolean "true" is.
### DD5.10 Indicatoren die gebruik maken van Booleans worden alleen geretourneerd als de waarde 'true' is
In diverse situaties worden booleans opgenomen als er sprake is van indicatoren. Deze booleans worden alleen geretourneerd als de waarde van de boolean ook informatief is. De indicator wordt dus alleen opgenomen als de waarde van de Boolean 'true' is.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ook relevant hier is dat we een indicator altijd definiëren als boolean. Eigenschappen die functioneel alleen een waarde Ja/aan/waar/wel of Nee/uit/onwaar/niet kunnen hebben, worden gedefinieerd als boolean. We gebruiken dus geen enumeratie zoals [J,N] voor dit soort situaties.

@melsk-r
Copy link
Contributor Author

melsk-r commented Apr 17, 2020

Ik zal al je reacties goed bestuderen. Over het algemeen geldt echter dat ik voornl. heb ingezet op herordenen en structureren. Inhoudelijk heb ik weinig gewijzigd, alleen daar waar de omschrijving n.m.m. te bondig of niet duidelijk genoeg was heb ik gepoogd dat te verbeteren. Inhoudelijk kan ik ook niet veel wijzigen aangezien ik tot nu toe natuurlijk niet betrokken was bij HaalCentraal en dus de ratio's achter deze regels niet of onvoldoende ken. Deze actie was voor mij juist een manier om daar meer kennis over op te doen.
Om die reden is het verstandig als ik even op de reacties van Johan op jouw opmerkingen wacht.

@fsamwel
Copy link
Collaborator

fsamwel commented Apr 20, 2020

@melsk-r ik heb mijn opmerkingen, plus een paar toevoegingen opgenomen in issues #27, #28 en #29. Wat mij betreft kan je deze pull request dus mergen. Ik heb namelijk geen opmerkingen over de herstructurering die je gedaan hebt.

@melsk-r melsk-r merged commit d2aeff9 into master Apr 20, 2020
@JohanBoer JohanBoer deleted the melsk-r-patch-1 branch October 27, 2020 09:29
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.

None yet

3 participants