Remove committee-user relation

[jsangmeister]

@emanuelschuetze said in OpenSlides/openslides-client#2210 (review) that the number of assigned accounts for committee managers could be removed. Since this is AFAIK the only use of the relation committee/user-ids, we could remove the relation altogether. This relation (specifically, the calculated fields handler for it) was the root cause for #1840. We could potentially prevent such errors in the future by removing this relation, maybe together with the meeting-user relation so we no longer have any calculated fields.

[jsangmeister]

Some permission calculation in the client is also based on this relation. It could be rewritten to use other existing permissions, but I'm not sure then if it's worth the work. Maybe we'll postpone this until another issue with the calculated fields comes up.

[jsangmeister]

Well, that did not take long: #1857

[jsangmeister]

Overview of used places in the client:

• Definitions:
  • committee/user_ids
  • user/committee_ids
• Uses of committee/user_ids:
  • ViewCommittee.memberAmount(): only for display and sorting in the committee list view
  • In the committee meeting edit view to determine whether or not the current user should be added as an admin - I'm sure we can find an alternative solution here
• Uses of user/committee_ids:
  • Display of committee badge in the account detail view (here and here): could be either cut completely or calculated on the fly - iterating over all meetings and counting the different committees should not take that much time
  • OperatorService.hasCommitteePermissions: can just be removed, because a CML implies committee membership
  • OperatorService.isInCommittees: This is used here and here for access determination.

The last function (OperatorService.isInCommittees) is the only function I could find which we'd probably have to replace. As far as I understand, a user can see a committee if he is part of it, meaning he has management rights or is part of any meeting. The last part is the relevant part for which the relation user/committee_ids is currently used. We could just subsitute it as above and calculate all committees on the fly, but I don't know how often these access functions get called and if we would run into performance problems then. Let's talk about this next week @bastianjoel @luisa-beerboom

[r-peschke]

With the next step moving to relational database model we will have these fields as view fields with a simple query without great effort. But, removing is even better!

[jsangmeister]

How the committee membership is defined is described here: https://github.com/OpenSlides/OpenSlides/wiki/Users#user-association-to-committees. This is currently done in the backend on each user.update. After the removal, the restrictor and the client would have to do this calculation on the fly, if needed. @ostcar FYI

[ostcar]

How the committee membership is defined is described here: https://github.com/OpenSlides/OpenSlides/wiki/Users#user-association-to-committees. This is currently done in the backend on each user.update. After the removal, the restrictor and the client would have to do this calculation on the fly, if needed. @ostcar FYI

Are you sure the text is still accurate? It says, that the relation is done via group membership. But I think it is

for meeting in user.meeting_ids:
  for committ in meeting.committee_id:
    ...

There are currently three places in the restricter that uses that relation.

1. One user wants to see a list of committees: https://github.com/OpenSlides/openslides-autoupdate-service/blob/2c230942831e596213c66a656c3a636f3099b501/internal/restrict/collection/committee.go#L59
2. One user wants to see a list of (other) users (he can see all users from a committee, that he manages): https://github.com/OpenSlides/openslides-autoupdate-service/blob/2c230942831e596213c66a656c3a636f3099b501/internal/restrict/collection/user.go#L119
3. Same as 2. but for another user restriction mode: https://github.com/OpenSlides/openslides-autoupdate-service/blob/2c230942831e596213c66a656c3a636f3099b501/internal/restrict/collection/user.go#L377

I think place 2 and 3 can be changed. This is only relevant for committee-managers. This should only be a few users.

But the first case will get a speed penalty for every user for all connections that request a committee if the user is in many meetings. I don't now how often a client requests a committee.

If I finish this PR, it should not be a problem anymore.

[jsangmeister]

Are you sure the text is still accurate? It says, that the relation is done via group membership. But I think it is

for meeting in user.meeting_ids:
  for committ in meeting.committee_id:
    ...

Both are accurate: The relation user/meeting_ids is calculated on the basis of group membership, so both solutions are possible. Note: The second loop does not exist, since meeting/committee_id is only a single committee. Also note that the loop might be improved because multiple meetings might belong to the same committee.

I think place 2 and 3 can be changed. This is only relevant for committee-managers. This should only be a few users.

But the first case will get a speed penalty for every user for all connections that request a committee if the user is in many meetings. I don't now how often a client requests a committee.

If I finish this PR, it should not be a problem anymore.

Sounds good. Do you have a plan for when and if that could be the case?

[ostcar]

If I finish this PR, it should not be a problem anymore.

Sounds good. Do you have a plan for when and if that could be the case?

This will take some time. I need a new implementation for the hole restricter. This will introduce new bugs. I have to discuss with @rrenkert if I should do it for 4.1 or for a later version.

We could remove the relation right now, live with the performance penalty and hope for the improvements in a later version. Or we remove the relation after the 4.1

[jsangmeister]

Sounds to me like the latter option is the better one. @rrenkert?