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

Fixed set of values for 'partij-identificatoren' (ENUM) #233

Open
mariusvandam opened this issue Aug 29, 2024 · 20 comments · May be fixed by #291
Open

Fixed set of values for 'partij-identificatoren' (ENUM) #233

mariusvandam opened this issue Aug 29, 2024 · 20 comments · May be fixed by #291
Assignees
Labels

Comments

@mariusvandam
Copy link

mariusvandam commented Aug 29, 2024

Thema / Theme

Klantinteracties API

Omschrijving / Description

Omschrijving

We willen dat de waardes voor de identificatoren van een Partij enums worden, zodat er nooit verschillen kunnen ontstaan in de waarden van de verschillende onderdelen van de Identificator

Specific details

Voor de volgende properties moeten enums komen.

partijIdentificator.codeRegister -> toegestane waarden:

  • brp
  • hr
  • overig

partijIdentificator.codeObjecttype -> toegestane waarden:

  • natuurlijkPersoon
  • vestiging
  • nietNatuurlijkPersoon
  • overig

partijIdentificator. codeSoortObjectId -> toegestane waarden:

  • bsn
  • kvkNummer
  • rsin
  • vestigingsnummer
  • overig

Aanvullende opmerkingen / Additional context

No response

@mariusvandam mariusvandam added enhancement New feature or request triage labels Aug 29, 2024
@mariusvandam
Copy link
Author

Dimpact backlog: https://dimpact.atlassian.net/browse/POD-246

@andyverberne
Copy link

andyverberne commented Sep 2, 2024

Ik heb gekeken naar de identificaties voor personen en bedrijven in de Zaken API's bij een Rol.
Mogelijk is onderstaande lijst dan de lijst met mogelijke waarden:

  • anp_identificatie Het door de gemeente uitgegeven unieke nummer voor een ANDER NATUURLIJK PERSOON
  • a_nummer Het administratienummer van de persoon, bedoeld in de Wet BRP
  • bsn Het burgerservicenummer, bedoeld in artikel 1.1 van de Wet algemene bepalingen burgerservicenummer
  • ann_identificatie Het door de gemeente uitgegeven unieke nummer voor een ANDER NIET-NATUURLIJK PERSOON
  • nnp_id Het door een kamer toegekend uniek nummer voor de INGESCHREVEN NIET-NATUURLIJK PERSOON
  • vestigingsnummer Een korte unieke aanduiding van de Vestiging.
  • kvk_nummer

@joeribekker
Copy link
Member

I'd like to limit the choices now to registers that are official, to make things less vague for client side implementations.

Technical implications:

partijIdentificatorCodeRegister (becomes an ENUM)

There are 10 registers of which 2 are relevant for Open Klant:

  • BRP (this includes the RNI)
  • HR
  • Overige

We'll add Overige to allow some flexibility in the other properties.

partijIdentificatorCodeObjecttype (adds VALIDATION)

CodeObjecttype choices are limited based on CodeRegister:

  • BRP
    • NatuurlijkPersoon (i would like to use IngeschrevenPersoon because these cover both RNI and GBA persons and not other persons. This also hinted at in the RSGB) However, since this entity is not widely used, we keep to the more general entity.
  • HR (also called: NHR - Nieuw Handelsregister, but let's not do that)
    • Vestiging
    • NietNatuurlijkPersoon (imo this should be IngeschrevenNietNatuurlijkPersoon but this entity was deliberately kept out of RSGB. The same reasoning as NatuurlijkPersoon above. So we use NietNatuurlijkPersoon.)
  • Overige -- everything goes

NOTE: I left Vestiging in since it's modelled this way in RSGB/RGBZ but it's a specialization of NNP and its in the same register. The use of a separate objecttype might complicate things for no reason, however, technically, I don't think you need to pass CodeObjecttype as a search/filter param and just use the CodeSoortObjectId.

partijIdentificatorCodeSoortObjectId (adds VALIDATION)

CodeObjecttypes choices are limited based on CodeRegister:

partijIdentificatorObjectId (adds VALIDATION)

ObjectId validation takes place, based on CodeSoortObjectId
So, essentially, we need 4 validators:

  • bsn - 8/9 numbers + 11 proef
  • rsin - 8/9 numbers + 11 proef
  • kvkNummer- 8 numbers
  • vestigingsnummer - 12 numbers

@joeribekker
Copy link
Member

It also seems useful to pick up #267 at the same time.

@sytskevanhasselt
Copy link

@joeribekker , one question about "CodeObjecttypes choices are limited based on CodeRegister:"
Do I understand correctly, it will not be possible to connect a kvkNummer to a vestiging?
I wonder if that might be too limiting. As I understand it, a vestging will keep its vestigingsnummer, but when there is a take over, for instance, alle La Place-vestigingen will change hands from Vendex to Jumbo, than the KvK-nummer of that vestiging will change. To what extent will it be necessary to differentiate between La Place-when-it-was-Vendex, and La Place-when-it-was-Jumbo? In that case there would be 2 different partijen, eacht with 2 partijIdentificatoren: 1 kvknummer, and 1 vestigingsnummer.
@andyverberne , what would be your view?

danielmursa-dev added a commit that referenced this issue Nov 18, 2024
danielmursa-dev added a commit that referenced this issue Nov 18, 2024
danielmursa-dev added a commit that referenced this issue Nov 18, 2024
@sytskevanhasselt
Copy link

@andyverberne has confirmed, from Eviden internally and also Gemeente Roermond, that the combinatin Vestigingsnummer + KvKnrA is a different 'client' than the combination Vestigingsnummer + KvKnrB.
In the case of La Place, going from Vendex to Jumbo: if an employee from Jumbo logs on with eHerkenning, they are not per se supposed to be able to see Zaken and contacten of that La Place, when it was still a Vendex Vestiging.

Which means that the proper uniqueness of a vestiging lies in the combination of KvK-nr AND Vestigingsnummer.

@joeribekker
Copy link
Member

joeribekker commented Nov 20, 2024

@sytskevanhasselt if the conclusion is that we need 2 attributes, the structure might not be sufficient since we only have one "ObjectId". A structure change can not be realized just like that.

So, we could look at alternatives that fit within the structure:

  1. Use KvK-number + Vestigingsnumber as ObjectId, something like: ObjectId=<kvk-number>:<vestigings-number>, ie: ObjectId=12345678:987654321987 We then also use CodeObjecttypes=kvkNummer:vestigingsnummer as an extra option

2a. We could try to make a unique that spans 2 identificatoren (kvk and vestigingsnummer), which might or might not be there at all times.

2b. We can not make the unique and make it a pattern that can be used (so nothing formal, just a suggestion in the API spec) to always store 2 identificatoren in case of vestigings.

@swrichards
Copy link

I think the conclusion from @andyverberne and @sytskevanhasselt examples might be even stronger: it is not only that kvk + vestiging should be unique, but that vestiging shouldn't exist as an independent identifier at all. As I understand it, referring to a partij solely using a vestigingsnummer should not be possible, because it cannot be guaranteed to point to a stable entity through time (e.g. when, after a merger, vestigingen get moved to a different parent kvk). So the only way to refer to a vestiging is in conjunction with its parent KvK nummer.

As for how to model this, I would prefer something like @joeribekker 's first option, so we would get:

  • HR
    • Vestiging
      • vestigingsnummer vestigingMetKvKNummer in format {kvkNummer}:{vestigingsnummer} (with each component being validated according to the checksums)
    • NietNatuurlijkPersoon
      • kvkNummer
      • rsin

This means that you can still refer to the NietNatuurlijkPersoon as a legal entity (without specifying a vestiging) using kvkNummer, but to refer to a vestiging you would always need both numbers in the form of vestigingMetKvKNummer.

@joeribekker
Copy link
Member

@sytskevanhasselt agree as well with option 1?

danielmursa-dev added a commit that referenced this issue Nov 25, 2024
danielmursa-dev added a commit that referenced this issue Nov 25, 2024
danielmursa-dev added a commit that referenced this issue Nov 25, 2024
@mariusvandam
Copy link
Author

Content aangepast @sytskevanhasselt

danielmursa-dev added a commit that referenced this issue Dec 18, 2024
[#233] Fix admin

[#233] Migrations

[#233] Revert commits

[#233] Remove admin blocks

[#233] Update models and migrations

[#233] Fix tests

[#233] Fix and extend tests

[#233] Create validators

[#233] Fix validators

[#233] Fix tests

[#233] Fix migrations

[#233] Formatting

[#233] Merge

[#233] Run isort

[#233] Run black

[#233] Run flake and fix tests
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 19, 2024
danielmursa-dev added a commit that referenced this issue Dec 20, 2024
danielmursa-dev added a commit that referenced this issue Dec 20, 2024
danielmursa-dev added a commit that referenced this issue Dec 20, 2024
danielmursa-dev added a commit that referenced this issue Dec 20, 2024
danielmursa-dev added a commit that referenced this issue Dec 20, 2024
danielmursa-dev added a commit that referenced this issue Dec 20, 2024
danielmursa-dev added a commit that referenced this issue Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

9 participants