From d86b9751d76fb57e7cfd1aee2a35ac5d3a4e3c83 Mon Sep 17 00:00:00 2001 From: Luisa Date: Wed, 24 Apr 2024 18:16:48 +0200 Subject: [PATCH 01/14] Start implementing new flags in motion.create_forwarded --- .../action/actions/motion/create_base.py | 11 +- .../action/actions/motion/create_forwarded.py | 163 +++++++++------ .../action/actions/motion/update.py | 2 - .../action/motion/test_create_forwarded.py | 186 ++++++++++++++++++ 4 files changed, 301 insertions(+), 61 deletions(-) diff --git a/openslides_backend/action/actions/motion/create_base.py b/openslides_backend/action/actions/motion/create_base.py index 31bdbf2f8..319158e53 100644 --- a/openslides_backend/action/actions/motion/create_base.py +++ b/openslides_backend/action/actions/motion/create_base.py @@ -79,10 +79,7 @@ def set_sequential_number(self, instance: dict[str, Any]) -> None: ) def set_created_last_modified_and_number(self, instance: dict[str, Any]) -> None: - timestamp = round(time.time()) - set_workflow_timestamp_helper(self.datastore, instance, timestamp) - instance["last_modified"] = timestamp - instance["created"] = timestamp + self.set_created_last_modified(instance) self.set_number( instance, instance["meeting_id"], @@ -90,3 +87,9 @@ def set_created_last_modified_and_number(self, instance: dict[str, Any]) -> None instance.get("lead_motion_id"), instance.get("category_id"), ) + + def set_created_last_modified(self, instance: dict[str, Any]) -> None: + timestamp = round(time.time()) + set_workflow_timestamp_helper(self.datastore, instance, timestamp) + instance["last_modified"] = timestamp + instance["created"] = timestamp diff --git a/openslides_backend/action/actions/motion/create_forwarded.py b/openslides_backend/action/actions/motion/create_forwarded.py index 247bb1b91..1a7bde741 100644 --- a/openslides_backend/action/actions/motion/create_forwarded.py +++ b/openslides_backend/action/actions/motion/create_forwarded.py @@ -10,12 +10,14 @@ from ....permissions.permissions import Permissions from ....services.datastore.commands import GetManyRequest from ....shared.exceptions import ActionException, PermissionDenied +from ....shared.filters import FilterOperator from ....shared.patterns import fqid_from_collection_and_id from ...util.default_schema import DefaultSchema from ...util.register import register_action from ...util.typing import ActionData from ..meeting_user.create import MeetingUserCreate from ..meeting_user.update import MeetingUserUpdate +from ..motion_submitter.create import MotionSubmitterCreateAction from ..user.create import UserCreate from .create_base import MotionCreateBase @@ -29,6 +31,10 @@ class MotionCreateForwarded(TextHashMixin, MotionCreateBase): schema = DefaultSchema(Motion()).get_create_schema( required_properties=["meeting_id", "title", "text", "origin_id"], optional_properties=["reason"], + additional_optional_fields={ + "use_original_submitter": {"type": "boolean"}, + "use_original_number": {"type": "boolean"}, + }, ) def prefetch(self, action_data: ActionData) -> None: @@ -76,6 +82,7 @@ def prefetch(self, action_data: ActionData) -> None: "all_origin_ids", "derived_motion_ids", "all_derived_motion_ids", + "number", ], ), ] @@ -97,73 +104,119 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: fqid_from_collection_and_id("meeting", instance["meeting_id"]), ["id", "default_group_id"], ) - if committee.get("forwarding_user_id"): - forwarding_user_id = committee["forwarding_user_id"] - meeting_id = instance["meeting_id"] - forwarding_user_groups = self.get_groups_from_meeting_user( - meeting_id, forwarding_user_id + + if instance.pop("use_original_submitter", None): + submitters = self.datastore.filter( + "motion_submitter", + FilterOperator("motion_id", "=", instance["origin_id"]), + ["meeting_user_id"], ) - if target_meeting["default_group_id"] not in forwarding_user_groups: - meeting_user = self.get_meeting_user( - meeting_id, forwarding_user_id, ["id", "group_ids"] - ) - if not meeting_user: + if submitters: + self.apply_instance(instance) + weight = 1 + for submitter in submitters.values(): + meeting_user_id = submitter["meeting_user_id"] + data = { + "motion_id": instance["id"], + "meeting_user_id": meeting_user_id, + "weight": weight, + } + weight += 1 self.execute_other_action( - MeetingUserCreate, - [ - { - "meeting_id": meeting_id, - "user_id": forwarding_user_id, - "group_ids": [target_meeting["default_group_id"]], - } - ], + MotionSubmitterCreateAction, [data], skip_history=True ) - else: - self.execute_other_action( - MeetingUserUpdate, - [ - { - "id": meeting_user["id"], - "group_ids": (meeting_user.get("group_ids") or []) - + [target_meeting["default_group_id"]], - } - ], + else: + # TODO: Was wenn die motion keine submitter hat + pass + else: + if committee.get("forwarding_user_id"): + forwarding_user_id = committee["forwarding_user_id"] + meeting_id = instance["meeting_id"] + forwarding_user_groups = self.get_groups_from_meeting_user( + meeting_id, forwarding_user_id + ) + if target_meeting["default_group_id"] not in forwarding_user_groups: + meeting_user = self.get_meeting_user( + meeting_id, forwarding_user_id, ["id", "group_ids"] ) + if not meeting_user: + self.execute_other_action( + MeetingUserCreate, + [ + { + "meeting_id": meeting_id, + "user_id": forwarding_user_id, + "group_ids": [target_meeting["default_group_id"]], + } + ], + ) + else: + self.execute_other_action( + MeetingUserUpdate, + [ + { + "id": meeting_user["id"], + "group_ids": (meeting_user.get("group_ids") or []) + + [target_meeting["default_group_id"]], + } + ], + ) - else: - username = committee.get("name", "Committee User") - meeting_id = instance["meeting_id"] - committee_user_create_payload = { - "last_name": username, - "is_physical_person": False, - "is_active": False, - "forwarding_committee_ids": [committee["id"]], - } - action_result = self.execute_other_action( - UserCreate, [committee_user_create_payload], skip_history=True - ) - assert action_result and action_result[0] - forwarding_user_id = action_result[0]["id"] - self.execute_other_action( - MeetingUserCreate, - [ - { - "user_id": forwarding_user_id, - "meeting_id": meeting_id, - "group_ids": [target_meeting["default_group_id"]], - } - ], - ) - instance["submitter_ids"] = [forwarding_user_id] + else: + username = committee.get("name", "Committee User") + meeting_id = instance["meeting_id"] + committee_user_create_payload = { + "last_name": username, + "is_physical_person": False, + "is_active": False, + "forwarding_committee_ids": [committee["id"]], + } + action_result = self.execute_other_action( + UserCreate, [committee_user_create_payload], skip_history=True + ) + assert action_result and action_result[0] + forwarding_user_id = action_result[0]["id"] + self.execute_other_action( + MeetingUserCreate, + [ + { + "user_id": forwarding_user_id, + "meeting_id": meeting_id, + "group_ids": [target_meeting["default_group_id"]], + } + ], + ) + instance["submitter_ids"] = [forwarding_user_id] + + self.create_submitters(instance) - self.create_submitters(instance) self.set_sequential_number(instance) - self.set_created_last_modified_and_number(instance) + self.handle_number(instance) self.set_origin_ids(instance) self.set_text_hash(instance) instance["forwarded"] = round(time.time()) return instance + def handle_number(self, instance: dict[str, Any]) -> dict[str, Any]: + origin = self.datastore.get( + fqid_from_collection_and_id("motion", instance["origin_id"]), ["number"] + ) + if instance.pop("use_original_number", None) and (num := origin.get("number")): + number = self.get_clean_number(num, instance["meeting_id"]) + self.set_created_last_modified(instance) + instance["number"] = number + else: + self.set_created_last_modified_and_number(instance) + return instance + + def get_clean_number(self, number: str, meeting_id: int) -> str: + new_number = number + next_identifier = 1 + while not self._check_if_unique(new_number, meeting_id, None): + new_number = f"{number}-{next_identifier}" + next_identifier += 1 + return new_number + def check_for_origin_id(self, instance: dict[str, Any]) -> dict[str, Any]: meeting = self.datastore.get( fqid_from_collection_and_id("meeting", instance["meeting_id"]), diff --git a/openslides_backend/action/actions/motion/update.py b/openslides_backend/action/actions/motion/update.py index 2d906ce0e..7dd5c1061 100644 --- a/openslides_backend/action/actions/motion/update.py +++ b/openslides_backend/action/actions/motion/update.py @@ -27,7 +27,6 @@ set_workflow_timestamp_helper, ) from .payload_validation_mixin import MotionUpdatePayloadValidationMixin -from .set_number_mixin import SetNumberMixin @register_action("motion.update") @@ -35,7 +34,6 @@ class MotionUpdate( MotionUpdatePayloadValidationMixin, AmendmentParagraphHelper, PermissionHelperMixin, - SetNumberMixin, TextHashMixin, UpdateAction, ): diff --git a/tests/system/action/motion/test_create_forwarded.py b/tests/system/action/motion/test_create_forwarded.py index 15b50fbd9..7a62b1fb6 100644 --- a/tests/system/action/motion/test_create_forwarded.py +++ b/tests/system/action/motion/test_create_forwarded.py @@ -855,3 +855,189 @@ def test_permissions(self) -> None: }, ) self.assert_status_code(response, 200) + + def test_forward_multiple_to_meeting_with_set_number(self) -> None: + """Forwarding of 1 motion to 2 meetings in 1 transaction""" + self.set_models(self.test_model) + self.set_models( + { + "meeting/1": { + "motion_ids": [12, 13], + }, + "motion/13": { + "title": "title_FcnPUXJB2", + "meeting_id": 1, + "state_id": 30, + }, + "motion_state/30": {"motion_ids": [12, 13]}, + "motion_state/34": {"set_number": True}, + } + ) + response = self.request_multi( + "motion.create_forwarded", + [ + { + "title": "title_12", + "meeting_id": 2, + "origin_id": 12, + "text": "test2", + "reason": "reason_jLvcgAMx2", + }, + { + "title": "title_13", + "meeting_id": 2, + "origin_id": 13, + "text": "test3", + "reason": "reason_jLvcgAMx3", + }, + ], + ) + self.assert_status_code(response, 200) + created = [date["id"] for date in response.json["results"][0]] + for i in range(2): + self.assert_model_exists(f"motion/{created[i]}", {"number": f"{i+1}"}) + + def test_forward_multiple_to_meeting_with_set_number_and_use_original_number( + self, + ) -> None: + """Forwarding of 1 motion to 2 meetings in 1 transaction""" + self.set_models(self.test_model) + self.set_models( + { + "meeting/1": { + "motion_ids": [12, 13], + }, + "motion/13": { + "title": "title_FcnPUXJB2", + "meeting_id": 1, + "state_id": 30, + "number": "1", + }, + "motion_state/30": {"motion_ids": [12, 13]}, + "motion_state/34": {"set_number": True}, + } + ) + response = self.request_multi( + "motion.create_forwarded", + [ + { + "title": "title_12", + "meeting_id": 2, + "origin_id": 12, + "text": "test2", + "reason": "reason_jLvcgAMx2", + }, + { + "title": "title_13", + "meeting_id": 2, + "origin_id": 13, + "text": "test3", + "reason": "reason_jLvcgAMx3", + "use_original_number": True, + }, + ], + ) + self.assert_status_code(response, 200) + created = [date["id"] for date in response.json["results"][0]] + self.assert_model_exists(f"motion/{created[0]}", {"number": "1"}) + self.assert_model_exists(f"motion/{created[1]}", {"number": "1-1"}) + + def test_forward_multiple_to_meeting_with_set_number_and_use_original_number_2( + self, + ) -> None: + """Forwarding of 1 motion to 2 meetings in 1 transaction""" + self.set_models(self.test_model) + self.set_models( + { + "meeting/1": { + "motion_ids": [12, 13], + }, + "motion/12": {"number": "1"}, + "motion/13": { + "title": "title_FcnPUXJB2", + "meeting_id": 1, + "state_id": 30, + }, + "motion_state/30": {"motion_ids": [12, 13]}, + "motion_state/34": {"set_number": True}, + } + ) + response = self.request_multi( + "motion.create_forwarded", + [ + { + "title": "title_12", + "meeting_id": 2, + "origin_id": 12, + "text": "test2", + "reason": "reason_jLvcgAMx2", + "use_original_number": True, + }, + { + "title": "title_13", + "meeting_id": 2, + "origin_id": 13, + "text": "test3", + "reason": "reason_jLvcgAMx3", + }, + ], + ) + self.assert_status_code(response, 200) + created = [date["id"] for date in response.json["results"][0]] + self.assert_model_exists(f"motion/{created[0]}", {"number": "1"}) + self.assert_model_exists(f"motion/{created[1]}", {"number": "2"}) + + def test_forward_multiple_to_meeting_with_set_number_and_use_original_number_3( + self, + ) -> None: + """Forwarding of 1 motion to 2 meetings in 1 transaction""" + self.set_models(self.test_model) + self.set_models( + { + "meeting/1": { + "motion_ids": [12, 13], + }, + "meeting/2": { + "motion_ids": [14], + }, + "motion/12": {"number": "1"}, + "motion/13": { + "title": "title_FcnPUXJB2", + "meeting_id": 1, + "state_id": 30, + "number": "1", + }, + "motion/14": { + "title": "title_FcnPUXJB2", + "meeting_id": 2, + "state_id": 30, + "number": "1", + }, + "motion_state/30": {"motion_ids": [12, 13]}, + } + ) + response = self.request_multi( + "motion.create_forwarded", + [ + { + "title": "title_12", + "meeting_id": 2, + "origin_id": 12, + "text": "test2", + "reason": "reason_jLvcgAMx2", + "use_original_number": True, + }, + { + "title": "title_13", + "meeting_id": 2, + "origin_id": 13, + "text": "test3", + "reason": "reason_jLvcgAMx3", + "use_original_number": True, + }, + ], + ) + self.assert_status_code(response, 200) + created = [date["id"] for date in response.json["results"][0]] + self.assert_model_exists(f"motion/{created[0]}", {"number": "1-1"}) + self.assert_model_exists(f"motion/{created[1]}", {"number": "1-2"}) From c3c08f6b3c37997610119d0e0bbf83116225697e Mon Sep 17 00:00:00 2001 From: Luisa Date: Fri, 26 Apr 2024 14:20:22 +0200 Subject: [PATCH 02/14] Finish implementing use_original_submitter --- .../action/actions/motion/create_forwarded.py | 3 - .../action/actions/motion_submitter/create.py | 2 +- .../mixins/motion_meeting_user_create.py | 6 +- .../action/motion/test_create_forwarded.py | 109 +++++++++++++++++- 4 files changed, 111 insertions(+), 9 deletions(-) diff --git a/openslides_backend/action/actions/motion/create_forwarded.py b/openslides_backend/action/actions/motion/create_forwarded.py index 1a7bde741..c62d9c1c6 100644 --- a/openslides_backend/action/actions/motion/create_forwarded.py +++ b/openslides_backend/action/actions/motion/create_forwarded.py @@ -125,9 +125,6 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: self.execute_other_action( MotionSubmitterCreateAction, [data], skip_history=True ) - else: - # TODO: Was wenn die motion keine submitter hat - pass else: if committee.get("forwarding_user_id"): forwarding_user_id = committee["forwarding_user_id"] diff --git a/openslides_backend/action/actions/motion_submitter/create.py b/openslides_backend/action/actions/motion_submitter/create.py index 5758cf250..b289cad48 100644 --- a/openslides_backend/action/actions/motion_submitter/create.py +++ b/openslides_backend/action/actions/motion_submitter/create.py @@ -2,7 +2,7 @@ from ...mixins.motion_meeting_user_create import build_motion_meeting_user_create_action from ...util.register import register_action -BaseClass: type = build_motion_meeting_user_create_action(MotionSubmitter) +BaseClass: type = build_motion_meeting_user_create_action(MotionSubmitter, True) @register_action("motion_submitter.create") diff --git a/openslides_backend/action/mixins/motion_meeting_user_create.py b/openslides_backend/action/mixins/motion_meeting_user_create.py index f71e23f25..9edf3ccd1 100644 --- a/openslides_backend/action/mixins/motion_meeting_user_create.py +++ b/openslides_backend/action/mixins/motion_meeting_user_create.py @@ -18,7 +18,7 @@ def build_motion_meeting_user_create_action( - ModelClass: type[Model], + ModelClass: type[Model], ignore_meeting_if_internal: bool = False ) -> type[CreateAction]: class BaseMotionMeetingUserCreateAction( WeightMixin, CreateActionWithInferredMeetingMixin, CreateAction @@ -45,7 +45,9 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: ), ["user_id"], ) - if not has_organization_management_level( + if not ( + ignore_meeting_if_internal and self.internal + ) and not has_organization_management_level( self.datastore, meeting_user["user_id"], OrganizationManagementLevel.SUPERADMIN, diff --git a/tests/system/action/motion/test_create_forwarded.py b/tests/system/action/motion/test_create_forwarded.py index 7a62b1fb6..76de01284 100644 --- a/tests/system/action/motion/test_create_forwarded.py +++ b/tests/system/action/motion/test_create_forwarded.py @@ -2,6 +2,7 @@ from openslides_backend.action.actions.motion.mixins import TextHashMixin from openslides_backend.permissions.permissions import Permissions +from openslides_backend.shared.patterns import fqid_from_collection_and_id from tests.system.action.base import BaseActionTestCase @@ -1000,12 +1001,13 @@ def test_forward_multiple_to_meeting_with_set_number_and_use_original_number_3( "meeting/2": { "motion_ids": [14], }, - "motion/12": {"number": "1"}, + "motion/12": {"number": "1", "submitter_ids": [12]}, "motion/13": { "title": "title_FcnPUXJB2", "meeting_id": 1, "state_id": 30, "number": "1", + "submitter_ids": [13], }, "motion/14": { "title": "title_FcnPUXJB2", @@ -1014,6 +1016,16 @@ def test_forward_multiple_to_meeting_with_set_number_and_use_original_number_3( "number": "1", }, "motion_state/30": {"motion_ids": [12, 13]}, + "motion_submitter/12": { + "meeting_user_id": 1, + "motion_id": 12, + "meeting_id": 1, + }, + "motion_submitter/13": { + "meeting_user_id": 1, + "motion_id": 13, + "meeting_id": 1, + }, } ) response = self.request_multi( @@ -1026,6 +1038,7 @@ def test_forward_multiple_to_meeting_with_set_number_and_use_original_number_3( "text": "test2", "reason": "reason_jLvcgAMx2", "use_original_number": True, + "use_original_submitter": True, }, { "title": "title_13", @@ -1039,5 +1052,95 @@ def test_forward_multiple_to_meeting_with_set_number_and_use_original_number_3( ) self.assert_status_code(response, 200) created = [date["id"] for date in response.json["results"][0]] - self.assert_model_exists(f"motion/{created[0]}", {"number": "1-1"}) - self.assert_model_exists(f"motion/{created[1]}", {"number": "1-2"}) + motion1 = self.assert_model_exists(f"motion/{created[0]}", {"number": "1-1"}) + motion2 = self.assert_model_exists(f"motion/{created[1]}", {"number": "1-2"}) + self.assert_model_exists( + fqid_from_collection_and_id( + "motion_submitter", motion1["submitter_ids"][0] + ), + {"meeting_user_id": 1, "meeting_id": 2}, + ) + submitter = self.assert_model_exists( + fqid_from_collection_and_id("motion_submitter", motion2["submitter_ids"][0]) + ) + mUser = self.assert_model_exists( + fqid_from_collection_and_id("meeting_user", submitter["meeting_user_id"]) + ) + self.assert_model_exists( + fqid_from_collection_and_id("user", mUser["user_id"]), + {"username": "committee_forwarder"}, + ) + + def test_use_original_submitter_empty(self) -> None: + self.set_models(self.test_model) + response = self.request( + "motion.create_forwarded", + { + "title": "test_Xcdfgee", + "meeting_id": 2, + "origin_id": 12, + "text": "test", + "reason": "reason_jLvcgAMx", + "use_original_submitter": True, + }, + ) + self.assert_status_code(response, 200) + created_id = response.json["results"][0][0]["id"] + self.assert_model_exists( + f"motion/{created_id}", {"number": None, "submitter_ids": None} + ) + + def test_use_original_submitter_multiple(self) -> None: + self.set_models(self.test_model) + extra_user_id = self.create_user("user", [111]) + self.set_models( + { + "motion/12": {"submitter_ids": [12, 13]}, + "motion_submitter/12": { + "meeting_user_id": 1, + "motion_id": 12, + "meeting_id": 1, + }, + "motion_submitter/13": { + "meeting_user_id": 3, + "motion_id": 12, + "meeting_id": 1, + }, + "meeting_user/3": { + "motion_submitter_ids": [13], + }, + "meeting/1": { + "meeting_user_ids": [1, 3], + "motion_submitter_ids": [12, 13], + "user_ids": [1, extra_user_id], + }, + f"user/{extra_user_id}": {"meeting_user_ids": [3], "meeting_ids": [1]}, + } + ) + response = self.request( + "motion.create_forwarded", + { + "title": "test_Xcdfgee", + "meeting_id": 2, + "origin_id": 12, + "text": "test", + "reason": "reason_jLvcgAMx", + "use_original_submitter": True, + }, + ) + self.assert_status_code(response, 200) + created_id = response.json["results"][0][0]["id"] + motion = self.assert_model_exists(f"motion/{created_id}") + assert len(motion["submitter_ids"]) == 2 + submitter1 = self.assert_model_exists( + fqid_from_collection_and_id("motion_submitter", motion["submitter_ids"][0]) + )["meeting_user_id"] + submitter2 = self.assert_model_exists( + fqid_from_collection_and_id("motion_submitter", motion["submitter_ids"][1]) + )["meeting_user_id"] + expected_meeting_user_ids = [1, 3] + assert ( + submitter1 in expected_meeting_user_ids + and submitter2 in expected_meeting_user_ids + and submitter1 != submitter2 + ) From bf3c4c9c4b156b27cae2ea114643068cdd072616 Mon Sep 17 00:00:00 2001 From: Luisa Date: Fri, 26 Apr 2024 15:06:23 +0200 Subject: [PATCH 03/14] Also transfer string submitter --- .../action/actions/motion/create_forwarded.py | 8 +++++++- tests/system/action/motion/test_create_forwarded.py | 9 +++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/openslides_backend/action/actions/motion/create_forwarded.py b/openslides_backend/action/actions/motion/create_forwarded.py index c62d9c1c6..c1c4096b6 100644 --- a/openslides_backend/action/actions/motion/create_forwarded.py +++ b/openslides_backend/action/actions/motion/create_forwarded.py @@ -82,7 +82,6 @@ def prefetch(self, action_data: ActionData) -> None: "all_origin_ids", "derived_motion_ids", "all_derived_motion_ids", - "number", ], ), ] @@ -125,6 +124,13 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: self.execute_other_action( MotionSubmitterCreateAction, [data], skip_history=True ) + text_submitter = self.datastore.get( + fqid_from_collection_and_id("motion", instance["origin_id"]), + ["additional_submitter"], + ).get("additional_submitter") + if text_submitter: + instance["additional_submitter"] = text_submitter + else: if committee.get("forwarding_user_id"): forwarding_user_id = committee["forwarding_user_id"] diff --git a/tests/system/action/motion/test_create_forwarded.py b/tests/system/action/motion/test_create_forwarded.py index 76de01284..4285781a2 100644 --- a/tests/system/action/motion/test_create_forwarded.py +++ b/tests/system/action/motion/test_create_forwarded.py @@ -1095,7 +1095,10 @@ def test_use_original_submitter_multiple(self) -> None: extra_user_id = self.create_user("user", [111]) self.set_models( { - "motion/12": {"submitter_ids": [12, 13]}, + "motion/12": { + "submitter_ids": [12, 13], + "additional_submitter": "Sue B. Mid-Edit", + }, "motion_submitter/12": { "meeting_user_id": 1, "motion_id": 12, @@ -1130,7 +1133,9 @@ def test_use_original_submitter_multiple(self) -> None: ) self.assert_status_code(response, 200) created_id = response.json["results"][0][0]["id"] - motion = self.assert_model_exists(f"motion/{created_id}") + motion = self.assert_model_exists( + f"motion/{created_id}", {"additional_submitter": "Sue B. Mid-Edit"} + ) assert len(motion["submitter_ids"]) == 2 submitter1 = self.assert_model_exists( fqid_from_collection_and_id("motion_submitter", motion["submitter_ids"][0]) From dad817a44690e8afe1f87948a3f5359d4ee66911 Mon Sep 17 00:00:00 2001 From: Luisa Date: Fri, 26 Apr 2024 15:36:29 +0200 Subject: [PATCH 04/14] Update wiki entry --- docs/actions/motion.create_forwarded.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/actions/motion.create_forwarded.md b/docs/actions/motion.create_forwarded.md index 7a5e5680b..4754fca6c 100644 --- a/docs/actions/motion.create_forwarded.md +++ b/docs/actions/motion.create_forwarded.md @@ -9,6 +9,8 @@ // Optional reason: HTML; + use_original_submitter: boolean; + use_original_number: boolean; } ``` @@ -25,6 +27,8 @@ The original motion must be updated as well (this is done by the automatic relat * The unique `id` of the newly created motion has to be linked to the _origin motion_s `derived_motion_ids` field. * Deleting the newly created motion has to ensure that the corresponding entry was removed from the _origin motion_s `derived_motion_ids` field +The optional flags `use_original_submitter` and `use_original_number` will cause the original submitters and original numbers to be used in the new motion respectively. In case of the submitters, the action will use the origin meetings meeting_user for the `meeting_user_id` field of the new submitter instance in the target meeting. + ### Forwarding tree fields * `all_origin_ids` of the newly created motion must be set to `all_origin_ids` of the origin motion plus the given `origin_id`. It is important that the id is appended at the end of the list, since the order of this field represents the order of the tree in case a motion of the tree is deleted. From 7bb471ce3d7a383a6c8afda58930d911cffa25fa Mon Sep 17 00:00:00 2001 From: Luisa Date: Thu, 27 Jun 2024 14:32:40 +0200 Subject: [PATCH 05/14] Use additional_submitter instead of forwarding user --- global/meta | 2 +- .../action/actions/motion/create_forwarded.py | 175 ++++---- .../action/motion/test_create_forwarded.py | 420 ++++++------------ 3 files changed, 224 insertions(+), 373 deletions(-) diff --git a/global/meta b/global/meta index f8326caac..278414471 160000 --- a/global/meta +++ b/global/meta @@ -1 +1 @@ -Subproject commit f8326caac3e7554f531b0b3a9b7651fbc4fa33af +Subproject commit 278414471d7c873f040a28e6691ca0abc15a398e diff --git a/openslides_backend/action/actions/motion/create_forwarded.py b/openslides_backend/action/actions/motion/create_forwarded.py index c1c4096b6..fcbeea35d 100644 --- a/openslides_backend/action/actions/motion/create_forwarded.py +++ b/openslides_backend/action/actions/motion/create_forwarded.py @@ -15,10 +15,6 @@ from ...util.default_schema import DefaultSchema from ...util.register import register_action from ...util.typing import ActionData -from ..meeting_user.create import MeetingUserCreate -from ..meeting_user.update import MeetingUserUpdate -from ..motion_submitter.create import MotionSubmitterCreateAction -from ..user.create import UserCreate from .create_base import MotionCreateBase @@ -87,6 +83,69 @@ def prefetch(self, action_data: ActionData) -> None: ] ) + def get_user_verbose_names(self, meeting_user_ids: list[int]) -> str | None: + meeting_users = self.datastore.get_many( + [ + GetManyRequest( + "meeting_user", meeting_user_ids, ["user_id", "structure_level_ids"] + ) + ] + )["meeting_user"] + user_ids = [ + user_id + for meeting_user in meeting_users.values() + if (user_id := meeting_user.get("user_id")) + ] + if not len(user_ids): + return None + requests = [ + GetManyRequest( + "user", user_ids, ["id", "first_name", "last_name", "title", "pronoun"] + ) + ] + if structure_level_ids := list( + { + structure_level_id + for meeting_user in meeting_users.values() + for structure_level_id in meeting_user.get("structure_level_ids", []) + } + ): + requests.append( + GetManyRequest("structure_level", structure_level_ids, ["name"]) + ) + user_data = self.datastore.get_many(requests) + users = user_data["user"] + structure_levels = user_data["structure_level"] + names = [] + for meeting_user_id in meeting_user_ids: + meeting_user = meeting_users[meeting_user_id] + user = users.get(meeting_user.get("user_id", 0)) + if user: + additional_info: list[str] = [] + if pronoun := user.get("pronoun"): + additional_info = [pronoun] + if sl_ids := meeting_user.get("structure_level_ids"): + if slnames := ", ".join( + name + for structure_level_id in sl_ids + if ( + name := structure_levels.get(structure_level_id, {}).get( + "name" + ) + ) + ): + additional_info.append(slnames) + suffix = " · ".join(additional_info) + if suffix: + suffix = f"({suffix})" + if not any(user.get(field) for field in ["first_name", "last_name"]): + short_name = f"User {user['id']}" + else: + short_name = f"{user.get('first_name', '')} {user.get('last_name', '')}".strip() + long_name = f"{user.get('title', '')} {short_name} {suffix}".strip() + names.append(long_name) + return ", ".join(names) + def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: meeting = self.datastore.get( fqid_from_collection_and_id("meeting", instance["meeting_id"]), @@ -98,100 +157,36 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: committee = self.check_for_origin_id(instance) self.check_state_allow_forwarding(instance) - # handle forwarding user - target_meeting = self.datastore.get( - fqid_from_collection_and_id("meeting", instance["meeting_id"]), - ["id", "default_group_id"], - ) - if instance.pop("use_original_submitter", None): - submitters = self.datastore.filter( - "motion_submitter", - FilterOperator("motion_id", "=", instance["origin_id"]), - ["meeting_user_id"], + submitters = list( + self.datastore.filter( + "motion_submitter", + FilterOperator("motion_id", "=", instance["origin_id"]), + ["meeting_user_id"], + ).values() ) - if submitters: - self.apply_instance(instance) - weight = 1 - for submitter in submitters.values(): - meeting_user_id = submitter["meeting_user_id"] - data = { - "motion_id": instance["id"], - "meeting_user_id": meeting_user_id, - "weight": weight, - } - weight += 1 - self.execute_other_action( - MotionSubmitterCreateAction, [data], skip_history=True - ) + submitters = sorted(submitters, key=lambda x: x.get("weight", 10000)) + meeting_user_ids = [ + meeting_user_id + for submitter in submitters + if (meeting_user_id := submitter.get("meeting_user_id")) + ] + if len(meeting_user_ids): + instance["additional_submitter"] = self.get_user_verbose_names( + meeting_user_ids + ) text_submitter = self.datastore.get( fqid_from_collection_and_id("motion", instance["origin_id"]), ["additional_submitter"], ).get("additional_submitter") if text_submitter: - instance["additional_submitter"] = text_submitter - + if instance.get("additional_submitter"): + instance["additional_submitter"] += ", " + text_submitter + else: + instance["additional_submitter"] = text_submitter else: - if committee.get("forwarding_user_id"): - forwarding_user_id = committee["forwarding_user_id"] - meeting_id = instance["meeting_id"] - forwarding_user_groups = self.get_groups_from_meeting_user( - meeting_id, forwarding_user_id - ) - if target_meeting["default_group_id"] not in forwarding_user_groups: - meeting_user = self.get_meeting_user( - meeting_id, forwarding_user_id, ["id", "group_ids"] - ) - if not meeting_user: - self.execute_other_action( - MeetingUserCreate, - [ - { - "meeting_id": meeting_id, - "user_id": forwarding_user_id, - "group_ids": [target_meeting["default_group_id"]], - } - ], - ) - else: - self.execute_other_action( - MeetingUserUpdate, - [ - { - "id": meeting_user["id"], - "group_ids": (meeting_user.get("group_ids") or []) - + [target_meeting["default_group_id"]], - } - ], - ) - - else: - username = committee.get("name", "Committee User") - meeting_id = instance["meeting_id"] - committee_user_create_payload = { - "last_name": username, - "is_physical_person": False, - "is_active": False, - "forwarding_committee_ids": [committee["id"]], - } - action_result = self.execute_other_action( - UserCreate, [committee_user_create_payload], skip_history=True - ) - assert action_result and action_result[0] - forwarding_user_id = action_result[0]["id"] - self.execute_other_action( - MeetingUserCreate, - [ - { - "user_id": forwarding_user_id, - "meeting_id": meeting_id, - "group_ids": [target_meeting["default_group_id"]], - } - ], - ) - instance["submitter_ids"] = [forwarding_user_id] - - self.create_submitters(instance) + name = committee.get("name", f"Committee {committee['id']}") + instance["additional_submitter"] = name self.set_sequential_number(instance) self.handle_number(instance) diff --git a/tests/system/action/motion/test_create_forwarded.py b/tests/system/action/motion/test_create_forwarded.py index 4285781a2..23d75e10c 100644 --- a/tests/system/action/motion/test_create_forwarded.py +++ b/tests/system/action/motion/test_create_forwarded.py @@ -2,7 +2,6 @@ from openslides_backend.action.actions.motion.mixins import TextHashMixin from openslides_backend.permissions.permissions import Permissions -from openslides_backend.shared.patterns import fqid_from_collection_and_id from tests.system.action.base import BaseActionTestCase @@ -18,6 +17,7 @@ def setUp(self) -> None: "motion_ids": [12], "meeting_user_ids": [1], "user_ids": [1], + "structure_level_ids": [1, 2, 3], }, "meeting/2": { "name": "name_SNLGsvIV", @@ -31,7 +31,14 @@ def setUp(self) -> None: 1, ], }, - "user/1": {"meeting_ids": [1, 2]}, + "user/1": { + "meeting_ids": [1, 2], + "first_name": "the", + "last_name": "administrator", + "title": "Worship", + "pronoun": "he", + "meeting_user_ids": [1, 2], + }, "motion_workflow/12": { "name": "name_workflow1", "first_state_id": 34, @@ -75,6 +82,22 @@ def setUp(self) -> None: "user_id": 1, "meeting_id": 1, "group_ids": [111], + "structure_level_ids": [1, 2, 3], + }, + "structure_level/1": { + "meeting_user_ids": [1], + "meeting_id": 1, + "name": "is", + }, + "structure_level/2": { + "meeting_user_ids": [1], + "meeting_id": 1, + "name": "very", + }, + "structure_level/3": { + "meeting_user_ids": [1], + "meeting_id": 1, + "name": "good", }, "meeting_user/2": { "id": 2, @@ -107,40 +130,15 @@ def test_correct_origin_id_set(self) -> None: "all_derived_motion_ids": None, "all_origin_ids": [12], "reason": "reason_jLvcgAMx", - "submitter_ids": [1], + "submitter_ids": None, + "additional_submitter": "committee_forwarder", "state_id": 34, }, ) assert model.get("forwarded") - self.assert_model_exists( - "motion_submitter/1", - { - "meeting_user_id": 3, - "motion_id": 13, - }, - ) - self.assert_model_exists( - "user/2", - { - "username": "committee_forwarder", - "last_name": "committee_forwarder", - "is_physical_person": False, - "is_active": False, - "meeting_user_ids": [3], - "forwarding_committee_ids": [53], - }, - ) - self.assert_model_exists( - "meeting_user/3", - { - "meeting_id": 2, - "user_id": 2, - "motion_submitter_ids": [1], - "group_ids": [112], - }, - ) - self.assert_model_exists("group/112", {"meeting_user_ids": [2, 3]}) - self.assert_model_exists("committee/53", {"forwarding_user_id": 2}) + self.assert_model_not_exists("motion_submitter/1") + self.assert_model_not_exists("user/2") + self.assert_model_not_exists("meeting_user/3") self.assert_model_exists( "motion/12", {"derived_motion_ids": [13], "all_derived_motion_ids": [13]} ) @@ -176,139 +174,6 @@ def test_no_meeting_id(self) -> None: self.assert_status_code(response, 400) assert response.json["message"] == "data must contain ['origin_id'] properties" - def test_correct_existing_registered_forward_user(self) -> None: - self.set_models(self.test_model) - self.set_models( - { - "user/2": { - "username": "committee_forwarder53", - "is_physical_person": False, - "is_active": False, - "forwarding_committee_ids": [53], - }, - "group/113": {"name": "HPMHcWhk", "meeting_id": 2}, - "meeting/2": {"group_ids": [112, 113]}, - "committee/53": {"forwarding_user_id": 2}, - } - ) - self.set_user_groups( - 2, - [ - 113, - ], - ) - response = self.request( - "motion.create_forwarded", - { - "title": "test_Xcdfgee", - "meeting_id": 2, - "origin_id": 12, - "text": "test", - "reason": "reason_jLvcgAMx", - }, - ) - self.assert_status_code(response, 200) - self.assert_model_exists( - "committee/52", - { - "name": "committee_receiver", - "user_ids": [1, 2], - "meeting_ids": [2], - "receive_forwardings_from_committee_ids": [53], - }, - ) - self.assert_model_exists( - "committee/53", - { - "name": "committee_forwarder", - "user_ids": [1], - "forwarding_user_id": 2, - "forward_to_committee_ids": [52], - }, - ) - self.assert_model_exists( - "meeting/1", - { - "committee_id": 53, - "user_ids": [1], - "motion_ids": [ - 12, - ], - "forwarded_motion_ids": [13], - "group_ids": [111], - "meeting_user_ids": [1], - }, - ) - self.assert_model_exists( - "meeting/2", - { - "committee_id": 52, - "user_ids": [1, 2], - "meeting_user_ids": [2, 3], - "motion_ids": [13], - "motion_submitter_ids": [1], - "group_ids": [112, 113], - "list_of_speakers_ids": [1], - }, - ) - - self.assert_model_exists( - "user/2", - { - "username": "committee_forwarder53", - "is_physical_person": False, - "is_active": False, - "meeting_ids": [2], - "committee_ids": [52], - "meeting_user_ids": [3], - "forwarding_committee_ids": [53], - }, - ) - self.assert_model_exists( - "meeting_user/3", - { - "user_id": 2, - "meeting_id": 2, - "group_ids": [113, 112], - "motion_submitter_ids": [1], - }, - ) - self.assert_model_exists( - "group/112", {"meeting_id": 2, "meeting_user_ids": [2, 3]} - ) - self.assert_model_exists( - "group/113", {"meeting_id": 2, "meeting_user_ids": [3]} - ) - self.assert_model_exists( - "motion/12", - { - "title": "title_FcnPUXJB", - "meeting_id": 1, - "origin_id": None, - "all_origin_ids": None, - "derived_motion_ids": [13], - "all_derived_motion_ids": [13], - }, - ) - motion13 = self.assert_model_exists( - "motion/13", - { - "title": "test_Xcdfgee", - "text": "test", - "meeting_id": 2, - "origin_id": 12, - "all_derived_motion_ids": None, - "all_origin_ids": [12], - "reason": "reason_jLvcgAMx", - "submitter_ids": [1], - "list_of_speakers_id": 1, - }, - ) - assert motion13.get("forwarded") - self.assert_model_exists( - "motion_submitter/1", {"motion_id": 13, "meeting_user_id": 3} - ) - def test_correct_existing_unregistered_forward_user(self) -> None: self.set_models(self.test_model) self.set_models( @@ -338,41 +203,14 @@ def test_correct_existing_unregistered_forward_user(self) -> None: "origin_id": 12, "all_derived_motion_ids": None, "all_origin_ids": [12], - "submitter_ids": [1], + "submitter_ids": None, + "additional_submitter": "committee_forwarder", }, ) assert model.get("forwarded") - self.assert_model_exists( - "user/3", - { - "username": "committee_forwarder1", - "last_name": "committee_forwarder", - "is_physical_person": False, - "is_active": False, - "meeting_user_ids": [3], - "forwarding_committee_ids": [53], - "committee_ids": [52], - "meeting_ids": [2], - }, - ) - self.assert_model_exists( - "meeting_user/3", - { - "meeting_id": 2, - "user_id": 3, - "group_ids": [112], - "motion_submitter_ids": [1], - }, - ) - self.assert_model_exists("group/112", {"meeting_user_ids": [2, 3]}) - self.assert_model_exists("committee/53", {"forwarding_user_id": 3}) self.assert_model_exists( "motion/12", {"derived_motion_ids": [13], "all_derived_motion_ids": [13]} ) - self.assert_model_exists( - "motion_submitter/1", - {"meeting_user_id": 3, "motion_id": 13, "meeting_id": 2}, - ) def test_correct_origin_id_wrong_1(self) -> None: self.test_model["committee/53"]["forward_to_committee_ids"] = [] @@ -688,31 +526,12 @@ def test_forward_to_2_meetings_1_transaction(self) -> None: "all_derived_motion_ids": None, "all_origin_ids": [12], "reason": "reason_jLvcgAMx2", - "submitter_ids": [1], + "submitter_ids": None, + "additional_submitter": "committee_forwarder", "state_id": 34, }, ) assert model.get("forwarded") - self.assert_model_exists( - "motion_submitter/1", - { - "meeting_id": 2, - "meeting_user_id": 3, - "motion_id": 13, - }, - ) - self.assert_model_exists( - "meeting_user/3", - { - "user_id": 2, - "meeting_id": 2, - "motion_submitter_ids": [1], - "group_ids": [112], - }, - ) - self.assert_model_exists( - "group/112", {"meeting_user_ids": [2, 3], "meeting_id": 2} - ) model = self.assert_model_exists( "motion/14", @@ -724,47 +543,12 @@ def test_forward_to_2_meetings_1_transaction(self) -> None: "all_derived_motion_ids": None, "all_origin_ids": [12], "reason": "reason_jLvcgAMx3", - "submitter_ids": [2], + "submitter_ids": None, + "additional_submitter": "committee_forwarder", "state_id": 33, }, ) assert model.get("forwarded") - self.assert_model_exists( - "motion_submitter/2", - { - "meeting_user_id": 4, - "meeting_id": 3, - "motion_id": 14, - }, - ) - self.assert_model_exists( - "meeting_user/4", - { - "user_id": 2, - "meeting_id": 3, - "motion_submitter_ids": [2], - "group_ids": [113], - }, - ) - self.assert_model_exists( - "group/113", {"meeting_user_ids": [4], "meeting_id": 3} - ) - - self.assert_model_exists( - "user/2", - { - "username": "committee_forwarder", - "last_name": "committee_forwarder", - "is_physical_person": False, - "is_active": False, - "meeting_user_ids": [3, 4], - "forwarding_committee_ids": [53], - "meeting_ids": [2, 3], - "committee_ids": [52], - }, - ) - - self.assert_model_exists("committee/53", {"forwarding_user_id": 2}) self.assert_model_exists( "motion/12", {"derived_motion_ids": [13, 14], "all_derived_motion_ids": [13, 14]}, @@ -1052,24 +836,14 @@ def test_forward_multiple_to_meeting_with_set_number_and_use_original_number_3( ) self.assert_status_code(response, 200) created = [date["id"] for date in response.json["results"][0]] - motion1 = self.assert_model_exists(f"motion/{created[0]}", {"number": "1-1"}) - motion2 = self.assert_model_exists(f"motion/{created[1]}", {"number": "1-2"}) self.assert_model_exists( - fqid_from_collection_and_id( - "motion_submitter", motion1["submitter_ids"][0] - ), - {"meeting_user_id": 1, "meeting_id": 2}, - ) - submitter = self.assert_model_exists( - fqid_from_collection_and_id("motion_submitter", motion2["submitter_ids"][0]) - ) - mUser = self.assert_model_exists( - fqid_from_collection_and_id("meeting_user", submitter["meeting_user_id"]) - ) - self.assert_model_exists( - fqid_from_collection_and_id("user", mUser["user_id"]), - {"username": "committee_forwarder"}, + f"motion/{created[0]}", + { + "number": "1-1", + "additional_submitter": "Worship the administrator (he · is, very, good)", + }, ) + self.assert_model_exists(f"motion/{created[1]}", {"number": "1-2"}) def test_use_original_submitter_empty(self) -> None: self.set_models(self.test_model) @@ -1133,19 +907,101 @@ def test_use_original_submitter_multiple(self) -> None: ) self.assert_status_code(response, 200) created_id = response.json["results"][0][0]["id"] - motion = self.assert_model_exists( - f"motion/{created_id}", {"additional_submitter": "Sue B. Mid-Edit"} + self.assert_model_exists( + f"motion/{created_id}", + { + "additional_submitter": "Worship the administrator (he · is, very, good), User 2, Sue B. Mid-Edit" + }, ) - assert len(motion["submitter_ids"]) == 2 - submitter1 = self.assert_model_exists( - fqid_from_collection_and_id("motion_submitter", motion["submitter_ids"][0]) - )["meeting_user_id"] - submitter2 = self.assert_model_exists( - fqid_from_collection_and_id("motion_submitter", motion["submitter_ids"][1]) - )["meeting_user_id"] - expected_meeting_user_ids = [1, 3] - assert ( - submitter1 in expected_meeting_user_ids - and submitter2 in expected_meeting_user_ids - and submitter1 != submitter2 + + def test_name_generation(self) -> None: + self.set_models(self.test_model) + extra_user_data: list[tuple[dict[str, Any], ...]] = [ + ({"title": "He is", "pronoun": "he"}, {"structure_level_ids": [1, 3]}), + ({"first_name": "King", "pronoun": "Kong"}, {"structure_level_ids": [2]}), + ( + { + "last_name": "Good", + }, + {"structure_level_ids": [3]}, + ), + ( + { + "title": "He,", + "first_name": "she,", + "last_name": "it", + "pronoun": "ein 's' muss mit", + }, + {}, + ), + ( + { + "title": "Grandma", + "first_name": "not", + "last_name": "see", + }, + {"structure_level_ids": []}, + ), + ] + amount = len(extra_user_data) + extra_user_ids = [self.create_user(f"user{i}", [111]) for i in range(amount)] + self.set_models( + { + "motion/12": { + "submitter_ids": list(range(12, 13 + amount)), + "additional_submitter": "Sue B. Mid-Edit", + }, + "motion_submitter/12": { + "meeting_user_id": 1, + "motion_id": 12, + "meeting_id": 1, + }, + "meeting/1": { + "meeting_user_ids": [1, *range(3, 3 + amount)], + "motion_submitter_ids": list(range(12, 13 + amount)), + "user_ids": [1, *extra_user_ids], + }, + **{ + f"user/{extra_user_ids[i]}": { + "meeting_user_ids": [i + 3], + "meeting_ids": [1], + **extra_user_data[i][0], + } + for i in range(amount) + }, + **{ + f"meeting_user/{i + 3}": { + "motion_submitter_ids": [13 + i], + **extra_user_data[i][1], + } + for i in range(amount) + }, + **{ + f"motion_submitter/{13 + i}": { + "meeting_user_id": i + 3, + "motion_id": 12, + "meeting_id": 1, + } + for i in range(amount) + }, + } + ) + response = self.request( + "motion.create_forwarded", + { + "title": "test_Xcdfgee", + "meeting_id": 2, + "origin_id": 12, + "text": "test", + "reason": "reason_jLvcgAMx", + "use_original_submitter": True, + }, + ) + self.assert_status_code(response, 200) + created_id = response.json["results"][0][0]["id"] + self.assert_model_exists( + f"motion/{created_id}", + { + "additional_submitter": "Worship the administrator (he · is, very, good), He is User 2 (he · is, good), King (Kong · very), Good (good), He, she, it (ein 's' muss mit), Grandma not see, Sue B. Mid-Edit" + }, ) From d348cb4217af3625762494950eb3fc25021ad206 Mon Sep 17 00:00:00 2001 From: Luisa Date: Thu, 27 Jun 2024 17:57:58 +0200 Subject: [PATCH 06/14] Remove forward user relation and add migration --- global/data/example-data.json | 2 +- global/data/initial-data.json | 2 +- .../action/actions/meeting/import_.py | 1 - .../action/actions/motion/create_forwarded.py | 2 +- .../action/actions/user/create.py | 2 +- .../user/create_update_permissions_mixin.py | 4 +- .../migrations/0054_remove_forwarding_user.py | 28 ++++++++ openslides_backend/models/models.py | 4 -- tests/system/action/meeting/test_import.py | 56 +++++++-------- tests/system/action/user/test_create.py | 22 +++--- .../test_0054_remove_forwarding_user.py | 68 +++++++++++++++++++ 11 files changed, 141 insertions(+), 50 deletions(-) create mode 100644 openslides_backend/migrations/migrations/0054_remove_forwarding_user.py create mode 100644 tests/system/migrations/test_0054_remove_forwarding_user.py diff --git a/global/data/example-data.json b/global/data/example-data.json index 13ef253f0..92213daea 100644 --- a/global/data/example-data.json +++ b/global/data/example-data.json @@ -1,5 +1,5 @@ { - "_migration_index": 54, + "_migration_index": 55, "organization": { "1": { "id": 1, diff --git a/global/data/initial-data.json b/global/data/initial-data.json index 4983d8cbf..745d7e429 100644 --- a/global/data/initial-data.json +++ b/global/data/initial-data.json @@ -1,5 +1,5 @@ { - "_migration_index": 54, + "_migration_index": 55, "organization": { "1": { "id": 1, diff --git a/openslides_backend/action/actions/meeting/import_.py b/openslides_backend/action/actions/meeting/import_.py index 21624c0b6..8a6bbdc1d 100644 --- a/openslides_backend/action/actions/meeting/import_.py +++ b/openslides_backend/action/actions/meeting/import_.py @@ -162,7 +162,6 @@ def remove_not_allowed_fields(self, instance: dict[str, Any]) -> None: user.pop("organization_management_level", None) user.pop("committee_ids", None) user.pop("committee_management_ids", None) - user.pop("forwarding_committee_ids", None) self.get_meeting_from_json(json_data).pop("organization_tag_ids", None) json_data.pop("action_worker", None) json_data.pop("import_preview", None) diff --git a/openslides_backend/action/actions/motion/create_forwarded.py b/openslides_backend/action/actions/motion/create_forwarded.py index fcbeea35d..9e188a10c 100644 --- a/openslides_backend/action/actions/motion/create_forwarded.py +++ b/openslides_backend/action/actions/motion/create_forwarded.py @@ -234,7 +234,7 @@ def check_for_origin_id(self, instance: dict[str, Any]) -> dict[str, Any]: fqid_from_collection_and_id( "committee", forwarded_from_meeting["committee_id"] ), - ["id", "name", "forward_to_committee_ids", "forwarding_user_id"], + ["id", "name", "forward_to_committee_ids"], ) if meeting["committee_id"] not in committee.get("forward_to_committee_ids", []): raise ActionException( diff --git a/openslides_backend/action/actions/user/create.py b/openslides_backend/action/actions/user/create.py index 2dfaf6819..5c682f7fe 100644 --- a/openslides_backend/action/actions/user/create.py +++ b/openslides_backend/action/actions/user/create.py @@ -51,7 +51,7 @@ class UserCreate( "is_present_in_meeting_ids", "committee_management_ids", "is_demo_user", - "forwarding_committee_ids", + # "forwarding_committee_ids", "saml_id", "member_number", ], diff --git a/openslides_backend/action/actions/user/create_update_permissions_mixin.py b/openslides_backend/action/actions/user/create_update_permissions_mixin.py index 9a599c5c1..110d9fddb 100644 --- a/openslides_backend/action/actions/user/create_update_permissions_mixin.py +++ b/openslides_backend/action/actions/user/create_update_permissions_mixin.py @@ -213,8 +213,8 @@ def check_permissions(self, instance: dict[str, Any]) -> None: """ self.assert_not_anonymous() - if "forwarding_committee_ids" in instance: - raise PermissionDenied("forwarding_committee_ids is not allowed.") + # if "forwarding_committee_ids" in instance: + # raise PermissionDenied("forwarding_committee_ids is not allowed.") if not hasattr(self, "permstore"): self.permstore = PermissionVarStore( diff --git a/openslides_backend/migrations/migrations/0054_remove_forwarding_user.py b/openslides_backend/migrations/migrations/0054_remove_forwarding_user.py new file mode 100644 index 000000000..b37176aa2 --- /dev/null +++ b/openslides_backend/migrations/migrations/0054_remove_forwarding_user.py @@ -0,0 +1,28 @@ +from datastore.migrations import BaseModelMigration +from datastore.shared.util import fqid_from_collection_and_id +from datastore.writer.core import BaseRequestEvent, RequestUpdateEvent + + +class Migration(BaseModelMigration): + """ + This migration removes the forwarding user relation + """ + + target_migration_index = 55 + + def migrate_models(self) -> list[BaseRequestEvent] | None: + events: list[BaseRequestEvent] = [] + for collection, field in [ + ("user", "forwarding_committee_ids"), + ("committee", "forwarding_user_id"), + ]: + data = self.reader.get_all(collection, ["id", field]) + for id_, model in data.items(): + if field in model: + events.append( + RequestUpdateEvent( + fqid_from_collection_and_id(collection, id_), + {field: None}, + ) + ) + return events diff --git a/openslides_backend/models/models.py b/openslides_backend/models/models.py index 75cb8a57e..cf857cc03 100644 --- a/openslides_backend/models/models.py +++ b/openslides_backend/models/models.py @@ -117,9 +117,6 @@ class User(Model): constraints={"description": "Calculated field."}, ) committee_management_ids = fields.RelationListField(to={"committee": "manager_ids"}) - forwarding_committee_ids = fields.RelationListField( - to={"committee": "forwarding_user_id"} - ) meeting_user_ids = fields.RelationListField( to={"meeting_user": "user_id"}, on_delete=fields.OnDelete.CASCADE ) @@ -296,7 +293,6 @@ class Committee(Model): receive_forwardings_from_committee_ids = fields.RelationListField( to={"committee": "forward_to_committee_ids"} ) - forwarding_user_id = fields.RelationField(to={"user": "forwarding_committee_ids"}) organization_tag_ids = fields.RelationListField( to={"organization_tag": "tagged_ids"} ) diff --git a/tests/system/action/meeting/test_import.py b/tests/system/action/meeting/test_import.py index 1084f3dc0..45cdfdae7 100644 --- a/tests/system/action/meeting/test_import.py +++ b/tests/system/action/meeting/test_import.py @@ -823,34 +823,34 @@ def test_check_usernames_new_and_twice(self) -> None: ) self.assert_model_not_exists("user/3") - def test_with_forwarding_committees(self) -> None: - request_data = self.create_request_data( - { - "user": { - "2": self.get_user_data( - 2, - { - "username": "user2", - "last_name": "new user", - "email": "tesT@email.de", - "forwarding_committee_ids": [4], - }, - ) - } - } - ) - - response = self.request("meeting.import", request_data) - self.assert_status_code(response, 200) - user2 = self.assert_model_exists( - "user/3", - { - "username": "user2", - "last_name": "new user", - "email": "tesT@email.de", - }, - ) - assert not user2.get("forwarding_committee_ids") + # def test_with_forwarding_committees(self) -> None: + # request_data = self.create_request_data( + # { + # "user": { + # "2": self.get_user_data( + # 2, + # { + # "username": "user2", + # "last_name": "new user", + # "email": "tesT@email.de", + # "forwarding_committee_ids": [4], + # }, + # ) + # } + # } + # ) + + # response = self.request("meeting.import", request_data) + # self.assert_status_code(response, 200) + # user2 = self.assert_model_exists( + # "user/3", + # { + # "username": "user2", + # "last_name": "new user", + # "email": "tesT@email.de", + # }, + # ) + # assert not user2.get("forwarding_committee_ids") def test_check_negative_default_vote_weight(self) -> None: request_data = self.create_request_data({}) diff --git a/tests/system/action/user/test_create.py b/tests/system/action/user/test_create.py index 121ba0d08..a32881564 100644 --- a/tests/system/action/user/test_create.py +++ b/tests/system/action/user/test_create.py @@ -1217,17 +1217,17 @@ def test_create_default_vote_weight_none(self) -> None: user = self.get_model("user/2") assert "default_vote_weight" not in user - def test_create_forwarding_committee_ids_not_allowed(self) -> None: - self.set_models({"meeting/1": {"is_active_in_organization_id": 1}}) - response = self.request( - "user.create", - { - "username": "test_Xcdfgee", - "forwarding_committee_ids": [], - }, - ) - self.assert_status_code(response, 403) - assert "forwarding_committee_ids is not allowed." in response.json["message"] + # def test_create_forwarding_committee_ids_not_allowed(self) -> None: + # self.set_models({"meeting/1": {"is_active_in_organization_id": 1}}) + # response = self.request( + # "user.create", + # { + # "username": "test_Xcdfgee", + # "forwarding_committee_ids": [], + # }, + # ) + # self.assert_status_code(response, 403) + # assert "forwarding_committee_ids is not allowed." in response.json["message"] def test_create_negative_vote_weight(self) -> None: self.set_models( diff --git a/tests/system/migrations/test_0054_remove_forwarding_user.py b/tests/system/migrations/test_0054_remove_forwarding_user.py new file mode 100644 index 000000000..7b84e0a58 --- /dev/null +++ b/tests/system/migrations/test_0054_remove_forwarding_user.py @@ -0,0 +1,68 @@ +def test_migration_forwarding_migration(write, finalize, assert_model): + committee_ids_by_user_id: dict[int, list[int]] = {2: [1, 2, 3], 3: [4], 4: []} + write( + { + "type": "create", + "fqid": "organization/1", + "fields": { + "id": 1, + "user_ids": [2, 3, 4, 5], + "committee_ids": [1, 2, 3, 4, 5], + }, + }, + *[ + { + "type": "create", + "fqid": f"user/{user_id}", + "fields": { + "id": user_id, + "organization_id": 1, + "forwarding_committee_ids": committee_ids, + }, + } + for user_id, committee_ids in committee_ids_by_user_id.items() + ], + *[ + { + "type": "create", + "fqid": f"committee/{committee_id}", + "fields": { + "id": committee_id, + "organization_id": 1, + "forwarding_user_id": user_id, + }, + } + for user_id, committee_ids in committee_ids_by_user_id.items() + for committee_id in committee_ids + ], + { + "type": "create", + "fqid": "user/5", + "fields": { + "id": 5, + "organization_id": 1, + }, + }, + { + "type": "create", + "fqid": "committee/5", + "fields": { + "id": 5, + "organization_id": 1, + }, + }, + ) + + finalize("0054_remove_forwarding_user") + + for collection, id_ in [ + *[("user", id_) for id_ in range(2, 6)], + *[("committee", id_) for id_ in range(1, 6)], + ]: + assert_model( + f"{collection}/{id_}", + { + "id": id_, + "organization_id": 1, + }, + ) From ba60fc1a3227b886aeee5eefc8a1ab372a7592e9 Mon Sep 17 00:00:00 2001 From: Luisa Date: Fri, 28 Jun 2024 11:17:31 +0200 Subject: [PATCH 07/14] Amend wiki --- docs/actions/motion.create_forwarded.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docs/actions/motion.create_forwarded.md b/docs/actions/motion.create_forwarded.md index 4754fca6c..6bcad2b8a 100644 --- a/docs/actions/motion.create_forwarded.md +++ b/docs/actions/motion.create_forwarded.md @@ -27,17 +27,13 @@ The original motion must be updated as well (this is done by the automatic relat * The unique `id` of the newly created motion has to be linked to the _origin motion_s `derived_motion_ids` field. * Deleting the newly created motion has to ensure that the corresponding entry was removed from the _origin motion_s `derived_motion_ids` field -The optional flags `use_original_submitter` and `use_original_number` will cause the original submitters and original numbers to be used in the new motion respectively. In case of the submitters, the action will use the origin meetings meeting_user for the `meeting_user_id` field of the new submitter instance in the target meeting. +The optional flags `use_original_submitter` and `use_original_number` will cause the original submitters and original numbers to be used in the new motion respectively. In case of the submitters, the action will generate the full name of the submitters and write the entire list of them and the value of the origin motions `additional_submitter` comma separated into the new motions `additional_submitter` field. If `use_original_submitter` is false the name of the origin motions committee will be written into the `additional_submitter` field instead ### Forwarding tree fields * `all_origin_ids` of the newly created motion must be set to `all_origin_ids` of the origin motion plus the given `origin_id`. It is important that the id is appended at the end of the list, since the order of this field represents the order of the tree in case a motion of the tree is deleted. * The id of the newly created motion must be added to the `all_derived_motion_ids` field of all motions in the `all_origin_ids` field of this motion. Order is not important here. -### New user in receiving meeting - -* A new user on committee level will be generated automatically _inactive_ with meeting standard group and committee's name. This user is stored in the committee as `forwarding_user` and used in further forwardings, if necessary with new membership in standard group of new meetings. - ### State needs to allow forwarding * The origin state must allow forwarding (`allow_motion_forwarding` must be set to True). From 257f5d0569dc4fd66c72831d4193b5c2ef9980aa Mon Sep 17 00:00:00 2001 From: Luisa Date: Tue, 2 Jul 2024 16:53:02 +0200 Subject: [PATCH 08/14] Start implementing amendment forwarding --- docs/actions/motion.create_forwarded.md | 5 + .../actions/motion/base_create_forwarded.py | 346 ++++++++++++++++++ .../action/actions/motion/create_forwarded.py | 279 +------------- .../motion/create_forwarded_amendment.py | 45 +++ .../action/motion/test_create_forwarded.py | 47 ++- 5 files changed, 457 insertions(+), 265 deletions(-) create mode 100644 openslides_backend/action/actions/motion/base_create_forwarded.py create mode 100644 openslides_backend/action/actions/motion/create_forwarded_amendment.py diff --git a/docs/actions/motion.create_forwarded.md b/docs/actions/motion.create_forwarded.md index 6bcad2b8a..44ca2527b 100644 --- a/docs/actions/motion.create_forwarded.md +++ b/docs/actions/motion.create_forwarded.md @@ -11,6 +11,7 @@ reason: HTML; use_original_submitter: boolean; use_original_number: boolean; + with_amendments: boolean; } ``` @@ -29,6 +30,10 @@ The original motion must be updated as well (this is done by the automatic relat The optional flags `use_original_submitter` and `use_original_number` will cause the original submitters and original numbers to be used in the new motion respectively. In case of the submitters, the action will generate the full name of the submitters and write the entire list of them and the value of the origin motions `additional_submitter` comma separated into the new motions `additional_submitter` field. If `use_original_submitter` is false the name of the origin motions committee will be written into the `additional_submitter` field instead +If `with_amendments` is set to True, all amendments of the motion, that have a state that can forward, will also be forwarded to the target meeting and connected to the newly forwarded lead motion + +If the forwarded amendments have amendments themselves, those will also be treated the same way + ### Forwarding tree fields * `all_origin_ids` of the newly created motion must be set to `all_origin_ids` of the origin motion plus the given `origin_id`. It is important that the id is appended at the end of the list, since the order of this field represents the order of the tree in case a motion of the tree is deleted. diff --git a/openslides_backend/action/actions/motion/base_create_forwarded.py b/openslides_backend/action/actions/motion/base_create_forwarded.py new file mode 100644 index 000000000..019a0212d --- /dev/null +++ b/openslides_backend/action/actions/motion/base_create_forwarded.py @@ -0,0 +1,346 @@ +import time +from collections import defaultdict +from typing import Any + +from openslides_backend.action.actions.motion.mixins import TextHashMixin +from openslides_backend.shared.typing import HistoryInformation + +from ....models.models import Motion +from ....permissions.permission_helper import has_perm +from ....permissions.permissions import Permissions +from ....services.datastore.commands import GetManyRequest +from ....shared.exceptions import ActionException, PermissionDenied +from ....shared.filters import FilterOperator, Or +from ....shared.patterns import fqid_from_collection_and_id +from ...util.default_schema import DefaultSchema +from ...util.register import register_action +from ...util.typing import ActionData, ActionResultElement, ActionResults +from .create_base import MotionCreateBase +from ...action import Action +from ....shared.interfaces.write_request import WriteRequest + + +class BaseMotionCreateForwarded(TextHashMixin, MotionCreateBase): + """ + Base create action for forwarded motions. + """ + + def prefetch(self, action_data: ActionData) -> None: + self.datastore.get_many( + [ + GetManyRequest( + "meeting", + list( + { + meeting_id + for instance in action_data + if (meeting_id := instance.get("meeting_id")) + } + ), + [ + "id", + "is_active_in_organization_id", + "name", + "motions_default_workflow_id", + "committee_id", + "default_group_id", + "motion_submitter_ids", + "motions_number_type", + "motions_number_min_digits", + "agenda_item_creation", + "list_of_speakers_initially_closed", + "list_of_speakers_ids", + "motion_ids", + ], + ), + GetManyRequest( + "motion", + list( + { + origin_id + for instance in action_data + if (origin_id := instance.get("origin_id")) + } + ), + [ + "meeting_id", + "lead_motion_id", + "statute_paragraph_id", + "state_id", + "all_origin_ids", + "derived_motion_ids", + "all_derived_motion_ids", + ], + ), + ] + ) + + def get_user_verbose_names(self, meeting_user_ids: list[int]) -> str | None: + meeting_users = self.datastore.get_many( + [ + GetManyRequest( + "meeting_user", meeting_user_ids, ["user_id", "structure_level_ids"] + ) + ] + )["meeting_user"] + user_ids = [ + user_id + for meeting_user in meeting_users.values() + if (user_id := meeting_user.get("user_id")) + ] + if not len(user_ids): + return None + requests = [ + GetManyRequest( + "user", user_ids, ["id", "first_name", "last_name", "title", "pronoun"] + ) + ] + if structure_level_ids := list( + { + structure_level_id + for meeting_user in meeting_users.values() + for structure_level_id in meeting_user.get("structure_level_ids", []) + } + ): + requests.append( + GetManyRequest("structure_level", structure_level_ids, ["name"]) + ) + user_data = self.datastore.get_many(requests) + users = user_data["user"] + structure_levels = user_data["structure_level"] + names = [] + for meeting_user_id in meeting_user_ids: + meeting_user = meeting_users[meeting_user_id] + user = users.get(meeting_user.get("user_id", 0)) + if user: + additional_info: list[str] = [] + if pronoun := user.get("pronoun"): + additional_info = [pronoun] + if sl_ids := meeting_user.get("structure_level_ids"): + if slnames := ", ".join( + name + for structure_level_id in sl_ids + if ( + name := structure_levels.get(structure_level_id, {}).get( + "name" + ) + ) + ): + additional_info.append(slnames) + suffix = " · ".join(additional_info) + if suffix: + suffix = f"({suffix})" + if not any(user.get(field) for field in ["first_name", "last_name"]): + short_name = f"User {user['id']}" + else: + short_name = f"{user.get('first_name', '')} {user.get('last_name', '')}".strip() + long_name = f"{user.get('title', '')} {short_name} {suffix}".strip() + names.append(long_name) + return ", ".join(names) + + def perform(self, action_data: ActionData, user_id: int, internal: bool = False) -> tuple[WriteRequest | None, ActionResults | None]: + self.id_to_result_extra_data: dict[int,dict[str,Any]] = {} + return super().perform(action_data, user_id, internal) + + def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: + meeting = self.datastore.get( + fqid_from_collection_and_id("meeting", instance["meeting_id"]), + [ + "motions_default_workflow_id", + "motions_default_amendment_workflow_id" + ], + ) + self.set_state_from_workflow(instance, meeting) + committee = self.check_for_origin_id(instance) + self.check_state_allow_forwarding(instance) + + if instance.pop("use_original_submitter", None): + submitters = list( + self.datastore.filter( + "motion_submitter", + FilterOperator("motion_id", "=", instance["origin_id"]), + ["meeting_user_id"], + ).values() + ) + submitters = sorted(submitters, key=lambda x: x.get("weight", 10000)) + meeting_user_ids = [ + meeting_user_id + for submitter in submitters + if (meeting_user_id := submitter.get("meeting_user_id")) + ] + if len(meeting_user_ids): + instance["additional_submitter"] = self.get_user_verbose_names( + meeting_user_ids + ) + text_submitter = self.datastore.get( + fqid_from_collection_and_id("motion", instance["origin_id"]), + ["additional_submitter"], + ).get("additional_submitter") + if text_submitter: + if instance.get("additional_submitter"): + instance["additional_submitter"] += ", " + text_submitter + else: + instance["additional_submitter"] = text_submitter + else: + name = committee.get("name", f"Committee {committee['id']}") + instance["additional_submitter"] = name + + self.set_sequential_number(instance) + self.handle_number(instance) + self.set_origin_ids(instance) + self.set_text_hash(instance) + instance["forwarded"] = round(time.time()) + amendment_ids = self.datastore.get(fqid_from_collection_and_id("motion", instance["origin_id"]), ["amendment_ids"]).get("amendment_ids", []) + if self.should_forward_amendments(instance): + new_amendments = self.datastore.get_many([GetManyRequest("motion", amendment_ids, [ + "title", + "text", + "amendment_paragraphs", + "reason", + "id", + "state_id" + ])])["motion"] + total = len(new_amendments) + state_ids = {state_id for amendment in new_amendments.values() if (state_id:= amendment.get("state_id"))} + if len(state_ids): + states = self.datastore.get_many([GetManyRequest("motion_state",list(state_ids), ["allow_motion_forwarding"])])["motion_state"] + else: + states = {} + states = {id_:state for id_, state in states.items() if state.get("allow_motion_forwarding")} + for amendment in new_amendments.values(): + if not ((state_id := amendment.pop("state_id", None)) and state_id in states): + new_amendments.pop(amendment["id"]) + amendment_data = new_amendments.values() + for amendment in amendment_data: + amendment.update({ + "origin_id": amendment["id"], + "meeting_id": instance["meeting_id"], + "use_original_submitter": instance.get("use_original_submitter", False), + "use_original_number": instance.get("use_original_number", False) + }) + amendment.pop("meta_position", 0) + amendment.pop("id") + amendment_results = self.create_amendments(list(amendment_data)) or [] + instance["amendment_ids"] = [result["id"] for result in amendment_results if result] + self.id_to_result_extra_data[instance["id"]] = { + "non_forwarded_amendment_amount": total-len(amendment_results), + "amendment_result_data": amendment_results + } + else: + self.id_to_result_extra_data[instance["id"]] = { + "non_forwarded_amendment_amount": len(amendment_ids), + "amendment_result_data": [] + } + return instance + + def create_amendments(self, amendment_data: ActionData) -> ActionResults | None: + raise ActionException("Not implemented") + + def create_action_result_element(self, instance: dict[str, Any]) -> ActionResultElement | None: + result = super().create_action_result_element(instance) or {} + result.update(self.id_to_result_extra_data.get(result["id"], {})) + return result + + def handle_number(self, instance: dict[str, Any]) -> dict[str, Any]: + origin = self.datastore.get( + fqid_from_collection_and_id("motion", instance["origin_id"]), ["number"] + ) + if instance.pop("use_original_number", None) and (num := origin.get("number")): + number = self.get_clean_number(num, instance["meeting_id"]) + self.set_created_last_modified(instance) + instance["number"] = number + else: + self.set_created_last_modified_and_number(instance) + return instance + + def get_clean_number(self, number: str, meeting_id: int) -> str: + new_number = number + next_identifier = 1 + while not self._check_if_unique(new_number, meeting_id, None): + new_number = f"{number}-{next_identifier}" + next_identifier += 1 + return new_number + + def check_for_origin_id(self, instance: dict[str, Any]) -> dict[str, Any]: + meeting = self.datastore.get( + fqid_from_collection_and_id("meeting", instance["meeting_id"]), + ["committee_id"], + ) + forwarded_from = self.datastore.get( + fqid_from_collection_and_id("motion", instance["origin_id"]), + ["meeting_id"], + ) + forwarded_from_meeting = self.datastore.get( + fqid_from_collection_and_id("meeting", forwarded_from["meeting_id"]), + ["committee_id"], + ) + # use the forwarding user id and id later in the handle forwarding user + # code. + committee = self.datastore.get( + fqid_from_collection_and_id( + "committee", forwarded_from_meeting["committee_id"] + ), + ["id", "name", "forward_to_committee_ids"], + ) + if meeting["committee_id"] not in committee.get("forward_to_committee_ids", []): + raise ActionException( + f"Committee id {meeting['committee_id']} not in {committee.get('forward_to_committee_ids', [])}" + ) + return committee + + def should_forward_amendments(self, instance: dict[str, Any]) -> bool: + raise ActionException("Not implemented") + + def check_permissions(self, instance: dict[str, Any]) -> None: + origin = self.datastore.get( + fqid_from_collection_and_id(self.model.collection, instance["origin_id"]), + ["meeting_id"], + lock_result=False, + ) + perm_origin = Permissions.Motion.CAN_FORWARD + if not has_perm( + self.datastore, self.user_id, perm_origin, origin["meeting_id"] + ): + msg = f"You are not allowed to perform action {self.name}." + msg += f" Missing permission: {perm_origin}" + raise PermissionDenied(msg) + + def set_origin_ids(self, instance: dict[str, Any]) -> None: + if instance.get("origin_id"): + origin = self.datastore.get( + fqid_from_collection_and_id("motion", instance["origin_id"]), + ["all_origin_ids", "meeting_id"], + ) + instance["origin_meeting_id"] = origin["meeting_id"] + instance["all_origin_ids"] = origin.get("all_origin_ids", []) + instance["all_origin_ids"].append(instance["origin_id"]) + + def check_state_allow_forwarding(self, instance: dict[str, Any]) -> None: + origin = self.datastore.get( + fqid_from_collection_and_id(self.model.collection, instance["origin_id"]), + ["state_id"], + ) + state = self.datastore.get( + fqid_from_collection_and_id("motion_state", origin["state_id"]), + ["allow_motion_forwarding"], + ) + if not state.get("allow_motion_forwarding"): + raise ActionException("State doesn't allow to forward motion.") + + def get_history_information(self) -> HistoryInformation | None: + forwarded_entries = defaultdict(list) + for instance in self.instances: + forwarded_entries[ + fqid_from_collection_and_id("motion", instance["origin_id"]) + ].extend( + [ + "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 + } \ No newline at end of file diff --git a/openslides_backend/action/actions/motion/create_forwarded.py b/openslides_backend/action/actions/motion/create_forwarded.py index 9e188a10c..1a1bb4293 100644 --- a/openslides_backend/action/actions/motion/create_forwarded.py +++ b/openslides_backend/action/actions/motion/create_forwarded.py @@ -10,18 +10,22 @@ from ....permissions.permissions import Permissions from ....services.datastore.commands import GetManyRequest from ....shared.exceptions import ActionException, PermissionDenied -from ....shared.filters import FilterOperator +from ....shared.filters import FilterOperator, Or from ....shared.patterns import fqid_from_collection_and_id from ...util.default_schema import DefaultSchema from ...util.register import register_action -from ...util.typing import ActionData +from ...util.typing import ActionData, ActionResultElement, ActionResults +from ...util.action_type import ActionType from .create_base import MotionCreateBase +from .base_create_forwarded import BaseMotionCreateForwarded +from .create_forwarded_amendment import MotionCreateForwardedAmendment @register_action("motion.create_forwarded") -class MotionCreateForwarded(TextHashMixin, MotionCreateBase): +class MotionCreateForwarded(BaseMotionCreateForwarded): """ - Create action for forwarded motions. + Create action for forwarded amendments. + Result amendment will not have a lead_motion_id yet, that will have to be set via the calling action. """ schema = DefaultSchema(Motion()).get_create_schema( @@ -30,231 +34,12 @@ class MotionCreateForwarded(TextHashMixin, MotionCreateBase): additional_optional_fields={ "use_original_submitter": {"type": "boolean"}, "use_original_number": {"type": "boolean"}, + "with_amendments": {"type": "boolean"}, }, ) - def prefetch(self, action_data: ActionData) -> None: - self.datastore.get_many( - [ - GetManyRequest( - "meeting", - list( - { - meeting_id - for instance in action_data - if (meeting_id := instance.get("meeting_id")) - } - ), - [ - "id", - "is_active_in_organization_id", - "name", - "motions_default_workflow_id", - "committee_id", - "default_group_id", - "motion_submitter_ids", - "motions_number_type", - "motions_number_min_digits", - "agenda_item_creation", - "list_of_speakers_initially_closed", - "list_of_speakers_ids", - "motion_ids", - ], - ), - GetManyRequest( - "motion", - list( - { - origin_id - for instance in action_data - if (origin_id := instance.get("origin_id")) - } - ), - [ - "meeting_id", - "lead_motion_id", - "statute_paragraph_id", - "state_id", - "all_origin_ids", - "derived_motion_ids", - "all_derived_motion_ids", - ], - ), - ] - ) - - def get_user_verbose_names(self, meeting_user_ids: list[int]) -> str | None: - meeting_users = self.datastore.get_many( - [ - GetManyRequest( - "meeting_user", meeting_user_ids, ["user_id", "structure_level_ids"] - ) - ] - )["meeting_user"] - user_ids = [ - user_id - for meeting_user in meeting_users.values() - if (user_id := meeting_user.get("user_id")) - ] - if not len(user_ids): - return None - requests = [ - GetManyRequest( - "user", user_ids, ["id", "first_name", "last_name", "title", "pronoun"] - ) - ] - if structure_level_ids := list( - { - structure_level_id - for meeting_user in meeting_users.values() - for structure_level_id in meeting_user.get("structure_level_ids", []) - } - ): - requests.append( - GetManyRequest("structure_level", structure_level_ids, ["name"]) - ) - user_data = self.datastore.get_many(requests) - users = user_data["user"] - structure_levels = user_data["structure_level"] - names = [] - for meeting_user_id in meeting_user_ids: - meeting_user = meeting_users[meeting_user_id] - user = users.get(meeting_user.get("user_id", 0)) - if user: - additional_info: list[str] = [] - if pronoun := user.get("pronoun"): - additional_info = [pronoun] - if sl_ids := meeting_user.get("structure_level_ids"): - if slnames := ", ".join( - name - for structure_level_id in sl_ids - if ( - name := structure_levels.get(structure_level_id, {}).get( - "name" - ) - ) - ): - additional_info.append(slnames) - suffix = " · ".join(additional_info) - if suffix: - suffix = f"({suffix})" - if not any(user.get(field) for field in ["first_name", "last_name"]): - short_name = f"User {user['id']}" - else: - short_name = f"{user.get('first_name', '')} {user.get('last_name', '')}".strip() - long_name = f"{user.get('title', '')} {short_name} {suffix}".strip() - names.append(long_name) - return ", ".join(names) - - def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: - meeting = self.datastore.get( - fqid_from_collection_and_id("meeting", instance["meeting_id"]), - [ - "motions_default_workflow_id", - ], - ) - self.set_state_from_workflow(instance, meeting) - committee = self.check_for_origin_id(instance) - self.check_state_allow_forwarding(instance) - - if instance.pop("use_original_submitter", None): - submitters = list( - self.datastore.filter( - "motion_submitter", - FilterOperator("motion_id", "=", instance["origin_id"]), - ["meeting_user_id"], - ).values() - ) - submitters = sorted(submitters, key=lambda x: x.get("weight", 10000)) - meeting_user_ids = [ - meeting_user_id - for submitter in submitters - if (meeting_user_id := submitter.get("meeting_user_id")) - ] - if len(meeting_user_ids): - instance["additional_submitter"] = self.get_user_verbose_names( - meeting_user_ids - ) - text_submitter = self.datastore.get( - fqid_from_collection_and_id("motion", instance["origin_id"]), - ["additional_submitter"], - ).get("additional_submitter") - if text_submitter: - if instance.get("additional_submitter"): - instance["additional_submitter"] += ", " + text_submitter - else: - instance["additional_submitter"] = text_submitter - else: - name = committee.get("name", f"Committee {committee['id']}") - instance["additional_submitter"] = name - - self.set_sequential_number(instance) - self.handle_number(instance) - self.set_origin_ids(instance) - self.set_text_hash(instance) - instance["forwarded"] = round(time.time()) - return instance - - def handle_number(self, instance: dict[str, Any]) -> dict[str, Any]: - origin = self.datastore.get( - fqid_from_collection_and_id("motion", instance["origin_id"]), ["number"] - ) - if instance.pop("use_original_number", None) and (num := origin.get("number")): - number = self.get_clean_number(num, instance["meeting_id"]) - self.set_created_last_modified(instance) - instance["number"] = number - else: - self.set_created_last_modified_and_number(instance) - return instance - - def get_clean_number(self, number: str, meeting_id: int) -> str: - new_number = number - next_identifier = 1 - while not self._check_if_unique(new_number, meeting_id, None): - new_number = f"{number}-{next_identifier}" - next_identifier += 1 - return new_number - - def check_for_origin_id(self, instance: dict[str, Any]) -> dict[str, Any]: - meeting = self.datastore.get( - fqid_from_collection_and_id("meeting", instance["meeting_id"]), - ["committee_id"], - ) - forwarded_from = self.datastore.get( - fqid_from_collection_and_id("motion", instance["origin_id"]), - ["meeting_id"], - ) - forwarded_from_meeting = self.datastore.get( - fqid_from_collection_and_id("meeting", forwarded_from["meeting_id"]), - ["committee_id"], - ) - # use the forwarding user id and id later in the handle forwarding user - # code. - committee = self.datastore.get( - fqid_from_collection_and_id( - "committee", forwarded_from_meeting["committee_id"] - ), - ["id", "name", "forward_to_committee_ids"], - ) - if meeting["committee_id"] not in committee.get("forward_to_committee_ids", []): - raise ActionException( - f"Committee id {meeting['committee_id']} not in {committee.get('forward_to_committee_ids', [])}" - ) - return committee - def check_permissions(self, instance: dict[str, Any]) -> None: - origin = self.datastore.get( - fqid_from_collection_and_id(self.model.collection, instance["origin_id"]), - ["meeting_id"], - lock_result=False, - ) - perm_origin = Permissions.Motion.CAN_FORWARD - if not has_perm( - self.datastore, self.user_id, perm_origin, origin["meeting_id"] - ): - msg = f"You are not allowed to perform action {self.name}." - msg += f" Missing permission: {perm_origin}" - raise PermissionDenied(msg) + super().check_permissions(instance) # check if origin motion is amendment or statute_amendment origin = self.datastore.get( @@ -265,43 +50,9 @@ def check_permissions(self, instance: dict[str, Any]) -> None: if origin.get("lead_motion_id") or origin.get("statute_paragraph_id"): msg = "Amendments cannot be forwarded." raise PermissionDenied(msg) + + def create_amendments(self, amendment_data: ActionData) -> ActionResults | None: + return self.execute_other_action(MotionCreateForwardedAmendment, amendment_data) - def set_origin_ids(self, instance: dict[str, Any]) -> None: - if instance.get("origin_id"): - origin = self.datastore.get( - fqid_from_collection_and_id("motion", instance["origin_id"]), - ["all_origin_ids", "meeting_id"], - ) - instance["origin_meeting_id"] = origin["meeting_id"] - instance["all_origin_ids"] = origin.get("all_origin_ids", []) - instance["all_origin_ids"].append(instance["origin_id"]) - - def check_state_allow_forwarding(self, instance: dict[str, Any]) -> None: - origin = self.datastore.get( - fqid_from_collection_and_id(self.model.collection, instance["origin_id"]), - ["state_id"], - ) - state = self.datastore.get( - fqid_from_collection_and_id("motion_state", origin["state_id"]), - ["allow_motion_forwarding"], - ) - if not state.get("allow_motion_forwarding"): - raise ActionException("State doesn't allow to forward motion.") - - def get_history_information(self) -> HistoryInformation | None: - forwarded_entries = defaultdict(list) - for instance in self.instances: - forwarded_entries[ - fqid_from_collection_and_id("motion", instance["origin_id"]) - ].extend( - [ - "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 - } + def should_forward_amendments(self, instance: dict[str, Any]) -> bool: + return instance.pop("with_amendments", False) \ No newline at end of file diff --git a/openslides_backend/action/actions/motion/create_forwarded_amendment.py b/openslides_backend/action/actions/motion/create_forwarded_amendment.py new file mode 100644 index 000000000..b190a9d26 --- /dev/null +++ b/openslides_backend/action/actions/motion/create_forwarded_amendment.py @@ -0,0 +1,45 @@ +from typing import Any +from ....shared.exceptions import PermissionDenied +from ....shared.patterns import fqid_from_collection_and_id +from .base_create_forwarded import BaseMotionCreateForwarded +from ...util.register import register_action +from ...util.default_schema import DefaultSchema +from ....models.models import Motion +from ...util.typing import ActionData, ActionResultElement, ActionResults +from ...util.action_type import ActionType + + +@register_action("motion.create_forwarded_amendment", action_type=ActionType.BACKEND_INTERNAL) +class MotionCreateForwardedAmendment(BaseMotionCreateForwarded): + """ + Create action for forwarded motions. + """ + + schema = DefaultSchema(Motion()).get_create_schema( + required_properties=["meeting_id", "title", "origin_id"], + optional_properties=["reason", "text", "amendment_paragraphs"], + additional_optional_fields={ + "use_original_submitter": {"type": "boolean"}, + "use_original_number": {"type": "boolean"}, + "with_amendments": {"type": "boolean"}, + }, + ) + + def check_permissions(self, instance: dict[str, Any]) -> None: + super().check_permissions(instance) + + # check if origin motion is normal or statute_amendment + origin = self.datastore.get( + fqid_from_collection_and_id(self.model.collection, instance["origin_id"]), + ["lead_motion_id", "statute_paragraph_id"], + lock_result=False, + ) + if not origin.get("lead_motion_id") or origin.get("statute_paragraph_id"): + msg = "Can only forward amendments in internal forward." + raise PermissionDenied(msg) + + def create_amendments(self, amendment_data: ActionData) -> ActionResults | None: + return self.execute_other_action(MotionCreateForwardedAmendment, amendment_data) + + def should_forward_amendments(self, instance: dict[str, Any]) -> bool: + return True diff --git a/tests/system/action/motion/test_create_forwarded.py b/tests/system/action/motion/test_create_forwarded.py index 23d75e10c..d1f6b68de 100644 --- a/tests/system/action/motion/test_create_forwarded.py +++ b/tests/system/action/motion/test_create_forwarded.py @@ -22,6 +22,7 @@ def setUp(self) -> None: "meeting/2": { "name": "name_SNLGsvIV", "motions_default_workflow_id": 12, + "motions_default_amendment_workflow_id": 12, "committee_id": 52, "is_active_in_organization_id": 1, "default_group_id": 112, @@ -419,7 +420,7 @@ def test_forward_with_deleted_motion_in_all_origin_ids(self) -> None: ) self.assert_status_code(response, 200) - def test_not_allowed_to_forward_amendments(self) -> None: + def test_not_allowed_to_forward_amendments_directly(self) -> None: self.set_models( { "meeting/1": { @@ -435,6 +436,7 @@ def test_not_allowed_to_forward_amendments(self) -> None: "derived_motion_ids": [], "all_origin_ids": [], "all_derived_motion_ids": [], + "amendment_ids": [11] }, "motion/11": { "title": "test11 layer 2", @@ -469,6 +471,49 @@ def test_not_allowed_to_forward_amendments(self) -> None: self.assert_status_code(response, 403) assert "Amendments cannot be forwarded." in response.json["message"] + def test_allowed_to_forward_amendments_indirectly(self) -> None: + self.set_models(self.test_model) + self.set_models( + { + "meeting/1": { + "name": "name_XDAddEAW", + "committee_id": 53, + "is_active_in_organization_id": 1, + "motion_ids": [12,13] + }, + "user/1": {"meeting_ids": [1, 2]}, + "motion/12": { + "title": "title_FcnPUXJB layer 1", + "meeting_id": 1, + "derived_motion_ids": [], + "all_origin_ids": [], + "all_derived_motion_ids": [], + "amendment_ids": [13] + }, + "motion/13": { + "title": "test11 layer 2", + "meeting_id": 1, + "derived_motion_ids": [], + "all_origin_ids": [], + "all_derived_motion_ids": [], + "lead_motion_id": 12, + "state_id": 30, + }, + } + ) + response = self.request( + "motion.create_forwarded", + { + "title": "test_foo", + "meeting_id": 2, + "origin_id": 12, + "text": "test", + "with_amendments": True + }, + ) + self.assert_status_code(response, 200) + assert False #TODO: Check resulting data, results and write a few more tests + def test_forward_to_2_meetings_1_transaction(self) -> None: """Forwarding of 1 motion to 2 meetings in 1 transaction""" self.set_models(self.test_model) From f6f8a86053828f522ce7388df5447b54e173529f Mon Sep 17 00:00:00 2001 From: Luisa Date: Wed, 3 Jul 2024 14:44:12 +0200 Subject: [PATCH 09/14] Finish implementing amendment forward --- .../actions/motion/base_create_forwarded.py | 112 ++++--- .../action/actions/motion/create_forwarded.py | 19 +- .../motion/create_forwarded_amendment.py | 17 +- .../action/motion/test_create_forwarded.py | 275 +++++++++++++++++- 4 files changed, 353 insertions(+), 70 deletions(-) diff --git a/openslides_backend/action/actions/motion/base_create_forwarded.py b/openslides_backend/action/actions/motion/base_create_forwarded.py index 019a0212d..7c537bc2b 100644 --- a/openslides_backend/action/actions/motion/base_create_forwarded.py +++ b/openslides_backend/action/actions/motion/base_create_forwarded.py @@ -5,19 +5,15 @@ from openslides_backend.action.actions.motion.mixins import TextHashMixin from openslides_backend.shared.typing import HistoryInformation -from ....models.models import Motion from ....permissions.permission_helper import has_perm from ....permissions.permissions import Permissions from ....services.datastore.commands import GetManyRequest from ....shared.exceptions import ActionException, PermissionDenied -from ....shared.filters import FilterOperator, Or +from ....shared.filters import FilterOperator +from ....shared.interfaces.write_request import WriteRequest from ....shared.patterns import fqid_from_collection_and_id -from ...util.default_schema import DefaultSchema -from ...util.register import register_action from ...util.typing import ActionData, ActionResultElement, ActionResults from .create_base import MotionCreateBase -from ...action import Action -from ....shared.interfaces.write_request import WriteRequest class BaseMotionCreateForwarded(TextHashMixin, MotionCreateBase): @@ -138,23 +134,23 @@ def get_user_verbose_names(self, meeting_user_ids: list[int]) -> str | None: names.append(long_name) return ", ".join(names) - def perform(self, action_data: ActionData, user_id: int, internal: bool = False) -> tuple[WriteRequest | None, ActionResults | None]: - self.id_to_result_extra_data: dict[int,dict[str,Any]] = {} + def perform( + self, action_data: ActionData, user_id: int, internal: bool = False + ) -> tuple[WriteRequest | None, ActionResults | None]: + self.id_to_result_extra_data: dict[int, dict[str, Any]] = {} return super().perform(action_data, user_id, internal) def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: meeting = self.datastore.get( fqid_from_collection_and_id("meeting", instance["meeting_id"]), - [ - "motions_default_workflow_id", - "motions_default_amendment_workflow_id" - ], + ["motions_default_workflow_id", "motions_default_amendment_workflow_id"], ) self.set_state_from_workflow(instance, meeting) committee = self.check_for_origin_id(instance) self.check_state_allow_forwarding(instance) + use_original_number = instance.get("use_original_number", False) - if instance.pop("use_original_submitter", None): + if use_original_submitter := instance.pop("use_original_submitter", False): submitters = list( self.datastore.filter( "motion_submitter", @@ -190,53 +186,87 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: self.set_origin_ids(instance) self.set_text_hash(instance) instance["forwarded"] = round(time.time()) - amendment_ids = self.datastore.get(fqid_from_collection_and_id("motion", instance["origin_id"]), ["amendment_ids"]).get("amendment_ids", []) + self.datastore.apply_changed_model( + fqid_from_collection_and_id("motion", instance["id"]), instance + ) + amendment_ids = self.datastore.get( + fqid_from_collection_and_id("motion", instance["origin_id"]), + ["amendment_ids"], + ).get("amendment_ids", []) if self.should_forward_amendments(instance): - new_amendments = self.datastore.get_many([GetManyRequest("motion", amendment_ids, [ - "title", - "text", - "amendment_paragraphs", - "reason", - "id", - "state_id" - ])])["motion"] + new_amendments = self.datastore.get_many( + [ + GetManyRequest( + "motion", + amendment_ids, + [ + "title", + "text", + "amendment_paragraphs", + "reason", + "id", + "state_id", + ], + ) + ] + )["motion"] total = len(new_amendments) - state_ids = {state_id for amendment in new_amendments.values() if (state_id:= amendment.get("state_id"))} + state_ids = { + state_id + for amendment in new_amendments.values() + if (state_id := amendment.get("state_id")) + } if len(state_ids): - states = self.datastore.get_many([GetManyRequest("motion_state",list(state_ids), ["allow_motion_forwarding"])])["motion_state"] + states = self.datastore.get_many( + [ + GetManyRequest( + "motion_state", list(state_ids), ["allow_motion_forwarding"] + ) + ] + )["motion_state"] else: states = {} - states = {id_:state for id_, state in states.items() if state.get("allow_motion_forwarding")} - for amendment in new_amendments.values(): - if not ((state_id := amendment.pop("state_id", None)) and state_id in states): + states = { + id_: state + for id_, state in states.items() + if state.get("allow_motion_forwarding") + } + for amendment in list(new_amendments.values()): + if not ( + (state_id := amendment.pop("state_id", None)) and state_id in states + ): new_amendments.pop(amendment["id"]) amendment_data = new_amendments.values() for amendment in amendment_data: - amendment.update({ - "origin_id": amendment["id"], - "meeting_id": instance["meeting_id"], - "use_original_submitter": instance.get("use_original_submitter", False), - "use_original_number": instance.get("use_original_number", False) - }) + amendment.update( + { + "lead_motion_id": instance["id"], + "origin_id": amendment["id"], + "meeting_id": instance["meeting_id"], + "use_original_submitter": use_original_submitter, + "use_original_number": use_original_number, + } + ) amendment.pop("meta_position", 0) amendment.pop("id") amendment_results = self.create_amendments(list(amendment_data)) or [] - instance["amendment_ids"] = [result["id"] for result in amendment_results if result] self.id_to_result_extra_data[instance["id"]] = { - "non_forwarded_amendment_amount": total-len(amendment_results), - "amendment_result_data": amendment_results + "non_forwarded_amendment_amount": total - len(amendment_results), + "amendment_result_data": amendment_results, } else: self.id_to_result_extra_data[instance["id"]] = { "non_forwarded_amendment_amount": len(amendment_ids), - "amendment_result_data": [] + "amendment_result_data": [], } return instance - + def create_amendments(self, amendment_data: ActionData) -> ActionResults | None: raise ActionException("Not implemented") - - def create_action_result_element(self, instance: dict[str, Any]) -> ActionResultElement | None: + + def create_action_result_element( + self, instance: dict[str, Any] + ) -> ActionResultElement | None: result = super().create_action_result_element(instance) or {} result.update(self.id_to_result_extra_data.get(result["id"], {})) return result @@ -343,4 +373,4 @@ def get_history_information(self) -> HistoryInformation | None: "Motion created (forwarded)" ] for instance in self.instances - } \ No newline at end of file + } diff --git a/openslides_backend/action/actions/motion/create_forwarded.py b/openslides_backend/action/actions/motion/create_forwarded.py index 1a1bb4293..bfa6e4c44 100644 --- a/openslides_backend/action/actions/motion/create_forwarded.py +++ b/openslides_backend/action/actions/motion/create_forwarded.py @@ -1,22 +1,11 @@ -import time -from collections import defaultdict from typing import Any -from openslides_backend.action.actions.motion.mixins import TextHashMixin -from openslides_backend.shared.typing import HistoryInformation - from ....models.models import Motion -from ....permissions.permission_helper import has_perm -from ....permissions.permissions import Permissions -from ....services.datastore.commands import GetManyRequest -from ....shared.exceptions import ActionException, PermissionDenied -from ....shared.filters import FilterOperator, Or +from ....shared.exceptions import PermissionDenied from ....shared.patterns import fqid_from_collection_and_id from ...util.default_schema import DefaultSchema from ...util.register import register_action -from ...util.typing import ActionData, ActionResultElement, ActionResults -from ...util.action_type import ActionType -from .create_base import MotionCreateBase +from ...util.typing import ActionData, ActionResults from .base_create_forwarded import BaseMotionCreateForwarded from .create_forwarded_amendment import MotionCreateForwardedAmendment @@ -50,9 +39,9 @@ def check_permissions(self, instance: dict[str, Any]) -> None: if origin.get("lead_motion_id") or origin.get("statute_paragraph_id"): msg = "Amendments cannot be forwarded." raise PermissionDenied(msg) - + def create_amendments(self, amendment_data: ActionData) -> ActionResults | None: return self.execute_other_action(MotionCreateForwardedAmendment, amendment_data) def should_forward_amendments(self, instance: dict[str, Any]) -> bool: - return instance.pop("with_amendments", False) \ No newline at end of file + return instance.pop("with_amendments", False) diff --git a/openslides_backend/action/actions/motion/create_forwarded_amendment.py b/openslides_backend/action/actions/motion/create_forwarded_amendment.py index b190a9d26..3f29f8133 100644 --- a/openslides_backend/action/actions/motion/create_forwarded_amendment.py +++ b/openslides_backend/action/actions/motion/create_forwarded_amendment.py @@ -1,22 +1,25 @@ from typing import Any + +from ....models.models import Motion from ....shared.exceptions import PermissionDenied from ....shared.patterns import fqid_from_collection_and_id -from .base_create_forwarded import BaseMotionCreateForwarded -from ...util.register import register_action -from ...util.default_schema import DefaultSchema -from ....models.models import Motion -from ...util.typing import ActionData, ActionResultElement, ActionResults from ...util.action_type import ActionType +from ...util.default_schema import DefaultSchema +from ...util.register import register_action +from ...util.typing import ActionData, ActionResults +from .base_create_forwarded import BaseMotionCreateForwarded -@register_action("motion.create_forwarded_amendment", action_type=ActionType.BACKEND_INTERNAL) +@register_action( + "motion.create_forwarded_amendment", action_type=ActionType.BACKEND_INTERNAL +) class MotionCreateForwardedAmendment(BaseMotionCreateForwarded): """ Create action for forwarded motions. """ schema = DefaultSchema(Motion()).get_create_schema( - required_properties=["meeting_id", "title", "origin_id"], + required_properties=["meeting_id", "title", "origin_id", "lead_motion_id"], optional_properties=["reason", "text", "amendment_paragraphs"], additional_optional_fields={ "use_original_submitter": {"type": "boolean"}, diff --git a/tests/system/action/motion/test_create_forwarded.py b/tests/system/action/motion/test_create_forwarded.py index d1f6b68de..8e05fadc8 100644 --- a/tests/system/action/motion/test_create_forwarded.py +++ b/tests/system/action/motion/test_create_forwarded.py @@ -436,7 +436,7 @@ def test_not_allowed_to_forward_amendments_directly(self) -> None: "derived_motion_ids": [], "all_origin_ids": [], "all_derived_motion_ids": [], - "amendment_ids": [11] + "amendment_ids": [11], }, "motion/11": { "title": "test11 layer 2", @@ -479,7 +479,7 @@ def test_allowed_to_forward_amendments_indirectly(self) -> None: "name": "name_XDAddEAW", "committee_id": 53, "is_active_in_organization_id": 1, - "motion_ids": [12,13] + "motion_ids": [12, 13], }, "user/1": {"meeting_ids": [1, 2]}, "motion/12": { @@ -488,16 +488,17 @@ def test_allowed_to_forward_amendments_indirectly(self) -> None: "derived_motion_ids": [], "all_origin_ids": [], "all_derived_motion_ids": [], - "amendment_ids": [13] + "amendment_ids": [13], }, "motion/13": { - "title": "test11 layer 2", + "title": "amendment", "meeting_id": 1, "derived_motion_ids": [], "all_origin_ids": [], "all_derived_motion_ids": [], "lead_motion_id": 12, "state_id": 30, + "amendment_paragraphs": {"0": "texts"}, }, } ) @@ -508,11 +509,269 @@ def test_allowed_to_forward_amendments_indirectly(self) -> None: "meeting_id": 2, "origin_id": 12, "text": "test", - "with_amendments": True + "with_amendments": True, }, ) self.assert_status_code(response, 200) - assert False #TODO: Check resulting data, results and write a few more tests + assert response.json["results"][0] == [ + { + "id": 14, + "non_forwarded_amendment_amount": 0, + "sequential_number": 1, + "amendment_result_data": [ + { + "amendment_result_data": [], + "id": 15, + "non_forwarded_amendment_amount": 0, + "sequential_number": 2, + } + ], + } + ] + self.assert_model_exists( + "motion/14", + { + "origin_id": 12, + "title": "test_foo", + "meeting_id": 2, + "text": "test", + "amendment_ids": [15], + "state_id": 34, + "additional_submitter": "committee_forwarder", + }, + ) + self.assert_model_exists( + "motion/15", + { + "lead_motion_id": 14, + "origin_id": 13, + "title": "amendment", + "meeting_id": 2, + "state_id": 34, + "amendment_paragraphs": {"0": "texts"}, + "additional_submitter": "committee_forwarder", + }, + ) + + def test_allowed_to_forward_amendments_indirectly_complex(self) -> None: + self.set_models(self.test_model) + user1 = self.create_user("first_submitter", [111]) + user2 = self.create_user("second_submitter", [111]) + self.set_models( + { + f"user/{user1}": {"first_name": "A", "last_name": "man"}, + f"user/{user2}": { + "title": "A", + "first_name": "hairy", + "last_name": "woman", + }, + "meeting/1": { + "name": "name_XDAddEAW", + "committee_id": 53, + "is_active_in_organization_id": 1, + "motion_ids": [12, 13, 14, 15], + }, + "user/1": {"meeting_ids": [1, 2]}, + "motion/12": { + "number": "MAIN", + "title": "title_FcnPUXJB layer 1", + "meeting_id": 1, + "derived_motion_ids": [], + "all_origin_ids": [], + "all_derived_motion_ids": [], + "amendment_ids": [13, 14, 15], + "submitter_ids": [1, 2], + }, + "motion_submitter/1": { + "motion_id": 12, + "meeting_user_id": 3, + "weight": 1, + }, + "motion_submitter/2": { + "motion_id": 12, + "meeting_user_id": 4, + "weight": 2, + }, + "motion/13": { + "number": "AMNDMNT1", + "title": "amendment1", + "meeting_id": 1, + "derived_motion_ids": [], + "all_origin_ids": [], + "all_derived_motion_ids": [], + "lead_motion_id": 12, + "state_id": 30, + "amendment_paragraphs": {"0": "texts"}, + "submitter_ids": [3], + }, + "motion_submitter/3": { + "motion_id": 13, + "meeting_user_id": 3, + "weight": 1, + }, + "motion/14": { + "number": "AMNDMNT2", + "title": "amendment2", + "meeting_id": 1, + "derived_motion_ids": [], + "all_origin_ids": [], + "all_derived_motion_ids": [], + "lead_motion_id": 12, + "state_id": 31, + "amendment_paragraphs": {"0": "NO!!!"}, + }, + "motion/15": { + "number": "AMNDMNT3", + "title": "amendment3", + "meeting_id": 1, + "derived_motion_ids": [], + "all_origin_ids": [], + "all_derived_motion_ids": [], + "lead_motion_id": 12, + "state_id": 30, + "amendment_paragraphs": {"0": "tests"}, + "amendment_ids": [16, 17], + }, + "motion/16": { + "number": "AMNDMNT4", + "title": "amendment4", + "meeting_id": 1, + "derived_motion_ids": [], + "all_origin_ids": [], + "all_derived_motion_ids": [], + "lead_motion_id": 15, + "state_id": 30, + "amendment_paragraphs": {"0": "testssss"}, + }, + "motion/17": { + "number": "AMNDMNT5", + "title": "amendment5", + "meeting_id": 1, + "derived_motion_ids": [], + "all_origin_ids": [], + "all_derived_motion_ids": [], + "lead_motion_id": 15, + "state_id": 31, + "amendment_paragraphs": {"0": "test"}, + }, + "meeting/2": { + "motions_default_workflow_id": 12, + "motions_default_amendment_workflow_id": 13, + }, + "motion_state/31": { + "name": "No forward state", + "meeting_id": 1, + }, + "motion_workflow/13": { + "name": "name_workflow2", + "first_state_id": 35, + "state_ids": [35], + "meeting_id": 2, + }, + "motion_state/35": { + "name": "name_state35", + "meeting_id": 2, + }, + } + ) + response = self.request( + "motion.create_forwarded", + { + "title": "test_foo", + "meeting_id": 2, + "origin_id": 12, + "text": "test", + "with_amendments": True, + "use_original_submitter": True, + "use_original_number": True, + }, + ) + self.assert_status_code(response, 200) + assert response.json["results"][0] == [ + { + "id": 18, + "non_forwarded_amendment_amount": 1, + "sequential_number": 1, + "amendment_result_data": [ + { + "id": 19, + "non_forwarded_amendment_amount": 0, + "sequential_number": 2, + "amendment_result_data": [], + }, + { + "id": 20, + "non_forwarded_amendment_amount": 1, + "sequential_number": 3, + "amendment_result_data": [ + { + "id": 21, + "non_forwarded_amendment_amount": 0, + "sequential_number": 4, + "amendment_result_data": [], + }, + ], + }, + ], + } + ] + self.assert_model_exists( + "motion/18", + { + "number": "MAIN", + "origin_id": 12, + "title": "test_foo", + "meeting_id": 2, + "text": "test", + "amendment_ids": [19, 20], + "additional_submitter": "A man, A hairy woman", + "sequential_number": 1, + "state_id": 34, + }, + ) + self.assert_model_exists( + "motion/19", + { + "number": "AMNDMNT1", + "lead_motion_id": 18, + "origin_id": 13, + "title": "amendment1", + "meeting_id": 2, + "amendment_paragraphs": {"0": "texts"}, + "additional_submitter": "A man", + "sequential_number": 2, + "state_id": 35, + }, + ) + self.assert_model_exists( + "motion/20", + { + "number": "AMNDMNT3", + "lead_motion_id": 18, + "origin_id": 15, + "title": "amendment3", + "meeting_id": 2, + "state_id": 35, + "amendment_paragraphs": {"0": "tests"}, + "additional_submitter": None, + "amendment_ids": [21], + "sequential_number": 3, + }, + ) + self.assert_model_exists( + "motion/21", + { + "number": "AMNDMNT4", + "lead_motion_id": 20, + "origin_id": 16, + "title": "amendment4", + "meeting_id": 2, + "state_id": 35, + "amendment_paragraphs": {"0": "testssss"}, + "additional_submitter": None, + "sequential_number": 4, + }, + ) def test_forward_to_2_meetings_1_transaction(self) -> None: """Forwarding of 1 motion to 2 meetings in 1 transaction""" @@ -725,7 +984,9 @@ def test_forward_multiple_to_meeting_with_set_number(self) -> None: self.assert_status_code(response, 200) created = [date["id"] for date in response.json["results"][0]] for i in range(2): - self.assert_model_exists(f"motion/{created[i]}", {"number": f"{i+1}"}) + self.assert_model_exists( + f"motion/{created[i]}", {"number": f"{i+1}", "sequential_number": 1 + i} + ) def test_forward_multiple_to_meeting_with_set_number_and_use_original_number( self, From 9fb1c81b5697310793f924d7526da7e8b8ab7106 Mon Sep 17 00:00:00 2001 From: Luisa Date: Wed, 3 Jul 2024 15:14:38 +0200 Subject: [PATCH 10/14] Commentary --- docs/actions/motion.create_forwarded.md | 15 ++++++++++++- .../motion.create_forwarded_amendment.md | 22 +++++++++++++++++++ .../motion/create_forwarded_amendment.py | 4 ++-- 3 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 docs/actions/motion.create_forwarded_amendment.md diff --git a/docs/actions/motion.create_forwarded.md b/docs/actions/motion.create_forwarded.md index 44ca2527b..5dba5949d 100644 --- a/docs/actions/motion.create_forwarded.md +++ b/docs/actions/motion.create_forwarded.md @@ -30,7 +30,8 @@ The original motion must be updated as well (this is done by the automatic relat The optional flags `use_original_submitter` and `use_original_number` will cause the original submitters and original numbers to be used in the new motion respectively. In case of the submitters, the action will generate the full name of the submitters and write the entire list of them and the value of the origin motions `additional_submitter` comma separated into the new motions `additional_submitter` field. If `use_original_submitter` is false the name of the origin motions committee will be written into the `additional_submitter` field instead -If `with_amendments` is set to True, all amendments of the motion, that have a state that can forward, will also be forwarded to the target meeting and connected to the newly forwarded lead motion +If `with_amendments` is set to True, all amendments of the motion, that have a state that can forward, will also be forwarded to the target meeting and connected to the newly forwarded lead motion. +The three boolean flags for extra rules will be applied to the amendments as well. If the forwarded amendments have amendments themselves, those will also be treated the same way @@ -43,6 +44,18 @@ If the forwarded amendments have amendments themselves, those will also be treat * The origin state must allow forwarding (`allow_motion_forwarding` must be set to True). +## Result + +The result object for each instance has the format +``` +{ + id: Id, + sequential_number: int, + non_forwarded_amendment_amount: int, // Number of amendments that couldn't be returned because of forwarding being not allowed in the state + amendment_result_data: [...], // List of result data objects in the same format, for all newly created amendments for the newly created motion +} +``` + ## Permissions The request user needs `motion.can_forward` in the source meeting. `motion.can_manage` is not explicitly needed for the request user, because it is included. There are no rights needed in the receiving meeting. diff --git a/docs/actions/motion.create_forwarded_amendment.md b/docs/actions/motion.create_forwarded_amendment.md new file mode 100644 index 000000000..53338ec57 --- /dev/null +++ b/docs/actions/motion.create_forwarded_amendment.md @@ -0,0 +1,22 @@ +## Payload +``` +{ +// Required + meeting_id: Id; + title: string; + lead_motion_id: Id; + origin_id: Id; + +// Optional + text: HTML; + reason: HTML; + amendment_paragraphs: JSON + use_original_submitter: boolean; + use_original_number: boolean; +} +``` + +## Internal action +Forwards an amendment in a manner that is to what is done with normal motions in [motion.create_forwarded](motion.create_forwarded.md) + +The only change is that the `with_amendments` flag is not in the payload, because it is assumed to be true. \ No newline at end of file diff --git a/openslides_backend/action/actions/motion/create_forwarded_amendment.py b/openslides_backend/action/actions/motion/create_forwarded_amendment.py index 3f29f8133..82b84d30a 100644 --- a/openslides_backend/action/actions/motion/create_forwarded_amendment.py +++ b/openslides_backend/action/actions/motion/create_forwarded_amendment.py @@ -15,7 +15,8 @@ ) class MotionCreateForwardedAmendment(BaseMotionCreateForwarded): """ - Create action for forwarded motions. + Internal create action for forwarded motion amendments. + Should only be called by motion.create_forwarded """ schema = DefaultSchema(Motion()).get_create_schema( @@ -24,7 +25,6 @@ class MotionCreateForwardedAmendment(BaseMotionCreateForwarded): additional_optional_fields={ "use_original_submitter": {"type": "boolean"}, "use_original_number": {"type": "boolean"}, - "with_amendments": {"type": "boolean"}, }, ) From 6ab8a0608222ac4341be35bfd02899b24a82dc52 Mon Sep 17 00:00:00 2001 From: Luisa Date: Wed, 10 Jul 2024 16:29:12 +0200 Subject: [PATCH 11/14] Make changes --- .../action/actions/motion/create_forwarded.py | 7 ++++- .../action/actions/user/create.py | 1 - .../user/create_update_permissions_mixin.py | 3 -- tests/system/action/meeting/test_import.py | 29 ------------------- tests/system/action/user/test_create.py | 12 -------- 5 files changed, 6 insertions(+), 46 deletions(-) diff --git a/openslides_backend/action/actions/motion/create_forwarded.py b/openslides_backend/action/actions/motion/create_forwarded.py index bfa6e4c44..23339127f 100644 --- a/openslides_backend/action/actions/motion/create_forwarded.py +++ b/openslides_backend/action/actions/motion/create_forwarded.py @@ -43,5 +43,10 @@ def check_permissions(self, instance: dict[str, Any]) -> None: def create_amendments(self, amendment_data: ActionData) -> ActionResults | None: return self.execute_other_action(MotionCreateForwardedAmendment, amendment_data) + def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: + self.with_amendments = instance.pop("with_amendments", False) + super().update_instance(instance) + return instance + def should_forward_amendments(self, instance: dict[str, Any]) -> bool: - return instance.pop("with_amendments", False) + return self.with_amendments diff --git a/openslides_backend/action/actions/user/create.py b/openslides_backend/action/actions/user/create.py index 5c682f7fe..ffa29be4c 100644 --- a/openslides_backend/action/actions/user/create.py +++ b/openslides_backend/action/actions/user/create.py @@ -51,7 +51,6 @@ class UserCreate( "is_present_in_meeting_ids", "committee_management_ids", "is_demo_user", - # "forwarding_committee_ids", "saml_id", "member_number", ], diff --git a/openslides_backend/action/actions/user/create_update_permissions_mixin.py b/openslides_backend/action/actions/user/create_update_permissions_mixin.py index 110d9fddb..4db8f3692 100644 --- a/openslides_backend/action/actions/user/create_update_permissions_mixin.py +++ b/openslides_backend/action/actions/user/create_update_permissions_mixin.py @@ -213,9 +213,6 @@ def check_permissions(self, instance: dict[str, Any]) -> None: """ self.assert_not_anonymous() - # if "forwarding_committee_ids" in instance: - # raise PermissionDenied("forwarding_committee_ids is not allowed.") - if not hasattr(self, "permstore"): self.permstore = PermissionVarStore( self.datastore, self.user_id, self.permission diff --git a/tests/system/action/meeting/test_import.py b/tests/system/action/meeting/test_import.py index 45cdfdae7..25e88a7bc 100644 --- a/tests/system/action/meeting/test_import.py +++ b/tests/system/action/meeting/test_import.py @@ -823,35 +823,6 @@ def test_check_usernames_new_and_twice(self) -> None: ) self.assert_model_not_exists("user/3") - # def test_with_forwarding_committees(self) -> None: - # request_data = self.create_request_data( - # { - # "user": { - # "2": self.get_user_data( - # 2, - # { - # "username": "user2", - # "last_name": "new user", - # "email": "tesT@email.de", - # "forwarding_committee_ids": [4], - # }, - # ) - # } - # } - # ) - - # response = self.request("meeting.import", request_data) - # self.assert_status_code(response, 200) - # user2 = self.assert_model_exists( - # "user/3", - # { - # "username": "user2", - # "last_name": "new user", - # "email": "tesT@email.de", - # }, - # ) - # assert not user2.get("forwarding_committee_ids") - def test_check_negative_default_vote_weight(self) -> None: request_data = self.create_request_data({}) request_data["meeting"]["user"]["1"] = self.get_user_data( diff --git a/tests/system/action/user/test_create.py b/tests/system/action/user/test_create.py index a32881564..6bf93040f 100644 --- a/tests/system/action/user/test_create.py +++ b/tests/system/action/user/test_create.py @@ -1217,18 +1217,6 @@ def test_create_default_vote_weight_none(self) -> None: user = self.get_model("user/2") assert "default_vote_weight" not in user - # def test_create_forwarding_committee_ids_not_allowed(self) -> None: - # self.set_models({"meeting/1": {"is_active_in_organization_id": 1}}) - # response = self.request( - # "user.create", - # { - # "username": "test_Xcdfgee", - # "forwarding_committee_ids": [], - # }, - # ) - # self.assert_status_code(response, 403) - # assert "forwarding_committee_ids is not allowed." in response.json["message"] - def test_create_negative_vote_weight(self) -> None: self.set_models( { From 78cef3fd7ef97d6b4f06a9edbcbb71b4a66934f5 Mon Sep 17 00:00:00 2001 From: Luisa Date: Wed, 17 Jul 2024 15:19:12 +0200 Subject: [PATCH 12/14] Remove datastore call result locking --- .../actions/motion/base_create_forwarded.py | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/openslides_backend/action/actions/motion/base_create_forwarded.py b/openslides_backend/action/actions/motion/base_create_forwarded.py index 7c537bc2b..c6b31f105 100644 --- a/openslides_backend/action/actions/motion/base_create_forwarded.py +++ b/openslides_backend/action/actions/motion/base_create_forwarded.py @@ -38,6 +38,7 @@ def prefetch(self, action_data: ActionData) -> None: "is_active_in_organization_id", "name", "motions_default_workflow_id", + "motions_default_amendment_workflow_id", "committee_id", "default_group_id", "motion_submitter_ids", @@ -66,9 +67,11 @@ def prefetch(self, action_data: ActionData) -> None: "all_origin_ids", "derived_motion_ids", "all_derived_motion_ids", + "amendment_ids", ], ), - ] + ], + lock_result=False, ) def get_user_verbose_names(self, meeting_user_ids: list[int]) -> str | None: @@ -77,7 +80,8 @@ def get_user_verbose_names(self, meeting_user_ids: list[int]) -> str | None: GetManyRequest( "meeting_user", meeting_user_ids, ["user_id", "structure_level_ids"] ) - ] + ], + lock_result=False, )["meeting_user"] user_ids = [ user_id @@ -101,7 +105,7 @@ def get_user_verbose_names(self, meeting_user_ids: list[int]) -> str | None: requests.append( GetManyRequest("structure_level", structure_level_ids, ["name"]) ) - user_data = self.datastore.get_many(requests) + user_data = self.datastore.get_many(requests, lock_result=False) users = user_data["user"] structure_levels = user_data["structure_level"] names = [] @@ -144,6 +148,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: meeting = self.datastore.get( fqid_from_collection_and_id("meeting", instance["meeting_id"]), ["motions_default_workflow_id", "motions_default_amendment_workflow_id"], + lock_result=False, ) self.set_state_from_workflow(instance, meeting) committee = self.check_for_origin_id(instance) @@ -156,6 +161,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: "motion_submitter", FilterOperator("motion_id", "=", instance["origin_id"]), ["meeting_user_id"], + lock_result=False, ).values() ) submitters = sorted(submitters, key=lambda x: x.get("weight", 10000)) @@ -171,6 +177,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: text_submitter = self.datastore.get( fqid_from_collection_and_id("motion", instance["origin_id"]), ["additional_submitter"], + lock_result=False, ).get("additional_submitter") if text_submitter: if instance.get("additional_submitter"): @@ -192,6 +199,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: amendment_ids = self.datastore.get( fqid_from_collection_and_id("motion", instance["origin_id"]), ["amendment_ids"], + lock_result=False, ).get("amendment_ids", []) if self.should_forward_amendments(instance): new_amendments = self.datastore.get_many( @@ -222,7 +230,8 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: GetManyRequest( "motion_state", list(state_ids), ["allow_motion_forwarding"] ) - ] + ], + lock_result=False, )["motion_state"] else: states = {} @@ -273,7 +282,9 @@ def create_action_result_element( def handle_number(self, instance: dict[str, Any]) -> dict[str, Any]: origin = self.datastore.get( - fqid_from_collection_and_id("motion", instance["origin_id"]), ["number"] + fqid_from_collection_and_id("motion", instance["origin_id"]), + ["number"], + lock_result=False, ) if instance.pop("use_original_number", None) and (num := origin.get("number")): number = self.get_clean_number(num, instance["meeting_id"]) @@ -295,14 +306,17 @@ def check_for_origin_id(self, instance: dict[str, Any]) -> dict[str, Any]: meeting = self.datastore.get( fqid_from_collection_and_id("meeting", instance["meeting_id"]), ["committee_id"], + lock_result=False, ) forwarded_from = self.datastore.get( fqid_from_collection_and_id("motion", instance["origin_id"]), ["meeting_id"], + lock_result=False, ) forwarded_from_meeting = self.datastore.get( fqid_from_collection_and_id("meeting", forwarded_from["meeting_id"]), ["committee_id"], + lock_result=False, ) # use the forwarding user id and id later in the handle forwarding user # code. @@ -311,6 +325,7 @@ def check_for_origin_id(self, instance: dict[str, Any]) -> dict[str, Any]: "committee", forwarded_from_meeting["committee_id"] ), ["id", "name", "forward_to_committee_ids"], + lock_result=False, ) if meeting["committee_id"] not in committee.get("forward_to_committee_ids", []): raise ActionException( @@ -340,6 +355,7 @@ def set_origin_ids(self, instance: dict[str, Any]) -> None: origin = self.datastore.get( fqid_from_collection_and_id("motion", instance["origin_id"]), ["all_origin_ids", "meeting_id"], + lock_result=False, ) instance["origin_meeting_id"] = origin["meeting_id"] instance["all_origin_ids"] = origin.get("all_origin_ids", []) @@ -349,10 +365,12 @@ def check_state_allow_forwarding(self, instance: dict[str, Any]) -> None: origin = self.datastore.get( fqid_from_collection_and_id(self.model.collection, instance["origin_id"]), ["state_id"], + lock_result=False, ) state = self.datastore.get( fqid_from_collection_and_id("motion_state", origin["state_id"]), ["allow_motion_forwarding"], + lock_result=False, ) if not state.get("allow_motion_forwarding"): raise ActionException("State doesn't allow to forward motion.") From f13c902ef6e040b089e71bae2f5b6ca3abd1f990 Mon Sep 17 00:00:00 2001 From: Luisa Date: Thu, 25 Jul 2024 16:30:15 +0200 Subject: [PATCH 13/14] Possibly fix tests --- .../action/motion/test_create_forwarded.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/system/action/motion/test_create_forwarded.py b/tests/system/action/motion/test_create_forwarded.py index 8e05fadc8..18784a532 100644 --- a/tests/system/action/motion/test_create_forwarded.py +++ b/tests/system/action/motion/test_create_forwarded.py @@ -1305,9 +1305,14 @@ def test_name_generation(self) -> None: ) self.assert_status_code(response, 200) created_id = response.json["results"][0][0]["id"] - self.assert_model_exists( - f"motion/{created_id}", - { - "additional_submitter": "Worship the administrator (he · is, very, good), He is User 2 (he · is, good), King (Kong · very), Good (good), He, she, it (ein 's' muss mit), Grandma not see, Sue B. Mid-Edit" - }, - ) + motion = self.assert_model_exists(f"motion/{created_id}") + for name in [ + "Worship the administrator (he · is, very, good)", + "He is User 2 (he · is, good)", + "King (Kong · very)", + "Good (good)", + "He, she, it (ein 's' muss mit)", + "Grandma not see", + "Sue B. Mid-Edit", + ]: + assert name in motion["additional_submitter"] From 8e52b2a16f8b2c34436d8c736500a8270fe7dada Mon Sep 17 00:00:00 2001 From: Luisa Date: Thu, 1 Aug 2024 16:30:38 +0200 Subject: [PATCH 14/14] Fix tests after merge --- .../action/actions/user/merge_together.py | 1 - tests/system/action/user/test_merge_together.py | 14 -------------- 2 files changed, 15 deletions(-) diff --git a/openslides_backend/action/actions/user/merge_together.py b/openslides_backend/action/actions/user/merge_together.py index 09bb08bcd..6a52a65c5 100644 --- a/openslides_backend/action/actions/user/merge_together.py +++ b/openslides_backend/action/actions/user/merge_together.py @@ -99,7 +99,6 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: ], "error": [ "is_demo_user", - "forwarding_committee_ids", ], "merge": [ "committee_ids", diff --git a/tests/system/action/user/test_merge_together.py b/tests/system/action/user/test_merge_together.py index f578d285d..519f487bd 100644 --- a/tests/system/action/user/test_merge_together.py +++ b/tests/system/action/user/test_merge_together.py @@ -463,20 +463,6 @@ def test_merge_is_demo_user_error(self) -> None: response.json["message"], ) - def test_merge_forwarding_committee_ids_error(self) -> None: - self.set_models( - { - "committee/3": {"forwarding_user_id": 3}, - "user/3": {"forwarding_committee_ids": [3]}, - } - ) - response = self.request("user.merge_together", {"id": 2, "user_ids": [3, 4]}) - self.assert_status_code(response, 400) - self.assertIn( - "Cannot merge user models that have forwarding_committee_ids set: Problem in user/3", - response.json["message"], - ) - def test_merge_saml_id_error(self) -> None: self.set_models({"user/3": {"saml_id": "SAML"}}) response = self.request("user.merge_together", {"id": 2, "user_ids": [3, 4]})