helpscout / helpscout-api-php

PHP Wrapper for the Help Scout API
MIT License
98 stars 62 forks source link

400 bad request while migrating conversation threads #250

Closed abhishekrijal closed 4 years ago

abhishekrijal commented 4 years ago

Current behavior I am trying to migrate the conversation threads however getting bad request error while creating a new conversation. I am trying to use getThreads() to get conversation threads and setThreads() to add threads data to new conversation. Here's my code:

`// GET conversation with the threads $conversationRequest = (new ConversationRequest()) ->withThreads(); $conversationRequest = $conversationRequest->withThreads();

$filters = (new ConversationFilters())
    ->withMailbox($from_mailboxid)
    ->withSortField('createdAt')
    ->withSortOrder('asc')
    ->withStatus('all');

// $second_page = $from_conversations->getPage(2);
$from_conversations = $from_client->conversations()->list($filters, $conversationRequest);

foreach( $from_conversations as $key => $conv ) {
    /**
     * Create Conversation: Customer thread - as if the customer emailed in
     */
    $customer = $conv->getCustomer();
    $thread   = $conv->getThreads();
    $subject  = $conv->getSubject();
    $status   = $conv->getStatus();
    $type     = $conv->getType();

    $conversation = new Conversation();
    $conversation->setSubject( $subject );
    $conversation->setStatus( $status );
    $conversation->setType( $type );
    $conversation->setMailboxId( $to_mailboxid );
    $conversation->setCustomer( $customer );
    $conversation->setThreads( $thread );

    try {
        $conversationId = $to_client->conversations()->create($conversation);
    } catch (\HelpScout\Api\Exception\ValidationErrorException $e) {
        var_dump($e->getError()->getErrors());
    }`

Expected behavior How can I migrate the threads creating a new conversation?

Thank You.

bkuhl commented 4 years ago

Hey Abhishek,

I believe the response will include a reason for the rejection within the response body. Try catching the exception that's being encountered and look at the response body for a description of what's going wrong. If you're not seeing anything in the response body, if you can provide me the companyId and conversationId that's failing I can look into our logs to see what's going on.

Francois-Francois commented 4 years ago

Have the same issue here. Here are some additional information : logRef":"eab45a81-90a1-4f40-8a61-7b7dc0c43981#335375828"

bkuhl commented 4 years ago

@xqluzxwise I just noticed that in the example you provided, you'd need to recreate every entity (Customer, Thread, Conversation, etc.) to effectively "move" the data between accounts.

@rikless It looks like what's happening is the SDK is generating an invalid request body causing the JSON parser to fail when rendering the inbound request. I can't see the full request body in the logs. If you're experiencing this issue locally, would you be able to add some debug code to https://github.com/helpscout/helpscout-api-php/blob/master/src/Http/RestClient.php#L71 which shows the json being generated for your conversation? Here's an example:

    public function createResource(Extractable $entity, string $uri): ?int
    {
        $request = new Request(
            'POST',
            $uri,
            $this->getDefaultHeaders(),
            $this->encodeEntity($entity)
        );

        $response = $this->send($request);

        if ($response->getStatusCode() >= 400) {
            print_r($this->encodeEntity($entity));
        }

        return $response->hasHeader('Resource-ID')
            ? (int) \current($response->getHeader('Resource-ID'))
            : null;
    }
Francois-Francois commented 4 years ago

@bkuhl it's actually handled by Guzzle. Adding the print_r($this->encodeEntity($entity)); section doesn't change anything (maybe because exception is thrown on send().

I've tried to change guzzle to have complete error message but this doesn't give more information :

Client error: `POST https://api.helpscout.net/v2/conversations` resulted in a `400 Bad Request` response: {"logRef":"a40128d1-04d5-4198-89c1-8966d551752a#157438484","message":"Error parsing request body into JSON","_embedded":{"errors":[]},"_links":{"about":{"href":"http://developer.helpscout.net/mailbox-api/overview/errors#invalid-json"}}}

I think @xqluzxwise situation and mine are the same, we are trying to accomplish the same thing, but I dont understand your answer.

Here is my function :

 public function createConversation($mailboxId)
    {
        if ($this->conversationSource instanceof Conversation) {
            $customer = $this->conversationSource->getCustomer();
            $subject  = $this->conversationSource->getSubject();
            $status   = $this->conversationSource->getStatus();
            $type     = $this->conversationSource->getType();

            $conversation = new Conversation();
            $conversation->setSubject( $subject );
            $conversation->setStatus( $status );
            $conversation->setType( $type );
            $conversation->setMailboxId($mailboxId);
            $conversation->setCustomer( $customer );
            $conversation->setThreads( $this->conversationSource->getThreads() );

            try {
                $conversationId = $this->client->conversations()->create($conversation);
            } catch (\HelpScout\Api\Exception\ValidationErrorException $e) {
                var_dump($e->getError()->getErrors());
            }
        }

        throw new ConversationNotDefinedException('You have to set conversationSource property');
    }
bkuhl commented 4 years ago

I'm having trouble reproducing this failure on our end. My thinking with the print_r() suggestion is not to fix the problem, but to get insight into the json output of what is being sent to the API since it's incomplete in our logs. Can you run this script manually and pipe the output to a file so the print_r() output can be captured? Something like php migrate-conversation.php > output.txt? Once you've done this, please don't post the details here as I wouldn't want to expose your Customer's data, please email the attachment to help@helpscout.com with a link to this GitHub thread and it'll get routed to me.

Are you attempting to move Conversations from one account to another? In the code example you provided, the Customer and Threads that go along with that conversation are still attached to the original account and thus can't be attached to the new account. So this code is currently trying to do this:

Current Account         ->      Destination Account
------------------------------------------------
Customer
Conversation            ->      Conversation
Threads
Attachments

To re-iterate, the problem here is that the Destination Account cannot reference another account's Customer, Thread, Attachments, etc. so those need to be recreated as well. So what you'll need to do is in addition to recreate the Conversation you'll need to recreate all related entities as well, so that you end up with:

Current Account         ->      Destination Account
------------------------------------------------
Customer                ->      Customer
Conversation            ->      Conversation
Threads                 ->      Threads
Attachments             ->      Threads
Francois-Francois commented 4 years ago

My thinking with the print_r() suggestion is not to fix the problem, but to get insight into the json output of what is being sent to the API since it's incomplete in our logs

I understand, but Guzzle catch the error before, so it doesn't print anything.

For last part of your answer, thank you to take time to give quality answer. As a feedback, I think it's a bad design. This requires multiple useless call to your API, and a lot of code for basic things, and I think it's making me moving from this project.

bkuhl commented 4 years ago

I understand, but Guzzle catch the error before, so it doesn't print anything.

Yes, you're right. If you move the print_r() to before send() it'll still show output despite the failure:

        if ($response->getStatusCode() >= 400) {
            print_r($this->encodeEntity($entity));
        }

        $response = $this->send($request);

I understand it's painful to move data from one account to another and I'm sorry we don't have a better solution. Unfortunately recreating the entities is essential for security and data integrity.

Francois-Francois commented 4 years ago

Ok but if I load source conversation with all related data, the API should be able to understand that creating a new conversation on the new account, if the object contains all the data, it's to create the data.

Thank you for your help, it was just a feedback ;) Issue can be closed afaik.

bkuhl commented 4 years ago

Alright, feel free to reach out again if we can help with anything!