Implements 2.0 Contacts Endpoint methods and Conversation Search by lloydliyu · Pull Request #306 · intercom/intercom-php

lloydliyu · 2020-05-27T23:09:02Z

Why?

Brings Contacts and Conversation support in SDK up to 2.0.

Closes #293

How?

Removed IntercomCustomers resource which is no longer accessible.
Added new IntercomContacts resource.
Updated IntercomConversation resource with search method.
Added new nextSearch and nextCursor methods to IntercomClient.
Updated readme.

…sor pagination for contacts.

GabrielAnca

This looks great @lloydliyu! Very happy to see this happening in the PHP SDK! Lots of people will be very happy.

Made a first pass on the code itself and left some feedback 🙌

README.md

GabrielAnca · 2020-05-28T07:18:54Z

README.md

+$client->contacts->getContact("570680a8a1bcbca8a90001b9");
+
+/** Add companies to a contact */
+$client->contacts->create([


Even though technically it's also a call to POST /contacts, I would prefer to have two explicit methods in the IntercomCompanies resource to make this more straightforward, ie $client->companies->attachContact($id) & $client->companies->detachContact($id). I think that would make using this SDK more straightforward, even though they could also do it this way. It would also make the SDK more similar to our API docs 😃

I agree, I believe aligning it as close to the API documentation is ideal, plus the additional ease-of-use. On the API documentation, both a contact_id and id (for the company) are required, so should the method incorporate both parameters? In the example you gave, only $id is specified. (I apologize if I'm missing something, I'm relatively new to this repository)

README.md

src/IntercomClient.php

GabrielAnca · 2020-06-06T12:16:03Z

src/IntercomContacts.php

+     * @return stdClass
+     * @throws Exception
+     */
+    public function getContacts(array $options)


Let's add a default here so we don't need to pass empty arrays when we want to make a full list request 😄

Suggested change

public function getContacts(array $options)

public function getContacts(array $options = [])

GabrielAnca · 2020-06-06T12:23:16Z

src/IntercomContacts.php

+     * @return stdClass
+     * @throws Exception
+     */
+    public function getContact($id, $options = [])


I'm a strongly typing fan 😅

Suggested change

public function getContact($id, $options = [])

public function getContact($id, array $options = [])

GabrielAnca · 2020-06-06T12:24:27Z

src/IntercomContacts.php

+        return $this->client->nextSearchPage($path, $query, $pages);
+    }
+
+    /** Returns next page of a Contacts list.


Suggested change

/** Returns next page of a Contacts list.

/**

* Returns next page of a Contacts list.

GabrielAnca · 2020-06-06T12:25:14Z

src/IntercomContacts.php

+     * Returns next page of Contacts that match search query.
+     *
+     * @see     https://developers.intercom.com/intercom-api-reference/reference#pagination-search
+     * @param   array $options


From here to the end of the file there are some methods where the phpdocs don't match the method params 😃

GabrielAnca · 2020-06-06T12:27:57Z

src/IntercomClient.php

+                "starting_after" => $pages->next->starting_after,
+            ]
+        ];
+        $response = $this->sendRequest('POST', "https://api.intercom.io/$path", $options);


In order to avoid hardcoding the URL in multiple places, why don't we call $this->post here?

Suggested change

$response = $this->sendRequest('POST', "https://api.intercom.io/$path", $options);

$response = $this->post($path, $options);

Same for the next method

src/IntercomContacts.php

RaneLafraze

I started to explore this SDK repository today and was disappointed to see it has fallen behind in terms of compatibility. I plan to invest some time in helping update this repository, and I'm happy to help in whatever way I can!

RaneLafraze · 2020-06-25T16:42:52Z

README.md

+$client->contacts->getContact("570680a8a1bcbca8a90001b9");
+
+/** Add companies to a contact */
+$client->contacts->create([


I agree, I believe aligning it as close to the API documentation is ideal, plus the additional ease-of-use. On the API documentation, both a contact_id and id (for the company) are required, so should the method incorporate both parameters? In the example you gave, only $id is specified. (I apologize if I'm missing something, I'm relatively new to this repository)

src/IntercomClient.php

* Create dedicated attach and detach methods * Use post and get methods instead of sendRequest() * Add default value and update phpdocs * Lint fixes * Change string concatenation for consistency Co-authored-by: Gabriel Anca Corral <GabrielAnca@users.noreply.github.com> * Add companyId parameter and add types * Add method to generate detach contact request path * Add typing for parameter Co-authored-by: Gabriel Anca Corral <GabrielAnca@users.noreply.github.com> Co-authored-by: Gabriel Anca Corral <GabrielAnca@users.noreply.github.com>

Co-authored-by: Rane Lafraze <lafrazeRL@gmail.com>

GabrielAnca

Looks great to me, thank you so much for your effort @lloydliyu and @RaneLafraze 🚀 Let's wait for @lloydliyu to have a look at the latest changes and we'll be ready to ship 😄

GabrielAnca · 2020-07-21T08:39:54Z

Got Daniel's approval separately and we are good to go. I'll be merging this PR. Before releasing a new version, I will make a separate PR that will update the README

Daniel Carter added 9 commits May 27, 2020 21:27

Adds 2.0 contact methods

c2726e2

Adds contact methods to readme.

52d2032

Corrects search for contacts

5ee1845

Removes customers endpoint as this is no longer available in the API

7ee8a92

Adds conversation search

d57e9a5

Adds conversation search to Readme.

8f9c30a

Adds in search pagination for contacts and conversations, adds in cur…

3b3b275

…sor pagination for contacts.

Potential fix for tests

d772283

lint fixes

3c2ca35

GabrielAnca reviewed Jun 6, 2020

View reviewed changes

GabrielAnca mentioned this pull request Jun 6, 2020

API 2.0 Support? #293

Closed

RaneLafraze reviewed Jun 25, 2020

View reviewed changes

RaneLafraze mentioned this pull request Jun 26, 2020

Dan.c/2.0 #307

Merged

RaneLafraze and others added 2 commits July 20, 2020 11:35

Apply suggested docs changes

36744cc

Co-authored-by: Rane Lafraze <lafrazeRL@gmail.com>

GabrielAnca mentioned this pull request Jul 20, 2020

Adds missing endpoints #302

Closed

Minor docs and method arguments fixes

eb6a091

GabrielAnca approved these changes Jul 21, 2020

View reviewed changes

GabrielAnca merged commit 0b0f27b into master Jul 21, 2020

GabrielAnca deleted the dan.c/2.0 branch July 21, 2020 08:40

	public function getContacts(array $options)
	public function getContacts(array $options = [])

	public function getContact($id, $options = [])
	public function getContact($id, array $options = [])

	/** Returns next page of a Contacts list.
	/**
	* Returns next page of a Contacts list.

	$response = $this->sendRequest('POST', "https://api.intercom.io/$path", $options);
	$response = $this->post($path, $options);

Conversation

lloydliyu commented May 27, 2020 • edited by GabrielAnca Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why?

How?

Uh oh!

GabrielAnca left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RaneLafraze left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GabrielAnca left a comment

Choose a reason for hiding this comment

Uh oh!

GabrielAnca commented Jul 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lloydliyu commented May 27, 2020 •

edited by GabrielAnca

Loading