-
Notifications
You must be signed in to change notification settings - Fork 32
Remove the meeting_user when removing a user from a meeting
#3206
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
Remove the meeting_user when removing a user from a meeting
#3206
Conversation
…-in-meeting-users
…-in-meeting-users
…-in-meeting-users
hjanott
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change it will be impossible to remove a user from a meeting and rejoin him with all relations to meeting user submodels set in place again because the information is lost.
In general I'm against this change unless the user_id is stored in all meeting_user related models as well. However that's also not well designed.
openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py
Outdated
Show resolved
Hide resolved
| musers = self.reader.get_all( | ||
| "meeting_user", | ||
| list(COLLECTION_TO_MIGRATION_FIELDS["meeting_user"]) + ["group_ids"], | ||
| ) | ||
| musers_to_delete = { | ||
| id_: { | ||
| field: val | ||
| for field, val in model.items() | ||
| if val and field in list(COLLECTION_TO_MIGRATION_FIELDS["meeting_user"]) | ||
| } | ||
| for id_, model in musers.items() | ||
| if not model.get("group_ids") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be more efficient to filter all meeting users where group_ids is empty list or None.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did that initially. Didn't work.
If you have an idea on how to improve
filter_ = And(
Or(
FilterOperator("group_ids", "=", None),
FilterOperator("group_ids", "=", []),
),
FilterOperator("meta_deleted", "!=", True),
)
musers_to_delete = self.reader.filter(
"meeting_user",
filter_,
list(COLLECTION_TO_MIGRATION_FIELDS["meeting_user"]),
)
so it doesn't return an empty dict every time, feel free to tell me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be an issue with psycopg 2 and/or the json formatting of our models. If you send the empty list as a string it should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
openslides_backend/migrations/migrations/0072_remove_groupless_users.py
Outdated
Show resolved
Hide resolved
openslides_backend/migrations/migrations/0072_remove_groupless_users.py
Outdated
Show resolved
Hide resolved
openslides_backend/migrations/migrations/0072_remove_groupless_users.py
Outdated
Show resolved
Hide resolved
openslides_backend/migrations/migrations/0072_remove_groupless_users.py
Outdated
Show resolved
Hide resolved
openslides_backend/migrations/migrations/0072_remove_groupless_users.py
Outdated
Show resolved
Hide resolved
|
If a participant is allowed to create motions/amendments but cannot manage motions/ meta data the backend is stopping the participant from creating the motion/amenment: Motion: Response: Amendment: response: |
hjanott
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and IMO can be merged once the docs got updated.
For documentation purposes: It was decided that we will deliberately take the information loss and irredeemably delete the connection between the user and all its remaining meeting_user submodels every time removing a user from a meeting.
| - `workflow_id`: If it is given, the motion's state is set to the workflow's first state. The workflow must be from the same meeting. If the field is not given, one of the three default (`meeting/motions_default_workflow_id` or `meeting/motions_default_amendment_workflow_id`) workflows is used depending on the type of the motion to create. | ||
| - `additional_submitter` a text field where text-based submitter information may be entered. Cannot be set unless `meeting/motions_create_enable_additional_submitter_text` is `true`. Requires permissions `Motion.CAN_CREATE` and `Motion.CAN_MANAGE_METADATA`. | ||
| - `submitter_ids`: These are **user ids** and not ids of the `motion_submitter` model. If nothing is given (`[]`) and the field `additional_submitter` isn't filled, the request user's id is used. For each id in the list a `motion_submitter` model is created. The weight must be set to the order of the given list. Requires permissions `Motion.CAN_CREATE`, `Motion.CAN_MANAGE_METADATA` and `User.CAN_SEE`. | ||
| - `submitter_meeting_user_ids`: These are ids of the meeting users that should get a `motion_submitter` model. Can be left empty. The weight of the new submitters is set to the order of the given list. Requires permissions `Motion.CAN_CREATE`, `Motion.CAN_MANAGE_METADATA` and `User.CAN_SEE` (the latter two only if not setting oneself). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name needs to also be changed in the payload description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| def test_create_no_motion_can_manage_metadata_submitter(self) -> None: | ||
| """ | ||
| Asserts that the requesting user needs at least Motion.CAN_CREATE, | ||
| Motion.CAN_MANAGE_METADATA and User.CAN_SEE when sending submitter_ids. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw this in other places too.
| Motion.CAN_MANAGE_METADATA and User.CAN_SEE when sending submitter_ids. | |
| Motion.CAN_MANAGE_METADATA and User.CAN_SEE when sending submitter_meeting_user_ids. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Closes #2111
Closes #3133
See OpenSlides/openslides-meta#347
submitter_meeting_user_idsinstead of how it was done before (as user ids via the fieldsubmitter_ids)TODO: