participant import error with gender and meeting admin (#2689)

OpenSlides · Oct 16, 2024 · 8889d7c · 8889d7c
1 parent 28fc6c4
commit 8889d7c
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 48 deletions.
diff --git a/openslides_backend/action/actions/user/participant_import.py b/openslides_backend/action/actions/user/participant_import.py
@@ -120,9 +120,7 @@ def validate_entry(self, row: ImportRow) -> None:
         entry["meeting_id"] = self.meeting_id
 
         if isinstance(entry.get("gender"), dict):
-            if entry["gender"].get("info") != ImportState.WARNING:
-                entry["gender_id"] = entry["gender"]["id"]
-            entry.pop("gender")
+            entry["gender_id"] = entry.pop("gender")
 
         if "groups" not in entry:
             raise ActionException(
@@ -199,6 +197,8 @@ def validate_entry(self, row: ImportRow) -> None:
             entry, row["messages"], entry.get("groups", []), row
         )
 
+        if entry.get("gender_id"):
+            entry["gender"] = entry.pop("gender_id")
         entry.pop("meeting_id")
         if row["state"] == ImportState.ERROR and self.import_state == ImportState.DONE:
             self.import_state = ImportState.ERROR

diff --git a/openslides_backend/action/actions/user/participant_json_upload.py b/openslides_backend/action/actions/user/participant_json_upload.py
@@ -103,12 +103,10 @@ def validate_entry(self, entry: dict[str, Any]) -> dict[str, Any]:
         )
 
         payload_index = entry.pop("payload_index", None)
-        # only needed for get_failing_fields not to fail
-        if gender := entry.pop("gender", None):
-            entry["gender_id"] = {}
+        # swapping needed for get_failing_fields and setting import states not to fail
+        if entry.get("gender"):
+            entry["gender_id"] = entry.pop("gender")
         failing_fields = self.permission_check.get_failing_fields(entry)
-        if gender:
-            entry.pop("gender_id")
         entry.pop("group_ids")
         entry.pop("structure_level_ids")
         entry.pop("meeting_id")
@@ -157,8 +155,8 @@ def validate_entry(self, entry: dict[str, Any]) -> dict[str, Any]:
 
         if payload_index:
             entry["payload_index"] = payload_index
-        if gender:
-            entry["gender"] = gender
+        if entry.get("gender_id"):
+            entry["gender"] = entry.pop("gender_id")
 
         return results
 

diff --git a/tests/system/action/user/test_participant_import.py b/tests/system/action/user/test_participant_import.py
@@ -42,6 +42,13 @@ def setUp(self) -> None:
                                 "value": "male",
                                 "info": ImportState.DONE,
                             },
+                            "groups": [
+                                {
+                                    "id": 1,
+                                    "value": "group1",
+                                    "info": ImportState.DONE,
+                                }
+                            ],
                         },
                     },
                 ],
@@ -75,6 +82,8 @@ def setUp(self) -> None:
         )
 
     def test_import_without_any_group_in_import_data(self) -> None:
+        del self.import_preview1_data["result"]["rows"][0]["data"]["groups"]
+        self.update_model("import_preview/1", self.import_preview1_data)
         response = self.request("participant.import", {"id": 1, "import": True})
         self.assert_status_code(response, 400)
         assert (
@@ -100,10 +109,6 @@ def test_import_wrong_invalid_name_in_preview(self) -> None:
         self.assert_model_exists("import_preview/1", {"name": "account"})
 
     def test_import_names_and_email_and_create(self) -> None:
-        self.import_preview1_data["result"]["rows"][0]["data"]["groups"] = [
-            {"info": ImportState.DONE, "value": "group1", "id": 1}
-        ]
-        self.update_model("import_preview/1", self.import_preview1_data)
         response = self.request("participant.import", {"id": 1, "import": True})
         self.assert_status_code(response, 200)
         self.assert_model_exists(
@@ -135,10 +140,6 @@ def test_import_names_and_email_and_create(self) -> None:
         )
 
     def test_import_with_group_created_in_between(self) -> None:
-        self.import_preview1_data["result"]["rows"][0]["data"]["groups"] = [
-            {"info": ImportState.DONE, "value": "group1", "id": 1}
-        ]
-        self.update_model("import_preview/1", self.import_preview1_data)
         self.set_models(
             {
                 "group/123": {"meeting_id": 1, "name": "2Bcreated"},
@@ -195,9 +196,6 @@ def test_import_saml_id_error_new_and_saml_id_exists(self) -> None:
             "value": "testsaml",
             "info": ImportState.NEW,
         }
-        self.import_preview1_data["result"]["rows"][0]["data"]["groups"] = [
-            {"info": ImportState.DONE, "value": "group1", "id": 1}
-        ]
         self.set_models(
             {
                 "user/1": {"saml_id": "testsaml"},
@@ -219,9 +217,6 @@ def test_import_gender_warning(self) -> None:
             "value": "notAGender",
             "info": ImportState.WARNING,
         }
-        self.import_preview1_data["result"]["rows"][0]["data"]["groups"] = [
-            {"info": ImportState.DONE, "value": "group1", "id": 1}
-        ]
         self.import_preview1_data["result"]["rows"][0]["messages"] = [
             "Gender 'notAGender' is not in the allowed gender list."
         ]
@@ -255,9 +250,6 @@ def test_import_error_state_done_missing_user_in_db(self) -> None:
             "id": 111,
         }
         self.import_preview1_data["result"]["rows"][0]["data"]["id"] = 111
-        self.import_preview1_data["result"]["rows"][0]["data"]["groups"] = [
-            {"info": ImportState.DONE, "value": "group1", "id": 1}
-        ]
         self.update_model("import_preview/1", self.import_preview1_data)
         response = self.request("participant.import", {"id": 1, "import": True})
         self.assert_status_code(response, 200)
@@ -278,10 +270,6 @@ def test_import_no_permission(self) -> None:
         self.base_permission_test({}, "participant.import", {"id": 1, "import": True})
 
     def test_import_permission(self) -> None:
-        self.import_preview1_data["result"]["rows"][0]["data"]["groups"] = [
-            {"info": ImportState.DONE, "value": "group1", "id": 1}
-        ]
-        self.update_model("import_preview/1", self.import_preview1_data)
         self.base_permission_test(
             {},
             "participant.import",
@@ -290,10 +278,6 @@ def test_import_permission(self) -> None:
         )
 
     def test_import_permission_2(self) -> None:
-        self.import_preview1_data["result"]["rows"][0]["data"]["groups"] = [
-            {"info": ImportState.DONE, "value": "group1", "id": 1}
-        ]
-        self.update_model("import_preview/1", self.import_preview1_data)
         self.base_permission_test(
             {},
             "participant.import",
@@ -302,6 +286,33 @@ def test_import_permission_2(self) -> None:
             True,
         )
 
+    def test_import_permission_meeting_admin(self) -> None:
+        self.import_preview1_data["result"]["rows"][0]["data"]["id"] = 1
+        self.import_preview1_data["result"]["rows"][0]["state"] = ImportState.DONE
+        self.import_preview1_data["result"]["rows"][0]["data"]["gender"][
+            "info"
+        ] = ImportState.REMOVE
+        self.update_model("import_preview/1", self.import_preview1_data)
+
+        self.create_meeting()
+        self.create_meeting(4)
+        user_id = self.create_user_for_meeting(1)
+        other_user_id = 3
+        self.set_models(
+            {
+                f"user/{other_user_id}": self._get_user_data("jonny", {1: [], 4: []}),
+            }
+        )
+        self.set_user_groups(user_id, [2])
+        self.set_user_groups(other_user_id, [1, 4])
+        self.login(user_id)
+
+        response = self.request("participant.import", {"id": 1, "import": True})
+
+        self.assert_status_code(response, 200)
+        self.assert_model_exists("meeting_user/3", {"user_id": 3, "meeting_id": 4})
+        self.assert_model_not_exists("meeting_user/4")
+
     def test_import_locked_meeting(self) -> None:
         self.base_locked_out_superadmin_permission_test(
             {}, "participant.import", {"id": 1, "import": True}
@@ -734,7 +745,7 @@ def test_json_upload_one_structure_level_newly_created(self) -> None:
                 {"id": created_groups["group4"], "info": "new", "value": "group4"},
             ],
             "structure_level": [{"info": "new", "value": "level up", "id": 2}],
-            "gender_id": 3,
+            "gender": {"id": 3, "info": "done", "value": "diverse"},
         }
 
         row = result["rows"][1]
@@ -780,6 +791,7 @@ def test_json_upload_one_structure_level_newly_created(self) -> None:
                 {"info": ImportState.NEW, "value": "level up", "id": 2},
                 {"info": ImportState.DONE, "value": "no. 5", "id": 1},
             ],
+            "gender": {"info": "warning", "value": "unknown"},
         }
 
         self.assert_model_exists("structure_level/2", {"name": "level up"})
@@ -847,7 +859,7 @@ def test_json_upload_update_multiple_users_all_error(self) -> None:
                 {"info": "new", "value": "group4"},
             ],
             "structure_level": [{"info": "new", "value": "level up"}],
-            "gender_id": 3,
+            "gender": {"id": 3, "info": "done", "value": "diverse"},
         }
 
         row = result["rows"][1]
@@ -898,6 +910,7 @@ def test_json_upload_update_multiple_users_all_error(self) -> None:
                 {"info": ImportState.NEW, "value": "level up"},
                 {"info": ImportState.NEW, "value": "no. 5"},
             ],
+            "gender": {"info": "warning", "value": "unknown"},
         }
 
         row = result["rows"][4]

diff --git a/tests/system/action/user/test_participant_json_upload.py b/tests/system/action/user/test_participant_json_upload.py
@@ -9,6 +9,7 @@
 
 class ParticipantJsonUpload(BaseActionTestCase):
     def setUp(self) -> None:
+        self.maxDiff = None
         super().setUp()
         self.set_models(
             {
@@ -365,10 +366,18 @@ def test_json_upload_permission_2(self) -> None:
             True,
         )
 
-    def test_json_upload_permission_meeting_admin(self) -> None:
+    def test_json_upload_no_permission_meeting_admin(self) -> None:
         self.create_meeting()
+        self.create_meeting(4)
         user_id = self.create_user_for_meeting(1)
+        other_user_id = 3
+        self.set_models(
+            {
+                f"user/{other_user_id}": self._get_user_data("test", {1: [], 4: []}),
+            }
+        )
         self.set_user_groups(user_id, [2])
+        self.set_user_groups(other_user_id, [1, 4])
         self.login(user_id)
         response = self.request(
             "participant.json_upload",
@@ -389,28 +398,32 @@ def test_json_upload_permission_meeting_admin(self) -> None:
                     "meeting_id": 1,
                     "rows": [
                         {
-                            "state": ImportState.NEW,
-                            "messages": [],
+                            "state": ImportState.DONE,
+                            "messages": [
+                                "Following fields were removed from payload, because the user has no permissions to change them: username, gender_id, default_password"
+                            ],
                             "data": {
                                 "username": {
                                     "value": "test",
-                                    "info": ImportState.DONE,
+                                    "info": ImportState.REMOVE,
+                                    "id": 3,
+                                },
+                                "default_password": {
+                                    "value": "secret",
+                                    "info": ImportState.REMOVE,
                                 },
+                                "id": 3,
                                 "groups": [
                                     {
-                                        "id": 1,
                                         "info": ImportState.GENERATED,
                                         "value": "group1",
+                                        "id": 1,
                                     }
                                 ],
                                 "gender": {
-                                    "id": 1,
-                                    "info": ImportState.DONE,
+                                    "info": ImportState.REMOVE,
                                     "value": "male",
-                                },
-                                "default_password": {
-                                    "value": "secret",
-                                    "info": ImportState.DONE,
+                                    "id": 1,
                                 },
                             },
                         }