Silentscripter / laravel-prestashop-webservice

Laravel 5 wrapper for Prestashop Web Service Library
MIT License
56 stars 34 forks source link

Update Prestashop Library, to accept JSON output #18

Closed m-lopez-f closed 6 years ago

m-lopez-f commented 6 years ago

In original repository add this pull request: https://github.com/PrestaShop/PrestaShop-webservice-lib/pull/50

Add possibility to returned JSON data on request API to Prestashop.

m-lopez-f commented 6 years ago

In this link Coding Standards from prestashop documentation and i used psr that indicate in this doc. But the rest of code use tab indent. In this case only change on new methods.

m-lopez-f commented 6 years ago

I think that interesting add this option as parameter in config file. What is your opinion?

Silentscripter commented 6 years ago

@m-lopez-f what option are you referring to?

m-lopez-f commented 6 years ago

@Silentscripter to add on config file for example:

return [
    'url' => 'https://tienda2.logicsh.es',
    'token' => 'BYY2D6SLPLV2DDJEJ8936MDMXDV4HZ3Q',
    'debug' => env('APP_DEBUG', true),
    'output' => 'JSON'
];

And remove on request ths option: 'output_format' => 'JSON'

m-lopez-f commented 6 years ago

Yes, i search on documentation when add this option on web service and only i see in 1.6v

Silentscripter commented 6 years ago

The fact that json ouput_format is available only in 1.6 doesn't make the whole library not compatible with older versions as anyone can simply use XML format for older versions. You must also consider that getSchema method always returns an XML since the server response is always XML for the schema and you should then force XML output in the option parameters. Finally JSON response is intended for selecting resources, we should first analyze if it's possible to obtain a JSON output response when using POST, PUT and DELETE methods since it's not documented. I would also say that it seems quite strange to me to post or update a resource always using an XML document (that is required) and than asking for a JSON output for the response, seems bit confusing in my opinion.

m-lopez-f commented 6 years ago

I confusing how work this library. in http://doc.prestashop.com/display/PS16/Cheat-sheet+-+Concepts+outlined+in+this+tutorial they explain that is required and others query params

Silentscripter commented 6 years ago

The output format is not required as it is set to default as XML.

m-lopez-f commented 6 years ago

I know that this query param isn't required, but it's more easy to process data in JSON than XML. And only add possibility to returned data in this format.

Silentscripter commented 6 years ago

I agree with the receiving data in json format is surely better but we can't force people to use it so: 1) The minimum compatibility must be set back to 1.4 and not to 1.6 2) I'll investigate if it's possible to receive also JSON response data when using POST, PUT and DELETE resources in order to verify if it's ok with your pull request and let you know.

m-lopez-f commented 6 years ago

Ok, I will add in this pull request your proposal changes and other ideas, for example a function that generate an url with query params and replace code with this function.