jolicode / slack-php-api

:hash: PHP Slack Client based on the official OpenAPI specification
https://jolicode.github.io/slack-php-api/
MIT License
222 stars 56 forks source link

Change thread_ts for endpoint chat.postMessage from float to string. #40

Closed dredav closed 5 years ago

dredav commented 5 years ago

Related to #12, #13, #22 and #23:

Sending thread_ts as a float throw the client didn't create a thread for the slack conversation.

Example code:

$client->chatPostMessage([
    'username' => $username,
    'channel' => $channel,
    'text' => $message,
    'thread_ts' => $timestamp,
]);
damienalexandre commented 5 years ago

I tried to send a message in a thread and I do not see any error with using a float. But using a string does throw an error because we expect a float... I know we get the TS as string in some responses so it's not consistent...

Could you tell me what was the error you got? I would like to be sure we are not introducing a blocker by switching all the "ts" as string :fearful:

dredav commented 5 years ago

I tried to send a message in a thread and I do not see any error with using a float. But using a string does throw an error because we expect a float... I know we get the TS as string in some responses so it's not consistent...

Could you tell me what was the error you got? I would like to be sure we are not introducing a blocker by switching all the "ts" as string 😨

Thanks for your response. The API don't return an error. The response is normal as like a normal message was sent. But the thread message don't appear.

My code sample:

var_dump($apiClient->chatPostMessage([
    'username' => 'Test',
    'channel' => 'development',
    'text' => 'First message',
]));

// The message 'First message' appears in the channel.

var_dump($apiClient->chatPostMessage([
    'username' => 'Test',
    'channel' => 'development',
    'text' => 'First response',
    'thread_ts' => /* use the float from response */,
]));

// The message 'First response' don't appears in the channel or a thread.

Year. Set the responsets as a string gives the error

The option "thread_ts" with value "12345..." is expected to be of type "float", but is of type "string"

So I've changed the parameter to a string and regenerate the SDK. I've changed the parameter from float to string in the example and run the code. Now the new message First response appears in a thread.

var_dump($apiClient->chatPostMessage([
    'username' => 'Test',
    'channel' => 'development',
    'text' => 'First response',
    'thread_ts' => /* use the float from response */,
])); 
damienalexandre commented 5 years ago

Finally found the time to address this Pull Request :) Thanks a lot for your time, I found that the patch was missing a part, but in the end, I managed to get a thread reply to a message!

The Slack API is very bad on this subject, I got responses with "ok" and no messages visible in Slack when using floats...

Could you test the master branch on your project? It should work like a charm :)

       /** @var ChatPostMessagePostResponse200 $response */
        $response = $client->chatPostMessage([
            'username' => 'Test',
            'channel' => 'XXX',
            'text' => 'First message',
        ]);

        $response2 = $client->chatPostMessage([
            'username' => 'Test',
            'channel' => 'XXXX',
            'text' => 'First responsesss',
            'thread_ts' => $response->getMessage()->getTs()
        ]);

        $this->assertTrue($response2->getOk());