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

Extend op consumer to all unit types #606

Open
wants to merge 15 commits into
base: development
Choose a base branch
from

Conversation

claire-lovisa
Copy link
Contributor

@claire-lovisa claire-lovisa commented Oct 17, 2024

Context

DL-6217 & DL-6218

Details

In this PR, we:

  1. Flush all data that is going to be re-imported by the consumer as well as the sync jobs
  2. We extend the rules so that consumed administrative units have all they need (concept schemes mostly). An extensive analysis of what needs to be added can be found in the comments of DL-6218.

How to test

@cecemel said it all in his analysis on DL-5713:

How I would test: first create a bestuurseenheid in OP and see whether (after a while) you can:
    - CRUD an inzending voor toezicht
    - CRUD a bericht
    - CRUD BBCD
    - CRUD leidinggevenden
    - CRUD personeelsbeheer (if you fixe the migration)
    - CRUD manndatarissen, potentiall not needed as the master-app is going to be LMB, but still a good sanitycheck. Mind this is only for political stuff.

I have tested for PEVA Gemeente, it flow to Loket, the update-bestuurseenheid-mock-login service does create the mock-login user and we can access the app.

What's next

DL-6219: we should now create Personeelsdatabank data for those imported admin units. The mock-login service still does its job as long as we don't consume other organizations that are not besluit:Bestuurseenheid.

@claire-lovisa claire-lovisa force-pushed the feature/extend-op-consumer-to-all-unit-types branch from 505f927 to 6197ed4 Compare October 17, 2024 12:00
They shouldn't not be in Loket, only in OP and CVP. See OP-3306 for context
@claire-lovisa claire-lovisa marked this pull request as ready for review October 18, 2024 09:47
@cecemel cecemel changed the base branch from development to master October 24, 2024 09:23
@cecemel cecemel changed the base branch from master to development October 24, 2024 09:23
@cecemel
Copy link
Member

cecemel commented Oct 24, 2024

Thanks, so far it's progressing, but some bugs pop up during testing.

Too many bestuursorganen in toezicht.

It's seems we need a more fine grained mapping rule for bestuursorganen to add them to specific concept:schemes.
Now we have e.g. the situation:
image
Here it's clear we shouldn't see the leidinggevenden organen.

Too few bestuursorganen get exported by OP.

E.g. for https://data.lblod.info/id/bestuurseenheden/2c80d772-5a21-4e9d-be8a-35e9bb6d66c6
We expect 4 bestuursorganen. We only get 1. That's an issue at the OP side, not all bestuursorganenclass codes were added to
the export concept scheme.

SELECT DISTINCT ?s ?label {

?s ?in <http://lblod.data.gift/concept-schemes/10caf4a4-1ff5-4c05-917e-56489910d68b>.

?s skos:prefLabel ?label

}

@claire-lovisa
Copy link
Contributor Author

@cecemel Thanks for testing it out ! The too many bestuursorganen part will be to be investigated. For the too few part, can you check lblod/app-organization-portal#470 ? It might help !

claire-lovisa and others added 6 commits October 25, 2024 11:21
WIP because waiting for confirmation on which classifications to keep on
DL-6219, and the query seems to big to come through (at least for the
first run). Might be solved if we exclude more classifications.
1. there was a missing concept, not present in the loket-cli, but
probs added later.
2. moved filter to subquery, since that get's executed first. So less
heavy query.
3. added default obsValue
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.

2 participants