Repository: saleor/saleor
Pull request: add remove_shipping_method
I want to merge this change because it allows shipping method to be removed by user after it has been assigned to the checkout.

It fixes #13062

# Impact

* [ ] New migrations
* [ x] New/Updated API fields or mutations
* [ ] Deprecated API fields or mutations
* [ ] Removed API types, fields, or mutations
* [ x] Documentation needs to be updated

# Pull Request Checklist

<!-- Please keep this section. It will make the maintainer's life easier. -->

* [ ] Privileged queries and mutations are either absent or guarded by proper permission checks
* [ ] Database queries are optimized and the number of queries is constant
* [ ] Database migrations are either absent or optimized for zero downtime
* [ ] The changes are covered by test cases


Problem statement:
Allow to remove shipping method
Current state:
Once the Shipping Method is assigned to the checkout, it cannot be removed by the user.

DoD:
make `checkoutShippingMethodUpdate.shippingMethodId` argument nullable
if null is passed as a value, remove the shipping method from the checkout

Hints from issue discussion:

Commit message: Update CHANGELOG.md

Co-authored-by: Marcin Gębala <5421321+maarcingebala@users.noreply.github.com>

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cca5d813e33..cb434297c44 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@ All notable, unreleased changes to this project will be documented in this file.
 # 3.15.0 [Unreleased]
 
 ### Breaking changes
+Shipping methods can be removed by the user after it has been assigned to a checkout; `shippingMethodId` is now a nullable input in the `checkoutShippingMethodUpdate` mutation.  - #13068 by @FremahA
 
 ### GraphQL API
 
diff --git a/saleor/graphql/checkout/mutations/checkout_shipping_method_update.py b/saleor/graphql/checkout/mutations/checkout_shipping_method_update.py
index ac39680f833..7496d8a4532 100644
--- a/saleor/graphql/checkout/mutations/checkout_shipping_method_update.py
+++ b/saleor/graphql/checkout/mutations/checkout_shipping_method_update.py
@@ -47,7 +47,9 @@ class Arguments:
                 f"The ID of the checkout. {DEPRECATED_IN_3X_INPUT} Use `id` instead."
             ),
         )
-        shipping_method_id = graphene.ID(required=True, description="Shipping method.")
+        shipping_method_id = graphene.ID(
+            required=False, default_value=None, description="Shipping method."
+        )
 
     class Meta:
         description = "Updates the shipping method of the checkout."
@@ -77,7 +79,7 @@ def _resolve_delivery_method_type(id_) -> Optional[str]:
         return str_type
 
     @classmethod
-    def perform_mutation(  # type: ignore[override]
+    def perform_mutation(
         cls,
         _root,
         info: ResolveInfo,
@@ -85,7 +87,7 @@ def perform_mutation(  # type: ignore[override]
         *,
         checkout_id=None,
         id=None,
-        shipping_method_id,
+        shipping_method_id=None,
         token=None,
     ):
         checkout = get_checkout(cls, info, checkout_id=checkout_id, token=token, id=id)
@@ -117,6 +119,8 @@ def perform_mutation(  # type: ignore[override]
                     )
                 }
             )
+        if shipping_method_id is None:
+            return cls.remove_shipping_method(checkout, checkout_info, lines, manager)
 
         type_name = cls._resolve_delivery_method_type(shipping_method_id)
 
@@ -239,3 +243,21 @@ def perform_on_external_shipping_method(
         cls.call_event(manager.checkout_updated, checkout)
 
         return CheckoutShippingMethodUpdate(checkout=checkout)
+
+    @classmethod
+    def remove_shipping_method(cls, checkout, checkout_info, lines, manager):
+        checkout.shipping_method = None
+        delete_external_shipping_id(checkout=checkout)
+        invalidate_prices_updated_fields = invalidate_checkout_prices(
+            checkout_info, lines, manager, save=False
+        )
+        checkout.save(
+            update_fields=[
+                "shipping_method",
+            ]
+            + invalidate_prices_updated_fields
+        )
+        get_checkout_metadata(checkout).save()
+
+        cls.call_event(manager.checkout_updated, checkout)
+        return CheckoutShippingMethodUpdate(checkout=checkout)
diff --git a/saleor/graphql/checkout/tests/benchmark/test_checkout_mutations.py b/saleor/graphql/checkout/tests/benchmark/test_checkout_mutations.py
index f390070e389..99c21eec144 100644
--- a/saleor/graphql/checkout/tests/benchmark/test_checkout_mutations.py
+++ b/saleor/graphql/checkout/tests/benchmark/test_checkout_mutations.py
@@ -427,7 +427,7 @@ def test_add_shipping_to_checkout(
         FRAGMENT_CHECKOUT
         + """
             mutation updateCheckoutShippingOptions(
-              $id: ID, $shippingMethodId: ID!
+              $id: ID, $shippingMethodId: ID
             ) {
               checkoutShippingMethodUpdate(
                 id: $id, shippingMethodId: $shippingMethodId
diff --git a/saleor/graphql/checkout/tests/mutations/test_checkout_shipping_method_update.py b/saleor/graphql/checkout/tests/mutations/test_checkout_shipping_method_update.py
index 8f36fb1cbf2..cdcf8fa834b 100644
--- a/saleor/graphql/checkout/tests/mutations/test_checkout_shipping_method_update.py
+++ b/saleor/graphql/checkout/tests/mutations/test_checkout_shipping_method_update.py
@@ -21,7 +21,7 @@
 
 MUTATION_UPDATE_SHIPPING_METHOD = """
     mutation checkoutShippingMethodUpdate(
-            $id: ID, $shippingMethodId: ID!){
+            $id: ID, $shippingMethodId: ID){
         checkoutShippingMethodUpdate(
             id: $id, shippingMethodId: $shippingMethodId) {
             errors {
diff --git a/saleor/graphql/checkout/tests/mutations/test_checkout_shipping_method_update_nullable_shipping_method_id.py b/saleor/graphql/checkout/tests/mutations/test_checkout_shipping_method_update_nullable_shipping_method_id.py
new file mode 100644
index 00000000000..6b662092b87
--- /dev/null
+++ b/saleor/graphql/checkout/tests/mutations/test_checkout_shipping_method_update_nullable_shipping_method_id.py
@@ -0,0 +1,67 @@
+from unittest.mock import patch
+
+import pytest
+
+from ....core.utils import to_global_id_or_none
+from ....tests.utils import get_graphql_content
+
+MUTATION_UPDATE_SHIPPING_METHOD = """
+    mutation checkoutShippingMethodUpdate(
+        $id: ID, $shippingMethodId: ID
+    ) {
+        checkoutShippingMethodUpdate(
+            id: $id, shippingMethodId: $shippingMethodId
+        ) {
+            errors {
+                field
+                message
+                code
+            }
+            checkout {
+                id
+                token
+                shippingMethod {
+                    id
+                }
+            }
+        }
+    }
+"""
+
+
+@pytest.mark.django_db
+@pytest.mark.count_queries(autouse=False)
+@patch(
+    "saleor.graphql.checkout.mutations.checkout_shipping_method_update."
+    "clean_delivery_method"
+)
+def test_checkout_shipping_method_update_nullable_shipping_method_id(
+    mock_clean_delivery_method,
+    staff_api_client,
+    checkout_with_item_and_shipping_method,
+    count_queries,
+):
+    # Set up the initial state of the checkout
+    checkout = checkout_with_item_and_shipping_method
+
+    # Mock the clean_delivery_method function
+    mock_clean_delivery_method.return_value = True
+
+    # Execute the mutation to remove the shipping method from the checkout
+    response = staff_api_client.post_graphql(
+        MUTATION_UPDATE_SHIPPING_METHOD,
+        variables={"id": to_global_id_or_none(checkout), "shippingMethodId": None},
+    )
+
+    # Assert that the mutation was successful
+    data = get_graphql_content(response)["data"]["checkoutShippingMethodUpdate"]
+    errors = data["errors"]
+    assert not errors
+    assert data["checkout"]["shippingMethod"] is None
+
+    # Ensure the shipping method was removed from the checkout
+    checkout.refresh_from_db(fields=["shipping_method"])
+    assert checkout.shipping_method is None
+
+    # Ensure that the clean_delivery_method function was not called
+    mock_clean_delivery_method.assert_not_called()
diff --git a/saleor/graphql/checkout/tests/test_checkout_price_expiration.py b/saleor/graphql/checkout/tests/test_checkout_price_expiration.py
index 8cab31750fe..51d22905039 100644
--- a/saleor/graphql/checkout/tests/test_checkout_price_expiration.py
+++ b/saleor/graphql/checkout/tests/test_checkout_price_expiration.py
@@ -271,7 +271,7 @@ def test_checkout_billing_address_update_invalidate_prices(
 
 
 UPDATE_CHECKOUT_SHIPPING_METHOD = """
-mutation updateCheckoutShippingOptions($token: UUID!, $shippingMethodId: ID!) {
+mutation updateCheckoutShippingOptions($token: UUID!, $shippingMethodId: ID) {
   checkoutShippingMethodUpdate(token: $token, shippingMethodId: $shippingMethodId) {
     errors {
       field
diff --git a/saleor/graphql/schema.graphql b/saleor/graphql/schema.graphql
index 26d95cbc5d6..2f58e591de4 100644
--- a/saleor/graphql/schema.graphql
+++ b/saleor/graphql/schema.graphql
@@ -16361,7 +16361,7 @@ type Mutation {
     id: ID
 
     """Shipping method."""
-    shippingMethodId: ID!
+    shippingMethodId: ID
 
     """
     Checkout token.


Code review comments:
diff_hunk: @@ -0,0 +1,67 @@
+from unittest.mock import patch
+
+import pytest
+
+from ....core.utils import to_global_id_or_none
+from ....tests.utils import get_graphql_content
+
+MUTATION_UPDATE_SHIPPING_METHOD = """
+    mutation checkoutShippingMethodUpdate(
+        $id: ID, $shippingMethodId: ID
+    ) {
+        checkoutShippingMethodUpdate(
+            id: $id, shippingMethodId: $shippingMethodId
+        ) {
+            errors {
+                field
+                message
+                code
+            }
+            checkout {
+                id
+                token
+                shippingMethod {
+                    id
+                }
+            }
+        }
+    }
+"""
+
+
+@pytest.mark.django_db
+@pytest.mark.count_queries(autouse=False)
+@patch(
+    "saleor.graphql.checkout.mutations.checkout_shipping_method_update."
+    "clean_delivery_method"
+)
+def test_checkout_shipping_method_update_nullable_shipping_method_id(
+    mock_clean_delivery_method,
+    staff_api_client,
+    checkout_with_item_and_shipping_method,
+    count_queries,
path: saleor/graphql/checkout/tests/mutations/test_checkout_shipping_method_update_nullable_shipping_method_id.py
text: @user3:
Those lines are required for benchmarks. We should use them outside the `benchmark` module. To avoid mixing unit tests with benchmarks. 
```suggestion
@user1.mark.django_db
@user2(
    "saleor.graphql.checkout.mutations.checkout_shipping_method_update."
    "clean_delivery_method"
)
def test_checkout_shipping_method_update_nullable_shipping_method_id(
    mock_clean_delivery_method,
    staff_api_client,
    checkout_with_item_and_shipping_method,
```