Timeline API

[lukevers]

This PR adds support for the Timeline API!

🎉

[lukevers]

I've noticed some very weird things with the timeline API. Everything in here works, but some of the things that make some of it work are weird (not the code, just the way some of the API endpoints work).

For example:

When updating a property for a timeline event (docs), if you do not provide the original name (which cannot be changed anyways...) in the JSON body request, a 400 Bad Request is returned with the message "Cannot rename a property.". Also, if you do not include propertyType (even if you're not changing it...) you'll get a 500 Internal Server Error.

There are some other things that I've noticed adding support for the Timeline API, and if you're curious about them I can let you know, but I'm contacting someone at HubSpot to discuss the odd things I've found.

[ryanwinchester]

Not sure if I like the DateTime typehinting. If I see the variable name I am going to expect to send an int, but then it's a DateTime. Also, what if i am working with data I got from hubspot and it is already milliseconds?

How about something like:

if (is_numeric($timestamp) && strlen((string)$num) == 10) {
    $timestamp = $timestamp * 1000;
}

[lukevers]

Yeah, good point.

I kept sending seconds before I realized in the docs it said to use milliseconds instead of seconds, and that's what made me decide to go with a DateTime so other people wouldn't make that mistake.

Your solution is nice for any dates before Sat, 20 Nov 2286 17:46:39 GMT (9999999999), but after that date we'll have a problem. (Not sure if anyone will be using this PHP library in the year 2286...haha)

Maybe something more like this:

if ($timestamp instanceof \DateTime) {
    $timestamp = $timestamp->getTimestamp() * 1000;
}

if (isset($timestamp)) {
    $data['json']['timestamp'] = $timestamp;
}

That way the function can support both a DateTime and maybe in the PHPDoc we just mention that it either has to be a DateTime or an already converted to milliseconds timestamp. Could even rename it to be more like this:

/**
 * @param mixed $timestampMilliseconds - Timestamp in milliseconds format or DateTime
 */

Thoughts?

[ryanwinchester]

I think with a variable name of $timestamp people should know that we are expecting an integer timestamp and we don't need to muddy up the code with a bunch of checks.

If it really is a concern, maybe add a timestamp method like this in the Resource base class:

public function timestamp($timestamp)
{
    switch (true) {
        case: $timestamp instanceof \DateTime:
            return $timestamp->getTimestamp() * 1000;
        case: is_numeric($timestamp) && strlen((string)$timestamp) == 10:
            return $timestamp * 1000;
        default:
            return $timestamp;
        }
    }
}

then you can just do:

$data['json']['timestamp'] = $this->timestamp($timestamp);

or maybe call the method millisecondTimestamp()

[lukevers]

I like this solution. It could potentially be useful elsewhere too.

[ryanwinchester]

Also, I don't expect we'll be using a hubspot php library a couple of centuries in the future 😂