From a31fa7dda6cd9eafe1203cede16519eefccf66b1 Mon Sep 17 00:00:00 2001 From: Joshua Sangmeister Date: Thu, 28 May 2020 13:50:54 +0200 Subject: [PATCH] Adds reverse relations for motions and blocks --- openslides/motions/models.py | 5 +- openslides/motions/projector.py | 88 ++++++++++++----------- openslides/motions/serializers.py | 25 +++++++ openslides/motions/views.py | 13 ++++ tests/integration/motions/test_motions.py | 6 +- tests/integration/motions/test_viewset.py | 30 +++++++- tests/settings.py | 34 +++++++++ tests/unit/motions/test_projector.py | 3 + 8 files changed, 156 insertions(+), 48 deletions(-) diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 7d53b1845..66a3ec8ea 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -84,6 +84,7 @@ class MotionManager(BaseManager): "submitters", "supporters", "change_recommendations", + "amendments", ) ) @@ -192,7 +193,7 @@ class Motion(RESTModelMixin, AgendaItemWithListOfSpeakersMixin, models.Model): """ motion_block = models.ForeignKey( - "MotionBlock", on_delete=SET_NULL_AND_AUTOUPDATE, null=True, blank=True + "MotionBlock", on_delete=SET_NULL_AND_AUTOUPDATE, null=True, blank=True, ) """ ForeignKey to one block of motions. @@ -821,7 +822,7 @@ class MotionBlockManager(BaseManager): return ( super() .get_prefetched_queryset(*args, **kwargs) - .prefetch_related("agenda_items", "lists_of_speakers") + .prefetch_related("agenda_items", "lists_of_speakers", "motion_set") ) diff --git a/openslides/motions/projector.py b/openslides/motions/projector.py index c7a39040c..c9801c3a6 100644 --- a/openslides/motions/projector.py +++ b/openslides/motions/projector.py @@ -69,25 +69,24 @@ async def get_amendment_merge_into_motion_final(all_data_provider, amendment): async def get_amendments_for_motion(motion, all_data_provider): amendment_data = [] - all_motions = await all_data_provider.get_collection("motions/motion") - for amendment_id, amendment in all_motions.items(): - if amendment["parent_id"] == motion["id"]: - merge_amendment_into_final = await get_amendment_merge_into_motion_final( - all_data_provider, amendment - ) - merge_amendment_into_diff = await get_amendment_merge_into_motion_diff( - all_data_provider, amendment - ) - amendment_data.append( - { - "id": amendment["id"], - "identifier": amendment["identifier"], - "title": amendment["title"], - "amendment_paragraphs": amendment["amendment_paragraphs"], - "merge_amendment_into_diff": merge_amendment_into_diff, - "merge_amendment_into_final": merge_amendment_into_final, - } - ) + for amendment_id in motion["amendments_id"]: + amendment = await all_data_provider.get("motions/motion", amendment_id) + merge_amendment_into_final = await get_amendment_merge_into_motion_final( + all_data_provider, amendment + ) + merge_amendment_into_diff = await get_amendment_merge_into_motion_diff( + all_data_provider, amendment + ) + amendment_data.append( + { + "id": amendment["id"], + "identifier": amendment["identifier"], + "title": amendment["title"], + "amendment_paragraphs": amendment["amendment_paragraphs"], + "merge_amendment_into_diff": merge_amendment_into_diff, + "merge_amendment_into_final": merge_amendment_into_final, + } + ) return amendment_data @@ -334,32 +333,37 @@ async def motion_block_slide( # All title information for referenced motions in the recommendation referenced_motions: Dict[int, Dict[str, str]] = {} - # Search motions. - all_motions = await all_data_provider.get_collection("motions/motion") - for motion in all_motions.values(): - if motion["motion_block_id"] == motion_block["id"]: - motion_object = { - "title": motion["title"], - "identifier": motion["identifier"], + # iterate motions. + for motion_id in motion_block["motions_id"]: + motion = await all_data_provider.get("motions/motion", motion_id) + # primarily to please mypy, should theoretically not happen + if motion is None: + raise RuntimeError( + f"motion {motion_id} of block {element.get('id')} could not be found" + ) + + motion_object = { + "title": motion["title"], + "identifier": motion["identifier"], + } + + recommendation_id = motion["recommendation_id"] + if recommendation_id is not None: + recommendation = await get_state( + all_data_provider, motion, "recommendation_id" + ) + motion_object["recommendation"] = { + "name": recommendation["recommendation_label"], + "css_class": recommendation["css_class"], } - - recommendation_id = motion["recommendation_id"] - if recommendation_id is not None: - recommendation = await get_state( - all_data_provider, motion, "recommendation_id" + if recommendation["show_recommendation_extension_field"]: + recommendation_extension = motion["recommendation_extension"] + await extend_reference_motion_dict( + all_data_provider, recommendation_extension, referenced_motions ) - motion_object["recommendation"] = { - "name": recommendation["recommendation_label"], - "css_class": recommendation["css_class"], - } - if recommendation["show_recommendation_extension_field"]: - recommendation_extension = motion["recommendation_extension"] - await extend_reference_motion_dict( - all_data_provider, recommendation_extension, referenced_motions - ) - motion_object["recommendation_extension"] = recommendation_extension + motion_object["recommendation_extension"] = recommendation_extension - motions.append(motion_object) + motions.append(motion_object) return { "title": motion_block["title"], diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index 7b257c726..631bf47aa 100644 --- a/openslides/motions/serializers.py +++ b/openslides/motions/serializers.py @@ -83,6 +83,7 @@ class MotionBlockSerializer(ModelSerializer): write_only=True, required=False, min_value=1, max_value=3, allow_null=True ) agenda_parent_id = IntegerField(write_only=True, required=False, min_value=1) + motions_id = SerializerMethodField() class Meta: model = MotionBlock @@ -95,8 +96,12 @@ class MotionBlockSerializer(ModelSerializer): "agenda_type", "agenda_parent_id", "internal", + "motions_id", ) + def get_motions_id(self, block): + return [motion.id for motion in block.motion_set.all()] + def create(self, validated_data): """ Customized create method. Set information about related agenda item @@ -371,6 +376,7 @@ class MotionSerializer(ModelSerializer): agenda_parent_id = IntegerField(write_only=True, required=False, min_value=1) submitters = SubmitterSerializer(many=True, read_only=True) change_recommendations = IdPrimaryKeyRelatedField(many=True, read_only=True) + amendments_id = SerializerMethodField() class Meta: model = Motion @@ -409,14 +415,19 @@ class MotionSerializer(ModelSerializer): "created", "last_modified", "change_recommendations", + "amendments_id", ) read_only_fields = ( "state", "recommendation", "weight", "category_weight", + "amendments_id", ) # Some other fields are also read_only. See definitions above. + def get_amendments_id(self, motion): + return [amendment.id for amendment in motion.amendments.all()] + def validate(self, data): if "text" in data: data["text"] = validate_html_strict(data["text"]) @@ -488,6 +499,12 @@ class MotionSerializer(ModelSerializer): motion.supporters.add(*validated_data.get("supporters", [])) motion.attachments.add(*validated_data.get("attachments", [])) motion.tags.add(*validated_data.get("tags", [])) + + if motion.parent: + inform_changed_data(motion.parent) + if motion.motion_block: + inform_changed_data(motion.motion_block) + return motion @transaction.atomic @@ -508,6 +525,8 @@ class MotionSerializer(ModelSerializer): if validated_data.get("category") is not None else None ) + old_block = motion.motion_block + new_block = validated_data.get("motion_block") result = super().update(motion, validated_data) @@ -523,6 +542,12 @@ class MotionSerializer(ModelSerializer): inform_changed_data(motion) + if new_block != old_block: + if new_block: + inform_changed_data(new_block) + if old_block: + inform_changed_data(old_block) + return result def get_state_restriction(self, motion): diff --git a/openslides/motions/views.py b/openslides/motions/views.py index a7a699242..55f1f7afa 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -128,6 +128,12 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): user_id=request.user.pk, ) + # inform parents/blocks of deletion + if motion.parent: + inform_changed_data(motion.parent) + if motion.motion_block: + inform_changed_data(motion.motion_block) + return result def create(self, request, *args, **kwargs): @@ -675,6 +681,13 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): } ) + # inform old motion block + if motion.motion_block: + inform_changed_data(motion.motion_block) + # inform new motion block + if motion_block: + inform_changed_data(motion_block) + # Set motion bock motion.motion_block = motion_block diff --git a/tests/integration/motions/test_motions.py b/tests/integration/motions/test_motions.py index 24b48c128..bd1de5d1f 100644 --- a/tests/integration/motions/test_motions.py +++ b/tests/integration/motions/test_motions.py @@ -36,6 +36,7 @@ def test_motion_db_queries(): * 1 request for all motion comments * 1 request for all motion comment sections required for the comments * 1 request for all users required for the read_groups of the sections + * 1 request to get all amendments of all motions * 1 request to get the agenda item, * 1 request to get the list of speakers, * 1 request to get the attachments, @@ -90,7 +91,7 @@ def test_motion_db_queries(): ) poll.create_options() - assert count_queries(Motion.get_elements)() == 12 + assert count_queries(Motion.get_elements)() == 13 class CreateMotion(TestCase): @@ -109,7 +110,7 @@ class CreateMotion(TestCase): The created motion should have an identifier and the admin user should be the submitter. """ - with self.assertNumQueries(52): + with self.assertNumQueries(54): response = self.client.post( reverse("motion-list"), { @@ -141,6 +142,7 @@ class CreateMotion(TestCase): "title": "test_title_OoCoo3MeiT9li5Iengu9", "text": "test_text_thuoz0iecheiheereiCi", "amendment_paragraphs": None, + "amendments_id": [], "modified_final_version": "", "reason": "", "parent_id": None, diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index b311ba86c..52ffeb3df 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -53,13 +53,32 @@ def test_statute_paragraph_db_queries(): def test_workflow_db_queries(): """ Tests that only the following db queries are done: - * 1 requests to get the list of all workflows and + * 1 request to get the list of all workflows and * 1 request to get all states. """ assert count_queries(Workflow.get_elements)() == 2 +@pytest.mark.django_db(transaction=False) +def test_motion_block_db_queries(): + """ + Tests that only the following db queries are done: + * 1 request to get all motion blocks + * 1 request to get all agenda items + * 1 request to get all lists of speakers + * 1 request to get all motions + """ + for i in range(5): + motion_block = MotionBlock.objects.create(title=f"block{i}") + for j in range(3): + Motion.objects.create( + title=f"motion{i}_{j}", text="text", motion_block=motion_block + ) + + assert count_queries(MotionBlock.get_elements)() == 4 + + class TestStatuteParagraphs(TestCase): """ Tests all CRUD operations of statute paragraphs. @@ -1100,7 +1119,14 @@ class TestMotionBlock(TestCase): self.assertEqual( sorted(response.data.keys()), sorted( - ("agenda_item_id", "id", "internal", "list_of_speakers_id", "title") + ( + "agenda_item_id", + "id", + "internal", + "list_of_speakers_id", + "title", + "motions_id", + ) ), ) diff --git a/tests/settings.py b/tests/settings.py index 1366534df..37f1e2ac6 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -77,3 +77,37 @@ RESTRICTED_DATA_CACHE = False REST_FRAMEWORK = {"TEST_REQUEST_DEFAULT_FORMAT": "json"} ENABLE_ELECTRONIC_VOTING = True + + +# https://stackoverflow.com/questions/24876343/django-traceback-on-queries +if os.environ.get("DEBUG_SQL_TRACEBACK"): + import traceback + import django.db.backends.utils as bakutils + + cursor_debug_wrapper_orig = bakutils.CursorDebugWrapper + + def print_stack_in_project(sql): + stack = traceback.extract_stack() + for path, lineno, func, line in stack: + if "lib/python" in path or "settings.py" in path: + continue + print(f'File "{path}", line {lineno}, in {func}') + print(f" {line}") + print(sql) + print("\n") + + class CursorDebugWrapperLoud(cursor_debug_wrapper_orig): # type: ignore + def execute(self, sql, params=None): + try: + return super().execute(sql, params) + finally: + sql = self.db.ops.last_executed_query(self.cursor, sql, params) + print_stack_in_project(sql) + + def executemany(self, sql, param_list): + try: + return super().executemany(sql, param_list) + finally: + print_stack_in_project(sql) + + bakutils.CursorDebugWrapper = CursorDebugWrapperLoud diff --git a/tests/unit/motions/test_projector.py b/tests/unit/motions/test_projector.py index 257d60150..9f191b06a 100644 --- a/tests/unit/motions/test_projector.py +++ b/tests/unit/motions/test_projector.py @@ -74,6 +74,7 @@ def all_data_provider(): "created": "2019-01-19T18:37:34.741336+01:00", "last_modified": "2019-01-19T18:37:34.741368+01:00", "change_recommendations_id": [1, 2], + "amendments_id": [2], }, 2: { "id": 2, @@ -107,6 +108,7 @@ def all_data_provider(): "created": "2019-01-19T18:37:34.741336+01:00", "last_modified": "2019-01-19T18:37:34.741368+01:00", "change_recommendations": [], + "amendments_id": [], }, 3: { "id": 3, @@ -140,6 +142,7 @@ def all_data_provider(): "created": "2019-01-19T18:37:34.741336+01:00", "last_modified": "2019-01-19T18:37:34.741368+01:00", "change_recommendations": [], + "amendments_id": [], }, } data["motions/workflow"] = {