muxinc / mux-php

Official Mux API wrapper for PHP projects, supporting both Mux Data and Mux Video.
MIT License
40 stars 30 forks source link

Update docs to fix example #66

Closed tim-hanssen closed 2 years ago

tim-hanssen commented 2 years ago

Currently, the example used in the docs is not working if users will copy and paste them.

The updateAsset function requires an array to be set, 'json' is not a valid input.

TypeError: Argument 1 passed to MuxPhp\Models\UpdateAssetRequest::__construct() must be of the type array or null, string given, called in /filename.php on line 74 in file /vendor/muxinc/mux-php/MuxPhp/Models/UpdateAssetRequest.php on line 183.

jaredsmith commented 2 years ago

I'm looking into this -- but because we generate the PHP SDK from our OpenAPI specification, it's not as easy as just updating the docs as is done here in your PR. Thank you for bringing this to our attention, and I'll get back to you as soon as I have a workable solution.

jaredsmith commented 2 years ago

@tim-hanssen what would you think if I changed the documentation for this type of call to look like this:

$asset_id = 'asset_id_example'; // string | The asset ID.

// This API method wants a \MuxPhp\Models\UpdateAssetRequest
// as the second parameter.  That being said, these API docs are
// auto-generated from our OpenAPI specification, which
// gives us the example parameter as a JSON string.  In this example,
// we'll use json_decode() to turn it into an associative array, which
// is compatible with the model.
//
// In your own code you should use an associative array, or
// use a "new \MuxPhp\Models\UpdateAssetRequest" directly.
$update_asset_request = json_decode('{"passthrough":"Example2"}',true); // \MuxPhp\Models\UpdateAssetRequest

try {
    $result = $apiInstance->updateAsset($asset_id, $update_asset_request);
    print_r($result);
} catch (Exception $e) {
    echo 'Exception when calling AssetsApi->updateAsset: ', $e->getMessage(), PHP_EOL;
}

Do you think that's clear enough?

tim-hanssen commented 2 years ago

Hi @jaredsmith this is much better I think! You could also consider putting something in the readme since it's not only this function, but I guess all of them.

jaredsmith commented 2 years ago

@tim-hanssen Would you please mind reviewing #67 and see if that meets your needs, as an alternative to this PR?

jaredsmith commented 2 years ago

I just released version 3.8.0 of this package, based on #67 -- please open a new issue if that needs additional work.