From e16f70d48cffcef29773d26e00b60707c6840717 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:34:45 +0000 Subject: [PATCH 01/51] Bump dependency-injector from 4.42.0 to 4.43.0 in /requirements/partial (#2712) Bumps [dependency-injector](https://github.com/ets-labs/python-dependency-injector) from 4.42.0 to 4.43.0. - [Commits](https://github.com/ets-labs/python-dependency-injector/compare/4.42.0...4.43.0) --- updated-dependencies: - dependency-name: dependency-injector dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements/partial/requirements_production.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/partial/requirements_production.txt b/requirements/partial/requirements_production.txt index fb1fc89c85..c0b1241800 100644 --- a/requirements/partial/requirements_production.txt +++ b/requirements/partial/requirements_production.txt @@ -1,7 +1,7 @@ Babel==2.16.0 beautifulsoup4==4.12.3 bleach[css]==6.1.0 -dependency_injector==4.42.0 +dependency_injector==4.43.0 fastjsonschema==2.20.0 gunicorn==23.0.0 pypdf[crypto]==5.0.1 From 936a4cc84fc96bec92fd2ed54e9a3b66da02c4a4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:49:56 +0000 Subject: [PATCH 02/51] Bump types-beautifulsoup4 in /requirements/partial (#2693) Bumps [types-beautifulsoup4](https://github.com/python/typeshed) from 4.12.0.20240907 to 4.12.0.20241020. - [Commits](https://github.com/python/typeshed/commits) --- updated-dependencies: - dependency-name: types-beautifulsoup4 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements/partial/requirements_development.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/partial/requirements_development.txt b/requirements/partial/requirements_development.txt index 479338e0b7..47e94684e5 100644 --- a/requirements/partial/requirements_development.txt +++ b/requirements/partial/requirements_development.txt @@ -14,7 +14,7 @@ pyyaml==6.0.2 # typing types-babel==2.11.0.15 -types-beautifulsoup4==4.12.0.20240907 +types-beautifulsoup4==4.12.0.20241020 types-bleach==6.1.0.20240331 types-PyYAML==6.0.12.20240917 types-requests==2.32.0.20241016 From b1fe0838a2c2fa41ddd68a5659a43d00695204db Mon Sep 17 00:00:00 2001 From: luisa-beerboom <101706784+luisa-beerboom@users.noreply.github.com> Date: Mon, 11 Nov 2024 17:30:39 +0100 Subject: [PATCH 03/51] Bump opentelemetry dependencies update datastore and auth hashes (#2726) * Bump the opentelemetry-dependencies group across 1 directory with 5 updates Bumps the opentelemetry-dependencies group with 5 updates in the /requirements/partial directory: | Package | From | To | | --- | --- | --- | | [opentelemetry-api](https://github.com/open-telemetry/opentelemetry-python) | `1.27.0` | `1.28.1` | | [opentelemetry-sdk](https://github.com/open-telemetry/opentelemetry-python) | `1.27.0` | `1.28.1` | | [opentelemetry-exporter-otlp](https://github.com/open-telemetry/opentelemetry-python) | `1.27.0` | `1.28.1` | | [opentelemetry-instrumentation-flask](https://github.com/open-telemetry/opentelemetry-python-contrib) | `0.48b0` | `0.49b1` | | [opentelemetry-instrumentation-requests](https://github.com/open-telemetry/opentelemetry-python-contrib) | `0.48b0` | `0.49b1` | Updates `opentelemetry-api` from 1.27.0 to 1.28.1 - [Release notes](https://github.com/open-telemetry/opentelemetry-python/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-python/blob/v1.28.1/CHANGELOG.md) - [Commits](https://github.com/open-telemetry/opentelemetry-python/compare/v1.27.0...v1.28.1) Updates `opentelemetry-sdk` from 1.27.0 to 1.28.1 - [Release notes](https://github.com/open-telemetry/opentelemetry-python/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-python/blob/v1.28.1/CHANGELOG.md) - [Commits](https://github.com/open-telemetry/opentelemetry-python/compare/v1.27.0...v1.28.1) Updates `opentelemetry-exporter-otlp` from 1.27.0 to 1.28.1 - [Release notes](https://github.com/open-telemetry/opentelemetry-python/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-python/blob/v1.28.1/CHANGELOG.md) - [Commits](https://github.com/open-telemetry/opentelemetry-python/compare/v1.27.0...v1.28.1) Updates `opentelemetry-instrumentation-flask` from 0.48b0 to 0.49b1 - [Release notes](https://github.com/open-telemetry/opentelemetry-python-contrib/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/CHANGELOG.md) - [Commits](https://github.com/open-telemetry/opentelemetry-python-contrib/commits) Updates `opentelemetry-instrumentation-requests` from 0.48b0 to 0.49b1 - [Release notes](https://github.com/open-telemetry/opentelemetry-python-contrib/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/CHANGELOG.md) - [Commits](https://github.com/open-telemetry/opentelemetry-python-contrib/commits) --- updated-dependencies: - dependency-name: opentelemetry-api dependency-type: direct:production update-type: version-update:semver-minor dependency-group: opentelemetry-dependencies - dependency-name: opentelemetry-sdk dependency-type: direct:production update-type: version-update:semver-minor dependency-group: opentelemetry-dependencies - dependency-name: opentelemetry-exporter-otlp dependency-type: direct:production update-type: version-update:semver-minor dependency-group: opentelemetry-dependencies - dependency-name: opentelemetry-instrumentation-flask dependency-type: direct:production dependency-group: opentelemetry-dependencies - dependency-name: opentelemetry-instrumentation-requests dependency-type: direct:production dependency-group: opentelemetry-dependencies ... Signed-off-by: dependabot[bot] * Bump opentelemetry dependencies update datastore and auth hashes --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements/export_service_commits.sh | 4 ++-- requirements/partial/requirements_production.txt | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/requirements/export_service_commits.sh b/requirements/export_service_commits.sh index f647b4bc96..82a9e372bf 100755 --- a/requirements/export_service_commits.sh +++ b/requirements/export_service_commits.sh @@ -1,3 +1,3 @@ #!/bin/bash -export DATASTORE_COMMIT_HASH=ff32410426631356f67013ebdd607c099cd676a1 -export AUTH_COMMIT_HASH=31bf8d3e965f59ab4203223fff1d9a640f3a3444 +export DATASTORE_COMMIT_HASH=827476ebb2f796a2a8ac665d5dbb577e870fc4de +export AUTH_COMMIT_HASH=85f5d0e10f7ab455b45f8da73ea7c69ce953e569 diff --git a/requirements/partial/requirements_production.txt b/requirements/partial/requirements_production.txt index c0b1241800..09aac616bd 100644 --- a/requirements/partial/requirements_production.txt +++ b/requirements/partial/requirements_production.txt @@ -13,8 +13,8 @@ python-magic==0.4.27 pygments==2.18.0 # opentelemetry -opentelemetry-api==1.27.0 -opentelemetry-sdk==1.27.0 -opentelemetry-exporter-otlp==1.27.0 -opentelemetry-instrumentation-flask==0.48b0 -opentelemetry-instrumentation-requests==0.48b0 +opentelemetry-api==1.28.1 +opentelemetry-sdk==1.28.1 +opentelemetry-exporter-otlp==1.28.1 +opentelemetry-instrumentation-flask==0.49b1 +opentelemetry-instrumentation-requests==0.49b1 From 45bdd24b638223c3bef87986f6955fe3d3016434 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Nov 2024 09:32:27 +0100 Subject: [PATCH 04/51] Bump pypdf[crypto] from 5.0.1 to 5.1.0 in /requirements/partial (#2705) Bumps [pypdf[crypto]](https://github.com/py-pdf/pypdf) from 5.0.1 to 5.1.0. - [Release notes](https://github.com/py-pdf/pypdf/releases) - [Changelog](https://github.com/py-pdf/pypdf/blob/main/CHANGELOG.md) - [Commits](https://github.com/py-pdf/pypdf/compare/5.0.1...5.1.0) --- updated-dependencies: - dependency-name: pypdf[crypto] dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements/partial/requirements_production.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/partial/requirements_production.txt b/requirements/partial/requirements_production.txt index 09aac616bd..ee1a3c7080 100644 --- a/requirements/partial/requirements_production.txt +++ b/requirements/partial/requirements_production.txt @@ -4,7 +4,7 @@ bleach[css]==6.1.0 dependency_injector==4.43.0 fastjsonschema==2.20.0 gunicorn==23.0.0 -pypdf[crypto]==5.0.1 +pypdf[crypto]==5.1.0 requests==2.32.3 roman==4.2 simplejson==3.19.3 From cb07383d8cc109a9b892cf354e07b5d62a38400c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Nov 2024 09:53:12 +0100 Subject: [PATCH 05/51] Bump bleach[css] from 6.1.0 to 6.2.0 in /requirements/partial (#2709) Bumps [bleach[css]](https://github.com/mozilla/bleach) from 6.1.0 to 6.2.0. - [Changelog](https://github.com/mozilla/bleach/blob/main/CHANGES) - [Commits](https://github.com/mozilla/bleach/compare/v6.1.0...v6.2.0) --- updated-dependencies: - dependency-name: bleach[css] dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements/partial/requirements_production.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/partial/requirements_production.txt b/requirements/partial/requirements_production.txt index ee1a3c7080..83ac300a93 100644 --- a/requirements/partial/requirements_production.txt +++ b/requirements/partial/requirements_production.txt @@ -1,6 +1,6 @@ Babel==2.16.0 beautifulsoup4==4.12.3 -bleach[css]==6.1.0 +bleach[css]==6.2.0 dependency_injector==4.43.0 fastjsonschema==2.20.0 gunicorn==23.0.0 From b5921c5f6d29a2b3f7b2c070befb9e7eda569d06 Mon Sep 17 00:00:00 2001 From: luisa-beerboom <101706784+luisa-beerboom@users.noreply.github.com> Date: Wed, 13 Nov 2024 12:14:41 +0100 Subject: [PATCH 06/51] Check for poll permission correctly (#2622) * Check for poll permission correctly * Update documentation --- docs/actions/option.update.md | 2 +- docs/actions/poll.anonymize.md | 2 +- docs/actions/poll.create.md | 2 +- docs/actions/poll.delete.md | 2 +- docs/actions/poll.publish.md | 2 +- docs/actions/poll.reset.md | 2 +- docs/actions/poll.start.md | 2 +- docs/actions/poll.stop.md | 2 +- docs/actions/poll.update.md | 2 +- .../action/actions/poll/functions.py | 10 +++++++--- openslides_backend/action/actions/poll/mixins.py | 4 +++- tests/system/action/poll/test_create.py | 6 ++---- tests/system/action/poll/test_delete.py | 14 +++++++++++--- tests/system/action/poll/test_publish.py | 9 +++++++-- 14 files changed, 39 insertions(+), 22 deletions(-) diff --git a/docs/actions/option.update.md b/docs/actions/option.update.md index 8a197c2010..81d99e4bdf 100644 --- a/docs/actions/option.update.md +++ b/docs/actions/option.update.md @@ -20,4 +20,4 @@ If the poll's state is *created* and at least one vote value is given (`Y`, `N` The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/docs/actions/poll.anonymize.md b/docs/actions/poll.anonymize.md index ac0fbeb14f..442ffb07e5 100644 --- a/docs/actions/poll.anonymize.md +++ b/docs/actions/poll.anonymize.md @@ -12,4 +12,4 @@ Only for non-analog polls in the state *finished* or *published*. Sets all `vote The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/docs/actions/poll.create.md b/docs/actions/poll.create.md index 543194f794..7398cad8d8 100644 --- a/docs/actions/poll.create.md +++ b/docs/actions/poll.create.md @@ -68,4 +68,4 @@ The `entitled_group_ids` may not contain the meetings `anonymous_group_id`. The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/docs/actions/poll.delete.md b/docs/actions/poll.delete.md index f6ef35e38d..cfea2d9a28 100644 --- a/docs/actions/poll.delete.md +++ b/docs/actions/poll.delete.md @@ -10,4 +10,4 @@ Deletes the given poll and all linked options with all votes, too. The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/docs/actions/poll.publish.md b/docs/actions/poll.publish.md index 38df11f899..a5a0c8af5e 100644 --- a/docs/actions/poll.publish.md +++ b/docs/actions/poll.publish.md @@ -10,4 +10,4 @@ Sets the state to *published*. Only allowed for polls in the *finished* state. The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/docs/actions/poll.reset.md b/docs/actions/poll.reset.md index 33081bc0eb..0d8368b0bc 100644 --- a/docs/actions/poll.reset.md +++ b/docs/actions/poll.reset.md @@ -12,4 +12,4 @@ If `type != "pseudoanonymous"`, `is_pseudoanonymized` may be reset to `false` (i The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/docs/actions/poll.start.md b/docs/actions/poll.start.md index d50b49991c..1278779135 100644 --- a/docs/actions/poll.start.md +++ b/docs/actions/poll.start.md @@ -12,4 +12,4 @@ If `meeting/poll_couple_countdown` is true and the poll is an electronic poll, t The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/docs/actions/poll.stop.md b/docs/actions/poll.stop.md index a44d7abd42..d271a3e2bb 100644 --- a/docs/actions/poll.stop.md +++ b/docs/actions/poll.stop.md @@ -16,4 +16,4 @@ Some fields have to be calculated upon stopping a poll: The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/docs/actions/poll.update.md b/docs/actions/poll.update.md index c880c442da..4544c4d64c 100644 --- a/docs/actions/poll.update.md +++ b/docs/actions/poll.update.md @@ -43,4 +43,4 @@ The `entitled_group_ids` may not contain the meetings `anonymous_group_id`. The request user needs: - `motion.can_manage_polls` if the poll's content object is a motion - `assignment.can_manage` if the poll's content object is an assignment -- `poll.can_manage` else +- `poll.can_manage` if the poll's content object is a topic diff --git a/openslides_backend/action/actions/poll/functions.py b/openslides_backend/action/actions/poll/functions.py index ebd9666f9f..d4b9bbcc24 100644 --- a/openslides_backend/action/actions/poll/functions.py +++ b/openslides_backend/action/actions/poll/functions.py @@ -1,8 +1,8 @@ from ....permissions.permission_helper import has_perm from ....permissions.permissions import Permission, Permissions from ....services.datastore.interface import DatastoreService -from ....shared.exceptions import MissingPermission -from ....shared.patterns import KEYSEPARATOR +from ....shared.exceptions import ActionException, MissingPermission +from ....shared.patterns import KEYSEPARATOR, collection_from_fqid def check_poll_or_option_perms( @@ -15,7 +15,11 @@ def check_poll_or_option_perms( perm: Permission = Permissions.Motion.CAN_MANAGE_POLLS elif content_object_id.startswith("assignment" + KEYSEPARATOR): perm = Permissions.Assignment.CAN_MANAGE - else: + elif content_object_id.startswith("topic" + KEYSEPARATOR): perm = Permissions.Poll.CAN_MANAGE + else: + raise ActionException( + f"'{collection_from_fqid(content_object_id)}' is not a valid poll collection." + ) if not has_perm(datastore, user_id, perm, meeting_id): raise MissingPermission(perm) diff --git a/openslides_backend/action/actions/poll/mixins.py b/openslides_backend/action/actions/poll/mixins.py index 557b72c203..6616952e55 100644 --- a/openslides_backend/action/actions/poll/mixins.py +++ b/openslides_backend/action/actions/poll/mixins.py @@ -5,7 +5,7 @@ from openslides_backend.shared.typing import HistoryInformation from ....services.datastore.commands import GetManyRequest -from ....shared.exceptions import VoteServiceException +from ....shared.exceptions import ActionException, VoteServiceException from ....shared.interfaces.write_request import WriteRequest from ....shared.patterns import ( collection_from_fqid, @@ -33,6 +33,8 @@ def check_permissions(self, instance: dict[str, Any]) -> None: ) content_object_id = poll.get("content_object_id", "") meeting_id = poll["meeting_id"] + if not content_object_id: + raise ActionException("No 'content_object_id' was given") check_poll_or_option_perms( content_object_id, self.datastore, self.user_id, meeting_id ) diff --git a/tests/system/action/poll/test_create.py b/tests/system/action/poll/test_create.py index eb3e7bea6f..75f6d765f2 100644 --- a/tests/system/action/poll/test_create.py +++ b/tests/system/action/poll/test_create.py @@ -276,6 +276,7 @@ def test_create_wrong_publish_immediately(self) -> None: response = self.request( "poll.create", { + "content_object_id": "assignment/1", "title": "test_title_ahThai4pae1pi4xoogoo", "pollmethod": "YN", "type": "pseudoanonymous", @@ -799,10 +800,7 @@ def test_create_without_content_object(self) -> None: }, ) self.assert_status_code(response, 400) - assert ( - response.json["message"] - == "Creation of poll/1: You try to set following required fields to an empty value: ['content_object_id']" - ) + assert response.json["message"] == "No 'content_object_id' was given" def test_create_no_permissions_assignment(self) -> None: self.base_permission_test( diff --git a/tests/system/action/poll/test_delete.py b/tests/system/action/poll/test_delete.py index 639d04c9f8..714034d1f7 100644 --- a/tests/system/action/poll/test_delete.py +++ b/tests/system/action/poll/test_delete.py @@ -33,15 +33,18 @@ def test_delete_wrong_id(self) -> None: def test_delete_correct_cascading(self) -> None: self.set_models( { + "topic/1": {"poll_ids": [111], "meeting_id": 1}, "poll/111": { "option_ids": [42], "meeting_id": 1, "projection_ids": [1], + "content_object_id": "topic/1", }, "option/42": {"poll_id": 111, "meeting_id": 1}, "meeting/1": { "is_active_in_organization_id": 1, "all_projection_ids": [1], + "topic_ids": [1], }, "projection/1": { "content_object_id": "poll/111", @@ -64,9 +67,11 @@ def test_delete_correct_cascading(self) -> None: def test_delete_cascading_poll_candidate_list(self) -> None: self.set_models( { + "topic/1": {"poll_ids": [111], "meeting_id": 1}, "poll/111": { "option_ids": [42], "meeting_id": 1, + "content_object_id": "topic/1", }, "option/42": { "poll_id": 111, @@ -77,6 +82,7 @@ def test_delete_cascading_poll_candidate_list(self) -> None: "is_active_in_organization_id": 1, "poll_candidate_list_ids": [12], "poll_candidate_ids": [13], + "topic_ids": [1], }, "poll_candidate_list/12": { "meeting_id": 1, @@ -100,12 +106,14 @@ def test_delete_cascading_poll_candidate_list(self) -> None: def test_delete_no_permissions(self) -> None: self.base_permission_test( - {"poll/111": {"meeting_id": 1}}, "poll.delete", {"id": 111} + {"poll/111": {"meeting_id": 1, "content_object_id": "topic/1"}}, + "poll.delete", + {"id": 111}, ) def test_delete_permissions(self) -> None: self.base_permission_test( - {"poll/111": {"meeting_id": 1}}, + {"poll/111": {"meeting_id": 1, "content_object_id": "topic/1"}}, "poll.delete", {"id": 111}, Permissions.Poll.CAN_MANAGE, @@ -113,7 +121,7 @@ def test_delete_permissions(self) -> None: def test_delete_permissions_locked_meeting(self) -> None: self.base_locked_out_superadmin_permission_test( - {"poll/111": {"meeting_id": 1}}, + {"poll/111": {"meeting_id": 1, "content_object_id": "topic/1"}}, "poll.delete", {"id": 111}, ) diff --git a/tests/system/action/poll/test_publish.py b/tests/system/action/poll/test_publish.py index b7d5fad5cd..b2c9126c5e 100644 --- a/tests/system/action/poll/test_publish.py +++ b/tests/system/action/poll/test_publish.py @@ -42,8 +42,13 @@ def test_publish_assignment(self) -> None: def test_publish_wrong_state(self) -> None: self.set_models( { - "poll/1": {"state": "created", "meeting_id": 1}, - "meeting/1": {"is_active_in_organization_id": 1}, + "topic/1": {"poll_ids": [111], "meeting_id": 1}, + "poll/1": { + "state": "created", + "meeting_id": 1, + "content_object_id": "topic/1", + }, + "meeting/1": {"is_active_in_organization_id": 1, "topic_ids": [1]}, } ) response = self.request("poll.publish", {"id": 1}) From cf020c471b66f3d7f0d2e681a296eb7e8d3db9b5 Mon Sep 17 00:00:00 2001 From: luisa-beerboom <101706784+luisa-beerboom@users.noreply.github.com> Date: Wed, 20 Nov 2024 10:01:09 +0100 Subject: [PATCH 07/51] Agenda item permission checks for motion.create (#2728) --- .../action/actions/motion/create.py | 5 +++ tests/system/action/motion/test_create.py | 43 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/openslides_backend/action/actions/motion/create.py b/openslides_backend/action/actions/motion/create.py index 2a7128b83c..01c241e999 100644 --- a/openslides_backend/action/actions/motion/create.py +++ b/openslides_backend/action/actions/motion/create.py @@ -149,6 +149,11 @@ def check_permissions(self, instance: dict[str, Any]) -> None: # whitelist the fields depending on the user's permissions whitelist = [] forbidden_fields = set() + perm = Permissions.AgendaItem.CAN_MANAGE + if has_perm(self.datastore, self.user_id, perm, instance["meeting_id"]): + whitelist = [*agenda_creation_properties.keys()] + elif contained := set(agenda_creation_properties.keys()).intersection(instance): + forbidden_fields.update(contained) perm = Permissions.Mediafile.CAN_SEE if has_perm(self.datastore, self.user_id, perm, instance["meeting_id"]): whitelist.append("attachment_mediafile_ids") diff --git a/tests/system/action/motion/test_create.py b/tests/system/action/motion/test_create.py index 4740c9e8a6..5e4b305fea 100644 --- a/tests/system/action/motion/test_create.py +++ b/tests/system/action/motion/test_create.py @@ -4,6 +4,7 @@ from openslides_backend.action.mixins.delegation_based_restriction_mixin import ( DelegationBasedRestriction, ) +from openslides_backend.models.models import AgendaItem from openslides_backend.permissions.base_classes import Permission from openslides_backend.permissions.permissions import Permissions from tests.system.action.base import BaseActionTestCase @@ -422,6 +423,48 @@ def setup_permission_test( if additional_data: self.set_models(additional_data) + def test_create_permission_agenda_allowed(self) -> None: + self.setup_permission_test( + [ + Permissions.AgendaItem.CAN_MANAGE, + Permissions.Motion.CAN_CREATE, + Permissions.Motion.CAN_MANAGE_METADATA, + ] + ) + response = self.request( + "motion.create", + { + "title": "test_Xcdfgee", + "meeting_id": 1, + "text": "test", + "agenda_create": True, + "agenda_type": AgendaItem.INTERNAL_ITEM, + }, + ) + self.assert_status_code(response, 200) + + def test_create_permission_agenda_forbidden(self) -> None: + self.setup_permission_test( + [ + Permissions.Motion.CAN_CREATE, + Permissions.Motion.CAN_MANAGE_METADATA, + ] + ) + response = self.request( + "motion.create", + { + "title": "test_Xcdfgee", + "meeting_id": 1, + "text": "test", + "agenda_create": True, + "agenda_type": AgendaItem.INTERNAL_ITEM, + }, + ) + self.assert_status_code(response, 403) + assert "Forbidden fields: " in response.json["message"] + assert "agenda_create" in response.json["message"] + assert "agenda_type" in response.json["message"] + def test_create_permission_missing_can_manage(self) -> None: self.setup_permission_test([Permissions.Motion.CAN_CREATE]) response = self.request( From dec9a053cc99851f9d2223cb03d5129a5fea9f85 Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Mon, 25 Nov 2024 14:56:05 +0100 Subject: [PATCH 08/51] beautify presenter docs (#2732) Improving js syntax Adding js highlighting Deleting deleted presenters Specifying required and optional parameters --- docs/migration_route.md | 4 +-- docs/presenters/check_database.md | 12 +++---- docs/presenters/check_database_all.md | 10 +++--- docs/presenters/export_meeting.md | 4 +-- docs/presenters/get_active_users_amount.md | 2 +- docs/presenters/get_forwarding_committees.md | 6 ++-- docs/presenters/get_forwarding_meetings.md | 12 +++---- docs/presenters/get_mediafile_context.md | 18 +++++----- docs/presenters/get_meetings.md | 28 --------------- docs/presenters/get_user_related_models.md | 20 +++++------ docs/presenters/get_user_scope.md | 4 +-- docs/presenters/get_users.md | 11 +++--- docs/presenters/search_deleted_models.md | 36 ------------------- .../search_for_id_by_external_id.md | 12 +++---- docs/presenters/search_users.md | 5 +-- 15 files changed, 61 insertions(+), 123 deletions(-) delete mode 100644 docs/presenters/get_meetings.md delete mode 100644 docs/presenters/search_deleted_models.md diff --git a/docs/migration_route.md b/docs/migration_route.md index 26bd427f6d..8a80883d2a 100644 --- a/docs/migration_route.md +++ b/docs/migration_route.md @@ -23,7 +23,7 @@ enum MigrationState { "success": true, "status"?: MigrationState, "output"?: str, - "exception"?: str, + "exception"?: str } ``` `output` always contains the full output of the migration command up to this point. `exception` contains the thrown exception, if any, which can only be the case if the command is finished (meaning `status != "migration_running"`). After issuing a migration command, it is waited a short period of time for the thread to finish, so the status can be all of these things for any command (e.g. after calling `migrate`, the returned status can be either `MIGRATION_RUNNING` if the migrations did not finish directly or `FINALIZATION_REQUIRED` if the migration is already done). @@ -41,7 +41,7 @@ The `stats` return value is the following: "positions": int, "events": int, "partially_migrated_positions": int, - "fully_migrated_positions": int, + "fully_migrated_positions": int } } ``` diff --git a/docs/presenters/check_database.md b/docs/presenters/check_database.md index 57518ec938..cf1574bf81 100644 --- a/docs/presenters/check_database.md +++ b/docs/presenters/check_database.md @@ -1,23 +1,23 @@ # Payload -``` +```js { // optional - meeting_id: integer; + meeting_id: integer } ``` # Returns If okay: -``` +```js { - ok: boolean, + ok: boolean } ``` else: -``` +```js { ok: boolean, - errors: string, + errors: string } ``` diff --git a/docs/presenters/check_database_all.md b/docs/presenters/check_database_all.md index 393c25ae7e..e2881c5bac 100644 --- a/docs/presenters/check_database_all.md +++ b/docs/presenters/check_database_all.md @@ -1,21 +1,21 @@ # Payload -``` +```js { } ``` # Returns If okay: -``` +```js { - ok: boolean, + ok: boolean } ``` else: -``` +```js { ok: boolean, - errors: string, + errors: string } ``` diff --git a/docs/presenters/export_meeting.md b/docs/presenters/export_meeting.md index 02d73abc1b..b94aaab652 100644 --- a/docs/presenters/export_meeting.md +++ b/docs/presenters/export_meeting.md @@ -1,8 +1,8 @@ # Payload -``` +```js { - meeting_id: Id + meeting_id: Id // required } ``` diff --git a/docs/presenters/get_active_users_amount.md b/docs/presenters/get_active_users_amount.md index 018973a645..5e6ce7a141 100644 --- a/docs/presenters/get_active_users_amount.md +++ b/docs/presenters/get_active_users_amount.md @@ -6,7 +6,7 @@ Nothing ```js { - active_users_amount: Number; + active_users_amount: Number } ``` diff --git a/docs/presenters/get_forwarding_committees.md b/docs/presenters/get_forwarding_committees.md index df01a69435..afc410ec47 100644 --- a/docs/presenters/get_forwarding_committees.md +++ b/docs/presenters/get_forwarding_committees.md @@ -1,14 +1,14 @@ # Payload -``` +```js { - meeting_id: Id + meeting_id: Id // required } ``` # Returns -``` +```js string[] ``` diff --git a/docs/presenters/get_forwarding_meetings.md b/docs/presenters/get_forwarding_meetings.md index b76a5c454a..d971356012 100644 --- a/docs/presenters/get_forwarding_meetings.md +++ b/docs/presenters/get_forwarding_meetings.md @@ -1,19 +1,19 @@ # Payload -``` +```js { - meeting_id: Id + meeting_id: Id // required } ``` # Returns -``` +```js [ { - id: Id - name: string - default_meeting_id: Id + id: Id, + name: string, + default_meeting_id: Id, meeting: [{id: Id, name: string, start_time:timestamp|null, end_time:timestamp|null}, ...] }, ... diff --git a/docs/presenters/get_mediafile_context.md b/docs/presenters/get_mediafile_context.md index e67fb81923..cd91e4352d 100644 --- a/docs/presenters/get_mediafile_context.md +++ b/docs/presenters/get_mediafile_context.md @@ -2,7 +2,7 @@ ```js { - mediafile_ids: Id[]; + mediafile_ids: Id[] // required } ``` @@ -15,16 +15,16 @@ published: boolean, meetings_of_interest: { [meeting_id: Id]: { - name: string; - holds_attachments: boolean; - holds_logos: boolean; - holds_fonts: boolean; - holds_current_projections: boolean; - holds_history_projections: boolean; - holds_preview_projections: boolean; + name: string, + holds_attachments: boolean, + holds_logos: boolean, + holds_fonts: boolean, + holds_current_projections: boolean, + holds_history_projections: boolean, + holds_preview_projections: boolean } }, - children_amount: int, + children_amount: int } } ``` diff --git a/docs/presenters/get_meetings.md b/docs/presenters/get_meetings.md deleted file mode 100644 index 00567e7c34..0000000000 --- a/docs/presenters/get_meetings.md +++ /dev/null @@ -1,28 +0,0 @@ -# Payload - -``` -{ - with_deleted: boolean, # default: False - with_archived: boolean, # default: False -} -``` - -# Returns - -``` -[ - { - id: Id, - name: string, - deleted: boolean, - is_active_in_organization: int, - }, - ... -] -``` - -# Logic - -The request user needs OML `can_manage_users` or higher or CML `can_manage`. - -This presenter creates a filtered list of meetings for various situations. With CML permission the list shows only meetings of committees, where the user has the needed permission. diff --git a/docs/presenters/get_user_related_models.md b/docs/presenters/get_user_related_models.md index db71df47f8..6f7a9808de 100644 --- a/docs/presenters/get_user_related_models.md +++ b/docs/presenters/get_user_related_models.md @@ -2,7 +2,7 @@ ```js { - user_ids: Id[]; + user_ids: Id[] // required } ``` @@ -12,16 +12,16 @@ { [user_id: Id]: { organization_management_level: OML-String, - committees: [{ id: Id; name: String; cml: CML-String; }], + committees: [{ id: Id, name: String, cml: CML-String }], meetings: [{ - id: Id; - name: String; - is_active_in_organization_id: Id; - is_locked: boolean; - motion_submitter_ids: Id[]; - assignment_candidate_ids: Id[]; - speaker_ids: Id[]; - locked_out: boolean; + id: Id, + name: String, + is_active_in_organization_id: Id, + is_locked: boolean, + motion_submitter_ids: Id[], + assignment_candidate_ids: Id[], + speaker_ids: Id[], + locked_out: boolean }] } } diff --git a/docs/presenters/get_user_scope.md b/docs/presenters/get_user_scope.md index 928d8fc250..b4b4eca3a6 100644 --- a/docs/presenters/get_user_scope.md +++ b/docs/presenters/get_user_scope.md @@ -2,7 +2,7 @@ ```js { - user_ids: Id[]; + user_ids: Id[] // required } ``` @@ -13,7 +13,7 @@ user_id: Id: { collection: String, # one of "meeting", "committee" or "organization" id: Id, - user_oml: String, # one of "superadmin", "can_manage_organization", "can_manage_users", "" + user_oml: String, # one of "superadmin", "can_manage_organization", "can_manage_users", "" committee_ids: int[] // Ids of all committees the user is part of } } diff --git a/docs/presenters/get_users.md b/docs/presenters/get_users.md index b20707f5f4..65b147a663 100644 --- a/docs/presenters/get_users.md +++ b/docs/presenters/get_users.md @@ -1,24 +1,25 @@ # Payload -``` +```js { + // optional start_index: number, entries: number, sort_criteria: string[], // can contain ["username", "first_name", "last_name"], reverse: boolean, - filter?: string, + filter?: string } ``` # Returns -``` +```js [ { id: Id, username: string, first_name: string, - last_name: string, + last_name: string }, ... ] @@ -28,4 +29,4 @@ The request user needs OML `can_manage_users` or higher. Otherwise an error is returned. -Returns all users, that have `filer` in `username`, `first_name`, `last_name`. If filter is `null`, all users are returned. The users are sorted by `sort_criteria`. If it is not given, the default is `["username", "first_name", "last_name"]`. If `reverse` is true, the order is reversed. Lastly, the users are paginated beginning at `start_index` with at max `entries` number of users. +Returns all users, that have `filter` in `username`, `first_name`, `last_name`. If filter is `null`, all users are returned. The users are sorted by `sort_criteria`. If it is not given, the default is `["username", "first_name", "last_name"]`. If `reverse` is true, the order is reversed. Lastly, the users can be paginated beginning at `start_index` with at max `entries` number of users. diff --git a/docs/presenters/search_deleted_models.md b/docs/presenters/search_deleted_models.md deleted file mode 100644 index 3abb15b104..0000000000 --- a/docs/presenters/search_deleted_models.md +++ /dev/null @@ -1,36 +0,0 @@ -## Payload - -```js -{ - collection: string, - filter_string: string, - meeting_id: Id, -} -``` - -## Presenter - -Searches all deleted models of the given collection in the given meeting for the given filter string. The fields which -are searched differ from collection to collection: -``` -{ - "assignment": ["title"], - "motion": ["number", "title"], - "user": [ - "username", - "first_name", - "last_name", - "title", - "pronoun", - "structure_level", - "number", - "email", - ], -} -``` -These 3 are also the only allowed collections. The result list is returned as a mapping from the -model's id to the searched fields of the model (see above). - -## Permissions - -TODO diff --git a/docs/presenters/search_for_id_by_external_id.md b/docs/presenters/search_for_id_by_external_id.md index ebbd42a818..b90145c899 100644 --- a/docs/presenters/search_for_id_by_external_id.md +++ b/docs/presenters/search_for_id_by_external_id.md @@ -2,22 +2,22 @@ ```js { // required - collection: string; - external_id: string; - context_id: Id; + collection: string, + external_id: string, + context_id: Id } ``` ## Returns ```js { - id: Id; + id: Id } ``` in the case one id is found. ```js { - id: null; - error: string; + id: null, + error: string } ``` else. diff --git a/docs/presenters/search_users.md b/docs/presenters/search_users.md index 416cafead2..a220075874 100644 --- a/docs/presenters/search_users.md +++ b/docs/presenters/search_users.md @@ -2,6 +2,7 @@ ```js { + // required permission_type: "meeting" | "committee" | "organization" permission_id: number, // Id of permission scope object search: { @@ -10,7 +11,7 @@ "first_name": string, "last_name": string, "email": string, - "member_number": string, + "member_number": string }[] } ``` @@ -24,7 +25,7 @@ "first_name": string, "last_name": string, "email": string, - "member_number": string, + "member_number": string }[][] ``` A double array: The outer array has the same length as the request's `search` array and contains From 1bdd315cf7ffe5eaa35c1a2a7bee029ae543acfe Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Mon, 25 Nov 2024 15:48:00 +0100 Subject: [PATCH 09/51] remove participant presence status when removed from meeting (#2730) --- global/data/example-data.json | 2 +- global/data/initial-data.json | 2 +- .../action/actions/meeting_user/delete.py | 4 +- .../user/conditional_speaker_cascade_mixin.py | 14 +++ .../action/actions/user/user_mixins.py | 6 +- .../0062_unset_presence_of_removed_users.py | 87 +++++++++++++++++ .../system/action/meeting_user/test_delete.py | 18 +++- tests/system/action/user/test_delete.py | 6 +- tests/system/action/user/test_update.py | 16 ++++ ...st_0062_unset_presence_of_removed_users.py | 93 +++++++++++++++++++ 10 files changed, 238 insertions(+), 10 deletions(-) create mode 100644 openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py create mode 100644 tests/system/migrations/test_0062_unset_presence_of_removed_users.py diff --git a/global/data/example-data.json b/global/data/example-data.json index 3f2550c9f0..d842581ee9 100644 --- a/global/data/example-data.json +++ b/global/data/example-data.json @@ -1,5 +1,5 @@ { - "_migration_index": 62, + "_migration_index": 63, "gender":{ "1":{ "id": 1, diff --git a/global/data/initial-data.json b/global/data/initial-data.json index 57df08e21b..ba6880235f 100644 --- a/global/data/initial-data.json +++ b/global/data/initial-data.json @@ -1,5 +1,5 @@ { - "_migration_index": 62, + "_migration_index": 63, "gender":{ "1":{ "id": 1, diff --git a/openslides_backend/action/actions/meeting_user/delete.py b/openslides_backend/action/actions/meeting_user/delete.py index c4ac6fb585..da82a4a221 100644 --- a/openslides_backend/action/actions/meeting_user/delete.py +++ b/openslides_backend/action/actions/meeting_user/delete.py @@ -34,9 +34,11 @@ def get_history_information(self) -> HistoryInformation | None: def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: meeting_user = self.datastore.get( - fqid_from_collection_and_id("meeting_user", instance["id"]), ["speaker_ids"] + fqid_from_collection_and_id("meeting_user", instance["id"]), + ["speaker_ids", "user_id", "meeting_id"], ) speaker_ids = meeting_user.get("speaker_ids", []) self.conditionally_delete_speakers(speaker_ids) + self.remove_presence(meeting_user["user_id"], meeting_user["meeting_id"]) return super().update_instance(instance) diff --git a/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py b/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py index 35ef5f1bb3..77fede8615 100644 --- a/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py +++ b/openslides_backend/action/actions/user/conditional_speaker_cascade_mixin.py @@ -6,6 +6,7 @@ from ....shared.patterns import fqid_from_collection_and_id from ...action import Action from ..speaker.delete import SpeakerDeleteAction +from .set_present import UserSetPresentAction class ConditionalSpeakerCascadeMixinHelper(Action): @@ -41,6 +42,18 @@ def conditionally_delete_speakers(self, speaker_ids: list[int]) -> None: [{"id": speaker["id"]} for speaker in speakers_to_delete], ) + def remove_presence(self, user_id: int, meeting_id: int) -> None: + self.execute_other_action( + UserSetPresentAction, + [ + { + "id": user_id, + "meeting_id": meeting_id, + "present": False, + } + ], + ) + class ConditionalSpeakerCascadeMixin(ConditionalSpeakerCascadeMixinHelper): """ @@ -65,6 +78,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: for speaker_id in val.get("speaker_ids", []) ] self.conditionally_delete_speakers(speaker_ids) + self.remove_presence(instance["id"], removed_meeting_id) return super().update_instance(instance) diff --git a/openslides_backend/action/actions/user/user_mixins.py b/openslides_backend/action/actions/user/user_mixins.py index f2b3c836d5..9f5c99f899 100644 --- a/openslides_backend/action/actions/user/user_mixins.py +++ b/openslides_backend/action/actions/user/user_mixins.py @@ -142,10 +142,8 @@ def strip_field(self, field: str, instance: dict[str, Any]) -> None: def check_meeting_and_users( self, instance: dict[str, Any], user_fqid: FullQualifiedId ) -> None: - if instance.get("meeting_id") is not None: - self.datastore.apply_changed_model( - user_fqid, {"meeting_id": instance.get("meeting_id")} - ) + if (meeting_id := instance.get("meeting_id")) is not None: + self.datastore.apply_changed_model(user_fqid, {"meeting_id": meeting_id}) def meeting_user_set_data(self, instance: dict[str, Any]) -> None: meeting_user_data = {} diff --git a/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py new file mode 100644 index 0000000000..4ee49676de --- /dev/null +++ b/openslides_backend/migrations/migrations/0062_unset_presence_of_removed_users.py @@ -0,0 +1,87 @@ +from collections import defaultdict + +from datastore.migrations import BaseModelMigration +from datastore.reader.core import GetManyRequestPart +from datastore.writer.core import BaseRequestEvent, RequestUpdateEvent + +from openslides_backend.shared.patterns import fqid_from_collection_and_id + +from ...shared.filters import And, FilterOperator + + +class Migration(BaseModelMigration): + """ + This migration removes the presence status if the user is not part of the meeting anymore. + """ + + target_migration_index = 63 + + def migrate_models(self) -> list[BaseRequestEvent] | None: + present_users_per_meeting: dict[int, list[int]] = defaultdict(list) + meetings_per_present_user: dict[int, list[int]] = defaultdict(list) + + meeting_users = self.reader.filter( + "meeting_user", + And( + FilterOperator("group_ids", "=", "[]"), + FilterOperator("meta_deleted", "!=", True), + ), + ["user_id", "meeting_id"], + ) + for meeting_user in meeting_users.values(): + user_id = meeting_user["user_id"] + meeting_id = meeting_user["meeting_id"] + meetings_per_present_user[user_id].append(meeting_id) + present_users_per_meeting[meeting_id].append(user_id) + + meetings = self.reader.get_many( + [ + GetManyRequestPart( + "meeting", + [meeting_id for meeting_id in present_users_per_meeting], + ["present_user_ids"], + ) + ] + ).get("meeting", dict()) + users = self.reader.get_many( + [ + GetManyRequestPart( + "user", + [present_user_id for present_user_id in meetings_per_present_user], + ["is_present_in_meeting_ids"], + ) + ] + ).get("user", dict()) + return [ + *[ + RequestUpdateEvent( + fqid_from_collection_and_id("meeting", meeting_id), + { + "present_user_ids": [ + id_ + for id_ in meetings.get(meeting_id, dict()).get( + "present_user_ids", [] + ) + if id_ not in user_ids + ] + }, + ) + for meeting_id, user_ids in present_users_per_meeting.items() + if meetings + ], + *[ + RequestUpdateEvent( + fqid_from_collection_and_id("user", user_id), + { + "is_present_in_meeting_ids": [ + id_ + for id_ in users.get(user_id, dict()).get( + "is_present_in_meeting_ids", [] + ) + if id_ not in meeting_ids + ] + }, + ) + for user_id, meeting_ids in meetings_per_present_user.items() + ], + ] diff --git a/tests/system/action/meeting_user/test_delete.py b/tests/system/action/meeting_user/test_delete.py index d2e50b2fa9..54d9841acd 100644 --- a/tests/system/action/meeting_user/test_delete.py +++ b/tests/system/action/meeting_user/test_delete.py @@ -16,7 +16,18 @@ def test_delete(self) -> None: def test_delete_with_speaker(self) -> None: self.set_models( { - "meeting/10": {"is_active_in_organization_id": 1}, + "meeting/10": { + "is_active_in_organization_id": 1, + "present_user_ids": [1], + }, + "meeting/101": { + "is_active_in_organization_id": 1, + "present_user_ids": [1], + }, + "user/1": { + "is_present_in_meeting_ids": [10, 101], + "meeting_user_ids": [5], + }, "meeting_user/5": { "user_id": 1, "meeting_id": 10, @@ -38,6 +49,11 @@ def test_delete_with_speaker(self) -> None: self.assert_model_deleted("meeting_user/5") self.assert_model_deleted("speaker/1") self.assert_model_exists("speaker/2", {"meeting_id": 10, "begin_time": 123456}) + self.assert_model_exists( + "user/1", {"is_present_in_meeting_ids": [101], "meeting_user_ids": []} + ) + self.assert_model_exists("meeting/10", {"present_user_ids": []}) + self.assert_model_exists("meeting/101", {"present_user_ids": [1]}) def test_delete_with_chat_message(self) -> None: self.set_models( diff --git a/tests/system/action/user/test_delete.py b/tests/system/action/user/test_delete.py index a21ceac962..890ff3a31e 100644 --- a/tests/system/action/user/test_delete.py +++ b/tests/system/action/user/test_delete.py @@ -74,13 +74,14 @@ def test_delete_with_speaker(self) -> None: "user/111": { "username": "username_srtgb123", "meeting_user_ids": [1111], + "is_present_in_meeting_ids": [1], }, "meeting_user/1111": { "meeting_id": 1, "user_id": 111, "speaker_ids": [15, 16], }, - "meeting/1": {}, + "meeting/1": {"present_user_ids": [111]}, "speaker/15": { "meeting_user_id": 1111, "meeting_id": 1, @@ -95,13 +96,14 @@ def test_delete_with_speaker(self) -> None: response = self.request("user.delete", {"id": 111}) self.assert_status_code(response, 200) - self.assert_model_deleted("user/111") + self.assert_model_deleted("user/111", {"is_present_in_meeting_ids": []}) self.assert_model_deleted("meeting_user/1111") self.assert_model_exists( "speaker/15", {"meeting_user_id": None, "meeting_id": 1, "begin_time": 12345678}, ) self.assert_model_deleted("speaker/16") + self.assert_model_exists("meeting/1", {"present_user_ids": []}) def test_delete_with_candidate(self) -> None: self.set_models( diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index 75405ef691..2e3ace2e9c 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -2436,6 +2436,7 @@ def test_group_removal_with_speaker(self) -> None: "user/1234": { "username": "username_abcdefgh123", "meeting_user_ids": [4444, 5555], + "is_present_in_meeting_ids": [4, 5], }, "meeting_user/4444": { "meeting_id": 4, @@ -2453,11 +2454,13 @@ def test_group_removal_with_speaker(self) -> None: "is_active_in_organization_id": 1, "meeting_user_ids": [4444], "committee_id": 1, + "present_user_ids": [1234], }, "meeting/5": { "is_active_in_organization_id": 1, "meeting_user_ids": [5555], "committee_id": 1, + "present_user_ids": [1234], }, "committee/1": {"meeting_ids": [4, 5]}, "speaker/14": {"meeting_user_id": 4444, "meeting_id": 4}, @@ -2481,6 +2484,19 @@ def test_group_removal_with_speaker(self) -> None: { "username": "username_abcdefgh123", "meeting_user_ids": [4444, 5555], + "is_present_in_meeting_ids": [5], + }, + ) + self.assert_model_exists( + "meeting/4", + { + "present_user_ids": [], + }, + ) + self.assert_model_exists( + "meeting/5", + { + "present_user_ids": [1234], }, ) self.assert_model_exists( diff --git a/tests/system/migrations/test_0062_unset_presence_of_removed_users.py b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py new file mode 100644 index 0000000000..19481a8e0c --- /dev/null +++ b/tests/system/migrations/test_0062_unset_presence_of_removed_users.py @@ -0,0 +1,93 @@ +from typing import Any + + +def create_data() -> dict[str, dict[str, Any]]: + return { + "meeting/1": { + "id": 1, + "name": "meeting name", + "present_user_ids": [1, 3, 2], + "meeting_user_ids": [3, 4, 5], + "group_ids": [10], + }, + "meeting/2": { + "id": 1, + "name": "meeting name", + "present_user_ids": [2], + "meeting_user_ids": [6], + }, + "user/1": { + "id": 1, + "username": "correct_user", + "is_present_in_meeting_ids": [1], + "meeting_user_ids": [3], + }, + "user/2": { + "id": 2, + "username": "wrong_user", + "is_present_in_meeting_ids": [2, 1], + # meeting users do exist after user left meeting but have no group ids + "meeting_user_ids": [ + 5, + 6, + ], + }, + "user/3": { + "id": 3, + "username": "correct_user", + "is_present_in_meeting_ids": [1], + "meeting_user_ids": [4], + }, + "meeting_user/3": {"id": 3, "user_id": 1, "meeting_id": 1, "group_ids": [10]}, + "meeting_user/4": {"id": 4, "user_id": 3, "meeting_id": 1, "group_ids": [10]}, + "meeting_user/5": {"id": 5, "user_id": 2, "meeting_id": 1, "group_ids": []}, + "meeting_user/6": {"id": 6, "user_id": 2, "meeting_id": 2, "group_ids": []}, + "group/10": {"id": 10, "meeting_user_ids": [3, 4], "meeting_id": 1}, + } + + +def test_migration_both_ways(write, finalize, assert_model): + data = create_data() + for fqid, fields in data.items(): + write({"type": "create", "fqid": fqid, "fields": fields}) + + finalize("0062_unset_presence_of_removed_users") + + data["meeting/1"]["present_user_ids"] = [1, 3] + data["meeting/2"]["present_user_ids"] = [] + data["user/2"]["is_present_in_meeting_ids"] = [] + + for fqid, fields in data.items(): + assert_model(fqid, fields) + + +def test_migration_one_way(write, finalize, assert_model): + data = create_data() + data["meeting/1"]["present_user_ids"] = [1, 3] + data["meeting/2"]["present_user_ids"] = [] + + for fqid, fields in data.items(): + write({"type": "create", "fqid": fqid, "fields": fields}) + + finalize("0062_unset_presence_of_removed_users") + + data["user/2"]["is_present_in_meeting_ids"] = [] + + for fqid, fields in data.items(): + assert_model(fqid, fields) + + +def test_migration_other_way(write, finalize, assert_model): + data = create_data() + data["user/2"]["is_present_in_meeting_ids"] = [] + + for fqid, fields in data.items(): + write({"type": "create", "fqid": fqid, "fields": fields}) + + finalize("0062_unset_presence_of_removed_users") + + data["meeting/1"]["present_user_ids"] = [1, 3] + data["meeting/2"]["present_user_ids"] = [] + + for fqid, fields in data.items(): + assert_model(fqid, fields) From deac9e116c0d5ff3e3fbde035b9218bfd11a716c Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Mon, 25 Nov 2024 16:04:14 +0100 Subject: [PATCH 10/51] new saml meeting mapping (#2722) remove old direct meeting mapping --------- Co-authored-by: rrenkert --- docs/actions/user.save_saml_account.md | 58 +- global/data/example-data.json | 3 +- global/data/initial-data.json | 3 +- .../action/actions/meeting_user/create.py | 58 +- .../action/actions/organization/update.py | 23 +- .../action/actions/speaker/create.py | 2 +- .../action/actions/user/save_saml_account.py | 447 +++++++++++--- .../system/action/organization/test_update.py | 73 ++- tests/system/action/speaker/test_create.py | 2 +- tests/system/action/user/test_create.py | 2 +- .../action/user/test_save_saml_account.py | 584 ++++++++++++++++-- tests/system/action/user/test_update.py | 8 +- 12 files changed, 1091 insertions(+), 172 deletions(-) diff --git a/docs/actions/user.save_saml_account.md b/docs/actions/user.save_saml_account.md index 523507efce..15e2891bd6 100644 --- a/docs/actions/user.save_saml_account.md +++ b/docs/actions/user.save_saml_account.md @@ -11,6 +11,8 @@ pronoun: string, is_active: boolean, is_physical_person: boolean, + member_number: string, + // Additional meeting related data can be given. See below explanation on meeting mappers. } ``` @@ -31,7 +33,61 @@ Extras to do on creation: As you can see there is no password for local login and the user can't change it. -- Add user to the meeting by adding him to the group given in the organization-wide field-mapping as `"meeting": { "external_id": "xyz", "external_group_id": "delegates"}` if a `meeting`-entry is given. If it fails for any reason, a log entry is written, but no exception thrown. Add the user always to the group, if it fails try to add him to the default group. +### Meeting Mappers +- The saml attribute mapping can have a list of 'meeting_mappers' that can be used to assign users meeting related data. (See example below.) + - A mapper can be given a 'name' for debugging purposes. + - The 'external_id' maps to the meeting and is required (logged as warning if meeting does not exist). Multiple mappers can map to the same meeting. + - If 'allow_update' is set to false, the mapper is only used if the user does not already exist. If it is not given it defaults to true. + - Mappers are only used if every condition in the list of 'conditions' resolves to true. For this the 'attribute' in the payload data needs to match the string or regex given in 'condition'. If no condition is given this defaults to true. + - The actual mappings are objects or lists of objects of attribute-default pairs (exception: number, which only has the option of an attribute). + - The attribute refers to the payloads data. + - A default value can be given in case the payloads attribute does not exist or contains no data. (Logged as debug) + - Groups and structure levels are given as a list of attribute-default pairs. +- On conflict of multiple mappers mappings on a same meetings field the last given mappers data for that field is used. Exception to this are groups and structure levels. Their data is combined. +- Values for groups and structure levels can additionally be given in comma separated lists composed as a single string. +- Values for groups are interpreted as their external ID and structure levels as their name within that meeting. +- If no group exists for a meeting and no default is given, the meetings default group is used. (Logged as warning) +- If a structure level does not exist, it is created. +- Vote weights need to be given as 6 digit decimal strings. + +``` +"meeting_mappers": [{ + "name": "Mapper-Name", + "external_id": "M2025", + "allow_update": "false", + "conditions": [{ + "attribute": "membernumber", + "condition": "1426\d{4,6}$" + }, { + "attribute": "function", + "condition": "board" + }], + "mappings": { + "groups": [{ + "attribute": "membership", + "default": "admin, standard" + }], + "structure_levels": [{ + "attribute": "ovname", + "default": "struct1, struct2" + }], + "number": {"attribute": "p_number"}, + "comment": { + "attribute": "idp_comment", + "default": "Group set via SSO" + }, + "vote_weight": { + "attribute": "vote", + "default":"1.000000" + }, + "present": { + "attribute": "present_key", + "default":"True" + } + } +}] +``` +If you are using Keycloak as your SAML-server, make sure to fill the attributes of all users. Then you also need to configure for each attribute in 'Clients' a mapping for your Openslides services 'Client Scopes'. Choose 'User Attribute' and assign the 'User Attribute' as in the step before and the 'SAML Attribut Name' as defined in Openslides 'meeting_mappers'. ## Return Value diff --git a/global/data/example-data.json b/global/data/example-data.json index d842581ee9..f0a1400f69 100644 --- a/global/data/example-data.json +++ b/global/data/example-data.json @@ -72,7 +72,8 @@ "gender": "gender", "pronoun": "pronoun", "is_active": "is_active", - "is_physical_person": "is_person" + "is_physical_person": "is_person", + "member_number": "member_number" } } }, diff --git a/global/data/initial-data.json b/global/data/initial-data.json index ba6880235f..a219af8dd1 100644 --- a/global/data/initial-data.json +++ b/global/data/initial-data.json @@ -58,7 +58,8 @@ "gender": "gender", "pronoun": "pronoun", "is_active": "is_active", - "is_physical_person": "is_person" + "is_physical_person": "is_person", + "member_number": "member_number" } } }, diff --git a/openslides_backend/action/actions/meeting_user/create.py b/openslides_backend/action/actions/meeting_user/create.py index 929cfc4083..5b08281db6 100644 --- a/openslides_backend/action/actions/meeting_user/create.py +++ b/openslides_backend/action/actions/meeting_user/create.py @@ -54,24 +54,22 @@ def get_history_information(self) -> HistoryInformation | None: information = {} for instance in self.instances: instance_information = [] - if "group_ids" in instance: - if len(instance["group_ids"]) == 1: - instance_information.extend( - [ - "Participant added to group {} in meeting {}", - fqid_from_collection_and_id( - "group", instance["group_ids"][0] - ), - ] + fqids_per_collection = { + collection_name: [ + fqid_from_collection_and_id( + collection_name, + _id, ) - else: - instance_information.append( - "Participant added to multiple groups in meeting {}", - ) - else: - instance_information.append( - "Participant added to meeting {}", - ) + for _id in ids + ] + for collection_name in ["group", "structure_level"] + if (ids := instance.get(f"{collection_name}_ids")) + } + instance_information.append( + self.compose_history_string(list(fqids_per_collection.items())) + ) + for collection_name, fqids in fqids_per_collection.items(): + instance_information.extend(fqids) instance_information.append( fqid_from_collection_and_id("meeting", instance["meeting_id"]), ) @@ -79,3 +77,29 @@ def get_history_information(self) -> HistoryInformation | None: instance_information ) return information + + def compose_history_string( + self, fqids_per_collection: list[tuple[str, list[str]]] + ) -> str: + """ + Composes a string of the shape: + Participant added to groups {}, {} and structure levels {} in meeting {}. + """ + middle_sentence_parts = [ + " ".join( + [ # prefix and to collection name if it's not the first in list + ("and " if collection_name != fqids_per_collection[0][0] else "") + + collection_name.replace("_", " ") # replace for human readablity + + ("s" if len(fqids) != 1 else ""), # plural s + ", ".join(["{}" for _ in range(len(fqids))]), + ] + ) + for collection_name, fqids in fqids_per_collection + ] + return " ".join( + [ + "Participant added to", + *middle_sentence_parts, + ("in " if fqids_per_collection else "") + "meeting {}.", + ] + ) diff --git a/openslides_backend/action/actions/organization/update.py b/openslides_backend/action/actions/organization/update.py index c82a1c90fc..95ff496010 100644 --- a/openslides_backend/action/actions/organization/update.py +++ b/openslides_backend/action/actions/organization/update.py @@ -60,13 +60,24 @@ class OrganizationUpdate( field: {**optional_str_schema, "max_length": 256} for field in allowed_user_fields } - saml_props["meeting"] = { - "type": ["object", "null"], - "properties": { - field: {**optional_str_schema, "max_length": 256} - for field in ("external_id", "external_group_id") + saml_props["meeting_mappers"] = { + "type": ["array", "null"], + "items": { + "type": "object", + "properties": { + **{ + field: {**optional_str_schema, "max_length": 256} + for field in ("external_id", "name", "allow_update") + }, + "conditions": { + "type": ["array", "null"], + "max_length": 256, + }, # , "items": {"object"} + "mappings": {"type": ["object", "array"], "max_length": 256}, + }, + "required": ["external_id"], + "additionalProperties": False, }, - "additionalProperties": False, } schema = DefaultSchema(Organization()).get_update_schema( optional_properties=group_A_fields + group_B_fields, diff --git a/openslides_backend/action/actions/speaker/create.py b/openslides_backend/action/actions/speaker/create.py index cf9aaed91f..e0eff2684a 100644 --- a/openslides_backend/action/actions/speaker/create.py +++ b/openslides_backend/action/actions/speaker/create.py @@ -294,7 +294,7 @@ def validate_fields(self, instance: dict[str, Any]) -> dict[str, Any]: user = self.datastore.get(user_fqid, ["is_present_in_meeting_ids"]) if meeting_id not in user.get("is_present_in_meeting_ids", ()): raise ActionException( - "Only present users can be on the lists of speakers." + "Only present users can be on the list of speakers." ) if not meeting.get("list_of_speakers_allow_multiple_speakers"): diff --git a/openslides_backend/action/actions/user/save_saml_account.py b/openslides_backend/action/actions/user/save_saml_account.py index 1989cde148..35648b3fb4 100644 --- a/openslides_backend/action/actions/user/save_saml_account.py +++ b/openslides_backend/action/actions/user/save_saml_account.py @@ -1,4 +1,6 @@ -from collections.abc import Iterable +import re +from collections import defaultdict +from collections.abc import Generator, Iterable from typing import Any, cast import fastjsonschema @@ -7,7 +9,7 @@ from ....models.models import User from ....shared.exceptions import ActionException -from ....shared.filters import And, FilterOperator +from ....shared.filters import And, FilterOperator, Or from ....shared.interfaces.event import Event from ....shared.schema import schema_version from ....shared.typing import Schema @@ -18,6 +20,7 @@ from ...util.register import register_action from ...util.typing import ActionData, ActionResultElement from ..gender.create import GenderCreate +from ..structure_level.create import StructureLevelCreateAction from .create import UserCreate from .update import UserUpdate from .user_mixins import UsernameMixin @@ -32,6 +35,16 @@ "pronoun", "is_active", "is_physical_person", + "member_number", +] + +allowed_meeting_user_fields = [ + "groups", + "structure_levels", + "number", + "comment", + "vote_weight", + "present", ] @@ -63,7 +76,9 @@ def validate_instance(self, instance: dict[str, Any]) -> None: raise ActionException( "SingleSignOn is not enabled in OpenSlides configuration" ) - self.saml_attr_mapping = organization.get("saml_attr_mapping", {}) + self.saml_attr_mapping: dict[str, Any] = organization.get( + "saml_attr_mapping", dict() + ) if not self.saml_attr_mapping or not isinstance(self.saml_attr_mapping, dict): raise ActionException( "SingleSignOn field attributes are not configured in OpenSlides" @@ -111,7 +126,10 @@ def validate_instance(self, instance: dict[str, Any]) -> None: def validate_fields(self, instance_old: dict[str, Any]) -> dict[str, Any]: """ - Transforms the payload fields into model fields, removes the possible array-wrapped format + Transforms the payload fields into model fields, removes the possible array-wrapped format. + Mapper data is comprised on a per meeting basis. On conflicts the last statement is used. + Groups and structure levels are combined, however. + Meeting related data will be transformed via the idp attributes to the actual model data. """ instance: dict[str, Any] = dict() for model_field, payload_field in self.saml_attr_mapping.items(): @@ -120,14 +138,14 @@ def validate_fields(self, instance_old: dict[str, Any]) -> dict[str, Any]: and payload_field in instance_old and model_field in allowed_user_fields ): - value = ( + idp_attribute = ( tx[0] if isinstance((tx := instance_old[payload_field]), list) and len(tx) else tx ) - if value not in (None, []): - instance[model_field] = value - + if idp_attribute not in (None, []): + instance[model_field] = idp_attribute + self.apply_meeting_mapping(instance, instance_old) return super().validate_fields(instance) def prepare_action_data(self, action_data: ActionData) -> ActionData: @@ -138,49 +156,49 @@ def check_permissions(self, instance: dict[str, Any]) -> None: pass def base_update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: - meeting_id, group_id = self.check_for_group_add() users = self.datastore.filter( "user", FilterOperator("saml_id", "=", instance["saml_id"]), - ["id", "gender_id", *allowed_user_fields], + [ + "id", + "meeting_user_ids", + "is_present_in_meeting_ids", + "gender_id", + *allowed_user_fields, + ], ) - - if gender := instance.get("gender"): - if gender == "": - instance["gender_id"] = None + if gender := instance.pop("gender", None): + gender_dict = self.datastore.filter( + "gender", + FilterOperator("name", "=", gender), + ["id"], + ) + gender_id = None + if gender_dict: + gender_id = next(iter(gender_dict.keys())) else: - gender_dict = self.datastore.filter( - "gender", - FilterOperator("name", "=", gender), - ["id"], + action_result = self.execute_other_action( + GenderCreate, [{"name": gender}] ) - if gender_dict: - gender_id = next(iter(gender_dict.keys())) - else: - action_result = self.execute_other_action( - GenderCreate, [{"name": gender}] - ) - gender_id = action_result[0].get("id", 0) # type: ignore + if action_result and action_result[0]: + gender_id = action_result[0].get("id", 0) + if gender_id: instance["gender_id"] = gender_id - del instance["gender"] + else: + self.logger.warning( + f"save_saml_account could neither find nor create {gender}. Not handling gender." + ) + # Empty string: remove gender_id elif gender == "": instance["gender_id"] = None - del instance["gender"] + meeting_users: dict[int, dict[str, Any]] | None = dict() + user_id = None if len(users) == 1: self.user = next(iter(users.values())) instance["id"] = (user_id := cast(int, self.user["id"])) - if meeting_id and group_id: - meeting_user = get_meeting_user( - self.datastore, meeting_id, user_id, ["id", "group_ids"] - ) - if meeting_user: - old_group_ids = meeting_user["group_ids"] - if group_id not in old_group_ids: - instance["meeting_id"] = meeting_id - instance["group_ids"] = old_group_ids + [group_id] - else: - instance["meeting_id"] = meeting_id - instance["group_ids"] = [group_id] + meeting_users = self.apply_meeting_user_data(instance, user_id, True) + if meeting_users: + self.update_meeting_users_from_db(meeting_users, user_id) instance = { k: v for k, v in instance.items() if k == "id" or v != self.user.get(k) } @@ -188,19 +206,24 @@ def base_update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: self.execute_other_action(UserUpdate, [instance]) elif len(users) == 0: instance = self.set_defaults(instance) - if group_id: - instance["meeting_id"] = meeting_id - instance["group_ids"] = [group_id] - self.execute_other_action(UserCreate, [instance]) + meeting_users = instance.pop("meeting_user_data", None) + response = self.execute_other_action(UserCreate, [instance]) + if response and response[0]: + user_id = response[0].get("id") + instance["meeting_user_data"] = meeting_users + if user_id: + meeting_users = self.apply_meeting_user_data(instance, user_id, False) else: ActionException( f"More than one existing user found in database with saml_id {instance['saml_id']}" ) + if meeting_users: + self.execute_other_action(UserUpdate, [mu for mu in meeting_users.values()]) return instance def create_events(self, instance: dict[str, Any]) -> Iterable[Event]: """ - delegated to execute_other_actions + delegated to execute_other_action """ return [] @@ -218,46 +241,312 @@ def set_defaults(self, instance: dict[str, Any]) -> dict[str, Any]: instance["username"] = self.generate_usernames([instance.get("saml_id", "")])[0] return instance - def check_for_group_add(self) -> tuple[int, int] | tuple[None, None]: - NoneResult = (None, None) - if not ( - meeting_info := cast(dict, self.saml_attr_mapping.get("meeting")) - ) or not (external_id := meeting_info.get("external_id")): - return NoneResult - - meetings = self.datastore.filter( - collection="meeting", - filter=FilterOperator("external_id", "=", external_id), - mapped_fields=["id", "default_group_id"], + def validate_meeting_mapper( + self, instance: dict[str, Any], meeting_mapper: dict[str, Any] + ) -> bool: + """ + Validates the meeting mapper to be complete. Returns False if not. + Returns True if the mapper matches its criteria on instances values or no conditions were given. + Instances values can not be None or empty string. + """ + if not meeting_mapper.get("external_id"): + return False + if not (mapper_conditions := meeting_mapper.get("conditions")): + return True + return all( + ( + (instance_value := instance.get(mapper_condition.get("attribute"))) + and regex_condition.search(instance_value) + ) + for mapper_condition in mapper_conditions + if (regex_condition := re.compile(mapper_condition.get("condition"))) ) - if len(meetings) == 1: - meeting = next(iter(meetings.values())) - group_id = meeting["default_group_id"] - else: + + def apply_meeting_mapping( + self, instance: dict[str, Any], instance_old: dict[str, Any] + ) -> None: + if meeting_mappers := cast( + list[dict[str, Any]], + self.saml_attr_mapping.get("meeting_mappers", []), + ): + meeting_user_data: dict[str, Any] = defaultdict(dict) + for meeting_mapper in meeting_mappers: + if self.validate_meeting_mapper(instance_old, meeting_mapper): + meeting_external_id = cast(str, meeting_mapper["external_id"]) + mapping_results = meeting_user_data[meeting_external_id] + allow_update: str | bool + if isinstance( + allow_update := cast( + str, meeting_mapper.get("allow_update", "True") + ), + str, + ): + allow_update = allow_update.casefold() != "False".casefold() + result = { + **{ + key: value + for key, value in self.get_field_data( + instance_old, + mapping_results.get("for_create", dict()), + meeting_mapper, + ) + }, + } + if allow_update: + mapping_results["for_create"] = result + mapping_results["for_update"] = { + **{ + key: value + for key, value in self.get_field_data( + instance_old, + mapping_results.get("for_update", dict()), + meeting_mapper, + ) + }, + } + else: + mapping_results["for_create"] = result + if meeting_user_data: + instance["meeting_user_data"] = meeting_user_data + else: + self.logger.warning( + "save_saml_account found no matching meeting mappers." + ) + + def apply_meeting_user_data( + self, instance: dict[str, Any], user_id: int, is_update: bool + ) -> dict[int, dict[str, Any]] | None: + if not (meeting_user_data := instance.pop("meeting_user_data", None)) or not ( + external_meeting_ids := sorted( + [ext_id for ext_id in meeting_user_data.keys()] + ) + ): + return None + meetings = { + meeting_id: meeting + for meeting_id, meeting in sorted( + self.datastore.filter( + "meeting", + Or( + FilterOperator("external_id", "=", external_meeting_id) + for external_meeting_id in external_meeting_ids + ), + ["id", "default_group_id", "external_id"], + ).items() + ) + } + missing_meetings = [ + external_meeting_id + for external_meeting_id in external_meeting_ids + if external_meeting_id + not in {meeting.get("external_id") for meeting in meetings.values()} + ] + if missing_meetings: self.logger.warning( - f"save_saml_account found {len(meetings)} meetings with external_id '{external_id}'" + f"save_saml_account found no meetings for {len(missing_meetings)} meetings with external_ids {missing_meetings}" + ) + # declare and half way through initialize mu data + result: dict[int, dict[str, Any]] = dict() + for ( + meeting_id, + meeting, + ) in meetings.items(): + if not ( + instance_meeting_user_data := meeting_user_data.get( + meeting["external_id"] + ) + ): + continue + if is_update: + instance_meeting_user = instance_meeting_user_data.get("for_update") + else: + instance_meeting_user = instance_meeting_user_data.get("for_create") + if instance_meeting_user is not None: + instance_meeting_user["id"] = user_id + instance_meeting_user["meeting_id"] = meeting_id + for saml_meeting_user_field in ["groups", "structure_levels"]: + names = sorted( + instance_meeting_user.pop(saml_meeting_user_field, []) + ) + if saml_meeting_user_field == "groups": + ids = self.get_group_ids(names, meeting) + elif saml_meeting_user_field == "structure_levels": + ids = self.get_structure_level_ids(names, meeting) + if ids: + instance_meeting_user[ + f"{saml_meeting_user_field.rstrip('s')}_ids" + ] = ids + if instance_meeting_user.pop("present", ""): + present_in_meeting_ids = instance.get( + "is_present_in_meeting_ids", [] + ) + if meeting_id not in present_in_meeting_ids: + present_in_meeting_ids.append(meeting_id) + instance["is_present_in_meeting_ids"] = present_in_meeting_ids + result[meeting_id] = instance_meeting_user + return result + + def update_meeting_users_from_db( + self, meeting_users: dict[int, dict[str, Any]], user_id: int + ) -> None: + """updates meeting users with groups and structure level relations from database""" + for meeting_id, meeting_user in meeting_users.items(): + if meeting_user_db := get_meeting_user( + self.datastore, + meeting_id, + user_id, + ["id", "group_ids", "structure_level_ids"], + ): + for field_name in ["group_ids", "structure_level_ids"]: + if old_ids := meeting_user_db.get(field_name): + ids = meeting_user.get(field_name, []) + for _id in ids: + if _id not in old_ids: + meeting_user[field_name] = old_ids + [_id] + + def get_field_data( + self, + instance: dict[str, Any], + meeting_user: dict[str, Any], + meeting_mapper: dict[str, dict[str, Any]], + ) -> Generator[tuple[str, Any]]: + """ + returns the field data for the given idp mapping field. Groups the groups and structure levels for each meeting. + Uses mappers for generating default values. + """ + missing_attributes = [] + for saml_meeting_user_field in allowed_meeting_user_fields: + result: set[str] | str | bool = "" + meeting_mapping = meeting_mapper.get("mappings", dict()) + result = meeting_user.get(saml_meeting_user_field, "") + if saml_meeting_user_field in ["groups", "structure_levels"]: + attr_default_list = meeting_mapping.get(saml_meeting_user_field, []) + else: + attr_default_list = [ + meeting_mapping.get(saml_meeting_user_field, dict()) + ] + for attr_default in attr_default_list: + idp_attribute = attr_default.get("attribute", "") + if saml_meeting_user_field == "number": + # Number cannot have a default. + if value := instance.get(idp_attribute): + result = cast(str, value) + else: + missing_attributes.append(idp_attribute) + elif not (value := instance.get(idp_attribute)): + missing_attributes.append(idp_attribute) + value = attr_default.get("default") + if value: + if saml_meeting_user_field in ["groups", "structure_levels"]: + # Need to append to group and structure_level for same meeting. + if not result: + result = set() + cast(set, result).update(value.split(", ")) + elif saml_meeting_user_field == "comment": + # Want comments from all matching mappers. + if result: + result = cast(str, result) + " " + value + else: + result = value + elif saml_meeting_user_field == "present": + # Result is int or bool. int will later be interpreted as bool. + result = ( + value + if not isinstance(value, str) + else ( + False + if value.casefold() == "false".casefold() + else True + ) + ) + else: + result = value + if result: + yield saml_meeting_user_field, result + if fields := ",".join(missing_attributes): + mapper_name = meeting_mapper.get("name", "unnamed") + self.logger.debug( + f"Meeting mapper: {mapper_name} could not find value in idp data for fields: {fields}. Using default if available." ) - return NoneResult - if external_group_id := meeting_info.get("external_group_id"): + + def get_group_ids(self, group_names: list[str], meeting: dict) -> list[int]: + """ + Gets the group ids from given group names in that meeting. + If none of the groups exists in the meeting, the meetings default group is returned. + """ + if group_names: groups = self.datastore.filter( - collection="group", - filter=And( - [ - FilterOperator("external_id", "=", external_group_id), - FilterOperator("meeting_id", "=", meeting.get("id")), - ] + "group", + And( + FilterOperator("meeting_id", "=", meeting["id"]), + Or( + FilterOperator("external_id", "=", group_name) + for group_name in group_names + ), ), - mapped_fields=["id"], + ["meeting_user_ids"], ) - if len(groups) == 1: - group_id = next(iter(groups.keys())) - else: - self.logger.warning( - f"save_saml_account found no group in meeting '{external_id}' for '{external_group_id}', but use default_group of meeting" - ) - if not group_id: + if len(groups) > 0: + return sorted(groups) + if default_group_id := meeting["default_group_id"]: + external_meeting_id = meeting["external_id"] self.logger.warning( - f"save_saml_account found no group in meeting '{external_id}' for '{external_group_id}'" + f"save_saml_account found no group in meeting '{external_meeting_id}' for {group_names}, but used default_group of meeting" ) - return NoneResult - return meeting.get("id"), group_id + return [default_group_id] + else: + assert False + + def get_structure_level_ids( + self, structure_level_names: list[str], meeting: dict[str, Any] + ) -> list[int]: + """ + Gets the structure level ids from given structure level names in that meeting. + For this also creates new structure levels not already existing in the meeting. + """ + if structure_level_names: + meeting_id = meeting["id"] + found_structure_levels = self.datastore.filter( + "structure_level", + And( + FilterOperator("meeting_id", "=", meeting_id), + Or( + FilterOperator("name", "=", structure_level_name) + for structure_level_name in structure_level_names + if structure_level_name + ), + ), + ["meeting_user_ids"], + ) + found_structure_level_ids = list(found_structure_levels.keys()) + if len(found_structure_levels) == len(structure_level_names): + return found_structure_level_ids + else: + found_structure_level_names = [ + structure_level.get("name") + for structure_level in found_structure_levels.values() + ] + to_be_created_structure_levels = [ + sl_name + for sl_name in structure_level_names + if sl_name and sl_name not in found_structure_level_names + ] + # meeting_user_ids are only known during UserUpdate. Hence we cannot do batch create for all meeting users + if structure_levels_result := ( + self.execute_other_action( + StructureLevelCreateAction, + [ + {"name": structure_level_name, "meeting_id": meeting_id} + for structure_level_name in to_be_created_structure_levels + ], + ) + ): + return sorted( + [ + structure_level["id"] + for structure_level in structure_levels_result + if structure_level + ] + + found_structure_level_ids + ) + return [] diff --git a/tests/system/action/organization/test_update.py b/tests/system/action/organization/test_update.py index 182db03a65..d13555d9a6 100644 --- a/tests/system/action/organization/test_update.py +++ b/tests/system/action/organization/test_update.py @@ -6,7 +6,11 @@ class OrganizationUpdateActionTest(BaseActionTestCase): - saml_attr_mapping: dict[str, str | dict[str, str]] = { + ListOfDicts = list[dict[str, str]] + MeetingMappers = list[ + dict[str, str | ListOfDicts | dict[str, str | dict[str, str] | ListOfDicts]] + ] + saml_attr_mapping: dict[str, str | MeetingMappers] = { "saml_id": "username", "title": "title", "first_name": "firstName", @@ -16,6 +20,7 @@ class OrganizationUpdateActionTest(BaseActionTestCase): "pronoun": "pronoun", "is_active": "is_active", "is_physical_person": "is_person", + "member_number": "member_number", } def setUp(self) -> None: @@ -64,7 +69,46 @@ def test_update(self) -> None: def test_update_with_meeting(self) -> None: self.saml_attr_mapping.update( - {"meeting": {"external_id": "Landtag", "external_group_id": "Delegated"}} + { + "meeting_mappers": [ + { + "name": "Mapper-Name", + "external_id": "Landtag", + "allow_update": "false", + "conditions": [ + {"attribute": "membernumber", "condition": r"1426\d{4,6}$"}, + {"attribute": "function", "condition": "board"}, + ], + "mappings": { + "groups": [ + { + "attribute": "membership", + "default": "admin, standard", + } + ], + "structure_levels": [ + { + "attribute": "ovname", + "default": "struct1, struct2", + } + ], + "number": {"attribute": "p_number"}, + "comment": { + "attribute": "idp_comment", + "default": "Group set via SSO", + }, + "vote_weight": { + "attribute": "vote", + "default": "1.000000", + }, + "present": { + "attribute": "present_key", + "default": "True", + }, + }, + } + ] + } ), response = self.request( "organization.update", @@ -85,9 +129,28 @@ def test_update_with_meeting(self) -> None: }, ) - def test_update_with_meeting_error(self) -> None: + def test_update_with_meeting_missing_ext_id(self) -> None: + self.saml_attr_mapping.update( + {"meeting_mappers": [{"external_idx": "Landtag"}]} + ), + response = self.request( + "organization.update", + { + "id": 1, + "name": "testtest", + "description": "blablabla", + "saml_attr_mapping": self.saml_attr_mapping, + }, + ) + self.assert_status_code(response, 400) + assert ( + "data.saml_attr_mapping.meeting_mappers[0] must contain ['external_id'] properties" + in response.json["message"] + ) + + def test_update_with_meeting_wrong_attr(self) -> None: self.saml_attr_mapping.update( - {"meeting": {"external_idx": "Landtag", "external_group_id": "Delegated"}} + {"meeting_mappers": [{"external_id": "Landtag", "unkown_field": " "}]} ), response = self.request( "organization.update", @@ -100,7 +163,7 @@ def test_update_with_meeting_error(self) -> None: ) self.assert_status_code(response, 400) assert ( - "data.saml_attr_mapping.meeting must not contain {'external_idx'} properties" + "data.saml_attr_mapping.meeting_mappers[0] must not contain {'unkown_field'} properties" in response.json["message"] ) diff --git a/tests/system/action/speaker/test_create.py b/tests/system/action/speaker/test_create.py index 93000e44d4..c5a04b445f 100644 --- a/tests/system/action/speaker/test_create.py +++ b/tests/system/action/speaker/test_create.py @@ -353,7 +353,7 @@ def test_create_user_not_present(self) -> None: self.assert_status_code(response, 400) self.assert_model_not_exists("speaker/1") self.assertIn( - "Only present users can be on the lists of speakers.", + "Only present users can be on the list of speakers.", response.json["message"], ) diff --git a/tests/system/action/user/test_create.py b/tests/system/action/user/test_create.py index 96e76bf4d3..dfcc537b8f 100644 --- a/tests/system/action/user/test_create.py +++ b/tests/system/action/user/test_create.py @@ -140,7 +140,7 @@ def test_create_some_more_fields(self) -> None: ) self.assert_history_information( "user/2", - ["Account created", "Participant added to meeting {}", "meeting/111"], + ["Account created", "Participant added to meeting {}.", "meeting/111"], ) def test_create_comment(self) -> None: diff --git a/tests/system/action/user/test_save_saml_account.py b/tests/system/action/user/test_save_saml_account.py index 795f948812..4374924d00 100644 --- a/tests/system/action/user/test_save_saml_account.py +++ b/tests/system/action/user/test_save_saml_account.py @@ -500,6 +500,7 @@ def setUp(self) -> None: self.organization = { "saml_enabled": True, "saml_attr_mapping": { + "member_number": "member_number", "saml_id": "username", "title": "title", "first_name": "firstName", @@ -509,130 +510,575 @@ def setUp(self) -> None: "pronoun": "pronoun", "is_active": "is_active", "is_physical_person": "is_person", - "meeting": { - "external_id": "Landtag", - "external_group_id": "Delegates", - }, }, } + self.meeting_mappers = [ + { + "name": "works", + "external_id": "Landtag", + "conditions": [ + {"attribute": "member_number", "condition": "LV_.*"}, + { + "attribute": "email", + "condition": "[\\w\\.]+@([\\w-]+\\.)+[\\w]{2,4}", + }, + ], + "mappings": { + "comment": { + "attribute": "idp_commentary", + "default": "Vote weight, groups and structure levels set via SSO.", + }, + "number": {"attribute": "participant_number"}, + "structure_levels": [ + { + "attribute": "structure", + "default": "structure1", + } + ], + "groups": [ + { + "attribute": "idp_group_attribute", + "default": "not_a_group", + } + ], + "vote_weight": {"attribute": "vw", "default": "1.000000"}, + "present": {"attribute": "presence", "default": "True"}, + }, + }, + { + "name": "works_too", + "external_id": "Kreistag", + "conditions": [{"attribute": "kv_member_number", "condition": "KV_.*"}], + "mappings": { + "comment": { + "attribute": "idp_commentary", + "default": "Vote weight, groups and structure levels set via SSO.", + }, + "number": {"attribute": "participant_kv_number"}, + "structure_levels": [ + { + "attribute": "kv_structure", + "default": "structure1", + } + ], + "groups": [ + { + "attribute": "kv_group_attribute", + "default": "not_a_group", + } + ], + "vote_weight": { + "attribute": "kv_vw", + "default": "1.000000", + }, + "present": {"attribute": "kv_presence", "default": "True"}, + }, + }, + ] + self.organization["saml_attr_mapping"]["meeting_mappers"] = self.meeting_mappers # type: ignore self.create_meeting() + self.create_meeting(4) self.set_models( { "organization/1": self.organization, "group/1": {"external_id": "Default"}, "group/2": {"external_id": "Delegates"}, "group/3": {"external_id": "Admin"}, + "group/4": {"external_id": "Default"}, + "group/5": {"external_id": "Delegates"}, + "group/6": {"external_id": "Admin"}, "meeting/1": {"external_id": "Landtag", "default_group_id": 1}, + "meeting/4": {"external_id": "Kreistag", "default_group_id": 4}, "user/1": {"saml_id": "admin_saml"}, } ) - def test_create_user_with_membership(self) -> None: - response = self.request("user.save_saml_account", {"username": ["111"]}) + def test_create_user_with_multi_membership(self) -> None: + """ + Shows: + * generally works for multiple matching mappers on different meetings + * if default for 'groups' doesn't resolve, default group of that meeting is used + """ + response = self.request( + "user.save_saml_account", + { + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "idp_kv_group_attribute": "Delegates", + "kv_structure": "structure2", + "idp_commentary": "normal data used", + }, + ) self.assert_status_code(response, 200) + self.app.logger.warning.assert_called_with( # type: ignore + "save_saml_account found no group in meeting 'Kreistag' for ['not_a_group'], but used default_group of meeting" + ) self.assert_model_exists( "user/2", { "saml_id": "111", "username": "111", - "meeting_user_ids": [1], - "meeting_ids": [1], + "email": "holzi@holz.de", + "meeting_user_ids": [1, 2], + "meeting_ids": [1, 4], + "is_present_in_meeting_ids": [1, 4], }, ) self.assert_model_exists( - "meeting_user/1", {"user_id": 2, "group_ids": [2], "meeting_id": 1} + "meeting_user/1", + { + "user_id": 2, + "meeting_id": 1, + "group_ids": [2], + "structure_level_ids": [1], + "vote_weight": "1.000000", + "number": "MG_1254", + "comment": "normal data used", + }, + ) + self.assert_model_exists( + "meeting_user/2", + { + "user_id": 2, + "group_ids": [4], + "meeting_id": 4, + "comment": "normal data used", + "structure_level_ids": [2], + "vote_weight": "1.000000", + "number": "MG_1254", + }, ) self.assert_model_exists( "group/2", {"meeting_user_ids": [1], "external_id": "Delegates"} ) + self.assert_model_exists( + "structure_level/1", + {"meeting_user_ids": [1], "name": "structure1"}, + ) + self.assert_model_exists( + "structure_level/2", + {"meeting_user_ids": [2], "name": "structure2"}, + ) - def test_update_user_with_membership(self) -> None: - response = self.request("user.save_saml_account", {"username": ["admin_saml"]}) + def test_create_user_with_multi_membership_multi(self) -> None: + """ + Shows: + * group and structure levels can be multiple values in concatenated, comma separated list string for: + * default values + * saml datas values + * multiple entries in structure level and group lists are respected. + * multiple values can be repeated + * mappers can share idp data fields + """ + self.meeting_mappers[1]["mappings"]["groups"] = [ # type: ignore + { + "attribute": "use_default", + "default": "Default, Delegates, Delegates", + }, + {"attribute": "idp_group_attribute", "default": ""}, + ] + self.set_models({"organization/1": self.organization}) + response = self.request( + "user.save_saml_account", + { + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates, Admin, Delegates", + "kv_member_number": "KV_Könighols", + "participant_kv_number": "MG_1254", + "idp_kv_group_attribute": "Delegates", + "kv_structure": "structure2", + }, + ) self.assert_status_code(response, 200) self.assert_model_exists( - "user/1", + "user/2", { - "saml_id": "admin_saml", - "username": "admin", + "saml_id": "111", + "username": "111", + "meeting_user_ids": [1, 2], + "meeting_ids": [1, 4], + }, + ) + self.assert_model_exists( + "meeting_user/1", {"user_id": 2, "group_ids": [2, 3], "meeting_id": 1} + ) + self.assert_model_exists( + "meeting_user/2", {"user_id": 2, "group_ids": [4, 5, 6], "meeting_id": 4} + ) + self.assert_model_exists( + "group/2", {"meeting_user_ids": [1], "external_id": "Delegates"} + ) + self.assert_model_exists( + "structure_level/1", + {"meeting_user_ids": [1], "name": "structure1"}, + ) + self.assert_model_exists( + "structure_level/2", + {"meeting_user_ids": [2], "name": "structure2"}, + ) + + def test_create_user_with_multi_membership_not_matching(self) -> None: + """ + shows: + * matching only one mapper -> creating only one meeting user + """ + response = self.request( + "user.save_saml_account", + { + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "LV_Königholz", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "idp_kv_group_attribute": "Delegates", + "kv_structure": "structure2", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/2", + { + "saml_id": "111", + "username": "111", "meeting_user_ids": [1], "meeting_ids": [1], }, ) self.assert_model_exists( - "meeting_user/1", {"user_id": 1, "group_ids": [2], "meeting_id": 1} + "meeting_user/1", {"user_id": 2, "group_ids": [2], "meeting_id": 1} ) self.assert_model_exists( "group/2", {"meeting_user_ids": [1], "external_id": "Delegates"} ) + self.assert_model_exists( + "structure_level/1", + {"meeting_user_ids": [1], "name": "structure1"}, + ) + self.assert_model_not_exists("structure_level/2") - def test_create_user_invalid_meeting(self) -> None: - """silent fail, user created and logged in""" - self.organization["saml_attr_mapping"]["meeting"]["external_id"] = "Kreistag" # type: ignore + def test_create_user_mapping_no_mapper(self) -> None: + del self.organization["saml_attr_mapping"]["meeting_mappers"] # type: ignore self.set_models({"organization/1": self.organization}) - response = self.request("user.save_saml_account", {"username": ["111"]}) - self.assert_status_code(response, 200) - self.app.logger.warning.assert_called_with( # type: ignore - "save_saml_account found 0 meetings with external_id 'Kreistag'" + response = self.request( + "user.save_saml_account", + { + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "idp_kv_group_attribute": "Delegates", + "kv_structure": "structure2", + "kv_presence": "True", + }, ) + self.assert_status_code(response, 200) self.assert_model_exists( - "user/2", {"saml_id": "111", "username": "111", "meeting_user_ids": None} + "user/2", + { + "saml_id": "111", + "username": "111", + }, ) self.assert_model_not_exists("meeting_user/1") + self.assert_model_not_exists("structure_level/1") + + def test_create_user_mapping_no_mapping(self) -> None: + del self.meeting_mappers[0]["mappings"] + del self.meeting_mappers[1] + self.set_models({"organization/1": self.organization}) + response = self.request( + "user.save_saml_account", + { + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "idp_kv_group_attribute": "Delegates", + "kv_structure": "structure2", + "kv_presence": "True", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/2", + { + "saml_id": "111", + "username": "111", + }, + ) self.assert_model_exists( - "group/2", {"meeting_user_ids": None, "external_id": "Delegates"} + "meeting_user/1", {"user_id": 2, "group_ids": [1], "meeting_id": 1} ) + self.assert_model_exists( + "group/1", {"meeting_user_ids": [1], "external_id": "Default"} + ) + self.assert_model_not_exists("structure_level/1") - def test_create_user_invalid_group_but_default(self) -> None: - """silent fail, but added to default group and logged in""" - self.organization["saml_attr_mapping"]["meeting"][ # type: ignore - "external_group_id" - ] = "Developers" + def test_create_user_mapping_empty_mappings(self) -> None: + self.meeting_mappers[0]["mappings"] = dict() + del self.meeting_mappers[1] self.set_models({"organization/1": self.organization}) - response = self.request("user.save_saml_account", {"username": ["111"]}) - self.assert_status_code(response, 200) - self.app.logger.warning.assert_called_with( # type: ignore - "save_saml_account found no group in meeting 'Landtag' for 'Developers', but use default_group of meeting" + response = self.request( + "user.save_saml_account", + { + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "idp_kv_group_attribute": "Delegates", + "kv_structure": "structure2", + "kv_presence": "True", + }, ) + self.assert_status_code(response, 200) self.assert_model_exists( - "user/2", {"saml_id": "111", "meeting_user_ids": [1], "meeting_ids": [1]} + "user/2", + { + "saml_id": "111", + "username": "111", + }, ) self.assert_model_exists( "meeting_user/1", {"user_id": 2, "group_ids": [1], "meeting_id": 1} ) self.assert_model_exists( - "group/1", + "group/1", {"meeting_user_ids": [1], "external_id": "Default"} + ) + self.assert_model_not_exists("structure_level/1") + + def test_create_user_meeting_not_exists(self) -> None: + """ + Shows: if meeting does not exist error is logged. + """ + self.meeting_mappers[0]["external_id"] = "Bundestag" + del self.meeting_mappers[1] + self.set_models({"organization/1": self.organization}) + response = self.request( + "user.save_saml_account", { - "meeting_user_ids": [1], - "external_id": "Default", - "default_group_for_meeting_id": 1, + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "kv_group_attribute": "Delegates", + "kv_structure": "structure2", + }, + ) + self.assert_status_code(response, 200) + self.app.logger.warning.assert_called_with( # type: ignore + "save_saml_account found no meetings for 1 meetings with external_ids ['Bundestag']" + ) + self.assert_model_exists( + "user/2", + { + "saml_id": "111", + "username": "111", + "meeting_user_ids": None, + "meeting_ids": None, }, ) + self.assert_model_not_exists("meeting_user/1") + self.assert_model_not_exists("structure_level/1") - def test_create_user_only_meeting_given(self) -> None: - """silent fail, but added to default group and logged in""" - del self.organization["saml_attr_mapping"]["meeting"]["external_group_id"] # type: ignore - self.set_models({"organization/1": self.organization}) - response = self.request("user.save_saml_account", {"username": ["111"]}) + def test_create_user_only_one_sl_exists(self) -> None: + """Shows: no errors if one structure level exists and the other doesn't. Latter being created.""" + self.create_model("structure_level/1", {"name": "structure1", "meeting_id": 1}) + response = self.request( + "user.save_saml_account", + { + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "idp_kv_group_attribute": "Delegates", + "kv_structure": "structure2", + }, + ) self.assert_status_code(response, 200) self.assert_model_exists( - "user/2", {"saml_id": "111", "meeting_user_ids": [1], "meeting_ids": [1]} + "user/2", + { + "saml_id": "111", + "username": "111", + "meeting_user_ids": [1, 2], + "meeting_ids": [1, 4], + }, + ) + self.assert_model_exists( + "meeting_user/1", {"user_id": 2, "group_ids": [2], "meeting_id": 1} ) self.assert_model_exists( - "meeting_user/1", {"user_id": 2, "group_ids": [1], "meeting_id": 1} + "group/2", {"meeting_user_ids": [1], "external_id": "Delegates"} + ) + self.assert_model_exists( + "structure_level/1", + {"meeting_user_ids": [1], "name": "structure1"}, + ) + self.assert_model_exists( + "structure_level/2", + {"meeting_user_ids": [2], "name": "structure2"}, + ) + + def test_create_user_mapping_one_meeting_twice(self) -> None: + self.meeting_mappers[1]["external_id"] = "Landtag" + self.meeting_mappers[1]["mappings"]["groups"] = [ # type: ignore + { + "attribute": "use_default", + "default": "Default", + } + ] + self.set_models({"organization/1": self.organization}) + response = self.request( + "user.save_saml_account", + { + "username": ["111"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "idp_kv_group_attribute": "Delegates", + "kv_structure": "structure2", + "kv_presence": "True", + }, ) + self.assert_status_code(response, 200) self.assert_model_exists( - "group/1", + "user/2", { + "saml_id": "111", + "username": "111", "meeting_user_ids": [1], - "external_id": "Default", - "default_group_for_meeting_id": 1, + "meeting_ids": [1], }, ) + self.assert_model_exists( + "meeting_user/1", {"user_id": 2, "group_ids": [1, 2], "meeting_id": 1} + ) + self.assert_model_exists( + "group/1", {"meeting_user_ids": [1], "external_id": "Default"} + ) + self.assert_model_exists( + "group/2", {"meeting_user_ids": [1], "external_id": "Delegates"} + ) + self.assert_model_exists( + "structure_level/1", + {"meeting_user_ids": [1], "name": "structure1"}, + ) + self.assert_model_exists( + "structure_level/2", + {"meeting_user_ids": [1], "name": "structure2"}, + ) - def test_update_user_existing_member_in_group(self) -> None: - """user created and logged in""" + def test_update_user_with_default_membership(self) -> None: + """ + Shows: + * deleting all conditions of a single mapper defaults this mapper to true + * updating without any group data in saml payload and not existing group in default inserts user in meetings default group + * updating without group data in saml payload but existing group in default works + """ + del self.meeting_mappers[0]["conditions"] + self.meeting_mappers[1]["conditions"] = [ + {"attribute": "yes", "condition": ".*"} + ] + self.meeting_mappers[1]["mappings"]["groups"][0]["default"] = "Delegates" # type: ignore + self.set_models({"organization/1": self.organization}) + response = self.request( + "user.save_saml_account", {"username": ["admin_saml"], "yes": "to_all"} + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/1", + { + "saml_id": "admin_saml", + "username": "admin", + "meeting_user_ids": [1, 2], + "meeting_ids": [1, 4], + }, + ) + self.assert_model_exists( + "meeting_user/1", {"user_id": 1, "group_ids": [1], "meeting_id": 1} + ) + self.assert_model_exists( + "meeting_user/2", {"user_id": 1, "group_ids": [5], "meeting_id": 4} + ) + self.assert_model_exists( + "group/1", {"meeting_user_ids": [1], "external_id": "Default"} + ) + self.assert_model_exists( + "group/5", {"meeting_user_ids": [2], "external_id": "Delegates"} + ) + + def test_update_user_participant_already_in_group(self) -> None: + """ + Shows: + * user stays in group 2 + * structure_level/1 left untouched structure_level/2 added + * second mapper is ignored due to being an update and allow_update being false + """ + self.meeting_mappers[1]["allow_update"] = "False" + self.set_models({"organization/1": self.organization}) self.set_user_groups(1, [2]) - response = self.request("user.save_saml_account", {"username": ["admin_saml"]}) + self.set_models( + { + "structure_level/1": { + "name": "structure1", + "meeting_user_ids": [1], + "meeting_id": 1, + } + } + ) + self.update_model("meeting_user/1", {"structure_level_ids": [1]}) + response = self.request( + "user.save_saml_account", + { + "username": ["admin_saml"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "participant_number": "MG_1254", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_email": "hols@holz.de", + "participant_kv_number": "MG_1254", + "kv_group_attribute": "Delegates", + "kv_structure": "structure2", + "structure": "structure2", + }, + ) self.assert_status_code(response, 200) self.assert_model_exists( "user/1", @@ -644,29 +1090,57 @@ def test_update_user_existing_member_in_group(self) -> None: }, ) self.assert_model_exists( - "meeting_user/1", {"user_id": 1, "group_ids": [2], "meeting_id": 1} + "meeting_user/1", + { + "user_id": 1, + "group_ids": [2], + "meeting_id": 1, + "structure_level_ids": [1, 2], + }, ) + self.assert_model_not_exists("meeting_user/2") self.assert_model_exists( "group/2", {"meeting_user_ids": [1], "external_id": "Delegates"} ) + self.assert_model_exists( + "structure_level/1", {"name": "structure1", "meeting_user_ids": [1]} + ) + self.assert_model_exists( + "structure_level/2", {"name": "structure2", "meeting_user_ids": [1]} + ) + self.assert_model_not_exists("structure_level/3") def test_update_user_add_group_to_existing_groups(self) -> None: """group added, user created and logged in""" self.set_user_groups(1, [1, 3]) - response = self.request("user.save_saml_account", {"username": ["admin_saml"]}) + response = self.request( + "user.save_saml_account", + { + "username": ["admin_saml"], + "member_number": "LV_Königholz", + "email": "holzi@holz.de", + "idp_group_attribute": "Delegates", + "kv_member_number": "KV_Könighols", + "kv_group_attribute": "Delegates", + "kv_structure": "structure2", + }, + ) self.assert_status_code(response, 200) self.assert_model_exists( "user/1", { "saml_id": "admin_saml", "username": "admin", - "meeting_user_ids": [1], - "meeting_ids": [1], + "meeting_user_ids": [1, 2], + "meeting_ids": [1, 4], }, ) self.assert_model_exists( "meeting_user/1", {"user_id": 1, "group_ids": [1, 3, 2], "meeting_id": 1} ) + self.assert_model_exists( + "meeting_user/2", {"user_id": 1, "group_ids": [5], "meeting_id": 4} + ) self.assert_model_exists( "group/2", {"meeting_user_ids": [1], "external_id": "Delegates"} ) diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index 2e3ace2e9c..0b1b33e3a6 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -153,7 +153,7 @@ def test_update_with_meeting_user_fields(self) -> None: self.assert_history_information( "user/22", [ - "Participant added to meeting {}", + "Participant added to meeting {}.", "meeting/1", "Committee management changed", ], @@ -2354,7 +2354,7 @@ def test_update_participant_data_with_existing_meetings(self) -> None: self.assert_history_information( "user/222", [ - "Participant added to meeting {}", + "Participant added to meeting {}.", "meeting/2", ], ) @@ -2395,9 +2395,9 @@ def test_update_participant_data_in_multiple_meetings_with_existing_meetings( self.assert_history_information( "user/222", [ - "Participant added to meeting {}", + "Participant added to meeting {}.", "meeting/2", - "Participant added to meeting {}", + "Participant added to meeting {}.", "meeting/3", ], ) From c13b26f64d1e94ad2f1f3688b4130a08e76ca9b7 Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Mon, 25 Nov 2024 16:38:09 +0100 Subject: [PATCH 11/51] Allow meeting admin user to update a non admin user that shares all his meetings with requesting user. (#2576) * Allow meeting admin user to update a non admin user that shares all his meetings with requesting admin user. * Use user.can_update and user.can_manage. * Implement get_user_editable presenter with payload field names to support all payload field groups. --------- Co-authored-by: Elblinator <69210919+Elblinator@users.noreply.github.com> Co-authored-by: luisa-beerboom <101706784+luisa-beerboom@users.noreply.github.com> --- docs/Presenters-Overview.md | 1 + docs/actions/user.set_password.md | 1 - docs/presenters/get_user_editable.md | 31 ++ docs/presenters/get_user_related_models.md | 4 +- docs/presenters/get_user_scope.md | 11 +- .../action/actions/user/create.py | 5 +- .../action/actions/user/participant_common.py | 9 +- .../action/actions/user/update.py | 5 +- .../action/actions/user/user_mixins.py | 4 + .../action/mixins/import_mixins.py | 2 +- openslides_backend/presenter/__init__.py | 1 + openslides_backend/presenter/base.py | 1 + .../presenter/get_user_editable.py | 85 +++ .../presenter/get_user_scope.py | 5 +- openslides_backend/shared/exceptions.py | 25 +- .../user_create_update_permissions_mixin.py} | 146 ++--- .../shared/mixins/user_scope_mixin.py | 201 ++++++- .../action/user/scope_permissions_mixin.py | 28 +- tests/system/action/user/test_delete.py | 4 +- .../action/user/test_generate_new_password.py | 4 +- .../user/test_participant_json_upload.py | 1 - .../user/test_reset_password_to_default.py | 4 +- tests/system/action/user/test_set_password.py | 65 ++- tests/system/action/user/test_update.py | 498 ++++++++++++++++- tests/system/presenter/base.py | 71 +++ .../presenter/test_get_user_editable.py | 515 ++++++++++++++++++ .../presenter/test_get_user_related_models.py | 56 ++ 27 files changed, 1671 insertions(+), 112 deletions(-) create mode 100644 docs/presenters/get_user_editable.md create mode 100644 openslides_backend/presenter/get_user_editable.py rename openslides_backend/{action/actions/user/create_update_permissions_mixin.py => shared/mixins/user_create_update_permissions_mixin.py} (83%) create mode 100644 tests/system/presenter/test_get_user_editable.py diff --git a/docs/Presenters-Overview.md b/docs/Presenters-Overview.md index 5ffc1db58a..58ef16d284 100644 --- a/docs/Presenters-Overview.md +++ b/docs/Presenters-Overview.md @@ -6,6 +6,7 @@ Available presenters: - [get_forwarding_meetings](presenters/get_forwarding_meetings.md) - [get_meetings](presenters/get_meetings.md) - [get_users](presenters/get_users.md) +- [get_user_editable](presenters/get_user_editable.md) - [get_user_related_models](presenters/get_user_related_models.md) - [get_user_scope](presenters/get_user_scope.md) - [search_deleted_models](presenters/search_deleted_models.md) diff --git a/docs/actions/user.set_password.md b/docs/actions/user.set_password.md index af0a4e856d..cc64952402 100644 --- a/docs/actions/user.set_password.md +++ b/docs/actions/user.set_password.md @@ -9,7 +9,6 @@ set_as_default: boolean; // default false, if not given } ``` - ## Action Sets the password of the user given by `id` to `password`. If `set_as_default` is true, the `default_password` is also updated. diff --git a/docs/presenters/get_user_editable.md b/docs/presenters/get_user_editable.md new file mode 100644 index 0000000000..480e5bafb2 --- /dev/null +++ b/docs/presenters/get_user_editable.md @@ -0,0 +1,31 @@ +## Payload + +```js +{ + user_ids: Id[], // required + fields: string[] // required +} +``` + +## Returns + +```js +{ + user_id: Id: { + field: str: ( + editable: boolean, // true if user can be updated or deleted, + message?: string // error message if an exception was caught + ), + ... + }, + ... +} +``` + +## Logic + +It iterates over the given `user_ids` and calculates whether a user can be updated depending on the given payload fields, permissions in shared committees and meetings, OML and the user-scope. The user scope is defined [here](https://github.com/OpenSlides/OpenSlides/wiki/Users#user-scopes). The payload field permissions are described [here](https://github.com/OpenSlides/openslides-backend/blob/main/docs/actions/user.update.md) and [here](https://github.com/OpenSlides/openslides-backend/blob/main/docs/actions/user.create.md). + +## Permissions + +There are no special permissions necessary. \ No newline at end of file diff --git a/docs/presenters/get_user_related_models.md b/docs/presenters/get_user_related_models.md index 6f7a9808de..b5e057be54 100644 --- a/docs/presenters/get_user_related_models.md +++ b/docs/presenters/get_user_related_models.md @@ -1,6 +1,6 @@ ## Payload -```js +``` { user_ids: Id[] // required } @@ -8,7 +8,7 @@ ## Returns -```js +``` { [user_id: Id]: { organization_management_level: OML-String, diff --git a/docs/presenters/get_user_scope.md b/docs/presenters/get_user_scope.md index b4b4eca3a6..0119f79b2e 100644 --- a/docs/presenters/get_user_scope.md +++ b/docs/presenters/get_user_scope.md @@ -1,6 +1,6 @@ ## Payload -```js +``` { user_ids: Id[] // required } @@ -8,14 +8,15 @@ ## Returns -```js +``` { user_id: Id: { collection: String, # one of "meeting", "committee" or "organization" id: Id, - user_oml: String, # one of "superadmin", "can_manage_organization", "can_manage_users", "" - committee_ids: int[] // Ids of all committees the user is part of - } + user_oml: String, # one of "superadmin", "can_manage_organization", "can_manage_users", "" + committee_ids: Id[] // Ids of all committees the user is part of + }, + ... } ``` diff --git a/openslides_backend/action/actions/user/create.py b/openslides_backend/action/actions/user/create.py index 49494ec5f1..e4eef4b38b 100644 --- a/openslides_backend/action/actions/user/create.py +++ b/openslides_backend/action/actions/user/create.py @@ -2,6 +2,9 @@ from typing import Any from openslides_backend.permissions.permissions import Permissions +from openslides_backend.shared.mixins.user_create_update_permissions_mixin import ( + CreateUpdatePermissionsMixin, +) from ....models.models import User from ....shared.exceptions import ActionException @@ -15,13 +18,13 @@ from ...util.register import register_action from ...util.typing import ActionResultElement from ..meeting_user.mixin import CheckLockOutPermissionMixin -from .create_update_permissions_mixin import CreateUpdatePermissionsMixin from .password_mixins import SetPasswordMixin from .user_mixins import LimitOfUserMixin, UserMixin, UsernameMixin, check_gender_exists @register_action("user.create") class UserCreate( + UserMixin, EmailCheckMixin, CreateAction, CreateUpdatePermissionsMixin, diff --git a/openslides_backend/action/actions/user/participant_common.py b/openslides_backend/action/actions/user/participant_common.py index 2069cc94c0..54f11e63e4 100644 --- a/openslides_backend/action/actions/user/participant_common.py +++ b/openslides_backend/action/actions/user/participant_common.py @@ -11,14 +11,14 @@ ) from openslides_backend.permissions.permissions import Permissions from openslides_backend.shared.exceptions import MissingPermission +from openslides_backend.shared.mixins.user_create_update_permissions_mixin import ( + CreateUpdatePermissionsFailingFields, + PermissionVarStore, +) from openslides_backend.shared.patterns import fqid_from_collection_and_id from ....shared.filters import And, FilterOperator, Or from ..meeting_user.mixin import CheckLockOutPermissionMixin -from ..user.create_update_permissions_mixin import ( - CreateUpdatePermissionsFailingFields, - PermissionVarStore, -) class ParticipantCommon(BaseImportJsonUploadAction, CheckLockOutPermissionMixin): @@ -51,6 +51,7 @@ def check_permissions(self, instance: dict[str, Any]) -> None: ) self.permission_check = CreateUpdatePermissionsFailingFields( + self.user_id, permstore, self.services, self.datastore, diff --git a/openslides_backend/action/actions/user/update.py b/openslides_backend/action/actions/user/update.py index a5df918baa..d3fe226832 100644 --- a/openslides_backend/action/actions/user/update.py +++ b/openslides_backend/action/actions/user/update.py @@ -2,6 +2,9 @@ from typing import Any from openslides_backend.permissions.permissions import Permissions +from openslides_backend.shared.mixins.user_create_update_permissions_mixin import ( + CreateUpdatePermissionsMixin, +) from ....action.action import original_instances from ....action.util.typing import ActionData @@ -17,7 +20,6 @@ from ...util.register import register_action from ..meeting_user.mixin import CheckLockOutPermissionMixin from .conditional_speaker_cascade_mixin import ConditionalSpeakerCascadeMixin -from .create_update_permissions_mixin import CreateUpdatePermissionsMixin from .user_mixins import ( AdminIntegrityCheckMixin, LimitOfUserMixin, @@ -29,6 +31,7 @@ @register_action("user.update") class UserUpdate( + UserMixin, EmailCheckMixin, CreateUpdatePermissionsMixin, UpdateAction, diff --git a/openslides_backend/action/actions/user/user_mixins.py b/openslides_backend/action/actions/user/user_mixins.py index 9f5c99f899..807e00fb91 100644 --- a/openslides_backend/action/actions/user/user_mixins.py +++ b/openslides_backend/action/actions/user/user_mixins.py @@ -91,6 +91,10 @@ class UserMixin(CheckForArchivedMeetingMixin): "locked_out": {"type": "boolean"}, } + def check_permissions(self, instance: dict[str, Any]) -> None: + self.assert_not_anonymous() + super().check_permissions(instance) + def validate_instance(self, instance: dict[str, Any]) -> None: super().validate_instance(instance) if "meeting_id" not in instance and any( diff --git a/openslides_backend/action/mixins/import_mixins.py b/openslides_backend/action/mixins/import_mixins.py index 87df99e143..dfe4e68b7b 100644 --- a/openslides_backend/action/mixins/import_mixins.py +++ b/openslides_backend/action/mixins/import_mixins.py @@ -317,7 +317,7 @@ def flatten_copied_object_fields( ) -> list[ImportRow]: """The self.rows will be deepcopied, flattened and returned, without changes on the self.rows. - This is necessary for using the data in the executution of actions. + This is necessary for using the data in the execution of actions. The requests response should be given with the unchanged self.rows. Parameter: hook_method: diff --git a/openslides_backend/presenter/__init__.py b/openslides_backend/presenter/__init__.py index 432c547183..f06f93cb07 100644 --- a/openslides_backend/presenter/__init__.py +++ b/openslides_backend/presenter/__init__.py @@ -7,6 +7,7 @@ get_forwarding_meetings, get_history_information, get_mediafile_context, + get_user_editable, get_user_related_models, get_user_scope, get_users, diff --git a/openslides_backend/presenter/base.py b/openslides_backend/presenter/base.py index 72e44e971e..81a0ff7994 100644 --- a/openslides_backend/presenter/base.py +++ b/openslides_backend/presenter/base.py @@ -17,6 +17,7 @@ class BasePresenter(BaseServiceProvider): Base class for presenters. """ + internal: bool = False data: Any schema: Callable[[Any], None] | None = None diff --git a/openslides_backend/presenter/get_user_editable.py b/openslides_backend/presenter/get_user_editable.py new file mode 100644 index 0000000000..7104da3190 --- /dev/null +++ b/openslides_backend/presenter/get_user_editable.py @@ -0,0 +1,85 @@ +from collections import defaultdict +from typing import Any + +import fastjsonschema + +from openslides_backend.permissions.permissions import Permissions +from openslides_backend.shared.exceptions import ( + ActionException, + MissingPermission, + PermissionDenied, + PresenterException, +) +from openslides_backend.shared.mixins.user_create_update_permissions_mixin import ( + CreateUpdatePermissionsMixin, +) +from openslides_backend.shared.schema import id_list_schema, str_list_schema + +from ..shared.schema import schema_version +from .base import BasePresenter +from .presenter import register_presenter + +get_user_editable_schema = fastjsonschema.compile( + { + "$schema": schema_version, + "type": "object", + "title": "get_user_editable", + "description": "get user editable", + "properties": { + "user_ids": id_list_schema, + "fields": str_list_schema, + }, + "required": ["user_ids", "fields"], + "additionalProperties": False, + } +) + + +@register_presenter("get_user_editable") +class GetUserEditable(CreateUpdatePermissionsMixin, BasePresenter): + """ + Checks for each given user whether the given fields are editable by calling user on a per payload group basis. + """ + + schema = get_user_editable_schema + name = "get_user_editable" + permission = Permissions.User.CAN_MANAGE + + def get_result(self) -> Any: + if not self.data["fields"]: + raise PresenterException( + "Need at least one field name to check editability." + ) + reversed_field_rights = { + field: group + for group, fields in self.field_rights.items() + for field in fields + } + one_field_per_group = { + group_fields[0] + for field_name in self.data["fields"] + for group_fields in self.field_rights.values() + if field_name in group_fields + } + result: defaultdict[str, dict[str, tuple[bool, str]]] = defaultdict(dict) + for user_id in self.data["user_ids"]: + result[str(user_id)] = {} + groups_editable = {} + for field_name in one_field_per_group: + try: + self.check_permissions({"id": user_id, field_name: None}) + groups_editable[reversed_field_rights[field_name]] = (True, "") + except (PermissionDenied, MissingPermission, ActionException) as e: + groups_editable[reversed_field_rights[field_name]] = ( + False, + e.message, + ) + result[str(user_id)].update( + { + data_field_name: groups_editable[ + reversed_field_rights[data_field_name] + ] + for data_field_name in self.data["fields"] + } + ) + return result diff --git a/openslides_backend/presenter/get_user_scope.py b/openslides_backend/presenter/get_user_scope.py index 8c07f485a9..fa609bad92 100644 --- a/openslides_backend/presenter/get_user_scope.py +++ b/openslides_backend/presenter/get_user_scope.py @@ -36,7 +36,10 @@ def get_result(self) -> Any: result: dict[str, Any] = {} user_ids = self.data["user_ids"] for user_id in user_ids: - scope, scope_id, user_oml, committee_ids = self.get_user_scope(user_id) + scope, scope_id, user_oml, committee_meeting_ids = self.get_user_scope( + user_id + ) + committee_ids = [ci for ci in committee_meeting_ids.keys()] result[str(user_id)] = { "collection": scope, "id": scope_id, diff --git a/openslides_backend/shared/exceptions.py b/openslides_backend/shared/exceptions.py index 59404c29a6..94ea1811d6 100644 --- a/openslides_backend/shared/exceptions.py +++ b/openslides_backend/shared/exceptions.py @@ -131,16 +131,29 @@ def __init__(self, action_name: str) -> None: class MissingPermission(PermissionDenied): def __init__( self, - permissions: AnyPermission | dict[AnyPermission, int], + permissions: AnyPermission | dict[AnyPermission, int | set[int]], ) -> None: if isinstance(permissions, dict): - self.message = ( - "Missing permission" + ("s" if len(permissions) > 1 else "") + ": " - ) + to_remove = [] + for permission, id_or_ids in permissions.items(): + if isinstance(id_or_ids, set) and not id_or_ids: + to_remove.append(permission) + for permission in to_remove: + del permissions[permission] + self.message = "Missing permission" + self._plural_s(permissions) + ": " self.message += " or ".join( - f"{permission.get_verbose_type()} {permission} in {permission.get_base_model()} {id}" - for permission, id in permissions.items() + f"{permission.get_verbose_type()} {permission} in {permission.get_base_model()}{self._plural_s(id_or_ids)} {id_or_ids}" + for permission, id_or_ids in permissions.items() ) else: self.message = f"Missing {permissions.get_verbose_type()}: {permissions}" super().__init__(self.message) + + def _plural_s(self, permission_or_id_or_ids: dict | int | set[int]) -> str: + if ( + isinstance(permission_or_id_or_ids, set) + or (isinstance(permission_or_id_or_ids, dict)) + ) and len(permission_or_id_or_ids) > 1: + return "s" + else: + return "" diff --git a/openslides_backend/action/actions/user/create_update_permissions_mixin.py b/openslides_backend/shared/mixins/user_create_update_permissions_mixin.py similarity index 83% rename from openslides_backend/action/actions/user/create_update_permissions_mixin.py rename to openslides_backend/shared/mixins/user_create_update_permissions_mixin.py index 4a64e86a96..a7d0874c3e 100644 --- a/openslides_backend/action/actions/user/create_update_permissions_mixin.py +++ b/openslides_backend/shared/mixins/user_create_update_permissions_mixin.py @@ -3,7 +3,6 @@ from functools import reduce from typing import Any, cast -from openslides_backend.action.action import Action from openslides_backend.action.relations.relation_manager import RelationManager from openslides_backend.permissions.base_classes import Permission from openslides_backend.permissions.management_levels import ( @@ -13,8 +12,10 @@ from openslides_backend.permissions.permissions import Permissions, permission_parents from openslides_backend.services.datastore.commands import GetManyRequest from openslides_backend.services.datastore.interface import DatastoreService +from openslides_backend.shared.base_service_provider import BaseServiceProvider from openslides_backend.shared.exceptions import ( ActionException, + AnyPermission, MissingPermission, PermissionDenied, ) @@ -24,8 +25,6 @@ from openslides_backend.shared.mixins.user_scope_mixin import UserScope, UserScopeMixin from openslides_backend.shared.patterns import fqid_from_collection_and_id -from .user_mixins import UserMixin - class PermissionVarStore: permission: Permission @@ -171,9 +170,10 @@ def _get_user_meetings_with_permission( return user_meetings -class CreateUpdatePermissionsMixin(UserMixin, UserScopeMixin, Action): +class CreateUpdatePermissionsMixin(UserScopeMixin, BaseServiceProvider): permstore: PermissionVarStore permission: Permission + internal: bool field_rights: dict[str, list] = { "A": [ @@ -203,7 +203,7 @@ class CreateUpdatePermissionsMixin(UserMixin, UserScopeMixin, Action): "is_present", # participant import ], "C": ["meeting_id", "group_ids"], - "D": ["committee_ids", "committee_management_ids"], + "D": ["committee_management_ids"], "E": ["organization_management_level"], "F": ["default_password"], "G": ["is_demo_user"], @@ -213,10 +213,12 @@ class CreateUpdatePermissionsMixin(UserMixin, UserScopeMixin, Action): def check_permissions(self, instance: dict[str, Any]) -> None: """ Checks the permissions on a per field and user.scope base, details see - https://github.com/OpenSlides/OpenSlides/wiki/user.update or user.create + https://github.com/OpenSlides/OpenSlides/wiki/Users + https://github.com/OpenSlides/OpenSlides/wiki/Permission-System + https://github.com/OpenSlides/OpenSlides/wiki/Restrictions-Overview The fields groups and their necessary permissions are also documented there. + Returns true if permissions are given. """ - self.assert_not_anonymous() if "forwarding_committee_ids" in instance: raise PermissionDenied("forwarding_committee_ids is not allowed.") @@ -227,12 +229,12 @@ def check_permissions(self, instance: dict[str, Any]) -> None: ) actual_group_fields = self._get_actual_grouping_from_instance(instance) - # store scope, id and OML-permission for requested user + # store scope, scope id, OML-permission and committee ids including the the respective meetings for requested user ( self.instance_user_scope, self.instance_user_scope_id, self.instance_user_oml_permission, - self.instance_committee_ids, + self.instance_committee_meeting_ids, ) = self.get_user_scope(instance.get("id") or instance) if self.permstore.user_oml != OrganizationManagementLevel.SUPERADMIN: @@ -247,40 +249,38 @@ def check_permissions(self, instance: dict[str, Any]) -> None: lock_result=False, ).get("locked_from_inside", False) - # Ordered by supposed velocity advantages. Changing order can only effect the sequence of detected errors for tests + # Ordered by supposed speed advantages. Changing order can only effect the sequence of detected errors for tests self.check_group_H(actual_group_fields["H"]) self.check_group_E(actual_group_fields["E"], instance) self.check_group_D(actual_group_fields["D"], instance) self.check_group_C(actual_group_fields["C"], instance, locked_from_inside) self.check_group_B(actual_group_fields["B"], instance, locked_from_inside) - self.check_group_A(actual_group_fields["A"]) - self.check_group_F(actual_group_fields["F"]) + self.check_group_A(actual_group_fields["A"], instance) + self.check_group_F(actual_group_fields["F"], instance) self.check_group_G(actual_group_fields["G"]) - def check_group_A( - self, - fields: list[str], - ) -> None: + def check_group_A(self, fields: list[str], instance: dict[str, Any]) -> None: """Check Group A: Depending on scope of user to act on""" if ( - self.permstore.user_oml == OrganizationManagementLevel.SUPERADMIN - or not fields + not fields or self.permstore.user_oml >= OrganizationManagementLevel.CAN_MANAGE_USERS ): return + missing_permissions: dict[AnyPermission, int | set[int]] = dict() if self.instance_user_scope == UserScope.Organization: - if self.permstore.user_committees.intersection(self.instance_committee_ids): - return - raise MissingPermission({OrganizationManagementLevel.CAN_MANAGE_USERS: 1}) - if self.instance_user_scope == UserScope.Committee: - if self.instance_user_scope_id not in self.permstore.user_committees: - raise MissingPermission( - { - OrganizationManagementLevel.CAN_MANAGE_USERS: 1, - CommitteeManagementLevel.CAN_MANAGE: self.instance_user_scope_id, - } + if not ( + self.permstore.user_committees.intersection( + self.instance_committee_meeting_ids ) + ): + missing_permissions = {OrganizationManagementLevel.CAN_MANAGE_USERS: 1} + elif self.instance_user_scope == UserScope.Committee: + if self.instance_user_scope_id not in self.permstore.user_committees: + missing_permissions = { + OrganizationManagementLevel.CAN_MANAGE_USERS: 1, + CommitteeManagementLevel.CAN_MANAGE: self.instance_user_scope_id, + } elif ( self.instance_user_scope_id not in self.permstore.user_committees_meetings and self.instance_user_scope_id not in self.permstore.user_meetings @@ -290,13 +290,26 @@ def check_group_A( ["committee_id"], lock_result=False, ) - raise MissingPermission( + missing_permissions = { + OrganizationManagementLevel.CAN_MANAGE_USERS: 1, + CommitteeManagementLevel.CAN_MANAGE: meeting["committee_id"], + self.permission: self.instance_user_scope_id, + } + if missing_permissions and not self.check_for_admin_in_all_meetings( + instance.get("id", 0) + ): + missing_permissions.update( { - OrganizationManagementLevel.CAN_MANAGE_USERS: 1, - CommitteeManagementLevel.CAN_MANAGE: meeting["committee_id"], - self.permission: self.instance_user_scope_id, + Permissions.User.CAN_UPDATE: { + meeting_id + for meeting_ids in self.instance_committee_meeting_ids.values() + if meeting_ids is not None + for meeting_id in meeting_ids + if meeting_id is not None + }, } ) + raise MissingPermission(missing_permissions) def check_group_B( self, fields: list[str], instance: dict[str, Any], locked_from_inside: bool @@ -360,10 +373,7 @@ def check_group_E(self, fields: list[str], instance: dict[str, Any]) -> None: f"Your organization management level is not high enough to set a Level of {instance.get('organization_management_level', OrganizationManagementLevel.CAN_MANAGE_USERS.get_verbose_type())}." ) - def check_group_F( - self, - fields: list[str], - ) -> None: + def check_group_F(self, fields: list[str], instance: dict[str, Any]) -> None: """Check F common fields: scoped permissions necessary, but if instance user has an oml-permission, that of the request user must be higher""" if ( @@ -372,6 +382,7 @@ def check_group_F( ): return + missing_permissions: dict[AnyPermission, int | set[int]] = dict() if ( self.instance_user_oml_permission or self.instance_user_scope == UserScope.Organization @@ -382,25 +393,22 @@ def check_group_F( ) else: if self.permstore.user_committees.intersection( - self.instance_committee_ids + self.instance_committee_meeting_ids ): return expected_oml_permission = OrganizationManagementLevel.CAN_MANAGE_USERS if expected_oml_permission > self.permstore.user_oml: - raise MissingPermission({expected_oml_permission: 1}) + missing_permissions = {expected_oml_permission: 1} else: return - else: - if self.permstore.user_oml >= OrganizationManagementLevel.CAN_MANAGE_USERS: - return - if self.instance_user_scope == UserScope.Committee: + elif self.permstore.user_oml >= OrganizationManagementLevel.CAN_MANAGE_USERS: + return + elif self.instance_user_scope == UserScope.Committee: if self.instance_user_scope_id not in self.permstore.user_committees: - raise MissingPermission( - { - OrganizationManagementLevel.CAN_MANAGE_USERS: 1, - CommitteeManagementLevel.CAN_MANAGE: self.instance_user_scope_id, - } - ) + missing_permissions = { + OrganizationManagementLevel.CAN_MANAGE_USERS: 1, + CommitteeManagementLevel.CAN_MANAGE: self.instance_user_scope_id, + } elif ( self.instance_user_scope_id not in self.permstore.user_committees_meetings and self.instance_user_scope_id not in self.permstore.user_meetings @@ -410,13 +418,26 @@ def check_group_F( ["committee_id"], lock_result=False, ) - raise MissingPermission( + missing_permissions = { + OrganizationManagementLevel.CAN_MANAGE_USERS: 1, + CommitteeManagementLevel.CAN_MANAGE: meeting["committee_id"], + self.permission: self.instance_user_scope_id, + } + if missing_permissions and not self.check_for_admin_in_all_meetings( + instance.get("id", 0) + ): + missing_permissions.update( { - OrganizationManagementLevel.CAN_MANAGE_USERS: 1, - CommitteeManagementLevel.CAN_MANAGE: meeting["committee_id"], - self.permission: self.instance_user_scope_id, + Permissions.User.CAN_UPDATE: { + meeting_id + for meeting_ids in self.instance_committee_meeting_ids.values() + if meeting_ids is not None + for meeting_id in meeting_ids + if meeting_id is not None + }, } ) + raise MissingPermission(missing_permissions) def check_group_G(self, fields: list[str]) -> None: """Group G: OML SUPERADMIN necessary""" @@ -475,8 +496,9 @@ def _get_actual_grouping_from_instance( """ Returns a dictionary with an entry for each field group A-E with a list of fields from payload instance. - The field groups A-F refer to https://github.com/OpenSlides/OpenSlides/wiki/user.create - or user.update + The field groups A-F refer to https://github.com/OpenSlides/openslides-meta/blob/main/models.yml + or https://github.com/OpenSlides/openslides-backend/blob/main/docs/actions/user.create.md + or https://github.com/OpenSlides/openslides-backend/blob/main/docs/actions/user.update.md """ act_grouping: dict[str, list[str]] = defaultdict(list) for key, _ in instance.items(): @@ -519,7 +541,9 @@ def _meetings_from_group_B_fields_from_instance( any other group B field. """ meetings: set[int] = set(instance.get("is_present_in_meeting_ids", [])) - meetings.add(cast(int, instance.get("meeting_id"))) + meeting_id = cast(int, instance.get("meeting_id")) + if meeting_id: + meetings.add(meeting_id) return meetings @@ -528,6 +552,7 @@ class CreateUpdatePermissionsFailingFields(CreateUpdatePermissionsMixin): def __init__( self, + user_id: int, permstore: PermissionVarStore, services: Services, datastore: DatastoreService, @@ -538,14 +563,11 @@ def __init__( use_meeting_ids_for_archived_meeting_check: bool | None = None, ) -> None: self.permstore = permstore + self.user_id = user_id super().__init__( services, datastore, - relation_manager, logging, - env, - skip_archived_meeting_check, - use_meeting_ids_for_archived_meeting_check, ) def get_failing_fields(self, instance: dict[str, Any]) -> list[str]: @@ -570,7 +592,7 @@ def get_failing_fields(self, instance: dict[str, Any]) -> list[str]: self.instance_user_scope, self.instance_user_scope_id, self.instance_user_oml_permission, - self.instance_committee_ids, + self.instance_committee_meeting_ids, ) = self.get_user_scope(instance.get("id") or instance) instance_meeting_id = instance.get("meeting_id") @@ -598,8 +620,8 @@ def get_failing_fields(self, instance: dict[str, Any]) -> list[str]: instance, locked_from_inside, ), - (self.check_group_A, actual_group_fields["A"], None, None), - (self.check_group_F, actual_group_fields["F"], None, None), + (self.check_group_A, actual_group_fields["A"], instance, None), + (self.check_group_F, actual_group_fields["F"], instance, None), (self.check_group_G, actual_group_fields["G"], None, None), ]: try: diff --git a/openslides_backend/shared/mixins/user_scope_mixin.py b/openslides_backend/shared/mixins/user_scope_mixin.py index b84b6050fa..47c22b5c3e 100644 --- a/openslides_backend/shared/mixins/user_scope_mixin.py +++ b/openslides_backend/shared/mixins/user_scope_mixin.py @@ -1,3 +1,4 @@ +from collections import defaultdict from enum import Enum from typing import Any @@ -29,21 +30,26 @@ def __repr__(self) -> str: class UserScopeMixin(BaseServiceProvider): + instance_committee_meeting_ids: dict + name: str + def get_user_scope( self, id_or_instance: int | dict[str, Any] - ) -> tuple[UserScope, int, str, list[int]]: + ) -> tuple[UserScope, int, str, dict[int, Any]]: """ Parameter id_or_instance: id for existing user or instance for user to create Returns the scope of the given user id together with the relevant scope id (either meeting, committee or organization), the OML level of the user as string (empty string if the user - has none) and the ids of all committees that the user is either a manager in or a member of. + has none) and the ids of all committees that the user is either a manager in or a member of + together with their respective meetings the user being part of. A committee can have no meetings if the + user just has committee management rights and is not part of any of its meetings. """ - meetings: set[int] = set() + meeting_ids: set[int] = set() committees_manager: set[int] = set() if isinstance(id_or_instance, dict): if "group_ids" in id_or_instance: if "meeting_id" in id_or_instance: - meetings.add(id_or_instance["meeting_id"]) + meeting_ids.add(id_or_instance["meeting_id"]) committees_manager.update( set(id_or_instance.get("committee_management_ids", [])) ) @@ -56,15 +62,16 @@ def get_user_scope( "organization_management_level", "committee_management_ids", ], + lock_result=False, ) - meetings.update(user.get("meeting_ids", [])) + meeting_ids.update(user.get("meeting_ids", [])) committees_manager.update(set(user.get("committee_management_ids") or [])) oml_right = user.get("organization_management_level", "") result = self.datastore.get_many( [ GetManyRequest( "meeting", - list(meetings), + list(meeting_ids), ["committee_id", "is_active_in_organization_id"], ) ] @@ -76,25 +83,32 @@ def get_user_scope( if meeting_data.get("is_active_in_organization_id") } committees = committees_manager | set(meetings_committee.values()) + committee_meetings: dict[int, Any] = defaultdict(list) + for meeting, committee in meetings_committee.items(): + committee_meetings[committee].append(meeting) + for committee in committees: + if committee not in committee_meetings.keys(): + committee_meetings[committee] = None + if len(meetings_committee) == 1 and len(committees) == 1: return ( UserScope.Meeting, next(iter(meetings_committee)), oml_right, - list(committees), + committee_meetings, ) elif len(committees) == 1: return ( UserScope.Committee, next(iter(committees)), oml_right, - list(committees), + committee_meetings, ) - return UserScope.Organization, 1, oml_right, list(committees) + return UserScope.Organization, 1, oml_right, committee_meetings def check_permissions_for_scope( self, - id: int, + instance_id: int, always_check_user_oml: bool = True, meeting_permission: Permission = Permissions.User.CAN_MANAGE, ) -> None: @@ -105,7 +119,9 @@ def check_permissions_for_scope( Reason: A user with OML-level-permission has scope "meeting" or "committee" if he belongs to only 1 meeting or 1 committee. """ - scope, scope_id, user_oml, committees = self.get_user_scope(id) + scope, scope_id, user_oml, committees_to_meetings = self.get_user_scope( + instance_id + ) if ( always_check_user_oml and user_oml @@ -160,7 +176,166 @@ def check_permissions_for_scope( self.datastore, self.user_id, CommitteeManagementLevel.CAN_MANAGE, - committees, + [ci for ci in committees_to_meetings.keys()], ): return - raise MissingPermission({OrganizationManagementLevel.CAN_MANAGE_USERS: 1}) + meeting_ids = { + meeting_id + for mids in committees_to_meetings.values() + for meeting_id in mids + } + if not meeting_ids or not self.check_for_admin_in_all_meetings( + instance_id, meeting_ids + ): + raise MissingPermission( + { + OrganizationManagementLevel.CAN_MANAGE_USERS: 1, + **{ + Permissions.User.CAN_UPDATE: meeting_id + for meeting_id in meeting_ids + }, + } + ) + + def check_for_admin_in_all_meetings( + self, + instance_id: int, + b_meeting_ids: set[int] | None = None, + ) -> bool: + """ + This function checks the special permission condition for scope request, user.update/create with + payload fields A and F and other user altering actions like user.delete or set_default_password. + This function returns true if: + * requested user is no committee manager and + * requested user doesn't have any admin/user.can_update/user.can_manage rights in his meetings and + * requesting user has those permissions in all of those meetings + """ + if not self._check_not_committee_manager(instance_id): + return False + + if not (meetings := self._get_meetings_if_subset(b_meeting_ids)): + return False + admin_meeting_users = self._collect_admin_meeting_users(meetings) + return self._analyze_meeting_admins(admin_meeting_users, meetings) + + def _check_not_committee_manager(self, instance_id: int) -> bool: + """ + Helper function used in method check_for_admin_in_all_meetings. + Checks that requested user is not a committee manager. + """ + if not (hasattr(self, "name") and self.name == "user.create"): + if self.datastore.get( + fqid_from_collection_and_id("user", instance_id), + ["committee_management_ids"], + lock_result=False, + use_changed_models=False, + ).get("committee_management_ids", []): + return False + return True + + def _get_meetings_if_subset(self, b_meeting_ids: set[int] | None) -> dict[int, Any]: + """ + Helper function used in method check_for_admin_in_all_meetings. + Gets the requested users meetings if these are subset of requesting user. Returns False if this is not possible. + """ + if not b_meeting_ids and not ( + b_meeting_ids := { + m_id + for m_ids in self.instance_committee_meeting_ids.values() + for m_id in m_ids + } + ): + return {} + if not ( + a_meeting_ids := set( + self.datastore.get( + fqid_from_collection_and_id("user", self.user_id), + ["meeting_ids"], + lock_result=False, + ).get("meeting_ids", []) + ) + ): + return {} + if not b_meeting_ids.issubset(a_meeting_ids): + return {} + return self.datastore.get_many( + [ + GetManyRequest( + "meeting", + list(b_meeting_ids), + ["admin_group_id", "group_ids"], + ) + ], + lock_result=False, + ).get("meeting", {}) + + def _collect_admin_meeting_users(self, meetings: dict[int, Any]) -> set[int]: + """ + Gets the admin groups and those groups with permission User.CAN_UPDATE and USER.CAN_MANAGE of the meetings. + Returns a set of the groups meeting_user_ids. + """ + group_ids = [ + group_id + for meeting_id, meeting_dict in meetings.items() + for group_id in meeting_dict.get("group_ids", []) + ] + return { + mu_id + for group_id, group in self.datastore.get_many( + [ + GetManyRequest( + "group", + group_ids, + [ + "meeting_user_ids", + "permissions", + "admin_group_for_meeting_id", + ], + ) + ], + lock_result=False, + ) + .get("group", {}) + .items() + if ( + group.get("admin_group_for_meeting_id") + or "user.can_update" in group.get("permissions", []) + or "user.can_manage" in group.get("permissions", []) + ) + for mu_id in group.get("meeting_user_ids", []) + } + + def _analyze_meeting_admins( + self, + admin_meeting_user_ids: set[int], + all_meetings: dict[int, Any], + ) -> bool: + """ + Helper function used in method check_for_admin_in_all_meetings. + Compares the users of admin meeting users of all meetings with the ids of requested user and requesting user. + Requesting user must be admin in all meetings. Requested user cannot be admin in any. + """ + meeting_id_to_admin_user_ids: dict[int, set[int]] = { + meeting_id: set() for meeting_id in all_meetings + } + for meeting_user in ( + self.datastore.get_many( + [ + GetManyRequest( + "meeting_user", + list(admin_meeting_user_ids), + ["user_id", "meeting_id"], + ) + ], + lock_result=False, + ) + .get("meeting_user", {}) + .values() + ): + meeting_id_to_admin_user_ids[meeting_user["meeting_id"]].add( + meeting_user["user_id"] + ) + return all( + self.user_id in admin_users + for admin_users in meeting_id_to_admin_user_ids.values() + ) diff --git a/tests/system/action/user/scope_permissions_mixin.py b/tests/system/action/user/scope_permissions_mixin.py index a2e78f506d..cbe016abcd 100644 --- a/tests/system/action/user/scope_permissions_mixin.py +++ b/tests/system/action/user/scope_permissions_mixin.py @@ -32,6 +32,26 @@ def setup_admin_scope_permissions( self.set_organization_management_level(None) self.set_user_groups(1, [3]) self.set_group_permissions(3, [meeting_permission]) + self.set_models( + { + "user/777": { + "username": "admin_group_filler", + "meeting_user_ids": [666, 667], + }, + "meeting_user/666": { + "group_ids": [12, 23], + "meeting_id": 1, + "user_id": 777, + }, + "meeting_user/667": { + "group_ids": [12, 23], + "meeting_id": 2, + "user_id": 777, + }, + "group/12": {"meeting_user_ids": [666]}, + "group/23": {"meeting_user_ids": [667]}, + } + ) def setup_scoped_user(self, scope: UserScope) -> None: """ @@ -45,13 +65,15 @@ def setup_scoped_user(self, scope: UserScope) -> None: "meeting/1": { "user_ids": [111], "committee_id": 1, - "group_ids": [11], + "group_ids": [11, 12], + "admin_group_id": 12, "is_active_in_organization_id": 1, }, "meeting/2": { "user_ids": [111], "committee_id": 2, - "group_ids": [22], + "group_ids": [22, 23], + "admin_group_id": 23, "is_active_in_organization_id": 1, }, "user/111": { @@ -70,7 +92,9 @@ def setup_scoped_user(self, scope: UserScope) -> None: "group_ids": [22], }, "group/11": {"meeting_id": 1, "meeting_user_ids": [11]}, + "group/12": {"meeting_id": 1, "meeting_user_ids": [666]}, "group/22": {"meeting_id": 2, "meeting_user_ids": [22]}, + "group/23": {"meeting_id": 2, "meeting_user_ids": [667]}, } ) elif scope == UserScope.Committee: diff --git a/tests/system/action/user/test_delete.py b/tests/system/action/user/test_delete.py index 890ff3a31e..250f212465 100644 --- a/tests/system/action/user/test_delete.py +++ b/tests/system/action/user/test_delete.py @@ -429,7 +429,7 @@ def test_delete_scope_organization_no_permission(self) -> None: response = self.request("user.delete", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.delete. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.delete. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) @@ -479,7 +479,7 @@ def test_delete_scope_organization_permission_in_meeting(self) -> None: response = self.request("user.delete", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.delete. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.delete. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) diff --git a/tests/system/action/user/test_generate_new_password.py b/tests/system/action/user/test_generate_new_password.py index 922b99fc6f..bae8c8fcc8 100644 --- a/tests/system/action/user/test_generate_new_password.py +++ b/tests/system/action/user/test_generate_new_password.py @@ -111,7 +111,7 @@ def test_scope_organization_no_permission(self) -> None: response = self.request("user.generate_new_password", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.generate_new_password. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.generate_new_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) @@ -165,7 +165,7 @@ def test_scope_organization_permission_in_meeting(self) -> None: response = self.request("user.generate_new_password", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.generate_new_password. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.generate_new_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) diff --git a/tests/system/action/user/test_participant_json_upload.py b/tests/system/action/user/test_participant_json_upload.py index 6005ffc1ad..e8c180b88d 100644 --- a/tests/system/action/user/test_participant_json_upload.py +++ b/tests/system/action/user/test_participant_json_upload.py @@ -9,7 +9,6 @@ class ParticipantJsonUpload(BaseActionTestCase): def setUp(self) -> None: - self.maxDiff = None super().setUp() self.set_models( { diff --git a/tests/system/action/user/test_reset_password_to_default.py b/tests/system/action/user/test_reset_password_to_default.py index dbd33d406d..491f017420 100644 --- a/tests/system/action/user/test_reset_password_to_default.py +++ b/tests/system/action/user/test_reset_password_to_default.py @@ -118,7 +118,7 @@ def test_scope_organization_no_permission(self) -> None: response = self.request("user.reset_password_to_default", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.reset_password_to_default. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.reset_password_to_default. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) @@ -172,7 +172,7 @@ def test_scope_organization_permission_in_meeting(self) -> None: response = self.request("user.reset_password_to_default", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.reset_password_to_default. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.reset_password_to_default. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) diff --git a/tests/system/action/user/test_set_password.py b/tests/system/action/user/test_set_password.py index 0ab5e700ad..e607d801e8 100644 --- a/tests/system/action/user/test_set_password.py +++ b/tests/system/action/user/test_set_password.py @@ -23,6 +23,67 @@ def test_update_correct(self) -> None: self.assert_history_information("user/2", ["Password changed"]) self.assert_logged_in() + def test_two_meetings(self) -> None: + self.create_meeting() + self.create_meeting(4) # meeting 4 + user_id = self.create_user("test", group_ids=[1]) + self.login(user_id) + self.set_models( + { + "user/111": {"password": "old_pw"}, + "meeting_user/666": { + "group_ids": [12, 23], + "meeting_id": 12, + "user_id": 1, + }, + } + ) + self.update_model( + "user/1", + {"meeting_user_ids": [666]}, + ) + # only to make sure every meeting has an admin at all times + self.set_user_groups(1, [2, 5]) + # Admin groups of meeting/1 for test user meeting/2 as normal user + self.set_user_groups(user_id, [2, 4]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.set_password", {"id": 111, "password": self.PASSWORD} + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.set_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 4", + response.json["message"], + ) + model = self.get_model("user/111") + assert "old_pw" == model.get("password", "") + # Admin groups of meeting/1 for test user + self.set_user_groups(user_id, [2]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.set_password", {"id": 111, "password": self.PASSWORD} + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.set_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 4", + response.json["message"], + ) + model = self.get_model("user/111") + assert "old_pw" == model.get("password", "") + # Admin groups of meeting/1 and meeting/4 for test user + self.set_user_groups(user_id, [2, 5]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.set_password", {"id": 111, "password": self.PASSWORD} + ) + self.assert_status_code(response, 200) + model = self.get_model("user/111") + assert self.auth.is_equal(self.PASSWORD, model.get("password", "")) + self.assert_history_information("user/111", ["Password changed"]) + def test_update_correct_default_case(self) -> None: self.update_model("user/1", {"password": "old_pw"}) response = self.request( @@ -143,7 +204,7 @@ def test_scope_organization_no_permission(self) -> None: ) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.set_password. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.set_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) @@ -205,7 +266,7 @@ def test_scope_organization_permission_in_meeting(self) -> None: ) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.set_password. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.set_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index 0b1b33e3a6..f91b7dd938 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -18,6 +18,136 @@ def permission_setup(self) -> None: } ) + def two_meetings_test_fail_ADEFGH( + self, committee_id: None | int = None, group_B_success: bool = False + ) -> None: + # test group A + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "pronoun": None, + }, + ) + # test group D + response = self.request( + "user.update", + { + "id": 111, + "committee_management_ids": [1, 2], + }, + ) + self.assert_status_code(response, 403) + if committee_id: + self.assertIn( + f"You are not allowed to perform action user.update. Missing permission: CommitteeManagementLevel can_manage in committee {committee_id}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "committee_management_ids": [committee_id], + }, + ) + else: + self.assertIn( + "You are not allowed to perform action user.update. Missing permission: CommitteeManagementLevel can_manage in committee ", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "committee_management_ids": None, + }, + ) + # test group E + response = self.request( + "user.update", + { + "id": 111, + "organization_management_level": OrganizationManagementLevel.CAN_MANAGE_USERS, + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "Your organization management level is not high enough to set a Level of can_manage_users.", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "organization_management_level": None, + }, + ) + # test group F + response = self.request( + "user.update", + { + "id": 111, + "default_password": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "default_password": None, + }, + ) + # test group G + response = self.request( + "user.update", + { + "id": 111, + "is_demo_user": True, + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing OrganizationManagementLevel: superadmin", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "is_demo_user": None, + }, + ) + # test group H + response = self.request( + "user.update", + { + "id": 111, + "saml_id": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 400) + self.assertIn( + "The field 'saml_id' can only be used in internal action calls", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "saml_id": None, + }, + ) + def test_update_correct(self) -> None: self.create_model( "user/111", @@ -843,11 +973,61 @@ def test_perm_group_A_cml_manage_user_archived_meeting_in_other_committee( self.assertCountEqual(user111["meeting_ids"], [1, 4]) def test_perm_group_A_meeting_manage_user(self) -> None: - """May update group A fields on meeting scope. User belongs to 1 meeting without being part of a committee""" + """ + May update group A fields on meeting scope. User belongs to 1 meeting without being part of a committee. + Testing various scenarios: + * both default group + * default group has user.can_update permission + * requesting user is in admin group + """ self.permission_setup() - self.set_user_groups(self.user_id, [2]) + self.set_user_groups(self.user_id, [1]) self.set_user_groups(111, [1]) + response = self.request( + "user.update", + { + "id": 111, + "username": "new_username", + "pronoun": "pronoun", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 60 or Permission user.can_update in meeting {1}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "username": "User111", + "pronoun": None, + "meeting_ids": [1], + "committee_ids": None, + }, + ) + + self.update_model("group/1", {"permissions": ["user.can_update"]}) + response = self.request( + "user.update", + { + "id": 111, + "username": "new_user", + "pronoun": "pro", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "username": "new_user", + "pronoun": "pro", + "meeting_ids": [1], + "committee_ids": None, + }, + ) + + self.set_user_groups(self.user_id, [2]) response = self.request( "user.update", { @@ -867,6 +1047,172 @@ def test_perm_group_A_meeting_manage_user(self) -> None: }, ) + def test_perm_group_A_belongs_to_same_meetings(self) -> None: + """May update group A fields on any scope as long as admin user Ann belongs to all meetings user Ben belongs to. See issue 2522.""" + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + # Admin groups of meeting/1 and meeting/4 for requesting user + self.set_user_groups(self.user_id, [1, 4]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + self.two_meetings_test_fail_ADEFGH() + # Admin group of meeting/1 and default group for meeting/4 for request user + self.set_user_groups(self.user_id, [2, 4]) + # 111 into both meetings (admin group for meeting/4) + self.set_user_groups(111, [1, 5]) + self.two_meetings_test_fail_ADEFGH() + # test group B and C + response = self.request( + "user.update", + {"id": 111, "number": "I'm not gonna get updated.", "meeting_id": 4}, + ) + self.assertIn( + "The user needs OrganizationManagementLevel.can_manage_users or CommitteeManagementLevel.can_manage for committee of following meeting or Permission user.can_update for meeting 4", + response.json["message"], + ) + self.assert_status_code(response, 403) + self.assert_model_exists( + "user/111", + { + "number": None, + }, + ) + # Admin groups of meeting/1 and meeting/4 for request user + self.set_user_groups(self.user_id, [2, 5]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm gonna get updated.", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "pronoun": "I'm gonna get updated.", + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_can_update(self) -> None: + """ + May update group A fields on any scope as long as requesting user has + user.can_update rights in requested users meetings. + Also makes sure being in multiple groups of a single meeting is no problem. + """ + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + self.update_model( + "group/6", + {"permissions": ["user.can_update"]}, + ) + # Admin group of meeting/1 and default group of meeting/4 for requesting user + self.set_user_groups(self.user_id, [2, 4]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + self.two_meetings_test_fail_ADEFGH() + # Admin groups of meeting/1 and meeting/4 (via group permission) for requesting user + self.set_user_groups(self.user_id, [2, 4, 6]) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm gonna get updated.", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "pronoun": "I'm gonna get updated.", + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_can_manage(self) -> None: + """ + May update group A fields on any scope as long as requesting user has + user.can_update rights in requested users meetings. + Also makes sure being in multiple groups of a single meeting is no problem. + """ + self.permission_setup() # meeting 1 + logged in requesting user + user 111 + self.create_meeting(4) # meeting 4 + self.update_model( + "group/6", + {"permissions": ["user.can_manage"]}, + ) + # Admin group of meeting/1 and default group of meeting/4 for requesting user + self.set_user_groups(self.user_id, [2, 4]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + self.two_meetings_test_fail_ADEFGH() + # Admin groups of meeting/1 and meeting/4 (via group permission) for requesting user + self.set_user_groups(self.user_id, [1, 2, 4, 6]) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm gonna get updated.", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "pronoun": "I'm gonna get updated.", + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_committee_admin(self) -> None: + """May not update group A fields on any scope as long as admin user Ann belongs + to all meetings user Ben belongs to but Ben is committee admin. See issue 2522. + """ + self.permission_setup() # meeting 1 + logged in requesting user + user 111 + self.create_meeting(4) # meeting 4 + # Admin groups of meeting/1 and meeting/4 for requesting user + self.set_user_groups(self.user_id, [2, 5]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + # 111 is committee admin + committee_id = 60 + self.set_committee_management_level([committee_id], 111) + self.two_meetings_test_fail_ADEFGH(committee_id) + # test group B and C + response = self.request( + "user.update", + {"id": 111, "number": "I'm not gonna get updated.", "meeting_id": 4}, + ) + self.assert_status_code(response, 200) + self.assertIn( + "Actions handled successfully", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "number": None, + }, + ) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "pronoun": None, + }, + ) + def test_perm_group_A_meeting_manage_user_archived_meeting(self) -> None: self.perm_group_A_meeting_manage_user_archived_meeting( Permissions.User.CAN_UPDATE @@ -929,7 +1275,7 @@ def test_perm_group_A_no_permission(self) -> None: ) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.update. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", response.json["message"], ) @@ -992,7 +1338,7 @@ def test_perm_group_F_default_password_for_superadmin_no_permission(self) -> Non ) def test_perm_group_F_cml_manage_user_with_two_committees(self) -> None: - """May update group A fields on committee scope. User belongs to 1 meeting in 1 committee""" + """May update group F fields on committee scope. User belongs to two meetings.""" self.permission_setup() self.create_meeting(4) self.set_committee_management_level([60], self.user_id) @@ -1014,6 +1360,150 @@ def test_perm_group_F_cml_manage_user_with_two_committees(self) -> None: }, ) + def test_perm_group_F_with_meeting_scope(self) -> None: + """ + Test user update with various scenarios (admin in different meeting and committee no interference) + * not in same meeting fails + * same meeting but requesting user not in admin or permission group fails + * same meeting requesting user with permission user.can_update works + * same meeting both admin works + * same meeting requesting user is committee admin works + """ + self.permission_setup() + self.create_meeting(4) + self.set_user_groups(111, [2]) + self.set_user_groups(self.user_id, [5]) + self.set_committee_management_level([63], self.user_id) + + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 60 or Permission user.can_update in meeting {1}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "default_password": None, + }, + ) + + self.set_user_groups(self.user_id, [1, 5]) + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 403) + self.assert_model_exists( + "user/111", + { + "default_password": None, + }, + ) + + self.update_model("group/1", {"permissions": ["user.can_update"]}) + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "default_password": "new_one", + }, + ) + + self.set_user_groups(self.user_id, [2, 5]) + response = self.request( + "user.update", + { + "id": 111, + "default_password": "newer_one", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "default_password": "newer_one", + }, + ) + + self.set_committee_management_level([60], self.user_id) + response = self.request( + "user.update", + { + "id": 111, + "default_password": "newest_one", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "default_password": "newest_one", + }, + ) + + def test_perm_group_F_with_two_meeting_across_committees(self) -> None: + """ + May not update group F fields unless requesting user has admin rights in + all of requested users meetings. Also requested user can be admin himself. + """ + self.permission_setup() + self.create_meeting(4) + self.set_user_groups(111, [1, 4]) + self.set_user_groups(self.user_id, [2, 4]) + + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "default_password": None, + }, + ) + # assert meeting admin can change normal user + self.set_user_groups(111, [1, 5]) + self.set_user_groups(self.user_id, [2, 5]) + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "default_password": "new_one", + }, + ) + def test_perm_group_B_user_can_update(self) -> None: """update group B fields for 2 meetings with simple user.can_update permissions""" self.permission_setup() diff --git a/tests/system/presenter/base.py b/tests/system/presenter/base.py index 286e68c396..4230f12090 100644 --- a/tests/system/presenter/base.py +++ b/tests/system/presenter/base.py @@ -1,3 +1,4 @@ +from collections import defaultdict from typing import Any from openslides_backend.http.application import OpenSlidesBackendWSGIApplication @@ -26,3 +27,73 @@ def request( if isinstance(response.json, list) and len(response.json) == 1: return (response.status_code, response.json[0]) return (response.status_code, response.json) + + def create_meeting_for_two_users( + self, user1: int, user2: int, base: int = 1 + ) -> None: + """ + Creates meeting with id 1, committee 60 and groups with ids 1, 2, 3 by default. + With base you can setup other meetings, but be cautious because of group-ids + The groups have no permissions and no users by default. + Uses usernumber to create meeting users with the concatenation of base and usernumber. + """ + committee_id = base + 59 + self.set_models( + { + f"meeting/{base}": { + "group_ids": [base, base + 1, base + 2], + "default_group_id": base, + "admin_group_id": base + 1, + "committee_id": committee_id, + "is_active_in_organization_id": 1, + }, + f"group/{base}": { + "meeting_id": base, + "default_group_for_meeting_id": base, + "name": f"group{base}", + }, + f"group/{base+1}": { + "meeting_id": base, + "admin_group_for_meeting_id": base, + "name": f"group{base+1}", + }, + f"group/{base+2}": { + "meeting_id": base, + "name": f"group{base+2}", + }, + f"committee/{committee_id}": { + "organization_id": 1, + "name": f"Commitee{committee_id}", + "meeting_ids": [base], + }, + "organization/1": { + "limit_of_meetings": 0, + "active_meeting_ids": [base], + "enable_electronic_voting": True, + }, + f"meeting_user/{base}{user1}": {"user_id": user1, "meeting_id": base}, + f"meeting_user/{base}{user2}": {"user_id": user2, "meeting_id": base}, + } + ) + + def move_user_to_group(self, meeting_user_to_groups: dict[int, Any]) -> None: + """ + Sets the users groups, returns the meeting_user_ids + Be careful as it does not reset previously set groups if the related meeting + users are not in meeting_user_to_groups. + """ + groups_to_meeting_user = defaultdict(list) + for meeting_user_id, group_id in meeting_user_to_groups.items(): + if group_id: + self.update_model( + f"meeting_user/{meeting_user_id}", {"group_ids": [group_id]} + ) + groups_to_meeting_user[group_id].append(meeting_user_id) + else: + self.update_model( + f"meeting_user/{meeting_user_id}", {"group_ids": None} + ) + for group_id, meeting_user_ids in groups_to_meeting_user.items(): + self.update_model( + f"group/{group_id}", {"meeting_user_ids": meeting_user_ids} + ) diff --git a/tests/system/presenter/test_get_user_editable.py b/tests/system/presenter/test_get_user_editable.py new file mode 100644 index 0000000000..a000997432 --- /dev/null +++ b/tests/system/presenter/test_get_user_editable.py @@ -0,0 +1,515 @@ +from openslides_backend.permissions.management_levels import OrganizationManagementLevel + +from .base import BasePresenterTestCase + + +class TestGetUSerEditable(BasePresenterTestCase): + def set_up(self) -> None: + self.create_model( + "user/111", + { + "username": "Helmhut", + "last_name": "Schmidt", + "is_active": True, + "password": self.auth.hash("Kohl"), + "default_password": "Kohl", + }, + ) + self.login(111) + self.set_models( + { + "meeting/1": { + "committee_id": 2, + "is_active_in_organization_id": 1, + }, + # archived meeting + "meeting/2": { + "committee_id": 2, + "is_active_in_organization_id": None, + "is_archived_in_organization_id": 1, + }, + "committee/1": {}, + "committee/2": {"meeting_ids": [1, 2]}, + "user/2": { + "username": "only_oml_level", + "organization_management_level": OrganizationManagementLevel.CAN_MANAGE_USERS, + }, + "user/3": { + "username": "only_cml_level", + "committee_management_ids": [1], + "meeting_ids": [], + }, + "user/4": { + "username": "cml_and_meeting", + "meeting_ids": [1], + "committee_management_ids": [2], + }, + "user/5": { + "username": "no_organization", + "meeting_ids": [], + }, + "user/6": { + "username": "oml_and_meeting", + "organization_management_level": OrganizationManagementLevel.SUPERADMIN, + "meeting_ids": [1], + }, + "user/7": { + "username": "meeting_and_archived_meeting", + "meeting_ids": [1, 2], + }, + } + ) + + def test_with_oml(self) -> None: + self.set_up() + self.update_model( + "user/111", + { + "organization_management_level": OrganizationManagementLevel.CAN_MANAGE_USERS, + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [2, 3, 4, 5, 6, 7], + "fields": ["first_name", "default_password"], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "2": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "3": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "4": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "5": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "6": { + "default_password": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "first_name": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + }, + "7": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + }, + ) + + def test_with_cml(self) -> None: + self.set_up() + self.update_model( + "user/111", + { + "committee_management_ids": [2], + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [2, 3, 4, 5, 6, 7], + "fields": ["first_name", "default_password"], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "2": { + "default_password": [ + False, + "Your organization management level is not high enough to change a user with a Level of can_manage_users!", + ], + "first_name": [ + False, + "Your organization management level is not high enough to change a user with a Level of can_manage_users!", + ], + }, + "3": { + "default_password": [ + False, + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 1", + ], + "first_name": [ + False, + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 1", + ], + }, + "4": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "5": { + "default_password": [ + False, + "Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + ], + "first_name": [ + False, + "Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + ], + }, + "6": { + "default_password": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "first_name": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + }, + "7": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + }, + ) + + def test_with_same_meeting(self) -> None: + """ + User 5 can be edited because he is only in meetings which User 111 is admin of. + User 7 can not be edited because he is in two of the same meetings but User 111 is not admin in all of them. + """ + self.set_up() + self.create_meeting_for_two_users(5, 111, 1) + self.create_meeting_for_two_users(5, 111, 4) + self.create_meeting_for_two_users(7, 111, 7) + self.create_meeting_for_two_users(7, 111, 10) + self.update_model("meeting/1", {"committee_id": 1}) + self.update_model("meeting/4", {"committee_id": 2}) + self.update_model("meeting/7", {"committee_id": 1}) + self.update_model("meeting/10", {"committee_id": 2}) + # User 111 is meeting admin in meeting 1, 4 and 7 but normal user in 10 + # User 5 is normal user in meeting 1 and 4 + # User 7 is normal user in meeting 7 and 10 + meeting_user_to_group = { + 1111: 2, + 4111: 5, + 15: 1, + 45: 4, + 7111: 8, + 10111: 10, + 77: 7, + 107: 10, + } + self.move_user_to_group(meeting_user_to_group) + self.update_model( + "user/5", + { + "meeting_user_ids": [ + 15, + 45, + ], + "meeting_ids": [1, 4], + }, + ) + self.update_model( + "user/7", + { + "meeting_user_ids": [77, 107], + "meeting_ids": [7, 10], + }, + ) + self.update_model( + "user/111", + { + "meeting_user_ids": [1111, 4111, 7111, 10111], + "meeting_ids": [1, 4, 7, 10], + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [5, 7], + "fields": ["first_name", "default_password"], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "5": {"default_password": [True, ""], "first_name": [True, ""]}, + "7": { + "default_password": [ + False, + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {10, 7}", + ], + "first_name": [ + False, + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {10, 7}", + ], + }, + }, + ) + + def test_with_same_meeting_can_update(self) -> None: + """ + User 5 can be edited because he is only in meetings which User 111 has can_manage of. + User 7 can be edited because he is only in meetings which User 111 has can_update of. + """ + self.set_up() + self.create_meeting_for_two_users(5, 111, 1) + self.create_meeting_for_two_users(5, 111, 4) + self.create_meeting_for_two_users(7, 111, 7) + self.create_meeting_for_two_users(7, 111, 10) + self.update_model("meeting/1", {"committee_id": 1}) + self.update_model("meeting/4", {"committee_id": 2}) + self.update_model("meeting/7", {"committee_id": 1}) + self.update_model("meeting/10", {"committee_id": 2}) + self.update_model("group/3", {"permissions": ["user.can_update"]}) + self.update_model("group/6", {"permissions": ["user.can_update"]}) + self.update_model("group/9", {"permissions": ["user.can_manage"]}) + self.update_model("group/12", {"permissions": ["user.can_manage"]}) + # User 111 has sufficient group rights in meeting 1, 4, 7 and 10 + # User 5 is normal user in meeting 1 and 4 + # User 7 is normal user in meeting 7 and 10 + meeting_user_to_group = { + 1111: 3, + 4111: 6, + 15: 1, + 45: 4, + 7111: 9, + 10111: 12, + 77: 7, + 107: 10, + } + self.move_user_to_group(meeting_user_to_group) + self.update_model( + "user/5", + { + "meeting_user_ids": [ + 15, + 45, + ], + "meeting_ids": [1, 4], + }, + ) + self.update_model( + "user/7", + { + "meeting_user_ids": [77, 107], + "meeting_ids": [7, 10], + }, + ) + self.update_model( + "user/111", + { + "meeting_user_ids": [1111, 4111, 7111, 10111], + "meeting_ids": [1, 4, 7, 10], + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [5, 7], + "fields": ["first_name", "default_password"], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "5": {"default_password": [True, ""], "first_name": [True, ""]}, + "7": {"default_password": [True, ""], "first_name": [True, ""]}, + }, + ) + + def test_with_all_payload_groups(self) -> None: + """ + Tests all user.create/update payload field groups. Especially the field 'saml_id'. + """ + self.set_up() + self.update_model( + "user/111", + { + "organization_management_level": OrganizationManagementLevel.CAN_MANAGE_USERS, + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [2, 3, 4, 5, 6, 7], + "fields": [ + "first_name", + "default_password", + "vote_weight", + "group_ids", + "committee_management_ids", + "organization_management_level", + "is_demo_user", + "saml_id", + ], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "2": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + "3": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + "4": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + "5": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + "6": { + "committee_management_ids": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "default_password": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "first_name": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "organization_management_level": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "saml_id": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "vote_weight": [True, ""], + }, + "7": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + }, + ) + + def test_payload_list_of_None(self) -> None: + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [None], + "fields": [ + "first_name", + "default_password", + ], + }, + ) + self.assertEqual(status_code, 400) + self.assertIn("data.user_ids[0] must be integer", data["message"]) + status_code, data = self.request( + "get_user_editable", {"user_ids": [1, 2], "fields": [None]} + ) + self.assertEqual(status_code, 400) + self.assertIn("data.fields[0] must be string", data["message"]) + + def test_payload_empty_list(self) -> None: + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [], + "fields": [ + "first_name", + "default_password", + ], + }, + ) + self.assertEqual(status_code, 200) + assert data == {} + status_code, data = self.request( + "get_user_editable", {"user_ids": [1, 2], "fields": []} + ) + self.assertEqual(status_code, 400) + assert data == { + "message": "Need at least one field name to check editability.", + "success": False, + } diff --git a/tests/system/presenter/test_get_user_related_models.py b/tests/system/presenter/test_get_user_related_models.py index b4c8846473..4fe09c80b5 100644 --- a/tests/system/presenter/test_get_user_related_models.py +++ b/tests/system/presenter/test_get_user_related_models.py @@ -137,6 +137,62 @@ def test_get_user_related_models_meeting(self) -> None: } } + def test_two_meetings(self) -> None: + user_id = 2 + self.set_models( + { + f"user/{user_id}": { + "username": "executor", + "default_password": "DEFAULT_PASSWORD", + "password": self.auth.hash("DEFAULT_PASSWORD"), + "is_active": True, + "meeting_ids": [1, 4], + }, + f"user/{111}": {"username": "untouchable", "meeting_ids": [1, 4]}, + } + ) + self.create_meeting_for_two_users(user_id, 111) + self.create_meeting_for_two_users(user_id, 111, 4) # meeting 4 + self.set_models( + { + "user/777": {"meeting_user_ids": [666]}, + "meeting_user/666": { + "meeting_id": 1, + "user_id": 777, + }, + } + ) + self.update_model("group/5", {"meeting_user_ids": [666]}) + self.login(user_id) + # Admin groups of meeting/1 for requesting user meeting/2 as normal user + # 111 into both meetings + # 777 additional admin for meeting/2 doesn't affect outcome + meeting_user_to_group = {12: 2, 42: 4, 1111: 1, 4111: 4, 666: 5} + self.move_user_to_group(meeting_user_to_group) + status_code, data = self.request( + "get_user_related_models", {"user_ids": [111, 777]} + ) + self.assertEqual(status_code, 403) + self.assertEqual( + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 4", + data["message"], + ) + # Admin groups of meeting/1 for requesting user + # 111 into both meetings + self.move_user_to_group({12: 2, 42: None, 1111: 1, 4111: 4}) + status_code, data = self.request("get_user_related_models", {"user_ids": [111]}) + self.assertEqual(status_code, 403) + self.assertEqual( + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 4", + data["message"], + ) + # Admin groups of meeting/1 and meeting/4 for requesting user + # 111 into both meetings + meeting_user_to_group = {12: 2, 42: 5, 1111: 1, 4111: 4} + self.move_user_to_group(meeting_user_to_group) + status_code, data = self.request("get_user_related_models", {"user_ids": [111]}) + self.assertEqual(status_code, 200) + def test_get_user_related_models_meetings_more_users(self) -> None: self.set_models( { From d0773328e88d6ac46198331e331521d52ddec36c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:57:21 +0000 Subject: [PATCH 12/51] Bump werkzeug from 3.0.4 to 3.1.3 in /requirements/partial (#2721) Bumps [werkzeug](https://github.com/pallets/werkzeug) from 3.0.4 to 3.1.3. - [Release notes](https://github.com/pallets/werkzeug/releases) - [Changelog](https://github.com/pallets/werkzeug/blob/main/CHANGES.rst) - [Commits](https://github.com/pallets/werkzeug/compare/3.0.4...3.1.3) --- updated-dependencies: - dependency-name: werkzeug dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements/partial/requirements_production.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/partial/requirements_production.txt b/requirements/partial/requirements_production.txt index 83ac300a93..9136f005f9 100644 --- a/requirements/partial/requirements_production.txt +++ b/requirements/partial/requirements_production.txt @@ -8,7 +8,7 @@ pypdf[crypto]==5.1.0 requests==2.32.3 roman==4.2 simplejson==3.19.3 -Werkzeug==3.0.4 +Werkzeug==3.1.3 python-magic==0.4.27 pygments==2.18.0 From 5e8f15358525c9e552425f9b412aee1df70d6e91 Mon Sep 17 00:00:00 2001 From: luisa-beerboom <101706784+luisa-beerboom@users.noreply.github.com> Date: Mon, 25 Nov 2024 17:29:29 +0100 Subject: [PATCH 13/51] Remove motion import (#2736) --- docs/actions/motion.import.md | 21 - docs/actions/motion.json_upload.md | 54 - .../action/actions/motion/__init__.py | 2 - .../action/actions/motion/import_.py | 635 --------- .../action/actions/motion/json_upload.py | 599 --------- tests/system/action/motion/test_import.py | 1161 ----------------- .../system/action/motion/test_json_upload.py | 974 -------------- 7 files changed, 3446 deletions(-) delete mode 100644 docs/actions/motion.import.md delete mode 100644 docs/actions/motion.json_upload.md delete mode 100644 openslides_backend/action/actions/motion/import_.py delete mode 100644 openslides_backend/action/actions/motion/json_upload.py delete mode 100644 tests/system/action/motion/test_import.py delete mode 100644 tests/system/action/motion/test_json_upload.py diff --git a/docs/actions/motion.import.md b/docs/actions/motion.import.md deleted file mode 100644 index d93d50c8ee..0000000000 --- a/docs/actions/motion.import.md +++ /dev/null @@ -1,21 +0,0 @@ -## Payload -```js -{ -// required - id: Id; // action worker id - import: boolean; -} -``` - - -## Action -If `import` is `True`, use the rows from the given action worker and check that the import type -matches and whether it should still be created (row state `new`) or update (row state `done`). -On row state `new`, the username must not exist yet. On row state `done`, -the record with the matching `id` should still have the same username. On error, don't import anything, -but create data as in json_upload. Do the actual import with bulk actions. - -If `import` is `False` or the import was successful, remove the action worker. - -## Permission -The request user needs permission `motion.can_manage`, but only allow importing data if there are no errors in preview. \ No newline at end of file diff --git a/docs/actions/motion.json_upload.md b/docs/actions/motion.json_upload.md deleted file mode 100644 index eae90a8ac2..0000000000 --- a/docs/actions/motion.json_upload.md +++ /dev/null @@ -1,54 +0,0 @@ -## Payload - -Because the data fields are all converted from CSV import file, **they are all of type `string`**. -The types noted below are the internal types after conversion in the backend. See [here](preface_special_imports.md#internal-types) for the representation of the types. -```js -{ - // required for new motions - data: { - // required in create - title: string, // info: done, error - text: string, // info: done, error - // all optional, but see rules below - number: string, // unique when set, info: done, generated or error - reason: string, // required for create if the meeting has "motions_reason_required", info: done or error - submitters_verbose: string[], - submitters_username: string[], // info: done, generated, warning, error if len(submitters_verbose) > len(submitters_username) - supporters_verbose: string[], - supporters_username: string[], // info: done, warning, error if len(supporters_verbose) > len(supporters_username) - category_name: string, // info: done or warning, partial reference to: motion_category - category_prefix: string, - tags: string[], // info: done or warning, reference to: tag - block: string, // info: done or warning, reference to: motion_block - motion_amendment: boolean, // info: done or warning, if True, warning, that motion amendments cannot be imported - }[]; - meeting_id: Id, // id of the current meeting. -} -``` -## Return value and object fields - -Besides the usual headers as seen in payload (name and type), there are these differences: - -- `submitters`, `supporter_users`, `category_name/prefix`, `tags` and `block`: Objects that show if the model has been found (`done`) or not (`warning`). -- `text`: will be surrounded in html `

` tags if the string isn't encased in html tags already. -- `number`: will be object with error, if the field is set and another row in the payload has the same number. If the `number` field is left empty and the motion is going to be created in a `motion_state` where `set_number` is true, a new `number` will be generated and the object is going to have the info `generated`. - -The row state can be one of "new", "done" or "error". In case of an error, no import should be possible. - -See [common description](preface_special_imports.md#general-format-of-the-result-send-to-the-client-for-preview). - -Other than the validity check for the username-fields, `submitters_verbose` and `supporters_verbose` are NOT otherwise used or taken note of in the import. They are merely accepted in order to check that someone didn't accidentally edit the wrong column in a file that has both verbose and non-verbose columns. -They are not included in the return value. - - -## Action -The data will create or update motions. - -### Motion matching - -Motions can be updated via their `number`. -If a motion has a `number`, it will be matched with and updated with the data of any import date that has the same `number`. -Therefore motions that don't have a number can not be overwritten. - -## Permission -Permission `motion.can_manage` \ No newline at end of file diff --git a/openslides_backend/action/actions/motion/__init__.py b/openslides_backend/action/actions/motion/__init__.py index f4b731bfe1..9f504ec2a3 100644 --- a/openslides_backend/action/actions/motion/__init__.py +++ b/openslides_backend/action/actions/motion/__init__.py @@ -3,8 +3,6 @@ create_forwarded, delete, follow_recommendation, - import_, - json_upload, reset_recommendation, reset_state, set_recommendation, diff --git a/openslides_backend/action/actions/motion/import_.py b/openslides_backend/action/actions/motion/import_.py deleted file mode 100644 index 3e17e5d6c6..0000000000 --- a/openslides_backend/action/actions/motion/import_.py +++ /dev/null @@ -1,635 +0,0 @@ -from typing import Any, cast - -from openslides_backend.action.mixins.import_mixins import ( - BaseImportAction, - ImportRow, - ImportState, - Lookup, - ResultType, -) -from openslides_backend.action.util.register import register_action -from openslides_backend.permissions.permissions import Permissions -from openslides_backend.shared.exceptions import ActionException -from openslides_backend.shared.filters import And, Filter, FilterOperator, Or - -from ....models.models import ImportPreview -from ....shared.patterns import fqid_from_collection_and_id -from ....shared.schema import required_id_schema -from ...util.default_schema import DefaultSchema -from ..meeting_user.create import MeetingUserCreate -from ..motion_submitter.create import MotionSubmitterCreateAction -from ..motion_submitter.delete import MotionSubmitterDeleteAction -from ..motion_submitter.sort import MotionSubmitterSort -from .create import MotionCreate -from .payload_validation_mixin import ( - MotionActionErrorData, - MotionCreatePayloadValidationMixin, - MotionErrorType, - MotionUpdatePayloadValidationMixin, -) -from .update import MotionUpdate - - -@register_action("motion.import") -class MotionImport( - BaseImportAction, - MotionCreatePayloadValidationMixin, - MotionUpdatePayloadValidationMixin, -): - """ - Action to import a result from the import_preview. - """ - - model = ImportPreview() - schema = DefaultSchema(ImportPreview()).get_default_schema( - additional_required_fields={ - "id": required_id_schema, - "import": {"type": "boolean"}, - } - ) - permission = Permissions.Motion.CAN_MANAGE - skip_archived_meeting_check = True - import_name = "motion" - number_lookup: Lookup - username_lookup: dict[str, list[dict[str, Any]]] - category_lookup: dict[str, list[dict[str, Any]]] - tags_lookup: dict[str, list[dict[str, Any]]] - block_lookup: dict[str, list[dict[str, Any]]] - _user_ids_to_meeting_user: dict[int, Any] - _submitter_ids_to_user_id: dict[int, int] - - def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: - if not instance["import"]: - return {} - - instance = super().update_instance(instance) - meeting_id = self.get_meeting_id(instance) - self.setup_lookups(meeting_id) - - self.rows = [self.validate_entry(row) for row in self.result["rows"]] - - if self.import_state != ImportState.ERROR: - create_action_payload: list[dict[str, Any]] = [] - update_action_payload: list[dict[str, Any]] = [] - submitter_create_action_payload: list[dict[str, Any]] = [] - submitter_delete_action_payload: list[dict[str, Any]] = [] - - motion_to_submitter_user_ids: dict[int, list[int]] = {} - old_submitters: dict[int, dict[int, int]] = ( - {} - ) # {motion_id: {user_id:submitter_id}} - for row in self.rows: - payload: dict[str, Any] = row["data"].copy() - used_list = ["text", "reason", "title", "number"] - for field in used_list: - if field in payload: - if type(dvalue := payload[field]) is dict: - payload[field] = dvalue["value"] - self.remove_fields_from_data( - payload, - ["submitters_verbose", "supporters_verbose", "motion_amendment"], - ) - if (category := payload.pop("category_name", None)) and category[ - "info" - ] == ImportState.DONE: - payload["category_id"] = ( - category["id"] if category.get("id") else None - ) - if (block := payload.pop("block", None)) and block[ - "info" - ] == ImportState.DONE: - payload["block_id"] = block["id"] if block.get("id") else None - payload["tag_ids"] = self.get_ids_from_object_list( - payload.pop("tags", []) - ) - meeting_users_to_create = [ - {"user_id": submitter["id"], "meeting_id": meeting_id} - for submitter in payload["submitters_username"] - if submitter["info"] == ImportState.GENERATED - and submitter["id"] not in self._user_ids_to_meeting_user.keys() - ] - if len(meeting_users_to_create): - meeting_users = cast( - list[dict[str, int]], - self.execute_other_action( - MeetingUserCreate, meeting_users_to_create - ), - ) - for i in range(len(meeting_users)): - self._user_ids_to_meeting_user[ - meeting_users_to_create[i]["user_id"] - ] = meeting_users[i] - submitters = self.get_ids_from_object_list( - payload.pop("submitters_username") - ) - supporters = [ - self._user_ids_to_meeting_user[supporter_id]["id"] - for supporter_id in self.get_ids_from_object_list( - payload.pop("supporters_username", []) - ) - ] - payload["supporter_meeting_user_ids"] = supporters - payload.pop("category_prefix", None) - errors: list[MotionActionErrorData] = [] - if row["state"] == ImportState.NEW: - payload.update({"submitter_ids": submitters}) - create_action_payload.append(payload) - errors = self.get_create_payload_integrity_error_message( - payload, meeting_id - ) - else: - id_ = payload["id"] - motion_to_submitter_user_ids[id_] = submitters - motion = { - k: v - for k, v in ( - [ - motion - for motion in self.number_lookup.name_to_ids.get( - payload["number"], [] - ) - if motion.get("id") - ][0] - ).items() - } - for field in ["category_id", "block_id"]: - if payload.get(field) is None: - if not motion.get(field): - payload.pop(field, None) - if len(submitters): - motion_submitter_ids: list[int] = ( - motion.get("submitter_ids", []) or [] - ) - matched_submitters = { - self._submitter_ids_to_user_id[submitter_id]: submitter_id - for submitter_id in motion_submitter_ids - if self._submitter_ids_to_user_id.get(submitter_id) - in submitters - } - submitter_create_action_payload.extend( - [ - { - "meeting_user_id": self._user_ids_to_meeting_user[ - user_id - ]["id"], - "motion_id": id_, - } - for user_id in submitters - if user_id not in matched_submitters.keys() - ] - ) - submitter_delete_action_payload.extend( - [ - {"id": submitter_id} - for submitter_id in motion_submitter_ids - if submitter_id not in matched_submitters.values() - ] - ) - old_submitters[id_] = matched_submitters - - payload.pop("meeting_id", None) - update_action_payload.append(payload) - errors = self.get_update_payload_integrity_error_message( - payload, meeting_id - ) - for err in errors: - if err["type"] != MotionErrorType.REASON: - raise ActionException("Error: " + err["message"]) - if not ( - row["data"].get("reason") - and isinstance(row["data"]["reason"], dict) - ): - row["data"]["reason"] = { - "value": row["data"].get("reason", ""), - "info": ImportState.ERROR, - } - else: - row["data"]["reason"]["info"] = ImportState.ERROR - row["data"]["reason"].pop("id", 0) - row["messages"].append("Error: " + err["message"]) - row["state"] = ImportState.ERROR - self.import_state = ImportState.ERROR - if self.import_state != ImportState.ERROR: - created_submitters: list[dict[str, int]] = [] - if create_action_payload: - self.execute_other_action(MotionCreate, create_action_payload) - if update_action_payload: - self.execute_other_action(MotionUpdate, update_action_payload) - if len(submitter_create_action_payload): - created_submitters = cast( - list[dict[str, int]], - self.execute_other_action( - MotionSubmitterCreateAction, submitter_create_action_payload - ), - ) - if len(submitter_delete_action_payload): - self.execute_other_action( - MotionSubmitterDeleteAction, submitter_delete_action_payload - ) - new_submitters: dict[int, dict[int, int]] = ( - {} - ) # {motion_id: {meeting_user_id:submitter_id}} - for i in range(len(created_submitters)): - motion_id = submitter_create_action_payload[i]["motion_id"] - new_submitters[motion_id] = { - **new_submitters.get(motion_id, {}), - submitter_create_action_payload[i][ - "meeting_user_id" - ]: created_submitters[i]["id"], - } - sort_payload: list[dict[str, Any]] = [] - for motion_id in motion_to_submitter_user_ids: - sorted_motion_submitter_ids: list[int] = [] - for submitter_user_id in motion_to_submitter_user_ids[motion_id]: - meeting_user_id = cast( - int, self._user_ids_to_meeting_user[submitter_user_id]["id"] - ) - if ( - submitter_user_id - in old_submitters.get(motion_id, {}).keys() - ): - sorted_motion_submitter_ids.append( - old_submitters[motion_id][submitter_user_id] - ) - elif ( - meeting_user_id in new_submitters.get(motion_id, {}).keys() - ): - sorted_motion_submitter_ids.append( - new_submitters[motion_id][meeting_user_id] - ) - else: - raise Exception( - f"Submitter sorting failed due to submitter for user/{submitter_user_id} not being found" - ) - if len(sorted_motion_submitter_ids): - sort_payload.append( - { - "motion_id": motion_id, - "motion_submitter_ids": sorted_motion_submitter_ids, - } - ) - for payload in sort_payload: - self.execute_other_action(MotionSubmitterSort, [payload]) - - return {} - - def get_ids_from_object_list(self, object_list: list[dict[str, Any]]) -> list[int]: - return [ - obj["id"] - for obj in object_list - if obj.get("info") != ImportState.WARNING - and obj.get("info") != ImportState.ERROR - ] - - def remove_fields_from_data( - self, data: dict[str, Any], fieldnames: list[str] - ) -> None: - for fieldname in fieldnames: - data.pop(fieldname, None) - - def validate_entry(self, row: ImportRow) -> ImportRow: - entry = row["data"] - - if ("id" in entry) != ("id" in entry.get("number", {})): - raise ActionException( - f"Invalid JsonUpload data: A data row with state '{ImportState.DONE}' must have an 'id'" - ) - - number = self.get_value_from_union_str_object(entry.get("number")) - if number: - check_result = self.number_lookup.check_duplicate(number) - id_ = cast(int, self.number_lookup.get_field_by_name(number, "id")) - - if check_result == ResultType.FOUND_ID and id_ != 0: - if row["state"] != ImportState.DONE: - row["messages"].append( - f"Error: Row state expected to be '{ImportState.DONE}', but it is '{row['state']}'." - ) - row["state"] = ImportState.ERROR - entry["number"]["info"] = ImportState.ERROR - elif entry["id"] != id_: - row["state"] = ImportState.ERROR - entry["number"]["info"] = ImportState.ERROR - row["messages"].append( - f"Error: Number '{number}' found in different id ({id_} instead of {entry['id']})" - ) - elif check_result == ResultType.FOUND_MORE_IDS: - row["state"] = ImportState.ERROR - entry["number"]["info"] = ImportState.ERROR - row["messages"].append( - f"Error: Number '{number}' is duplicated in import." - ) - elif check_result == ResultType.NOT_FOUND_ANYMORE: - row["messages"].append( - f"Error: Motion {entry['number']['id']} not found anymore for updating motion '{number}'." - ) - row["state"] = ImportState.ERROR - - category_name = self.get_value_from_union_str_object(entry.get("category_name")) - if category_name and entry["category_name"].get("info") == ImportState.DONE: - category_prefix = entry.get("category_prefix") or None - if "id" not in entry["category_name"]: - raise ActionException( - f"Invalid JsonUpload data: A category_name entry with state '{ImportState.DONE}' must have an 'id'" - ) - categories = self.category_lookup.get(category_name, []) - categories = [ - category - for category in categories - if category.get("prefix") == category_prefix - ] - if len(categories) > 0: - if not any( - [ - category.get("id") == entry["category_name"].get("id") - for category in categories - ] - ): - row["messages"].append( - "Error: Category search didn't deliver the same result as in the preview" - ) - entry["category_name"] = { - "value": category_name, - "info": ImportState.ERROR, - } - row["state"] = ImportState.ERROR - else: - entry["category_name"] = { - "value": category_name, - "info": ImportState.ERROR, - } - row["state"] = ImportState.ERROR - row["messages"].append("Error: Category could not be found anymore") - - block = self.get_value_from_union_str_object(entry.get("block")) - if block and entry["block"].get("info") == ImportState.DONE: - if "id" not in entry["block"]: - raise ActionException( - f"Invalid JsonUpload data: A block entry with state '{ImportState.DONE}' must have an 'id'" - ) - found_blocks = self.block_lookup.get(block, []) - if len(found_blocks) > 0: - if not any( - [block.get("id") == entry["block"]["id"] for block in found_blocks] - ): - entry["block"] = {"value": block, "info": ImportState.ERROR} - row["messages"].append( - "Error: Motion block search didn't deliver the same result as in the preview" - ) - row["state"] = ImportState.ERROR - else: - entry["block"] = { - "value": block, - "info": ImportState.ERROR, - } - row["messages"].append("Error: Couldn't find motion block anymore") - row["state"] = ImportState.ERROR - - if isinstance(entry.get("tags"), list): - different: list[str] = [] - not_found: list[str] = [] - for tag_entry in entry.get("tags", []): - tag = self.get_value_from_union_str_object(tag_entry) - if tag and tag_entry.get("info") == ImportState.DONE: - if "id" not in tag_entry: - raise ActionException( - f"Invalid JsonUpload data: A tag entry with state '{ImportState.DONE}' must have an 'id'" - ) - found_tags = self.tags_lookup.get(tag, []) - if len(found_tags) > 0: - if not any( - [tag.get("id") == tag_entry["id"] for tag in found_tags] - ): - tag_entry["info"] = ImportState.ERROR - tag_entry.pop("id") - different.append(tag) - else: - tag_entry["info"] = ImportState.ERROR - tag_entry.pop("id") - not_found.append(tag) - if len(different): - row["messages"].append( - "Error: Tag search didn't deliver the same result as in the preview: " - + ", ".join(different) - ) - row["state"] = ImportState.ERROR - if len(not_found): - row["messages"].append( - "Error: Couldn't find tag anymore: " + ", ".join(not_found) - ) - row["state"] = ImportState.ERROR - - for fieldname in ["submitter", "supporter"]: - if isinstance(entry.get(f"{fieldname}s_username"), list): - different = [] - not_found = [] - for user_entry in entry.get(f"{fieldname}s_username", []): - user = self.get_value_from_union_str_object(user_entry) - if user and ( - user_entry.get("info") == ImportState.DONE - or user_entry.get("info") == ImportState.GENERATED - ): - if "id" not in user_entry: - raise ActionException( - f"Invalid JsonUpload data: A {fieldname} entry with state '{ImportState.DONE}' or '{ImportState.GENERATED}' must have an 'id'" - ) - found_users = self.username_lookup.get(user, []) - if len(found_users) == 1: - if found_users[0].get("id") != user_entry["id"]: - user_entry["info"] = ImportState.ERROR - user_entry.pop("id") - different.append(user) - elif len(found_users) > 1: - raise ActionException( - f"Database corrupt: Found multiple users with the username {user}." - ) - else: - user_entry["info"] = ImportState.ERROR - user_entry.pop("id") - not_found.append(user) - if len(different): - row["messages"].append( - f"Error: {fieldname[0].capitalize() + fieldname[1:]} search didn't deliver the same result as in the preview: " - + ", ".join(different) - ) - row["state"] = ImportState.ERROR - if len(not_found): - row["messages"].append( - f"Error: Couldn't find {fieldname} anymore: " - + ", ".join(not_found) - ) - row["state"] = ImportState.ERROR - - row["messages"] = list(set(row["messages"])) - - if row["state"] == ImportState.ERROR and self.import_state == ImportState.DONE: - self.import_state = ImportState.ERROR - return { - "state": row["state"], - "data": row["data"], - "messages": row.get("messages", []), - } - - def setup_lookups(self, meeting_id: int) -> None: - rows = self.result["rows"] - self.number_lookup = Lookup( - self.datastore, - "motion", - [ - (entry["number"]["value"], entry) - for row in rows - if "number" in (entry := row["data"]) - and entry["number"].get("info") != ImportState.WARNING - ], - field="number", - mapped_fields=["submitter_ids", "category_id", "block_id"], - global_and_filter=FilterOperator("meeting_id", "=", meeting_id), - ) - self.block_lookup = self.get_lookup_dict( - "motion_block", - [ - entry["block"]["value"] - for row in rows - if "block" in (entry := row["data"]) - and entry["block"].get("info") != ImportState.WARNING - ], - "title", - and_filters=[FilterOperator("meeting_id", "=", meeting_id)], - ) - self.category_lookup = self.get_lookup_dict( - "motion_category", - [ - entry["category_name"]["value"] - for row in rows - if "category_name" in (entry := row["data"]) - and entry["category_name"].get("info") != ImportState.WARNING - ], - "name", - ["prefix"], - and_filters=[FilterOperator("meeting_id", "=", meeting_id)], - ) - - self.username_lookup = self.get_lookup_dict( - "user", - list( - { - user["value"] - for row in rows - if ( - users := [ - *row["data"].get("submitters_username", []), - *row["data"].get("supporters_username", []), - ] - ) - for user in users - if user and user.get("info") != ImportState.WARNING - } - ), - "username", - ["meeting_ids", "meeting_user_ids"], - ) - self.username_lookup = { - username: [ - date - for date in self.username_lookup[username] - if ( - date.get("meeting_ids") - and (meeting_id in date["meeting_ids"]) - or date.get("id") == self.user_id - ) - ] - for username in self.username_lookup - } - all_user_ids = list( - [ - submitter["id"] - for submitters in self.username_lookup.values() - for submitter in submitters - ] - ) - all_meeting_users: dict[int, dict[str, Any]] = {} - if len(all_user_ids): - all_meeting_users = self.datastore.filter( - "meeting_user", - And( - FilterOperator("meeting_id", "=", meeting_id), - Or( - *[ - FilterOperator("user_id", "=", user_id) - for user_id in all_user_ids - ] - ), - ), - [ - "id", - "user_id", - "motion_submitter_ids", - "supported_motion_ids", - ], - lock_result=False, - ) - self._user_ids_to_meeting_user = { - all_meeting_users[meeting_user_id]["user_id"]: all_meeting_users[ - meeting_user_id - ] - for meeting_user_id in all_meeting_users - if all_meeting_users[meeting_user_id].get("user_id") - } - self._submitter_ids_to_user_id = { - submitter_id: all_meeting_users[meeting_user_id]["user_id"] - for meeting_user_id in all_meeting_users - for submitter_id in ( - all_meeting_users[meeting_user_id].get("motion_submitter_ids", []) or [] - ) - if all_meeting_users[meeting_user_id].get("user_id") - } - self.tags_lookup = self.get_lookup_dict( - "tag", - [ - tag["value"] - for row in rows - if "tags" in (entry := row["data"]) - for tag in entry["tags"] - if tag.get("info") != ImportState.WARNING - ], - "name", - and_filters=[FilterOperator("meeting_id", "=", meeting_id)], - ) - - def get_lookup_dict( - self, - collection: str, - entries: list[str], - fieldname: str = "name", - mapped_fields: list[str] = [], - and_filters: list[Filter] = [], - ) -> dict[str, list[dict[str, Any]]]: - lookup: dict[str, list[dict[str, Any]]] = {} - if len(entries): - data = self.datastore.filter( - collection, - And( - *and_filters, - Or([FilterOperator(fieldname, "=", name) for name in set(entries)]), - ), - [*mapped_fields, "id", fieldname], - lock_result=False, - ) - for date_id in data: - date = data[date_id] - lookup[date[fieldname]] = [ - *lookup.get(date[fieldname], []), - date, - ] - return lookup - - def get_meeting_id(self, instance: dict[str, Any]) -> int: - store_id = instance["id"] - worker = self.datastore.get( - fqid_from_collection_and_id("import_preview", store_id), - ["name", "result"], - lock_result=False, - ) - if worker.get("name") == self.import_name: - return next(iter(worker.get("result", {})["rows"]))["data"]["meeting_id"] - raise ActionException("Import data cannot be found.") diff --git a/openslides_backend/action/actions/motion/json_upload.py b/openslides_backend/action/actions/motion/json_upload.py deleted file mode 100644 index fcb5741a41..0000000000 --- a/openslides_backend/action/actions/motion/json_upload.py +++ /dev/null @@ -1,599 +0,0 @@ -from collections import defaultdict -from collections.abc import Iterable -from re import search, sub -from typing import Any, cast - -from openslides_backend.shared.filters import And, Filter, FilterOperator, Or - -from ....models.models import Motion -from ....permissions.permissions import Permissions -from ....shared.exceptions import ActionException -from ....shared.schema import required_id_schema -from ...mixins.import_mixins import ( - BaseJsonUploadAction, - ImportState, - Lookup, - ResultType, -) -from ...util.default_schema import DefaultSchema -from ...util.register import register_action -from .payload_validation_mixin import ( - MotionActionErrorData, - MotionCreatePayloadValidationMixin, - MotionErrorType, - MotionUpdatePayloadValidationMixin, -) - -LIST_TYPE = { - "anyOf": [ - { - "type": "array", - "items": {"type": "string"}, - }, - {"type": "string"}, - ] -} - - -@register_action("motion.json_upload") -class MotionJsonUpload( - BaseJsonUploadAction, - MotionCreatePayloadValidationMixin, - MotionUpdatePayloadValidationMixin, -): - """ - Action to allow to upload a json. It is used as first step of an import. - """ - - model = Motion() - schema = DefaultSchema(Motion()).get_default_schema( - additional_required_fields={ - "data": { - "type": "array", - "items": { - "type": "object", - "properties": { - **model.get_properties( - "title", - "text", - "number", - "reason", - ), - **{ - "submitters_verbose": LIST_TYPE, - "submitters_username": LIST_TYPE, - "supporters_verbose": LIST_TYPE, - "supporters_username": LIST_TYPE, - "category_name": {"type": "string"}, - "category_prefix": {"type": "string"}, - "tags": LIST_TYPE, - "block": {"type": "string"}, - "motion_amendment": {"type": "boolean"}, - }, - }, - "required": [], - "additionalProperties": False, - }, - "minItems": 1, - "uniqueItems": False, - }, - "meeting_id": required_id_schema, - } - ) - - headers = [ - {"property": "title", "type": "string", "is_object": True}, - {"property": "text", "type": "string", "is_object": True}, - {"property": "number", "type": "string", "is_object": True}, - {"property": "reason", "type": "string", "is_object": True}, - { - "property": "submitters_verbose", - "type": "string", - "is_list": True, - "is_hidden": True, - }, - { - "property": "submitters_username", - "type": "string", - "is_object": True, - "is_list": True, - }, - { - "property": "supporters_verbose", - "type": "string", - "is_list": True, - "is_hidden": True, - }, - { - "property": "supporters_username", - "type": "string", - "is_object": True, - "is_list": True, - }, - {"property": "category_name", "type": "string", "is_object": True}, - {"property": "category_prefix", "type": "string"}, - {"property": "tags", "type": "string", "is_object": True, "is_list": True}, - {"property": "block", "type": "string", "is_object": True}, - { - "property": "motion_amendment", - "type": "boolean", - "is_object": True, - "is_hidden": True, - }, - ] - permission = Permissions.Motion.CAN_MANAGE - row_state: ImportState - number_lookup: Lookup - username_lookup: dict[str, list[dict[str, Any]]] = {} - category_lookup: dict[str, list[dict[str, Any]]] = {} - tags_lookup: dict[str, list[dict[str, Any]]] = {} - block_lookup: dict[str, list[dict[str, Any]]] = {} - _first_state_id: int | None = None - _operator_username: str | None = None - _previous_numbers: list[str] - import_name = "motion" - - def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: - # transform instance into a correct create/update payload - # try to find a pre-existing motion with the same number - # if there is one, validate for a motion.update, otherwise for a motion.create - # using get_update_payload_integrity_error_message and get_create_payload_integrity_error_message - - data = instance.pop("data") - data = self.add_payload_index_to_action_data(data) - self.setup_lookups(data, instance["meeting_id"]) - - # enrich data with meeting_id - for entry in data: - entry["meeting_id"] = instance["meeting_id"] - - self._previous_numbers = [] - self.rows = [self.validate_entry(entry) for entry in data] - - # generate statistics - self.generate_statistics() - return {} - - def validate_entry(self, entry: dict[str, Any]) -> dict[str, Any]: - messages: list[str] = [] - id_: int | None = None - meeting_id: int = entry["meeting_id"] - set_entry_id = False - - if (is_amendment := entry.get("motion_amendment")) is not None: - entry["motion_amendment"] = { - "value": is_amendment, - "info": ImportState.DONE, - } - if is_amendment: - entry["motion_amendment"]["info"] = ImportState.WARNING - messages.append("Amendments cannot be correctly imported") - - if category_name := entry.get("category_name"): - category_prefix = entry.get("category_prefix") - categories = self.category_lookup.get(category_name, []) - categories = [ - category - for category in categories - if category.get("prefix") == category_prefix - ] - if len(categories) == 1 and categories[0].get("id") != 0: - entry["category_name"] = { - "value": category_name, - "info": ImportState.DONE, - "id": categories[0].get("id"), - } - else: - entry["category_name"] = { - "value": category_name, - "info": ImportState.WARNING, - } - messages.append("Category could not be found") - elif category_prefix := entry.get("category_prefix"): - entry["category_name"] = {"value": "", "info": ImportState.WARNING} - messages.append("Category could not be found") - - if number := entry.get("number"): - check_result = self.number_lookup.check_duplicate(number) - id_ = cast(int, self.number_lookup.get_field_by_name(number, "id")) - if check_result == ResultType.FOUND_ID and id_ != 0: - self.row_state = ImportState.DONE - set_entry_id = True - entry["number"] = { - "value": number, - "info": ImportState.DONE, - "id": id_, - } - elif check_result == ResultType.NOT_FOUND or id_ == 0: - self.row_state = ImportState.NEW - entry["number"] = { - "value": number, - "info": ImportState.DONE, - } - elif check_result == ResultType.FOUND_MORE_IDS: - self.row_state = ImportState.ERROR - entry["number"] = { - "value": number, - "info": ImportState.ERROR, - } - messages.append("Error: Found multiple motions with the same number") - else: - category_id: int | None = None - if entry.get("category_name"): - category_id = entry["category_name"].get("id") - self.row_state = ImportState.NEW - value: dict[str, Any] = {} - self.set_number( - value, - meeting_id, - self._get_first_workflow_state_id(meeting_id), - None, - category_id, - other_forbidden_numbers=self._previous_numbers, - ) - if number := value.get("number"): - entry["number"] = {"value": number, "info": ImportState.GENERATED} - self._previous_numbers.append(number) - - has_submitter_error: bool = False - for fieldname in ["submitter", "supporter"]: - if users := entry.get(f"{fieldname}s_username"): - verbose = entry.get(f"{fieldname}s_verbose", []) - verbose_user_mismatch = len(verbose) > len(users) - username_set: set[str] = set() - entry_list: list[dict[str, Any]] = [] - duplicates: set[str] = set() - not_found: set[str] = set() - for user in users: - if verbose_user_mismatch: - entry_list.append({"value": user, "info": ImportState.ERROR}) - elif user in username_set: - entry_list.append({"value": user, "info": ImportState.WARNING}) - duplicates.add(user) - else: - username_set.add(user) - found_users = self.username_lookup.get(user, []) - if len(found_users) == 1 and found_users[0].get("id") != 0: - user_id = cast(int, found_users[0].get("id")) - entry_list.append( - { - "value": user, - "info": ImportState.DONE, - "id": user_id, - } - ) - elif len(found_users) <= 1: - entry_list.append( - { - "value": user, - "info": ImportState.WARNING, - } - ) - not_found.add(user) - else: - raise ActionException( - f"Database corrupt: Found multiple users with the username {user}" - ) - entry[f"{fieldname}s_username"] = entry_list - if verbose_user_mismatch: - self.row_state = ImportState.ERROR - messages.append( - f"Error: Verbose field is set and has more entries than the username field for {fieldname}s" - ) - if fieldname == "submitter": - has_submitter_error = True - if len(duplicates): - messages.append( - f"At least one {fieldname} has been referenced multiple times: " - + ", ".join(duplicates) - ) - if len(not_found): - messages.append( - f"Could not find at least one {fieldname}: " - + ", ".join(not_found) - ) - - if not has_submitter_error: - if ( - len(cast(list[dict[str, Any]], entry.get("submitters_username", []))) - == 0 - ): - entry["submitters_username"] = [self._get_self_username_object()] - elif ( - len( - [ - entry - for entry in ( - cast( - list[dict[str, Any]], - entry.get("submitters_username", []), - ) - ) - if entry.get("info") and (entry["info"] != ImportState.WARNING) - ] - ) - == 0 - ): - entry["submitters_username"].append(self._get_self_username_object()) - - if tags := entry.get("tags"): - entry_list = [] - duplicates = set() - not_found = set() - multiple: set[str] = set() - tags_set: set[str] = set() - for tag in tags: - if tag in tags_set: - entry_list.append({"value": tag, "info": ImportState.WARNING}) - duplicates.add(tag) - else: - tags_set.add(tag) - found_tags = self.tags_lookup.get(tag, []) - if len(found_tags) == 1 and found_tags[0].get("id") != 0: - tag_id = cast(int, found_tags[0].get("id")) - entry_list.append( - { - "value": tag, - "info": ImportState.DONE, - "id": tag_id, - } - ) - elif len(found_tags) <= 1: - entry_list.append( - { - "value": tag, - "info": ImportState.WARNING, - } - ) - not_found.add(tag) - else: - entry_list.append( - { - "value": tag, - "info": ImportState.WARNING, - } - ) - multiple.add(tag) - entry["tags"] = entry_list - if len(duplicates): - messages.append( - "At least one tag has been referenced multiple times: " - + ", ".join(duplicates) - ) - if len(not_found): - messages.append( - "Could not find at least one tag: " + ", ".join(not_found) - ) - if len(multiple): - messages.append( - "Found multiple tags with the same name: " + ", ".join(multiple) - ) - - if (block := entry.get("block")) and isinstance(block, str): - found_blocks = self.block_lookup.get(block, []) - if len(found_blocks) == 1 and found_blocks[0].get("id") != 0: - block_id = cast(int, found_blocks[0].get("id")) - entry["block"] = { - "value": block, - "info": ImportState.DONE, - "id": block_id, - } - elif len(found_blocks) <= 1: - entry["block"] = { - "value": block, - "info": ImportState.WARNING, - } - messages.append("Could not find motion block") - else: - entry["block"] = { - "value": block, - "info": ImportState.WARNING, - } - messages.append("Found multiple motion blocks with the same name") - - if id_ and set_entry_id: - entry["id"] = id_ - - if (text := entry.get("text")) and not search( - r"^<\w+[^>]*>[\w\W]*?<\/\w>$", text - ): - entry["text"] = ( - "

" - + sub(r"\n", "
", sub(r"\n([ \t]*\n)+", "

", text)) - + "

" - ) - - for field in ["title", "text", "reason"]: - if (date := entry.get(field)) and isinstance(date, str): - if date == "": - del entry[field] - else: - entry[field] = {"value": date, "info": ImportState.DONE} - - # check via mixin - payload = { - **{ - k: v.get("value") - for k, v in entry.items() - if k in ["title", "text", "number", "reason"] - }, - **{ - k: self._get_field_ids(entry, v) - for k, v in { - "submitter_ids": "submitters_username", - "supporter_meeting_user_ids": "supporters_username", - "tag_ids": "tags", - }.items() - }, - **{ - k: self._get_field_id(entry, v) - for k, v in { - "category_id": "category_name", - "block_id": "block", - }.items() - if entry.get(v) - }, - } - - errors: list[MotionActionErrorData] = [] - if id_: - payload = {"id": id_, **payload} - errors = self.get_update_payload_integrity_error_message( - payload, meeting_id - ) - else: - payload = {"meeting_id": meeting_id, **payload} - errors = self.get_create_payload_integrity_error_message( - payload, meeting_id - ) - - for err in errors: - entry = self._add_error_to_entry(entry, err) - messages.append("Error: " + err["message"]) - - return {"state": self.row_state, "messages": messages, "data": entry} - - def setup_lookups(self, data: Iterable[dict[str, Any]], meeting_id: int) -> None: - self.number_lookup = Lookup( - self.datastore, - "motion", - [(number, entry) for entry in data if (number := entry.get("number"))], - field="number", - mapped_fields=[], - global_and_filter=FilterOperator("meeting_id", "=", meeting_id), - ) - self.block_lookup = self.get_lookup_dict( - "motion_block", - [title for entry in data if (title := entry.get("block"))], - "title", - and_filters=[FilterOperator("meeting_id", "=", meeting_id)], - ) - self.category_lookup = self.get_lookup_dict( - "motion_category", - [name for entry in data if (name := entry.get("category_name"))], - "name", - ["prefix"], - and_filters=[FilterOperator("meeting_id", "=", meeting_id)], - ) - self.username_lookup = self.get_lookup_dict( - "user", - list( - { - username - for entry in data - if ( - usernames := [ - *entry.get("submitters_username", []), - *entry.get("supporters_username", []), - ] - ) - for username in usernames - if username - } - ), - "username", - ["meeting_ids"], - ) - self.username_lookup = { - username: [ - date - for date in self.username_lookup[username] - if date.get("meeting_ids") and (meeting_id in date["meeting_ids"]) - ] - for username in self.username_lookup - } - self.tags_lookup = self.get_lookup_dict( - "tag", - [ - name - for entry in data - if (names := entry.get("tags")) - for name in names - if name - ], - "name", - and_filters=[FilterOperator("meeting_id", "=", meeting_id)], - ) - - def get_lookup_dict( - self, - collection: str, - entries: list[str], - fieldname: str = "name", - mapped_fields: list[str] = [], - and_filters: list[Filter] = [], - ) -> dict[str, list[dict[str, Any]]]: - lookup: dict[str, list[dict[str, Any]]] = defaultdict(list) - if len(entries): - data = self.datastore.filter( - collection, - And( - *and_filters, - Or([FilterOperator(fieldname, "=", name) for name in set(entries)]), - ), - [*mapped_fields, "id", fieldname], - lock_result=False, - ) - for date in data.values(): - lookup[date[fieldname]].append(date) - return lookup - - def _get_self_username_object(self) -> dict[str, Any]: - if not self._operator_username: - user = self.datastore.get("user/" + str(self.user_id), ["username"]) - if not (user and user.get("username")): - raise ActionException("Couldn't find operator's username") - self._operator_username = cast(str, user["username"]) - return { - "value": self._operator_username, - "info": ImportState.GENERATED, - "id": self.user_id, - } - - def _get_first_workflow_state_id(self, meeting_id: int) -> int: - if not self._first_state_id: - default_workflows = self.datastore.filter( - "motion_workflow", - FilterOperator("default_workflow_meeting_id", "=", meeting_id), - mapped_fields=["first_state_id"], - ).values() - if len(default_workflows) != 1: - raise ActionException("Couldn't determine default workflow") - self._first_state_id = cast( - int, list(default_workflows)[0].get("first_state_id") - ) - return self._first_state_id - - def _get_field_ids(self, entry: dict[str, Any], fieldname: str) -> list[int]: - value = entry.get(fieldname, []) - if not isinstance(value, list): - value = [entry[fieldname]] - return [val["id"] for val in value if val.get("id")] - - def _get_field_id(self, entry: dict[str, Any], fieldname: str) -> int: - return entry[fieldname].get("id") - - def _add_error_to_entry( - self, entry: dict[str, Any], err: MotionActionErrorData - ) -> dict[str, Any]: - fieldname = "" - match err["type"]: - case MotionErrorType.UNIQUE_NUMBER: - fieldname = "number" - case MotionErrorType.TEXT: - fieldname = "text" - case MotionErrorType.REASON: - fieldname = "reason" - case MotionErrorType.TITLE: - fieldname = "title" - case _: - raise ActionException("Error: " + err["message"]) - if not (entry.get(fieldname) and isinstance(entry[fieldname], dict)): - entry[fieldname] = { - "value": entry.get(fieldname, ""), - "info": ImportState.ERROR, - } - else: - entry[fieldname]["info"] = ImportState.ERROR - self.row_state = ImportState.ERROR - return entry diff --git a/tests/system/action/motion/test_import.py b/tests/system/action/motion/test_import.py deleted file mode 100644 index da996334e5..0000000000 --- a/tests/system/action/motion/test_import.py +++ /dev/null @@ -1,1161 +0,0 @@ -from openslides_backend.action.mixins.import_mixins import ImportState - -from .test_json_upload import MotionJsonUploadForUseInImport - - -class MotionImport(MotionJsonUploadForUseInImport): - def test_import_database_corrupt(self) -> None: - self.create_meeting(42) - self.set_models( - { - "import_preview/2": { - "state": ImportState.DONE, - "name": "motion", - "result": { - "rows": [ - { - "state": (ImportState.DONE), - "messages": [], - "data": { - "title": { - "value": "New", - "info": ImportState.DONE, - }, - "text": { - "value": "Motion", - "info": ImportState.DONE, - }, - "reason": {"value": "", "info": ImportState.DONE}, - "supporters_username": [], - "category_name": { - "value": "", - "info": ImportState.DONE, - }, - "tags": [], - "block": {"value": "", "info": ImportState.DONE}, - "number": { - "info": ImportState.DONE, - "id": 101, - "value": "NUM01", - }, - "submitters_username": [ - { - "info": ImportState.DONE, - "id": 12345678, - "value": "bob", - } - ], - "id": 101, - "meeting_id": 42, - }, - } - ], - }, - }, - "user/12345678": { - "username": "bob", - "meeting_ids": [42], - "meeting_user_ids": [12345678], - }, - "meeting_user/12345678": {}, - "user/123456789": { - "username": "bob", - "meeting_ids": [42], - "meeting_user_ids": [123456789], - }, - "meeting_user/123456789": {}, - "meeting/42": {"meeting_user_ids": [12345678, 123456789]}, - } - ) - response = self.request("motion.import", {"id": 2, "import": True}) - self.assert_status_code(response, 400) - assert ( - "Database corrupt: Found multiple users with the username bob." - in response.json["message"] - ) - - def test_import_abort(self) -> None: - self.create_meeting(42) - self.set_models( - { - "import_preview/2": { - "state": ImportState.DONE, - "name": "motion", - "result": { - "rows": [ - { - "state": ImportState.NEW, - "messages": [], - "data": { - "title": { - "value": "New", - "info": ImportState.DONE, - }, - "text": { - "value": "Motion", - "info": ImportState.DONE, - }, - "number": {"value": "", "info": ImportState.DONE}, - "reason": {"value": "", "info": ImportState.DONE}, - "submitters_username": [ - { - "value": "admin", - "info": ImportState.GENERATED, - "id": 1, - } - ], - "supporters_username": [], - "category_name": { - "value": "", - "info": ImportState.DONE, - }, - "tags": [], - "block": {"value": "", "info": ImportState.DONE}, - "meeting_id": 42, - }, - } - ], - }, - }, - } - ) - response = self.request("motion.import", {"id": 2, "import": False}) - self.assert_status_code(response, 200) - self.assert_model_not_exists("import_preview/2") - self.assert_model_not_exists("motion/1") - - def test_import_wrong_import_preview(self) -> None: - self.create_meeting(42) - self.set_models( - { - "import_preview/3": {"result": None}, - "import_preview/4": { - "state": ImportState.DONE, - "name": "topic", - "result": { - "rows": [ - { - "state": ImportState.NEW, - "messages": [], - "data": { - "title": {"value": "test", "info": ImportState.NEW}, - "meeting_id": 42, - }, - }, - ], - }, - }, - } - ) - response = self.request("motion.import", {"id": 3, "import": True}) - self.assert_status_code(response, 400) - assert ( - "Wrong id doesn't point on motion import data." in response.json["message"] - ) - - def test_import_non_existant_ids(self) -> None: - self.create_meeting(42) - preview_rows = [ - { - "state": ImportState.DONE, - "messages": [], - "data": { - "id": 1, - "title": { - "value": "Update", - "info": ImportState.DONE, - }, - "text": { - "value": "

of non-existant motion", - "info": ImportState.DONE, - }, - "number": { - "id": 1, - "value": "NOMNOMNOM1", - "info": ImportState.DONE, - }, - "submitters_username": [ - { - "value": "nonExistantUser", - "info": ImportState.DONE, - "id": 2, - } - ], - "supporters_username": [ - { - "value": "NoOne", - "info": ImportState.DONE, - "id": 3, - } - ], - "category_name": { - "id": 8, - "value": "NonCategory", - "info": ImportState.DONE, - }, - "tags": [ - { - "id": 9, - "value": "NonTag", - "info": ImportState.DONE, - } - ], - "block": {"id": 9, "value": "NonBlock", "info": ImportState.DONE}, - "meeting_id": 42, - }, - }, - { - "state": ImportState.NEW, - "messages": [], - "data": { - "title": { - "value": "Create", - "info": ImportState.DONE, - }, - "text": { - "value": "

a new motion

", - "info": ImportState.DONE, - }, - "number": {"value": "NEW", "info": ImportState.DONE}, - "submitters_username": [ - { - "value": "nonUser", - "info": ImportState.DONE, - "id": 4, - } - ], - "supporters_username": [ - { - "value": "NoOne", - "info": ImportState.DONE, - "id": 5, - } - ], - "category_name": { - "id": 10, - "value": "NonCategoryTwoElectricBoogaloo", - "info": ImportState.DONE, - }, - "tags": [ - { - "id": 11, - "value": "TagNot", - "info": ImportState.DONE, - } - ], - "block": { - "id": 12, - "value": "JustBlockIt", - "info": ImportState.DONE, - }, - "meeting_id": 42, - }, - }, - ] - self.set_models( - { - "import_preview/2": { - "state": ImportState.DONE, - "name": "motion", - "result": { - "rows": preview_rows, - }, - }, - } - ) - response = self.request("motion.import", {"id": 2, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_not_exists("motion/1") - self.assert_model_not_exists("motion/2") - meeting = self.assert_model_exists("meeting/42") - assert "motion_ids" not in meeting - response_rows = response.json["results"][0][0]["rows"] - assert response_rows[0]["data"] == { - "id": 1, - "tags": [{"info": "error", "value": "NonTag"}], - "text": {"info": "done", "value": "

of non-existant motion"}, - "block": {"value": "NonBlock", "info": "error"}, - "title": {"info": "done", "value": "Update"}, - "number": {"id": 1, "info": "error", "value": "NOMNOMNOM1"}, - "meeting_id": 42, - "category_name": {"value": "NonCategory", "info": "error"}, - "submitters_username": [{"info": "error", "value": "nonExistantUser"}], - "supporters_username": [{"info": "error", "value": "NoOne"}], - } - assert sorted(response_rows[0]["messages"]) == sorted( - [ - "Error: Couldn't find motion block anymore", - "Error: Couldn't find supporter anymore: NoOne", - "Error: Couldn't find tag anymore: NonTag", - "Error: Category could not be found anymore", - "Error: Couldn't find submitter anymore: nonExistantUser", - "Error: Motion 1 not found anymore for updating motion 'NOMNOMNOM1'.", - ] - ) - assert response_rows[1]["data"] == { - "tags": [{"info": "error", "value": "TagNot"}], - "text": {"info": "done", "value": "

a new motion

"}, - "block": {"value": "JustBlockIt", "info": "error"}, - "title": {"info": "done", "value": "Create"}, - "number": {"info": "done", "value": "NEW"}, - "meeting_id": 42, - "category_name": { - "value": "NonCategoryTwoElectricBoogaloo", - "info": "error", - }, - "submitters_username": [{"info": "error", "value": "nonUser"}], - "supporters_username": [{"info": "error", "value": "NoOne"}], - } - assert sorted(response_rows[1]["messages"]) == sorted( - [ - "Error: Couldn't find motion block anymore", - "Error: Couldn't find supporter anymore: NoOne", - "Error: Couldn't find tag anymore: TagNot", - "Error: Category could not be found anymore", - "Error: Couldn't find submitter anymore: nonUser", - ] - ) - - def test_import_with_deleted_references(self) -> None: - self.json_upload_multi_row() - self.request("motion.delete", {"id": 100}) - self.request("user.delete", {"id": 2}) - self.request("meeting_user.delete", {"id": 3}) - self.request("motion_category.delete", {"id": 100}) - self.request("motion_category.delete", {"id": 1000}) - self.request("motion_block.delete", {"id": 1}) - self.request("tag.delete", {"id": 1}) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/1", - { - "title": "A", - "text": "

nice little

", - "reason": "motion", - }, - ) - self.assert_model_exists("meeting/1", {"motion_ids": [1]}) - self.assert_model_deleted("motion/100") - self.assert_model_not_exists("motion/101") - self.assert_model_not_exists("motion/102") - self.assert_model_not_exists("motion/103") - rows = response.json["results"][0][0]["rows"] - assert len(rows) == 5 - row = rows[0] - assert row["state"] == ImportState.ERROR - assert row["messages"] == ["Error: Couldn't find supporter anymore: user1"] - assert row["data"]["supporters_username"] == [ - {"info": ImportState.ERROR, "value": "user1"} - ] - row = rows[1] - assert row["state"] == ImportState.ERROR - assert sorted(row["messages"]) == sorted( - [ - "Error: Motion 100 not found anymore for updating motion 'NUM02'.", - "Error: Couldn't find submitter anymore: user1", - "Error: Category could not be found anymore", - ] - ) - assert row["data"]["number"] == { - "id": 100, - "value": "NUM02", - "info": ImportState.ERROR, - } - assert row["data"]["category_name"] == { - "info": ImportState.ERROR, - "value": "Other motion", - } - assert row["data"]["submitters_username"] == [ - {"info": ImportState.ERROR, "value": "user1"} - ] - row = rows[2] - assert row["state"] == ImportState.ERROR - assert row["messages"] == ["Error: Couldn't find tag anymore: Tag1"] - assert row["data"]["tags"] == [{"info": ImportState.ERROR, "value": "Tag1"}] - row = rows[3] - assert row["state"] == ImportState.ERROR - assert sorted(row["messages"]) == sorted( - [ - "Error: Couldn't find supporter anymore: user1, anotherUser", - "Error: Couldn't find motion block anymore", - ] - ) - assert row["data"]["supporters_username"] == [ - {"info": ImportState.ERROR, "value": "user1"}, - {"id": 1, "info": ImportState.DONE, "value": "admin"}, - {"info": ImportState.ERROR, "value": "anotherUser"}, - ] - assert row["data"]["block"] == {"info": ImportState.ERROR, "value": "Block1"} - row = rows[4] - assert row["state"] == ImportState.ERROR - assert row["messages"] == ["Error: Category could not be found anymore"] - assert row["data"]["category_name"] == { - "info": ImportState.ERROR, - "value": "Other motion", - } - - def test_import_with_changed_references(self) -> None: - self.json_upload_multi_row() - self.set_models( - { - "user/2": {"username": "changedName"}, - "user/3": {"username": "changedNameToo"}, - "motion_category/100": {"name": "changedName"}, - "motion_category/1000": {"prefix": "changedPREFIX"}, - "motion_block/1": {"title": "changedTitle"}, - "tag/1": {"name": "changedName"}, - } - ) - self.create_user("anotherUser", [1]) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - rows = response.json["results"][0][0]["rows"] - assert len(rows) == 5 - row = rows[0] - assert row["state"] == ImportState.ERROR - assert row["messages"] == ["Error: Couldn't find supporter anymore: user1"] - assert row["data"]["supporters_username"] == [ - {"info": ImportState.ERROR, "value": "user1"} - ] - row = rows[1] - assert row["state"] == ImportState.ERROR - assert sorted(row["messages"]) == sorted( - [ - "Error: Couldn't find submitter anymore: user1", - "Error: Category could not be found anymore", - ] - ) - assert row["data"]["category_name"] == { - "info": ImportState.ERROR, - "value": "Other motion", - } - assert row["data"]["submitters_username"] == [ - {"info": ImportState.ERROR, "value": "user1"} - ] - row = rows[2] - assert row["state"] == ImportState.ERROR - assert row["messages"] == ["Error: Couldn't find tag anymore: Tag1"] - assert row["data"]["tags"] == [{"info": ImportState.ERROR, "value": "Tag1"}] - row = rows[3] - assert row["state"] == ImportState.ERROR - assert sorted(row["messages"]) == sorted( - [ - "Error: Supporter search didn't deliver the same result as in the preview: anotherUser", - "Error: Couldn't find motion block anymore", - "Error: Couldn't find supporter anymore: user1", - ] - ) - assert row["data"]["supporters_username"] == [ - {"info": ImportState.ERROR, "value": "user1"}, - {"id": 1, "info": ImportState.DONE, "value": "admin"}, - {"info": ImportState.ERROR, "value": "anotherUser"}, - ] - assert row["data"]["block"] == {"info": ImportState.ERROR, "value": "Block1"} - row = rows[4] - assert row["state"] == ImportState.ERROR - assert row["messages"] == ["Error: Category could not be found anymore"] - assert row["data"]["category_name"] == { - "info": ImportState.ERROR, - "value": "Other motion", - } - - def test_import_wrong_meeting_model_import_preview(self) -> None: - self.create_meeting(42) - self.set_models( - { - "import_preview/4": { - "state": ImportState.DONE, - "name": "topic", - "result": { - "rows": [ - { - "state": ImportState.NEW, - "messages": [], - "data": { - "title": {"value": "test", "info": ImportState.NEW}, - "meeting_id": 42, - }, - }, - ], - }, - }, - } - ) - response = self.request("motion.import", {"id": 4, "import": True}) - self.assert_status_code(response, 400) - assert ( - "Wrong id doesn't point on motion import data." in response.json["message"] - ) - - def test_json_upload_amendment(self) -> None: - self.json_upload_amendment() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.DONE - - def test_json_upload_with_errors(self) -> None: - self.json_upload_create_missing_title() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 400) - assert "Error in import. Data will not be imported." in response.json["message"] - - def test_json_upload_update_missing_title(self) -> None: - self.json_upload_update_missing_title() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.DONE - self.assert_model_exists("motion/42", {"title": "A"}) - - def test_json_upload_update_missing_reason_although_required(self) -> None: - self.json_upload_update_missing_reason_although_required() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.DONE - self.assert_model_exists("motion/42", {"reason": "motion"}) - - def test_json_upload_multi_row(self) -> None: - self.json_upload_multi_row() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/1", - { - "title": "first", - "text": "

test my stuff

", - "reason": "motion", # retained from before - "supporter_meeting_user_ids": [1], - "submitter_ids": [4], - }, - ) - self.assert_model_exists("motion_submitter/4", {"meeting_user_id": 2}) - self.assert_model_exists("meeting_user/1", {"user_id": 2}) - self.assert_model_exists("meeting_user/2", {"user_id": 1}) - self.assert_model_exists( - "motion/100", - { - "title": "then", - "text": "

nice little

", # retained from before - "reason": "test my other stuff", - "submitter_ids": [5], - "category_id": 1000, - }, - ) - self.assert_model_exists("motion_submitter/5", {"meeting_user_id": 1}) - self.assert_model_exists( - "motion/101", - { - "number": "NUM03", - "title": "also", - "text": "

test the other peoples stuff

", - "submitter_ids": [1], - "tag_ids": [1], - }, - ) - self.assert_model_exists("motion_submitter/1", {"meeting_user_id": 2}) - self.assert_model_exists( - "motion/102", - { - "number": "03", - "title": "after that", - "text": "

test even more stuff

", - "submitter_ids": [2], - "supporter_meeting_user_ids": [1, 2, 3], - "block_id": 1, - }, - ) - self.assert_model_exists("motion_submitter/2", {"meeting_user_id": 2}) - self.assert_model_exists( - "motion/103", - { - "number": "OTHER01", - "title": "finally", - "text": "

finish testing

", - "submitter_ids": [3], - "category_id": 100, - }, - ) - self.assert_model_exists("motion_submitter/3", {"meeting_user_id": 2}) - - def test_simple_create(self) -> None: - self.json_upload_simple_create() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - motion = self.assert_model_exists( - "motion/4201", - { - "title": "test", - "text": "

my

", - "reason": "stuff", - "submitter_ids": [1], - }, - ) - assert "number" not in motion - self.assert_model_exists("motion_submitter/1", {"meeting_user_id": 2}) - self.assert_model_exists("meeting_user/2", {"user_id": 1}) - - def test_simple_create_with_reason_required(self) -> None: - self.json_upload_simple_create(True) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - motion = self.assert_model_exists( - "motion/4201", - { - "title": "test", - "text": "

my

", - "reason": "stuff", - "submitter_ids": [1], - }, - ) - assert "number" not in motion - self.assert_model_exists("motion_submitter/1", {"meeting_user_id": 2}) - self.assert_model_exists("meeting_user/2", {"user_id": 1}) - - def test_simple_create_with_set_number(self) -> None: - self.json_upload_simple_create(is_set_number=True) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/4201", - { - "number": "03", - "title": "test", - "text": "

my

", - "reason": "stuff", - "submitter_ids": [1], - }, - ) - self.assert_model_exists("motion_submitter/1", {"meeting_user_id": 2}) - self.assert_model_exists("meeting_user/2", {"user_id": 1}) - - def test_simple_update(self) -> None: - self.json_upload_simple_update() - self.assert_simple_update_successful() - - def test_simple_update_with_reason_required(self) -> None: - self.json_upload_simple_update(True) - self.assert_simple_update_successful() - - def test_simple_update_with_set_number(self) -> None: - self.json_upload_simple_update(is_set_number=True) - self.assert_simple_update_successful() - - def assert_simple_update_successful(self) -> None: - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/42", - { - "number": "NUM01", - "title": "test", - "text": "

my

", - "reason": "stuff", - "submitter_ids": [1], - }, - ) - self.assert_model_exists("motion_submitter/1", {"meeting_user_id": 2}) - self.assert_model_exists("meeting_user/2", {"user_id": 1}) - - def test_json_upload_update_with_foreign_meeting(self) -> None: - self.json_upload_update_with_foreign_meeting() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/42", - { - "number": "NUM01", - "title": "test", - "text": "

my

", - "reason": "stuff", - "category_id": 42, - "submitter_ids": [1], - "supporter_meeting_user_ids": [1], - }, - ) - self.assert_model_exists("motion_submitter/1", {"meeting_user_id": 3}) - self.assert_model_exists("meeting_user/3", {"user_id": 1}) - - def test_json_upload_custom_number_create(self) -> None: - self.json_upload_custom_number_create() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/4201", - { - "number": "Z01", - "title": "test", - "text": "

my

", - "reason": "stuff", - "category_id": 420, - }, - ) - - def test_json_upload_custom_number_create_with_set_number(self) -> None: - self.json_upload_custom_number_create_with_set_number() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/4201", - { - "number": "Z01", - "title": "test", - "text": "

my

", - "reason": "stuff", - "category_id": 420, - }, - ) - - def test_with_warnings(self) -> None: - self.json_upload_with_warnings() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - motion = self.assert_model_exists("motion/10") - assert "category_id" not in motion - assert motion["submitter_ids"] == [3] - self.assert_model_exists("motion_submitter/3", {"meeting_user_id": 1}) - self.assert_model_exists("meeting_user/1", {"user_id": 2}) - - motion = self.assert_model_exists("motion/1001") - assert "category_id" not in motion - assert motion["submitter_ids"] == [1] - self.assert_model_exists("motion_submitter/1", {"meeting_user_id": 2}) - self.assert_model_exists("meeting_user/2", {"user_id": 1}) - assert motion["supporter_meeting_user_ids"] == [1] - - motion = self.assert_model_exists("motion/1002") - assert "category_id" not in motion - assert motion["submitter_ids"] == [2] - self.assert_model_exists("motion_submitter/2", {"meeting_user_id": 2}) - assert len(motion["supporter_meeting_user_ids"]) == 0 - - def test_with_non_matching_verbose_users_okay(self) -> None: - self.json_upload_with_non_matching_verbose_users_okay() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/123", - { - "number": "NUM01", - "title": "Up", - "text": "

date

", - "submitter_ids": [2, 3], - "supporter_meeting_user_ids": [1, 2], - }, - ) - newMotion = self.assert_model_exists( - "motion/12301", - { - "title": "Newer", - "text": "

motion

", - "submitter_ids": [1], - }, - ) - assert len(newMotion.get("supporter_meeting_user_ids", [])) == 0 - self.assert_model_exists( - "meeting_user/1", {"user_id": 2, "motion_submitter_ids": [2]} - ) - self.assert_model_exists( - "meeting_user/2", {"user_id": 3, "motion_submitter_ids": [3]} - ) - self.assert_model_exists( - "meeting_user/3", {"user_id": 1, "motion_submitter_ids": [1]} - ) - - def test_with_tags_and_blocks(self) -> None: - self.json_upload_with_tags_and_blocks() - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/42", - { - "number": "NUM01", - "tag_ids": [1, 2], - "block_id": 1, - }, - ) - self.assert_model_exists("motion/5501", {"tag_ids": [1, 2], "block_id": 2}) - mot3 = self.assert_model_exists("motion/5502", {"tag_ids": []}) - assert "block_id" not in mot3 - mot4 = self.assert_model_exists("motion/5503", {"tag_ids": []}) - assert "block_id" not in mot4 - - def test_with_new_duplicate_tags_and_blocks(self) -> None: - self.json_upload_with_tags_and_blocks() - self.set_models( - { - "tag/7": {"name": "Tag-liatelle", "meeting_id": 42}, - "motion_block/7": {"title": "Blockolade", "meeting_id": 42}, - "meeting/42": { - "tag_ids": [1, 2, 3, 4, 7], - "motion_block_ids": [1, 2, 3, 4, 7], - }, - } - ) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - self.assert_model_exists( - "motion/42", - { - "number": "NUM01", - "tag_ids": [1, 2], - "block_id": 1, - }, - ) - self.assert_model_exists("motion/5501", {"tag_ids": [1, 2], "block_id": 2}) - mot3 = self.assert_model_exists("motion/5502", {"tag_ids": []}) - assert "block_id" not in mot3 - mot4 = self.assert_model_exists("motion/5503", {"tag_ids": []}) - assert "block_id" not in mot4 - - def test_update_with_changed_number(self) -> None: - self.json_upload_simple_update() - self.set_models({"motion/42": {"number": "CHANGED01"}}) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["rows"][0]["state"] == ImportState.ERROR - assert response.json["results"][0][0]["rows"][0]["data"]["number"] == { - "id": 42, - "info": ImportState.ERROR, - "value": "NUM01", - } - assert response.json["results"][0][0]["rows"][0]["messages"] == [ - "Error: Motion 42 not found anymore for updating motion 'NUM01'." - ] - - def test_import_with_newly_duplicate_number(self) -> None: - self.setup_meeting_with_settings(5, True, True) - self.set_models( - { - "import_preview/55": { - "state": ImportState.DONE, - "name": "motion", - "result": { - "rows": [ - { - "state": ImportState.NEW, - "messages": [], - "data": { - "title": { - "value": "New", - "info": ImportState.DONE, - }, - "text": { - "value": "Motion", - "info": ImportState.DONE, - }, - "number": { - "info": ImportState.DONE, - "value": "NUM01", - }, - "submitters_username": [ - { - "value": "admin", - "info": ImportState.GENERATED, - "id": 1, - } - ], - "meeting_id": 5, - }, - } - ], - }, - } - } - ) - response = self.request("motion.import", {"id": 55, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["rows"][0]["state"] == ImportState.ERROR - assert response.json["results"][0][0]["rows"][0]["messages"] == [ - "Error: Row state expected to be 'done', but it is 'new'." - ] - assert ( - response.json["results"][0][0]["rows"][0]["data"]["number"]["info"] - == ImportState.ERROR - ) - - def test_import_without_reason_when_required(self) -> None: - self.setup_meeting_with_settings(5, True, True) - self.set_models( - { - "import_preview/55": { - "state": ImportState.DONE, - "name": "motion", - "result": { - "rows": [ - { - "state": ImportState.NEW, - "messages": [], - "data": { - "title": { - "value": "New", - "info": ImportState.DONE, - }, - "text": { - "value": "Motion", - "info": ImportState.DONE, - }, - "submitters_username": [ - { - "value": "admin", - "info": ImportState.GENERATED, - "id": 1, - } - ], - "meeting_id": 5, - }, - } - ], - }, - } - } - ) - response = self.request("motion.import", {"id": 55, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["rows"][0]["state"] == ImportState.ERROR - assert response.json["results"][0][0]["rows"][0]["messages"] == [ - "Error: Reason is required" - ] - assert response.json["results"][0][0]["rows"][0]["data"]["reason"] == { - "value": "", - "info": ImportState.ERROR, - } - - def test_update_with_replaced_number(self) -> None: - self.json_upload_simple_update() - self.set_models( - { - "motion/42": {"number": "CHANGED01"}, - "motion/56": { - "meeting_id": 42, - "number": "NUM01", - "title": "Impostor", - "text": "

motion

", - }, - "meeting/42": {"motion_ids": [42, 56, 4200]}, - } - ) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["rows"][0]["state"] == ImportState.ERROR - assert response.json["results"][0][0]["rows"][0]["data"]["number"] == { - "id": 42, - "info": ImportState.ERROR, - "value": "NUM01", - } - assert response.json["results"][0][0]["rows"][0]["messages"] == [ - "Error: Number 'NUM01' found in different id (56 instead of 42)" - ] - - def test_update_with_duplicated_number(self) -> None: - self.json_upload_simple_update() - self.set_models( - { - "motion/56": { - "meeting_id": 42, - "number": "NUM01", - "title": "Impostor", - "text": "

motion

", - }, - "meeting/42": {"motion_ids": [42, 56, 4200]}, - } - ) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["rows"][0]["state"] == ImportState.ERROR - assert response.json["results"][0][0]["rows"][0]["data"]["number"] == { - "id": 42, - "info": ImportState.ERROR, - "value": "NUM01", - } - assert response.json["results"][0][0]["rows"][0]["messages"] == [ - "Error: Number 'NUM01' is duplicated in import." - ] - - def test_update_with_duplicated_number_2(self) -> None: - self.setup_meeting_with_settings(5, True, True) - row = { - "state": ImportState.DONE, - "messages": [], - "data": { - "id": 5, - "title": { - "value": "New", - "info": ImportState.DONE, - }, - "text": { - "value": "Motion", - "info": ImportState.DONE, - }, - "number": {"value": "NUM01", "info": ImportState.DONE, "id": 5}, - "submitters_username": [ - { - "value": "admin", - "info": ImportState.GENERATED, - "id": 1, - } - ], - "meeting_id": 5, - }, - } - self.set_models( - { - "import_preview/123": { - "state": ImportState.DONE, - "name": "motion", - "result": { - "rows": [row, row.copy()], - }, - } - } - ) - response = self.request("motion.import", {"id": 123, "import": True}) - self.assert_status_code(response, 200) - for i in range(2): - assert ( - response.json["results"][0][0]["rows"][i]["state"] == ImportState.ERROR - ) - assert response.json["results"][0][0]["rows"][i]["data"]["number"] == { - "id": 5, - "info": ImportState.ERROR, - "value": "NUM01", - } - assert response.json["results"][0][0]["rows"][i]["messages"] == [ - "Error: Number 'NUM01' is duplicated in import." - ] - - def test_with_replaced_tags_and_blocks(self) -> None: - self.json_upload_with_tags_and_blocks() - self.set_models( - { - "tag/1": {"name": "Changed"}, - "tag/7": {"name": "Tag-liatelle", "meeting_id": 42}, - "motion_block/1": {"title": "Changed"}, - "motion_block/7": {"title": "Blockolade", "meeting_id": 42}, - "meeting/42": { - "tag_ids": [1, 2, 3, 4, 7], - "motion_block_ids": [1, 2, 3, 4, 7], - }, - } - ) - response = self.request("motion.import", {"id": 1, "import": True}) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["rows"][0]["state"] == ImportState.ERROR - assert response.json["results"][0][0]["rows"][0]["data"]["tags"] == [ - { - "info": ImportState.ERROR, - "value": "Tag-liatelle", - }, - {"id": 2, "info": ImportState.DONE, "value": "Tag-you're-it"}, - {"value": "Tag-ether", "info": "warning"}, - {"value": "Price tag", "info": "warning"}, - {"value": "Not a tag", "info": "warning"}, - ] - assert response.json["results"][0][0]["rows"][0]["data"]["block"] == { - "info": ImportState.ERROR, - "value": "Blockolade", - } - assert ( - "Error: Tag search didn't deliver the same result as in the preview: Tag-liatelle" - in response.json["results"][0][0]["rows"][0]["messages"] - ) - assert ( - "Error: Motion block search didn't deliver the same result as in the preview" - in response.json["results"][0][0]["rows"][0]["messages"] - ) - - def test_update_with_broken_id_entries(self) -> None: - self.setup_meeting_with_settings(5, True, True) - self.set_models( - { - "import_preview/123": { - "state": ImportState.DONE, - "name": "motion", - "result": { - "rows": [ - { - "id": 5, - "state": ImportState.DONE, - "messages": [], - "data": { - "title": { - "value": "New", - "info": ImportState.DONE, - }, - "text": { - "value": "Motion", - "info": ImportState.DONE, - }, - "number": { - "value": "NUM01", - "info": ImportState.DONE, - "id": 5, - }, - "submitters_username": [ - { - "value": "admin", - "info": ImportState.GENERATED, - "id": 1, - } - ], - "meeting_id": 5, - }, - }, - ], - }, - } - } - ) - response = self.request("motion.import", {"id": 123, "import": True}) - self.assert_status_code(response, 400) - assert ( - "Invalid JsonUpload data: A data row with state 'done' must have an 'id'" - in response.json["message"] - ) - - def test_update_with_broken_id_entries_2(self) -> None: - self.setup_meeting_with_settings(5, True, True) - self.set_models( - { - "import_preview/123": { - "state": ImportState.DONE, - "name": "motion", - "result": { - "rows": [ - { - "state": ImportState.DONE, - "messages": [], - "data": { - "id": 5, - "title": { - "value": "New", - "info": ImportState.DONE, - }, - "text": { - "value": "Motion", - "info": ImportState.DONE, - }, - "number": { - "value": "NUM01", - "info": ImportState.DONE, - }, - "submitters_username": [ - { - "value": "admin", - "info": ImportState.GENERATED, - "id": 1, - } - ], - "meeting_id": 5, - }, - }, - ], - }, - } - } - ) - response = self.request("motion.import", {"id": 123, "import": True}) - self.assert_status_code(response, 400) - assert ( - "Invalid JsonUpload data: A data row with state 'done' must have an 'id'" - in response.json["message"] - ) diff --git a/tests/system/action/motion/test_json_upload.py b/tests/system/action/motion/test_json_upload.py deleted file mode 100644 index 903708ae47..0000000000 --- a/tests/system/action/motion/test_json_upload.py +++ /dev/null @@ -1,974 +0,0 @@ -from openslides_backend.action.mixins.import_mixins import ImportState -from tests.system.action.base import BaseActionTestCase - - -class BaseMotionJsonUpload(BaseActionTestCase): - def setup_meeting_with_settings( - self, - id_: int = 42, - is_reason_required: bool = False, - is_set_number: bool = False, - ) -> None: - self.create_meeting(id_) - self.create_user(f"user{id_}", [id_], None) - self.set_models( - { - f"meeting/{id_}": { - "motions_reason_required": is_reason_required, - "motions_number_type": "per_category", - "motions_number_min_digits": 2, - "motion_ids": [id_, id_ * 100], - "motion_category_ids": [id_, id_ * 10, id_ * 100, id_ * 1000], - }, - f"motion_state/{id_}": {"set_number": is_set_number}, - f"motion/{id_}": { - "title": "A", - "text": "

nice little

", - "reason": "motion", - "meeting_id": id_, - "number": "NUM01", - "number_value": 1, - }, - f"motion/{id_ * 100}": { - "title": "Another", - "text": "

nice little

", - "reason": "motion", - "meeting_id": id_, - "number": "NUM02", - "number_value": 2, - }, - f"motion_category/{id_}": { - "name": "Normal motion", - "prefix": "NORM", - "meeting_id": id_, - }, - f"motion_category/{id_ * 10}": { - "name": "Other motion", - "prefix": "NORM", - "meeting_id": id_, - }, - f"motion_category/{id_ * 100}": { - "name": "Other motion", - "prefix": "OTHER", - "meeting_id": id_, - }, - f"motion_category/{id_ * 1000}": { - "name": "Other motion", - "meeting_id": id_, - }, - } - ) - - -class MotionJsonUpload(BaseMotionJsonUpload): - def test_json_upload_empty_data(self) -> None: - response = self.request( - "motion.json_upload", - {"data": [], "meeting_id": 42}, - ) - self.assert_status_code(response, 400) - assert "data.data must contain at least 1 items" in response.json["message"] - - def test_json_upload_unknown_meeting_id(self) -> None: - self.setup_meeting_with_settings(42) - response = self.request( - "motion.json_upload", - { - "data": [{"title": "test", "reason": "stuff"}], - "meeting_id": 41, - }, - ) - self.assert_status_code(response, 400) - assert "Import tries to use non-existent meeting 41" in response.json["message"] - - def test_json_upload_create_missing_text(self) -> None: - self.setup_meeting_with_settings(42) - response = self.request( - "motion.json_upload", - { - "data": [{"title": "test", "reason": "stuff"}], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.ERROR - assert ( - "Error: Text is required" - in response.json["results"][0][0]["rows"][0]["messages"] - ) - assert response.json["results"][0][0]["rows"][0]["data"]["text"] == { - "value": "", - "info": ImportState.ERROR, - } - - def test_json_upload_create_missing_reason_although_required(self) -> None: - self.setup_meeting_with_settings(42, is_reason_required=True) - response = self.request( - "motion.json_upload", - { - "data": [{"title": "test", "text": "my"}], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.ERROR - assert ( - "Error: Reason is required" - in response.json["results"][0][0]["rows"][0]["messages"] - ) - assert response.json["results"][0][0]["rows"][0]["data"]["reason"] == { - "value": "", - "info": ImportState.ERROR, - } - - def assert_duplicate_numbers(self, number: str) -> None: - self.setup_meeting_with_settings(22) - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "NUM01", - "title": "test", - "text": "my", - "reason": "stuff", - }, - { - "number": "NUM01", - "title": "test also", - "text": "

my other

", - "reason": "stuff", - }, - { - "number": "NUM04", - "title": "test", - "text": "my", - "reason": "stuff", - }, - { - "number": "NUM04", - "title": "test also", - "text": "

my other

", - "reason": "stuff", - }, - ], - "meeting_id": 22, - }, - ) - self.assert_status_code(response, 200) - assert len(response.json["results"][0][0]["rows"]) == 4 - result = response.json["results"][0][0] - assert result["state"] == ImportState.ERROR - for i in range(4): - assert result["rows"][i]["state"] == ImportState.ERROR - assert ( - "Error: Found multiple motions with the same number" - in result["rows"][i]["messages"] - ) - assert result["rows"][i]["data"]["number"] == { - "value": number, - "info": ImportState.ERROR, - } - - def test_duplicate_numbers_in_datastore(self) -> None: - self.setup_meeting_with_settings(22) - self.set_models( - { - "motion/23": { - "meeting_id": 22, - "number": "NUM01", - "title": "Title", - "text": "

Text

", - }, - "meeting/22": {"motion_ids": [22, 23, 2200]}, - } - ) - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "NUM01", - "title": "test", - "text": "my", - "reason": "stuff", - }, - ], - "meeting_id": 22, - }, - ) - self.assert_status_code(response, 200) - assert len(response.json["results"][0][0]["rows"]) == 1 - result = response.json["results"][0][0] - assert result["state"] == ImportState.ERROR - assert result["rows"][0]["state"] == ImportState.ERROR - assert sorted(result["rows"][0]["messages"]) == sorted( - [ - "Error: Found multiple motions with the same number", - "Error: Number is not unique.", - ] - ) - assert result["rows"][0]["data"]["number"] == { - "value": "NUM01", - "info": ImportState.ERROR, - } - - def test_with_non_matching_verbose_users_with_errors(self) -> None: - self.setup_meeting_with_settings(123) - self.create_user("anotherOne", [123]) - knights = [ - "Sir Lancelot the Brave", - "Sir Galahad the Pure", - "Sir Bedivere the Wise", - "Sir Robin the-not-quite-so-brave-as-Sir-Lancelot", - "Arthur, King of the Britons", - ] - response = self.request( - "motion.json_upload", - { - "data": [ - { - "title": "New", - "text": "motion", - "submitters_username": ["user123", "anotherOne"], - "submitters_verbose": knights, - "supporters_username": ["user123", "anotherOne"], - "supporters_verbose": knights, - }, - ], - "meeting_id": 123, - }, - ) - self.assert_status_code(response, 200) - assert len(response.json["results"][0][0]["rows"]) == 1 - result = response.json["results"][0][0] - assert result["state"] == ImportState.ERROR - row = result["rows"][0] - assert row["state"] == ImportState.ERROR - assert row["messages"] == [ - "Error: Verbose field is set and has more entries than the username field for submitters", - "Error: Verbose field is set and has more entries than the username field for supporters", - ] - assert row["data"]["submitters_username"] == [ - {"info": ImportState.ERROR, "value": "user123"}, - {"info": ImportState.ERROR, "value": "anotherOne"}, - ] - assert row["data"]["supporters_username"] == [ - {"info": ImportState.ERROR, "value": "user123"}, - {"info": ImportState.ERROR, "value": "anotherOne"}, - ] - - -class MotionJsonUploadForUseInImport(BaseMotionJsonUpload): - def json_upload_amendment(self) -> None: - self.create_meeting(42) - response = self.request( - "motion.json_upload", - { - "data": [{"title": "test", "text": "my", "motion_amendment": "1"}], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert len(response.json["results"][0][0]["rows"]) == 1 - assert response.json["results"][0][0]["state"] == ImportState.WARNING - data = { - "meeting_id": 42, - "title": {"value": "test", "info": ImportState.DONE}, - "text": {"value": "

my

", "info": ImportState.DONE}, - "motion_amendment": {"value": True, "info": ImportState.WARNING}, - "submitters_username": [{"id": 1, "info": "generated", "value": "admin"}], - } - expected = { - "state": ImportState.NEW, - "messages": ["Amendments cannot be correctly imported"], - "data": data, - } - assert response.json["results"][0][0]["rows"][0] == expected - - def json_upload_create_missing_title(self) -> None: - self.setup_meeting_with_settings(42) - response = self.request( - "motion.json_upload", - { - "data": [{"text": "my", "reason": "stuff"}], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.ERROR - assert ( - "Error: Title is required" - in response.json["results"][0][0]["rows"][0]["messages"] - ) - - assert response.json["results"][0][0]["rows"][0]["data"]["title"] == { - "value": "", - "info": ImportState.ERROR, - } - - def json_upload_update_missing_title(self) -> None: - self.setup_meeting_with_settings(42) - response = self.request( - "motion.json_upload", - { - "data": [{"text": "my", "reason": "stuff", "number": "NUM01"}], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.DONE - assert response.json["results"][0][0]["rows"][0]["messages"] == [] - assert "title" not in response.json["results"][0][0]["rows"][0]["data"] - - def json_upload_update_missing_reason_although_required(self) -> None: - self.setup_meeting_with_settings(42, is_reason_required=True) - response = self.request( - "motion.json_upload", - { - "data": [{"title": "test", "text": "my", "number": "NUM01"}], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.DONE - assert response.json["results"][0][0]["rows"][0]["messages"] == [] - assert "reason" not in response.json["results"][0][0]["rows"][0]["data"] - - def json_upload_multi_row(self) -> None: - self.setup_meeting_with_settings(1, is_set_number=True) - self.set_user_groups(1, [1]) - self.create_user("anotherUser", [1]) - self.set_models( - { - "meeting/1": {"tag_ids": [1], "motion_block_ids": [1]}, - "tag/1": {"meeting_id": 1, "name": "Tag1"}, - "motion_block/1": {"meeting_id": 1, "title": "Block1"}, - } - ) - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "NUM01", - "title": "first", - "text": "test my stuff", - "supporters_username": ["user1"], - }, - { - "number": "NUM02", - "title": "then", - "reason": "test my other stuff", - "category_name": "Other motion", - "submitters_username": ["user1"], - "submitters_verbose": ["Lancelot the brave"], - }, - { - "number": "NUM03", - "title": "also", - "text": "test the other peoples stuff", - "tags": ["Tag1"], - }, - { - "title": "after that", - "text": "test even more stuff", - "supporters_username": ["user1", "admin", "anotherUser"], - "block": "Block1", - }, - { - "title": "finally", - "text": "finish testing", - "category_name": "Other motion", - "category_prefix": "OTHER", - }, - ], - "meeting_id": 1, - }, - ) - self.assert_status_code(response, 200) - rows = response.json["results"][0][0]["rows"] - simple_payload_addition = { - "meeting_id": 1, - "submitters_username": [ - {"id": 1, "info": ImportState.GENERATED, "value": "admin"} - ], - } - assert len(rows) == 5 - row = rows[0] - assert row["state"] == ImportState.DONE - assert row["messages"] == [] - assert row["data"] == { - **simple_payload_addition, - "id": 1, - "number": {"value": "NUM01", "info": ImportState.DONE, "id": 1}, - "title": {"info": ImportState.DONE, "value": "first"}, - "text": {"info": ImportState.DONE, "value": "

test my stuff

"}, - "supporters_username": [ - {"id": 2, "info": ImportState.DONE, "value": "user1"} - ], - } - row = rows[1] - assert row["state"] == ImportState.DONE - assert row["messages"] == [] - assert row["data"] == { - **simple_payload_addition, - "id": 100, - "number": {"value": "NUM02", "info": ImportState.DONE, "id": 100}, - "title": {"info": ImportState.DONE, "value": "then"}, - "reason": {"info": ImportState.DONE, "value": "test my other stuff"}, - "category_name": { - "info": ImportState.DONE, - "value": "Other motion", - "id": 1000, - }, - "submitters_username": [ - {"id": 2, "info": ImportState.DONE, "value": "user1"} - ], - "submitters_verbose": ["Lancelot the brave"], - } - row = rows[2] - assert row["state"] == ImportState.NEW - assert row["messages"] == [] - assert row["data"] == { - **simple_payload_addition, - "number": {"value": "NUM03", "info": ImportState.DONE}, - "title": {"info": ImportState.DONE, "value": "also"}, - "text": { - "info": ImportState.DONE, - "value": "

test the other peoples stuff

", - }, - "tags": [{"id": 1, "info": ImportState.DONE, "value": "Tag1"}], - } - row = rows[3] - assert row["state"] == ImportState.NEW - assert row["messages"] == [] - assert row["data"] == { - **simple_payload_addition, - "number": {"value": "03", "info": ImportState.GENERATED}, - "title": {"info": ImportState.DONE, "value": "after that"}, - "text": {"info": ImportState.DONE, "value": "

test even more stuff

"}, - "supporters_username": [ - {"id": 2, "info": ImportState.DONE, "value": "user1"}, - {"id": 1, "info": ImportState.DONE, "value": "admin"}, - {"id": 3, "info": ImportState.DONE, "value": "anotherUser"}, - ], - "block": {"id": 1, "info": ImportState.DONE, "value": "Block1"}, - } - row = rows[4] - assert row["state"] == ImportState.NEW - assert row["messages"] == [] - assert row["data"] == { - **simple_payload_addition, - "number": {"value": "OTHER01", "info": ImportState.GENERATED}, - "title": {"info": ImportState.DONE, "value": "finally"}, - "text": {"info": ImportState.DONE, "value": "

finish testing

"}, - "category_name": { - "info": ImportState.DONE, - "value": "Other motion", - "id": 100, - }, - "category_prefix": "OTHER", - } - - def json_upload_simple_create( - self, - is_reason_required: bool = False, - is_set_number: bool = False, - ) -> None: - self.setup_meeting_with_settings(42, is_reason_required, is_set_number) - response = self.request( - "motion.json_upload", - { - "data": [{"title": "test", "text": "my", "reason": "stuff"}], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert len(response.json["results"][0][0]["rows"]) == 1 - data = { - "meeting_id": 42, - "title": {"value": "test", "info": ImportState.DONE}, - "text": {"value": "

my

", "info": ImportState.DONE}, - "reason": {"value": "stuff", "info": ImportState.DONE}, - "submitters_username": [{"id": 1, "info": "generated", "value": "admin"}], - } - if is_set_number: - data.update({"number": {"info": ImportState.GENERATED, "value": "03"}}) - expected = { - "state": ImportState.NEW, - "messages": [], - "data": data, - } - assert response.json["results"][0][0]["rows"][0] == expected - - def json_upload_simple_update( - self, - is_reason_required: bool = False, - is_set_number: bool = False, - ) -> None: - self.setup_meeting_with_settings(42, is_reason_required, is_set_number) - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "NUM01", - "title": "test", - "text": "my", - "reason": "stuff", - } - ], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert len(response.json["results"][0][0]["rows"]) == 1 - expected = { - "state": ImportState.DONE, - "messages": [], - "data": { - "id": 42, - "meeting_id": 42, - "number": {"id": 42, "value": "NUM01", "info": ImportState.DONE}, - "title": {"value": "test", "info": ImportState.DONE}, - "text": {"value": "

my

", "info": ImportState.DONE}, - "reason": {"value": "stuff", "info": ImportState.DONE}, - "submitters_username": [ - {"id": 1, "info": "generated", "value": "admin"} - ], - }, - } - assert response.json["results"][0][0]["rows"][0] == expected - - def json_upload_update_with_foreign_meeting(self) -> None: - self.setup_meeting_with_settings(42, is_set_number=True) - self.setup_meeting_with_settings(55) - self.create_user("orgaUser") - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "NUM01", - "title": "test", - "text": "my", - "reason": "stuff", - "category_name": "Normal motion", - "category_prefix": "NORM", - "submitters_username": ["user55"], - "supporters_username": ["user42", "nonExistant", "orgaUser"], - } - ], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - assert response.json["results"][0][0]["state"] == ImportState.WARNING - assert response.json["results"][0][0]["headers"] == [ - {"property": "title", "type": "string", "is_object": True}, - {"property": "text", "type": "string", "is_object": True}, - {"property": "number", "type": "string", "is_object": True}, - {"property": "reason", "type": "string", "is_object": True}, - { - "property": "submitters_verbose", - "type": "string", - "is_list": True, - "is_hidden": True, - }, - { - "property": "submitters_username", - "type": "string", - "is_object": True, - "is_list": True, - }, - { - "property": "supporters_verbose", - "type": "string", - "is_list": True, - "is_hidden": True, - }, - { - "property": "supporters_username", - "type": "string", - "is_object": True, - "is_list": True, - }, - {"property": "category_name", "type": "string", "is_object": True}, - {"property": "category_prefix", "type": "string"}, - {"property": "tags", "type": "string", "is_object": True, "is_list": True}, - {"property": "block", "type": "string", "is_object": True}, - { - "property": "motion_amendment", - "type": "boolean", - "is_object": True, - "is_hidden": True, - }, - ] - assert response.json["results"][0][0]["statistics"] == [ - {"name": "total", "value": 1}, - {"name": "created", "value": 0}, - {"name": "updated", "value": 1}, - {"name": "error", "value": 0}, - {"name": "warning", "value": 1}, - ] - assert len(response.json["results"][0][0]["rows"]) == 1 - assert response.json["results"][0][0]["rows"][0]["state"] == ImportState.DONE - messages = response.json["results"][0][0]["rows"][0]["messages"] - assert len(messages) == 2 - assert messages[0] == "Could not find at least one submitter: user55" - assert messages[1].startswith("Could not find at least one supporter:") - assert " nonExistant" in messages[1] - assert " orgaUser" in messages[1] - assert response.json["results"][0][0]["rows"][0]["data"] == { - "number": {"value": "NUM01", "info": ImportState.DONE, "id": 42}, - "title": {"value": "test", "info": ImportState.DONE}, - "text": {"value": "

my

", "info": ImportState.DONE}, - "reason": {"value": "stuff", "info": ImportState.DONE}, - "category_name": { - "value": "Normal motion", - "info": ImportState.DONE, - "id": 42, - }, - "category_prefix": "NORM", - "meeting_id": 42, - "submitters_username": [ - {"value": "user55", "info": ImportState.WARNING}, - {"id": 1, "info": ImportState.GENERATED, "value": "admin"}, - ], - "id": 42, - "supporters_username": [ - {"value": "user42", "info": ImportState.DONE, "id": 2}, - {"value": "nonExistant", "info": ImportState.WARNING}, - {"value": "orgaUser", "info": ImportState.WARNING}, - ], - } - - def json_upload_custom_number_create(self) -> None: - self.assert_custom_number_create() - - def json_upload_custom_number_create_with_set_number(self) -> None: - self.assert_custom_number_create(True) - - def assert_custom_number_create(self, is_set_number: bool = False) -> None: - self.setup_meeting_with_settings(42, is_set_number) - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "Z01", - "title": "test", - "text": "my", - "reason": "stuff", - "category_name": "Other motion", - "category_prefix": "NORM", - } - ], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - rows = response.json["results"][0][0]["rows"] - assert len(rows) == 1 - assert rows[0] == { - "state": ImportState.NEW, - "messages": [], - "data": { - "meeting_id": 42, - "number": {"value": "Z01", "info": ImportState.DONE}, - "title": {"value": "test", "info": ImportState.DONE}, - "text": {"value": "

my

", "info": ImportState.DONE}, - "reason": {"value": "stuff", "info": ImportState.DONE}, - "submitters_username": [ - {"id": 1, "info": "generated", "value": "admin"} - ], - "category_prefix": "NORM", - "category_name": { - "info": ImportState.DONE, - "value": "Other motion", - "id": 420, - }, - }, - } - - def json_upload_with_warnings(self) -> None: - self.setup_meeting_with_settings(10) - self.set_models( - { - "motion_category/100000": { - "name": "Normal motion", - "prefix": "NORM", - "meeting_id": 10, - }, - "meeting/10": {"motion_category_ids": [10, 100, 1000, 10000, 100000]}, - } - ) - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "NUM01", - "title": "Up", - "text": "date", - "category_name": "Unknown", - "category_prefix": "CAT", - "submitters_username": ["user10", "user10"], - }, - { - "title": "New", - "text": "motion", - "category_prefix": "Shouldn't be found", - "supporters_username": ["user10", "user10", "user10", "user10"], - }, - { - "title": "Newer", - "text": "motion", - "category_name": "Normal motion", - "category_prefix": "NORM", - "submitters_username": ["nonExistant"], - "supporters_username": ["nonExistant", "nonExistant"], - }, - ], - "meeting_id": 10, - }, - ) - self.assert_status_code(response, 200) - assert len(response.json["results"][0][0]["rows"]) == 3 - result = response.json["results"][0][0] - assert result["state"] == ImportState.WARNING - row = result["rows"][0] - assert row["state"] == ImportState.DONE - assert sorted(row["messages"]) == sorted( - [ - "Category could not be found", - "At least one submitter has been referenced multiple times: user10", - ] - ) - assert row["data"] == { - "number": {"value": "NUM01", "info": ImportState.DONE, "id": 10}, - "title": {"value": "Up", "info": ImportState.DONE}, - "text": {"value": "

date

", "info": ImportState.DONE}, - "category_name": {"value": "Unknown", "info": ImportState.WARNING}, - "category_prefix": "CAT", - "meeting_id": 10, - "submitters_username": [ - {"value": "user10", "info": ImportState.DONE, "id": 2}, - {"value": "user10", "info": ImportState.WARNING}, - ], - "id": 10, - } - row = result["rows"][1] - assert row["state"] == ImportState.NEW - assert sorted(row["messages"]) == sorted( - [ - "Category could not be found", - "At least one supporter has been referenced multiple times: user10", - ] - ) - assert row["data"] == { - "title": {"value": "New", "info": ImportState.DONE}, - "text": {"value": "

motion

", "info": ImportState.DONE}, - "category_name": {"value": "", "info": ImportState.WARNING}, - "category_prefix": "Shouldn't be found", - "meeting_id": 10, - "submitters_username": [ - {"value": "admin", "info": ImportState.GENERATED, "id": 1} - ], - "supporters_username": [ - {"value": "user10", "info": ImportState.DONE, "id": 2}, - {"value": "user10", "info": ImportState.WARNING}, - {"value": "user10", "info": ImportState.WARNING}, - {"value": "user10", "info": ImportState.WARNING}, - ], - } - row = result["rows"][2] - assert row["state"] == ImportState.NEW - assert sorted(row["messages"]) == sorted( - [ - "Category could not be found", - "Could not find at least one submitter: nonExistant", - "At least one supporter has been referenced multiple times: nonExistant", - "Could not find at least one supporter: nonExistant", - ] - ) - assert row["data"] == { - "title": {"value": "Newer", "info": ImportState.DONE}, - "text": {"value": "

motion

", "info": ImportState.DONE}, - "category_name": {"value": "Normal motion", "info": ImportState.WARNING}, - "category_prefix": "NORM", - "meeting_id": 10, - "submitters_username": [ - {"value": "nonExistant", "info": ImportState.WARNING}, - {"value": "admin", "info": ImportState.GENERATED, "id": 1}, - ], - "supporters_username": [ - {"value": "nonExistant", "info": ImportState.WARNING}, - {"value": "nonExistant", "info": ImportState.WARNING}, - ], - } - - def json_upload_with_non_matching_verbose_users_okay(self) -> None: - self.setup_meeting_with_settings(123) - self.create_user("anotherOne", [123]) - knights = [ - "Sir Lancelot the Brave", - "Sir Galahad the Pure", - "Sir Bedivere the Wise", - "Sir Robin the-not-quite-so-brave-as-Sir-Lancelot", - "Arthur, King of the Britons", - ] - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "NUM01", - "title": "Up", - "text": "date", - "submitters_username": ["user123", "anotherOne"], - "submitters_verbose": [knights[0]], - "supporters_username": ["user123", "anotherOne"], - "supporters_verbose": [knights[0]], - }, - { - "title": "Newer", - "text": "motion", - "submitters_verbose": knights, - "supporters_verbose": knights, - }, - ], - "meeting_id": 123, - }, - ) - self.assert_status_code(response, 200) - assert len(response.json["results"][0][0]["rows"]) == 2 - result = response.json["results"][0][0] - assert result["state"] == ImportState.DONE - row = result["rows"][0] - assert row["state"] == ImportState.DONE - assert row["messages"] == [] - assert row["data"]["submitters_username"] == [ - {"id": 2, "info": ImportState.DONE, "value": "user123"}, - {"id": 3, "info": ImportState.DONE, "value": "anotherOne"}, - ] - assert row["data"]["supporters_username"] == [ - {"id": 2, "info": ImportState.DONE, "value": "user123"}, - {"id": 3, "info": ImportState.DONE, "value": "anotherOne"}, - ] - row = result["rows"][1] - assert row["state"] == ImportState.NEW - assert row["messages"] == [] - assert row["data"]["submitters_username"] == [ - {"value": "admin", "info": ImportState.GENERATED, "id": 1} - ] - assert row["data"]["submitters_verbose"] == knights - assert "supporters_username" not in row["data"] - assert row["data"]["supporters_verbose"] == knights - - def json_upload_with_tags_and_blocks(self) -> None: - self.setup_meeting_with_settings(42) - self.setup_meeting_with_settings(55) - self.set_models( - { - "meeting/42": { - "tag_ids": [1, 2, 3, 4], - "motion_block_ids": [1, 2, 3, 4], - }, - "meeting/55": {"tag_ids": [5, 6], "motion_block_ids": [5, 6]}, - "tag/1": {"name": "Tag-liatelle", "meeting_id": 42}, - "tag/2": {"name": "Tag-you're-it", "meeting_id": 42}, - "tag/3": {"name": "Tag-ether", "meeting_id": 42}, - "tag/4": {"name": "Tag-ether", "meeting_id": 42}, - "tag/5": {"name": "Tag-you're-it", "meeting_id": 55}, - "tag/6": {"name": "Price tag", "meeting_id": 55}, - "motion_block/1": {"title": "Blockolade", "meeting_id": 42}, - "motion_block/2": {"title": "Blockodile", "meeting_id": 42}, - "motion_block/3": {"title": "Block chain", "meeting_id": 42}, - "motion_block/4": {"title": "Block chain", "meeting_id": 42}, - "motion_block/5": {"title": "Blockodile", "meeting_id": 55}, - "motion_block/6": {"title": "Blockoli", "meeting_id": 55}, - } - ) - response = self.request( - "motion.json_upload", - { - "data": [ - { - "number": "NUM01", - "title": "Up", - "text": "date", - "tags": [ - "Tag-liatelle", - "Tag-you're-it", - "Tag-ether", - "Price tag", - "Not a tag", - ], - "block": "Blockolade", - }, - { - "title": "New", - "text": "motion", - "tags": [ - "Tag-liatelle", - "Tag-liatelle", - "Tag-you're-it", - "Tag-you're-it", - ], - "block": "Blockodile", - }, - {"title": "Newer", "text": "motion", "block": "Block chain"}, - {"title": "Newest", "text": "motion", "block": "Blockoli"}, - ], - "meeting_id": 42, - }, - ) - self.assert_status_code(response, 200) - result = response.json["results"][0][0] - assert result["state"] == ImportState.WARNING - assert len(result["rows"]) == 4 - row = result["rows"][0] - assert row["state"] == ImportState.DONE - assert row["messages"][0].startswith("Could not find at least one tag:") - assert " Not a tag" in row["messages"][0] - assert " Price tag" in row["messages"][0] - assert row["messages"][1] == "Found multiple tags with the same name: Tag-ether" - assert row["data"]["tags"] == [ - {"value": "Tag-liatelle", "info": "done", "id": 1}, - {"value": "Tag-you're-it", "info": "done", "id": 2}, - {"value": "Tag-ether", "info": "warning"}, - {"value": "Price tag", "info": "warning"}, - {"value": "Not a tag", "info": "warning"}, - ] - assert row["data"]["block"] == {"value": "Blockolade", "info": "done", "id": 1} - row = result["rows"][1] - assert row["state"] == ImportState.NEW - assert len(row["messages"]) == 1 - assert row["messages"][0].startswith( - "At least one tag has been referenced multiple times:" - ) - assert "Tag-liatelle" in row["messages"][0] - assert "Tag-you're-it" in row["messages"][0] - assert row["data"]["tags"] == [ - {"value": "Tag-liatelle", "info": "done", "id": 1}, - {"value": "Tag-liatelle", "info": ImportState.WARNING}, - {"value": "Tag-you're-it", "info": "done", "id": 2}, - {"value": "Tag-you're-it", "info": ImportState.WARNING}, - ] - assert row["data"]["block"] == {"value": "Blockodile", "info": "done", "id": 2} - row = result["rows"][2] - assert row["state"] == ImportState.NEW - assert row["messages"] == ["Found multiple motion blocks with the same name"] - assert row["data"]["block"] == { - "value": "Block chain", - "info": ImportState.WARNING, - } - row = result["rows"][3] - assert row["state"] == ImportState.NEW - assert row["messages"] == ["Could not find motion block"] - assert row["data"]["block"] == { - "value": "Blockoli", - "info": ImportState.WARNING, - } From 3ef90521b9e04e51883574226a00b2a278847a87 Mon Sep 17 00:00:00 2001 From: luisa-beerboom <101706784+luisa-beerboom@users.noreply.github.com> Date: Tue, 26 Nov 2024 09:33:21 +0100 Subject: [PATCH 14/51] Update debugpy and datastore commit hash (#2738) --- requirements/export_service_commits.sh | 2 +- requirements/partial/requirements_development.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements/export_service_commits.sh b/requirements/export_service_commits.sh index 82a9e372bf..7a2f20256f 100755 --- a/requirements/export_service_commits.sh +++ b/requirements/export_service_commits.sh @@ -1,3 +1,3 @@ #!/bin/bash -export DATASTORE_COMMIT_HASH=827476ebb2f796a2a8ac665d5dbb577e870fc4de +export DATASTORE_COMMIT_HASH=f3ef84416fb3dcf4a949234e6b9b459fdc949d85 export AUTH_COMMIT_HASH=85f5d0e10f7ab455b45f8da73ea7c69ce953e569 diff --git a/requirements/partial/requirements_development.txt b/requirements/partial/requirements_development.txt index 47e94684e5..a602883211 100644 --- a/requirements/partial/requirements_development.txt +++ b/requirements/partial/requirements_development.txt @@ -1,7 +1,7 @@ aiosmtpd==1.4.6 autoflake==2.3.1 black==24.10.0 -debugpy==1.8.8 +debugpy==1.8.9 flake8==7.1.1 isort==5.13.2 mypy==1.13.0 From a2d779e2b0346a565ef376dfd07d8d72a18ada05 Mon Sep 17 00:00:00 2001 From: Hannes Janott Date: Fri, 29 Nov 2024 10:04:03 +0100 Subject: [PATCH 15/51] make database check work again (#2747) --- openslides_backend/presenter/check_database.py | 2 +- openslides_backend/shared/export_helper.py | 6 ++---- tests/system/presenter/test_check_database.py | 2 ++ 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openslides_backend/presenter/check_database.py b/openslides_backend/presenter/check_database.py index 624e9b2821..b051ee690c 100644 --- a/openslides_backend/presenter/check_database.py +++ b/openslides_backend/presenter/check_database.py @@ -41,7 +41,7 @@ def check_meetings( errors: dict[int, str] = {} for meeting_id in meeting_ids: - export = export_meeting(datastore, meeting_id) + export = export_meeting(datastore, meeting_id, True) try: Checker( data=export, diff --git a/openslides_backend/shared/export_helper.py b/openslides_backend/shared/export_helper.py index 17d987ceb5..f2ccb62e39 100644 --- a/openslides_backend/shared/export_helper.py +++ b/openslides_backend/shared/export_helper.py @@ -165,11 +165,9 @@ def add_users( user["is_present_in_meeting_ids"] = [meeting_id] else: user["is_present_in_meeting_ids"] = None - if not internal_target: + if not internal_target and (gender_id := user.pop("gender_id", None)): gender_dict = datastore.get_all("gender", ["name"], lock_result=False) - if user.get("gender_id"): - user["gender"] = gender_dict.get(user["gender_id"], {}).get("name") - del user["gender_id"] + user["gender"] = gender_dict.get(gender_id, {}).get("name") # limit user fields to exported objects collection_field_tupels = [ ("meeting_user", "meeting_user_ids"), diff --git a/tests/system/presenter/test_check_database.py b/tests/system/presenter/test_check_database.py index c4d1d3257c..d7cebc7780 100644 --- a/tests/system/presenter/test_check_database.py +++ b/tests/system/presenter/test_check_database.py @@ -250,6 +250,7 @@ def get_new_user(self, username: str, datapart: dict[str, Any]) -> dict[str, Any } def test_correct_relations(self) -> None: + """Also asserts that the internal flag of meeting export is used for the gender""" self.set_models( { "organization/1": { @@ -340,6 +341,7 @@ def test_correct_relations(self) -> None: "is_physical_person": True, "default_vote_weight": "1.000000", "organization_id": 1, + "gender_id": 2, }, "user/2": self.get_new_user( "present_user", From eb967ae718f7492a8edd4fe0c65504d95f7b191f Mon Sep 17 00:00:00 2001 From: luisa-beerboom <101706784+luisa-beerboom@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:11:07 +0100 Subject: [PATCH 16/51] Translate public group name (#2748) --- .../action/actions/meeting/update.py | 12 ++++++++-- tests/system/action/base.py | 1 + tests/system/action/meeting/test_settings.py | 10 ++++++--- tests/system/action/meeting/test_update.py | 22 +++++++++++++++++++ .../action/test_action_command_format.py | 4 ++++ 5 files changed, 44 insertions(+), 5 deletions(-) diff --git a/openslides_backend/action/actions/meeting/update.py b/openslides_backend/action/actions/meeting/update.py index 27528d0020..edb0848e24 100644 --- a/openslides_backend/action/actions/meeting/update.py +++ b/openslides_backend/action/actions/meeting/update.py @@ -4,6 +4,8 @@ CheckUniqueInContextMixin, ) +from ....i18n.translator import Translator +from ....i18n.translator import translate as _ from ....models.models import Meeting from ....permissions.management_levels import ( CommitteeManagementLevel, @@ -214,9 +216,15 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: set_as_template = instance.pop("set_as_template", None) db_meeting = self.datastore.get( fqid_from_collection_and_id("meeting", instance["id"]), - ["template_for_organization_id", "locked_from_inside", "admin_group_id"], + [ + "template_for_organization_id", + "locked_from_inside", + "admin_group_id", + "language", + ], lock_result=False, ) + Translator.set_translation_language(db_meeting["language"]) lock_meeting = ( instance.get("locked_from_inside") if instance.get("locked_from_inside") is not None @@ -311,7 +319,7 @@ def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]: if instance.get("enable_anonymous") and not anonymous_group_id: group_result = self.execute_other_action( GroupCreate, - [{"name": "Public", "weight": 0, "meeting_id": instance["id"]}], + [{"name": _("Public"), "weight": 0, "meeting_id": instance["id"]}], ) instance["anonymous_group_id"] = anonymous_group_id = cast( list[dict[str, Any]], group_result diff --git a/tests/system/action/base.py b/tests/system/action/base.py index 90a638d7df..cd66fb5136 100644 --- a/tests/system/action/base.py +++ b/tests/system/action/base.py @@ -177,6 +177,7 @@ def create_meeting(self, base: int = 1) -> None: "motions_default_workflow_id": base, "committee_id": committee_id, "is_active_in_organization_id": 1, + "language": "en", }, f"group/{base}": { "meeting_id": base, diff --git a/tests/system/action/meeting/test_settings.py b/tests/system/action/meeting/test_settings.py index 4b5c22b156..1174712428 100644 --- a/tests/system/action/meeting/test_settings.py +++ b/tests/system/action/meeting/test_settings.py @@ -8,6 +8,7 @@ def test_group_ids(self) -> None: "meeting/1": { "motion_poll_default_group_ids": [1], "is_active_in_organization_id": 1, + "language": "en", }, "group/1": {"used_as_motion_poll_default_id": 1}, "group/2": {"name": "2", "used_as_motion_poll_default_id": None}, @@ -29,7 +30,8 @@ def test_group_ids(self) -> None: def test_html_field_iframe(self) -> None: self.create_model( - "meeting/1", {"welcome_text": "Hi", "is_active_in_organization_id": 1} + "meeting/1", + {"welcome_text": "Hi", "is_active_in_organization_id": 1, "language": "en"}, ) response = self.request( "meeting.update", {"id": 1, "welcome_text": '