Throw GraphQL input exception when provided store is not enabled.

danslo commented 4 years ago

Description (*)

Instead of just checking if the store exists, also check if it is enabled.

Fixed Issues (if relevant)

871

Manual testing scenarios (*)

See original ticket for reproducible steps.

Questions or comments

Added an integration test, maybe it could be done cleaner? Suggestions welcomed.

Contribution checklist (*)

[x] Pull request has a meaningful description of its purpose
[x] All commits are accompanied by meaningful commit messages
[x] All new or changed code is covered with unit/integration tests (if applicable)
[ ] All automated tests passed successfully (all builds are green)

TomashKhamlai commented 4 years ago

During testing of this PR I found 2 problems:

:question: the first problem is not related to your changes I guess. The problem is that GraphQL Playground sends a request on any "keyUp" event so while I edit the Store header letter by letter I recieve error 500 and it is normal, but when I end typing Playground still has troubles with obtaining schema. So I decided to test this PR using bash.
- :x: the second problem is that the error message that I receive when Magento application is in developer mode contains trace. Please look at this and compare with other errors

Steps to reproduce:

Run in the terminal that includes bash, next statement to set the variable
```
graphql='http://{{  YOUR_DOMAIN  }}/graphql';
```
In my case, I run
```
graphql='http://graphqlce.loc/graphql';
```

Run in the terminal that includes bash, next statement to set the function

function send () {
local store=$1;
read -r -d '' query << 'QUERY'
{
category (id: 2) {
available_sort_by
breadcrumbs {
  category_url_key
  category_level
  category_id
  category_name
}
children {
  children_count
}
children_count
created_at
default_sort_by
description
display_mode
filter_price_range
filter_price_range
id
image
include_in_menu
is_anchor
landing_page
level
meta_description
meta_keywords
meta_title
name
path
path_in_store
position
product_count
products {
  items {
    __typename
  }
  page_info {
    current_page
    page_size
    total_pages
  }
  total_count
}
updated_at
url_key
url_path
}
}   
QUERY
query='{"query":"'${query//[[:space:]]/ }'"'};
query=${query//+([[:blank:]])/ };

curl \
    -X POST $graphql \
    -H 'Content-Type: application/json' \
    -H "Store: $store" \
    -d "$query"
}

Send the request using function send and provide the code of the store in a variable quoted by double quotes
```
send "default"
```
Optionally you may pipe the output to prettify JSON like I usually do with json_pp
```
send "default" | json_pp
```
Make the same request to the store(store view) that was created but is disabled. In my case, the code is "testtest"
```
send "testtest" | json_pp
```

Actual result:

{
   "errors" : [
      {
         "extensions" : {
            "category" : "graphql-input"
         },
         "message" : "Requested store is not found.",
         "trace" : [
            {
               "file" : "/var/www/html/graphqlce/app/code/Magento/GraphQl/Controller/HttpRequestProcessor.php",
               "line" : 59,
               "call" : "Magento\\StoreGraphQl\\Controller\\HttpRequestValidator\\StoreValidator::validate(instance of Magento\\Framework\\App\\Request\\Http)"
            },
            {
               "file" : "/var/www/html/graphqlce/app/code/Magento/GraphQl/Controller/GraphQl.php",
               "line" : 143,
               "call" : "Magento\\GraphQl\\Controller\\HttpRequestProcessor::validateRequest(instance of Magento\\Framework\\App\\Request\\Http)"
            },
            {
               "call" : "Magento\\GraphQl\\Controller\\GraphQl::dispatch(instance of Magento\\Framework\\App\\Request\\Http)",
               "line" : 58,
               "file" : "/var/www/html/graphqlce/lib/internal/Magento/Framework/Interception/Interceptor.php"
            },
            {
               "call" : "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::___callParent('dispatch', array(1))",
               "line" : 144,
               "file" : "/var/www/html/graphqlce/lib/internal/Magento/Framework/Interception/Interceptor.php"
            },
            {
               "call" : "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::Magento\\Framework\\Interception\\{closure}(instance of Magento\\Framework\\App\\Request\\Http)",
               "line" : 73,
               "file" : "/var/www/html/graphqlce/app/code/Magento/PageCache/Model/App/FrontController/BuiltinPlugin.php"
            },
            {
               "call" : "Magento\\PageCache\\Model\\App\\FrontController\\BuiltinPlugin::aroundDispatch(instance of Magento\\GraphQl\\Controller\\GraphQl\\Interceptor, instance of Closure, instance of Magento\\Framework\\App\\Request\\Http)",
               "file" : "/var/www/html/graphqlce/lib/internal/Magento/Framework/Interception/Interceptor.php",
               "line" : 141
            },
            {
               "file" : "/var/www/html/graphqlce/lib/internal/Magento/Framework/Interception/Interceptor.php",
               "line" : 159,
               "call" : "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::Magento\\Framework\\Interception\\{closure}(instance of Magento\\Framework\\App\\Request\\Http)"
            },
            {
               "call" : "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::___callPlugins('dispatch', array(1), array(2))",
               "file" : "/var/www/html/graphqlce/generated/code/Magento/GraphQl/Controller/GraphQl/Interceptor.php",
               "line" : 26
            },
            {
               "call" : "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::dispatch(instance of Magento\\Framework\\App\\Request\\Http)",
               "file" : "/var/www/html/graphqlce/lib/internal/Magento/Framework/App/Http.php",
               "line" : 116
            },
            {
               "call" : "Magento\\Framework\\App\\Http::launch()",
               "file" : "/var/www/html/graphqlce/generated/code/Magento/Framework/App/Http/Interceptor.php",
               "line" : 24
            },
            {
               "line" : 260,
               "file" : "/var/www/html/graphqlce/lib/internal/Magento/Framework/App/Bootstrap.php",
               "call" : "Magento\\Framework\\App\\Http\\Interceptor::launch()"
            },
            {
               "file" : "/var/www/html/graphqlce/index.php",
               "line" : 39,
               "call" : "Magento\\Framework\\App\\Bootstrap::run(instance of Magento\\Framework\\App\\Http\\Interceptor)"
            }
         ]
      }
   ]
}

TomashKhamlai commented 4 years ago

LayoutUpdate.php is missing here https://github.com/magento/graphql-ce/tree/e159faf54d0994102c2b0b3cf2e189c0c43c561d/app/code/Magento/Catalog/Model/Category/Attribute/Backend

TomashKhamlai commented 4 years ago

The problem is still the same: "errors" property should be an array that contains:

"message"

"path"
"locations"

"extensions" like this one:

{
"errors": [{
"message": "Could not find a cart with ID \"X\"",
"extensions": {
    "category": "graphql-no-such-entity"
},
"locations": [{
    "line": 1,
    "column": 12
}],
"path": ["applyCouponToCart"]
}],
"data": {
"applyCouponToCart": null
}
}

also "data" is a sibling property.

Here in response we have: :heavy_check_mark: "errors" property with:

:heavy_check_mark: "message"
:heavy_check_mark: "extensions"
:x: "trace"

:x: "data" is not a sibling to error property

{
    "errors": [{
        "message": "Requested store is not enabled.",
        "extensions": {
            "category": "graphql-input"
        },
        "trace": [{
            "file": "\/var\/www\/html\/graphqlce\/app\/code\/Magento\/GraphQl\/Controller\/HttpRequestProcessor.php",
            "line": 59,
            "call": "Magento\\StoreGraphQl\\Controller\\HttpRequestValidator\\StoreValidator::validate(instance of Magento\\Framework\\App\\Request\\Http)"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/app\/code\/Magento\/GraphQl\/Controller\/GraphQl.php",
            "line": 143,
            "call": "Magento\\GraphQl\\Controller\\HttpRequestProcessor::validateRequest(instance of Magento\\Framework\\App\\Request\\Http)"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/lib\/internal\/Magento\/Framework\/Interception\/Interceptor.php",
            "line": 58,
            "call": "Magento\\GraphQl\\Controller\\GraphQl::dispatch(instance of Magento\\Framework\\App\\Request\\Http)"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/lib\/internal\/Magento\/Framework\/Interception\/Interceptor.php",
            "line": 144,
            "call": "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::___callParent('dispatch', array(1))"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/app\/code\/Magento\/PageCache\/Model\/App\/FrontController\/BuiltinPlugin.php",
            "line": 73,
            "call": "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::Magento\\Framework\\Interception\\{closure}(instance of Magento\\Framework\\App\\Request\\Http)"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/lib\/internal\/Magento\/Framework\/Interception\/Interceptor.php",
            "line": 141,
            "call": "Magento\\PageCache\\Model\\App\\FrontController\\BuiltinPlugin::aroundDispatch(instance of Magento\\GraphQl\\Controller\\GraphQl\\Interceptor, instance of Closure, instance of Magento\\Framework\\App\\Request\\Http)"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/lib\/internal\/Magento\/Framework\/Interception\/Interceptor.php",
            "line": 159,
            "call": "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::Magento\\Framework\\Interception\\{closure}(instance of Magento\\Framework\\App\\Request\\Http)"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/generated\/code\/Magento\/GraphQl\/Controller\/GraphQl\/Interceptor.php",
            "line": 26,
            "call": "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::___callPlugins('dispatch', array(1), array(2))"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/lib\/internal\/Magento\/Framework\/App\/Http.php",
            "line": 116,
            "call": "Magento\\GraphQl\\Controller\\GraphQl\\Interceptor::dispatch(instance of Magento\\Framework\\App\\Request\\Http)"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/generated\/code\/Magento\/Framework\/App\/Http\/Interceptor.php",
            "line": 24,
            "call": "Magento\\Framework\\App\\Http::launch()"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/lib\/internal\/Magento\/Framework\/App\/Bootstrap.php",
            "line": 260,
            "call": "Magento\\Framework\\App\\Http\\Interceptor::launch()"
        }, {
            "file": "\/var\/www\/html\/graphqlce\/index.php",
            "line": 39,
            "call": "Magento\\Framework\\App\\Bootstrap::run(instance of Magento\\Framework\\App\\Http\\Interceptor)"
        }]
    }]
}

lenaorobei commented 4 years ago

@magento run Magento Health Index

lenaorobei commented 4 years ago

@magento run Unit Tests

lenaorobei commented 4 years ago

Delivered to 2.3.4-develop. PR will be marked as merged once branches will be synced.

ghost commented 4 years ago

Hi @danslo, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento / graphql-ce