From 7b5f725cec3a3c945d652428848a478c4bb27ebf Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Mon, 18 Nov 2024 17:47:22 +0100 Subject: [PATCH 1/4] fix error --- .../action/actions/user/user_mixins.py | 18 ++++++++++++++---- tests/system/action/user/test_update.py | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/openslides_backend/action/actions/user/user_mixins.py b/openslides_backend/action/actions/user/user_mixins.py index f2b3c836d..bb495c134 100644 --- a/openslides_backend/action/actions/user/user_mixins.py +++ b/openslides_backend/action/actions/user/user_mixins.py @@ -17,6 +17,7 @@ from ...mixins.archived_meeting_check_mixin import CheckForArchivedMeetingMixin from ...util.typing import ActionData from ..meeting_user.set_data import MeetingUserSetData +from .set_present import UserSetPresentAction class UsernameMixin(Action): @@ -142,10 +143,19 @@ def strip_field(self, field: str, instance: dict[str, Any]) -> None: def check_meeting_and_users( self, instance: dict[str, Any], user_fqid: FullQualifiedId ) -> None: - if instance.get("meeting_id") is not None: - self.datastore.apply_changed_model( - user_fqid, {"meeting_id": instance.get("meeting_id")} - ) + if (meeting_id := instance.get("meeting_id")) is not None: + if instance.get("group_ids") == []: + self.execute_other_action( + UserSetPresentAction, + [ + { + "id": instance["id"], + "meeting_id": meeting_id, + "present": False, + } + ], + ) + self.datastore.apply_changed_model(user_fqid, {"meeting_id": meeting_id}) def meeting_user_set_data(self, instance: dict[str, Any]) -> None: meeting_user_data = {} diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index 75405ef69..2e3ace2e9 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -2436,6 +2436,7 @@ def test_group_removal_with_speaker(self) -> None: "user/1234": { "username": "username_abcdefgh123", "meeting_user_ids": [4444, 5555], + "is_present_in_meeting_ids": [4, 5], }, "meeting_user/4444": { "meeting_id": 4, @@ -2453,11 +2454,13 @@ def test_group_removal_with_speaker(self) -> None: "is_active_in_organization_id": 1, "meeting_user_ids": [4444], "committee_id": 1, + "present_user_ids": [1234], }, "meeting/5": { "is_active_in_organization_id": 1, "meeting_user_ids": [5555], "committee_id": 1, + "present_user_ids": [1234], }, "committee/1": {"meeting_ids": [4, 5]}, "speaker/14": {"meeting_user_id": 4444, "meeting_id": 4}, @@ -2481,6 +2484,19 @@ def test_group_removal_with_speaker(self) -> None: { "username": "username_abcdefgh123", "meeting_user_ids": [4444, 5555], + "is_present_in_meeting_ids": [5], + }, + ) + self.assert_model_exists( + "meeting/4", + { + "present_user_ids": [], + }, + ) + self.assert_model_exists( + "meeting/5", + { + "present_user_ids": [1234], }, ) self.assert_model_exists( From 3f0a7ec0955d1fc86f7d079107e70d6c91e61ed3 Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Tue, 19 Nov 2024 15:28:26 +0100 Subject: [PATCH 2/4] write migration --- global/data/example-data.json | 2 +- global/data/initial-data.json | 2 +- .../0062_unset_presence_of_removed_users.py | 63 ++++++++++++++++ ...st_0062_unset_presence_of_removed_users.py | 75 +++++++++++++++++++ 4 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py create mode 100644 tests/system/migrations/test_0062_unset_presence_of_removed_users.py diff --git a/global/data/example-data.json b/global/data/example-data.json index 3f2550c9f..d842581ee 100644 --- a/global/data/example-data.json +++ b/global/data/example-data.json @@ -1,5 +1,5 @@ { - "_migration_index": 62, + "_migration_index": 63, "gender":{ "1":{ "id": 1, diff --git a/global/data/initial-data.json b/global/data/initial-data.json index 57df08e21..ba6880235 100644 --- a/global/data/initial-data.json +++ b/global/data/initial-data.json @@ -1,5 +1,5 @@ { - "_migration_index": 62, + "_migration_index": 63, "gender":{ "1":{ "id": 1, diff --git a/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py new file mode 100644 index 000000000..cd68c97c5 --- /dev/null +++ b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py @@ -0,0 +1,63 @@ +from collections import defaultdict + +from datastore.migrations import BaseModelMigration +from datastore.writer.core import BaseRequestEvent, RequestUpdateEvent + +from openslides_backend.shared.patterns import fqid_from_collection_and_id + + +class Migration(BaseModelMigration): + """ + This migration adds the user_id "-1" to all existing action_workers. + This is the number usually used for calls using the internal route. + """ + + target_migration_index = 63 + + def migrate_models(self) -> list[BaseRequestEvent] | None: + present_users_per_meeting: dict[int, list[int]] = defaultdict(list) + meetings_per_present_user: dict[int, list[int]] = defaultdict(list) + meetings = self.reader.get_all( + "meeting", ["group_ids", "present_user_ids", "meeting_user_ids"] + ) + users = self.reader.get_all( + "user", ["is_present_in_meeting_ids", "meeting_user_ids"] + ) + + def helper() -> None: + if not ( + (meeting_user_ids := user.get("meeting_user_ids", [])) + and any( + meeting_user_id in meeting_user_ids + for meeting_user_id in meeting.get("meeting_user_ids", []) + ) + ): + meetings_per_present_user[user_id].append(meeting_id) + present_users_per_meeting[meeting_id].append(user_id) + + for meeting_id, meeting in meetings.items(): + for user_id in meeting.get("present_user_ids", []): + user = users.get(user_id, dict()) + helper() + + for user_id, user in users.items(): + for meeting_id in user.get("is_present_in_meeting_ids", []): + helper() + + return [ + RequestUpdateEvent( + fqid_from_collection_and_id(what_collection, what_id), + { + field: [ + id_ + for id_ in lookup.get(what_id, dict()).get(field, []) + if id_ not in which_ids + ] + }, + ) + for lookup, cross_lookup, what_collection, field in [ + (meetings, present_users_per_meeting, "meeting", "present_user_ids"), + (users, meetings_per_present_user, "user", "is_present_in_meeting_ids"), + ] + for what_id, which_ids in cross_lookup.items() + ] diff --git a/tests/system/migrations/test_0062_unset_presence_of_removed_users.py b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py new file mode 100644 index 000000000..b80b7a172 --- /dev/null +++ b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py @@ -0,0 +1,75 @@ +from typing import Any + + +def create_data() -> dict[str, dict[str, Any]]: + return { + "meeting/11": { + "id": 1, + "name": "meeting name", + "present_user_ids": [1, 2], + "meeting_user_ids": [3], + }, + "meeting/111": { + "id": 1, + "name": "meeting name", + "present_user_ids": [2], + "meeting_user_ids": [4], + }, + "user/1": { + "id": 1, + "username": "wrong_user", + "is_present_in_meeting_ids": [11], + }, + "user/2": { + "id": 2, + "username": "correct_user", + "is_present_in_meeting_ids": [11, 111], + "meeting_user_ids": [3, 4], + }, + "meeting_user/3": {"id": 3, "user_id": 2, "meeting_id": 11}, + "meeting_user/4": {"id": 4, "user_id": 2, "meeting_id": 111}, + } + + +def test_migration_both_ways(write, finalize, assert_model): + data = create_data() + for fqid, fields in data.items(): + write({"type": "create", "fqid": fqid, "fields": fields}) + + finalize("0062_unset_presence_of_removed_users") + + data["meeting/11"]["present_user_ids"] = [2] + data["user/1"]["is_present_in_meeting_ids"] = [] + + for fqid, fields in data.items(): + assert_model(fqid, fields) + + +def test_migration_one_way(write, finalize, assert_model): + data = create_data() + data["meeting/11"]["present_user_ids"] = [2] + + for fqid, fields in data.items(): + write({"type": "create", "fqid": fqid, "fields": fields}) + + finalize("0062_unset_presence_of_removed_users") + + data["user/1"]["is_present_in_meeting_ids"] = [] + + for fqid, fields in data.items(): + assert_model(fqid, fields) + + +def test_migration_other_way(write, finalize, assert_model): + data = create_data() + data["user/1"]["is_present_in_meeting_ids"] = [] + + for fqid, fields in data.items(): + write({"type": "create", "fqid": fqid, "fields": fields}) + + finalize("0062_unset_presence_of_removed_users") + + data["meeting/11"]["present_user_ids"] = [2] + + for fqid, fields in data.items(): + assert_model(fqid, fields) From c389c81d2c2b3402f458fe273904c9d4ea5d1ae4 Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Thu, 21 Nov 2024 14:07:16 +0100 Subject: [PATCH 3/4] move fix to same condition place, general code improvements --- .../action/actions/meeting_user/delete.py | 4 +- .../user/conditional_speaker_cascade_mixin.py | 14 ++++ .../action/actions/user/user_mixins.py | 12 --- .../0062_unset_presence_of_removed_users.py | 73 ++++++++++++++----- .../system/action/meeting_user/test_delete.py | 18 ++++- tests/system/action/user/test_delete.py | 6 +- ...st_0062_unset_presence_of_removed_users.py | 16 +++- 7 files changed, 105 insertions(+), 38 deletions(-) diff --git a/openslides_backend/action/actions/meeting_user/delete.py b/openslides_backend/action/actions/meeting_user/delete.py index c4ac6fb58..da82a4a22 100644 --- a/openslides_backend/action/actions/meeting_user/delete.py +++ b/openslides_backend/action/actions/meeting_user/delete.py @@ -34,9 +34,11 @@ def get_history_information(self) -> HistoryInformation | None: def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: meeting_user = self.datastore.get( - fqid_from_collection_and_id("meeting_user", instance["id"]), ["speaker_ids"] + fqid_from_collection_and_id("meeting_user", instance["id"]), + ["speaker_ids", "user_id", "meeting_id"], ) speaker_ids = meeting_user.get("speaker_ids", []) self.conditionally_delete_speakers(speaker_ids) + self.remove_presence(meeting_user["user_id"], meeting_user["meeting_id"]) return super().update_instance(instance) 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 35ef5f1bb..77fede861 100644 --- a/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py +++ b/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py @@ -6,6 +6,7 @@ from ....shared.patterns import fqid_from_collection_and_id from ...action import Action from ..speaker.delete import SpeakerDeleteAction +from .set_present import UserSetPresentAction class ConditionalSpeakerCascadeMixinHelper(Action): @@ -41,6 +42,18 @@ def conditionally_delete_speakers(self, speaker_ids: list[int]) -> None: [{"id": speaker["id"]} for speaker in speakers_to_delete], ) + def remove_presence(self, user_id: int, meeting_id: int) -> None: + self.execute_other_action( + UserSetPresentAction, + [ + { + "id": user_id, + "meeting_id": meeting_id, + "present": False, + } + ], + ) + class ConditionalSpeakerCascadeMixin(ConditionalSpeakerCascadeMixinHelper): """ @@ -65,6 +78,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: for speaker_id in val.get("speaker_ids", []) ] self.conditionally_delete_speakers(speaker_ids) + self.remove_presence(instance["id"], removed_meeting_id) return super().update_instance(instance) diff --git a/openslides_backend/action/actions/user/user_mixins.py b/openslides_backend/action/actions/user/user_mixins.py index bb495c134..9f5c99f89 100644 --- a/openslides_backend/action/actions/user/user_mixins.py +++ b/openslides_backend/action/actions/user/user_mixins.py @@ -17,7 +17,6 @@ from ...mixins.archived_meeting_check_mixin import CheckForArchivedMeetingMixin from ...util.typing import ActionData from ..meeting_user.set_data import MeetingUserSetData -from .set_present import UserSetPresentAction class UsernameMixin(Action): @@ -144,17 +143,6 @@ def check_meeting_and_users( self, instance: dict[str, Any], user_fqid: FullQualifiedId ) -> None: if (meeting_id := instance.get("meeting_id")) is not None: - if instance.get("group_ids") == []: - self.execute_other_action( - UserSetPresentAction, - [ - { - "id": instance["id"], - "meeting_id": meeting_id, - "present": False, - } - ], - ) self.datastore.apply_changed_model(user_fqid, {"meeting_id": meeting_id}) def meeting_user_set_data(self, instance: dict[str, Any]) -> None: diff --git a/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py index cd68c97c5..2a8a3e895 100644 --- a/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py +++ b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py @@ -1,4 +1,5 @@ from collections import defaultdict +from typing import Any from datastore.migrations import BaseModelMigration from datastore.writer.core import BaseRequestEvent, RequestUpdateEvent @@ -8,8 +9,7 @@ class Migration(BaseModelMigration): """ - This migration adds the user_id "-1" to all existing action_workers. - This is the number usually used for calls using the internal route. + This migration removes the presence status if the user is not part of the meeting anymore. """ target_migration_index = 63 @@ -24,7 +24,13 @@ def migrate_models(self) -> list[BaseRequestEvent] | None: "user", ["is_present_in_meeting_ids", "meeting_user_ids"] ) - def helper() -> None: + def collect_removals( + meeting_id: int, + user_id: int, + user: dict[str, Any], + meetings_per_present_user: dict[int, list[int]], + present_users_per_meeting: dict[int, list[int]], + ) -> None: if not ( (meeting_user_ids := user.get("meeting_user_ids", [])) and any( @@ -38,26 +44,53 @@ def helper() -> None: for meeting_id, meeting in meetings.items(): for user_id in meeting.get("present_user_ids", []): user = users.get(user_id, dict()) - helper() + collect_removals( + meeting_id, + user_id, + user, + meetings_per_present_user, + present_users_per_meeting, + ) for user_id, user in users.items(): for meeting_id in user.get("is_present_in_meeting_ids", []): - helper() + collect_removals( + meeting_id, + user_id, + user, + meetings_per_present_user, + present_users_per_meeting, + ) return [ - RequestUpdateEvent( - fqid_from_collection_and_id(what_collection, what_id), - { - field: [ - id_ - for id_ in lookup.get(what_id, dict()).get(field, []) - if id_ not in which_ids - ] - }, - ) - for lookup, cross_lookup, what_collection, field in [ - (meetings, present_users_per_meeting, "meeting", "present_user_ids"), - (users, meetings_per_present_user, "user", "is_present_in_meeting_ids"), - ] - for what_id, which_ids in cross_lookup.items() + *[ + RequestUpdateEvent( + fqid_from_collection_and_id("meeting", meeting_id), + { + "present_user_ids": [ + id_ + for id_ in meetings.get(meeting_id, dict()).get( + "present_user_ids", [] + ) + if id_ not in user_ids + ] + }, + ) + for meeting_id, user_ids in present_users_per_meeting.items() + ], + *[ + RequestUpdateEvent( + fqid_from_collection_and_id("user", user_id), + { + "is_present_in_meeting_ids": [ + id_ + for id_ in users.get(user_id, dict()).get( + "is_present_in_meeting_ids", [] + ) + if id_ not in meeting_ids + ] + }, + ) + for user_id, meeting_ids in meetings_per_present_user.items() + ], ] diff --git a/tests/system/action/meeting_user/test_delete.py b/tests/system/action/meeting_user/test_delete.py index d2e50b2fa..54d9841ac 100644 --- a/tests/system/action/meeting_user/test_delete.py +++ b/tests/system/action/meeting_user/test_delete.py @@ -16,7 +16,18 @@ def test_delete(self) -> None: def test_delete_with_speaker(self) -> None: self.set_models( { - "meeting/10": {"is_active_in_organization_id": 1}, + "meeting/10": { + "is_active_in_organization_id": 1, + "present_user_ids": [1], + }, + "meeting/101": { + "is_active_in_organization_id": 1, + "present_user_ids": [1], + }, + "user/1": { + "is_present_in_meeting_ids": [10, 101], + "meeting_user_ids": [5], + }, "meeting_user/5": { "user_id": 1, "meeting_id": 10, @@ -38,6 +49,11 @@ def test_delete_with_speaker(self) -> None: self.assert_model_deleted("meeting_user/5") self.assert_model_deleted("speaker/1") self.assert_model_exists("speaker/2", {"meeting_id": 10, "begin_time": 123456}) + self.assert_model_exists( + "user/1", {"is_present_in_meeting_ids": [101], "meeting_user_ids": []} + ) + self.assert_model_exists("meeting/10", {"present_user_ids": []}) + self.assert_model_exists("meeting/101", {"present_user_ids": [1]}) def test_delete_with_chat_message(self) -> None: self.set_models( diff --git a/tests/system/action/user/test_delete.py b/tests/system/action/user/test_delete.py index a21ceac96..890ff3a31 100644 --- a/tests/system/action/user/test_delete.py +++ b/tests/system/action/user/test_delete.py @@ -74,13 +74,14 @@ def test_delete_with_speaker(self) -> None: "user/111": { "username": "username_srtgb123", "meeting_user_ids": [1111], + "is_present_in_meeting_ids": [1], }, "meeting_user/1111": { "meeting_id": 1, "user_id": 111, "speaker_ids": [15, 16], }, - "meeting/1": {}, + "meeting/1": {"present_user_ids": [111]}, "speaker/15": { "meeting_user_id": 1111, "meeting_id": 1, @@ -95,13 +96,14 @@ def test_delete_with_speaker(self) -> None: response = self.request("user.delete", {"id": 111}) self.assert_status_code(response, 200) - self.assert_model_deleted("user/111") + self.assert_model_deleted("user/111", {"is_present_in_meeting_ids": []}) self.assert_model_deleted("meeting_user/1111") self.assert_model_exists( "speaker/15", {"meeting_user_id": None, "meeting_id": 1, "begin_time": 12345678}, ) self.assert_model_deleted("speaker/16") + self.assert_model_exists("meeting/1", {"present_user_ids": []}) def test_delete_with_candidate(self) -> None: self.set_models( diff --git a/tests/system/migrations/test_0062_unset_presence_of_removed_users.py b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py index b80b7a172..1228d3b20 100644 --- a/tests/system/migrations/test_0062_unset_presence_of_removed_users.py +++ b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py @@ -6,13 +6,13 @@ def create_data() -> dict[str, dict[str, Any]]: "meeting/11": { "id": 1, "name": "meeting name", - "present_user_ids": [1, 2], + "present_user_ids": [1, 2, 3], "meeting_user_ids": [3], }, "meeting/111": { "id": 1, "name": "meeting name", - "present_user_ids": [2], + "present_user_ids": [2, 3], "meeting_user_ids": [4], }, "user/1": { @@ -26,6 +26,12 @@ def create_data() -> dict[str, dict[str, Any]]: "is_present_in_meeting_ids": [11, 111], "meeting_user_ids": [3, 4], }, + "user/3": { + "id": 2, + "username": "correct_user", + "is_present_in_meeting_ids": [11, 111], + "meeting_user_ids": [], + }, "meeting_user/3": {"id": 3, "user_id": 2, "meeting_id": 11}, "meeting_user/4": {"id": 4, "user_id": 2, "meeting_id": 111}, } @@ -39,7 +45,9 @@ def test_migration_both_ways(write, finalize, assert_model): finalize("0062_unset_presence_of_removed_users") data["meeting/11"]["present_user_ids"] = [2] + data["meeting/111"]["present_user_ids"] = [2] data["user/1"]["is_present_in_meeting_ids"] = [] + data["user/3"]["is_present_in_meeting_ids"] = [] for fqid, fields in data.items(): assert_model(fqid, fields) @@ -55,6 +63,8 @@ def test_migration_one_way(write, finalize, assert_model): finalize("0062_unset_presence_of_removed_users") data["user/1"]["is_present_in_meeting_ids"] = [] + data["meeting/111"]["present_user_ids"] = [2] + data["user/3"]["is_present_in_meeting_ids"] = [] for fqid, fields in data.items(): assert_model(fqid, fields) @@ -70,6 +80,8 @@ def test_migration_other_way(write, finalize, assert_model): finalize("0062_unset_presence_of_removed_users") data["meeting/11"]["present_user_ids"] = [2] + data["meeting/111"]["present_user_ids"] = [2] + data["user/3"]["is_present_in_meeting_ids"] = [] for fqid, fields in data.items(): assert_model(fqid, fields) From 95362095685ab7e67b948d8ef50a5b41c05691cf Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Fri, 22 Nov 2024 09:58:25 +0100 Subject: [PATCH 4/4] refactor migration --- .../0062_unset_presence_of_removed_users.py | 75 ++++++++----------- ...st_0062_unset_presence_of_removed_users.py | 62 ++++++++------- 2 files changed, 67 insertions(+), 70 deletions(-) diff --git a/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py index 2a8a3e895..4ee49676d 100644 --- a/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py +++ b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py @@ -1,11 +1,13 @@ from collections import defaultdict -from typing import Any from datastore.migrations import BaseModelMigration +from datastore.reader.core import GetManyRequestPart from datastore.writer.core import BaseRequestEvent, RequestUpdateEvent from openslides_backend.shared.patterns import fqid_from_collection_and_id +from ...shared.filters import And, FilterOperator + class Migration(BaseModelMigration): """ @@ -17,51 +19,39 @@ class Migration(BaseModelMigration): def migrate_models(self) -> list[BaseRequestEvent] | None: present_users_per_meeting: dict[int, list[int]] = defaultdict(list) meetings_per_present_user: dict[int, list[int]] = defaultdict(list) - meetings = self.reader.get_all( - "meeting", ["group_ids", "present_user_ids", "meeting_user_ids"] - ) - users = self.reader.get_all( - "user", ["is_present_in_meeting_ids", "meeting_user_ids"] - ) - def collect_removals( - meeting_id: int, - user_id: int, - user: dict[str, Any], - meetings_per_present_user: dict[int, list[int]], - present_users_per_meeting: dict[int, list[int]], - ) -> None: - if not ( - (meeting_user_ids := user.get("meeting_user_ids", [])) - and any( - meeting_user_id in meeting_user_ids - for meeting_user_id in meeting.get("meeting_user_ids", []) - ) - ): - meetings_per_present_user[user_id].append(meeting_id) - present_users_per_meeting[meeting_id].append(user_id) + meeting_users = self.reader.filter( + "meeting_user", + And( + FilterOperator("group_ids", "=", "[]"), + FilterOperator("meta_deleted", "!=", True), + ), + ["user_id", "meeting_id"], + ) + for meeting_user in meeting_users.values(): + user_id = meeting_user["user_id"] + meeting_id = meeting_user["meeting_id"] + meetings_per_present_user[user_id].append(meeting_id) + present_users_per_meeting[meeting_id].append(user_id) - for meeting_id, meeting in meetings.items(): - for user_id in meeting.get("present_user_ids", []): - user = users.get(user_id, dict()) - collect_removals( - meeting_id, - user_id, - user, - meetings_per_present_user, - present_users_per_meeting, + meetings = self.reader.get_many( + [ + GetManyRequestPart( + "meeting", + [meeting_id for meeting_id in present_users_per_meeting], + ["present_user_ids"], ) - - for user_id, user in users.items(): - for meeting_id in user.get("is_present_in_meeting_ids", []): - collect_removals( - meeting_id, - user_id, - user, - meetings_per_present_user, - present_users_per_meeting, + ] + ).get("meeting", dict()) + users = self.reader.get_many( + [ + GetManyRequestPart( + "user", + [present_user_id for present_user_id in meetings_per_present_user], + ["is_present_in_meeting_ids"], ) - + ] + ).get("user", dict()) return [ *[ RequestUpdateEvent( @@ -77,6 +67,7 @@ def collect_removals( }, ) for meeting_id, user_ids in present_users_per_meeting.items() + if meetings ], *[ RequestUpdateEvent( diff --git a/tests/system/migrations/test_0062_unset_presence_of_removed_users.py b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py index 1228d3b20..19481a8e0 100644 --- a/tests/system/migrations/test_0062_unset_presence_of_removed_users.py +++ b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py @@ -3,37 +3,46 @@ def create_data() -> dict[str, dict[str, Any]]: return { - "meeting/11": { + "meeting/1": { "id": 1, "name": "meeting name", - "present_user_ids": [1, 2, 3], - "meeting_user_ids": [3], + "present_user_ids": [1, 3, 2], + "meeting_user_ids": [3, 4, 5], + "group_ids": [10], }, - "meeting/111": { + "meeting/2": { "id": 1, "name": "meeting name", - "present_user_ids": [2, 3], - "meeting_user_ids": [4], + "present_user_ids": [2], + "meeting_user_ids": [6], }, "user/1": { "id": 1, - "username": "wrong_user", - "is_present_in_meeting_ids": [11], + "username": "correct_user", + "is_present_in_meeting_ids": [1], + "meeting_user_ids": [3], }, "user/2": { "id": 2, - "username": "correct_user", - "is_present_in_meeting_ids": [11, 111], - "meeting_user_ids": [3, 4], + "username": "wrong_user", + "is_present_in_meeting_ids": [2, 1], + # meeting users do exist after user left meeting but have no group ids + "meeting_user_ids": [ + 5, + 6, + ], }, "user/3": { - "id": 2, + "id": 3, "username": "correct_user", - "is_present_in_meeting_ids": [11, 111], - "meeting_user_ids": [], + "is_present_in_meeting_ids": [1], + "meeting_user_ids": [4], }, - "meeting_user/3": {"id": 3, "user_id": 2, "meeting_id": 11}, - "meeting_user/4": {"id": 4, "user_id": 2, "meeting_id": 111}, + "meeting_user/3": {"id": 3, "user_id": 1, "meeting_id": 1, "group_ids": [10]}, + "meeting_user/4": {"id": 4, "user_id": 3, "meeting_id": 1, "group_ids": [10]}, + "meeting_user/5": {"id": 5, "user_id": 2, "meeting_id": 1, "group_ids": []}, + "meeting_user/6": {"id": 6, "user_id": 2, "meeting_id": 2, "group_ids": []}, + "group/10": {"id": 10, "meeting_user_ids": [3, 4], "meeting_id": 1}, } @@ -44,10 +53,9 @@ def test_migration_both_ways(write, finalize, assert_model): finalize("0062_unset_presence_of_removed_users") - data["meeting/11"]["present_user_ids"] = [2] - data["meeting/111"]["present_user_ids"] = [2] - data["user/1"]["is_present_in_meeting_ids"] = [] - data["user/3"]["is_present_in_meeting_ids"] = [] + data["meeting/1"]["present_user_ids"] = [1, 3] + data["meeting/2"]["present_user_ids"] = [] + data["user/2"]["is_present_in_meeting_ids"] = [] for fqid, fields in data.items(): assert_model(fqid, fields) @@ -55,16 +63,15 @@ def test_migration_both_ways(write, finalize, assert_model): def test_migration_one_way(write, finalize, assert_model): data = create_data() - data["meeting/11"]["present_user_ids"] = [2] + data["meeting/1"]["present_user_ids"] = [1, 3] + data["meeting/2"]["present_user_ids"] = [] for fqid, fields in data.items(): write({"type": "create", "fqid": fqid, "fields": fields}) finalize("0062_unset_presence_of_removed_users") - data["user/1"]["is_present_in_meeting_ids"] = [] - data["meeting/111"]["present_user_ids"] = [2] - data["user/3"]["is_present_in_meeting_ids"] = [] + data["user/2"]["is_present_in_meeting_ids"] = [] for fqid, fields in data.items(): assert_model(fqid, fields) @@ -72,16 +79,15 @@ def test_migration_one_way(write, finalize, assert_model): def test_migration_other_way(write, finalize, assert_model): data = create_data() - data["user/1"]["is_present_in_meeting_ids"] = [] + data["user/2"]["is_present_in_meeting_ids"] = [] for fqid, fields in data.items(): write({"type": "create", "fqid": fqid, "fields": fields}) finalize("0062_unset_presence_of_removed_users") - data["meeting/11"]["present_user_ids"] = [2] - data["meeting/111"]["present_user_ids"] = [2] - data["user/3"]["is_present_in_meeting_ids"] = [] + data["meeting/1"]["present_user_ids"] = [1, 3] + data["meeting/2"]["present_user_ids"] = [] for fqid, fields in data.items(): assert_model(fqid, fields)