braze-php-sdk icon indicating copy to clipboard operation
braze-php-sdk copied to clipboard

Enable setting non-scalar types

Open rickwest opened this issue 1 year ago • 1 comments

🚨 Proposed changes

Please review the guidelines for contributing to this repository.

Afternoon guys.

Great package! We've just started integrating our platform with Braze and this has made life much easier and is a pleasure to work with! All the types are 👌🏻

I've stumbled on a potential issue when instantiating objects using the fromArray method. For example, I'd like to be able to create a TrackRequest as below but it's not currently possible as the fillFromArray method only sets scalar properties.

$attribute1 = UserAttributes::fromArray([]);

$trackRequest = TrackRequest::fromArray([
    'attributes' => [ $attribute1 ]
]);

Is there a strong argument against being able to set a property like this or is it just an oversight? Created a PR for how a potential solution might look. What do you think?

Thanks again for your work on this package!

⚙️ Types of changes

What types of changes does your code introduce? Put an x in the boxes that apply

  • [x] New feature (non-breaking change which adds functionality)
  • [x] Bugfix (non-breaking change which fixes an issue)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Documentation Update (if none of the other choices apply)
  • [ ] Refactor

rickwest avatar Jul 24 '23 15:07 rickwest

Hi Rick,

the fromArray method is there to populate the primitive properties in response objects, and the management of arrays or nested objects is delegated to specific response classes.

So we probably made a mistake putting the method in the BaseObject class, it should have been in the BaseResponse class.

However, since we had a previous request about it, it's probably useful for request objects as well, so we're thinking about removing that check entirely. Perhaps it is better to get an error if the type is wrong, rather than silently ignoring the data. What do you think about that?

In the future we might consider introducing a dependency on a serialization library to deserialize responses (and perhaps support creating requests from an array).

antonioturdo avatar Jul 29 '23 13:07 antonioturdo