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/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 f2b3c836d..9f5c99f89 100644 --- a/openslides_backend/action/actions/user/user_mixins.py +++ b/openslides_backend/action/actions/user/user_mixins.py @@ -142,10 +142,8 @@ 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: + 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/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..4ee49676d --- /dev/null +++ b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py @@ -0,0 +1,87 @@ +from collections import defaultdict + +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): + """ + This migration removes the presence status if the user is not part of the meeting anymore. + """ + + 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) + + 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) + + meetings = self.reader.get_many( + [ + GetManyRequestPart( + "meeting", + [meeting_id for meeting_id in present_users_per_meeting], + ["present_user_ids"], + ) + ] + ).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( + 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() + if meetings + ], + *[ + 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/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( 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..19481a8e0 --- /dev/null +++ b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py @@ -0,0 +1,93 @@ +from typing import Any + + +def create_data() -> dict[str, dict[str, Any]]: + return { + "meeting/1": { + "id": 1, + "name": "meeting name", + "present_user_ids": [1, 3, 2], + "meeting_user_ids": [3, 4, 5], + "group_ids": [10], + }, + "meeting/2": { + "id": 1, + "name": "meeting name", + "present_user_ids": [2], + "meeting_user_ids": [6], + }, + "user/1": { + "id": 1, + "username": "correct_user", + "is_present_in_meeting_ids": [1], + "meeting_user_ids": [3], + }, + "user/2": { + "id": 2, + "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": 3, + "username": "correct_user", + "is_present_in_meeting_ids": [1], + "meeting_user_ids": [4], + }, + "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}, + } + + +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/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) + + +def test_migration_one_way(write, finalize, assert_model): + data = create_data() + 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/2"]["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/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/1"]["present_user_ids"] = [1, 3] + data["meeting/2"]["present_user_ids"] = [] + + for fqid, fields in data.items(): + assert_model(fqid, fields)