From 8cba807a237722a1f3c25b4ad091c1bc97577948 Mon Sep 17 00:00:00 2001 From: Luisa Date: Wed, 17 Jan 2024 17:48:52 +0100 Subject: [PATCH 1/5] Rewrite user update to delete meeting_users if groups are emptied --- .../actions/user/conditional_speaker_cascade_mixin.py | 7 +++++++ openslides_backend/action/actions/user/update.py | 8 +++++++- tests/system/action/user/test_update.py | 6 +++--- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py b/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py index f4d3ea538..e5ee86f60 100644 --- a/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py +++ b/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py @@ -5,6 +5,7 @@ from ....services.datastore.commands import GetManyRequest from ...action import Action from ..speaker.delete import SpeakerDeleteAction +from ..meeting_user.delete import MeetingUserDelete class ConditionalSpeakerCascadeMixin(Action): @@ -52,6 +53,12 @@ def update_instance(self, instance: Dict[str, Any]) -> Dict[str, Any]: SpeakerDeleteAction, [{"id": speaker["id"]} for speaker in speakers_to_delete], ) + + # if len(meeting_users): + # self.execute_other_action( + # MeetingUserDelete, + # [{"id": id_} for id_ in meeting_users] + # ) return super().update_instance(instance) diff --git a/openslides_backend/action/actions/user/update.py b/openslides_backend/action/actions/user/update.py index 54dc864a7..702ed9cd9 100644 --- a/openslides_backend/action/actions/user/update.py +++ b/openslides_backend/action/actions/user/update.py @@ -18,6 +18,8 @@ UserMixin, check_gender_helper, ) +from ....shared.filters import And, FilterOperator +from ..meeting_user.delete import MeetingUserDelete @register_action("user.update") @@ -63,14 +65,18 @@ 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=[ "is_active", "organization_management_level", "saml_id", - "password", + "password" ], ) if user.get("saml_id") and ( diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index 9b6860c88..5815ae971 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -356,13 +356,13 @@ 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( - "meeting_user/1111", {"group_ids": [], "meta_deleted": False} + self.assert_model_deleted( + "meeting_user/1111", {"group_ids": [600], "meta_deleted": False} ) self.assert_history_information( "user/111", From e7977a3c9febafd015d9a697a45a1c8383f7d672 Mon Sep 17 00:00:00 2001 From: Luisa Date: Thu, 18 Jan 2024 17:57:31 +0100 Subject: [PATCH 2/5] Finish function and start writing migration --- .../actions/meeting_user/history_mixin.py | 2 +- .../user/conditional_speaker_cascade_mixin.py | 7 - .../action/actions/user/update.py | 19 +- .../0048_fix_broken_meeting_clones.py | 1 - .../0049_delete_removed_meeting_users.py | 25 +++ tests/system/action/user/test_update.py | 39 +++- ...test_0049_delete_removed_meeting_clones.py | 184 ++++++++++++++++++ 7 files changed, 253 insertions(+), 24 deletions(-) create mode 100644 openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py create mode 100644 tests/system/migrations/test_0049_delete_removed_meeting_clones.py diff --git a/openslides_backend/action/actions/meeting_user/history_mixin.py b/openslides_backend/action/actions/meeting_user/history_mixin.py index e135814fb..488fb1dc2 100644 --- a/openslides_backend/action/actions/meeting_user/history_mixin.py +++ b/openslides_backend/action/actions/meeting_user/history_mixin.py @@ -62,7 +62,7 @@ def get_history_information(self) -> Optional[HistoryInformation]: 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: diff --git a/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py b/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py index e5ee86f60..f4d3ea538 100644 --- a/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py +++ b/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py @@ -5,7 +5,6 @@ from ....services.datastore.commands import GetManyRequest from ...action import Action from ..speaker.delete import SpeakerDeleteAction -from ..meeting_user.delete import MeetingUserDelete class ConditionalSpeakerCascadeMixin(Action): @@ -53,12 +52,6 @@ def update_instance(self, instance: Dict[str, Any]) -> Dict[str, Any]: SpeakerDeleteAction, [{"id": speaker["id"]} for speaker in speakers_to_delete], ) - - # if len(meeting_users): - # self.execute_other_action( - # MeetingUserDelete, - # [{"id": id_} for id_ in meeting_users] - # ) return super().update_instance(instance) diff --git a/openslides_backend/action/actions/user/update.py b/openslides_backend/action/actions/user/update.py index 702ed9cd9..14f9c1953 100644 --- a/openslides_backend/action/actions/user/update.py +++ b/openslides_backend/action/actions/user/update.py @@ -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 ( @@ -18,8 +20,6 @@ UserMixin, check_gender_helper, ) -from ....shared.filters import And, FilterOperator -from ..meeting_user.delete import MeetingUserDelete @register_action("user.update") @@ -68,15 +68,24 @@ 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]) + 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=[ "is_active", "organization_management_level", "saml_id", - "password" + "password", ], ) if user.get("saml_id") and ( diff --git a/openslides_backend/migrations/migrations/0048_fix_broken_meeting_clones.py b/openslides_backend/migrations/migrations/0048_fix_broken_meeting_clones.py index 93b97f624..7f47cf2c6 100644 --- a/openslides_backend/migrations/migrations/0048_fix_broken_meeting_clones.py +++ b/openslides_backend/migrations/migrations/0048_fix_broken_meeting_clones.py @@ -12,7 +12,6 @@ class Migration(BaseModelMigration): """ target_migration_index = 49 - fields = ["set_workflow_timestamp", "allow_motion_forwarding"] def migrate_models(self) -> Optional[List[BaseRequestEvent]]: events: List[BaseRequestEvent] = [] diff --git a/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py b/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py new file mode 100644 index 000000000..a4431090e --- /dev/null +++ b/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py @@ -0,0 +1,25 @@ +from typing import List, Optional + +from datastore.migrations import BaseModelMigration +from datastore.shared.util import fqid_from_collection_and_id +from datastore.writer.core import BaseRequestEvent, RequestDeleteEvent + + +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 + + def migrate_models(self) -> Optional[List[BaseRequestEvent]]: + db_models = self.reader.get_all("meeting_user") + # to_be_deleted = [id_ for id_, meeting_user in db_models.items() if len(meeting_user.get("group_ids", [])) == 0] + events: List[BaseRequestEvent] = [ + RequestDeleteEvent( + fqid_from_collection_and_id("meeting_user", id_), + ) + for id_, meeting_user in db_models.items() + if len(meeting_user.get("group_ids", [])) == 0 + ] + return events diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index 5815ae971..2600e0321 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -362,13 +362,12 @@ def test_committee_manager_without_committee_ids(self) -> None: }, ) self.assert_model_deleted( - "meeting_user/1111", {"group_ids": [600], "meta_deleted": False} + "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", @@ -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": []}) @@ -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( @@ -2130,25 +2130,44 @@ 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_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( { diff --git a/tests/system/migrations/test_0049_delete_removed_meeting_clones.py b/tests/system/migrations/test_0049_delete_removed_meeting_clones.py new file mode 100644 index 000000000..8d78c6a2b --- /dev/null +++ b/tests/system/migrations/test_0049_delete_removed_meeting_clones.py @@ -0,0 +1,184 @@ +def test_migration(write, finalize, assert_model): + write( + { + "type": "create", + "fqid": "meeting/1", + "fields": { + "id": 1, + "group_ids": [1], + "meeting_user_ids": [101, 201], + }, + }, + { + "type": "create", + "fqid": "meeting/2", + "fields": { + "id": 2, + "group_ids": [2], + "meeting_user_ids": [102, 202], + }, + }, + { + "type": "create", + "fqid": "group/1", + "fields": { + "id": 1, + "meeting_id": 1, + "meeting_user_ids": [101], + }, + }, + { + "type": "create", + "fqid": "group/2", + "fields": { + "id": 2, + "meeting_id": 2, + "meeting_user_ids": [], + }, + }, + { + "type": "create", + "fqid": "user/10", + "fields": { + "id": 10, + "meeting_user_ids": [101, 102], + }, + }, + { + "type": "create", + "fqid": "user/20", + "fields": { + "id": 20, + "meeting_user_ids": [201, 202], + }, + }, + { + "type": "create", + "fqid": "meeting_user/101", + "fields": { + "id": 102, + "meeting_id": 1, + "group_ids": [1], + "user_id": 10, + }, + }, + { + "type": "create", + "fqid": "meeting_user/102", + "fields": { + "id": 102, + "meeting_id": 2, + "group_ids": [], + "user_id": 10, + }, + }, + { + "type": "create", + "fqid": "meeting_user/201", + "fields": { + "id": 201, + "meeting_id": 1, + "group_ids": [], + "user_id": 20, + }, + }, + { + "type": "create", + "fqid": "meeting_user/202", + "fields": { + "id": 202, + "meeting_id": 2, + "group_ids": [], + "user_id": 20, + }, + }, + ) + + finalize("0049_delete_removed_meeting_users") + + assert_model( + "meeting/1", + { + "id": 1, + "group_ids": [1], + "meeting_user_ids": [101], + }, + ) + assert_model( + "meeting/2", + { + "id": 2, + "group_ids": [2], + "meeting_user_ids": [], + }, + ) + assert_model( + "group/1", + { + "id": 1, + "meeting_id": 1, + "meeting_user_ids": [101], + }, + ) + assert_model( + "group/2", + { + "id": 2, + "meeting_id": 2, + "meeting_user_ids": [], + }, + ) + assert_model( + "user/10", + { + "id": 10, + "meeting_user_ids": [101], + }, + ) + assert_model( + "user/20", + { + "id": 20, + "meeting_user_ids": [], + }, + ) + assert_model( + "meeting_user/101", + { + "id": 102, + "meeting_id": 1, + "group_ids": [1], + "user_id": 10, + }, + ) + + assert_model( + "meeting_user/102", + { + "id": 102, + "meeting_id": 2, + "group_ids": [], + "user_id": 10, + "meta_deleted": True, + }, + ) + assert_model( + "meeting_user/201", + { + "id": 201, + "meeting_id": 1, + "group_ids": [], + "user_id": 20, + "meta_deleted": True, + }, + ) + assert_model( + "meeting_user/202", + { + "id": 202, + "meeting_id": 2, + "group_ids": [], + "user_id": 20, + "meta_deleted": True, + }, + ) From d5139cf5de0f1fa256379e6a1d9e2137693bcd99 Mon Sep 17 00:00:00 2001 From: Luisa Date: Fri, 19 Jan 2024 14:44:47 +0100 Subject: [PATCH 3/5] Finish migration code --- .../0049_delete_removed_meeting_users.py | 157 +++++++++++++++++- 1 file changed, 148 insertions(+), 9 deletions(-) diff --git a/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py b/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py index a4431090e..0bfb09432 100644 --- a/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py +++ b/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py @@ -1,8 +1,27 @@ -from typing import List, Optional +from collections import defaultdict +from typing import Any, Dict, List, Optional, Tuple, TypedDict, Union -from datastore.migrations import BaseModelMigration +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 +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): @@ -11,15 +30,135 @@ class Migration(BaseModelMigration): """ target_migration_index = 50 + field_target_relation_list: List[FieldTargetRemoveType] = [ + ("user_id", "user", "meeting_user_ids"), + ("meeting_id", "meeting", "meeting_user_ids"), + ("supported_motion_ids", "motion", "supporter_meeting_user_ids"), + ( + "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"), + ( + "motion_submitter_ids", + "motion_submitter", + [ + ("motion_id", "motion", "submitter_ids"), + ("meeting_id", "meeting", "motion_submitter_ids"), + ], + ), # CASCADE + ("assignment_candidate_ids", "assignment_candidate", "meeting_user_id"), + ("vote_delegated_to_id", "meeting_user", "vote_delegations_from_ids"), + ("vote_delegations_from_ids", "meeting_user", "vote_delegated_to_id"), + ("chat_message_ids", "chat_message", "meeting_user_id"), + ("group_ids", "group", "meeting_user_ids"), + ] + 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_ for id_, meeting_user in db_models.items() if len(meeting_user.get("group_ids", [])) == 0] - events: List[BaseRequestEvent] = [ - RequestDeleteEvent( - fqid_from_collection_and_id("meeting_user", id_), - ) + 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 events + 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] = [*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) From 39024afc15caf935536940a0005b9efc4f528934 Mon Sep 17 00:00:00 2001 From: Luisa Date: Fri, 19 Jan 2024 17:02:42 +0100 Subject: [PATCH 4/5] Test the migration --- .../0049_delete_removed_meeting_users.py | 29 +- ...test_0049_delete_removed_meeting_clones.py | 509 ++++++++++++++++-- 2 files changed, 484 insertions(+), 54 deletions(-) diff --git a/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py b/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py index 0bfb09432..0e9c1d7c8 100644 --- a/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py +++ b/openslides_backend/migrations/migrations/0049_delete_removed_meeting_users.py @@ -31,9 +31,17 @@ class Migration(BaseModelMigration): target_migration_index = 50 field_target_relation_list: List[FieldTargetRemoveType] = [ - ("user_id", "user", "meeting_user_ids"), + ("assignment_candidate_ids", "assignment_candidate", "meeting_user_id"), + ("chat_message_ids", "chat_message", "meeting_user_id"), ("meeting_id", "meeting", "meeting_user_ids"), - ("supported_motion_ids", "motion", "supporter_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", @@ -43,19 +51,10 @@ class Migration(BaseModelMigration): ], ), # CASCADE ("speaker_ids", "speaker", "meeting_user_id"), - ( - "motion_submitter_ids", - "motion_submitter", - [ - ("motion_id", "motion", "submitter_ids"), - ("meeting_id", "meeting", "motion_submitter_ids"), - ], - ), # CASCADE - ("assignment_candidate_ids", "assignment_candidate", "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"), - ("chat_message_ids", "chat_message", "meeting_user_id"), - ("group_ids", "group", "meeting_user_ids"), ] remove_events: Dict[str, BaseRequestEvent] additional_events: Dict[str, Tuple[Dict[str, JSON], ListFieldsData]] @@ -141,7 +140,9 @@ def add_relation_migration_events_helper( if is_list: if remove := prior_event[1].get("remove"): if prior_empty_values := remove.get(to_empty): - remove[to_empty] = [*prior_empty_values, remove_id] + remove[to_empty] = list( + set([*prior_empty_values, remove_id]) + ) else: remove[to_empty] = [remove_id] else: diff --git a/tests/system/migrations/test_0049_delete_removed_meeting_clones.py b/tests/system/migrations/test_0049_delete_removed_meeting_clones.py index 8d78c6a2b..44d41a58f 100644 --- a/tests/system/migrations/test_0049_delete_removed_meeting_clones.py +++ b/tests/system/migrations/test_0049_delete_removed_meeting_clones.py @@ -6,7 +6,9 @@ def test_migration(write, finalize, assert_model): "fields": { "id": 1, "group_ids": [1], - "meeting_user_ids": [101, 201], + "meeting_user_ids": [101, 201, 301], + "motion_submitter_ids": [1011, 1021], + "personal_note_ids": [1011, 1021], }, }, { @@ -15,7 +17,9 @@ def test_migration(write, finalize, assert_model): "fields": { "id": 2, "group_ids": [2], - "meeting_user_ids": [102, 202], + "meeting_user_ids": [102, 202, 302], + "motion_submitter_ids": [2011, 2012], + "personal_note_ids": [2021, 2022], }, }, { @@ -24,7 +28,7 @@ def test_migration(write, finalize, assert_model): "fields": { "id": 1, "meeting_id": 1, - "meeting_user_ids": [101], + "meeting_user_ids": [101, 301], }, }, { @@ -33,7 +37,7 @@ def test_migration(write, finalize, assert_model): "fields": { "id": 2, "meeting_id": 2, - "meeting_user_ids": [], + "meeting_user_ids": [302], }, }, { @@ -52,6 +56,14 @@ def test_migration(write, finalize, assert_model): "meeting_user_ids": [201, 202], }, }, + { + "type": "create", + "fqid": "user/30", + "fields": { + "id": 30, + "meeting_user_ids": [301, 302], + }, + }, { "type": "create", "fqid": "meeting_user/101", @@ -60,6 +72,13 @@ def test_migration(write, finalize, assert_model): "meeting_id": 1, "group_ids": [1], "user_id": 10, + "assignment_candidate_ids": [1011], + "chat_message_ids": [1011], + "motion_submitter_ids": [1011], + "personal_note_ids": [1011], + "speaker_ids": [1011], + "supported_motion_ids": [1], + "vote_delegations_from_ids": [201, 301], }, }, { @@ -70,6 +89,10 @@ def test_migration(write, finalize, assert_model): "meeting_id": 2, "group_ids": [], "user_id": 10, + "motion_submitter_ids": [1021], + "personal_note_ids": [1021], + "supported_motion_ids": [1, 11], + "vote_delegations_from_ids": [202, 302], }, }, { @@ -80,6 +103,11 @@ def test_migration(write, finalize, assert_model): "meeting_id": 1, "group_ids": [], "user_id": 20, + "assignment_candidate_ids": [2011, 2012], + "chat_message_ids": [2011, 2012], + "motion_submitter_ids": [2011, 2012], + "supported_motion_ids": [2, 22], + "vote_delegated_to_id": 101, }, }, { @@ -90,18 +118,418 @@ def test_migration(write, finalize, assert_model): "meeting_id": 2, "group_ids": [], "user_id": 20, + "personal_note_ids": [2021, 2022], + "speaker_ids": [2021, 2022], + "supported_motion_ids": [22], + "vote_delegated_to_id": 102, + }, + }, + { + "type": "create", + "fqid": "meeting_user/301", + "fields": { + "id": 301, + "meeting_id": 1, + "group_ids": [1], + "user_id": 30, + "vote_delegated_to_id": 101, + }, + }, + { + "type": "create", + "fqid": "meeting_user/302", + "fields": { + "id": 302, + "meeting_id": 2, + "group_ids": [2], + "user_id": 30, + "vote_delegated_to_id": 102, + }, + }, + { + "type": "create", + "fqid": "assignment_candidate/1011", + "fields": {"id": 1011, "meeting_user_id": 101}, + }, + { + "type": "create", + "fqid": "assignment_candidate/2011", + "fields": {"id": 2011, "meeting_user_id": 201}, + }, + { + "type": "create", + "fqid": "assignment_candidate/2012", + "fields": {"id": 2012, "meeting_user_id": 201}, + }, + { + "type": "create", + "fqid": "chat_message/1011", + "fields": {"id": 1011, "meeting_user_id": 101}, + }, + { + "type": "create", + "fqid": "chat_message/2011", + "fields": {"id": 2011, "meeting_user_id": 201}, + }, + { + "type": "create", + "fqid": "chat_message/2012", + "fields": {"id": 2012, "meeting_user_id": 201}, + }, + { + "type": "create", + "fqid": "motion/1", + "fields": { + "id": 1, + "submitter_ids": [1011, 1021], + "personal_note_ids": [1011, 1021], + "supporter_meeting_user_ids": [101, 102], + }, + }, + { + "type": "create", + "fqid": "motion/2", + "fields": { + "id": 2, + "submitter_ids": [2011, 2012], + "personal_note_ids": [2021, 2022], + "supporter_meeting_user_ids": [201], + }, + }, + { + "type": "create", + "fqid": "motion/11", + "fields": { + "id": 11, + "supporter_meeting_user_ids": [102], + }, + }, + { + "type": "create", + "fqid": "motion/22", + "fields": { + "id": 22, + "supporter_meeting_user_ids": [201, 202], + }, + }, + { + "type": "create", + "fqid": "motion_submitter/1011", + "fields": { + "id": 1011, + "meeting_user_id": 101, + "meeting_id": 1, + "motion_id": 1, + }, + }, + { + "type": "create", + "fqid": "motion_submitter/1021", + "fields": { + "id": 1021, + "meeting_user_id": 102, + "meeting_id": 1, + "motion_id": 1, + }, + }, + { + "type": "create", + "fqid": "motion_submitter/2011", + "fields": { + "id": 2011, + "meeting_user_id": 201, + "meeting_id": 2, + "motion_id": 2, + }, + }, + { + "type": "create", + "fqid": "motion_submitter/2012", + "fields": { + "id": 2012, + "meeting_user_id": 201, + "meeting_id": 2, + "motion_id": 2, + }, + }, + { + "type": "create", + "fqid": "personal_note/1011", + "fields": { + "id": 1011, + "meeting_user_id": 101, + "meeting_id": 1, + "content_object_id": "motion/1", + }, + }, + { + "type": "create", + "fqid": "personal_note/1021", + "fields": { + "id": 1021, + "meeting_user_id": 102, + "meeting_id": 1, + "content_object_id": "motion/1", + }, + }, + { + "type": "create", + "fqid": "personal_note/2021", + "fields": { + "id": 2021, + "meeting_user_id": 202, + "meeting_id": 2, + "content_object_id": "motion/2", + }, + }, + { + "type": "create", + "fqid": "personal_note/2022", + "fields": { + "id": 2022, + "meeting_user_id": 202, + "meeting_id": 2, + "content_object_id": "motion/2", + }, + }, + { + "type": "create", + "fqid": "speaker/1011", + "fields": { + "id": 1011, + "meeting_user_id": 101, + }, + }, + { + "type": "create", + "fqid": "speaker/2021", + "fields": { + "id": 2021, + "meeting_user_id": 202, + }, + }, + { + "type": "create", + "fqid": "speaker/2022", + "fields": { + "id": 2022, + "meeting_user_id": 202, }, }, ) finalize("0049_delete_removed_meeting_users") + assert_model( + "meeting_user/102", + { + "id": 102, + "meeting_id": 2, + "group_ids": [], + "user_id": 10, + "motion_submitter_ids": [1021], + "personal_note_ids": [1021], + "supported_motion_ids": [1, 11], + "vote_delegations_from_ids": [202, 302], + "meta_deleted": True, + }, + ) + assert_model( + "meeting_user/201", + { + "id": 201, + "meeting_id": 1, + "group_ids": [], + "user_id": 20, + "assignment_candidate_ids": [2011, 2012], + "chat_message_ids": [2011, 2012], + "motion_submitter_ids": [2011, 2012], + "supported_motion_ids": [2, 22], + "vote_delegated_to_id": 101, + "meta_deleted": True, + }, + ) + assert_model( + "meeting_user/202", + { + "id": 202, + "meeting_id": 2, + "group_ids": [], + "user_id": 20, + "personal_note_ids": [2021, 2022], + "speaker_ids": [2021, 2022], + "supported_motion_ids": [22], + "meta_deleted": True, + "vote_delegated_to_id": 102, + }, + ) + + assert_model( + "meeting_user/101", + { + "id": 102, + "meeting_id": 1, + "group_ids": [1], + "user_id": 10, + "assignment_candidate_ids": [1011], + "chat_message_ids": [1011], + "motion_submitter_ids": [1011], + "personal_note_ids": [1011], + "speaker_ids": [1011], + "supported_motion_ids": [1], + "vote_delegations_from_ids": [301], + }, + ) + assert_model( + "meeting_user/301", + { + "id": 301, + "meeting_id": 1, + "group_ids": [1], + "user_id": 30, + "vote_delegated_to_id": 101, + }, + ) + assert_model( + "meeting_user/302", + { + "id": 302, + "meeting_id": 2, + "group_ids": [2], + "user_id": 30, + }, + ) + assert_model("assignment_candidate/1011", {"id": 1011, "meeting_user_id": 101}) + assert_model( + "assignment_candidate/2011", + { + "id": 2011, + }, + ) + assert_model( + "assignment_candidate/2012", + { + "id": 2012, + }, + ) + assert_model("chat_message/1011", {"id": 1011, "meeting_user_id": 101}) + assert_model( + "chat_message/2011", + { + "id": 2011, + }, + ) + assert_model( + "chat_message/2012", + { + "id": 2012, + }, + ) + assert_model( + "motion_submitter/1011", + { + "id": 1011, + "meeting_user_id": 101, + "meeting_id": 1, + "motion_id": 1, + }, + ) + assert_model( + "motion_submitter/1021", + { + "id": 1021, + "meeting_user_id": 102, + "meeting_id": 1, + "motion_id": 1, + "meta_deleted": True, + }, + ) + assert_model( + "motion_submitter/2011", + { + "id": 2011, + "meeting_user_id": 201, + "meeting_id": 2, + "motion_id": 2, + "meta_deleted": True, + }, + ) + assert_model( + "motion_submitter/2012", + { + "id": 2012, + "meeting_user_id": 201, + "meeting_id": 2, + "motion_id": 2, + "meta_deleted": True, + }, + ) + assert_model( + "personal_note/1011", + { + "id": 1011, + "meeting_user_id": 101, + "meeting_id": 1, + "content_object_id": "motion/1", + }, + ) + assert_model( + "personal_note/1021", + { + "id": 1021, + "meeting_user_id": 102, + "meeting_id": 1, + "content_object_id": "motion/1", + "meta_deleted": True, + }, + ) + assert_model( + "personal_note/2021", + { + "id": 2021, + "meeting_user_id": 202, + "meeting_id": 2, + "content_object_id": "motion/2", + "meta_deleted": True, + }, + ) + assert_model( + "personal_note/2022", + { + "id": 2022, + "meeting_user_id": 202, + "meeting_id": 2, + "content_object_id": "motion/2", + "meta_deleted": True, + }, + ) + assert_model( + "speaker/1011", + { + "id": 1011, + "meeting_user_id": 101, + }, + ) + assert_model( + "speaker/2021", + { + "id": 2021, + }, + ) + assert_model( + "speaker/2022", + { + "id": 2022, + }, + ) assert_model( "meeting/1", { "id": 1, "group_ids": [1], - "meeting_user_ids": [101], + "meeting_user_ids": [101, 301], + "motion_submitter_ids": [1011], + "personal_note_ids": [1011], }, ) assert_model( @@ -109,76 +537,77 @@ def test_migration(write, finalize, assert_model): { "id": 2, "group_ids": [2], - "meeting_user_ids": [], + "meeting_user_ids": [302], + "motion_submitter_ids": [], + "personal_note_ids": [], }, ) assert_model( - "group/1", + "motion/1", { "id": 1, - "meeting_id": 1, - "meeting_user_ids": [101], + "submitter_ids": [1011], + "personal_note_ids": [1011], + "supporter_meeting_user_ids": [101], }, ) assert_model( - "group/2", + "motion/2", { "id": 2, - "meeting_id": 2, - "meeting_user_ids": [], + "submitter_ids": [], + "personal_note_ids": [], + "supporter_meeting_user_ids": [], }, ) assert_model( - "user/10", + "motion/11", { - "id": 10, - "meeting_user_ids": [101], + "id": 11, + "supporter_meeting_user_ids": [], }, ) assert_model( - "user/20", + "motion/22", { - "id": 20, - "meeting_user_ids": [], + "id": 22, + "supporter_meeting_user_ids": [], }, ) assert_model( - "meeting_user/101", + "group/1", { - "id": 102, + "id": 1, "meeting_id": 1, - "group_ids": [1], - "user_id": 10, + "meeting_user_ids": [101, 301], }, ) - assert_model( - "meeting_user/102", + "group/2", { - "id": 102, + "id": 2, "meeting_id": 2, - "group_ids": [], - "user_id": 10, - "meta_deleted": True, + "meeting_user_ids": [302], }, ) assert_model( - "meeting_user/201", + "user/10", { - "id": 201, - "meeting_id": 1, - "group_ids": [], - "user_id": 20, - "meta_deleted": True, + "id": 10, + "meeting_user_ids": [101], }, ) assert_model( - "meeting_user/202", + "user/20", { - "id": 202, - "meeting_id": 2, - "group_ids": [], - "user_id": 20, - "meta_deleted": True, + "id": 20, + "meeting_user_ids": [], + }, + ) + assert_model( + "user/30", + { + "id": 30, + "meeting_user_ids": [301, 302], }, ) From b42ddf40d9f6fc1fa86ea97f9665a680fcd81aed Mon Sep 17 00:00:00 2001 From: Luisa Date: Mon, 22 Jan 2024 17:20:51 +0100 Subject: [PATCH 5/5] Fix migration related tests and add update test --- global/data/example-data.json | 2 +- global/data/initial-data.json | 2 +- tests/system/action/user/test_update.py | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/global/data/example-data.json b/global/data/example-data.json index 7d2024fe7..48abde0e0 100644 --- a/global/data/example-data.json +++ b/global/data/example-data.json @@ -1,5 +1,5 @@ { - "_migration_index": 49, + "_migration_index": 50, "organization": { "1": { "id": 1, diff --git a/global/data/initial-data.json b/global/data/initial-data.json index 3b2dae120..9ac2ffc52 100644 --- a/global/data/initial-data.json +++ b/global/data/initial-data.json @@ -1,5 +1,5 @@ { - "_migration_index": 49, + "_migration_index": 50, "organization": { "1": { "id": 1, diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index 2600e0321..7472be877 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -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, @@ -2102,6 +2103,8 @@ 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": { @@ -2109,6 +2112,15 @@ def test_group_removal_with_speaker(self) -> None: "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": { @@ -2159,6 +2171,7 @@ def test_group_removal_with_speaker(self) -> None: }, ) self.assert_model_deleted("meeting_user/4444") + self.assert_model_deleted("personal_note/444") self.assert_model_exists( "meeting_user/5556", {