Allow duplicate fields for list updates (#131)

OpenSlides · Jun 24, 2021 · daa548c · daa548c
1 parent a91afed
commit daa548c
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 20 deletions.
diff --git a/writer/tests/system/write/test_invalid_format.py b/writer/tests/system/write/test_invalid_format.py
@@ -119,18 +119,6 @@ def test_update_invalid_field(json_client, data):
     assert_error_response(response, ERROR_CODES.INVALID_FORMAT)
 
 
-def test_list_update_add_remove_duplicate_field(json_client, data):
-    data["events"].append(
-        {
-            "type": "update",
-            "fqid": "a/1",
-            "list_fields": {"add": {"f": [2]}, "remove": {"f": [1]}},
-        }
-    )
-    response = json_client.post(WRITE_URL, data)
-    assert_error_response(response, ERROR_CODES.INVALID_REQUEST)
-
-
 def test_update_list_update_duplicate_field(json_client, data):
     data["events"].append(
         {

diff --git a/writer/tests/system/write/test_write.py b/writer/tests/system/write/test_write.py
@@ -374,6 +374,23 @@ def test_list_update_add_remove(json_client, data, redis_connection, reset_redis
     assert_modified_fields(redis_connection, {"a/1": ["f", "f2"]}, meta_deleted=False)
 
 
+def test_list_update_add_remove_same_field(
+    json_client, data, redis_connection, reset_redis_data
+):
+    data["events"][0]["fields"]["f"] = [1]
+    create_model(json_client, data, redis_connection, reset_redis_data)
+
+    data["events"][0] = {
+        "type": "update",
+        "fqid": "a/1",
+        "list_fields": {"add": {"f": [2]}, "remove": {"f": [1]}},
+    }
+    response = json_client.post(WRITE_URL, data)
+    assert_response_code(response, 201)
+    assert_model("a/1", {"f": [2]}, 2)
+    assert_modified_fields(redis_connection, {"a/1": ["f"]}, meta_deleted=False)
+
+
 def test_update_and_list_update(json_client, data, redis_connection, reset_redis_data):
     data["events"][0]["fields"]["f"] = [1]
     create_model(json_client, data, redis_connection, reset_redis_data)

diff --git a/writer/tests/unit/core/test_write_request.py b/writer/tests/unit/core/test_write_request.py
@@ -104,16 +104,15 @@ def test_update_list_fields():
     assert event.list_fields == {"add": {"test": ["value"]}}
 
 
-def test_update_duplicate_fields_1():
+def test_update_duplicate_fields():
     with pytest.raises(InvalidRequest):
         RequestUpdateEvent("c/1", {"test": "stuff"}, {"add": {"test": ["value"]}})
 
 
 def test_update_duplicate_fields_2():
-    with pytest.raises(InvalidRequest):
-        RequestUpdateEvent(
-            "c/1", {}, {"add": {"test": ["value"]}, "remove": {"test": ["value"]}}
-        )
+    assert RequestUpdateEvent(
+        "c/1", {}, {"add": {"test": ["value"]}, "remove": {"test": ["value"]}}
+    )
 
 
 def test_create_update_wrong_field_format():

diff --git a/writer/writer/core/db_events.py b/writer/writer/core/db_events.py
@@ -107,7 +107,10 @@ def calculate_updated_fields(self, model: Model) -> Tuple[Model, List[str]]:
 
         for field, value in self.remove.items():
             if field in model:
-                db_list = model.get(field)
+                if field in updated_fields:
+                    db_list = updated_fields[field]
+                else:
+                    db_list = model.get(field)
                 updated_list = [el for el in db_list if el not in value]
                 updated_fields[field] = updated_list
             else:

diff --git a/writer/writer/core/write_request.py b/writer/writer/core/write_request.py
@@ -79,8 +79,12 @@ def __init__(
         all_keys = fields_keys.union(add_list_fields_keys, remove_list_fields_keys)
         if len(all_keys) == 0:
             raise InvalidRequest("No fields are given")
-        if len(all_keys) < len(fields) + len(add_list_fields) + len(remove_list_fields):
-            raise InvalidRequest("You cannot give a field name multiple times")
+        if fields_keys.intersection(add_list_fields_keys) or fields_keys.intersection(
+            remove_list_fields_keys
+        ):
+            raise InvalidRequest(
+                "Fields cannot be given both as normal fields and as list updates"
+            )
         for field in all_keys:
             assert_is_field(field)
             assert_no_special_field(field)