diff --git a/meta b/meta index ec166875b6..d16e66eb6a 160000 --- a/meta +++ b/meta @@ -1 +1 @@ -Subproject commit ec166875b618c19810cec43e934284f026efb9ed +Subproject commit d16e66eb6a061264f5c799df79556e9efe7643d6 diff --git a/openslides_backend/action/action.py b/openslides_backend/action/action.py index 2e32659e16..63a6d2f59d 100644 --- a/openslides_backend/action/action.py +++ b/openslides_backend/action/action.py @@ -9,6 +9,7 @@ from psycopg.types.json import Jsonb from openslides_backend.shared.base_service_provider import BaseServiceProvider +from openslides_backend.shared.history_events import update_history_information_multi from ..models.base import Model, model_registry from ..models.fields import BaseRelationField, GenericRelationField @@ -540,7 +541,7 @@ def get_history_information(self) -> HistoryInformation | None: if self.history_information is None: return None - information = {} + information: HistoryInformation = {} instances = ( self.get_instances_with_fields(["id", self.history_relation_field]) if self.history_relation_field @@ -558,8 +559,9 @@ def get_history_information(self) -> HistoryInformation | None: fqids.append( fqid_from_collection_and_id(self.model.collection, instance["id"]) ) - for fqid in fqids: - information[fqid] = [self.history_information] + update_history_information_multi( + information, fqids, [self.history_information] + ) return information def get_instances_with_fields( @@ -777,7 +779,9 @@ def merge_history_informations( b = {} for fqid, information in b.items(): if fqid in a: - a[fqid].extend(information) + a[fqid]["entries"].extend(information["entries"]) + if changed_fields := information.get("changed_fields"): + a[fqid].setdefault("changed_fields", dict()).update(changed_fields) else: a[fqid] = information return a diff --git a/openslides_backend/action/actions/meeting_user/base_delete.py b/openslides_backend/action/actions/meeting_user/base_delete.py index 171deb21c0..da1b06eac5 100644 --- a/openslides_backend/action/actions/meeting_user/base_delete.py +++ b/openslides_backend/action/actions/meeting_user/base_delete.py @@ -1,3 +1,4 @@ +from openslides_backend.shared.history_events import build_history_information_data from openslides_backend.shared.patterns import fqid_from_collection_and_id from openslides_backend.shared.typing import HistoryInformation @@ -17,9 +18,13 @@ class MeetingUserBaseDelete(DeleteAction): def get_history_information(self) -> HistoryInformation | None: users = self.get_instances_with_fields(["user_id", "meeting_id"]) return { - fqid_from_collection_and_id("user", user["user_id"]): [ - "Participant removed from meeting {}", - fqid_from_collection_and_id("meeting", user["meeting_id"]), - ] + fqid_from_collection_and_id( + "user", user["user_id"] + ): build_history_information_data( + [ + "Participant removed from meeting {}", + fqid_from_collection_and_id("meeting", user["meeting_id"]), + ], + ) for user in users } diff --git a/openslides_backend/action/actions/meeting_user/create.py b/openslides_backend/action/actions/meeting_user/create.py index f71b2edf6c..07168f1471 100644 --- a/openslides_backend/action/actions/meeting_user/create.py +++ b/openslides_backend/action/actions/meeting_user/create.py @@ -1,6 +1,7 @@ from typing import Any from openslides_backend.shared.exceptions import ActionException +from openslides_backend.shared.history_events import update_history_information from openslides_backend.shared.patterns import fqid_from_collection_and_id from openslides_backend.shared.typing import HistoryInformation @@ -51,9 +52,9 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: return super().update_instance(instance) def get_history_information(self) -> HistoryInformation | None: - information = {} + information: HistoryInformation = {} for instance in self.instances: - instance_information = [] + entries = [] fqids_per_collection = { collection_name: [ fqid_from_collection_and_id( @@ -65,15 +66,17 @@ def get_history_information(self) -> HistoryInformation | None: for collection_name in ["group", "structure_level"] if (ids := instance.get(f"{collection_name}_ids")) } - instance_information.append( + entries.append( self.compose_history_string(list(fqids_per_collection.items())) ) for collection_name, fqids in fqids_per_collection.items(): - instance_information.extend(fqids) - instance_information.append( + entries.extend(fqids) + entries.append( fqid_from_collection_and_id("meeting", instance["meeting_id"]), ) - information[fqid_from_collection_and_id("user", instance["user_id"])] = ( - instance_information + update_history_information( + information, + fqid_from_collection_and_id("user", instance["user_id"]), + entries, ) return information diff --git a/openslides_backend/action/actions/meeting_user/history_mixin.py b/openslides_backend/action/actions/meeting_user/history_mixin.py index 76a33e3263..134c8e0e92 100644 --- a/openslides_backend/action/actions/meeting_user/history_mixin.py +++ b/openslides_backend/action/actions/meeting_user/history_mixin.py @@ -1,16 +1,26 @@ from collections.abc import Iterable from copy import deepcopy -from typing import Any +from typing import Any, NotRequired, TypedDict from openslides_backend.action.mixins.extend_history_mixin import ExtendHistoryMixin +from openslides_backend.shared.filters import FilterOperator +from openslides_backend.shared.history_events import build_history_information_data from openslides_backend.shared.interfaces.event import Event, EventType from ....services.database.interface import GetManyRequest -from ....shared.patterns import fqid_from_collection_and_id +from ....shared.patterns import Field, FullQualifiedId, fqid_from_collection_and_id from ....shared.typing import HistoryInformation from ...action import Action +class ActionHistoryInformationData(TypedDict): + entries: list[tuple[str, ...]] + changed_fields: NotRequired[dict[Field, Any]] + + +ActionHistoryInformation = dict[FullQualifiedId, ActionHistoryInformationData] + + class MeetingUserHistoryMixin(ExtendHistoryMixin, Action): extend_history_to = "user_id" @@ -76,7 +86,7 @@ def create_events(self, instance: dict[str, Any]) -> Iterable[Event]: ) def get_history_information(self) -> HistoryInformation | None: - information: dict[str, list[tuple[str, ...]]] = {} + information: ActionHistoryInformation = {} # Scan the instances and collect the info for the history information # Copy instances first since they are modified @@ -97,7 +107,10 @@ def get_history_information(self) -> HistoryInformation | None: ) return { - fqid: [string for entry in history for string in entry] + fqid: build_history_information_data( + [string for entry in history.get("entries", []) for string in entry], + history.get("changed_fields", {}), + ) for fqid, history in information.items() } @@ -105,9 +118,10 @@ def add_updated_meeting_user_history_information( self, instance: dict[str, Any], db_instance: dict[str, Any], - information: dict[str, list[tuple[str, ...]]], + information: ActionHistoryInformation, ) -> None: - instance_information: list[tuple[str, ...]] = [] + instance_entries: list[tuple[str, ...]] = [] + instance_changed_fields: dict[str, list[int]] = {} user_id = db_instance["user_id"] meeting_id = db_instance["meeting_id"] @@ -120,25 +134,33 @@ def add_updated_meeting_user_history_information( # meeting specific data update_fields = ["structure_level_ids", "number", "vote_weight"] if any(field in instance for field in update_fields): - instance_information.append( + instance_entries.append( ( "Participant data updated in meeting {}", fqid_from_collection_and_id("meeting", meeting_id), ) ) - self.handle_group_updates(instance_information, instance, db_instance) - self.handle_delegations( - information, instance_information, instance, db_instance + self.handle_group_updates( + instance_entries, + instance_changed_fields, + instance, + db_instance, ) + self.handle_delegations(information, instance_entries, instance, db_instance) - if instance_information: + if instance_entries or instance_changed_fields: self.add_entries_to_history_information( - information, instance_information, for_user_id=user_id + information, + instance_entries, + for_user_id=user_id, + changed_fields=instance_changed_fields, ) def add_created_meeting_user_history_information( - self, instance: dict[str, Any], information: dict[str, list[tuple[str, ...]]] + self, + instance: dict[str, Any], + information: ActionHistoryInformation, ) -> None: db_instance = self.datastore.get( fqid_from_collection_and_id(self.model.collection, instance["id"]), @@ -175,15 +197,21 @@ def add_created_meeting_user_history_information( if instance_information: self.add_entries_to_history_information( - information, instance_information, for_user_id=db_instance["user_id"] + information, + instance_information, + for_user_id=db_instance["user_id"], + changed_fields={ + "group_ids": self.get_changed_group_ids(db_instance["user_id"]) + }, ) def add_entries_to_history_information( self, - information: dict[str, list[tuple[str, ...]]], + information: ActionHistoryInformation, entries: list[tuple[str, ...]], for_user_id: int | None = None, for_meeting_user_id: int | None = None, + changed_fields: dict[Field, Any] | None = None, ) -> None: if not for_user_id: if not for_meeting_user_id: @@ -197,11 +225,17 @@ def add_entries_to_history_information( user_id = for_user_id fqid = fqid_from_collection_and_id("user", user_id) if fqid not in information: - information[fqid] = entries + information[fqid] = {"entries": entries} + if changed_fields: + information[fqid]["changed_fields"] = changed_fields else: for entry in entries: - if entry not in information[fqid]: - information[fqid].append(entry) + if entry not in information[fqid]["entries"]: + information[fqid]["entries"].append(entry) + if changed_fields: + information[fqid].setdefault("changed_fields", dict()).update( + changed_fields + ) def compose_history_string( self, fqids_per_collection: list[tuple[str, list[str]]] @@ -231,7 +265,8 @@ def compose_history_string( def handle_group_updates( self, - instance_information: list[tuple[str, ...]], + entries: list[tuple[str, ...]], + changed_fields: dict[str, list[int]], instance: dict[str, Any], db_instance: dict[str, Any], ) -> None: @@ -242,6 +277,11 @@ def handle_group_updates( added = instance_group_ids - db_group_ids removed = db_group_ids - instance_group_ids + if added or removed: + changed_fields["group_ids"] = self.get_changed_group_ids( + db_instance["user_id"], {meeting_id: instance["group_ids"]} + ) + # remove default groups meeting = self.datastore.get( fqid_from_collection_and_id("meeting", meeting_id), @@ -273,11 +313,11 @@ def handle_group_updates( fqid_from_collection_and_id("meeting", meeting_id) ) if group_information: - instance_information.append(tuple(group_information)) + entries.append(tuple(group_information)) def handle_delegations( self, - information: dict[str, list[tuple[str, ...]]], + information: ActionHistoryInformation, instance_information: list[tuple[str, ...]], instance: dict[str, Any], db_instance: dict[str, Any], @@ -447,3 +487,22 @@ def handle_delegations( ], for_meeting_user_id=muser_id, ) + + def get_changed_group_ids( + self, user_id: int, update_data: dict[int, list[int]] = {} + ) -> list[int]: + db_groups: dict[int, dict[str, Any]] = self.datastore.filter( + "meeting_user", + FilterOperator("user_id", "=", user_id), + ["meeting_id", "group_ids"], + lock_result=False, + ) + changed_groups = [ + ( + updated_groups + if (updated_groups := update_data.get(db_data["meeting_id"])) + else db_data["group_ids"] + ) + for db_data in db_groups.values() + ] + return [id_ for group_ids in changed_groups for id_ in group_ids] diff --git a/openslides_backend/action/actions/motion/base_create_forwarded.py b/openslides_backend/action/actions/motion/base_create_forwarded.py index c5a808bad7..6d675e7c0d 100644 --- a/openslides_backend/action/actions/motion/base_create_forwarded.py +++ b/openslides_backend/action/actions/motion/base_create_forwarded.py @@ -6,6 +6,7 @@ from psycopg.types.json import Jsonb from openslides_backend.action.actions.motion.mixins import TextHashMixin +from openslides_backend.shared.history_events import update_history_information from openslides_backend.shared.typing import HistoryInformation from ....i18n.translator import Translator @@ -641,22 +642,23 @@ def forward_mediafiles( return instance def get_history_information(self) -> HistoryInformation | None: - forwarded_entries = defaultdict(list) + information: HistoryInformation = {} for instance in self.instances: - forwarded_entries[ - fqid_from_collection_and_id("motion", instance["origin_id"]) - ].extend( + update_history_information( + information, + fqid_from_collection_and_id("motion", instance["origin_id"]), [ "Forwarded to {}", fqid_from_collection_and_id("meeting", instance["meeting_id"]), - ] + ], ) - return forwarded_entries | { - fqid_from_collection_and_id("motion", instance["id"]): [ - "Motion created (forwarded)" - ] - for instance in self.instances - } + for instance in self.instances: + update_history_information( + information, + fqid_from_collection_and_id("motion", instance["id"]), + ["Motion created (forwarded)"], + ) + return information def check_can_forward_with_attachments(self) -> None: organization = self.datastore.get( diff --git a/openslides_backend/action/actions/motion/delete.py b/openslides_backend/action/actions/motion/delete.py index fd1f9883c5..0b8e18d7fd 100644 --- a/openslides_backend/action/actions/motion/delete.py +++ b/openslides_backend/action/actions/motion/delete.py @@ -2,6 +2,10 @@ from typing import Any from openslides_backend.action.action import merge_history_informations +from openslides_backend.shared.history_events import ( + build_history_information_data, + update_history_information_multi, +) from openslides_backend.shared.typing import HistoryInformation from ....models.models import Motion @@ -76,14 +80,16 @@ def get_history_information(self) -> HistoryInformation | None: if self.history_information is None: return information # generate the history informations for the deleted amendments - fqids = [ + if not information: + information = {} + all_fqids = [ fqid_from_collection_and_id("motion", id_) for id_ in self.all_motion_ids ] - if not information: - information = {fqid: [self.history_information] for fqid in fqids} - else: - for fqid in fqids: - information[fqid] = [self.history_information] + update_history_information_multi( + information, + [fqid for fqid in all_fqids if fqid not in information], + [self.history_information], + ) return information def get_full_history_information(self) -> HistoryInformation | None: @@ -97,12 +103,20 @@ def get_full_history_information(self) -> HistoryInformation | None: return merge_history_informations( information or {}, { - fqid_from_collection_and_id("motion", id): ["Forwarded motion deleted"] + fqid_from_collection_and_id( + "motion", id + ): build_history_information_data( + ["Forwarded motion deleted"], + ) for instance in instances for id in instance.get("all_origin_ids", []) }, { - fqid_from_collection_and_id("motion", id): ["Origin motion deleted"] + fqid_from_collection_and_id( + "motion", id + ): build_history_information_data( + ["Origin motion deleted"], + ) for instance in instances for id in instance.get("all_derived_motion_ids", []) }, diff --git a/openslides_backend/action/actions/motion/motion_state_history_information_mixin.py b/openslides_backend/action/actions/motion/motion_state_history_information_mixin.py index 11ea727bcb..5273101619 100644 --- a/openslides_backend/action/actions/motion/motion_state_history_information_mixin.py +++ b/openslides_backend/action/actions/motion/motion_state_history_information_mixin.py @@ -1,3 +1,4 @@ +from openslides_backend.shared.history_events import build_history_information_data from openslides_backend.shared.patterns import fqid_from_collection_and_id from openslides_backend.shared.typing import HistoryInformation @@ -9,9 +10,15 @@ def _get_state_history_information( self, instance_field: str, verbose_model: str ) -> HistoryInformation: return { - fqid_from_collection_and_id(self.model.collection, instance["id"]): [ - verbose_model + " set to {}", - fqid_from_collection_and_id("motion_state", instance[instance_field]), - ] + fqid_from_collection_and_id( + self.model.collection, instance["id"] + ): build_history_information_data( + [ + verbose_model + " set to {}", + fqid_from_collection_and_id( + "motion_state", instance[instance_field] + ), + ], + ) for instance in self.instances } diff --git a/openslides_backend/action/actions/motion/update.py b/openslides_backend/action/actions/motion/update.py index b5884a759a..27bde7caa2 100644 --- a/openslides_backend/action/actions/motion/update.py +++ b/openslides_backend/action/actions/motion/update.py @@ -4,6 +4,7 @@ from psycopg.types.json import Jsonb +from openslides_backend.shared.history_events import update_history_information from openslides_backend.shared.typing import HistoryInformation from ....models.models import Motion @@ -223,19 +224,17 @@ def check_permissions(self, instance: dict[str, Any]) -> None: raise PermissionDenied(msg) def get_history_information(self) -> HistoryInformation | None: - information = {} + information: HistoryInformation = {} for instance in deepcopy(self.instances): - instance_information = [] + entries = [] # workflow timestamp changed if "workflow_timestamp" in instance: timestamp = instance.pop("workflow_timestamp") - instance_information.extend( - ["Workflow_timestamp set to {}", f"{timestamp}"] - ) + entries.extend(["Workflow_timestamp set to {}", f"{timestamp}"]) # category changed - instance_information.extend( + entries.extend( self.create_history_information_for_field( instance, "category_id", @@ -245,7 +244,7 @@ def get_history_information(self) -> HistoryInformation | None: ) # block changed - instance_information.extend( + entries.extend( self.create_history_information_for_field( instance, "block_id", "motion_block", "Motion block" ) @@ -263,12 +262,14 @@ def get_history_information(self) -> HistoryInformation | None: ] if any(field in instance for field in generic_update_fields): # still other fields given, so we also add the generic "updated" message - instance_information.append("Motion updated") + entries.append("Motion updated") - if instance_information: - information[ - fqid_from_collection_and_id(self.model.collection, instance["id"]) - ] = instance_information + if entries: + update_history_information( + information, + fqid_from_collection_and_id(self.model.collection, instance["id"]), + entries, + ) return information diff --git a/openslides_backend/action/actions/motion_comment/create_delete_update.py b/openslides_backend/action/actions/motion_comment/create_delete_update.py index eb23817b89..4141659144 100644 --- a/openslides_backend/action/actions/motion_comment/create_delete_update.py +++ b/openslides_backend/action/actions/motion_comment/create_delete_update.py @@ -2,6 +2,7 @@ from openslides_backend.action.mixins.extend_history_mixin import ExtendHistoryMixin from openslides_backend.permissions.management_levels import OrganizationManagementLevel +from openslides_backend.shared.history_events import build_history_information_data from openslides_backend.shared.typing import HistoryInformation from ....models.models import MotionComment @@ -103,12 +104,16 @@ def get_history_information(self) -> HistoryInformation | None: instances = self.get_instances_with_fields(["motion_id", "section_id"]) _, action = self.name.split(".") return { - fqid_from_collection_and_id("motion", instance["motion_id"]): [ - "Comment {} " + action + "d", - fqid_from_collection_and_id( - "motion_comment_section", instance["section_id"] - ), - ] + fqid_from_collection_and_id( + "motion", instance["motion_id"] + ): build_history_information_data( + [ + "Comment {} " + action + "d", + fqid_from_collection_and_id( + "motion_comment_section", instance["section_id"] + ), + ], + ) for instance in instances } diff --git a/openslides_backend/action/actions/user/merge_mixins.py b/openslides_backend/action/actions/user/merge_mixins.py index e47fce259e..ee260e07bb 100644 --- a/openslides_backend/action/actions/user/merge_mixins.py +++ b/openslides_backend/action/actions/user/merge_mixins.py @@ -1,6 +1,7 @@ from typing import Any from openslides_backend.services.database.interface import PartialModel +from openslides_backend.shared.history_events import update_history_information from ....models.models import ( AssignmentCandidate, @@ -99,9 +100,11 @@ def get_full_history_information(self) -> HistoryInformation | None: ]: assignment_ids.add(data["assignment_id"]) for assignment_id in assignment_ids: - information[fqid_from_collection_and_id("assignment", assignment_id)] = [ - "Candidates merged" - ] + update_history_information( + information, + fqid_from_collection_and_id("assignment", assignment_id), + ["Candidates merged"], + ) return information @@ -128,11 +131,11 @@ def get_full_history_information(self) -> HistoryInformation | None: ]: motion_ids.add(data["motion_id"]) for motion_id in motion_ids: - fqid = fqid_from_collection_and_id("motion", motion_id) - if fqid not in information: - information[fqid] = ["Submitters merged"] - else: - information[fqid].append("Submitters merged") + update_history_information( + information, + fqid_from_collection_and_id("motion", motion_id), + ["Submitters merged"], + ) return information @@ -155,11 +158,11 @@ def get_full_history_information(self) -> HistoryInformation | None: ]: motion_ids.add(data["motion_id"]) for motion_id in motion_ids: - fqid = fqid_from_collection_and_id("motion", motion_id) - if fqid not in information: - information[fqid] = ["Supporters merged"] - else: - information[fqid].append("Supporters merged") + update_history_information( + information, + fqid_from_collection_and_id("motion", motion_id), + ["Supporters merged"], + ) return information diff --git a/openslides_backend/action/actions/user/merge_together.py b/openslides_backend/action/actions/user/merge_together.py index 5f690b0235..2dbe1e5e4f 100644 --- a/openslides_backend/action/actions/user/merge_together.py +++ b/openslides_backend/action/actions/user/merge_together.py @@ -1,6 +1,10 @@ from typing import Any from openslides_backend.services.database.interface import PartialModel +from openslides_backend.shared.history_events import ( + update_history_information, + update_history_information_multi, +) from ....action.mixins.archived_meeting_check_mixin import CheckForArchivedMeetingMixin from ....models.models import User @@ -449,12 +453,17 @@ def get_full_history_information(self) -> HistoryInformation | None: deleted_string = " and ".join( ["{}" for i in range(len(deleted_fqids))] ) - information[main_fqid] = [ - "Updated with data from " + deleted_string, - *deleted_fqids, - ] - for deleted_fqid in deleted_fqids: - information[deleted_fqid] = ["Merged into {}", main_fqid] + update_history_information( + information, + main_fqid, + [ + "Updated with data from " + deleted_string, + *deleted_fqids, + ], + ) + update_history_information_multi( + information, deleted_fqids, ["Merged into {}", main_fqid] + ) else: raise BadCodingException("No id found for user history generation") return information diff --git a/openslides_backend/action/actions/user/set_present.py b/openslides_backend/action/actions/user/set_present.py index d9ea8650bb..33473fa54c 100644 --- a/openslides_backend/action/actions/user/set_present.py +++ b/openslides_backend/action/actions/user/set_present.py @@ -1,5 +1,6 @@ from typing import Any +from openslides_backend.shared.history_events import build_history_information_data from openslides_backend.shared.typing import HistoryInformation from ....action.mixins.archived_meeting_check_mixin import CheckForArchivedMeetingMixin @@ -40,6 +41,7 @@ def get_updated_instances(self, action_data: ActionData) -> ActionData: add meeting_id if present is True. remove meeting_id if present is False. """ + self.base_history_information = {} for instance in action_data: meeting_id = instance.pop("meeting_id") present = instance.pop("present") @@ -47,17 +49,28 @@ def get_updated_instances(self, action_data: ActionData) -> ActionData: fqid_from_collection_and_id(self.model.collection, instance["id"]), ["is_present_in_meeting_ids"], ) + self.base_history_information[instance["id"]] = { + "present": present, + "meeting_id": meeting_id, + } if present: if meeting_id not in user.get("is_present_in_meeting_ids", []): - instance["is_present_in_meeting_ids"] = user.get( - "is_present_in_meeting_ids", [] - ) + [meeting_id] + is_present = user.get("is_present_in_meeting_ids", []) + [ + meeting_id + ] + instance["is_present_in_meeting_ids"] = is_present + self.base_history_information[instance["id"]][ + "is_present_in_meeting_ids" + ] = is_present yield instance elif present is False: is_present = user.get("is_present_in_meeting_ids", []) if meeting_id in is_present: is_present.remove(meeting_id) instance["is_present_in_meeting_ids"] = is_present + self.base_history_information[instance["id"]][ + "is_present_in_meeting_ids" + ] = is_present yield instance def check_permissions(self, instance: dict[str, Any]) -> None: @@ -94,9 +107,14 @@ def check_permissions(self, instance: dict[str, Any]) -> None: def get_history_information(self) -> HistoryInformation | None: return { - fqid_from_collection_and_id(self.model.collection, instance["id"]): [ - f"Set {'not ' if not instance['present'] else ''}present in meeting {{}}", - fqid_from_collection_and_id("meeting", instance["meeting_id"]), - ] - for instance in self.action_data + fqid_from_collection_and_id( + self.model.collection, id_ + ): build_history_information_data( + [ + f"Set {'not ' if not data['present'] else ''}present in meeting {{}}", + fqid_from_collection_and_id("meeting", data["meeting_id"]), + ], + {"is_present_in_meeting_ids": data["is_present_in_meeting_ids"]}, + ) + for id_, data in self.base_history_information.items() } diff --git a/openslides_backend/action/actions/user/user_mixins.py b/openslides_backend/action/actions/user/user_mixins.py index 5d0b652383..68870246c4 100644 --- a/openslides_backend/action/actions/user/user_mixins.py +++ b/openslides_backend/action/actions/user/user_mixins.py @@ -4,6 +4,7 @@ from openslides_backend.services.database.commands import GetManyRequest from openslides_backend.services.database.interface import PartialModel +from openslides_backend.shared.history_events import update_history_information from openslides_backend.shared.typing import HistoryInformation from openslides_backend.shared.util import ONE_ORGANIZATION_FQID @@ -154,12 +155,12 @@ def meeting_user_set_data(self, instance: dict[str, Any]) -> None: class UpdateHistoryMixin(Action): def get_history_information(self) -> HistoryInformation | None: - information = {} + information: HistoryInformation = {} # Scan the instances and collect the info for the history information # Copy instances first since they are modified for instance in deepcopy(self.instances): - instance_information = [] + entries = [] # Fetch the current instance from the db to diff with the given instance db_instance = self.datastore.get( @@ -190,22 +191,24 @@ def get_history_information(self) -> HistoryInformation | None: "default_vote_weight", ] if any(field in instance for field in update_fields): - instance_information.append("Personal data changed") + entries.append("Personal data changed") # other fields if "organization_management_level" in instance: - instance_information.append("Organization Management Level changed") + entries.append("Organization Management Level changed") if "committee_management_ids" in instance: - instance_information.append("Committee management changed") + entries.append("Committee management changed") if "is_active" in instance: if instance["is_active"]: - instance_information.append("Set active") + entries.append("Set active") else: - instance_information.append("Set inactive") + entries.append("Set inactive") - if instance_information: - information[fqid_from_collection_and_id("user", instance["id"])] = ( - instance_information + if entries: + update_history_information( + information, + fqid_from_collection_and_id("user", instance["id"]), + entries, ) return information diff --git a/openslides_backend/models/models.py b/openslides_backend/models/models.py index bbcfe6c07b..5cf7155e92 100644 --- a/openslides_backend/models/models.py +++ b/openslides_backend/models/models.py @@ -535,6 +535,7 @@ class HistoryEntry(Model): id = fields.IntegerField(required=True, constant=True) entries = fields.TextArrayField() + changed_fields = fields.JSONField() original_model_id = fields.CharField(constant=True) model_id = fields.GenericRelationField( to={ diff --git a/openslides_backend/shared/history_events.py b/openslides_backend/shared/history_events.py index 1f0eaef75c..0b980d210a 100644 --- a/openslides_backend/shared/history_events.py +++ b/openslides_backend/shared/history_events.py @@ -2,13 +2,58 @@ from typing import Any from zoneinfo import ZoneInfo +from psycopg.types.json import Jsonb + from .interfaces.event import ListFields from .patterns import FullQualifiedId, fqid_from_collection_and_id -from .typing import HistoryInformation +from .typing import HistoryInformation, HistoryInformationData EventPayload = tuple[FullQualifiedId, dict[str, Any] | ListFields] +def build_history_information_data( + entries: list[str] | None = None, + changed_fields: dict[str, Any] | None = None, +) -> HistoryInformationData: + data: HistoryInformationData = {} + if entries is not None: + data["entries"] = entries + if changed_fields is not None: + data["changed_fields"] = changed_fields + return data + + +def update_history_information( + information: HistoryInformation, + fqid: FullQualifiedId, + entries: list[str] | None = None, + changed_fields: dict[str, Any] | None = None, +) -> None: + """Updates history information for fqid""" + if fqid not in information: + information[fqid] = build_history_information_data(entries, changed_fields) + else: + if entries: + information[fqid].setdefault("entries", list()).extend(entries) + if changed_fields: + information[fqid].setdefault("changed_fields", dict()).update( + changed_fields + ) + + +def update_history_information_multi( + information: HistoryInformation, + fqids: list[FullQualifiedId], + entries: list[str] | None = None, + changed_fields: dict[str, Any] | None = None, +) -> None: + """ + Adds given HistoryInformation to the given information for every fqid in fqids. + """ + for fqid in fqids: + update_history_information(information, fqid, entries, changed_fields) + + def calculate_history_event_payloads( user_id: int | None, information: HistoryInformation, @@ -19,8 +64,13 @@ def calculate_history_event_payloads( timestamp: int | None = None, ) -> list[EventPayload]: transformed_information = [ - (model_fqid_to_entry_id[fqid], fqid, entries) - for fqid, entries in information.items() + ( + model_fqid_to_entry_id[fqid], + fqid, + data["entries"], + data.get("changed_fields"), + ) + for fqid, data in information.items() ] create_events: list[EventPayload] = [ ( @@ -28,13 +78,14 @@ def calculate_history_event_payloads( { "id": id_, "entries": entries, + "changed_fields": Jsonb(changed_fields) if changed_fields else None, "position_id": position_id, "original_model_id": fqid, "model_id": (fqid if fqid in existing_fqids else None), "meeting_id": model_fqid_to_meeting_id.get(fqid, None), }, ) - for id_, fqid, entries in transformed_information + for id_, fqid, entries, changed_fields in transformed_information ] create_events.append( ( diff --git a/openslides_backend/shared/patterns.py b/openslides_backend/shared/patterns.py index 3519e27cd3..6687622ede 100644 --- a/openslides_backend/shared/patterns.py +++ b/openslides_backend/shared/patterns.py @@ -137,7 +137,7 @@ def collection_and_id_from_fqid(fqid: str) -> tuple[str, int]: # Build FQIDs -def fqid_from_collection_and_id(collection: str, id: str | int) -> str: +def fqid_from_collection_and_id(collection: str, id: str | int) -> FullQualifiedId: return f"{collection}{KEYSEPARATOR}{id}" diff --git a/openslides_backend/shared/typing.py b/openslides_backend/shared/typing.py index fbba186e7b..e0a2d531f6 100644 --- a/openslides_backend/shared/typing.py +++ b/openslides_backend/shared/typing.py @@ -1,6 +1,6 @@ -from typing import Any, Union +from typing import Any, NotRequired, TypedDict, Union -from .patterns import Collection, Id +from .patterns import Collection, Field, FullQualifiedId, Id PartialModel = dict[str, Any] Model = dict[str, Any] @@ -8,7 +8,13 @@ Schema = dict[str, Any] -HistoryInformation = dict[str, list[str]] + +class HistoryInformationData(TypedDict): + entries: NotRequired[list[str]] + changed_fields: NotRequired[dict[Field, Any]] + + +HistoryInformation = dict[FullQualifiedId, HistoryInformationData] JSON = Union[str, int, float, bool, None, dict[str, Any], list[Any]] diff --git a/tests/database/reader/system/test_filter.py b/tests/database/reader/system/test_filter.py index 7f83e16244..42c6bdea42 100644 --- a/tests/database/reader/system/test_filter.py +++ b/tests/database/reader/system/test_filter.py @@ -176,6 +176,7 @@ def test_types_str_list(db_connection: Connection) -> None: 1: { "id": 1, "entries": ["User added to meetings"], + "changed_fields": None, "meeting_id": 1, "model_id": "user/1", "model_id_assignment_id": None, diff --git a/tests/system/action/base.py b/tests/system/action/base.py index 6a138b3a1d..a8a3bc475c 100644 --- a/tests/system/action/base.py +++ b/tests/system/action/base.py @@ -15,6 +15,7 @@ from openslides_backend.permissions.management_levels import OrganizationManagementLevel from openslides_backend.permissions.permissions import Permission from openslides_backend.services.database.commands import GetManyRequest +from openslides_backend.services.database.interface import PartialModel from openslides_backend.shared.exceptions import AuthenticationException from openslides_backend.shared.filters import FilterOperator from openslides_backend.shared.patterns import ( @@ -297,7 +298,9 @@ def base_locked_out_superadmin_permission_test( True, ) - def get_last_history_information(self, fqid: FullQualifiedId) -> list[str] | None: + def get_last_history_information( + self, fqid: FullQualifiedId + ) -> PartialModel | None: entry_id = self.datastore.max( "history_entry", FilterOperator("original_model_id", "=", fqid), @@ -305,37 +308,44 @@ def get_last_history_information(self, fqid: FullQualifiedId) -> list[str] | Non lock_result=False, ) if entry_id: - history_entry = self.datastore.get( + return self.datastore.get( fqid_from_collection_and_id("history_entry", entry_id), - ["entries"], + ["entries", "changed_fields"], lock_result=False, ) - return history_entry.get("entries") else: return None def assert_history_information( - self, fqid: FullQualifiedId, information: list[str] | None + self, + fqid: FullQualifiedId, + entries: list[str] | None, + changed_fields: dict[str, Any] | None = None, ) -> None: """ Asserts that the last history information for the given model is the given information. """ last_information = self.get_last_history_information(fqid) - if information is None: - assert not last_information + if entries is None and changed_fields is None: + assert ( + not last_information + ), f"Expected no history information to be generated for {fqid}. Got:\n{last_information}" else: - assert last_information - self.assertEqual(last_information, information) + assert ( + last_information + ), f"No history information was be generated for {fqid}." + self.assertEqual(last_information.get("entries"), entries) + self.assertEqual(last_information.get("changed_fields"), changed_fields) def assert_history_information_contains( - self, fqid: FullQualifiedId, information: str + self, fqid: FullQualifiedId, entry: str ) -> None: """ Asserts that the last history information for the given model is the given information. """ last_information = self.get_last_history_information(fqid) - assert last_information - assert information in last_information + assert last_information, f"No history information was be generated for {fqid}." + self.assertIn(entry, last_information["entries"]) def assert_logged_in(self) -> None: self.auth.authenticate() # assert that no exception is thrown diff --git a/tests/system/action/user/test_create.py b/tests/system/action/user/test_create.py index 1a3040ef3b..a4208090f9 100644 --- a/tests/system/action/user/test_create.py +++ b/tests/system/action/user/test_create.py @@ -133,6 +133,7 @@ def test_create_some_more_fields(self) -> None: "group/114", "meeting/114", ], + {"group_ids": [114]}, ) def test_create_comment(self) -> None: diff --git a/tests/system/action/user/test_delegation_history.py b/tests/system/action/user/test_delegation_history.py index df5ce87bc3..b491b0f988 100644 --- a/tests/system/action/user/test_delegation_history.py +++ b/tests/system/action/user/test_delegation_history.py @@ -44,6 +44,8 @@ def assert_delegated_to( to_id: int, prepend_to: list[str] = [], prepend_from: list[str] = [], + changed_fields_to: dict[str, list[int]] | None = None, + changed_fields_from: dict[str, list[int]] | None = None, ) -> None: self.assert_history_information( f"user/{to_id}", @@ -53,6 +55,7 @@ def assert_delegated_to( f"user/{from_id}", "meeting/1", ], + changed_fields_to, ) self.assert_history_information( f"user/{from_id}", @@ -62,9 +65,15 @@ def assert_delegated_to( f"user/{to_id}", "meeting/1", ], + changed_fields_from, ) - def assert_alice_redelegated_to(self, who_id: int, prepend: list[str] = []) -> None: + def assert_alice_redelegated_to( + self, + who_id: int, + prepend: list[str] = [], + changed_fields: dict[str, list[int]] | None = None, + ) -> None: self.assert_history_information( f"user/{who_id}", [ @@ -73,6 +82,7 @@ def assert_alice_redelegated_to(self, who_id: int, prepend: list[str] = []) -> N f"user/{self.alice_id}", "meeting/1", ], + changed_fields, ) self.assert_history_information( f"user/{self.alice_id}", @@ -116,6 +126,7 @@ def test_create_delegate_vote(self) -> None: "group/3", "meeting/1", ], + changed_fields_from={"group_ids": [3]}, ) def test_create_receive_delegated_vote(self) -> None: @@ -133,6 +144,7 @@ def test_create_receive_delegated_vote(self) -> None: "group/3", "meeting/1", ], + changed_fields_to={"group_ids": [3]}, ) def test_update_re_delegate_vote(self) -> None: @@ -162,6 +174,7 @@ def test_create_re_delegate_vote_reverse(self) -> None: "group/3", "meeting/1", ], + changed_fields={"group_ids": [3]}, ) def test_update_re_delegate_received_votes(self) -> None: @@ -371,6 +384,7 @@ def test_create_multiple_from_ids(self) -> None: ], "meeting/1", ], + {"group_ids": [3]}, ) for id_ in [self.alice_id, self.bob_id, self.colin_id, eric_id, fredric_id]: self.assert_history_information( @@ -404,6 +418,7 @@ def test_update_create_meeting_user_receiving_delegation(self) -> None: f"user/{self.alice_id}", "meeting/1", ], + {"group_ids": [3]}, ) def test_update_create_meeting_user_with_delegation(self) -> None: @@ -431,4 +446,5 @@ def test_update_create_meeting_user_with_delegation(self) -> None: f"user/{self.alice_id}", "meeting/1", ], + {"group_ids": [3]}, ) diff --git a/tests/system/action/user/test_set_present.py b/tests/system/action/user/test_set_present.py index 17b759b660..da18e3f5d0 100644 --- a/tests/system/action/user/test_set_present.py +++ b/tests/system/action/user/test_set_present.py @@ -18,23 +18,40 @@ def test_set_present_add_correct(self) -> None: "user.set_present", {"id": 111, "meeting_id": 1, "present": True} ) self.assert_status_code(response, 200) - model = self.get_model("user/111") - assert model.get("is_present_in_meeting_ids") == [1] - meeting = self.get_model("meeting/1") - assert meeting.get("present_user_ids") == [111] + self.assert_model_exists("user/111", {"is_present_in_meeting_ids": [1]}) + self.assert_model_exists("meeting/1", {"present_user_ids": [111]}) self.assert_history_information( - "user/111", ["Set present in meeting {}", "meeting/1"] + "user/111", + ["Set present in meeting {}", "meeting/1"], + {"is_present_in_meeting_ids": [1]}, + ) + + def test_set_present_add_second_correct(self) -> None: + self.set_models( + { + "meeting/1": {"present_user_ids": [111]}, + "user/111": {"username": "username_srtgb123"}, + } + ) + self.create_meeting(4) + response = self.request( + "user.set_present", {"id": 111, "meeting_id": 4, "present": True} + ) + self.assert_status_code(response, 200) + self.assert_model_exists("user/111", {"is_present_in_meeting_ids": [1, 4]}) + self.assert_model_exists("meeting/1", {"present_user_ids": [111]}) + self.assert_model_exists("meeting/4", {"present_user_ids": [111]}) + self.assert_history_information( + "user/111", + ["Set present in meeting {}", "meeting/4"], + {"is_present_in_meeting_ids": [1, 4]}, ) def test_set_present_del_correct(self) -> None: self.set_models( { - "meeting/1": { - "present_user_ids": [111], - }, - "user/111": { - "username": "username_srtgb123", - }, + "meeting/1": {"present_user_ids": [111]}, + "user/111": {"username": "username_srtgb123"}, } ) response = self.request( @@ -44,7 +61,9 @@ def test_set_present_del_correct(self) -> None: self.assert_model_exists("user/111", {"is_present_in_meeting_ids": None}) self.assert_model_exists("meeting/1", {"present_user_ids": None}) self.assert_history_information( - "user/111", ["Set not present in meeting {}", "meeting/1"] + "user/111", + ["Set not present in meeting {}", "meeting/1"], + {"is_present_in_meeting_ids": []}, ) def test_set_present_null_action(self) -> None: diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index faa8b64173..1299dd7480 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -289,6 +289,7 @@ def test_update_with_meeting_user_fields(self) -> None: "meeting/1", "Committee management changed", ], + {"group_ids": [1]}, ) self.assert_history_information( "user/23", ["Vote delegated to {} in meeting {}", "user/22", "meeting/1"] @@ -490,6 +491,7 @@ def test_committee_manager_without_committee_ids(self) -> None: "Personal data changed", "Committee management changed", ], + {"group_ids": []}, ) def test_committee_manager_remove_committee_ids(self) -> None: @@ -2680,6 +2682,7 @@ def test_update_history_add_group(self) -> None: self.assert_history_information( f"user/{user_id}", ["Participant added to group {} in meeting {}", "group/3", "meeting/1"], + {"group_ids": [2, 3, 10, 11, 12]}, ) def test_update_history_add_group_to_default_group(self) -> None: @@ -2700,6 +2703,7 @@ def test_update_history_add_group_to_default_group(self) -> None: self.assert_history_information( f"user/{user_id}", ["Participant added to group {} in meeting {}", "group/2", "meeting/1"], + {"group_ids": [2, 10, 11, 12]}, ) def test_update_history_add_multiple_groups(self) -> None: @@ -2720,6 +2724,7 @@ def test_update_history_add_multiple_groups(self) -> None: self.assert_history_information( f"user/{user_id}", ["Participant added to multiple groups in meeting {}", "meeting/1"], + {"group_ids": [2, 3, 10, 11, 12]}, ) def test_update_history_add_multiple_groups_with_default_group(self) -> None: @@ -2739,6 +2744,7 @@ def test_update_history_add_multiple_groups_with_default_group(self) -> None: self.assert_history_information( f"user/{user_id}", ["Participant added to group {} in meeting {}", "group/2", "meeting/1"], + {"group_ids": [1, 2]}, ) def test_update_history_remove_group(self) -> None: @@ -2763,6 +2769,7 @@ def test_update_history_remove_group(self) -> None: self.assert_history_information( f"user/{user_id}", ["Participant removed from meeting {}", "meeting/1"], + {"group_ids": []}, ) def test_update_fields_with_equal_value_no_history(self) -> None: @@ -2859,6 +2866,7 @@ def test_update_participant_data_with_existing_meetings(self) -> None: "group/4", "meeting/4", ], + {"group_ids": [1, 4]}, ) def test_update_participant_data_in_multiple_meetings_with_existing_meetings( @@ -2908,6 +2916,7 @@ def test_update_participant_data_in_multiple_meetings_with_existing_meetings( "group/7", "meeting/7", ], + {"group_ids": [1, 4, 7]}, ) def test_update_saml_id__can_change_own_password_error(self) -> None: