GraphQl-416: graphql-input provides to Customer as many errors as appeared instead of last one like on Magento Storefront

[XxXgeoXxX]

Description (*)

Issue: #416

Manual testing scenarios (*)

Preconditions (*)

Any supported product with more then 1 required customizable options
GraphQL:

{
  "errors": [
    {
      "message": "The product's required option(s) weren't entered.
Make sure the options are entered and try again.",
      "category": "graphql-input",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "addSimpleProductsToCart"
      ]
    }
  ],
  "data": {
    "addSimpleProductsToCart": null
  }
}

Contribution checklist (*)

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

[XxXgeoXxX]

Graph-ql return originl error string instead add message logic in
app/code/Magento/Checkout/Controller/Cart/Add.php:execute:155

[vpodorozh]

@XxXgeoXxX - any update on this PR?

[XxXgeoXxX]

@vpodorozh
after the discussion - "on Hold"

[naydav]

@XxXgeoXxX @vpodorozh
We can't change the current implementation because it will be BIC.
But what do you think about:

1. Introduce new Exception class like InvalidOptionInput extends LocalizedException. A new exception will take also product SKU and option id.

Something like:

if (count($extendedData) > 0) {
                throw new InvalidOptionInput(__(implode("\n", $results)), $extendedData);
            }

In this case it will not be BIC.

2. Take additional data from exception in GraphQL module and show as list of more detailed messages,.
   Thanks

[vpodorozh]

@XxXgeoXxX @vpodorozh
We can't change the current implementation because it will be BIC.
But what do you think about:

1. Introduce new Exception class like InvalidOptionInput extends LocalizedException. A new exception will take also product SKU and option id.

Something like:

if (count($extendedData) > 0) {
                throw new InvalidOptionInput(__(implode("\n", $results)), $extendedData);
            }

In this case it will not be BIC.

1. Take additional data from exception in GraphQL module and show as list of more detailed messages,.
   Thanks

@naydav - for me it looks like an overengineering. Why don't we make the fix in the Magento core (new PR in core) and cancel current PR/issue?
Do you see the other usages of the proposed solution, other than for invalid options exceptions?

@XxXgeoXxX - what is your opinion?

Thx!

[naydav]

@XxXgeoXxX @vpodorozh
We can't change the current implementation because it will be BIC.
But what do you think about:

1. Introduce new Exception class like InvalidOptionInput extends LocalizedException. A new exception will take also product SKU and option id.

Something like:

if (count($extendedData) > 0) {
                throw new InvalidOptionInput(__(implode("\n", $results)), $extendedData);
            }

In this case it will not be BIC.

1. Take additional data from exception in GraphQL module and show as list of more detailed messages,.
   Thanks

@naydav - for me it looks like an overengineering. Why don't we make the fix in the Magento core (new PR in core) and cancel current PR/issue?
Do you see the other usages of the proposed solution, other than for invalid options exceptions?

@XxXgeoXxX - what is your opinion?

Thx!

@vpodorozh
Of course, it will be overengineering :) but we need to keep backward compatibility.
Changing of exception message is backward incompatible changes. That's why we need to have some additional code.

[vpodorozh]

@XxXgeoXxX - let's do it!
Check the latest comment from @naydav with explanation, contact me in case you need some support.
Thx!

[vpodorozh]

@XxXgeoXxX has committed to pick-up this issue with the proposed solution.

[vpodorozh]

@novikor - please pick up this PR.
I do not have permissions for approval any more. Thx!

[naydav]

@see \Magento\Framework\Exception\AggregateExceptionInterface``

[naydav]

Hello @XxXgeoXxX ,
PR was closed due to inactivity (ticket was unassigned). Please reopen and update if you wish to continue.
Thanks!

[ghost]

Hi @XxXgeoXxX, 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.

[lenaorobei]

Please address @naydav comment about the use of Magento\Framework\Exception\AggregateExceptionInterface.

[lenaorobei]

Closing this PR in favor of #970
Should be fixed globally.

[ghost]

Hi @XxXgeoXxX, 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.