Repository: saleor/saleor
Pull request: Allow Update/Delete Metadata mutations to affect multiple items
I want to merge this change because...
Allow Metadata Update/Delete mutations affect more than one item.
<!-- Please mention all relevant issue numbers. -->
Closes #5417 

# Impact

* [ ] New migrations
* [x] New/Updated API fields or mutations
* [ ] Deprecated API fields or mutations
* [ ] Removed API types, fields, or mutations

# Pull Request Checklist

<!-- Please keep this section. It will make maintainer's life easier. -->

* [ ] Privileged queries and mutations are guarded by proper permission checks
* [ ] Database queries are optimized and the number of queries is constant
* [ ] Database migration files are up to date
* [ ] The changes are tested
* [ ] GraphQL schema and type definitions are up to date
* [ ] Changes are mentioned in the changelog


Problem statement:
Allow to update/delete multiple metadata items at once
### What I'm trying to achieve
I'm trying to update/delete multiple metadata items. With the current implementation, I need to send multiple mutations, while I could just have one that updates/deletes multiple keys at once e.g. by passing input like `input: [{key: "key-a", value: "a"}, {key: "key-b", value: "b"}]`

### Describe a proposed solution
- [ ] change the signature of `updateMetadata` mutation to:
```
updateMetadata(id: ID!, input: [MetadataInput!]!): UpdateMetadata
```
- [ ] change `updatePrivateMetadata` accordingly

- [ ] change the signature of `deleteMetadata` to:
```
deleteMetadata(id: ID!, keys: [String!]!): DeleteMetadata
```
- [ ] change `deletePrivateMetadata` accordingly

Hints from issue discussion:
@maarcingebala in `test_meta_mutations` we have tests for a single meta item for every model (User, Product, Order, etc). Do we want to have tests with multiple updated/deleted items for every model also?
I think it's fine if we refactor them to use lists with one element, but there need to be some tests that actually check that the logic works for more than one as well.

Commit message: Add entry to CHANGELOG

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c1c1df8fd76..aaec41bd3d4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -89,6 +89,7 @@ All notable, unreleased changes to this project will be documented in this file.
 - Remove unused settings and environment variables - #5420 by @maarcingebala
 - Add product price validation - #5413 by @kswiatek92
 - Add attribute validation to attributeAssign - #5423 by @kswiatek92
+- Add possibility to Update/Delete more than one item in metadata - #5446 by @koradon
 
 ## 2.9.0
 
diff --git a/saleor/graphql/meta/mutations.py b/saleor/graphql/meta/mutations.py
index 5c123c858ce..dabdf45f31d 100644
--- a/saleor/graphql/meta/mutations.py
+++ b/saleor/graphql/meta/mutations.py
@@ -102,7 +102,8 @@ class Meta:
 
     class Arguments:
         id = graphene.ID(description="ID of an object to update.", required=True)
-        input = MetadataInput(
+        input = graphene.List(
+            graphene.NonNull(MetadataInput),
             description="Fields required to update the object's metadata.",
             required=True,
         )
@@ -111,9 +112,9 @@ class Arguments:
     def perform_mutation(cls, root, info, **data):
         instance = cls.get_instance(info, **data)
         if instance:
-            metadata = data.pop("input")
-            item = {metadata.key: metadata.value}
-            instance.store_value_in_metadata(items=item)
+            metadata_list = data.pop("input")
+            items = {data.key: data.value for data in metadata_list}
+            instance.store_value_in_metadata(items=items)
             instance.save(update_fields=["metadata"])
         return cls.success_response(instance)
 
@@ -127,14 +128,19 @@ class Meta:
 
     class Arguments:
         id = graphene.ID(description="ID of an object to update.", required=True)
-        key = graphene.String(description="Metadata key to delete.", required=True)
+        keys = graphene.List(
+            graphene.NonNull(graphene.String),
+            description="Metadata keys to delete.",
+            required=True,
+        )
 
     @classmethod
     def perform_mutation(cls, root, info, **data):
         instance = cls.get_instance(info, **data)
         if instance:
-            metadata_key = data.pop("key")
-            instance.delete_value_from_metadata(metadata_key)
+            metadata_keys = data.pop("keys")
+            for key in metadata_keys:
+                instance.delete_value_from_metadata(key)
             instance.save(update_fields=["metadata"])
         return cls.success_response(instance)
 
@@ -148,7 +154,8 @@ class Meta:
 
     class Arguments:
         id = graphene.ID(description="ID of an object to update.", required=True)
-        input = MetadataInput(
+        input = graphene.List(
+            graphene.NonNull(MetadataInput),
             description=("Fields required to update the object's metadata."),
             required=True,
         )
@@ -157,9 +164,9 @@ class Arguments:
     def perform_mutation(cls, root, info, **data):
         instance = cls.get_instance(info, **data)
         if instance:
-            metadata = data.pop("input")
-            item = {metadata.key: metadata.value}
-            instance.store_value_in_private_metadata(items=item)
+            metadata_list = data.pop("input")
+            items = {data.key: data.value for data in metadata_list}
+            instance.store_value_in_private_metadata(items=items)
             instance.save(update_fields=["private_metadata"])
         return cls.success_response(instance)
 
@@ -173,13 +180,18 @@ class Meta:
 
     class Arguments:
         id = graphene.ID(description="ID of an object to update.", required=True)
-        key = graphene.String(description="Metadata key to delete.", required=True)
+        keys = graphene.List(
+            graphene.NonNull(graphene.String),
+            description="Metadata keys to delete.",
+            required=True,
+        )
 
     @classmethod
     def perform_mutation(cls, root, info, **data):
         instance = cls.get_instance(info, **data)
         if instance:
-            metadata_key = data.pop("key")
-            instance.delete_value_from_private_metadata(metadata_key)
+            metadata_keys = data.pop("keys")
+            for key in metadata_keys:
+                instance.delete_value_from_private_metadata(key)
             instance.save(update_fields=["private_metadata"])
         return cls.success_response(instance)
diff --git a/saleor/graphql/schema.graphql b/saleor/graphql/schema.graphql
index 96bda74bcb1..cd2d71364ef 100644
--- a/saleor/graphql/schema.graphql
+++ b/saleor/graphql/schema.graphql
@@ -2316,10 +2316,10 @@ type Mutation {
   orderUpdateShipping(order: ID!, input: OrderUpdateShippingInput): OrderUpdateShipping
   orderVoid(id: ID!): OrderVoid
   orderBulkCancel(ids: [ID]!, restock: Boolean!): OrderBulkCancel
-  deleteMetadata(id: ID!, key: String!): DeleteMetadata
+  deleteMetadata(id: ID!, keys: [String!]!): DeleteMetadata
   deletePrivateMetadata(id: ID!, key: String!): DeletePrivateMetadata
-  updateMetadata(id: ID!, input: MetadataInput!): UpdateMetadata
-  updatePrivateMetadata(id: ID!, input: MetadataInput!): UpdatePrivateMetadata
+  updateMetadata(id: ID!, input: [MetadataInput!]!): UpdateMetadata
+  updatePrivateMetadata(id: ID!, input: [MetadataInput!]!): UpdatePrivateMetadata
   assignNavigation(menu: ID, navigationType: NavigationType!): AssignNavigation
   menuCreate(input: MenuCreateInput!): MenuCreate
   menuDelete(id: ID!): MenuDelete
diff --git a/tests/api/test_meta_mutations.py b/tests/api/test_meta_mutations.py
index be7c6e1f0b1..daefc936551 100644
--- a/tests/api/test_meta_mutations.py
+++ b/tests/api/test_meta_mutations.py
@@ -11,11 +11,13 @@
 PRIVATE_VALUE = "private_vale"
 
 PUBLIC_KEY = "key"
+PUBLIC_KEY2 = "key2"
 PUBLIC_VALUE = "value"
+PUBLIC_VALUE2 = "value2"
 
 
 UPDATE_PUBLIC_METADATA_MUTATION = """
-mutation UpdatePublicMetadata($id: ID!, $input: MetadataInput!) {
+mutation UpdatePublicMetadata($id: ID!, $input: [MetadataInput!]!) {
     updateMetadata(
         id: $id
         input: $input
@@ -43,7 +45,31 @@ def execute_update_public_metadata_for_item(
 ):
     variables = {
         "id": item_id,
-        "input": {"key": key, "value": value},
+        "input": [{"key": key, "value": value}],
+    }
+
+    response = client.post_graphql(
+        UPDATE_PUBLIC_METADATA_MUTATION % item_type,
+        variables,
+        permissions=[permissions] if permissions else None,
+    )
+    response = get_graphql_content(response)
+    return response
+
+
+def execute_update_public_metadata_for_multiple_items(
+    client,
+    permissions,
+    item_id,
+    item_type,
+    key=PUBLIC_KEY,
+    value=PUBLIC_VALUE,
+    key2=PUBLIC_KEY2,
+    value2=PUBLIC_VALUE2,
+):
+    variables = {
+        "id": item_id,
+        "input": [{"key": key, "value": value}, {"key": key2, "value": value2}],
     }
 
     response = client.post_graphql(
@@ -64,6 +90,26 @@ def item_contains_proper_public_metadata(
     return item.get_value_from_metadata(key) == value
 
 
+def item_contains_multiple_proper_public_metadata(
+    item_from_response,
+    item,
+    item_id,
+    key=PUBLIC_KEY,
+    value=PUBLIC_VALUE,
+    key2=PUBLIC_KEY2,
+    value2=PUBLIC_VALUE2,
+):
+    if item_from_response["id"] != item_id:
+        return False
+    item.refresh_from_db()
+    return all(
+        [
+            item.get_value_from_metadata(key) == value,
+            item.get_value_from_metadata(key2) == value2,
+        ]
+    )
+
+
 def test_add_public_metadata_for_customer_as_staff(
     staff_api_client, permission_manage_users, customer_user
 ):
@@ -98,6 +144,23 @@ def test_add_public_metadata_for_customer_as_service_account(
     )
 
 
+def test_add_multiple_public_metadata_for_customer_as_service_account(
+    service_account_api_client, permission_manage_users, customer_user
+):
+    # given
+    customer_id = graphene.Node.to_global_id("User", customer_user.pk)
+
+    # when
+    response = execute_update_public_metadata_for_multiple_items(
+        service_account_api_client, permission_manage_users, customer_id, "User"
+    )
+
+    # then
+    assert item_contains_multiple_proper_public_metadata(
+        response["data"]["updateMetadata"]["item"], customer_user, customer_id
+    )
+
+
 def test_add_public_metadata_for_other_staff_as_staff(
     staff_api_client, permission_manage_staff, admin_user
 ):
@@ -428,10 +491,10 @@ def test_update_public_metadata_for_item_without_meta(api_client, address):
 
 
 DELETE_PUBLIC_METADATA_MUTATION = """
-mutation DeletePublicMetadata($id: ID!, $key: String!) {
+mutation DeletePublicMetadata($id: ID!, $keys: [String!]!) {
     deleteMetadata(
         id: $id
-        key: $key
+        keys: $keys
     ) {
         metadataErrors{
             field
@@ -456,7 +519,24 @@ def execute_clear_public_metadata_for_item(
 ):
     variables = {
         "id": item_id,
-        "key": key,
+        "keys": [key],
+    }
+
+    response = client.post_graphql(
+        DELETE_PUBLIC_METADATA_MUTATION % item_type,
+        variables,
+        permissions=[permissions] if permissions else None,
+    )
+    response = get_graphql_content(response)
+    return response
+
+
+def execute_clear_public_metadata_for_multiple_items(
+    client, permissions, item_id, item_type, key=PUBLIC_KEY, key2=PUBLIC_KEY2
+):
+    variables = {
+        "id": item_id,
+        "keys": [key, key2],
     }
 
     response = client.post_graphql(
@@ -477,6 +557,26 @@ def item_without_public_metadata(
     return item.get_value_from_metadata(key) != value
 
 
+def item_without_multiple_public_metadata(
+    item_from_response,
+    item,
+    item_id,
+    key=PUBLIC_KEY,
+    value=PUBLIC_VALUE,
+    key2=PUBLIC_KEY2,
+    value2=PUBLIC_VALUE2,
+):
+    if item_from_response["id"] != item_id:
+        return False
+    item.refresh_from_db()
+    return all(
+        [
+            item.get_value_from_metadata(key) != value,
+            item.get_value_from_metadata(key2) != value2,
+        ]
+    )
+
+
 def test_delete_public_metadata_for_customer_as_staff(
     staff_api_client, permission_manage_users, customer_user
 ):
@@ -515,6 +615,27 @@ def test_delete_public_metadata_for_customer_as_service_account(
     )
 
 
+def test_delete_multiple_public_metadata_for_customer_as_service_account(
+    service_account_api_client, permission_manage_users, customer_user
+):
+    # given
+    customer_user.store_value_in_metadata(
+        {PUBLIC_KEY: PUBLIC_VALUE, PUBLIC_KEY2: PUBLIC_VALUE2}
+    )
+    customer_user.save(update_fields=["metadata"])
+    customer_id = graphene.Node.to_global_id("User", customer_user.pk)
+
+    # when
+    response = execute_clear_public_metadata_for_multiple_items(
+        service_account_api_client, permission_manage_users, customer_id, "User"
+    )
+
+    # then
+    assert item_without_multiple_public_metadata(
+        response["data"]["deleteMetadata"]["item"], customer_user, customer_id
+    )
+
+
 def test_delete_public_metadata_for_other_staff_as_staff(
     staff_api_client, permission_manage_staff, admin_user
 ):
@@ -897,7 +1018,7 @@ def test_delete_public_metadata_for_one_key(api_client, checkout):
 
 
 UPDATE_PRIVATE_METADATA_MUTATION = """
-mutation UpdatePrivateMetadata($id: ID!, $input: MetadataInput!) {
+mutation UpdatePrivateMetadata($id: ID!, $input: [MetadataInput!]!) {
     updatePrivateMetadata(
         id: $id
         input: $input
@@ -925,7 +1046,31 @@ def execute_update_private_metadata_for_item(
 ):
     variables = {
         "id": item_id,
-        "input": {"key": key, "value": value},
+        "input": [{"key": key, "value": value}],
+    }
+
+    response = client.post_graphql(
+        UPDATE_PRIVATE_METADATA_MUTATION % item_type,
+        variables,
+        permissions=[permissions] if permissions else None,
+    )
+    response = get_graphql_content(response)
+    return response
+
+
+def execute_update_private_metadata_for_multiple_items(
+    client,
+    permissions,
+    item_id,
+    item_type,
+    key=PUBLIC_KEY,
+    value=PUBLIC_VALUE,
+    key2=PUBLIC_KEY2,
+    value2=PUBLIC_VALUE2,
+):
+    variables = {
+        "id": item_id,
+        "input": [{"key": key, "value": value}, {"key": key2, "value": value2}],
     }
 
     response = client.post_graphql(
@@ -946,6 +1091,26 @@ def item_contains_proper_private_metadata(
     return item.get_value_from_private_metadata(key) == value
 
 
+def item_contains_multiple_proper_private_metadata(
+    item_from_response,
+    item,
+    item_id,
+    key=PUBLIC_KEY,
+    value=PUBLIC_VALUE,
+    key2=PUBLIC_KEY2,
+    value2=PUBLIC_VALUE2,
+):
+    if item_from_response["id"] != item_id:
+        return False
+    item.refresh_from_db()
+    return all(
+        [
+            item.get_value_from_private_metadata(key) == value,
+            item.get_value_from_private_metadata(key2) == value2,
+        ]
+    )
+
+
 def test_add_private_metadata_for_customer_as_staff(
     staff_api_client, permission_manage_users, customer_user
 ):
@@ -980,6 +1145,23 @@ def test_add_private_metadata_for_customer_as_service_account(
     )
 
 
+def test_add_multiple_private_metadata_for_customer_as_service_account(
+    service_account_api_client, permission_manage_users, customer_user
+):
+    # given
+    customer_id = graphene.Node.to_global_id("User", customer_user.pk)
+
+    # when
+    response = execute_update_private_metadata_for_multiple_items(
+        service_account_api_client, permission_manage_users, customer_id, "User"
+    )
+
+    # then
+    assert item_contains_multiple_proper_private_metadata(
+        response["data"]["updatePrivateMetadata"]["item"], customer_user, customer_id
+    )
+
+
 def test_add_private_metadata_for_other_staff_as_staff(
     staff_api_client, permission_manage_staff, admin_user
 ):
@@ -1020,7 +1202,7 @@ def test_add_private_metadata_for_myself_as_customer_no_permission(user_api_clie
     customer = user_api_client.user
     variables = {
         "id": graphene.Node.to_global_id("User", customer.pk),
-        "input": {"key": PRIVATE_KEY, "value": PRIVATE_VALUE},
+        "input": [{"key": PRIVATE_KEY, "value": PRIVATE_VALUE}],
     }
 
     # when
@@ -1037,7 +1219,7 @@ def test_add_private_metadata_for_myself_as_staff(staff_api_client):
     staff = staff_api_client.user
     variables = {
         "id": graphene.Node.to_global_id("User", staff.pk),
-        "input": {"key": PRIVATE_KEY, "value": PRIVATE_VALUE},
+        "input": [{"key": PRIVATE_KEY, "value": PRIVATE_VALUE}],
     }
 
     # when
@@ -1330,10 +1512,10 @@ def test_update_private_metadata_for_item_without_meta(api_client, address):
 
 
 DELETE_PRIVATE_METADATA_MUTATION = """
-mutation DeletePrivateMetadata($id: ID!, $key: String!) {
+mutation DeletePrivateMetadata($id: ID!, $keys: [String!]!) {
     deletePrivateMetadata(
         id: $id
-        key: $key
+        keys: $keys
     ) {
         metadataErrors{
             field
@@ -1358,7 +1540,7 @@ def execute_clear_private_metadata_for_item(
 ):
     variables = {
         "id": item_id,
-        "key": key,
+        "keys": [key],
     }
 
     response = client.post_graphql(
@@ -1370,6 +1552,23 @@ def execute_clear_private_metadata_for_item(
     return response
 
 
+def execute_clear_private_metadata_for_multiple_items(
+    client, permissions, item_id, item_type, key=PUBLIC_KEY, key2=PUBLIC_KEY2
+):
+    variables = {
+        "id": item_id,
+        "keys": [key, key2],
+    }
+
+    response = client.post_graphql(
+        DELETE_PUBLIC_METADATA_MUTATION % item_type,
+        variables,
+        permissions=[permissions] if permissions else None,
+    )
+    response = get_graphql_content(response)
+    return response
+
+
 def item_without_private_metadata(
     item_from_response, item, item_id, key=PRIVATE_KEY, value=PRIVATE_VALUE,
 ):
@@ -1379,6 +1578,26 @@ def item_without_private_metadata(
     return item.get_value_from_private_metadata(key) != value
 
 
+def item_without_multiple_private_metadata(
+    item_from_response,
+    item,
+    item_id,
+    key=PUBLIC_KEY,
+    value=PUBLIC_VALUE,
+    key2=PUBLIC_KEY2,
+    value2=PUBLIC_VALUE2,
+):
+    if item_from_response["id"] != item_id:
+        return False
+    item.refresh_from_db()
+    return all(
+        [
+            item.get_value_from_private_metadata(key) != value,
+            item.get_value_from_private_metadata(key2) != value2,
+        ]
+    )
+
+
 def test_delete_private_metadata_for_customer_as_staff(
     staff_api_client, permission_manage_users, customer_user
 ):
@@ -1417,6 +1636,27 @@ def test_delete_private_metadata_for_customer_as_service_account(
     )
 
 
+def test_delete_multiple_private_metadata_for_customer_as_service_account(
+    service_account_api_client, permission_manage_users, customer_user
+):
+    # given
+    customer_user.store_value_in_metadata(
+        {PUBLIC_KEY: PUBLIC_VALUE, PUBLIC_KEY2: PUBLIC_VALUE2}
+    )
+    customer_user.save(update_fields=["metadata"])
+    customer_id = graphene.Node.to_global_id("User", customer_user.pk)
+
+    # when
+    response = execute_clear_private_metadata_for_multiple_items(
+        service_account_api_client, permission_manage_users, customer_id, "User"
+    )
+
+    # then
+    assert item_without_multiple_private_metadata(
+        response["data"]["deleteMetadata"]["item"], customer_user, customer_id
+    )
+
+
 def test_delete_private_metadata_for_other_staff_as_staff(
     staff_api_client, permission_manage_staff, admin_user
 ):
@@ -1463,7 +1703,7 @@ def test_delete_private_metadata_for_myself_as_customer_no_permission(user_api_c
     customer.save(update_fields=["private_metadata"])
     variables = {
         "id": graphene.Node.to_global_id("User", customer.pk),
-        "key": PRIVATE_KEY,
+        "keys": [PRIVATE_KEY],
     }
 
     # when
@@ -1484,7 +1724,7 @@ def test_delete_private_metadata_for_myself_as_staff_no_permission(
     staff.save(update_fields=["private_metadata"])
     variables = {
         "id": graphene.Node.to_global_id("User", staff.pk),
-        "key": PRIVATE_KEY,
+        "keys": [PRIVATE_KEY],
     }
 
     # when


Code review comments:
diff_hunk: @@ -2316,10 +2316,10 @@ type Mutation {
   orderUpdateShipping(order: ID!, input: OrderUpdateShippingInput): OrderUpdateShipping
   orderVoid(id: ID!): OrderVoid
   orderBulkCancel(ids: [ID]!, restock: Boolean!): OrderBulkCancel
-  deleteMetadata(id: ID!, key: String!): DeleteMetadata
+  deleteMetadata(id: ID!, keys: [String!]!): DeleteMetadata
   deletePrivateMetadata(id: ID!, key: String!): DeletePrivateMetadata
path: saleor/graphql/schema.graphql
text: @user1:
The schema is outdated? Here we should also have `keys`.

@user2:
Why Travis doesn't build this PR?


@author:
in delete mutations there was `key` I branch out from updated master branch so no idea

@user2:
@author In Saleor we use pre-commit with some checks. For example check for actual graphql schema. Maby you should install it because these checks get a result faster than Travis.  

@user1:
I think Travis didn't start because of merge conflicts. I've fixed them and now it's failing with outdated schema - it needs to be generated again.

@author:
ow now i see it, looks like I forgot one `s`

@author:
Done