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.

Sign up for GitHub

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

Jump to bottom

Feature/233 fix values partij identificatoren #291

Merged
merged 2 commits into from
Jan 17, 2025
Merged

Feature/233 fix values partij identificatoren #291

merged 2 commits into from
Jan 17, 2025

Conversation

danielmursa-dev
Copy link
Contributor

@danielmursa-dev danielmursa-dev commented Nov 25, 2024 •
edited
Loading

Fixes #233

image

ChoiceFields:

  • partij_identificator_code_register
  • partij_identificator_code_objecttype
  • partij_identificator_code_soort_object_id

Created a validator for the hierarchy specified in this #233 (comment)

In particular, each choice must be part of the group admitted by the top level choice.
If the top level choice is overige or is None, the rule no longer applies.

PartijIdentificatorCodeRegister -> PartijIdentificatorCodeObjectType -> PartijIdentificatorCodeSoortObjectId

Allowed cases:

  1. brp → natuurlijk_persoon → bsn
  2. brp → natuurlijk_persoon → overig
  3. hr → vestiging → vestigingsnummer
  4. hr → vestiging → overig
  5. hr → niet_natuurlijk_persoon → rsin
  6. hr → niet_natuurlijk_persoon → kvk_nummer
  7. hr → niet_natuurlijk_persoon → overig
  8. overig → natuurlijk_persoon → bsn (all previous cases with register = overig)
  9. '' → '' → ''
  10. brp → '' → ''
  11. '' → 'natuurlijk_persoon' → ''
  12. overig → overig → overig

Not allowd cases: all those that do not respect the hierarchy, and those where the level in between is overig, because once the register is chosen the whole structure must be respected

  1. brp → niet_natuurlijk_persoon → bsn
  2. brp → natuurlijk_persoon → kvk_nummer
  3. brp → overig → niet_natuurlijk_persoonbsn

@danielmursa-dev danielmursa-dev force-pushed the feature/233-fix-values-partij-identificatoren branch 5 times, most recently from c90b055 to d395b24 Compare November 26, 2024 08:55
@danielmursa-dev danielmursa-dev force-pushed the feature/233-fix-values-partij-identificatoren branch from d395b24 to 8a83b40 Compare December 18, 2024 09:59
@codecov-commenter
Copy link

codecov-commenter commented Dec 20, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.21%. Comparing base (4715811) to head (e727954).
Report is 7 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #291      +/-   ##
==========================================
+ Coverage   97.10%   97.21%   +0.11%     
==========================================
  Files         177      180       +3     
  Lines        8656     8998     +342     
==========================================
+ Hits         8405     8747     +342     
  Misses        251      251

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielmursa-dev danielmursa-dev force-pushed the feature/233-fix-values-partij-identificatoren branch 2 times, most recently from 3a04c9d to 634a0f3 Compare December 23, 2024 13:11
stevenbal
stevenbal requested changes Dec 24, 2024
View reviewed changes
src/openklant/components/klantinteracties/models/constants.py Show resolved Hide resolved
src/openklant/components/klantinteracties/models/constants.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/constants.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/constants.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
swrichards
swrichards requested changes Jan 6, 2025
View reviewed changes
src/openklant/components/klantinteracties/models/constants.py Show resolved Hide resolved
src/openklant/components/klantinteracties/models/constants.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/constants.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Show resolved Hide resolved
src/openklant/components/klantinteracties/tests/test_validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/partijen.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/partijen.py Outdated Show resolved Hide resolved
@danielmursa-dev danielmursa-dev marked this pull request as draft January 13, 2025 14:45
@danielmursa-dev danielmursa-dev force-pushed the feature/233-fix-values-partij-identificatoren branch from 5011d28 to 88b0cf2 Compare January 14, 2025 11:33
@danielmursa-dev danielmursa-dev marked this pull request as ready for review January 14, 2025 15:04
swrichards
swrichards requested changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@swrichards swrichards left a comment

Choose a reason for hiding this comment

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

Looks good, just some final comments/thoughts.

src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/partijen.py Show resolved Hide resolved
src/openklant/components/klantinteracties/models/partijen.py Outdated Show resolved Hide resolved
@danielmursa-dev danielmursa-dev marked this pull request as draft January 15, 2025 13:58
@danielmursa-dev danielmursa-dev force-pushed the feature/233-fix-values-partij-identificatoren branch 2 times, most recently from 09dcfc5 to 9269b0c Compare January 15, 2025 15:16
@danielmursa-dev danielmursa-dev marked this pull request as ready for review January 15, 2025 15:40
stevenbal
stevenbal approved these changes Jan 16, 2025
View reviewed changes
Copy link
Collaborator

@stevenbal stevenbal left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me, just two questions about potential cleanup. Make sure to rebase on master before merging

src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
src/openklant/components/klantinteracties/models/validators.py Outdated Show resolved Hide resolved
@danielmursa-dev
[#233] Fix values partij identificatoren
dedd953 
[#233] Fix tests

[#233] Install django-digid-eherkenning

[#233] Fix tests

[#233] Improve Validator

[#233] Fix tests

[#233] Isort and Flake8

[#233] Improvements

[#233] Remove django-digid-eherkenning

[#233] Fix requirements

[#233] Fix requirements 2

[#233] Fix tests

[#233] Flake8

[#233] Fix validation errors

[#233] Update class

[#233] Remove unused import

[#233] Update klantinteracties.json and help_text in migrations

[#233] Update overig constants

[#233] Pass variables through constructor

[#233] Change PartijIdentificatorValidator call

[#233] Use validators from vng_api_commons

[#233] Fix import

[#233] Fix validate api

[#233] Fix REGISTERS structure

[#233] Fix tests

[#233] Improve validator

[#233] Fix messages

[#233] Fix Flake

[#233] Update kvk_nummer naming

[#233] Update vestigingsnummer naming

[#233] Update some names

[#233] Update some names

[#233] Update migration file

[#233] Update naming

[#233] Fix test messages

[#233] Fix some tests

[#233] Improve error message

[#233] Fix third level check

[#233] Fix partij_identificator_object_id message

[#233] Fix tests and pre-merge

[#233] Update migrations

[#233] bin/generate_schema_for_component.sh klantinteracties and contactgegevens

[#233] Fix test

[#233] Fix test messages and create new

[#233] Change dict in list
@danielmursa-dev
[#233] Fix master conflicts
f41c29f
@danielmursa-dev danielmursa-dev force-pushed the feature/233-fix-values-partij-identificatoren branch from 980da3f to f41c29f Compare January 17, 2025 14:39
@danielmursa-dev danielmursa-dev merged commit e848392 into master Jan 17, 2025
28 checks passed
@danielmursa-dev danielmursa-dev deleted the feature/233-fix-values-partij-identificatoren branch January 17, 2025 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevenbal stevenbal stevenbal approved these changes

@swrichards swrichards swrichards approved these changes

@bart-maykin bart-maykin Awaiting requested review from bart-maykin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fixed set of values for 'partij-identificatoren' (ENUM)
4 participants
@danielmursa-dev @codecov-commenter @swrichards @stevenbal