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

Rewrite user update to delete meeting_users if groups are emptied #2134

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion global/data/example-data.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"_migration_index": 49,
"_migration_index": 50,
"organization": {
"1": {
"id": 1,
Expand Down
2 changes: 1 addition & 1 deletion global/data/initial-data.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"_migration_index": 49,
"_migration_index": 50,
"organization": {
"1": {
"id": 1,
Expand Down
2 changes: 1 addition & 1 deletion global/meta
Submodule meta updated 1 files
+80 −40 models.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def get_history_information(self) -> HistoryInformation | None:
group_information: list[str] = []
if added and removed:
group_information.append("Groups changed")
else:
elif len(instance_group_ids) != 0:
if added:
group_information.append("Participant added to")
else:
Expand Down
15 changes: 15 additions & 0 deletions openslides_backend/action/actions/user/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
from ....models.models import User
from ....permissions.management_levels import OrganizationManagementLevel
from ....shared.exceptions import ActionException, PermissionException
from ....shared.filters import And, FilterOperator
from ....shared.patterns import fqid_from_collection_and_id
from ....shared.schema import optional_id_schema
from ...generics.update import UpdateAction
from ...mixins.send_email_mixin import EmailCheckMixin
from ...util.default_schema import DefaultSchema
from ...util.register import register_action
from ..meeting_user.delete import MeetingUserDelete
from .conditional_speaker_cascade_mixin import ConditionalSpeakerCascadeMixin
from .create_update_permissions_mixin import CreateUpdatePermissionsMixin
from .user_mixins import (
Expand Down Expand Up @@ -63,7 +65,20 @@ class UserUpdate(
check_email_field = "email"

def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
removed_meeting_id = self.get_removed_meeting_id(instance)
instance = super().update_instance(instance)
if removed_meeting_id:
meeting_users = self.datastore.filter(
"meeting_user",
And(
FilterOperator("user_id", "=", instance["id"]),
FilterOperator("meeting_id", "=", removed_meeting_id),
),
[],
)
self.execute_other_action(
MeetingUserDelete, [{"id": id_} for id_ in meeting_users]
)
user = self.datastore.get(
fqid_from_collection_and_id("user", instance["id"]),
mapped_fields=[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ class Migration(BaseModelMigration):
"""

target_migration_index = 49
fields = ["set_workflow_timestamp", "allow_motion_forwarding"]

def migrate_models(self) -> list[BaseRequestEvent] | None:
events: list[BaseRequestEvent] = []
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
from collections import defaultdict
from typing import Any, Dict, List, Optional, Tuple, TypedDict, Union

from datastore.migrations import BaseModelMigration, MigrationException
from datastore.shared.typing import JSON
from datastore.shared.util import fqid_from_collection_and_id
from datastore.writer.core import (
BaseRequestEvent,
RequestDeleteEvent,
RequestUpdateEvent,
)

from openslides_backend.shared.patterns import collection_and_id_from_fqid

FieldTargetRemoveType = Tuple[
str, Union[str, List[str]], Union[str, List["FieldTargetRemoveType"]]
]

ListUpdatesDict = Dict[str, List[Union[str, int]]]
ListFieldsData = TypedDict(
"ListFieldsData",
{"add": ListUpdatesDict, "remove": ListUpdatesDict},
total=False,
)


class Migration(BaseModelMigration):
"""
This migration deletes all meeting users that don't have group_ids, as that is what the update function will do as well from now on.
"""

target_migration_index = 50
field_target_relation_list: List[FieldTargetRemoveType] = [
("assignment_candidate_ids", "assignment_candidate", "meeting_user_id"),
("chat_message_ids", "chat_message", "meeting_user_id"),
("meeting_id", "meeting", "meeting_user_ids"),
(
"motion_submitter_ids",
"motion_submitter",
[
("meeting_id", "meeting", "motion_submitter_ids"),
("motion_id", "motion", "submitter_ids"),
],
), # CASCADE
(
"personal_note_ids",
"personal_note",
[
("content_object_id", ["motion"], "personal_note_ids"), # GENERIC
("meeting_id", "meeting", "personal_note_ids"),
],
), # CASCADE
("speaker_ids", "speaker", "meeting_user_id"),
("supported_motion_ids", "motion", "supporter_meeting_user_ids"),
("user_id", "user", "meeting_user_ids"),
("vote_delegated_to_id", "meeting_user", "vote_delegations_from_ids"),
("vote_delegations_from_ids", "meeting_user", "vote_delegated_to_id"),
]
remove_events: Dict[str, BaseRequestEvent]
additional_events: Dict[str, Tuple[Dict[str, JSON], ListFieldsData]]

def migrate_models(self) -> Optional[List[BaseRequestEvent]]:
db_models = self.reader.get_all("meeting_user")
to_be_deleted = {
id_: meeting_user
for id_, meeting_user in db_models.items()
if len(meeting_user.get("group_ids", [])) == 0
}
self.remove_events = {
f"meeting_user/{id_}": RequestDeleteEvent(
fqid_from_collection_and_id("meeting_user", id_),
)
for id_ in to_be_deleted.keys()
}
self.additional_events = defaultdict()
for id_, meeting_user in to_be_deleted.items():
self.add_relation_migration_events(
id_, meeting_user, self.field_target_relation_list
)
additional = [
RequestUpdateEvent(fqid, fields, list_fields)
for fqid, (fields, list_fields) in self.additional_events.items()
]
return [*additional, *self.remove_events.values()]

def add_relation_migration_events(
self,
remove_id: int,
remove_model: Any,
instructions: List[FieldTargetRemoveType],
) -> None:
for field, collection, to_empty in instructions:
target_ids = remove_model.get(field)
if target_ids:
if isinstance(target_ids, int) or isinstance(target_ids, str):
if isinstance(target_ids, str): # GENERICS
collection, target_ids = collection_and_id_from_fqid(target_ids)
elif not isinstance(collection, str):
raise MigrationException(
f"Couldn't resolve generic field {field}, value '{target_ids}' was not an fqid"
)
self.add_relation_migration_events_helper(
collection, target_ids, to_empty, remove_id
)
elif isinstance(target_ids, list): # list
for target_date in target_ids:
if isinstance(target_date, int) or isinstance(target_date, str):
if isinstance(target_date, str): # GENERICS
collection, target_date = collection_and_id_from_fqid(
target_date
)
elif not isinstance(collection, str):
raise MigrationException(
f"Couldn't resolve generic field {field}, value '{target_ids}' was not an fqid"
)
self.add_relation_migration_events_helper(
collection, target_date, to_empty, remove_id
)
else:
raise MigrationException(
f"Couldn't resolve field {field} as id field, value: '{target_ids}'"
)
else:
raise MigrationException(
f"Couldn't resolve field {field} as id field, value: '{target_ids}'"
)

def add_relation_migration_events_helper(
self,
collection: str,
model_id: int,
to_empty: Union[str, List["FieldTargetRemoveType"]],
remove_id: int,
) -> None:
fqid = fqid_from_collection_and_id(collection, model_id)
if not self.remove_events.get(fqid):
if isinstance(to_empty, str):
is_list = to_empty.endswith("_ids")
if prior_event := self.additional_events.get(fqid):
if is_list:
if remove := prior_event[1].get("remove"):
if prior_empty_values := remove.get(to_empty):
remove[to_empty] = list(
set([*prior_empty_values, remove_id])
)
else:
remove[to_empty] = [remove_id]
else:
prior_event[1]["remove"] = {to_empty: [remove_id]}
else:
prior_event[0][to_empty] = None
else:
if is_list:
self.additional_events[fqid] = (
{},
{"remove": {to_empty: [remove_id]}},
)
else:
self.additional_events[fqid] = ({to_empty: None}, {})
else:
if self.additional_events.get(fqid):
del self.additional_events[fqid]
self.remove_events[fqid] = RequestDeleteEvent(fqid)
model = self.reader.get(fqid)
self.add_relation_migration_events(model_id, model, to_empty)
54 changes: 43 additions & 11 deletions tests/system/action/user/test_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,19 +356,18 @@ def test_committee_manager_without_committee_ids(self) -> None:
"user/111",
{
"meeting_ids": [],
"meeting_user_ids": [1111],
"meeting_user_ids": [],
"committee_management_ids": [60, 61],
"committee_ids": [60, 61],
},
)
self.assert_model_exists(
self.assert_model_deleted(
"meeting_user/1111", {"group_ids": [], "meta_deleted": False}
)
self.assert_history_information(
"user/111",
[
"Participant removed from group {} in meeting {}",
"group/600",
"Participant removed from meeting {}",
"meeting/60",
"Personal data changed",
"Committee management changed",
Expand Down Expand Up @@ -489,7 +488,7 @@ def test_committee_manager_add_and_remove_both(self) -> None:
"committee_management_ids": [4],
"meeting_ids": [22, 33],
"committee_ids": [2, 3, 4],
"meeting_user_ids": [111, 112, 113],
"meeting_user_ids": [112, 113],
},
)
self.assert_model_exists("committee/1", {"user_ids": []})
Expand All @@ -499,6 +498,7 @@ def test_committee_manager_add_and_remove_both(self) -> None:
self.assert_model_exists("meeting/11", {"user_ids": []})
self.assert_model_exists("meeting/22", {"user_ids": [123]})
self.assert_model_exists("meeting/33", {"user_ids": [123]})
self.assert_model_deleted("meeting_user/111")

def test_update_broken_email(self) -> None:
self.create_model(
Expand Down Expand Up @@ -2092,6 +2092,7 @@ def test_group_removal_with_speaker(self) -> None:
"user_id": 1234,
"speaker_ids": [14, 24],
"group_ids": [42],
"personal_note_ids": [444],
},
"meeting_user/5555": {
"meeting_id": 5,
Expand All @@ -2102,13 +2103,24 @@ def test_group_removal_with_speaker(self) -> None:
"meeting/4": {
"is_active_in_organization_id": 1,
"meeting_user_ids": [4444],
"motion_ids": [44],
"personal_note_ids": [444],
"committee_id": 1,
},
"meeting/5": {
"is_active_in_organization_id": 1,
"meeting_user_ids": [5555],
"committee_id": 1,
},
"motion/44": {
"meeting_id": 4,
"personal_note_ids": [444],
},
"personal_note/444": {
"content_object_id": "motion/44",
"meeting_user_id": 4444,
"meeting_id": 4,
},
"committee/1": {"meeting_ids": [4, 5]},
"speaker/14": {"meeting_user_id": 4444, "meeting_id": 4},
"speaker/24": {
Expand All @@ -2130,25 +2142,45 @@ def test_group_removal_with_speaker(self) -> None:
"user/1234",
{
"username": "username_abcdefgh123",
"meeting_user_ids": [4444, 5555],
"meeting_user_ids": [5555],
},
)
self.assert_model_exists(
"meeting_user/4444",
{"group_ids": [], "speaker_ids": [24], "meta_deleted": False},
)
self.assert_model_deleted("meeting_user/4444")
self.assert_model_exists(
"meeting_user/5555",
{"group_ids": [53], "speaker_ids": [25], "meta_deleted": False},
)
self.assert_model_exists(
"speaker/24", {"meeting_user_id": 4444, "meeting_id": 4}
"speaker/24", {"meeting_user_id": None, "meeting_id": 4}
)
self.assert_model_exists(
"speaker/25", {"meeting_user_id": 5555, "meeting_id": 5}
)
self.assert_model_deleted("speaker/14")

response2 = self.request(
"user.update", {"id": 1234, "group_ids": [42], "meeting_id": 4}
)

self.assert_status_code(response2, 200)
self.assert_model_exists(
"user/1234",
{
"username": "username_abcdefgh123",
"meeting_user_ids": [5555, 5556],
},
)
self.assert_model_deleted("meeting_user/4444")
self.assert_model_deleted("personal_note/444")
self.assert_model_exists(
"meeting_user/5556",
{
"user_id": 1234,
"meeting_id": 4,
"group_ids": [42],
},
)

def test_partial_group_removal_with_speaker(self) -> None:
self.set_models(
{
Expand Down
Loading
Loading