From 246067f1edd1a3c807b7ee858b03abaef57321c9 Mon Sep 17 00:00:00 2001 From: Ludwig Reiter Date: Wed, 26 Jul 2023 09:47:41 +0200 Subject: [PATCH 1/3] Allow to set agenda item tags with agenda_tag_ids Remove topic/tag_ids because it is not used in the client. --- global/meta/models.yml | 6 --- .../actions/agenda_item/agenda_creation.py | 6 ++- .../action/actions/agenda_item/create.py | 1 + .../action/actions/topic/create.py | 2 +- .../action/actions/topic/update.py | 2 +- openslides_backend/models/models.py | 12 +---- .../system/action/agenda_item/test_create.py | 6 +++ tests/system/action/tag/test_delete.py | 12 +++-- tests/system/action/topic/test_create.py | 53 ++++--------------- tests/system/action/topic/test_update.py | 44 --------------- 10 files changed, 34 insertions(+), 110 deletions(-) diff --git a/global/meta/models.yml b/global/meta/models.yml index b6240fcbf..574db7fa2 100644 --- a/global/meta/models.yml +++ b/global/meta/models.yml @@ -1894,7 +1894,6 @@ tag: - agenda_item - assignment - motion - - topic field: tag_ids equal_fields: meeting_id restriction_mode: A @@ -2150,11 +2149,6 @@ topic: on_delete: CASCADE equal_fields: meeting_id restriction_mode: A - tag_ids: - type: relation-list - to: tag/tagged_ids - equal_fields: meeting_id - restriction_mode: A poll_ids: type: relation-list to: poll/content_object_id diff --git a/openslides_backend/action/actions/agenda_item/agenda_creation.py b/openslides_backend/action/actions/agenda_item/agenda_creation.py index 7370487c1..cae52ca7b 100644 --- a/openslides_backend/action/actions/agenda_item/agenda_creation.py +++ b/openslides_backend/action/actions/agenda_item/agenda_creation.py @@ -2,7 +2,7 @@ from ....models.models import AgendaItem from ....shared.patterns import fqid_from_collection_and_id -from ....shared.schema import optional_id_schema +from ....shared.schema import id_list_schema, optional_id_schema from ...action import Action AGENDA_PREFIX = "agenda_" @@ -38,6 +38,10 @@ "description": "The weight of the agenda item.", "type": "integer", }, + f"{AGENDA_PREFIX}tag_ids": { + "description": "The ids of tags to be set.", + **id_list_schema, + }, } diff --git a/openslides_backend/action/actions/agenda_item/create.py b/openslides_backend/action/actions/agenda_item/create.py index f14e5be35..d96fb1ed3 100644 --- a/openslides_backend/action/actions/agenda_item/create.py +++ b/openslides_backend/action/actions/agenda_item/create.py @@ -27,6 +27,7 @@ class AgendaItemCreate(CreateActionWithInferredMeeting): "parent_id", "duration", "weight", + "tag_ids", ], ) permission = Permissions.AgendaItem.CAN_MANAGE diff --git a/openslides_backend/action/actions/topic/create.py b/openslides_backend/action/actions/topic/create.py index 17985286c..90d81f4b5 100644 --- a/openslides_backend/action/actions/topic/create.py +++ b/openslides_backend/action/actions/topic/create.py @@ -32,7 +32,7 @@ class TopicCreate( model = Topic() schema = DefaultSchema(Topic()).get_create_schema( required_properties=["meeting_id", "title"], - optional_properties=["text", "attachment_ids", "tag_ids"], + optional_properties=["text", "attachment_ids"], additional_optional_fields=agenda_creation_properties, ) dependencies = [AgendaItemCreate, ListOfSpeakersCreate] diff --git a/openslides_backend/action/actions/topic/update.py b/openslides_backend/action/actions/topic/update.py index 2f37dfe44..867bfe450 100644 --- a/openslides_backend/action/actions/topic/update.py +++ b/openslides_backend/action/actions/topic/update.py @@ -13,6 +13,6 @@ class TopicUpdate(UpdateAction): model = Topic() schema = DefaultSchema(Topic()).get_update_schema( - optional_properties=["title", "text", "attachment_ids", "tag_ids"] + optional_properties=["title", "text", "attachment_ids"] ) permission = Permissions.AgendaItem.CAN_MANAGE diff --git a/openslides_backend/models/models.py b/openslides_backend/models/models.py index 9920cacae..e08e5a20d 100644 --- a/openslides_backend/models/models.py +++ b/openslides_backend/models/models.py @@ -3,7 +3,7 @@ from openslides_backend.models import fields from openslides_backend.models.base import Model -MODELS_YML_CHECKSUM = "dea25af9a336309f96f7ccb9eca61cb9" +MODELS_YML_CHECKSUM = "4ee1bccdf2ec2a8d19562e1e4d4b18b2" class Organization(Model): @@ -905,12 +905,7 @@ class Tag(Model): id = fields.IntegerField() name = fields.CharField(required=True) tagged_ids = fields.GenericRelationListField( - to={ - "agenda_item": "tag_ids", - "assignment": "tag_ids", - "motion": "tag_ids", - "topic": "tag_ids", - }, + to={"agenda_item": "tag_ids", "assignment": "tag_ids", "motion": "tag_ids"}, equal_fields="meeting_id", ) meeting_id = fields.RelationField(to={"meeting": "tag_ids"}, required=True) @@ -1079,9 +1074,6 @@ class Topic(Model): required=True, equal_fields="meeting_id", ) - tag_ids = fields.RelationListField( - to={"tag": "tagged_ids"}, equal_fields="meeting_id" - ) poll_ids = fields.RelationListField( to={"poll": "content_object_id"}, on_delete=fields.OnDelete.CASCADE, diff --git a/tests/system/action/agenda_item/test_create.py b/tests/system/action/agenda_item/test_create.py index ee2ae5c00..51918bcd0 100644 --- a/tests/system/action/agenda_item/test_create.py +++ b/tests/system/action/agenda_item/test_create.py @@ -33,6 +33,7 @@ def test_create_more_fields(self) -> None: "meeting/1": {"is_active_in_organization_id": 1}, "topic/1": {"meeting_id": 1}, "agenda_item/42": {"comment": "test", "meeting_id": 1}, + "tag/561": {"meeting_id": 1}, } ) response = self.request( @@ -43,6 +44,7 @@ def test_create_more_fields(self) -> None: "type": AgendaItem.INTERNAL_ITEM, "parent_id": 42, "duration": 360, + "tag_ids": [561], }, ) self.assert_status_code(response, 200) @@ -54,6 +56,10 @@ def test_create_more_fields(self) -> None: self.assertEqual(agenda_item["weight"], 1) self.assertFalse(agenda_item.get("closed")) assert agenda_item.get("level") == 1 + assert agenda_item.get("tag_ids") == [561] + self.assert_model_exists( + "tag/561", {"meeting_id": 1, "tagged_ids": ["agenda_item/43"]} + ) def test_create_twice_without_parent(self) -> None: self.set_models( diff --git a/tests/system/action/tag/test_delete.py b/tests/system/action/tag/test_delete.py index f47e43f43..f03dc577e 100644 --- a/tests/system/action/tag/test_delete.py +++ b/tests/system/action/tag/test_delete.py @@ -31,20 +31,24 @@ def test_delete_correct_2(self) -> None: "tag/111": { "name": "name_srtgb123", "meeting_id": 1, - "tagged_ids": ["topic/222"], + "tagged_ids": ["agenda_item/222"], + }, + "agenda_item/222": { + "comment": "test_comment_ertgd590854398", + "tag_ids": [111], + "meeting_id": 1, }, - "topic/222": {"title": "test_title_ertgd590854398", "tag_ids": [111]}, } ) response = self.request("tag.delete", {"id": 111}) self.assert_status_code(response, 200) self.assert_model_not_exists("tag/112") self.assert_model_exists( - "topic/222", + "agenda_item/222", { "id": 222, "meta_deleted": False, - "title": "test_title_ertgd590854398", + "comment": "test_comment_ertgd590854398", "tag_ids": [], }, ) diff --git a/tests/system/action/topic/test_create.py b/tests/system/action/topic/test_create.py index 956fe4ba4..d0d6d455b 100644 --- a/tests/system/action/topic/test_create.py +++ b/tests/system/action/topic/test_create.py @@ -67,8 +67,11 @@ def test_create_multiple_requests(self) -> None: self.assert_model_not_exists("topic/4") def test_create_more_fields(self) -> None: - self.create_model( - "meeting/1", {"name": "test", "is_active_in_organization_id": 1} + self.set_models( + { + "meeting/1": {"name": "test", "is_active_in_organization_id": 1}, + "tag/37": {"meeting_id": 1}, + } ) response = self.request( "topic.create", @@ -77,6 +80,7 @@ def test_create_more_fields(self) -> None: "title": "test", "agenda_type": AgendaItem.INTERNAL_ITEM, "agenda_duration": 60, + "agenda_tag_ids": [37], }, ) self.assert_status_code(response, 200) @@ -91,6 +95,10 @@ def test_create_more_fields(self) -> None: self.assertEqual(agenda_item["type"], AgendaItem.INTERNAL_ITEM) self.assertEqual(agenda_item["duration"], 60) self.assertEqual(agenda_item["weight"], 1) + self.assertEqual(agenda_item["tag_ids"], [37]) + self.assert_model_exists( + "tag/37", {"meeting_id": 1, "tagged_ids": ["agenda_item/1"]} + ) def test_create_multiple_in_one_request(self) -> None: self.create_model("meeting/1", {"is_active_in_organization_id": 1}) @@ -159,47 +167,6 @@ def test_create_multiple_with_existing_sequential_number(self) -> None: topic = self.get_model("topic/3") self.assertEqual(topic.get("sequential_number"), 44) - def test_create_meeting_id_tag_ids_mismatch(self) -> None: - """Tag 8 is from meeting 8 and a topic for meeting 1 should be created. - This should lead to an error.""" - self.set_models( - { - "meeting/1": {"is_active_in_organization_id": 1}, - "meeting/8": { - "is_active_in_organization_id": 1, - "tag_ids": [8], - }, - "tag/8": {"name": "tag8", "meeting_id": 8}, - } - ) - response = self.request( - "topic.create", {"meeting_id": 1, "title": "A", "tag_ids": [8]} - ) - self.assert_status_code(response, 400) - assert ( - "The following models do not belong to meeting 1: ['tag/8']" - in response.json["message"] - ) - - def test_create_with_tag_ids(self) -> None: - """Tag 1 is from meeting 1 and a topic for meeting 1 should be created.""" - self.set_models( - { - "meeting/1": {"is_active_in_organization_id": 1, "tag_ids": [1]}, - "tag/1": {"name": "test tag", "meeting_id": 1}, - } - ) - response = self.request( - "topic.create", {"meeting_id": 1, "title": "A", "tag_ids": [1]} - ) - self.assert_status_code(response, 200) - self.assert_model_exists( - "topic/1", {"meeting_id": 1, "title": "A", "tag_ids": [1]} - ) - self.assert_model_exists( - "tag/1", {"meeting_id": 1, "name": "test tag", "tagged_ids": ["topic/1"]} - ) - def test_create_no_permission(self) -> None: self.base_permission_test( {}, "topic.create", {"meeting_id": 1, "title": "test"} diff --git a/tests/system/action/topic/test_update.py b/tests/system/action/topic/test_update.py index abbf86d2d..b7b2c2ad3 100644 --- a/tests/system/action/topic/test_update.py +++ b/tests/system/action/topic/test_update.py @@ -45,50 +45,6 @@ def test_update_text_with_iframe(self) -> None: }, ) - def test_update_tag_ids_add(self) -> None: - self.set_models( - { - "meeting/1": {"name": "test", "is_active_in_organization_id": 1}, - "topic/1": {"title": "test", "meeting_id": 1}, - "tag/1": {"name": "tag", "meeting_id": 1}, - } - ) - response = self.request("topic.update", {"id": 1, "tag_ids": [1]}) - self.assert_status_code(response, 200) - topic = self.get_model("topic/1") - self.assertEqual(topic.get("tag_ids"), [1]) - - def test_update_tag_ids_remove(self) -> None: - self.test_update_tag_ids_add() - response = self.request("topic.update", {"id": 1, "tag_ids": []}) - self.assert_status_code(response, 200) - topic = self.get_model("topic/1") - self.assertEqual(topic.get("tag_ids"), []) - - def test_update_multiple_with_tag(self) -> None: - self.set_models( - { - "meeting/1": {"name": "test", "is_active_in_organization_id": 1}, - "tag/1": { - "name": "tag", - "meeting_id": 1, - "tagged_ids": ["topic/1", "topic/2"], - }, - "topic/1": {"title": "test", "meeting_id": 1, "tag_ids": [1]}, - "topic/2": {"title": "test", "meeting_id": 1, "tag_ids": [1]}, - } - ) - response = self.request_multi( - "topic.update", [{"id": 1, "tag_ids": []}, {"id": 2, "tag_ids": []}] - ) - self.assert_status_code(response, 200) - topic = self.get_model("topic/1") - self.assertEqual(topic.get("tag_ids"), []) - topic = self.get_model("topic/2") - self.assertEqual(topic.get("tag_ids"), []) - tag = self.get_model("tag/1") - self.assertEqual(tag.get("tagged_ids"), []) - def test_update_no_permission(self) -> None: self.base_permission_test( self.permission_test_models, From 7a301c9e73a0208a38f90566047ec8bab5411555 Mon Sep 17 00:00:00 2001 From: Ludwig Reiter Date: Sat, 29 Jul 2023 16:07:25 +0200 Subject: [PATCH 2/3] Reactivate tests of topic create --- tests/system/action/topic/test_create.py | 65 ++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/system/action/topic/test_create.py b/tests/system/action/topic/test_create.py index d0d6d455b..3b91e09e9 100644 --- a/tests/system/action/topic/test_create.py +++ b/tests/system/action/topic/test_create.py @@ -167,6 +167,71 @@ def test_create_multiple_with_existing_sequential_number(self) -> None: topic = self.get_model("topic/3") self.assertEqual(topic.get("sequential_number"), 44) + def test_create_meeting_id_agenda_tag_ids_mismatch(self) -> None: + """Tag 8 is from meeting 8 and a topic for meeting 1 should be created. + This should lead to an error.""" + self.set_models( + { + "meeting/1": {"is_active_in_organization_id": 1}, + "meeting/8": { + "is_active_in_organization_id": 1, + "tag_ids": [8], + }, + "tag/8": {"name": "tag8", "meeting_id": 8}, + } + ) + response = self.request( + "topic.create", + { + "meeting_id": 1, + "title": "A", + "agenda_type": AgendaItem.INTERNAL_ITEM, + "agenda_duration": 60, + "agenda_tag_ids": [8], + }, + ) + self.assert_status_code(response, 400) + assert ( + "The following models do not belong to meeting 1: ['tag/8']" + in response.json["message"] + ) + + def test_create_with_agenda_tag_ids(self) -> None: + """Tag 1 is from meeting 1 and a topic for meeting 1 should be created.""" + self.set_models( + { + "meeting/1": {"is_active_in_organization_id": 1, "tag_ids": [1]}, + "tag/1": {"name": "test tag", "meeting_id": 1}, + } + ) + response = self.request( + "topic.create", + { + "meeting_id": 1, + "title": "A", + "agenda_type": AgendaItem.INTERNAL_ITEM, + "agenda_duration": 60, + "agenda_tag_ids": [1], + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "topic/1", {"meeting_id": 1, "title": "A", "agenda_item_id": 1} + ) + self.assert_model_exists( + "agenda_item/1", + { + "meeting_id": 1, + "type": AgendaItem.INTERNAL_ITEM, + "duration": 60, + "tag_ids": [1], + }, + ) + self.assert_model_exists( + "tag/1", + {"meeting_id": 1, "name": "test tag", "tagged_ids": ["agenda_item/1"]}, + ) + def test_create_no_permission(self) -> None: self.base_permission_test( {}, "topic.create", {"meeting_id": 1, "title": "test"} From 3a6c5a16bfa10f02d65b6a9bac2510e7be781a34 Mon Sep 17 00:00:00 2001 From: Ludwig Reiter Date: Mon, 31 Jul 2023 09:48:35 +0200 Subject: [PATCH 3/3] Move tag_ids test to agenda_item --- .../system/action/agenda_item/test_update.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/system/action/agenda_item/test_update.py b/tests/system/action/agenda_item/test_update.py index 0b59f28c3..7a90d13d8 100644 --- a/tests/system/action/agenda_item/test_update.py +++ b/tests/system/action/agenda_item/test_update.py @@ -84,6 +84,50 @@ def test_update_type_change_with_children(self) -> None: assert child_model.get("is_hidden") is True assert child_model.get("is_internal") is False + def test_update_tag_ids_add(self) -> None: + self.set_models( + { + "meeting/1": {"name": "test", "is_active_in_organization_id": 1}, + "agenda_item/1": {"comment": "test", "meeting_id": 1}, + "tag/1": {"name": "tag", "meeting_id": 1}, + } + ) + response = self.request("agenda_item.update", {"id": 1, "tag_ids": [1]}) + self.assert_status_code(response, 200) + agenda_item = self.get_model("agenda_item/1") + self.assertEqual(agenda_item.get("tag_ids"), [1]) + + def test_update_tag_ids_remove(self) -> None: + self.test_update_tag_ids_add() + response = self.request("agenda_item.update", {"id": 1, "tag_ids": []}) + self.assert_status_code(response, 200) + agenda_item = self.get_model("agenda_item/1") + self.assertEqual(agenda_item.get("tag_ids"), []) + + def test_update_multiple_with_tag(self) -> None: + self.set_models( + { + "meeting/1": {"name": "test", "is_active_in_organization_id": 1}, + "tag/1": { + "name": "tag", + "meeting_id": 1, + "tagged_ids": ["agenda_item/1", "agenda_item/2"], + }, + "agenda_item/1": {"comment": "test", "meeting_id": 1, "tag_ids": [1]}, + "agenda_item/2": {"comment": "test", "meeting_id": 1, "tag_ids": [1]}, + } + ) + response = self.request_multi( + "agenda_item.update", [{"id": 1, "tag_ids": []}, {"id": 2, "tag_ids": []}] + ) + self.assert_status_code(response, 200) + agenda_item = self.get_model("agenda_item/1") + self.assertEqual(agenda_item.get("tag_ids"), []) + agenda_item = self.get_model("agenda_item/2") + self.assertEqual(agenda_item.get("tag_ids"), []) + tag = self.get_model("tag/1") + self.assertEqual(tag.get("tagged_ids"), []) + def test_update_no_permissions(self) -> None: self.base_permission_test( {