OpenSlides / openslides-autoupdate-service

Autoupdate service for OpenSlides 4+
MIT License
2 stars 21 forks source link

Fix not displayed structure level in LoS #866

Open Elblinator opened 7 months ago

Elblinator commented 7 months ago

Current behaviour: If a participant is only allowed to put themselves on a LoS (is not allowed to manage or see the LoS) their own structure level is not displayed

Reproduction:

  1. project something onto the autopilot
  2. navigate to the LoS
  3. change the deligates group so that they cannot see other participants on the LoS
  4. connect structure level with countdown
  5. put user A (with one structure level) on the LoS
  6. as admin you can see the structure level from the participant
  7. login as User A
  8. Navigate to the LoS
  9. you cannot see your own structure level

autoupdate from the delegate:

(2) [{…}, {…}]0: 
agenda_item: 
{17: {…}}
list_of_speakers: 
{18: {…}}
meeting: 
{2: {…}}
meeting_user: 
{6: {…}}
projection: 
{7: {…}}
projector: 
{3: {…}}
speaker: 
{22: {…}}
topic: 
{10: {…}}
user: 
{2: {…}}

autoupdate from the admin:

[autoupdate] from stream: autopilot_content_7:subscription 386092060 
(2) [{…}, {…}]
0: 
agenda_item: 
{17: {…}}
list_of_speakers: 
{18: {…}}
meeting: 
{2: {…}}
meeting_user: 
{5: {…}, 6: {…}, 7: {…}}
projection: 
{7: {…}}
projector: 
{3: {…}}
speaker: 
{18: {…}, 21: {…}, 22: {…}}
structure_level: 
{4: {…}, 5: {…}, 6: {…}}
structure_level_list_of_speakers: 
{1: {…}, 2: {…}, 3: {…}}
topic: 
{10: {…}}
user: 
{1: {…}, 2: {…}, 3: {…}}

Wanted behaviour: As a participant you should be able to see your own structure level Note: You should only be able to see the structure level countdown IF you are allowed to manage or see the LoS

ostcar commented 7 months ago

This are the current definitions, if the user can see the collections:

https://github.com/OpenSlides/openslides-autoupdate-service/blob/80c2139e4729dfe7cc0d30c44ba7606e6cc16d22/internal/restrict/collection/structure_level.go#L11-L15

https://github.com/OpenSlides/openslides-autoupdate-service/blob/80c2139e4729dfe7cc0d30c44ba7606e6cc16d22/internal/restrict/collection/structure_level_list_of_speakers.go#L11-L15

What should it be in the future? Is it correct to say, that a user can see a structure level, if he can see the corresponding meeting_user?

What about the structure_level_list_of_speakers. Currently, this depends on the permission list_of_speakers.can_see. Maybe it can be seen, if the user can see the linked list_of_speaker?

jsangmeister commented 7 months ago

Seems to be that the definition for the countdown (meaning structure_level_list_of_speakers) is already correct, that's why it was only a note by @Elblinator - feel free to correct me though ;) For the structure level, I would just add a suffix to the "can see" definition: "... OR the user has a meeting user which has the requested structure level set", or would that impact the performance too much? Because that is basically what @Elblinator wrote above.

ostcar commented 7 months ago

To refer to another collection is good for performance, because there is a cache. If it was already checked, that the request user can see user X, then it is just a cache lookup. If there is another (complicated) condition, then it could be bad for performance. I think that every time, a user requests a structe_level, he also requests the meeting_user and user.

@Elblinator only asked for the case, that the user can see himself. But I don't think, that this is enough. If the user does not have the permission to see users or list of speakers and he looks at a motion with a submitter, then he can not see the structure level of this user.

jsangmeister commented 7 months ago

You are right, if we want to keep the old structure level behaviour from when it was a simple string, this would be the logical conclusion. So the new rule would be:

The user can see a structure level if he has `list_of_speakers.can_see` OR he can see any of the meeting users from `meeting_user_ids`.

What about the structure_level_list_of_speakers. Currently, this depends on the permission list_of_speakers.can_see. Maybe it can be seen, if the user can see the linked list_of_speaker?

And I also agree with this change ;)

Elblinator commented 7 months ago

You are right, if we want to keep the old structure level behaviour from when it was a simple string, this would be the logical conclusion. So the new rule would be:

[...]

And I also agree with this change ;)

I agree!