pipedrive / client-php

Pipedrive API client for PHP
MIT License
48 stars 55 forks source link

DealsTimeline mapping error: JsonMapper::map requires first argument to be an object, array given. #52

Closed BradGriffith closed 1 year ago

BradGriffith commented 3 years ago

InvalidArgumentException JsonMapper::map() requires first argument to be an object, array given. in vendor/apimatic/jsonmapper/src/JsonMapper.php:103

image

It looks like the getDealsTimeline API response returns an object with the data member as an array for like this:

object(stdClass)#2247 (2) {
    ["success"]=>
    bool(true)
    ["data"]=>
    array(12) {
      [0]=>
      object(stdClass)#2244 (4) {
        ["period_start"]=>
        string(19) "2021-08-01 00:00:00"
        ["period_end"]=>
        string(19) "2021-08-31 23:59:59"
        ["deals"]=>
        array(9) {
          [0]=>
          object(stdClass)#2246 (69) {
            ["id"]=>
            int(1193)
            ["creator_user_id"]=>
            int(149104)
            ["user_id"]=>
            ...

When jsonmapper tries to map this response in pipedrive/pipedrive/src/Controllers/DealsController.php:459 the data ends up being recognized as an object of type \Pipedrive\Models\Data25 rather than as an array. But in apimapper/jsonmapper/src/JsonMapper.php:103 an InvalidArgumentException is thrown because $json that is passed in is an array rather than an object.

Ultimately, I think JsonMapper should be calling mapArray or mapClassArray around JsonMapper.php:247 but it is instead trying to call map which only works with an object.

I wish I could have determined the root cause here to provide a merge request to fix this, but the best I could come up with is to skip using JsonMapper and to instead just return the object that comes back from the API. This looks like:

--- vendor/pipedrive/pipedrive/src/Controllers/DealsController.php.old
+++ vendor/pipedrive/pipedrive/src/Controllers/DealsController.php
@@ -454,7 +454,7 @@ class DealsController extends BaseContro

         $mapper = $this->getJsonMapper();

-        return $mapper->mapClass($response->body, 'Pipedrive\\Models\\GetDealsTimeline');
+        return $response->body;
     }

That solution neglects whatever value JsonMapper has.

Thanks for your help troubleshooting and fixing this issue!

BradGriffith commented 3 years ago

Note the above is for package version 1.01. I get similar errors in 3.2 but the line numbers are different.

image

InvalidArgumentException JsonMapper::map() requires first argument to be an object, array given.

apimatic/jsonmapper/src/JsonMapper.php:103 apimatic/jsonmapper/src/JsonMapper.php:270 apimatic/jsonmapper/src/JsonMapper.php:322 vendor/pipedrive/pipedrive/src/Controllers/DealsController.php:396

BradGriffith commented 3 years ago

I believe I've found a solution for this that's a very simple change in the GetDealsTimeline model to indicate that the data is an array of Data25 objects, not just a single Data25 object.

--- a/src/Models/GetDealsTimeline.php
+++ b/src/Models/GetDealsTimeline.php
@@ -23,14 +23,14 @@ class GetDealsTimeline implements JsonSerializable
     /**
      * Open and won Deals grouped into periods by defined interval, amount and date-type dealField
      * (field_key)
-     * @var \Pipedrive\Models\Data25|null $data public property
+     * @var \Pipedrive\Models\Data25[]|null $data public property
      */
     public $data;

     /**
      * Constructor to set initial or default values of member properties
      * @param bool   $success Initialization value for $this->success
-     * @param Data25 $data    Initialization value for $this->data
+     * @param Data25[] $data    Initialization value for $this->data
      */
     public function __construct()
     {

I opened a pull request for this change (see above)

SpaceOven commented 1 year ago

Hi! Thanks for the solution! I'm checking open PRs, one of them was related to this issue. The issue was solved on this commit: https://github.com/pipedrive/client-php/commit/7734d2075183c589899d7dd9e04bd07e3fa0a4bc