storyblok / storyblok-php-client

Storyblok - PHP Client
https://www.storyblok.com
32 stars 37 forks source link

`token`, `version` and `cv` ignored for API requests #109

Open Bl00D4NGEL opened 3 weeks ago

Bl00D4NGEL commented 3 weeks ago

When using the PHP Storyblok client one can pass in any of the documented parameters from the documentation. When using methods like getStories the client is calling the "Get multiple stories" endpoint described here which is expected. The documentation lists token, version and cv as possible query parameters (they're currently the first three parameters listed). Passing these parameters in however results in them being ignored/overwritten by the internal logic of the function.

This is caused by this array_merge call:

$options = array_merge($options, $this->getApiParameters());

this causes the result of $this->getApiParameters() to overwrite the values in $options which is unexpected (and probably unintended behaviour).

Switching the argument order in the array_merge call fixes this problem and makes the client respect any overwrites to token, version and/or cv.

Expected Behavior

Passing in either of token, version or cv in the $options array of for example Client::getStories() is not overwritten by internal parameters

Current Behavior

When passing in either of token, version or cv in the $options array of for example Client::getStories() the values are overwritten by internal parameters.

This happens for any function that calls the API and has the above described array_merge function parameter order.

Steps to Reproduce


$client = new Client('<your token here');
// The story "version" returned depends on whether `$_GET['_storyblok']` is set when constructing the client since it sets `Client::editModeEnabled` to it's value.
// See \Storyblok\Client::__construct and \Storyblok\Client::getVersion
// Passing in `'version' => 'draft'` here makes no difference, only manually calling `editMode` with either `true` or `false` changes the version
$stories = $client->getStories([
    'version' => 'draft',
]);
roberto-butti commented 2 weeks ago

Thank you, @Bl00D4NGEL, for the detailed explanation. For historical reasons, these parameters were considered specific in initializing the main Storyblok Client class. However, the moment the methods responsible for making the API calls offer the possibility of defining specific options, which means it's possible to define those three parameters as well, it makes sense to set the priority you suggest.

I will perform some tests and write some test cases to check this scenario. I'm going to check with @alvarosabu about the process for merging the PR on this repository, and eventually, I can open a PR.