magento / graphql-ce

[ARCHIVED] Please use magento/magento2 project
https://github.com/magento/magento2
Open Software License 3.0
131 stars 156 forks source link

Notice: Undefined index: country_code #1041

Closed TomashKhamlai closed 5 years ago

TomashKhamlai commented 5 years ago

Preconditions (*)

  1. The customer has an active cart

Steps to reproduce (*)

  1. Set shipping address without country_code parameter
    mutation shipping(
    $cart_id: String!
    ) {
    setShippingAddressesOnCart(
    input: {
      cart_id: $cart_id
      shipping_addresses: {
        address: {
          firstname: "Vero"
          lastname: "Cost"
          city: "LA"
          postcode: "90000"
          street: ["A", "B"]
        }
      }
    }
    ) {
    cart {
      shipping_addresses{
        firstname
        lastname
        city
        postcode
        street
      }
    }
    }
    }

Expected result (*)

  1. Required parameter "country_code" is missing

Actual result (*)

  1. Internal Server Error
    {
    "errors": [
    {
      "debugMessage": "Notice: Undefined index: country_code in /var/www/html/graphql-ce/app/code/Magento/QuoteGraphQl/Model/Cart/QuoteAddressFactory.php on line 64",
      "message": "Internal server error",
      "extensions": {
        "category": "internal"
      },
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "setShippingAddressesOnCart"
      ]
    }
    ],
    "data": {
    "setShippingAddressesOnCart": null
    }
    }
hAmpzter commented 5 years ago

Should the parameter really be required? Seems like row 64 should be if (isset($addressInput['country_code'])) { instead of having country_code required. This is my guess because how the other code handing parameter country_code looks like. I'll create a pull request for the fix the requested way and change if it should be optional.

TomashKhamlai commented 5 years ago

@hAmpzter, yes country_code should be always required. Please look at this image Any field marked with an :asterisk: is required. But be careful, some of the fields are not always required and depends on Magento configuration. Most of the time developers should believe the schema which has the information about required and not required fields. Required fields are marked with an exclamation sign.

input CartAddressInput {
    firstname: String!
    lastname: String!
    company: String
    street: [String!]!
    city: String!
    region: String
    postcode: String
    country_code: String!
    telephone: String!
    save_in_address_book: Boolean
}

In the image, you see that postcode is required, but it is not required for all countries and can be configured as not required field for any country.

lenaorobei commented 5 years ago

Hi @TomashKhamlai. The schema looks following:

input CartAddressInput {
 firstname: String!
 lastname: String!
 company: String
 street: [String!]!
 city: String!
 region: String
 postcode: String
 country_code: String!
 telephone: String!
 save_in_address_book: Boolean
}

country_code is required input parameter and the case that validation does not properly happening shows that it is a framework bug. Such validation should not happen in resolvers and such test cases are redundant.

Closing this PR since the framework bug is assigned to the internal team.