br33f / php-GA4-Measurement-Protocol

PHP GoogleAnalytics4 Measurement Protocol Library
88 stars 19 forks source link

Empty params for event leads to an error #10

Closed roelvanduijnhoven closed 2 years ago

roelvanduijnhoven commented 2 years ago

Given the following piece of code:

$event = new BaseEvent('bla');
//$event->setParamValue('test', 'value'); // Will only work, when this is uncommented.

$request = new \Br33f\Ga4\MeasurementProtocol\Dto\Request\BaseRequest('123');
$request->addEvent($event);

$client->send($request)

With the comment this will lead into this error message from the Analytics API:

Unable to parse Measurement Protocol JSON payload. (events[0]) params: Proto field is not repeating, cannot start list.", "validationCode": "VALUE_INVALID"

And that is because params will be sent as an empty array, instead of an empty object.

You can see that by looking at $request->getExport() for the situation with the comment:

array [
  "client_id" => "123"
  "non_personalized_ads" => false
  "events" => array:1 [
    0 => array [
      "name" => "bla"
      "params" => []
    ]
  ]
]

Versus without:

array [
  "client_id" => "123"
  "non_personalized_ads" => false
  "events" => array [
    0 => array [
      "name" => "bla"
      "params" => array [
        "test" => "value"
      ]
    ]
  ]
]

And that really is due to how json_encode works. We can probably solve this cleanly by working with an ArrayObject internally instead of a plain array. Why? Look at this:

var_dump(json_encode(['map' => []])); // {"map":[]}
var_dump(json_encode(['map' => new ArrayObject()])); // {"map":{}}
roelvanduijnhoven commented 2 years ago

We could change https://github.com/br33f/php-GA4-Measurement-Protocol/blob/master/src/Dto/Event/AbstractEvent.php#L172 to the following to resolve this probably:

'params' => new \ArrayObject($preparedParams)

But: it also depends on the given http library. Maybe it would be better to send JSON to the http client instead. So that we are not dependent on the underlying HTTP adapter.

Would you be open to accept this PR @br33f? If so: I'll prepare one.

br33f commented 2 years ago

@roelvanduijnhoven Sure - please go ahead with the PR

iMusicJJ commented 2 years ago

@br33f Glad this is fixed; Could you push a new release to packagist?

Cheers

br33f commented 2 years ago

@iMusicJJ https://packagist.org/packages/br33f/php-ga4-mp#v0.1.2