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

Feature/100 basic crud #101

Merged
merged 1 commit into from
Nov 24, 2023
Merged

Feature/100 basic crud #101

merged 1 commit into from
Nov 24, 2023

Conversation

bart-maykin
Copy link
Contributor

fixes #100

Added simple starting crud api for all model fields

@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch from e3e1219 to a69ebca Compare October 31, 2023 10:17
@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2023

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (e3c5fb6) 95.84% compared to head (f0c0533) 97.24%.

Files Patch % Lines
...klantinteracties/api/serializers/klantcontacten.py 95.43% 11 Missing ⚠️
...ponents/klantinteracties/api/filterset/partijen.py 80.76% 10 Missing ⚠️
...ant/components/klantinteracties/models/partijen.py 33.33% 4 Missing ⚠️
...s/klantinteracties/api/filterset/klantcontacten.py 97.56% 2 Missing ⚠️
...lant/components/klantinteracties/admin/partijen.py 92.30% 1 Missing ⚠️
...nents/klantinteracties/api/serializers/partijen.py 99.62% 1 Missing ⚠️
...teracties/models/tests/factories/klantcontacten.py 97.67% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
+ Coverage   95.84%   97.24%   +1.39%     
==========================================
  Files         121      151      +30     
  Lines        3998     7044    +3046     
==========================================
+ Hits         3832     6850    +3018     
- Misses        166      194      +28     

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

@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch 2 times, most recently from 4770f5a to d71a48b Compare November 2, 2023 00:42
@bart-maykin bart-maykin marked this pull request as draft November 2, 2023 00:44
@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch from d71a48b to bd758d2 Compare November 2, 2023 11:39
Copy link
Member

@joeribekker joeribekker left a comment

Choose a reason for hiding this comment

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

Tussendoor review:

Algemeen

  • Voor alle identificators (mixin wellicht) de query params toevoegen: objectidentifcatorObjecttype, objectidentifcatorSoortObjectId, objectidentificatorObjectId
  • Haal van alle inline resources de extra attributen maar even weg, zodat enkel URL en UUID overblijft.
  • Hernoem (in de API serializer) Contactpersoon.organisatie naar Contactpersoon.werkteVoorOrganisatie, en bij alle relaties graag :) Hier kan je ze goed zien: https://vng-realisatie.github.io/klantinteracties/informatiemodel/assets/SIM_Klantinteracties_v006.png

Betrokkene

  • digitaalAdres moet een array zijn 0 of meer digitaleAdressen. Model foutje? Vermoed dat we de relatie moeten omdraaien.
  • Inline toevoegen van Partij
  • Query params toevoegen: klantcontact__nummer, klantcontact__uuid, klantcontact (url), verstrektedigitaalAdres__uuid, verstrekteDigitaalAdres__adres, verstrekteDigitaalAdres (url), wasPartij__nummer, wasPartij__identificator+(attributen bij algemeen), partij (url)

Partij

  • Bij Organisatie graag toevoegen: contactpersonen (array van url/uuid partij-objecten)

Interne Taak

  • Query params toevoegen: nummer, status, toegewezenActor (url), toegewezenActor__naam, toegewezenActor__identificator+(attributen bij algemeen), navKlantcontact (url), navKlantcontact__uuid, navKlantcontact__nummer

Klantcontact

  • Dit is een van de hoofd-resources, dus ik wil hier veel relaties in zoals ook in de admin. Graag de relaties toevoegen naar: gingOverOnderwerpobjecten, omvatteBijlagen, hadBetrokkenen, leideTotInterneTaken
  • Query params toevoegen: nummer, kanaal, hadBetrokkene, inhoud__icontains (case insensitive search), onderwerp__icontains

@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch 5 times, most recently from 74174a5 to 45fe461 Compare November 2, 2023 16:51
@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch 11 times, most recently from f9644b0 to 447879d Compare November 16, 2023 10:17
@bart-maykin bart-maykin marked this pull request as ready for review November 16, 2023 10:30
@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch from 447879d to 66186f0 Compare November 16, 2023 10:36
Copy link
Member

@joeribekker joeribekker left a comment

Choose a reason for hiding this comment

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

Looking good but many minor changes and a few larger ones:

Actoren

  • Add list-query param: naam__icontains

Betrokkene

  • Rename URL-path to "betrokkenen" (plural)
  • Rename attribute "partij" to "wasPartij"
  • Make "wasPartij" a simple inline, just URL and UUID
  • Rename "klantcontact" to "hadKlantcontact"

Bijlage

  • Rename URL-path to "bijlagen" (plural)
  • Resources lack doc-string. The API spec says "bijlage_list" instead of "Alle bijlage opvragen"
  • Rename attribute "klantcontact" to "wasBijlageVanKlantcontact"

Digitaal adres

  • Rename URL-path to "digitaleadressen" (plural, without underscores)
  • Rename attribute "betrokkene" to "verstrektDoorBetrokkene"
  • Add attribute "verstrektDoorPartij"

Interne taak

  • Rename URL-path to "internetaken" (plural, without underscores)
  • Rename attribute "actor" to "toegewezenAanActor"
  • Rename attribute "klantcontact" to "aanleidinggevendKlantcontact"
  • Make attribute "toelichting" 1000 chars

Klantcontacten

  • Rename URL-path to "klantcontacten" (plural)
  • Rename attribute "actoren" to "hadBetrokkenActoren"
  • Rename attribute "hadBetrokkene" to "hadBetrokkenen" (plural)
  • Make "hadBetrokkenen" a simple inline, just URL and UUID
  • Rename attribute "leideTotInterneTaken" to "leiddeTotInterneTaken" (typo, double dd)
  • Rename attribute "gingOverOnderwerpobject" to "gingOverOnderwerpobjecten" (plural)

Onderwerpobject

  • Rename URL-path to "onderwerpobjecten" (plural)
  • Resources lack doc-string. The API spec says "onderwerpobject_list" instead of "Alle onderwerpobjecten opvragen"

Partij

  • Add list-query params: nummer, indicatieGeheimhouding, indicatieActief, soortPartij, werkteVoorPartij__url, werkteVoorPartij__uuid, werkteVoorPartij__nummer, partijIdentificator__objecttype, partijIdentificator__soortObjectID, partijIdentificator__objectId
  • Rename URL-path to "partijen" (plural)
  • Attribute "digitaalAdres" should be an array? Is this correct in the model? It should also be renamed to "digitaleAdressen"
  • Attribute "betrokkene" should be an array? Is this correct in the model? It should also be renamed to "betrokkenen"
  • Add attribute "partijIdentificatoren" as array of objects with uuid and url.

Partij identificatoren

  • Rename URL-path to "partij-identificatoren" (plural)
  • Rename attribute "partij" to "identificeerdePartij" (plural)

@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch 3 times, most recently from 85e6443 to 456624f Compare November 23, 2023 14:18
@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch 4 times, most recently from 2e14aac to 635aaa4 Compare November 24, 2023 14:10
@bart-maykin bart-maykin force-pushed the feature/100-basic-crud branch from 635aaa4 to f0c0533 Compare November 24, 2023 14:43
@joeribekker joeribekker merged commit e24af3e into master Nov 24, 2023
11 checks passed
@joeribekker joeribekker deleted the feature/100-basic-crud branch November 24, 2023 14:53
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.

Als projectmanager wil ik dat het resources ontsloten wordt via een CRUD API
3 participants