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

Legacy-Role haven't got removed in hc #44

Open
Vento-Nuenenen opened this issue Feb 16, 2022 · 18 comments
Open

Legacy-Role haven't got removed in hc #44

Vento-Nuenenen opened this issue Feb 16, 2022 · 18 comments
Assignees

Comments

@Vento-Nuenenen
Copy link

Describe the bug
Our previous AL-Person still apears in the HC as an actual AL. Even though the role got removed over 1 Year ago (13.01.2021).

Expected behavior
The person should not appear as a previous role ans also not counted in the role-count.

Environment

  • Version: Version 1.2.1
  • Environment: hc.scout.ch
@tobijuon
Copy link

Thanks for your feedback!
Is it possible, that the AL has been deleted completely from MiData?
The issue of deletion is being tackled here: #43
In this case, we would have at least no problem with the former AL still seeing data, as he/she could not log in anyway.

@Vento-Nuenenen
Copy link
Author

Vento-Nuenenen commented Feb 17, 2022

No, only the roles got removed.
He also still have an active role in the layer, as a "Rover".
(Tell me, if you'd need the layer / person id, if it helps you looking into the issue)

@tobijuon
Copy link

@TaminoWalter could you look for the root of this issue?

@TaminoWalter
Copy link
Member

It would be great if I could get the id of the affected person this would help a lot for debugging and testing

@Vento-Nuenenen
Copy link
Author

Shure @TaminoWalter
I think you reuse the PBS-ID? That's 26051.
THX

@TaminoWalter
Copy link
Member

I checked the db and found the problematic entry. But when i checked the api response we use to fetch the roles of the people i couldn't find this role entry. It seems like it got hard deleted from the side of MiData. Usualy we should get a softdeleted role entry so that we can update it in our db.

@tobijuon should hard-deletes even occur? if yes, we would need to update all our import scripts because this wasn't planned / implemented for the hc application

@carlobeltrame
Copy link

@TaminoWalter The role was not hard-deleted in MiData, and it is still output normally in the MiData API. It's on page 3079 of the roles.json query.

@TaminoWalter
Copy link
Member

TaminoWalter commented Feb 18, 2022

@carlobeltrame When i called this api on page 3079 I found the roles with the ids between 111783 and 111829. The role which causes this issue is the one with id 347620 though. If this entry still exists it should be on page 9687. but exactly the id 347620 gets skipped ...619 and ...621 do exist but ...620 doesn't.

@carlobeltrame
Copy link

No, the role 347260 does not even belong to person 26051. The role you should be looking at is 111803, which is an Abteilungsleitung role in "Pfadi Abteilung Nünenen". This role was soft deleted on Dec 31st 2020 at 0:00:00CET.

@TaminoWalter
Copy link
Member

TaminoWalter commented Feb 18, 2022

Here are a few rows from our database:

id (role) group_id person_id role_id created_at deleted_at
111803 1199 26051 147 2015-12-19 00:00:00 2020-12-31 00:00:00
 347620 1199 26051 147 2021-03-05 17:18:03 NULL

As you can see the person, group and role are exactly the same. But the one with the id 347620 wasn't softdeleted and it isn't available anymore through the API

@carlobeltrame
Copy link

Hmm, looks bad. That means some time in the past, the role has been imported as a duplicate. Maybe because the person has logged into HealthCheck? Are the role ids applied correctly in HC during the OAuth flow?

@TaminoWalter
Copy link
Member

We do not import any roles in the oauth process. All database entities we have are comming straight from the MiData API.

You said that the role 347260 belongs to another person, does this mean you can still see this role? could you please provide the role informations of this role that you can see?

@Vento-Nuenenen
Copy link
Author

Vento-Nuenenen commented Feb 18, 2022

@carlobeltrame Do you know which roles are exposed in the api? I think there are only those in the "Verlauf".
Which means, we may broke it by adding the Role "AL" twice and then delete it with an "Adressverwalter". Which in this case could happen all the time, as long as the HC don't check for abadoned role-entries...

(Because the deleted roles are also removed from the "Verlauf", so no end-date is available)

@Vento-Nuenenen
Copy link
Author

Vento-Nuenenen commented Feb 18, 2022

And also i think this is only fixable by digio, by adjusting the scripts or an db manipulation. Because any user with enough permission can cause this issue. And if it happens on an kids entry, you may won't even notice, due to the amount of such.
You only see it on really small groups.

@Vento-Nuenenen
Copy link
Author

Vento-Nuenenen commented Feb 18, 2022

And I can see a matching entry (create date) inside the users log for the role 347620 ...

@carlobeltrame
Copy link

@carlobeltrame Do you know which roles are exposed in the api? I think there are only those in the "Verlauf". Which means, we may broke it by adding the Role "AL" twice and then delete it with an "Adressverwalter". Which in this case could happen all the time, as long as the HC don't check for abadoned role-entries...

(Because the deleted roles are also removed from the "Verlauf", so no end-date is available)

The Entries in the "Verlauf" are not strictly tied to the roles which are output in the API. But now that you mention it, there is indeed a mechanism in MiData which allows to hard delete a role if it's not older than 7 days. This means, if you add a duplicate Abteilungsleiter role, then let it sync to HealthCheck and then delete it from MiData within 7 days, it will be hard deleted.

Also, there is a feature in MiData which allows people with high permission to merge duplicate person entries. When merging two people, the roles of the merged person will also be hard deleted, and similar new roles will be created on the remaining person.

Both cases happen rarely, but will definitely happen again in the future.

Side note: #28 would resolve this and possible future issues of similar nature, by flushing and re-aggregating all the data on every sync.

@TaminoWalter
Copy link
Member

@tobijuon I suggest we add a new logic to the data import script, where we set the deleted at attribute to the current date of all entities that don't get returned by the api anymore.

This would be a quite simple change to the current implementation.

@vouillamoz
Copy link
Member

@PapillonAudrey ist dieses Issue noch aktuell?

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

No branches or pull requests

6 participants