Update intercom-php to latest #2
Conversation
Migrate to Circle2.0
This `$json` parameter passed as a Guzzle `json` request option is intended to be an `array` as referenced in the calling Intercom endpoint classes.
This change introduces a new `archiveUser` method, and updates the `deleteUser` method to alias it. Additionally the change introduces a new `permanentlyDeleteUser` that leverages a `POST` to `user_delete_requests` to initiate a permanent delete from the intercom api. Addresses: #249
Adding PR template
Fix license identifier
…-json Update docblock type for `$json` parameter
Get a single company by ID
Fixes to README
* Fix readme examples styling * Fix code styling issues
* Adding info on delete and archive updates in README * Fix comment block
* Add Intercom\Test namespace to all the tests * Enforce PSR2 by CI * Fix PHPUnit imports * Import PHPUnit_Framework_TestCase
Co-Authored-By: murtron <[email protected]>
Add event metadata example to README
* Upgrade to PHP 7 * Update README.md Co-Authored-By: GabrielAnca <[email protected]>
* Add http-plug and remove guzzle * Move client initialization to the constructor * Allow HTTP Plug 1.0 or 2.0 * Update tests * Clarify readme * Update error handling section in README * Rename setClient to setHttpClient * Minor improvements * Fix exception handling * Change client implementation link in readme
* Add create conversation method * Add test of create conversation method * Add create conversation method's description to README Co-authored-by: mass-min <[email protected]>
update readme
Add FOSSA workflow to enable license scanning
Revert "Add FOSSA workflow to enable license scanning"
Co-authored-by: Dzmitry Chubryk <[email protected]>
WalkthroughLarge overhaul introducing a new PSR-based HTTP core (requests, serialization, pagination, retries), new domain clients (Admins, Articles, Companies, Contacts, Conversations, DataAttributes, Events, DataExport, AiContent, Calls, AwayStatusReasons) with typed requests/responses, major refactor of IntercomClient and legacy resources, README rewrite, tooling/CI migration to GitHub Actions, and config updates/removals. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor DevApp as SDK Consumer
participant Client as *Domain Client* (e.g., ArticlesClient)
participant JsonReq as JsonApiRequest
participant Raw as RawClient
participant HTTP as PSR-18/Guzzle
participant API as Intercom API
participant Deserialize as JsonDeserializer
DevApp->>Client: call list()/create()/find(...request)
Client->>JsonReq: build (baseUrl, path, method, headers, query, body)
Client->>Raw: sendRequest(JsonReq, options)
Raw->>HTTP: PSR-7 Request (with retries)
HTTP->>API: HTTP request
API-->>HTTP: HTTP response
HTTP-->>Raw: ResponseInterface
Raw-->>Client: ResponseInterface
Client->>Deserialize: parse JSON to typed model(s)
Deserialize-->>Client: Model / Pager page
Client-->>DevApp: Typed result (Model/Pager)
rect rgba(200, 230, 255, 0.25)
note over Raw,HTTP: RetryMiddleware handles 5xx/408/429 with exponential backoff
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~180 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (44)
.github/ISSUE_TEMPLATE.md (1)
8-8: Fix indentation for consistency.The indentation of the list item should be at the same level as other items in the list.
Apply this diff to fix the indentation:
- - Intercom API version: +- Intercom API version:🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
8-8: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
tests/IntercomSegmentsTest.php (1)
9-15: Enhance test coverage with realistic test data and endpoint verification.The test could be improved in the following ways:
- Use a realistic JSON response that matches the Intercom API format.
- Verify that the correct endpoint is called.
public function testSegmentList() { - $this->client->method('get')->willReturn('foo'); + $expectedResponse = (object)[ + 'type' => 'segment.list', + 'segments' => [ + (object)['id' => '1', 'name' => 'Active Users'], + (object)['id' => '2', 'name' => 'New Users'] + ] + ]; + $this->client->expects($this->once()) + ->method('get') + ->with('segments') + ->willReturn($expectedResponse); $segments = new IntercomSegments($this->client); - $this->assertSame('foo', $segments->getSegments()); + $this->assertEquals($expectedResponse, $segments->getSegments()); }tests/IntercomMessagesTest.php (1)
9-15: Enhance test coverage with realistic test data and request validation.The test could be improved in the following ways:
- Use a realistic JSON response that matches the Intercom API format.
- Test with valid message data instead of an empty array.
- Verify that the correct endpoint and request data are used.
public function testMessageCreate() { - $this->client->method('post')->willReturn('foo'); + $messageData = [ + 'message_type' => 'email', + 'subject' => 'Welcome', + 'body' => 'Welcome to our platform!', + 'template' => 'plain', + 'from' => ['type' => 'admin', 'id' => '123'], + 'to' => ['type' => 'user', 'id' => '456'] + ]; + $expectedResponse = (object)[ + 'type' => 'message', + 'id' => '789', + 'message_type' => 'email', + 'subject' => 'Welcome' + ]; + $this->client->expects($this->once()) + ->method('post') + ->with('messages', $messageData) + ->willReturn($expectedResponse); $messages = new IntercomMessages($this->client); - $this->assertSame('foo', $messages->create([])); + $this->assertEquals($expectedResponse, $messages->create($messageData)); }tests/IntercomTagsTest.php (2)
9-15: Improve variable naming and test coverage.The test follows a good pattern of mocking the client, but consider these improvements:
- Rename
$tagsto$tagsClientto better reflect its role as a client instance.- Add test cases with actual tag data to validate request payload.
- $tags = new IntercomTags($this->client); - $this->assertSame('foo', $tags->tag([])); + $tagsClient = new IntercomTags($this->client); + $this->assertSame('foo', $tagsClient->tag([ + 'name' => 'example-tag', + 'users' => [['id' => '1'], ['id' => '2']] + ]));
17-23: LGTM! Consider adding error case tests.The test follows a good pattern but could benefit from additional test cases for error scenarios.
Consider adding tests for:
- Invalid responses
- API errors
- Rate limiting scenarios
.circleci/config.yml (2)
3-14: Add caching and test artifacts for improved CI performance.The template configuration is well-structured but could benefit from these CI/CD best practices:
- Cache Composer dependencies
- Store test results as artifacts
steps: - checkout + - restore_cache: + keys: + - composer-v1-{{ checksum "composer.lock" }} + - composer-v1- - run: composer validate --strict - run: composer install --no-interaction --no-suggest --prefer-dist + - save_cache: + key: composer-v1-{{ checksum "composer.lock" }} + paths: + - vendor - run: vendor/bin/phpunit + - run: mkdir -p test-results/phpunit + - run: vendor/bin/phpunit --log-junit test-results/phpunit/junit.xml - run: vendor/bin/phpcs --standard=PSR2 src tests + - store_test_results: + path: test-results + - store_artifacts: + path: test-results
31-38: LGTM! Consider parallel execution for faster builds.The workflow configuration is correct but could be optimized for speed.
Consider removing the implicit sequential execution by adding:
workflows: version: 2 test_on_supported_php_versions: jobs: - test-7.4: filters: &version_filters branches: only: /.*/ - test-7.3: filters: *version_filters - test-7.2: filters: *version_filters - test-7.1: filters: *version_filterstests/IntercomTeamsTest.php (1)
9-15: Add test cases for error handling.The tests follow a good pattern but could benefit from additional test cases:
- Invalid team IDs
- API errors
- Rate limiting scenarios
Example test case for error handling:
public function testTeamsGetWithInvalidId() { $this->expectException(\InvalidArgumentException::class); $teams = new IntercomTeams($this->client); $teams->getTeam('invalid-id'); }Also applies to: 17-23
tests/IntercomAdminsTest.php (1)
9-29: Add test cases for error scenarios and edge cases.The tests follow a good pattern but need more comprehensive coverage.
Consider adding tests for:
- Invalid admin IDs
- API errors
- Rate limiting scenarios
- Empty admin lists
tests/IntercomEventsTest.php (2)
9-15: Enhance test coverage for event creation.The test is overly simplistic. Consider adding test cases that:
- Verify the request parameters passed to the client's
postmethod.- Test error handling scenarios.
- Test with actual event data structure.
public function testEventCreate() { - $this->client->method('post')->willReturn('foo'); + $eventData = [ + 'event_name' => 'test-event', + 'created_at' => time(), + 'email' => '[email protected]' + ]; + + $this->client + ->expects($this->once()) + ->method('post') + ->with('events', $eventData) + ->willReturn((object)['id' => '123']); $users = new IntercomEvents($this->client); - $this->assertSame('foo', $users->create([])); + $result = $users->create($eventData); + $this->assertInstanceOf(stdClass::class, $result); + $this->assertEquals('123', $result->id); }
17-23: Enhance test coverage for events retrieval.Similar to the event creation test, this test should:
- Verify the request parameters passed to the client's
getmethod.- Test error handling scenarios.
- Test with actual response data structure.
public function testEventsGet() { - $this->client->method('get')->willReturn('foo'); + $params = ['type' => 'user', 'email' => '[email protected]']; + $expectedResponse = (object)[ + 'type' => 'user.list', + 'events' => [(object)['event_name' => 'test-event']] + ]; + + $this->client + ->expects($this->once()) + ->method('get') + ->with('events', $params) + ->willReturn($expectedResponse); $users = new IntercomEvents($this->client); - $this->assertSame('foo', $users->getEvents([])); + $result = $users->getEvents($params); + $this->assertInstanceOf(stdClass::class, $result); + $this->assertEquals('user.list', $result->type); + $this->assertCount(1, $result->events); }src/IntercomTeams.php (1)
27-28: Fix inconsistent parameter type documentation.The
$idparameter is documented asintegeringetTeambut asstringinteamPath. Please ensure consistent type documentation across methods.- * @param integer $id + * @param string $idtests/IntercomBulkTest.php (1)
9-24: Enhance test coverage for bulk operations.The tests only verify endpoint paths. Consider enhancing the test coverage:
- Test with actual bulk data structure
- Verify request parameters
- Test error handling scenarios
- Test response handling
public function testBulkUsers() { - $this->client->method('post')->will($this->returnArgument(0)); + $bulkData = [ + 'items' => [ + ['method' => 'post', 'data_type' => 'user', 'data' => ['email' => '[email protected]']], + ['method' => 'delete', 'data_type' => 'user', 'data' => ['email' => '[email protected]']] + ] + ]; + $expectedResponse = (object)['job' => ['id' => 'job_123']]; + + $this->client + ->expects($this->once()) + ->method('post') + ->with('bulk/users', $bulkData) + ->willReturn($expectedResponse); $bulk = new IntercomBulk($this->client); - $this->assertSame('bulk/users', $bulk->users([])); + $result = $bulk->users($bulkData); + $this->assertInstanceOf(stdClass::class, $result); + $this->assertEquals('job_123', $result->job->id); }src/IntercomVisitors.php (1)
15-16: Validate return type and exception usage
The doc block indicates astdClassreturn. Confirm that the HTTP client always returns an object. Also consider specifying a narrower exception type if known.src/IntercomTags.php (1)
15-16: Check alignment of return type
This doc block now specifies astdClassreturn and a genericExceptionthrown. Verify that it correctly reflects the actual response type and exceptions.src/IntercomCounts.php (1)
15-16: Ensure the client consistently returnsstdClass
Verify that thecountsendpoint's response is always parsed into astdClass, including any edge or error cases. Also confirm thatExceptionis the correct thrown exception.src/IntercomMessages.php (1)
15-16: Ensure return type matches what$this->client->post()actually returns.
The docblock now specifiesstdClass. Verify thatpost("messages", $options)indeed returns astdClass, or consider adjusting the return type annotation if needed.tests/IntercomLeadsTest.php (5)
7-15: Basic happy-path coverage.
testLeadCreate()ensures the create method is exercised with a mock return. Consider adding tests for invalid payloads or exception handling to widen coverage.
17-23: Consistent testing pattern for update flow.
Method structure is identical totestLeadCreate(), which aids consistency. Again, consider negative or edge-case tests for thoroughness.
25-31: List retrieval test is straightforward.
testLeadsList()confirms thegetLeads([])call is made correctly. Further data-variation tests could improve confidence.
40-46: Retrieval logic test.
testLeadsGet()stands as a simple pass-through validation. It might be beneficial to include ID or parameter variations.
56-62: Delete method scenario tested.
testLeadsDelete()is consistent with other tests. If error handling is expected, consider adding a failure scenario.tests/IntercomClientTest.php (1)
65-81: Client error handling is properly validated.
testClientErrorHandling()triggers a 404 response. Confirm coverage of other 4xx errors if needed.tests/IntercomContactsTest.php (3)
10-48: Consider adding test cases for error scenarios.While the basic CRUD operations are well-tested, consider adding test cases for:
- Invalid input validation
- Error responses from the client
- Exception handling
Example test case for error handling:
public function testContactCreateWithError() { $this->client->method('post') ->willThrowException(new Exception('API Error')); $this->expectException(Exception::class); $this->expectExceptionMessage('API Error'); $contacts = new IntercomContacts($this->client); $contacts->create([]); }
50-69: Add test case for empty pagination object.Consider adding a test case to verify behavior when the pagination object is empty or missing required properties.
Example test case:
public function testContactNextSearchWithEmptyPagination() { $this->client->method('nextSearchPage')->willReturn('foo'); $pages = new stdClass; // pages->next is not set $contacts = new IntercomContacts($this->client); $this->expectException(\InvalidArgumentException::class); $contacts->nextSearch([], $pages); }
71-81: Add test case for invalid cursor.Consider adding a test case to verify behavior when the cursor is invalid or missing.
Example test case:
public function testConversationNextCursorWithInvalidCursor() { $this->client->method('nextCursorPage')->willReturn('foo'); $pages = new stdClass; $pages->next = new stdClass; $pages->next->starting_after = ""; // Empty cursor $contacts = new IntercomContacts($this->client); $this->expectException(\InvalidArgumentException::class); $contacts->nextCursor($pages); }tests/IntercomConversationsTest.php (3)
22-23: Fix variable naming inconsistency.The variable
$usersshould be named$conversationsto match the class being tested and maintain consistency with other test methods.- $users = new IntercomConversations($this->client); - $this->assertSame('foo', $users->getConversations([])); + $conversations = new IntercomConversations($this->client); + $this->assertSame('foo', $conversations->getConversations([]));
26-30: Add test cases for invalid conversation IDs.Consider adding test cases to verify behavior with empty, null, or invalid conversation IDs.
Example test cases:
public function testConversationPathWithEmptyId() { $conversations = new IntercomConversations($this->client); $this->expectException(\InvalidArgumentException::class); $conversations->conversationPath(""); } public function testConversationReplyPathWithEmptyId() { $conversations = new IntercomConversations($this->client); $this->expectException(\InvalidArgumentException::class); $conversations->conversationReplyPath(""); }Also applies to: 61-65
71-72: Fix variable naming and add error test cases.
- Fix variable naming inconsistency: rename
$usersto$conversations.- Consider adding test cases for error scenarios in reply operation.
Fix naming:
- $users = new IntercomConversations($this->client); - $this->assertSame('foo', $users->replyToConversation("bar", [])); + $conversations = new IntercomConversations($this->client); + $this->assertSame('foo', $conversations->replyToConversation("bar", []));Add error test case:
public function testReplyToConversationWithError() { $this->client->method('post') ->willThrowException(new Exception('API Error')); $this->expectException(Exception::class); $this->expectExceptionMessage('API Error'); $conversations = new IntercomConversations($this->client); $conversations->replyToConversation("bar", []); }src/IntercomLeads.php (1)
28-29: Verify doc block accuracy for update.
The block says “Creates Lead” but the method is clearly an update operation. Suggest adjusting the doc text to avoid confusion.- * Creates Lead. + * Updates an existing Lead or Contact.src/IntercomContacts.php (2)
10-21: Method create(array $options).
Correctly callsPOST /contacts. Consider documenting required fields in$optionsfor developer clarity.
66-79: Typo in docblock: "Permenently" → "Permanently".
Recommend fixing the spelling in the docblock to maintain professional documentation.- * Permenently Deletes a single Contact + * Permanently Deletes a single Contactsrc/IntercomUsers.php (3)
31-31: Reuse of create method.
Delegatingupdatetocreateis acceptable, but consider clarifying in docs that they are aliases.
80-84: Annotation for archiving a user.
The doc block references an archive endpoint. Make sure the method naming and doc string align to avoid user confusion.
92-104: Potential naming ambiguity.
deleteUseractually callsarchiveUser. While it may be a convenience, consider updating the doc to clarify that this “delete” is effectively an archive.src/IntercomCompanies.php (2)
31-31: Alias to create.
Similar toIntercomUsers::update, consider clarifying in the doc that it leverages create.
48-50: Company attach path.
Merges$optionswith the["id" => $companyId]. Consider using a distinct key to avoid collisions with existing$options['id'].README.md (6)
14-15: Rephrase for clarity.
Your phrase "In the meantime" is flagged as wordy, and "best effort basis" might read better as "best-effort basis."- In the meantime, we'll be operating on limited capacity ... - ... on a best effort basis... + Meanwhile, we'll operate with limited capacity ... + ... on a best-effort basis...🧰 Tools
🪛 LanguageTool
[style] ~14-~14: ‘In the meantime’ might be wordy. Consider a shorter alternative.
Context: ...de in-depth and dedicated SDK support. In the meantime, we'll be operating on limited capacity...(EN_WORDINESS_PREMIUM_IN_THE_MEANTIME)
[grammar] ~15-~15: Use “the” before the superlative.
Context: ... all pull requests will be evaluated on a best effort basis and will be limited t...(THE_SUPERLATIVE)
41-43: Add missing comma and hyphen.
Add a comma after "If you want to create or learn more about access tokens," and hyphenate "third-party application."- If you are building a third party application you can get... + If you are building a third-party application, you can get...🧰 Tools
🪛 LanguageTool
[uncategorized] ~41-~41: Possible missing comma found.
Context: ...nt to create or learn more about access tokens then you can find more info [here](http...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~43-~43: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...thorization#section-access-tokens). > > If you are building a third party applicat...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~43-~43: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...cess-tokens). > > If you are building a third party application you can get your OAuth toke...(EN_COMPOUND_ADJECTIVE_INTERNAL)
45-45: Include missing comma.
Consider adding a comma after "For most use cases" for better readability.- For most use cases the code snippet above should suffice. + For most use cases, the code snippet above should suffice.🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...tion-oauth) for Intercom. For most use cases the code snippet above should suffice. ...(AI_HYDRA_LEO_MISSING_COMMA)
85-85: Add a comma for clarity.
Insert a comma after "However" to improve sentence flow.🧰 Tools
🪛 LanguageTool
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...m-Version` header when initializing the client as follows: ```php use Intercom\Interc...(AI_HYDRA_LEO_MISSING_COMMA)
588-588: Insert missing comma.
Include a comma after "In API versions 2.0 and above" for clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~588-~588: Possible missing comma found.
Context: ...e->pages); ``` In API versions 2.0 and above subsequent pages for listing contacts c...(AI_HYDRA_LEO_MISSING_COMMA)
625-626: Use commas in complex sentences.
Add a comma after "for example" and after "If you want to catch errors," for improved sentence structure.🧰 Tools
🪛 LanguageTool
[uncategorized] ~625-~625: Possible missing comma found.
Context: ...turn an unsuccessful HTTP response, for example when a resource is not found (404). If ...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~626-~626: Possible missing comma found.
Context: ...s not found (404). If you want to catch errors you can wrap your API call into a try/c...(AI_HYDRA_LEO_MISSING_COMMA)
src/IntercomClient.php (1)
389-422: Handle potential JSON decode errors.
Currently, the code blindly decodes the response body without error handling. If the response is invalid JSON,json_decodewill returnnullwithout a descriptive error.Consider adding error checking for
json_last_error()and raising an exception if necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (60)
.circleci/config.yml(1 hunks).codeclimate.yml(1 hunks).editorconfig(1 hunks).gitattributes(1 hunks).github/ISSUE_TEMPLATE.md(1 hunks).github/PULL_REQUEST_TEMPLATE.md(1 hunks).gitignore(1 hunks).travis.yml(0 hunks)README.md(8 hunks)circle.yml(0 hunks)composer.json(1 hunks)phpunit.xml.dist(1 hunks)src/IntercomAdmins.php(2 hunks)src/IntercomBulk.php(2 hunks)src/IntercomClient.php(5 hunks)src/IntercomCompanies.php(3 hunks)src/IntercomContacts.php(1 hunks)src/IntercomConversations.php(4 hunks)src/IntercomCounts.php(1 hunks)src/IntercomEvents.php(2 hunks)src/IntercomLeads.php(7 hunks)src/IntercomMessages.php(1 hunks)src/IntercomNotes.php(3 hunks)src/IntercomResource.php(1 hunks)src/IntercomSegments.php(2 hunks)src/IntercomTags.php(2 hunks)src/IntercomTeams.php(1 hunks)src/IntercomUsers.php(5 hunks)src/IntercomVisitors.php(4 hunks)test/IntercomAdminsTest.php(0 hunks)test/IntercomBulkTest.php(0 hunks)test/IntercomClientTest.php(0 hunks)test/IntercomCompaniesTest.php(0 hunks)test/IntercomConversationsTest.php(0 hunks)test/IntercomCountsTest.php(0 hunks)test/IntercomEventsTest.php(0 hunks)test/IntercomLeadsTest.php(0 hunks)test/IntercomMessagesTest.php(0 hunks)test/IntercomNotesTest.php(0 hunks)test/IntercomSegmentsTest.php(0 hunks)test/IntercomTagsTest.php(0 hunks)test/IntercomUsersTest.php(0 hunks)test/IntercomVisitorsTest.php(0 hunks)tests/IntercomAdminsTest.php(1 hunks)tests/IntercomBulkTest.php(1 hunks)tests/IntercomClientTest.php(1 hunks)tests/IntercomCompaniesTest.php(1 hunks)tests/IntercomContactsTest.php(1 hunks)tests/IntercomConversationsTest.php(1 hunks)tests/IntercomCountsTest.php(1 hunks)tests/IntercomEventsTest.php(1 hunks)tests/IntercomLeadsTest.php(1 hunks)tests/IntercomMessagesTest.php(1 hunks)tests/IntercomNotesTest.php(1 hunks)tests/IntercomSegmentsTest.php(1 hunks)tests/IntercomTagsTest.php(1 hunks)tests/IntercomTeamsTest.php(1 hunks)tests/IntercomUsersTest.php(1 hunks)tests/IntercomVisitorsTest.php(1 hunks)tests/TestCase.php(1 hunks)
💤 Files with no reviewable changes (16)
- circle.yml
- .travis.yml
- test/IntercomAdminsTest.php
- test/IntercomCompaniesTest.php
- test/IntercomSegmentsTest.php
- test/IntercomBulkTest.php
- test/IntercomLeadsTest.php
- test/IntercomNotesTest.php
- test/IntercomMessagesTest.php
- test/IntercomEventsTest.php
- test/IntercomTagsTest.php
- test/IntercomVisitorsTest.php
- test/IntercomConversationsTest.php
- test/IntercomClientTest.php
- test/IntercomCountsTest.php
- test/IntercomUsersTest.php
✅ Files skipped from review due to trivial changes (4)
- .codeclimate.yml
- .github/PULL_REQUEST_TEMPLATE.md
- .editorconfig
- .gitattributes
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.github/ISSUE_TEMPLATE.md
8-8: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🪛 LanguageTool
README.md
[style] ~14-~14: ‘In the meantime’ might be wordy. Consider a shorter alternative.
Context: ...de in-depth and dedicated SDK support. In the meantime, we'll be operating on limited capacity...
(EN_WORDINESS_PREMIUM_IN_THE_MEANTIME)
[grammar] ~15-~15: Use “the” before the superlative.
Context: ... all pull requests will be evaluated on a best effort basis and will be limited t...
(THE_SUPERLATIVE)
[uncategorized] ~41-~41: Possible missing comma found.
Context: ...nt to create or learn more about access tokens then you can find more info [here](http...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~43-~43: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...thorization#section-access-tokens). > > If you are building a third party applicat...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~43-~43: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...cess-tokens). > > If you are building a third party application you can get your OAuth toke...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...tion-oauth) for Intercom. For most use cases the code snippet above should suffice. ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~77-~77: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... Use a custom URI factory This factory needs to implement Http\Message\UriFactory ``...
(REP_NEED_TO_VB)
[uncategorized] ~85-~85: Possible missing comma found.
Context: ...m-Version` header when initializing the client as follows: ```php use Intercom\Interc...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~588-~588: Possible missing comma found.
Context: ...e->pages); ``` In API versions 2.0 and above subsequent pages for listing contacts c...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~625-~625: Possible missing comma found.
Context: ...turn an unsuccessful HTTP response, for example when a resource is not found (404). If ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~626-~626: Possible missing comma found.
Context: ...s not found (404). If you want to catch errors you can wrap your API call into a try/c...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (109)
tests/TestCase.php (1)
8-21: Well-structured base test class!The implementation follows PHPUnit best practices:
- Proper use of abstract class for base test case
- Correct mock setup in setUp method
- Clear PHPDoc documentation
phpunit.xml.dist (1)
11-14: Good updates to PHPUnit configuration!The changes improve the test configuration by:
- Removing the deprecated
syntaxCheckattribute- Updating the test directory path to match the new structure
.gitignore (1)
1-8: LGTM! Standard ignores for a PHP library.The changes follow best practices for PHP libraries:
- Ignoring
composer.lockis correct as libraries should not commit lock files.- PHPUnit related files are properly ignored.
src/IntercomResource.php (1)
5-21: LGTM! Well-structured base class for Intercom resources.The abstract class provides a solid foundation for the refactoring:
- Follows PSR-4 autoloading standard
- Properly documented with PHPDoc
- Type-safe constructor
- Reduces code duplication across resource classes
tests/IntercomTeamsTest.php (1)
25-29: LGTM! Path construction test is a good practice.Testing path construction separately helps catch URL-related issues early.
src/IntercomTeams.php (1)
8-48: Well-structured implementation with clear documentation.The class follows best practices:
- Extends
IntercomResourcefor consistent behavior- Methods are well-documented with PHPDoc comments
- Clear separation of concerns with utility method for path construction
- Proper exception handling and return type declarations
src/IntercomSegments.php (1)
8-36: Clean implementation with standardized return types and exception handling.The class follows best practices:
- Extends
IntercomResourcefor consistent behavior- Methods are well-documented with PHPDoc comments
- Return types and exception handling have been standardized
src/IntercomVisitors.php (5)
5-6: Imports are consistent with the new approach
The newly added imports forHttp\Client\ExceptionandstdClassalign with the updated exception handling and return types. Good job using these standardized classes.
8-9: ConfirmIntercomResourcecompatibility
Now that the class extendsIntercomResource, ensure that$this->clientis correctly set up in the parent class.
30-31: VerifystdClassreturn consistency
Check that$this->client->get($path, $options)truly returns astdClassand not another structure.
45-46: Double-check the delete operation's return
Similarly, ensure that the response from deleting a visitor also adheres to astdClassstructure.
59-60: Confirm conversion response type
Confirm the conversion endpoint always returns astdClass. If there's an error case returning arrays or strings, it might conflict with this contract.src/IntercomTags.php (3)
5-6: Adoption ofHttp\Client\Exception
Switching from Guzzle's exception toHttp\Client\Exceptionis consistent with the new resource-based approach.
8-9: Inheritance fromIntercomResource
Ensure that the parent class properly provides$this->clientfor tag operations.
28-29: ConfirmstdClassusage
Same note: ensure the client always yieldsstdClass. If the API returns different structures for errors or empty results, the doc block might not be fully accurate.src/IntercomCounts.php (2)
5-6: Consistent use statements
The usage ofHttp\Client\ExceptionandstdClassappears to be part of the standardized approach across the codebase.
8-9: Resource-based refactor
ExtendingIntercomResourcehelps unify the code structure. Verify that the inherited$clientproperty is functioning correctly for thegetCountsmethod.src/IntercomMessages.php (2)
5-6: Well-structured import statements.
The new imports align with the switch toHttp\Client\Exceptionand the need to return an object of typestdClass.
8-9: Confirm inheritance fromIntercomResource.
Relying on$this->clientfrom the parent is fine ifIntercomResourcedeclares and initializes$clientproperly. Otherwise, ensure$this->clientis accessible and not null.tests/IntercomLeadsTest.php (4)
1-6: Good namespace and class reference.
The file correctly declares theIntercom\Testnamespace and importsIntercomLeads.
33-38: Helper method path check.
testLeadPath()verifying"contacts/foo"is valuable. Looks good.
48-54: Conversion test cleverly checks argument flow.
Returning the first argument ensures the correct path string is used. This approach is fine.
64-70: Scrolling test coverage.
testLeadsScroll()is straightforward, following the pattern of mockingget(). Looks good.tests/IntercomClientTest.php (6)
1-22: Mock client setup is properly configured.
UsingPsr18ClientDiscovery::prependStrategy(MockClientStrategy::class)ensures that all requests are routed to the mock. This is a clean approach.
23-41: Authorization header verification is solid.
testBasicClient()correctly checks Basic Auth encoding. This is beneficial for confirming credentials handling.
43-63: Extra header coverage encourages flexible configuration.
testClientWithExtraHeaders()ensures that custom headers are appended. Looks good.
82-98: Server error handling test is well done.
testServerErrorHandling()ensures 500 response handling. The usage ofErrorPluginis consistent with the approach for 4xx errors.
99-118: Pagination helper test.
testPaginationHelper()ensures next-page URLs are processed. Good practice testing.
120-156: Comprehensive rate limit check.
testRateLimitDetails()thoroughly verifies the limit, remaining, and reset time. Nicely covers critical usage of response headers.src/IntercomAdmins.php (5)
5-6: Imports align well with new error handling and return types
No issues spotted with these import statements. They correctly bring inHttp\Client\ExceptionandstdClass, which match the updated doc comments and intended usage in the class.
8-9: ConfirmIntercomResourceinheritance is correctly referencing$this->client
You're extendingIntercomResourceand presumably relying on its constructor or property assignment for$this->client. Please ensure that the parent class properly initializes$this->client.
15-16: Doc annotation reflects the updated return type and exception
ReturningstdClassand throwingHttp\Client\Exceptionis consistent with the new error-handling strategy. Just verify that theIntercomClientindeed returns an object of typestdClass.
29-30: Ensure consistency of exception type across all usages
The doc comment now declares@throws Exception. Confirm that any other methods in the class or parent classes also consistently report this exception type.
38-43: Docblock completeness
The docblock foradminPathis clear. No issues identified in the logic or documentation of this helper method.src/IntercomEvents.php (4)
5-6: Imports support new architecture
The usage ofHttp\Client\ExceptionandstdClasshere follows the new pattern established in other classes. No problems detected.
8-9: Inheritance fromIntercomResource
ExtendingIntercomResourceeliminates the direct$clientproperty in this class. Verify that the parent class provides the complete initialization and that the client usage here remains valid.
15-16: Updated doc: returningstdClassand throwingException
Matches the broader refactoring toward returning concrete types and handling exceptions viaHttp\Client\Exception. Implementation appears consistent.
28-29: Doc comment for getEvents
The@return stdClassand@throws Exceptiontags match the changes seen increate(). This aligns with the new standard across the codebase.tests/IntercomVisitorsTest.php (9)
1-2: File initialization
This file is newly created and properly declares PHP tags. No issues spotted.
3-3: Namespace usage
Defining theIntercom\Testnamespace is fine; ensure references toTestCaseor other framework classes are correctly imported if needed.
5-5: Base IntercomVisitors import
The class under test is correctly imported. Single import, no duplication. Looks good.
7-8: CheckTestCaseinheritance
The snippet doesn't show an import forTestCase. IfTestCaseis part of this same namespace, that's fine; otherwise, import its fully qualified name (e.g.,PHPUnit\Framework\TestCase).
9-15: testVisitorUpdate coverage
This test successfully mocks the client'sputmethod, ensuring theupdate([])call's return is tested. Consider adding assertions for request parameters or structure if needed.
17-21: testVisitorPath coverage
Straightforward check thatvisitorPathreturns"visitors/foo". The test is concise and clear.
23-29: testVisitorsGet usage
Mocks thegetmethod and verifies correct return value. No issues. This test ensures the method call aligns with the client’s interface.
31-38: testVisitorsConvert logic
Uses$this->returnArgument(0)to confirm the correct path is returned fromconvertVisitor(). Great approach to validating the request endpoint.
40-46: testVisitorsDelete
Mocks thedeletemethod, verifying the correct return value. Test thoroughly covers the scenario for deleting a visitor.src/IntercomBulk.php (3)
5-8: LGTM! Good improvements in code organization.The changes improve type safety by using specific types and enhance code organization through inheritance.
13-16: LGTM! Improved type safety and standardization.The changes enhance type safety with
stdClassreturn type and standardize error handling using PSR-compliant HTTP client exceptions.
25-28: LGTM! Consistent improvements across methods.The changes maintain consistency with the
usersmethod, applying the same improvements in type safety and exception handling.src/IntercomNotes.php (5)
5-6: Imports look good.
The usage ofHttp\Client\ExceptionandstdClassaligns with the new exception handling and return type strategy.
8-9: Inheritance structure updated.
ExtendingIntercomResourcecentralizes common functionality, which should simplify maintenance and keep the class lean.
15-16: Doc block matches implementation.
ReturningstdClassand throwingHttp\Client\Exceptionseem consistent with the client operations.
28-29: Consistency for getNotes.
The doc block accurately reflects the return type and exception thrown by$this->client->get().
41-42: Ensure getNote's doc references remain accurate.
Everything lines up with the final code, though it's worth verifying if the Intercom docs still reference the same endpoint path for single note retrieval.src/IntercomLeads.php (8)
5-6: Imports align with exception and return specs.
UsingHttp\Client\ExceptionandstdClassmaintains consistency with the refactored codebase.
8-9: Refactoring to extend IntercomResource.
This approach streamlines resource handling and fosters code reuse.
15-16: Potential mismatch in documentation.
The doc block references “create-lead” but the method callscontactsendpoint. Verify that the Intercom Docs link accurately describes creating leads viacontacts.
41-42: Doc block for listing leads.
Ensure the associated “list-leads” reference in Intercom’s docs matches thecontactsendpoint usage.
55-56: Return type alignment for getLead.
Doc block says@throws Exception, return type isstdClass; consistent for typical GET operations.
70-71: Check endpoint for deleteLead.
Double-check that “contacts” is the new canonical path to delete a lead, to avoid confusion with outdated docs.
84-85: convertLead references.
Same “create-lead” doc link might be outdated if the actual endpoint iscontacts/convert. Please confirm.
108-109: Confirm scrollLeads usage.
“contacts/scroll” endpoint is presumably the right path for leads in the new API, but confirm with current Intercom docs.src/IntercomContacts.php (9)
1-4: Namespace and file declaration.
Using theIntercomnamespace for a new resource class is consistent with the existing structure.
5-6: Imports align with usage.
Http\Client\Exceptionfor error handling andstdClassfor typed responses match the new project conventions.
8-9: New IntercomContacts class.
ExtendingIntercomResourceto inherit shared functionality is a strong choice for code consistency.
23-36: Method update(string $id, array $options).
UsingPUTfor updates is appropriate. Check if partial updates are supported or if a full object must be sent to avoid data overwrites.
38-49: Method getContacts(array $options = []).
Straightforward retrieval with optional filters. This approach is clear and ensures flexible usage.
51-64: Method getContact(string $id, array $options = []).
Implementation is similar to getContacts but fetches a single contact. All good here.
81-93: Method search(array $options).
POST /contacts/searchis correct according to Intercom’s newer search endpoints. Looks good for advanced queries.
95-109: Method nextSearch(array $query, $pages).
Great approach to handle pagination. Ensure$pages->nextis present before referencing->starting_after.
125-133: Helper contactPath(string $id).
A simple helper ensures consistent endpoint path building. This pattern is clean and extensible.src/IntercomUsers.php (12)
5-6: Imports are consistent with the new HTTP error handling and return types.
The addition ofHttp\Client\ExceptionandstdClassaligns with the PR's shift toward HTTPlug-based exceptions and standardized return objects.
8-9: Ensure parent dependency is resolved.
Confirm that$this->clientis accessible viaIntercomResourceso these child classes can properly call$this->client->get(),$this->client->post(), etc.
15-16: DocBlock updates look good.
The newstdClassreturn annotation andExceptionthrow tag accurately reflect the new code path.
18-18: Stricter array typing.
Usingarray $optionsclarifies incoming parameters and reduces ambiguity in method calls.
28-29: DocBlock reusability.
“update” reuses the same annotation style as “create,” which is consistent for easier maintainability.
41-42: Return and exception tags align.
DocBlock matches the method’s actual return type and potential exception.
44-44: Method signature consistency.
Acceptingarray $optionsfor filtering or pagination is standard for listing API resources.
55-56: Maintains uniform doc blocks.
The doc block’s return type and exception annotation are consistent with the rest of the class.
72-72: Scrolling functionality.
scrollUsersis a valuable addition for large data iteration. Ensure upstream usage includes cursors or next-page tokens.
86-86: Archive user method.
Renaming from “delete” to “archive” clarifies that this action is reversible in Intercom.
106-119: Permanent deletion.
The new method aligns with Intercom docs for unrecoverable deletion. Implementation looks correct.
125-125: Explicit typed parameter.
public function userPath(string $id)is an improvement over loosely typed IDs, reinforcing type safety.src/IntercomCompanies.php (14)
5-6: Imports match the updated approach.
UsingHttp\Client\ExceptionandstdClassis consistent with other Intercom resource classes.
8-9: Extends IntercomResource.
Check thatIntercomResourcesets$this->clientto avoid errors in method calls like$this->client->post().
11-16: DocBlock clarity.
Accurate reference and annotation for creating a company.
18-21: Creation method.
public function create($options)returningstdClassis aligned with the project-wide changes.
24-30: Unified approach for update.
DocBlock clarifies the ID or unique info is passed within$options.
37-45: Attaching a contact doc.
The doc references an Intercom link. Ensure correctness with current Intercom docs for “attach contact” endpoints.
46-46: Attach contact signature.
Use of typed parameters for$contactIdand$companyIdclarifies necessary arguments.
54-62: Detaching doc block.
Consistent doc references for the “detach contact” endpoint.
63-67: Detaching a contact.
Method signature and logic appear correct for removing the association.
74-75: List companies doc.
Indicates standard usage for listing resources.
88-89: Retrieving a company doc.
stdClassas a return is consistent with the rest of the codebase.
97-111: Get company users.
New method extends functionality. Confirm the endpoint matches latest Intercom documentation.
122-129: Path method for company users.
Returns'companies/{id}/users'. Straightforward approach for forming the endpoint.
131-149: Attach and detach path methods.
These utility methods keep path manipulations DRY and more maintainable.composer.json (9)
3-3: Updated description to mention HTTPlug.
This clarifies the move away from Guzzle-specific implementations.
4-9: Keywords updated.
The reordered keywords better reflect the new technology stack.
10-10: License field updated.
Switching to “Apache-2.0” is a minor but important correction for license clarity.
14-14: Homepage updated.
Reflects the newer Intercom domain.
17-20: New support section.
Providing direct links for issues/source is a best practice for open source.
23-27: Autoload improvements.
Separatingautoloadandautoload-devis helpful for test isolation and code clarity.Also applies to: 28-34
36-43: Raised PHP version requirement and switched dependencies.
Dropping Guzzle in favor of HTTPlug fosters a more flexible HTTP layer. Verify older environment compatibility if needed.
46-48: New dev dependencies.
nyholm/psr7andphp-http/mock-clienthelp test thoroughly under PSR-7 and HTTPlug.Also applies to: 50-50
51-56: Config section for package sorting and plugin allowances.
Good practice to keep dependencies consistent and limit plugin usage.src/IntercomConversations.php (2)
51-63: Great addition ofsearch()method.
This implementation aligns with the documented Intercom endpoint and returns the appropriatestdClass.
65-78: Check handling of$pagesdata.
Ensure$pages->per_pageand$pages->nextexist before accessing them, to avoid potential property access errors.
tests/IntercomCountsTest.php
Outdated
| public function testCountsList() | ||
| { | ||
| $this->client->method('get')->willReturn('foo'); | ||
|
|
||
| $counts = new IntercomCounts($this->client); | ||
| $this->assertSame('foo', $counts->getCounts([])); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage with realistic scenarios.
The current test is too simplistic:
- Using 'foo' as a return value doesn't represent real count data
- Missing test cases for error conditions and edge cases
Consider adding these test cases:
- Test with actual count response structure
- Test with various query parameters
- Test error handling for API failures
- Test edge cases (empty response, malformed data)
Example implementation:
public function testCountsList()
{
$expectedResponse = (object)[
'company' => (object)['count' => 50],
'user' => (object)['count' => 100],
'conversation' => (object)['count' => 25]
];
$this->client->method('get')
->with('counts')
->willReturn($expectedResponse);
$counts = new IntercomCounts($this->client);
$result = $counts->getCounts(['type' => 'company,user,conversation']);
$this->assertSame($expectedResponse, $result);
}
public function testCountsListWithError()
{
$this->client->method('get')
->willThrowException(new \Exception('API Error'));
$this->expectException(\Exception::class);
$counts = new IntercomCounts($this->client);
$counts->getCounts([]);
}
tests/IntercomAdminsTest.php
Outdated
| $users = new IntercomAdmins($this->client); | ||
| $this->assertSame('foo', $users->getAdmins()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix misleading variable names.
The variable $users is misleading as it represents an admin client instance.
- $users = new IntercomAdmins($this->client);
- $this->assertSame('foo', $users->getAdmins());
+ $adminClient = new IntercomAdmins($this->client);
+ $this->assertSame('foo', $adminClient->getAdmins());Also applies to: 21-22, 27-28
tests/IntercomUsersTest.php
Outdated
| $this->client->method('post')->willReturn('foo'); | ||
|
|
||
| $users = new IntercomUsers($this->client); | ||
| $this->assertSame('foo', $users->create([])); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage with realistic test data and error scenarios.
The tests are overly simplistic and don't validate:
- Request parameters and their types
- Error scenarios (e.g., invalid data, API errors)
- HTTP method parameters (URL, headers, body)
Consider enhancing the tests with:
- Realistic test data instead of empty arrays/strings
- Validation of request parameters
- Error scenarios
- HTTP method parameter assertions
Example for testUserCreate:
public function testUserCreate()
{
- $this->client->method('post')->willReturn('foo');
+ $userData = [
+ 'email' => '[email protected]',
+ 'name' => 'Test User',
+ 'custom_attributes' => ['plan' => 'premium']
+ ];
+
+ $this->client
+ ->expects($this->once())
+ ->method('post')
+ ->with(
+ $this->equalTo('users'),
+ $this->equalTo($userData)
+ )
+ ->willReturn((object)['id' => '123', 'email' => '[email protected]']);
$users = new IntercomUsers($this->client);
- $this->assertSame('foo', $users->create([]));
+ $result = $users->create($userData);
+
+ $this->assertInstanceOf(\stdClass::class, $result);
+ $this->assertEquals('123', $result->id);
+ $this->assertEquals('[email protected]', $result->email);
}Also applies to: 19-22, 27-30, 35-38, 43-46, 51-54
tests/IntercomCompaniesTest.php
Outdated
| $this->client->method('post')->willReturn('foo'); | ||
|
|
||
| $companies = new IntercomCompanies($this->client); | ||
| $this->assertSame('foo', $companies->create([])); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage with realistic test data and fix variable naming.
The tests have similar issues to IntercomUsersTest:
- Overly simplistic test data
- Missing error scenarios
- No parameter validation
- No HTTP method parameter assertions
Additionally, there's inconsistent variable naming in testCompanyPath and testCompanyGetById where $users is used instead of $companies.
- Fix variable naming:
public function testCompanyPath()
{
- $users = new IntercomCompanies($this->client);
- $this->assertSame('companies/foo', $users->companyPath("foo"));
+ $companies = new IntercomCompanies($this->client);
+ $this->assertSame('companies/foo', $companies->companyPath("foo"));
}
public function testCompanyGetById()
{
$this->client->method('get')->willReturn('foo');
- $users = new IntercomCompanies($this->client);
- $this->assertSame('foo', $users->getCompany("foo"));
+ $companies = new IntercomCompanies($this->client);
+ $this->assertSame('foo', $companies->getCompany("foo"));
}- Enhance test coverage with realistic data:
public function testCompanyCreate()
{
- $this->client->method('post')->willReturn('foo');
+ $companyData = [
+ 'name' => 'Acme Inc',
+ 'industry' => 'Software',
+ 'size' => 100,
+ 'website' => 'https://example.com',
+ 'custom_attributes' => ['tier' => 'enterprise']
+ ];
+
+ $this->client
+ ->expects($this->once())
+ ->method('post')
+ ->with(
+ $this->equalTo('companies'),
+ $this->equalTo($companyData)
+ )
+ ->willReturn((object)[
+ 'id' => '123',
+ 'name' => 'Acme Inc',
+ 'industry' => 'Software'
+ ]);
$companies = new IntercomCompanies($this->client);
- $this->assertSame('foo', $companies->create([]));
+ $result = $companies->create($companyData);
+
+ $this->assertInstanceOf(\stdClass::class, $result);
+ $this->assertEquals('123', $result->id);
+ $this->assertEquals('Acme Inc', $result->name);
+ $this->assertEquals('Software', $result->industry);
}Also applies to: 19-22, 27-30, 35-36, 41-44, 49-52, 57-58
tests/IntercomNotesTest.php
Outdated
| $this->client->method('post')->willReturn('foo'); | ||
|
|
||
| $notes = new IntercomNotes($this->client); | ||
| $this->assertSame('foo', $notes->create([])); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage and standardize mocking approach.
The tests have similar issues to other test files:
- Overly simplistic test data
- Missing error scenarios
- No parameter validation
- No HTTP method parameter assertions
Additionally, there's inconsistent mocking approach between tests - returnArgument vs hardcoded 'foo'.
- Standardize mocking approach and enhance test coverage:
public function testNoteCreate()
{
- $this->client->method('post')->willReturn('foo');
+ $noteData = [
+ 'body' => 'Test note content',
+ 'user' => ['id' => 'user_123'],
+ 'admin' => ['id' => 'admin_456']
+ ];
+
+ $expectedResponse = (object)[
+ 'id' => 'note_789',
+ 'body' => 'Test note content',
+ 'created_at' => time()
+ ];
+
+ $this->client
+ ->expects($this->once())
+ ->method('post')
+ ->with(
+ $this->equalTo('notes'),
+ $this->equalTo($noteData)
+ )
+ ->willReturn($expectedResponse);
$notes = new IntercomNotes($this->client);
- $this->assertSame('foo', $notes->create([]));
+ $result = $notes->create($noteData);
+
+ $this->assertInstanceOf(\stdClass::class, $result);
+ $this->assertEquals('note_789', $result->id);
+ $this->assertEquals('Test note content', $result->body);
}
public function testNotesList()
{
- $this->client->method('get')->willReturn('foo');
+ $params = ['user_id' => 'user_123'];
+ $expectedResponse = (object)[
+ 'type' => 'list',
+ 'data' => [
+ (object)['id' => 'note_1', 'body' => 'First note'],
+ (object)['id' => 'note_2', 'body' => 'Second note']
+ ]
+ ];
+
+ $this->client
+ ->expects($this->once())
+ ->method('get')
+ ->with(
+ $this->equalTo('notes'),
+ $this->equalTo($params)
+ )
+ ->willReturn($expectedResponse);
$notes = new IntercomNotes($this->client);
- $this->assertSame('foo', $notes->getNotes([]));
+ $result = $notes->getNotes($params);
+
+ $this->assertInstanceOf(\stdClass::class, $result);
+ $this->assertEquals('list', $result->type);
+ $this->assertCount(2, $result->data);
}
public function testNotesGet()
{
- $this->client->method('get')->will($this->returnArgument(0));
+ $noteId = 'note_123';
+ $expectedResponse = (object)[
+ 'id' => $noteId,
+ 'body' => 'Note content',
+ 'created_at' => time()
+ ];
+
+ $this->client
+ ->expects($this->once())
+ ->method('get')
+ ->with($this->equalTo("notes/$noteId"))
+ ->willReturn($expectedResponse);
$notes = new IntercomNotes($this->client);
- $this->assertSame('notes/foo', $notes->getNote("foo"));
+ $result = $notes->getNote($noteId);
+
+ $this->assertInstanceOf(\stdClass::class, $result);
+ $this->assertEquals($noteId, $result->id);
+ $this->assertEquals('Note content', $result->body);
}- Add error scenario tests:
public function testNoteCreateWithInvalidData()
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Note body is required');
$notes = new IntercomNotes($this->client);
$notes->create(['user' => ['id' => 'user_123']]); // Missing body
}Also applies to: 19-22, 27-30
| /** | ||
| * Returns next page of a Contacts list. | ||
| * | ||
| * @see https://developers.intercom.com/intercom-api-reference/reference#pagination | ||
| * @param stdClass $pages | ||
| * @return stdClass | ||
| * @throws Exception | ||
| */ | ||
| public function nextCursor($pages) | ||
| { | ||
| $path = 'contacts'; | ||
| $starting_after = $pages->next->starting_after; | ||
| return $this->client->nextCursorPage($path, $starting_after); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Method nextCursor($pages).
Further pagination support is consistent with Intercom’s style. Consider adding guard checks if $pages->next->starting_after is missing.
+ if (!isset($pages->next->starting_after)) {
+ throw new \InvalidArgumentException("No further pages available.");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Returns next page of a Contacts list. | |
| * | |
| * @see https://developers.intercom.com/intercom-api-reference/reference#pagination | |
| * @param stdClass $pages | |
| * @return stdClass | |
| * @throws Exception | |
| */ | |
| public function nextCursor($pages) | |
| { | |
| $path = 'contacts'; | |
| $starting_after = $pages->next->starting_after; | |
| return $this->client->nextCursorPage($path, $starting_after); | |
| } | |
| /** | |
| * Returns next page of a Contacts list. | |
| * | |
| * @see https://developers.intercom.com/intercom-api-reference/reference#pagination | |
| * @param stdClass $pages | |
| * @return stdClass | |
| * @throws Exception | |
| */ | |
| public function nextCursor($pages) | |
| { | |
| + if (!isset($pages->next->starting_after)) { | |
| + throw new \InvalidArgumentException("No further pages available."); | |
| + } | |
| $path = 'contacts'; | |
| $starting_after = $pages->next->starting_after; | |
| return $this->client->nextCursorPage($path, $starting_after); | |
| } |
This prevents a deprecation warning from showing up since PHP 8.4 ``` Deprecated: Intercom\IntercomClient::__construct(): Implicitly marking parameter $password as nullable is deprecated, the explicit nullable type must be used instead in /<redacted-path>/vendor/intercom/intercom-php/src/IntercomClient.php on line 148 ```
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/IntercomClient.php(5 hunks)
🔇 Additional comments (15)
src/IntercomClient.php (15)
5-20: PSR-compliant HTTP client implementation - great improvement!The shift from direct Guzzle dependencies to PSR HTTP client interfaces adds significant flexibility to the codebase. This allows users to plug in any PSR-18 compatible HTTP client of their choice rather than being locked into Guzzle.
29-44: Good architectural change using PSR HTTP factory interfacesUsing the PSR-17 HTTP factory interfaces (
RequestFactoryInterface,UriFactoryInterface,StreamFactoryInterface) allows for greater flexibility and interoperability. This is a solid improvement to the codebase architecture.
148-174: Well-implemented dependency initialization in constructorThe constructor now correctly initializes the PSR HTTP factories using discovery mechanisms, which is a best practice approach. The addition of the
IntercomTeamsresource is also properly integrated.
181-214: Good addition of setter methods for testing and customizationThese setter methods for HTTP client and factories enhance the testability of the code and allow for custom HTTP client implementations. This makes the library more adaptable to different environments and requirements.
223-227: Method signature improvement: accepting arrays instead of JSONChanging the method to accept arrays instead of JSON strings is a good practice. This provides type safety and eliminates the need for users to manually JSON encode their data.
262-272: Enhanced GET method with proper URI handlingThe updated GET method properly handles query parameters by using the URI factory. This is a more robust approach than the previous implementation.
280-284: Improved pagination method implementationThe
nextPagemethod has been refactored to use the newsendRequestmethod, making it consistent with the rest of the codebase.
294-304: Good addition: nextSearchPage method for search paginationThis new method properly handles the pagination for search queries, which requires different pagination parameters than standard requests. The implementation follows the Intercom API requirements appropriately.
313-316: Simplified cursor-based pagination implementationThe
nextCursorPagemethod provides an elegant solution for cursor-based pagination, which is a common pattern in modern APIs. This implementation is clean and straightforward.
323-326: Useful rate limit details accessorAdding a dedicated getter for rate limit details makes it easier for users of the library to respect API rate limits and implement backoff strategies.
331-337: Good default HTTP client configuration with error pluginThe default HTTP client is properly configured with an error plugin, which will convert HTTP error responses to exceptions. This ensures consistent error handling throughout the library.
359-367: Well-implemented authentication strategyThe authentication method properly handles both BasicAuth and Bearer token authentication scenarios, with good fallback behavior. This allows for both legacy API key authentication and the newer token-based approach.
389-408: Well-designed central request methodThe
sendRequestmethod centralizes the request creation, body handling, header application, and authentication in a clean way. This removes code duplication and ensures consistent request handling across all HTTP methods.
415-422: Simplified response handlingThe
handleResponsemethod has been updated to work with the PSR ResponseInterface and properly extracts the JSON data from the response body.
427-440: Consistent rate limit details extractionThe method for extracting rate limit details from response headers is well-implemented, with proper null handling when headers are missing.
src/IntercomClient.php
Outdated
|
|
||
| class IntercomClient | ||
| { | ||
| const SDK_VERSION = '4.4.0'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify SDK version is accurate for this release
Make sure the SDK version constant is updated appropriately for this release since significant architectural changes have been made.
🏁 Script executed:
#!/bin/bash
# Check if package.json or composer.json exists and extract current version
if [ -f "composer.json" ]; then
echo "Current version in composer.json:"
grep -E '"version"' composer.json
fi
# Check git tags to see previous versioning pattern
echo "Recent git tags (versions):"
git tag -l --sort=-v:refname | head -5Length of output: 280
SDK Version Constant Update Required
- The git tag output indicates that the latest release is v4.5.0, while the constant in
src/IntercomClient.phpremains set to'4.4.0'. - Please update the
SDK_VERSIONconstant on line 24 insrc/IntercomClient.phpto'4.5.0'in order to accurately reflect the current release that includes significant architectural changes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/label-ai-generated-prs.yml (3)
4-7: Consider adding thereopenedtrigger.
Currently it runs onopened,edited, andsynchronize. Includingreopenedensures labels are applied if a closed PR is reopened.
10-10: Pin the external workflow to a stable version.
Referencingmaincan introduce breaking changes when upstream updates. Use a specific tag or SHA (e.g.,@v1or@<commit-sha>) to lock to a known-good version.
11-11: Add a newline at end of file.
YAML linters expect a trailing newline to avoid parsing warnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/label-ai-generated-prs.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/label-ai-generated-prs.yml
[error] 11-11: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/label-ai-generated-prs.yml (1)
1-3: Workflow header and naming look good.
The comment andname:field clearly describe the purpose of this workflow.
Co-authored-by: fern-api <115122769+fern-api[bot]@users.noreply.github.com>
composer.json
Outdated
| { | ||
| "name": "intercom/intercom-php", | ||
| "description": "Intercom API client built on top of Guzzle 6", | ||
| "keywords": ["intercom", "intercom.io", "api", "guzzle"], | ||
| "license": "Apache Version 2", | ||
| "authors": [ | ||
| { | ||
| "name": "Intercom Platform Team", | ||
| "homepage": "http://intercom.io" | ||
| } | ||
| ], | ||
| "autoload": { | ||
| "psr-4": { | ||
| "Intercom\\": ["src"] | ||
| } | ||
| }, | ||
| "require": { | ||
| "guzzlehttp/guzzle": "~6.0", | ||
| "php": ">= 5.6" | ||
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "4.0.*", | ||
| "squizlabs/php_codesniffer": "^3.1" | ||
| "name": "intercom/intercom-php", | ||
| "version": "0.0.325", | ||
| "description": "Intercom API client.", | ||
| "keywords": [ | ||
| "intercom", | ||
| "api", | ||
| "sdk" | ||
| ], | ||
| "license": [], | ||
| "require": { | ||
| "php": "^8.1", | ||
| "ext-json": "*", | ||
| "guzzlehttp/guzzle": "^7.4" | ||
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "^9.0", | ||
| "friendsofphp/php-cs-fixer": "3.5.0", | ||
| "phpstan/phpstan": "^1.12" | ||
| }, | ||
| "autoload": { | ||
| "psr-4": { | ||
| "Intercom\\": "src/" | ||
| } | ||
| }, | ||
| "autoload-dev": { | ||
| "psr-4": { | ||
| "Intercom\\Tests\\": "tests/" | ||
| } | ||
| } | ||
| }, | ||
| "scripts": { | ||
| "build": [ | ||
| "@php -l src", | ||
| "@php -l tests" | ||
| ], | ||
| "test": "phpunit", | ||
| "analyze": "phpstan analyze src tests --memory-limit=1G" | ||
| }, | ||
| "author": { | ||
| "name": "Intercom Platform Team", | ||
| "url": "https://www.intercom.com" | ||
| }, | ||
| "homepage": "https://developers.intercom.com/docs" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Run composer schema checks in CI to prevent regressions.
Add a CI step to run composer validate --strict. See workflow comment for a concrete diff.
Run locally:
🏁 Script executed:
#!/bin/bash
composer validate --strictLength of output: 580
composer.json: Address schema errors and enforce strict validation in CI
Before adding a CI step for strict validation, the following schema errors must be fixed so that composer validate --strict passes:
-
Replace the singular
authorproperty with anauthorsarray. For example:"authors": [ { "name": "Intercom Platform Team", "homepage": "https://www.intercom.com" } ], -
Remove the
"version": "0.0.325"field, as it’s discouraged when publishing on Packagist. -
Specify a valid license. For closed-source packages you can use:
- "license": [], + "license": "proprietary",
Once those changes are in place and composer validate --strict exits cleanly, add the following CI step to your workflow to guard against future regressions:
- name: Validate composer.json
run: composer validate --strictLet me know if you need help updating the workflow or resolving any remaining schema issues.
🤖 Prompt for AI Agents
In composer.json lines 1-44, replace the singular "author" object with an
"authors" array containing objects (e.g., name and url), remove the top-level
"version" field entirely, and set a valid license string (either an SPDX
identifier like "MIT" or "proprietary" for closed-source) so composer validate
--strict will pass; after updating composer.json, add a CI step that runs
composer validate --strict to the workflow to prevent regressions.
composer.json
Outdated
| "license": [], | ||
| "require": { | ||
| "php": "^8.1", | ||
| "ext-json": "*", | ||
| "guzzlehttp/guzzle": "^7.4" | ||
| }, |
There was a problem hiding this comment.
composer.json schema issues: invalid/empty license array and non-standard "author".
- license must be a valid SPDX string or a non-empty array of SPDX identifiers; an empty array will fail composer validate.
- Composer expects "authors" (array), not "author" (object). Also, author objects use "homepage" (not "url").
Proposed fix (please replace MIT with your actual license if different):
"keywords": [
"intercom",
"api",
"sdk"
],
- "license": [],
+ "license": "MIT",
@@
- "author": {
- "name": "Intercom Platform Team",
- "url": "https://www.intercom.com"
- },
+ "authors": [
+ {
+ "name": "Intercom Platform Team",
+ "homepage": "https://www.intercom.com"
+ }
+ ],To catch these issues in CI, add a composer validate step (see CI comment).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "license": [], | |
| "require": { | |
| "php": "^8.1", | |
| "ext-json": "*", | |
| "guzzlehttp/guzzle": "^7.4" | |
| }, | |
| "keywords": [ | |
| "intercom", | |
| "api", | |
| "sdk" | |
| ], | |
| "license": "MIT", | |
| "require": { | |
| "php": "^8.1", | |
| "ext-json": "*", | |
| "guzzlehttp/guzzle": "^7.4" | |
| }, | |
| "authors": [ | |
| { | |
| "name": "Intercom Platform Team", | |
| "homepage": "https://www.intercom.com" | |
| } | |
| ], |
🤖 Prompt for AI Agents
In composer.json around lines 10 to 15, the license field is an empty array and
the manifest uses a non-standard "author" object; replace the empty license
value with a valid SPDX identifier (e.g. "MIT" or your actual license), convert
the single "author" object to an "authors" array containing an object with keys
"name", "email" and "homepage" (use "homepage" instead of "url"), and then add a
CI step that runs composer validate to catch schema issues automatically.
| "scripts": { | ||
| "build": [ | ||
| "@php -l src", | ||
| "@php -l tests" | ||
| ], | ||
| "test": "phpunit", | ||
| "analyze": "phpstan analyze src tests --memory-limit=1G" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
"build" script is not portable; php -l cannot lint directories.
php -l src and php -l tests will fail because php -l expects files, not directories. Use a linter over file sets or a simple finder/xargs. For portability and speed, prefer php-parallel-lint.
Option A (add php-parallel-lint):
"require-dev": {
"phpunit/phpunit": "^9.0",
- "friendsofphp/php-cs-fixer": "3.5.0",
+ "friendsofphp/php-cs-fixer": "^3.5.0",
- "phpstan/phpstan": "^1.12"
+ "phpstan/phpstan": "^1.12",
+ "php-parallel-lint/php-parallel-lint": "^1.3"
},
@@
"scripts": {
- "build": [
- "@php -l src",
- "@php -l tests"
- ],
+ "build": "parallel-lint src tests",
"test": "phpunit",
"analyze": "phpstan analyze src tests --memory-limit=1G"
},Option B (no new dev dependency, Unix-only):
- "build": [
- "@php -l src",
- "@php -l tests"
- ],
+ "build": "find src tests -type f -name \"*.php\" -print0 | xargs -0 -n1 php -l",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "scripts": { | |
| "build": [ | |
| "@php -l src", | |
| "@php -l tests" | |
| ], | |
| "test": "phpunit", | |
| "analyze": "phpstan analyze src tests --memory-limit=1G" | |
| "scripts": { | |
| "build": "find src tests -type f -name \"*.php\" -print0 | xargs -0 -n1 php -l", | |
| "test": "phpunit", | |
| "analyze": "phpstan analyze src tests --memory-limit=1G" |
🤖 Prompt for AI Agents
In composer.json around lines 31 to 37, the "build" script uses "php -l src" and
"php -l tests" which fail because php -l expects file paths, not directories;
replace this with a portable linter command: either add php-parallel-lint as a
dev dependency and change the script to run it against src and tests, or if you
cannot add deps use a shell command that finds PHP files (e.g., find . -name
'*.php' -print0 | xargs -0 php -l) but ensure composer scripts remain
cross-platform if required (prefer php-parallel-lint for portability and speed).
| /** | ||
| * @param array{ | ||
| * adminId: string, | ||
| * } $values | ||
| */ | ||
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->adminId = $values['adminId']; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard constructor input for clearer errors.
public function __construct(
array $values,
) {
- $this->adminId = $values['adminId'];
+ if (!\array_key_exists('adminId', $values)) {
+ throw new \InvalidArgumentException('adminId is required.');
+ }
+ $id = $values['adminId'];
+ if (!\is_string($id) || $id === '') {
+ throw new \InvalidArgumentException('adminId must be a non-empty string.');
+ }
+ $this->adminId = $id;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @param array{ | |
| * adminId: string, | |
| * } $values | |
| */ | |
| public function __construct( | |
| array $values, | |
| ) { | |
| $this->adminId = $values['adminId']; | |
| } | |
| /** | |
| * @param array{ | |
| * adminId: string, | |
| * } $values | |
| */ | |
| public function __construct( | |
| array $values, | |
| ) { | |
| if (!\array_key_exists('adminId', $values)) { | |
| throw new \InvalidArgumentException('adminId is required.'); | |
| } | |
| $id = $values['adminId']; | |
| if (!\is_string($id) || $id === '') { | |
| throw new \InvalidArgumentException('adminId must be a non-empty string.'); | |
| } | |
| $this->adminId = $id; | |
| } |
🤖 Prompt for AI Agents
In src/Admins/Requests/FindAdminRequest.php around lines 14 to 23, the
constructor reads $values['adminId'] without validation; add guards that check
the 'adminId' key exists and that its value is a non-empty string (or matches
expected format), and if not throw an InvalidArgumentException with a clear
message (e.g. "Missing or invalid adminId in FindAdminRequest constructor");
keep the array parameter but perform these checks before assigning
$this->adminId and update the docblock if you change expectations.
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->imageUrl = $values['imageUrl']; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against missing/empty imageUrl in constructor
Referencing $values['imageUrl'] directly will raise a notice (and then a TypeError on assignment) if the key is absent or empty. Add a small validation and accept the snake_case alias for convenience.
- ) {
- $this->imageUrl = $values['imageUrl'];
- }
+ ) {
+ if (\array_key_exists('imageUrl', $values)) {
+ $imageUrl = $values['imageUrl'];
+ } elseif (\array_key_exists('image_url', $values)) {
+ $imageUrl = $values['image_url'];
+ } else {
+ throw new \InvalidArgumentException('imageUrl is required.');
+ }
+ if (!\is_string($imageUrl) || $imageUrl === '') {
+ throw new \InvalidArgumentException('imageUrl must be a non-empty string.');
+ }
+ $this->imageUrl = $imageUrl;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function __construct( | |
| array $values, | |
| ) { | |
| $this->imageUrl = $values['imageUrl']; | |
| } | |
| public function __construct( | |
| array $values, | |
| ) { | |
| if (\array_key_exists('imageUrl', $values)) { | |
| $imageUrl = $values['imageUrl']; | |
| } elseif (\array_key_exists('image_url', $values)) { | |
| $imageUrl = $values['image_url']; | |
| } else { | |
| throw new \InvalidArgumentException('imageUrl is required.'); | |
| } | |
| if (!\is_string($imageUrl) || $imageUrl === '') { | |
| throw new \InvalidArgumentException('imageUrl must be a non-empty string.'); | |
| } | |
| $this->imageUrl = $imageUrl; | |
| } |
🤖 Prompt for AI Agents
In src/Admins/Types/AdminAvatar.php around lines 24 to 28, the constructor
currently accesses $values['imageUrl'] directly which will raise notices or
TypeErrors if the key is missing or empty; update the constructor to validate
and accept a snake_case alias by checking for non-empty 'imageUrl' or
'image_url' keys in $values, assign the found value to $this->imageUrl, and
throw an InvalidArgumentException (or set a sensible default) if neither key is
present or the value is empty.
| /** | ||
| * @param array{ | ||
| * jobIdentifier: string, | ||
| * } $values | ||
| */ | ||
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->jobIdentifier = $values['jobIdentifier']; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate constructor input to avoid TypeErrors.
public function __construct(
array $values,
) {
- $this->jobIdentifier = $values['jobIdentifier'];
+ if (!\array_key_exists('jobIdentifier', $values)) {
+ throw new \InvalidArgumentException('jobIdentifier is required.');
+ }
+ $id = $values['jobIdentifier'];
+ if (!\is_string($id) || $id === '') {
+ throw new \InvalidArgumentException('jobIdentifier must be a non-empty string.');
+ }
+ $this->jobIdentifier = $id;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @param array{ | |
| * jobIdentifier: string, | |
| * } $values | |
| */ | |
| public function __construct( | |
| array $values, | |
| ) { | |
| $this->jobIdentifier = $values['jobIdentifier']; | |
| } | |
| /** | |
| * @param array{ | |
| * jobIdentifier: string, | |
| * } $values | |
| */ | |
| public function __construct( | |
| array $values, | |
| ) { | |
| if (!\array_key_exists('jobIdentifier', $values)) { | |
| throw new \InvalidArgumentException('jobIdentifier is required.'); | |
| } | |
| $id = $values['jobIdentifier']; | |
| if (!\is_string($id) || $id === '') { | |
| throw new \InvalidArgumentException('jobIdentifier must be a non-empty string.'); | |
| } | |
| $this->jobIdentifier = $id; | |
| } |
🤖 Prompt for AI Agents
In src/DataExport/Requests/CancelDataExportRequest.php around lines 14 to 23,
the constructor blindly indexes $values['jobIdentifier'] which can cause
TypeError/undefined index; add explicit validation: check that $values is an
array containing key 'jobIdentifier' and that its value is a non-empty string
(use isset/array_key_exists and is_string), and if validation fails throw an
InvalidArgumentException with a clear message; assign the validated value to
$this->jobIdentifier and consider making the constructor signature stricter
(e.g. accept string $jobIdentifier or declare param as array $values with
docblock) to prevent runtime errors.
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->createdAtAfter = $values['createdAtAfter']; | ||
| $this->createdAtBefore = $values['createdAtBefore']; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate timestamp range and non-negativity in the constructor.
This prevents avoidable 400s from the API and keeps invariants consistent even if setters are never called.
- public function __construct(
- array $values,
- ) {
- $this->createdAtAfter = $values['createdAtAfter'];
- $this->createdAtBefore = $values['createdAtBefore'];
- }
+ public function __construct(array $values)
+ {
+ foreach (['createdAtAfter','createdAtBefore'] as $k) {
+ if (!isset($values[$k]) || !is_int($values[$k]) || $values[$k] < 0) {
+ throw new \InvalidArgumentException("$k must be a non-negative integer (unix timestamp).");
+ }
+ }
+ if ($values['createdAtAfter'] > $values['createdAtBefore']) {
+ throw new \InvalidArgumentException('createdAtAfter must be <= createdAtBefore.');
+ }
+ $this->createdAtAfter = $values['createdAtAfter'];
+ $this->createdAtBefore = $values['createdAtBefore'];
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function __construct( | |
| array $values, | |
| ) { | |
| $this->createdAtAfter = $values['createdAtAfter']; | |
| $this->createdAtBefore = $values['createdAtBefore']; | |
| } | |
| public function __construct(array $values) | |
| { | |
| foreach (['createdAtAfter', 'createdAtBefore'] as $k) { | |
| if (!isset($values[$k]) || !is_int($values[$k]) || $values[$k] < 0) { | |
| throw new \InvalidArgumentException("$k must be a non-negative integer (unix timestamp)."); | |
| } | |
| } | |
| if ($values['createdAtAfter'] > $values['createdAtBefore']) { | |
| throw new \InvalidArgumentException('createdAtAfter must be <= createdAtBefore.'); | |
| } | |
| $this->createdAtAfter = $values['createdAtAfter']; | |
| $this->createdAtBefore = $values['createdAtBefore']; | |
| } |
🤖 Prompt for AI Agents
In src/DataExport/Requests/CreateDataExportRequest.php around lines 28 to 33,
the constructor currently assigns createdAtAfter/createdAtBefore directly from
$values; add validation to ensure these timestamps are numeric integers and
non-negative (cast or is_int/is_numeric check and ensure >= 0), and if both are
provided ensure createdAtAfter <= createdAtBefore; if validation fails throw an
InvalidArgumentException (or appropriate domain exception) with a clear message
so bad input is rejected at construction time rather than later.
| class FindDataExportRequest extends JsonSerializableType | ||
| { | ||
| /** | ||
| * @var string $jobIdentifier job_identifier | ||
| */ | ||
| private string $jobIdentifier; | ||
|
|
||
| /** | ||
| * @param array{ | ||
| * jobIdentifier: string, | ||
| * } $values | ||
| */ | ||
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->jobIdentifier = $values['jobIdentifier']; | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| */ | ||
| public function getJobIdentifier(): string | ||
| { | ||
| return $this->jobIdentifier; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $value | ||
| */ | ||
| public function setJobIdentifier(string $value): self | ||
| { | ||
| $this->jobIdentifier = $value; | ||
| return $this; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Map jobIdentifier to job_identifier and validate input
- Unless
JsonSerializableTypeauto-transforms camelCase->snake_case, this property will serialize asjobIdentifierinstead of the API’s expectedjob_identifier. Add the explicit mapping attribute to prevent request failures. - Add a small guard to ensure a non-empty identifier, since the request is unusable without it.
Apply:
@@
namespace Intercom\DataExport\Requests;
use Intercom\Core\Json\JsonSerializableType;
+use Intercom\Core\Json\JsonProperty;
class FindDataExportRequest extends JsonSerializableType
{
@@
- private string $jobIdentifier;
+ #[JsonProperty("job_identifier")]
+ private string $jobIdentifier;
@@
- public function __construct(
- array $values,
- ) {
- $this->jobIdentifier = $values['jobIdentifier'];
- }
+ public function __construct(array $values,)
+ {
+ if (!isset($values['jobIdentifier']) || $values['jobIdentifier'] === '') {
+ throw new \InvalidArgumentException('jobIdentifier is required and cannot be empty.');
+ }
+ $this->jobIdentifier = $values['jobIdentifier'];
+ }
@@
public function setJobIdentifier(string $value): self
{
$this->jobIdentifier = $value;
return $this;
}
+
+ /**
+ * Convenience constructor.
+ */
+ public static function of(string $jobIdentifier): self
+ {
+ return new self(['jobIdentifier' => $jobIdentifier]);
+ }
}To confirm whether explicit mapping is required elsewhere:
🏁 Script executed:
#!/bin/bash
# Inspect other DTOs for mapping conventions
rg -n --type=php 'JsonProperty\('
# Check DataExport client usage
rg -nC2 --type=php 'class\s+DataExportClient|->find\(|job_identifier'Length of output: 223298
Ensure explicit JSON key mapping and input validation in FindDataExportRequest
The codebase does not auto-map camelCase to snake_case by default (all existing request/response DTOs use #[JsonProperty(...)] to specify JSON keys), and types already include explicit mappings for job_identifier (e.g. DataExport type, PostExportReportingDataEnqueueResponse) (php.net). Therefore:
• Add the #[JsonProperty("job_identifier")] attribute on $jobIdentifier.
• Guard against missing or empty identifiers in the constructor.
• (Optional) Add a static of() factory for convenience.
Apply the diff to src/DataExport/Requests/FindDataExportRequest.php:
--- a/src/DataExport/Requests/FindDataExportRequest.php
+++ b/src/DataExport/Requests/FindDataExportRequest.php
@@
use Intercom\Core\Json\JsonSerializableType;
+use Intercom\Core\Json\JsonProperty;
class FindDataExportRequest extends JsonSerializableType
{
@@
- /** @var string $jobIdentifier job_identifier */
- private string $jobIdentifier;
+ #[JsonProperty("job_identifier")]
+ private string $jobIdentifier;
@@
- public function __construct(
- array $values,
- ) {
- $this->jobIdentifier = $values['jobIdentifier'];
- }
+ public function __construct(array $values)
+ {
+ if (empty($values['jobIdentifier'])) {
+ throw new \InvalidArgumentException('jobIdentifier is required and cannot be empty.');
+ }
+ $this->jobIdentifier = $values['jobIdentifier'];
+ }
@@
public function setJobIdentifier(string $value): self
{
$this->jobIdentifier = $value;
return $this;
}
+
+ /**
+ * Convenience factory.
+ */
+ public static function of(string $jobIdentifier): self
+ {
+ return new self(['jobIdentifier' => $jobIdentifier]);
+ }Tag:
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class FindDataExportRequest extends JsonSerializableType | |
| { | |
| /** | |
| * @var string $jobIdentifier job_identifier | |
| */ | |
| private string $jobIdentifier; | |
| /** | |
| * @param array{ | |
| * jobIdentifier: string, | |
| * } $values | |
| */ | |
| public function __construct( | |
| array $values, | |
| ) { | |
| $this->jobIdentifier = $values['jobIdentifier']; | |
| } | |
| /** | |
| * @return string | |
| */ | |
| public function getJobIdentifier(): string | |
| { | |
| return $this->jobIdentifier; | |
| } | |
| /** | |
| * @param string $value | |
| */ | |
| public function setJobIdentifier(string $value): self | |
| { | |
| $this->jobIdentifier = $value; | |
| return $this; | |
| } | |
| } | |
| use Intercom\Core\Json\JsonSerializableType; | |
| use Intercom\Core\Json\JsonProperty; | |
| class FindDataExportRequest extends JsonSerializableType | |
| { | |
| #[JsonProperty("job_identifier")] | |
| private string $jobIdentifier; | |
| /** | |
| * @param array{ | |
| * jobIdentifier: string, | |
| * } $values | |
| */ | |
| public function __construct(array $values) | |
| { | |
| if (empty($values['jobIdentifier'])) { | |
| throw new \InvalidArgumentException('jobIdentifier is required and cannot be empty.'); | |
| } | |
| $this->jobIdentifier = $values['jobIdentifier']; | |
| } | |
| /** | |
| * @return string | |
| */ | |
| public function getJobIdentifier(): string | |
| { | |
| return $this->jobIdentifier; | |
| } | |
| /** | |
| * @param string $value | |
| */ | |
| public function setJobIdentifier(string $value): self | |
| { | |
| $this->jobIdentifier = $value; | |
| return $this; | |
| } | |
| /** | |
| * Convenience factory. | |
| */ | |
| public static function of(string $jobIdentifier): self | |
| { | |
| return new self(['jobIdentifier' => $jobIdentifier]); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/DataExport/Requests/FindDataExportRequest.php around lines 7 to 41, the
property and constructor lack explicit JSON key mapping and input validation;
add the attribute #[JsonProperty("job_identifier")] to the private string
$jobIdentifier, update the constructor to validate that $values contains a
non-empty 'jobIdentifier' (throw InvalidArgumentException if missing or empty),
and set the property from the validated value; optionally add a public static
function of(array $values): self factory that simply returns new self($values)
for convenience.
| /** | ||
| * @var string $type The value must be user | ||
| */ | ||
| private string $type; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Constrain type (“The value must be user”)
Right now type is any string. Enforce the allowed value(s) to fail fast.
@@
- private string $type;
+ private string $type;
+ private const ALLOWED_TYPES = ['user'];
@@ public function __construct(
- $this->type = $values['type'];
+ $this->setType($values['type']);
@@
- public function setType(string $value): self
+ public function setType(string $value): self
{
- $this->type = $value;
+ if (!in_array($value, self::ALLOWED_TYPES, true)) {
+ throw new \InvalidArgumentException("type must be one of: " . implode(', ', self::ALLOWED_TYPES));
+ }
+ $this->type = $value;
return $this;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @var string $type The value must be user | |
| */ | |
| private string $type; | |
| /** | |
| * @var string $type The value must be user | |
| */ | |
| private string $type; | |
| private const ALLOWED_TYPES = ['user']; | |
| public function __construct(array $values) | |
| { | |
| // other initializations… | |
| $this->setType($values['type']); | |
| } | |
| public function setType(string $value): self | |
| { | |
| if (! in_array($value, self::ALLOWED_TYPES, true)) { | |
| throw new \InvalidArgumentException( | |
| "type must be one of: " . implode(', ', self::ALLOWED_TYPES) | |
| ); | |
| } | |
| $this->type = $value; | |
| return $this; | |
| } |
🤖 Prompt for AI Agents
In src/Events/Requests/ListEventsRequest.php around lines 24 to 28, the $type
property is an unconstrained string but should only allow "user"; change the
code to enforce this by adding a constant (e.g. private const TYPE = 'user') and
set the property default to that constant or validate assignments in the
constructor/setter to ensure only 'user' is accepted, throwing an
InvalidArgumentException on invalid values; alternatively, define a backed enum
for the allowed values and type-hint the property with that enum and update
construction/deserialization accordingly.
| public function __toString(): string | ||
| { | ||
| if (empty($this->body)) { | ||
| return "$this->message; Status Code: $this->code\n"; | ||
| } | ||
| return "$this->message; Status Code: $this->code; Body: " . $this->body . "\n"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden __toString: cast message/code, stringify mixed body safely, and avoid empty() pitfalls.
- Fixes PHPStan complaints about interpolating mixed.
- Prevents "Array to string conversion" warnings when body is array/object.
- Treats only null as “no body,” not "0" or empty JSON.
Apply this diff:
- public function __toString(): string
- {
- if (empty($this->body)) {
- return "$this->message; Status Code: $this->code\n";
- }
- return "$this->message; Status Code: $this->code; Body: " . $this->body . "\n";
- }
+ public function __toString(): string
+ {
+ $message = (string) $this->getMessage();
+ $code = (string) $this->getCode();
+
+ // If no body was provided, render without a Body segment.
+ if ($this->body === null) {
+ return sprintf('%s; Status Code: %s' . PHP_EOL, $message, $code);
+ }
+
+ // Safely stringify mixed body without throwing.
+ $bodyStr = null;
+ if (is_scalar($this->body) || $this->body instanceof \Stringable) {
+ $bodyStr = (string) $this->body;
+ } else {
+ $json = json_encode($this->body, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
+ $bodyStr = $json !== false ? $json : '[unserializable body]';
+ }
+
+ return sprintf('%s; Status Code: %s; Body: %s' . PHP_EOL, $message, $code, $bodyStr);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function __toString(): string | |
| { | |
| if (empty($this->body)) { | |
| return "$this->message; Status Code: $this->code\n"; | |
| } | |
| return "$this->message; Status Code: $this->code; Body: " . $this->body . "\n"; | |
| } | |
| public function __toString(): string | |
| { | |
| $message = (string) $this->getMessage(); | |
| $code = (string) $this->getCode(); | |
| // If no body was provided, render without a Body segment. | |
| if ($this->body === null) { | |
| return sprintf('%s; Status Code: %s' . PHP_EOL, $message, $code); | |
| } | |
| // Safely stringify mixed body without throwing. | |
| $bodyStr = null; | |
| if (is_scalar($this->body) || $this->body instanceof \Stringable) { | |
| $bodyStr = (string) $this->body; | |
| } else { | |
| $json = json_encode($this->body, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); | |
| $bodyStr = $json !== false ? $json : '[unserializable body]'; | |
| } | |
| return sprintf('%s; Status Code: %s; Body: %s' . PHP_EOL, $message, $code, $bodyStr); | |
| } |
🧰 Tools
🪛 PHPStan (2.1.17)
49-49: Part $this->code (mixed) of encapsed string cannot be cast to string.
(encapsedStringPart.nonString)
49-49: Part $this->message (mixed) of encapsed string cannot be cast to string.
(encapsedStringPart.nonString)
51-51: Binary operation "." between non-falsy-string and mixed results in an error.
(binaryOp.invalid)
51-51: Part $this->code (mixed) of encapsed string cannot be cast to string.
(encapsedStringPart.nonString)
51-51: Part $this->message (mixed) of encapsed string cannot be cast to string.
(encapsedStringPart.nonString)
🤖 Prompt for AI Agents
In src/Exceptions/IntercomApiException.php around lines 46 to 52, the __toString
method should treat only null as “no body,” cast message and code to avoid
PHPStan/interpolation issues, and safely convert mixed body values to a string
to avoid "Array to string conversion" warnings: change the empty() check to
strict null check ($this->body === null), cast $this->message to string and
$this->code to int or string when building the output, and for body stringify it
by using is_scalar to cast scalars to string and for arrays/objects use
json_encode($this->body, JSON_UNESCAPED_UNICODE) with a fallback (e.g. serialize
or var_export) if json_encode fails; return a single consistent string including
the serialized body when present.
README.md
Outdated
| Exceptions are handled by HTTPlug. Every exception thrown implements `Http\Client\Exception`. See the [http client exceptions](http://docs.php-http.org/en/latest/httplug/exceptions.html) and the [client and server errors](http://docs.php-http.org/en/latest/plugins/error.html). | ||
| The Intercom API may return an unsuccessful HTTP response, for example when a resource is not found (404). |
There was a problem hiding this comment.
Fix grammatical issues in exception handling documentation.
There are grammatical issues with the sentence structure in the exception handling section.
Apply this diff to fix the grammatical issues:
-Exceptions are handled by HTTPlug. Every exception thrown implements `Http\Client\Exception`. See the [http client exceptions](http://docs.php-http.org/en/latest/httplug/exceptions.html) and the [client and server errors](http://docs.php-http.org/en/latest/plugins/error.html).
-The Intercom API may return an unsuccessful HTTP response, for example when a resource is not found (404).
-If you want to catch errors you can wrap your API call into a try/catch block:
+Exceptions are handled by HTTPlug. Every exception thrown implements `Http\Client\Exception`. See the [http client exceptions](http://docs.php-http.org/en/latest/httplug/exceptions.html) and the [client and server errors](http://docs.php-http.org/en/latest/plugins/error.html).
+The Intercom API may return an unsuccessful HTTP response, for example, when a resource is not found (404).
+If you want to catch errors, you can wrap your API call into a try/catch block:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Exceptions are handled by HTTPlug. Every exception thrown implements `Http\Client\Exception`. See the [http client exceptions](http://docs.php-http.org/en/latest/httplug/exceptions.html) and the [client and server errors](http://docs.php-http.org/en/latest/plugins/error.html). | |
| The Intercom API may return an unsuccessful HTTP response, for example when a resource is not found (404). | |
| Exceptions are handled by HTTPlug. Every exception thrown implements `Http\Client\Exception`. See the [http client exceptions](http://docs.php-http.org/en/latest/httplug/exceptions.html) and the [client and server errors](http://docs.php-http.org/en/latest/plugins/error.html). | |
| The Intercom API may return an unsuccessful HTTP response, for example, when a resource is not found (404). | |
| If you want to catch errors, you can wrap your API call into a try/catch block: |
🧰 Tools
🪛 LanguageTool
[grammar] ~622-~622: There might be a mistake here.
Context: ...-http.org/en/latest/plugins/error.html). The Intercom API may return an unsuccess...
(QB_NEW_EN)
[grammar] ~623-~623: There might be a mistake here.
Context: ...mple when a resource is not found (404). If you want to catch errors you can wrap...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In README.md around lines 622 to 623, fix awkward grammar in the exception
handling section by merging and rephrasing the sentences: replace "Exceptions
are handled by HTTPlug. Every exception thrown implements
`Http\Client\Exception`." with a single clear sentence such as "Exceptions are
handled by HTTPlug; all exceptions thrown implement `Http\Client\Exception`."
and ensure the rest reads smoothly (keep the links and the sentence about the
Intercom API possibly returning unsuccessful HTTP responses, e.g. 404).
| This SDK is built to work with any HTTP client that implements Guzzle's `ClientInterface`. | ||
| By default, if no client is provided, the SDK will use Guzzle's default HTTP client. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify HTTP client interface requirements.
The documentation mentions implementing "Guzzle's ClientInterface" but should clarify the specific interface.
Apply this diff to clarify the interface requirement:
-This SDK is built to work with any HTTP client that implements Guzzle's `ClientInterface`.
+This SDK is built to work with any HTTP client that implements `GuzzleHttp\ClientInterface`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This SDK is built to work with any HTTP client that implements Guzzle's `ClientInterface`. | |
| By default, if no client is provided, the SDK will use Guzzle's default HTTP client. | |
| This SDK is built to work with any HTTP client that implements `GuzzleHttp\ClientInterface`. | |
| By default, if no client is provided, the SDK will use Guzzle's default HTTP client. |
🤖 Prompt for AI Agents
In README.md around lines 709 to 710, the text mentions "Guzzle's
`ClientInterface`" but is ambiguous about which specific interface/version and
required methods; update the documentation to explicitly state the
fully-qualified interface (e.g., GuzzleHttp\ClientInterface or PSR-18
ClientInterface if applicable), the minimum supported Guzzle version, and list
the expected methods/signature (or note that any PSR-18/PSR-7 compatible client
is supported). Replace the vague phrase with a clear statement like: which
fully-qualified interface is required, the supported versions, and a brief note
about PSR compatibility and default behavior when no client is provided.
| /** | ||
| * @var string $createdAtAfter The start date that you request data for. It must be formatted as a UNIX timestamp. | ||
| */ | ||
| private string $createdAtAfter; | ||
|
|
||
| /** | ||
| * @var ?string $createdAtBefore The end date that you request data for. It must be formatted as a UNIX timestamp. | ||
| */ | ||
| private ?string $createdAtBefore; | ||
|
|
There was a problem hiding this comment.
Map fields to snake_case JSON keys (created_at_after / created_at_before).
Without explicit mapping these will serialize as camelCase, which is inconsistent with other DTOs in this PR and likely incorrect for the Intercom API.
use Intercom\Core\Json\JsonSerializableType;
+use Intercom\Core\Json\JsonProperty;
class ListAllActivityLogsRequest extends JsonSerializableType
{
/**
* @var string $createdAtAfter The start date that you request data for. It must be formatted as a UNIX timestamp.
*/
- private string $createdAtAfter;
+ #[JsonProperty('created_at_after')]
+ private string $createdAtAfter;
@@
/**
* @var ?string $createdAtBefore The end date that you request data for. It must be formatted as a UNIX timestamp.
*/
- private ?string $createdAtBefore;
+ #[JsonProperty('created_at_before')]
+ private ?string $createdAtBefore;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @var string $createdAtAfter The start date that you request data for. It must be formatted as a UNIX timestamp. | |
| */ | |
| private string $createdAtAfter; | |
| /** | |
| * @var ?string $createdAtBefore The end date that you request data for. It must be formatted as a UNIX timestamp. | |
| */ | |
| private ?string $createdAtBefore; | |
| use Intercom\Core\Json\JsonSerializableType; | |
| use Intercom\Core\Json\JsonProperty; | |
| class ListAllActivityLogsRequest extends JsonSerializableType | |
| { | |
| /** | |
| * @var string $createdAtAfter The start date that you request data for. It must be formatted as a UNIX timestamp. | |
| */ | |
| #[JsonProperty('created_at_after')] | |
| private string $createdAtAfter; | |
| /** | |
| * @var ?string $createdAtBefore The end date that you request data for. It must be formatted as a UNIX timestamp. | |
| */ | |
| #[JsonProperty('created_at_before')] | |
| private ?string $createdAtBefore; | |
| // ... rest of the class ... | |
| } |
🤖 Prompt for AI Agents
In src/Admins/Requests/ListAllActivityLogsRequest.php around lines 9 to 18, the
two properties will serialize to camelCase by default; add explicit
serialization mappings so they emit snake_case JSON keys. Annotate/attribute the
properties to map createdAtAfter -> created_at_after and createdAtBefore ->
created_at_before (e.g. add #[SerializedName('created_at_after')] and
#[SerializedName('created_at_before')] or the equivalent @SerializedName
annotations your serializer uses), keep the existing property types (nullable
for createdAtBefore) and add the necessary use/import for the SerializedName
annotation.
| /** | ||
| * @var array<int> $teamIds This object represents the avatar associated with the admin. | ||
| */ | ||
| #[JsonProperty('team_ids'), ArrayType(['integer'])] | ||
| private array $teamIds; |
There was a problem hiding this comment.
Fix incorrect docblock for teamIds
The docstring describes an avatar, but the property is a list of team IDs. This is misleading for consumers and IDEs.
Apply this diff to correct it:
- /**
- * @var array<int> $teamIds This object represents the avatar associated with the admin.
- */
+ /**
+ * @var array<int> $teamIds The list of team IDs this admin belongs to.
+ */🤖 Prompt for AI Agents
In src/Admins/Types/Admin.php around lines 63 to 67, the docblock for $teamIds
incorrectly describes an avatar; update the PHPDoc to accurately describe the
property as a list of team IDs (e.g., change the description to "List of team
IDs the admin belongs to" or similar) while keeping the @var array<int> type and
existing attributes intact so IDEs and consumers see the correct purpose.
| /** | ||
| * @var ?int $helpCenterId The ID of the Help Center to search in. | ||
| */ | ||
| private ?int $helpCenterId; | ||
|
|
There was a problem hiding this comment.
Missing JSON key mapping for helpCenterId
Intercom endpoints typically expect help_center_id. Without #[JsonProperty], this risks sending helpCenterId. Add the attribute and import.
@@
-use Intercom\Core\Json\JsonSerializableType;
+use Intercom\Core\Json\JsonSerializableType;
+use Intercom\Core\Json\JsonProperty;
@@
- private ?int $helpCenterId;
+ #[JsonProperty('help_center_id')]
+ private ?int $helpCenterId;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @var ?int $helpCenterId The ID of the Help Center to search in. | |
| */ | |
| private ?int $helpCenterId; | |
| use Intercom\Core\Json\JsonSerializableType; | |
| use Intercom\Core\Json\JsonProperty; | |
| /** | |
| * @var ?int $helpCenterId The ID of the Help Center to search in. | |
| */ | |
| #[JsonProperty('help_center_id')] | |
| private ?int $helpCenterId; |
🤖 Prompt for AI Agents
In src/Articles/Requests/SearchArticlesRequest.php around lines 19 to 23, the
private property $helpCenterId lacks a JSON mapping attribute so it will
serialize as helpCenterId instead of the expected help_center_id; add the
appropriate #[JsonProperty('help_center_id')] attribute above the property and
ensure the JsonProperty attribute class is imported at the top of the file (add
the use statement for the JsonProperty namespace used in the project).
| use Intercom\Core\Json\JsonSerializableType; | ||
|
|
||
| class DownloadDataExportRequest extends JsonSerializableType | ||
| { | ||
| /** | ||
| * @var string $jobIdentifier job_identifier | ||
| */ | ||
| private string $jobIdentifier; | ||
|
|
There was a problem hiding this comment.
Add explicit JSON mapping for job_identifier to avoid API mismatch.
Without a JsonProperty mapping, this will serialize as "jobIdentifier" instead of the expected "job_identifier", likely causing request failures or ignored fields.
Apply this diff:
use Intercom\Core\Json\JsonSerializableType;
+use Intercom\Core\Json\JsonProperty;
class DownloadDataExportRequest extends JsonSerializableType
{
/**
* @var string $jobIdentifier job_identifier
*/
- private string $jobIdentifier;
+ #[JsonProperty('job_identifier')]
+ private string $jobIdentifier;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use Intercom\Core\Json\JsonSerializableType; | |
| class DownloadDataExportRequest extends JsonSerializableType | |
| { | |
| /** | |
| * @var string $jobIdentifier job_identifier | |
| */ | |
| private string $jobIdentifier; | |
| ++ b/src/DataExport/Requests/DownloadDataExportRequest.php | |
| @@ | |
| use Intercom\Core\Json\JsonSerializableType; | |
| use Intercom\Core\Json\JsonProperty; | |
| class DownloadDataExportRequest extends JsonSerializableType | |
| { | |
| @@ | |
| /** | |
| * @var string $jobIdentifier job_identifier | |
| */ | |
| #[JsonProperty('job_identifier')] | |
| private string $jobIdentifier; |
🤖 Prompt for AI Agents
In src/DataExport/Requests/DownloadDataExportRequest.php around lines 5 to 13,
the private property $jobIdentifier will serialize as "jobIdentifier" but the
API expects "job_identifier"; add an explicit JSON mapping so the serialized key
is "job_identifier". Fix by annotating or configuring the JsonSerializableType
mapping for this property (e.g., add the appropriate JsonProperty/attribute or
update the class' json property map) to map jobIdentifier -> "job_identifier",
keep the property name the same and ensure serialization/deserialization use the
snake_case key.
|
|
||
| namespace Intercom\Events; | ||
|
|
||
| use GuzzleHttp\ClientInterface; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Adopt PSR-18 ClientInterface in options.
Same concern as other clients: using GuzzleHttp\ClientInterface ties options to Guzzle.
-use GuzzleHttp\ClientInterface;
+use Psr\Http\Client\ClientInterface;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use GuzzleHttp\ClientInterface; | |
| use Psr\Http\Client\ClientInterface; |
🤖 Prompt for AI Agents
In src/Events/EventsClient.php around line 5, replace the Guzzle-specific
import/use and typehints with the PSR-18 client: change use
GuzzleHttp\ClientInterface to use Psr\Http\Client\ClientInterface, update any
constructor, options array typing, parameter and return typehints and related
phpdoc annotations to reference Psr\Http\Client\ClientInterface, and remove or
stop referencing the Guzzle interface so the client accepts any PSR-18
implementation (ensure composer dependency for psr/http-client is present if not
already).
| public function list(ListEventsRequest $request, ?array $options = null): DataEventSummary | ||
| { | ||
| $options = array_merge($this->options, $options ?? []); | ||
| $query = []; | ||
| $query['type'] = $request->getType(); | ||
| if ($request->getUserId() != null) { | ||
| $query['user_id'] = $request->getUserId(); |
There was a problem hiding this comment.
Method name 'list' is risky and can conflict with the PHP language construct.
Rename to something explicit like listEvents.
- public function list(ListEventsRequest $request, ?array $options = null): DataEventSummary
+ public function listEvents(ListEventsRequest $request, ?array $options = null): DataEventSummaryAlso update call sites accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function list(ListEventsRequest $request, ?array $options = null): DataEventSummary | |
| { | |
| $options = array_merge($this->options, $options ?? []); | |
| $query = []; | |
| $query['type'] = $request->getType(); | |
| if ($request->getUserId() != null) { | |
| $query['user_id'] = $request->getUserId(); | |
| public function listEvents(ListEventsRequest $request, ?array $options = null): DataEventSummary | |
| { | |
| $options = array_merge($this->options, $options ?? []); | |
| $query = []; | |
| $query['type'] = $request->getType(); | |
| if ($request->getUserId() != null) { | |
| $query['user_id'] = $request->getUserId(); |
🤖 Prompt for AI Agents
In src/Events/EventsClient.php around lines 89 to 95, the public method named
'list' conflicts with the PHP language construct and should be renamed to a
clearer identifier like 'listEvents'; rename the method signature and all
internal references in this class accordingly, update the return type
declaration if unchanged, and then find and update every external call site
(tests, other classes, DI/container configs) to use 'listEvents' instead of
'list' to avoid breaking changes.
| public function summaries(ListEventSummariesRequest $request = new ListEventSummariesRequest(), ?array $options = null): void | ||
| { | ||
| $options = array_merge($this->options, $options ?? []); | ||
| try { | ||
| $response = $this->client->sendRequest( | ||
| new JsonApiRequest( | ||
| baseUrl: $options['baseUrl'] ?? $this->client->options['baseUrl'] ?? Environments::UsProduction->value, |
There was a problem hiding this comment.
Default parameter using new is invalid; initialize inside.
public function summaries(ListEventSummariesRequest $request = new ListEventSummariesRequest(), ...) won’t parse in PHP.
- public function summaries(ListEventSummariesRequest $request = new ListEventSummariesRequest(), ?array $options = null): void
+ public function summaries(?ListEventSummariesRequest $request = null, ?array $options = null): void
{
- $options = array_merge($this->options, $options ?? []);
+ $request ??= new ListEventSummariesRequest();
+ $options = array_merge($this->options, $options ?? []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function summaries(ListEventSummariesRequest $request = new ListEventSummariesRequest(), ?array $options = null): void | |
| { | |
| $options = array_merge($this->options, $options ?? []); | |
| try { | |
| $response = $this->client->sendRequest( | |
| new JsonApiRequest( | |
| baseUrl: $options['baseUrl'] ?? $this->client->options['baseUrl'] ?? Environments::UsProduction->value, | |
| public function summaries(?ListEventSummariesRequest $request = null, ?array $options = null): void | |
| { | |
| $request ??= new ListEventSummariesRequest(); | |
| $options = array_merge($this->options, $options ?? []); | |
| try { | |
| $response = $this->client->sendRequest( | |
| new JsonApiRequest( | |
| baseUrl: $options['baseUrl'] ?? $this->client->options['baseUrl'] ?? Environments::UsProduction->value, |
🤖 Prompt for AI Agents
In src/Events/EventsClient.php around lines 257 to 263, the method signature
uses a default value created with new (ListEventSummariesRequest) which is
invalid in PHP; change the signature to accept ?ListEventSummariesRequest
$request = null (or no default) and then inside the method check if $request is
null and instantiate it ($request = $request ?? new
ListEventSummariesRequest()); also update any docblock/type hints accordingly
and ensure downstream code uses the local $request variable.
| /** | ||
| * @var ?CreateDataEventSummariesRequestEventSummaries $eventSummaries A list of event summaries for the user. Each event summary should contain the event name, the time the event occurred, and the number of times the event occurred. The event name should be a past tense 'verb-noun' combination, to improve readability, for example `updated-plan`. | ||
| */ | ||
| #[JsonProperty('event_summaries')] | ||
| private ?CreateDataEventSummariesRequestEventSummaries $eventSummaries; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm whether eventSummaries should be a collection.
The docstring says “A list of event summaries,” but the type is a single CreateDataEventSummariesRequestEventSummaries. If it’s actually an array/collection, we should adjust the type and serialization accordingly.
Run to inspect the referenced type and its structure:
🏁 Script executed:
#!/bin/bash
# Inspect the type to verify if it encapsulates a list or a single item
rg -nP -C2 --type=php $'class\\s+CreateDataEventSummariesRequestEventSummaries\\b'
# Also search call sites to see how eventSummaries is built/passed
rg -nP -C3 --type=php $'eventSummaries\\s*[:=]'Length of output: 4124
🏁 Script executed:
#!/bin/bash
# Display the content of the event summaries type class to verify its structure
sed -n '1,200p' src/Events/Types/CreateDataEventSummariesRequestEventSummaries.phpLength of output: 2930
eventSummaries must be a collection, not a single object
The ListEventSummariesRequest::$eventSummaries property is currently typed as a single CreateDataEventSummariesRequestEventSummaries instance, but both its docblock (“A list of event summaries…”) and the endpoint’s semantics imply that this should be an array of summaries. You’ll need to:
• Change the property’s type to ?CreateDataEventSummariesRequestEventSummaries[] (or array<CreateDataEventSummariesRequestEventSummaries>|null).
• Update the docblock to reflect the array type:
/**
* @var ?CreateDataEventSummariesRequestEventSummaries[] $eventSummaries
*/• Modify the JsonProperty usage (or attribute parameters) so that the serializer knows to deserialize an array of nested objects.
• In the constructor, map each item in $values['eventSummaries'] to a new CreateDataEventSummariesRequestEventSummaries instance.
• Change the setter signature to accept ?array $value = null (or the typed array) and adjust getEventSummaries(): ?array.
These changes are required to correctly model “a list of event summaries” and ensure proper (de)serialization.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
composer.json (1)
31-35: Fix build script: php -l cannot lint directories; use a real linter.
php -l srcandphp -l testswill fail. Switch to php-parallel-lint (portable), or use find+xargs.Option A (recommended, portable):
"require-dev": { "phpunit/phpunit": "^9.0", - "friendsofphp/php-cs-fixer": "3.5.0", + "friendsofphp/php-cs-fixer": "^3.5.0", - "phpstan/phpstan": "^1.12" + "phpstan/phpstan": "^1.12", + "php-parallel-lint/php-parallel-lint": "^1.3" }, "scripts": { - "build": [ - "@php -l src", - "@php -l tests" - ], + "build": "parallel-lint src tests",Option B (no new dev dep, Unix-only):
- "build": [ - "@php -l src", - "@php -l tests" - ], + "build": "find src tests -type f -name \"*.php\" -print0 | xargs -0 -n1 php -l",README.md (1)
612-615: Grammar fixes in exception section.-The Intercom API may return an unsuccessful HTTP response, for example when a resource is not found (404). -If you want to catch errors you can wrap your API call into a try/catch block: +The Intercom API may return an unsuccessful HTTP response, for example, when a resource is not found (404). +If you want to catch errors, you can wrap your API call in a try/catch block:Also consider aligning this section with the later “Exception Handling” section that references
IntercomApiException; they should not contradict each other.
🧹 Nitpick comments (6)
composer.json (2)
18-18: Loosen cs-fixer version to receive bug fixes.Pinning to "3.5.0" can block security/bugfix updates.
- "friendsofphp/php-cs-fixer": "3.5.0", + "friendsofphp/php-cs-fixer": "^3.5.0",
11-15: Align dependencies with PSR-18/PSR-17 story (Guzzle optional vs. required).README suggests PSR-18 compatibility, but composer hard-requires Guzzle. Either:
- require PSR interfaces and keep Guzzle as a suggestion, or
- explicitly document that Guzzle 7 is required.
Option (PSR-first, Guzzle optional):
"require": { "php": "^8.1", "ext-json": "*", - "guzzlehttp/guzzle": "^7.4" + "psr/http-client": "^1.0", + "psr/http-message": "^1.0", + "psr/http-factory": "^1.0" }, + "suggest": { + "guzzlehttp/guzzle": "PSR-18 client implementation" + },Option (keep Guzzle, also declare PSR interfaces explicitly to match docs):
"require": { "php": "^8.1", "ext-json": "*", - "guzzlehttp/guzzle": "^7.4" + "guzzlehttp/guzzle": "^7.4", + "psr/http-client": "^1.0", + "psr/http-message": "^1.0", + "psr/http-factory": "^1.0" },README.md (4)
68-78: Wording: “overwrite” → “override”.-However, you can overwrite that version for a single request or for all the requests +However, you can override that version for a single request or for all requests
545-549: Use RFC3339/ATOM instead of DateTime::ISO8601 (ambiguous).-print_r($rate_limit['reset_at']->format(DateTime::ISO8601)); +print_r($rate_limit['reset_at']->format(DateTimeInterface::RFC3339));
265-267: API example uses empty string placeholder for ID; provide a dedicated method or clearer example.
getVisitor('', ['user_id' => '...'])is confusing. Prefer an explicit method (getVisitorByUserId) or show a single-parameter variant that accepts criteria.
239-245: Minor wording: “contacts objects” → “contact objects”.- * The result object contains an array of your contacts objects and a scroll_param which you can then + * The result contains an array of your contact objects and a scroll_param that you can
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
README.md(8 hunks)composer.json(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...thorization#section-access-tokens). > > If you are building a third party applicat...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~26-~26: There might be a mistake here.
Context: ... can get your OAuth token by [setting-up-oauth](https://developers.intercom.com/buildi...
(QB_NEW_EN)
[style] ~60-~60: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... Use a custom URI factory This factory needs to implement Http\Message\UriFactory ``...
(REP_NEED_TO_VB)
[grammar] ~612-~612: There might be a mistake here.
Context: ...-http.org/en/latest/plugins/error.html). The Intercom API may return an unsuccess...
(QB_NEW_EN)
[grammar] ~613-~613: There might be a mistake here.
Context: ...mple when a resource is not found (404). If you want to catch errors you can wrap...
(QB_NEW_EN)
[grammar] ~729-~729: There might be a mistake here.
Context: ...ckoff. A request will be retried as long as the request is deemed retryable and t...
(QB_NEW_EN)
[grammar] ~730-~730: There might be a mistake here.
Context: ...has not grown larger than the configured retry limit (default: 2). A request is ...
(QB_NEW_EN)
[grammar] ~735-~735: There might be a mistake here.
Context: ...n-US/docs/Web/HTTP/Status/408) (Timeout) - [429](https://developer.mozilla.org/en-US...
(QB_NEW_EN)
[grammar] ~736-~736: There might be a mistake here.
Context: ...Web/HTTP/Status/429) (Too Many Requests) - [5XX](https://developer.mozilla.org/en-US...
(QB_NEW_EN)
[style] ~767-~767: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...ritten upon the next generated release. Feel free to open a PR as a proof of concept, but kn...
(FEEL_FREE_TO_STYLE_ME)
[grammar] ~768-~768: There might be a mistake here.
Context: ...le to merge it as-is. We suggest opening an issue first to discuss with us! On t...
(QB_NEW_EN)
🔇 Additional comments (2)
composer.json (1)
39-46: LGTM: authors/homepage fields are valid and Composer-friendly.README.md (1)
557-566: Potential reserved-name issue: method named list().If the SDK exposes a method literally named
list, invocation may conflict with the language construct. Verify the actual method name; if it islist, consider renaming tolistArticles()or call via$client->articles->{'list'}(...).// Safer example in docs: $items = $client->articles->listArticles(['limit' => 10]);Also applies to: 574-579
| $client = new IntercomClient( | ||
| '<insert_token_here>', | ||
| null, | ||
| ['Custom-Header' => 'value'] | ||
| ); |
There was a problem hiding this comment.
Constructor signature appears inconsistent across examples; standardize one canonical form.
README shows (token), (token, null, headers), and (token, options: [...]). Pick one (prefer named-args with an options array) and update all snippets.
Example to use consistently:
$client = new IntercomClient(
token: '<token>',
options: [
'headers' => ['Intercom-Version' => '1.1'],
'baseUrl' => 'https://api.intercom.io',
'client' => $psr18Client,
],
);Also applies to: 560-563, 662-664, 711-724
🤖 Prompt for AI Agents
In README.md around lines 35-39 (and also update occurrences at 560-563,
662-664, 711-724), the IntercomClient constructor is shown with inconsistent
signatures; standardize all examples to use the canonical named-argument form
with a single options array. Replace the variants that pass token alone or token
with null and headers with the unified form using token: '<token>' and options:
[ 'headers' => [...], 'baseUrl' => 'https://api.intercom.io', 'client' =>
$psr18Client ] (or omit keys not needed), and ensure every snippet uses the same
parameter names and structure for clarity.
| This SDK is built to work with any HTTP client that implements Guzzle's `ClientInterface`. | ||
| By default, if no client is provided, the SDK will use Guzzle's default HTTP client. | ||
| However, you can pass your own client that adheres to `ClientInterface`: | ||
|
|
||
| ```php | ||
| use Intercom\IntercomClient; | ||
|
|
||
| // Create a custom Guzzle client with specific configuration. | ||
| $customClient = new \GuzzleHttp\Client([ | ||
| 'timeout' => 5.0, | ||
| ]); | ||
|
|
||
| // Pass the custom client when creating an instance of the class. | ||
| $client = new IntercomClient(options: [ | ||
| 'client' => $customClient | ||
| ]); | ||
|
|
||
| // You can also utilize the same technique to leverage advanced customizations to the client such as adding middleware | ||
| $handlerStack = \GuzzleHttp\HandlerStack::create(); | ||
| $handlerStack->push(MyCustomMiddleware::create()); | ||
| $customClient = new \GuzzleHttp\Client(['handler' => $handlerStack]); | ||
|
|
||
| // Pass the custom client when creating an instance of the class. | ||
| $client = new IntercomClient(options: [ | ||
| 'client' => $customClient | ||
| ]); | ||
| ``` |
There was a problem hiding this comment.
Advanced/Custom Client section contradicts PSR-18 guidance; avoid Guzzle-specific interface.
Replace "implements Guzzle's ClientInterface" with PSR-18, keep the Guzzle example as one option.
-This SDK is built to work with any HTTP client that implements Guzzle's `ClientInterface`.
-By default, if no client is provided, the SDK will use Guzzle's default HTTP client.
-However, you can pass your own client that adheres to `ClientInterface`:
+This SDK works with any PSR-18 client (`Psr\Http\Client\ClientInterface`).
+By default, you may pass any PSR-18-compliant client (e.g., Guzzle 7).
+Provide your own client that implements `Psr\Http\Client\ClientInterface`:If Guzzle remains the default, explicitly state “Guzzle 7.x” and still mention PSR-18 compatibility.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This SDK is built to work with any HTTP client that implements Guzzle's `ClientInterface`. | |
| By default, if no client is provided, the SDK will use Guzzle's default HTTP client. | |
| However, you can pass your own client that adheres to `ClientInterface`: | |
| ```php | |
| use Intercom\IntercomClient; | |
| // Create a custom Guzzle client with specific configuration. | |
| $customClient = new \GuzzleHttp\Client([ | |
| 'timeout' => 5.0, | |
| ]); | |
| // Pass the custom client when creating an instance of the class. | |
| $client = new IntercomClient(options: [ | |
| 'client' => $customClient | |
| ]); | |
| // You can also utilize the same technique to leverage advanced customizations to the client such as adding middleware | |
| $handlerStack = \GuzzleHttp\HandlerStack::create(); | |
| $handlerStack->push(MyCustomMiddleware::create()); | |
| $customClient = new \GuzzleHttp\Client(['handler' => $handlerStack]); | |
| // Pass the custom client when creating an instance of the class. | |
| $client = new IntercomClient(options: [ | |
| 'client' => $customClient | |
| ]); | |
| ``` | |
| This SDK works with any PSR-18 client (`Psr\Http\Client\ClientInterface`). | |
| By default, you may pass any PSR-18-compliant client (e.g., Guzzle 7). | |
| Provide your own client that implements `Psr\Http\Client\ClientInterface`: | |
🤖 Prompt for AI Agents
In README.md around lines 699 to 725, update the "Advanced/Custom Client"
section to stop referencing "implements Guzzle's ClientInterface" and instead
state that the SDK accepts any PSR-18 compatible HTTP client (example:
Http\Client\ClientInterface from PSR-18); explicitly note that if no client is
provided the SDK defaults to Guzzle 7.x, and keep the Guzzle-specific example as
an optional illustration of one supported client and how to configure
middleware, but do not present Guzzle as the required interface.
* fresh readme generation * nits --------- Co-authored-by: jsklan <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
README.md (5)
49-59: Complete the exception example or remove the unused import.
IntercomExceptionis imported but not demonstrated. Either remove it or add a catch for transport/SDK errors.try { $response = $client->aiContent->createContentImportSource(...); } catch (IntercomApiException $e) { echo 'API Exception occurred: ' . $e->getMessage() . "\n"; echo 'Status Code: ' . $e->getCode() . "\n"; echo 'Response Body: ' . $e->getBody() . "\n"; // Optionally, rethrow the exception or handle accordingly. -} +} catch (IntercomException $e) { + echo 'SDK/Transport exception: ' . $e->getMessage() . "\n"; +}
3-3: Fix duplicate UTM parameter in the Fern badge URL.There are two utm_source params.
-[](https://buildwithfern.com?utm_source=github&utm_medium=github&utm_campaign=readme&utm_source=https%3A%2F%2Fround-lake.dustinice.workers.dev%3A443%2Fhttps%2Fgithub.com%2Fintercom%2Fintercom-php) +[](https://buildwithfern.com?utm_source=github&utm_medium=github&utm_campaign=readme)
35-40: Use realistic placeholder values in examples.Replace
'url' => 'url'with a clear example URL.- 'url' => 'url', + 'url' => 'https://example.com/source.json',
69-72: Use Intercom base URL in examples (optional).Using a non-Intercom URL here may confuse readers.
- options: ['baseUrl' => 'https://api.example.com'], + options: ['baseUrl' => 'https://api.intercom.io'],
95-97: Clarify default HTTP client behavior in README (README.md:95-97)README says “uses Guzzle's default HTTP client” — update to state that when no client is provided the SDK constructs a GuzzleHttp\Client (see src/Core/Client/RawClient::createDefaultClient) with a HandlerStack that pushes the SDK's RetryMiddleware; to use a custom client pass an instance implementing GuzzleHttp\ClientInterface via options['client'].
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(1 hunks)
🔇 Additional comments (3)
README.md (3)
31-33: Standardize constructor usage: prefer named arguments.Examples mix positional and named-args. Use the named-arg pattern everywhere for clarity.
Apply this diff to the pagination example:
-$client = new IntercomClient( - '<token>', - ['baseUrl' => 'https://api.example.com'], -); +$client = new IntercomClient( + token: '<token>', + options: ['baseUrl' => 'https://api.example.com'], +);Also applies to: 69-72
95-99: Avoid Guzzle-specific interface; document PSR-18 instead.The SDK should advertise PSR-18 (
Psr\Http\Client\ClientInterface) compatibility, with Guzzle as one option.-This SDK is built to work with any HTTP client that implements Guzzle's `ClientInterface`. -By default, if no client is provided, the SDK will use Guzzle's default HTTP client. -However, you can pass your own client that adheres to `ClientInterface`: +This SDK works with any PSR-18 HTTP client (`Psr\Http\Client\ClientInterface`). +You can pass any PSR-18–compatible client (e.g., Guzzle 7 via guzzlehttp/psr18-adapter):
74-79: Incorrect —listmethod exists; README example is valid.
The repository declares public function list(...) on client classes (e.g. src/Articles/ArticlesClient.php:83), so $client->articles->list([...]) is a valid call.Likely an incorrect or invalid review comment.
* initial src file inclusion * add depts to composer.json * raw copy tests * update namespace and use statements * allow php-http discovery * update phpstan.neon to exclude legacy files from composer analyze * update readme * remove legacy tests * update version mentioned in readme --------- Co-authored-by: jsklan <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
README.md (1)
116-141: Doc still mandates Guzzle’s interface instead of PSR-18.The new client stack depends on PSR-18 + PSR-17 factories, yet this section says the SDK “implements Guzzle's
ClientInterface” and frames Guzzle as the default. Reword it to specify anyPsr\Http\Client\ClientInterfaceclient works (with Guzzle 7 as one option) and mention PSR-17 factories if customization is needed.composer.json (1)
38-41:composer run buildcurrently fails.
php -lcannot lint directories, so both commands exit with “Cannot open file” errors. Swap to a file-aware linter (e.g., php-parallel-lint) or a finder-based loop so the build script works cross-platform.- "build": [ - "@php -l src", - "@php -l tests" - ], + "build": "parallel-lint src tests",(Add
php-parallel-lint/php-parallel-linttorequire-devif you choose this fix.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.fernignore(1 hunks)README.md(1 hunks)composer.json(1 hunks)phpstan.neon(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .fernignore
- phpstan.neon
Co-authored-by: fern-api[bot] <115122769+fern-api[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (9)
src/Companies/Requests/UpdateCompanyRequest.php (2)
8-18: MissingjsonSerialize()override causes incorrect API key format.This class extends
JsonSerializableTypebut does not overridejsonSerialize(), so thecompanyIdproperty will be serialized as"companyId"instead of the snake_case"company_id"that Intercom's API expects. This issue was previously identified and remains unresolved.
26-31: Constructor lacks validation for requiredcompanyIdparameter.Directly accessing
$values['companyId']without validation will produce unclear errors if the key is missing or contains invalid data. This issue was previously identified and remains unresolved.README.md (1)
142-154: Custom client examples still omit the requiredtokenargument.The
IntercomClientconstructor requires authentication. These snippets will throw aTypeError.-$client = new IntercomClient(options: [ - 'client' => $customClient -]); +$client = new IntercomClient( + token: '<token>', + options: [ + 'client' => $customClient, + ], +);Apply to both custom client examples in this section.
src/Articles/Requests/DeleteArticleRequest.php (1)
19-23: ValidatearticleIdbefore assigning to typed propertyThe constructor indexes
$values['articleId']directly and assigns to a typedintproperty. If the key is missing or not anint, you’ll get a notice and likely aTypeErrorat runtime.It would be safer to validate presence and type and throw a clear
InvalidArgumentExceptioninstead of relying on runtime errors. This also aligns with previous feedback on this constructor.- public function __construct( - array $values, - ) { - $this->articleId = $values['articleId']; - } + public function __construct( + array $values, + ) { + if ( + !\array_key_exists('articleId', $values) + || !\is_int($values['articleId']) + || $values['articleId'] <= 0 + ) { + throw new \InvalidArgumentException('articleId is required and must be a positive integer.'); + } + + $this->articleId = $values['articleId']; + }src/Admins/Requests/FindAdminRequest.php (1)
14-23: Consider validatingadminIdin the constructor for clearer failuresThe constructor currently assumes
$values['adminId']is present and a valid int; if it’s missing or wrong-typed, callers will get an undefined-index notice or a TypeError on assignment rather than a clear argument error. Adding an explicitarray_key_exists+ type check with anInvalidArgumentExceptionwould give a much better signal to SDK consumers.src/Contacts/Types/ContactsFindResponse.php (1)
74-123: Constructor follows the same pattern as other contact response types.This file has the same repetitive field initialization pattern as ContactsCreateResponse. The duplication across multiple response files (ContactsCreateResponse, ContactsFindResponse, ShowContactByExternalIdResponse) suggests a potential refactoring opportunity, though the explicit approach provides type safety and IDE support.
src/Contacts/Types/ShowContactByExternalIdResponse.php (1)
74-123: Constructor follows the same pattern as other contact response types.This file has the same repetitive field initialization pattern as ContactsCreateResponse and ContactsFindResponse. The duplication has been noted in the earlier review comments.
src/Conversations/ConversationsClient.php (1)
448-448: Fix conversation rating field name typo in docs (admin_d→admin_id)The “Accepted Fields” tables for conversation search list
conversation_rating.admin_d, which appears to be a typo forconversation_rating.admin_id. This typo occurs in both the mainsearchdoc block and the_searchhelper’s repeated documentation; updating both toconversation_rating.admin_idwill keep the docs consistent with the underlying schema and avoid confusing consumers.- * | conversation_rating.admin_d | String | + * | conversation_rating.admin_id | String |Also applies to: 1063-1063
src/Admins/Types/Admin.php (1)
69-73: Correct misleading PHPDoc for$teamIdsThe
$teamIdsdocblock currently describes an avatar, but the property is actually anarray<int>of team IDs. Updating the description to reflect that it’s a list of team IDs will make IDE hints and generated docs accurate.- /** - * @var array<int> $teamIds This object represents the avatar associated with the admin. - */ + /** + * @var array<int> $teamIds The list of team IDs this admin belongs to. + */
🧹 Nitpick comments (16)
src/AiAgent/Types/AiAgent.php (1)
38-42: Consider documenting validation responsibility for rating constraint.The docblock specifies the rating is "from 1-5" but no validation enforces this in the setter. If this class is primarily a read-only DTO for API responses, this is acceptable. Otherwise, consider adding validation or clarifying in the docblock where validation occurs.
Also applies to: 174-178
src/Companies/Requests/UpdateCompanyRequest.php (1)
44-48: Consider validating non-empty string in setter.The
setCompanyIdsetter accepts any string including empty strings. Even if constructor validation prevents initial empty values, this setter could later allow an invalid state.Apply this diff to add validation:
public function setCompanyId(string $value): self { + if ($value === '') { + throw new \InvalidArgumentException('companyId must be a non-empty string.'); + } $this->companyId = $value; return $this; }.github/workflows/ci.yml (3)
3-3: Consider addingpull_requesttrigger for PR validation.The workflow only triggers on
push. Addingpull_requestensures CI runs before merging, catching issues earlier.-on: [push] +on: [push, pull_request]
5-48: Add Composer dependency caching to speed up CI runs.Both jobs install dependencies without caching. This increases CI time unnecessarily.
- name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: "8.1" + - name: Cache Composer dependencies + uses: actions/cache@v4 + with: + path: vendor + key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} + restore-keys: ${{ runner.os }}-composer- - name: Install tools run: | composer installApply similar caching to both jobs.
30-48: Consider adding job dependency to avoid redundant runs on failure.If
compilefails (build or analyze), runningtestmay be wasteful. Addingneeds: compileensures tests only run after successful compilation/analysis.test: runs-on: ubuntu-latest + needs: compileREADME.md (1)
81-84: Constructor style inconsistency with other examples.This example uses positional arguments while the main usage example (lines 44-46) uses named arguments. Consider standardizing for clarity.
-$client = new IntercomClient( - '<token>', - ['baseUrl' => 'https://api.example.com'], -); +$client = new IntercomClient( + token: '<token>', + options: ['baseUrl' => 'https://api.example.com'], +);src/Contacts/Types/ContactsUpdateResponse.php (1)
14-148: ContactsUpdateResponse: enabledPushMessaging wiring looks correctThe new
enabledPushMessagingfield is properly declared, mapped via#[JsonProperty('enabled_push_messaging')], initialized in the constructor, and exposed through getter/setter and__toString(). If this flag is or becomes common across multiple contact response types, you might consider moving it into theContacttrait to keep the model DRY, but the current implementation is functionally sound.src/Contacts/Types/ContactsCreateResponse.php (1)
74-123: Consider using a base class or automated mapping for repetitive field initialization.The constructor contains 46 lines of repetitive field assignments. While functionally correct, this pattern appears across multiple contact response types (ContactsCreateResponse, ContactsFindResponse, ShowContactByExternalIdResponse) and could benefit from consolidation.
Options to reduce duplication:
- Extract the common initialization logic into the Contact trait
- Use reflection or a property mapper to automate field assignment from the array
- Accept that explicit assignments provide better IDE support and type safety
Given that explicit initialization provides clarity and type safety, this may be acceptable as-is, but consider the maintenance burden if these DTOs need updates.
src/AiContent/Requests/CreateContentImportSourceRequest.php (1)
11-93: Optional: enforce'api'at runtime forsyncBehaviorDocs and phpdoc model
syncBehavioras the literal'api', but the implementation accepts any string. If you want to fail fast on misuse, you could add a simple runtime guard in the constructor (or setter), e.g.:public function __construct( array $values, ) { - $this->syncBehavior = $values['syncBehavior']; + $this->syncBehavior = $values['syncBehavior']; + if ($this->syncBehavior !== 'api') { + throw new \InvalidArgumentException('syncBehavior must be "api".'); + } $this->status = $values['status'] ?? null; $this->url = $values['url']; }Totally optional, but it would make the documented constraint executable.
src/AiContent/Requests/UpdateContentImportSourceRequest.php (1)
12-50: Clarify whethersourceIdshould participate in JSON serialization.Unlike the other fields,
sourceIdhas noJsonPropertyattribute. IfJsonSerializableTypeonly serializes annotated properties and the API expectssourceIdin the request body (rather than solely in the URL path), it may never be sent. For consistency with other DTOs here, consider explicitly annotating it (with the appropriate wire name per the API spec) or confirming it is intentionally path-only.src/Conversations/Requests/ConvertConversationToTicketRequest.php (1)
11-41: Confirm howconversationIdis transported (path vs JSON body).
conversationIdlacks aJsonPropertyattribute while the other fields have one. If this ID is meant only for URL construction in the client, that’s fine; but if the API expects it in the JSON payload, it may not be serialized depending on howJsonSerializableTypeworks. For consistency and safety, consider annotating it explicitly if it should be part of the request body.src/Admins/AdminsClient.php (1)
83-89: Avoid depending on RawClient internals for base URL if possibleEach method reads
$this->client->options['baseUrl']directly when resolvingbaseUrl. This works as long asoptionsstays a public/accessible detail onRawClient, but it tightly couples this client to that internal shape. Consider adding an accessor (e.g.,getBaseUrl()orgetOptions()) onRawClientand using that here to keep the boundary cleaner and reduce the risk of breakage ifRawClientchanges.src/Articles/ArticlesClient.php (1)
117-155: Consider extracting shared HTTP/exception handling into a helper
create,find,update,delete,search, and_listall duplicate the samesendRequest+status check+exception mapping logic. Centralizing this in a small private helper (e.g.,requestAndDecode(string $path, HttpMethod $method, ?object $body, ?array $query, string $type)) would reduce repetition and keep future behavior changes (like broadening the success status range or logging) in one place.src/Contacts/ContactsClient.php (3)
602-602: Questionable default parameter for merge operation.The
mergeLeadInUsermethod has a default emptyMergeContactsRequest(). A merge operation logically requires specifying which contacts to merge (lead and user IDs). Calling this method without arguments would likely result in an API error.Consider removing the default value to make the request parameter required:
- public function mergeLeadInUser(MergeContactsRequest $request = new MergeContactsRequest(), ?array $options = null): ContactsMergeLeadInUserResponse + public function mergeLeadInUser(MergeContactsRequest $request, ?array $options = null): ContactsMergeLeadInUserResponse
1102-1107: Prefer strict equality for null checks.Using
!=instead of!==for null comparisons can lead to unexpected behavior with falsy values like0or empty strings. Strict comparison is the safer choice.- if ($request->getPage() != null) { + if ($request->getPage() !== null) { $query['page'] = $request->getPage(); } - if ($request->getPerPage() != null) { + if ($request->getPerPage() !== null) { $query['per_page'] = $request->getPerPage(); }
1323-1331: Same loose equality issue in_listmethod.Apply strict null checks here as well for consistency and correctness.
- if ($request->getPage() != null) { + if ($request->getPage() !== null) { $query['page'] = $request->getPage(); } - if ($request->getPerPage() != null) { + if ($request->getPerPage() !== null) { $query['per_page'] = $request->getPerPage(); } - if ($request->getStartingAfter() != null) { + if ($request->getStartingAfter() !== null) { $query['starting_after'] = $request->getStartingAfter(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (73)
.fern/metadata.json(1 hunks).github/workflows/ci.yml(1 hunks)README.md(1 hunks)src/Admins/AdminsClient.php(1 hunks)src/Admins/Requests/ConfigureAwayAdminRequest.php(1 hunks)src/Admins/Requests/FindAdminRequest.php(1 hunks)src/Admins/Types/Admin.php(1 hunks)src/AiAgent/Types/AiAgent.php(1 hunks)src/AiAgent/Types/AiAgentLastAnswerType.php(1 hunks)src/AiAgent/Types/AiAgentResolutionState.php(1 hunks)src/AiContent/AiContentClient.php(1 hunks)src/AiContent/Requests/CreateContentImportSourceRequest.php(1 hunks)src/AiContent/Requests/CreateExternalPageRequest.php(1 hunks)src/AiContent/Requests/DeleteContentImportSourceRequest.php(1 hunks)src/AiContent/Requests/DeleteExternalPageRequest.php(1 hunks)src/AiContent/Requests/GetContentImportSourceRequest.php(1 hunks)src/AiContent/Requests/GetExternalPageRequest.php(1 hunks)src/AiContent/Requests/UpdateContentImportSourceRequest.php(1 hunks)src/AiContent/Requests/UpdateExternalPageRequest.php(1 hunks)src/AiContent/Types/ContentImportSource.php(1 hunks)src/AiContent/Types/ContentImportSourceStatus.php(1 hunks)src/AiContent/Types/ContentImportSourceSyncBehavior.php(1 hunks)src/AiContent/Types/ContentImportSourcesList.php(1 hunks)src/AiContent/Types/CreateContentImportSourceRequestStatus.php(1 hunks)src/AiContent/Types/ExternalPage.php(1 hunks)src/AiContent/Types/ExternalPagesList.php(1 hunks)src/AiContent/Types/UpdateContentImportSourceRequestStatus.php(1 hunks)src/AiContent/Types/UpdateContentImportSourceRequestSyncBehavior.php(1 hunks)src/AiContentSource/Types/ContentSource.php(1 hunks)src/Articles/ArticlesClient.php(1 hunks)src/Articles/Requests/DeleteArticleRequest.php(1 hunks)src/Articles/Requests/FindArticleRequest.php(1 hunks)src/Articles/Requests/UpdateArticleRequest.php(1 hunks)src/Articles/Types/ArticleSearchHighlights.php(1 hunks)src/Articles/Types/ArticleSearchResponse.php(1 hunks)src/Articles/Types/ArticleSearchResponseData.php(1 hunks)src/Articles/Types/UpdateArticleRequestState.php(1 hunks)src/AwayStatusReasons/AwayStatusReasonsClient.php(1 hunks)src/Calls/CallsClient.php(1 hunks)src/Calls/Requests/ListCallsRequest.php(1 hunks)src/Calls/Requests/ListCallsWithTranscriptsRequest.php(1 hunks)src/Calls/Requests/ShowCallRecordingRequest.php(1 hunks)src/Calls/Requests/ShowCallRequest.php(1 hunks)src/Calls/Requests/ShowCallTranscriptRequest.php(1 hunks)src/Calls/Traits/Call.php(1 hunks)src/Calls/Types/Call.php(1 hunks)src/Calls/Types/ListCallsWithTranscriptsResponse.php(1 hunks)src/Calls/Types/ListCallsWithTranscriptsResponseDataItem.php(1 hunks)src/Companies/CompaniesClient.php(1 hunks)src/Companies/Requests/ListAttachedContactsRequest.php(1 hunks)src/Companies/Requests/UpdateCompanyRequest.php(1 hunks)src/Companies/Types/Company.php(1 hunks)src/Companies/Types/CompanyPlan.php(1 hunks)src/Contacts/ContactsClient.php(1 hunks)src/Contacts/Requests/ArchiveContactRequest.php(1 hunks)src/Contacts/Requests/BlockContactRequest.php(1 hunks)src/Contacts/Requests/DeleteContactRequest.php(1 hunks)src/Contacts/Requests/FindContactRequest.php(1 hunks)src/Contacts/Requests/MergeContactsRequest.php(1 hunks)src/Contacts/Requests/ShowContactByExternalIdRequest.php(1 hunks)src/Contacts/Requests/UnarchiveContactRequest.php(1 hunks)src/Contacts/Traits/Contact.php(1 hunks)src/Contacts/Types/Contact.php(1 hunks)src/Contacts/Types/ContactsCreateResponse.php(1 hunks)src/Contacts/Types/ContactsFindResponse.php(1 hunks)src/Contacts/Types/ContactsMergeLeadInUserResponse.php(1 hunks)src/Contacts/Types/ContactsUpdateResponse.php(1 hunks)src/Contacts/Types/ShowContactByExternalIdResponse.php(1 hunks)src/Conversations/ConversationsClient.php(1 hunks)src/Conversations/Requests/ConvertConversationToTicketRequest.php(1 hunks)src/Conversations/Requests/DeleteConversationRequest.php(1 hunks)src/Conversations/Requests/FindConversationRequest.php(1 hunks)src/Conversations/Requests/UpdateConversationRequest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- src/Contacts/Requests/FindContactRequest.php
- src/Companies/Requests/ListAttachedContactsRequest.php
- src/Contacts/Requests/ArchiveContactRequest.php
- src/Companies/Types/CompanyPlan.php
- src/Articles/Requests/FindArticleRequest.php
- src/Companies/CompaniesClient.php
- src/AiContentSource/Types/ContentSource.php
- src/Companies/Types/Company.php
- src/Contacts/Requests/UnarchiveContactRequest.php
- src/Articles/Requests/UpdateArticleRequest.php
- src/Contacts/Requests/MergeContactsRequest.php
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~105-~105: Consider using a synonym to be more concise.
Context: ... ## Legacy SDK While the new SDK has a lot of improvements, we at Intercom understand...
(A_LOT_OF)
[grammar] ~182-~182: Use a hyphen to join words.
Context: ... ### Timeouts The SDK defaults to a 30 second timeout. Use the timeout option...
(QB_NEW_EN_HYPHEN)
[style] ~197-~197: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...ritten upon the next generated release. Feel free to open a PR as a proof of concept, but kn...
(FEEL_FREE_TO_STYLE_ME)
🔇 Additional comments (45)
src/AiAgent/Types/AiAgentResolutionState.php (1)
5-11: LGTM!Well-structured string-backed enum following PHP 8.1+ conventions. The case names appropriately use PascalCase while values use snake_case for API serialization.
src/AiAgent/Types/AiAgentLastAnswerType.php (1)
5-9: LGTM!Clean string-backed enum consistent with the other AiAgent type enums in this namespace.
src/AiAgent/Types/AiAgent.php (3)
12-67: LGTM!The class is well-structured with proper JsonProperty annotations for snake_case API mapping. The use of
value-of<EnumType>PHPDoc syntax correctly documents the string representation while maintaining type flexibility for forward compatibility with new API values.
81-93: LGTM!Clean array-based constructor with proper null-coalescing defaults for all optional fields.
248-254: LGTM!Clean
__toStringimplementation delegating to the parent'stoJson()method for consistent serialization.src/Companies/Requests/UpdateCompanyRequest.php (1)
50-65: LGTM!The getter and setter for the optional
bodyproperty are well-implemented with proper nullable type hints and fluent interface support.src/Contacts/Requests/BlockContactRequest.php (2)
1-12: LGTM!The class structure, namespace, and property declaration follow PHP best practices and the established pattern in the codebase.
25-31: LGTM!The getter implementation is correct and follows PHP conventions.
.fern/metadata.json (2)
1-20: JSON structure and configuration look correct.The metadata file is well-formed and the generator configuration appears aligned with the PR's objectives. Key points:
- Namespace and client naming conventions are reasonable.
propertyAccess: "private"andinlinePathParameters: trueare sensible defaults for a modern PHP SDK.- ComposerJSON metadata correctly references the Intercom Platform Team and documentation.
4-4: The Fern PHP SDK generator version in metadata.json cannot be verified against public release history.Fern uses internal versioning for generators that is not publicly tracked. Without access to Fern's internal release registry or changelog, the claim that
generatorVersion: "1.25.0"represents an up-to-date version cannot be verified. If this file is auto-generated by the Fern CLI during setup, this concern may be moot; if manually maintained, consider checking Fern's official documentation or registry for the current recommended version.src/Calls/Requests/ListCallsRequest.php (1)
1-65: LGTM!Clean DTO implementation with proper typing, fluent setters, and well-documented properties. The pagination parameters follow the established pattern in the codebase.
Note: The
perPagedocblock mentions "Max 25" but no client-side validation is enforced. This is acceptable if the API handles validation server-side.src/Articles/Types/ArticleSearchHighlights.php (1)
1-105: LGTM!Well-structured DTO with proper JSON serialization attributes. The nested array types for
highlightedTitleandhighlightedSummaryare correctly annotated withArrayType, and the PHPDoc accurately reflects the expected structure.src/Calls/Types/Call.php (1)
1-566: LGTM!Comprehensive DTO for call data with proper JSON serialization. The use of union types (
DateTime|int|null) for timestamps provides flexibility for varying API response formats. Property mappings correctly handle the camelCase to snake_case conversion.src/AiContent/Types/CreateContentImportSourceRequestStatus.php (1)
5-9: Enum definition looks clean and consistentString-backed enum with constrained values for create-status is appropriate and matches the other status enums in this module. No changes needed.
src/AiContent/Types/UpdateContentImportSourceRequestStatus.php (1)
5-9: Consistent status enum for update requestsThe enum mirrors the other status types (
Active/Deactivated) and keeps update payloads type-safe. Looks good as-is.src/AiContent/Types/ContentImportSourceStatus.php (1)
5-9: Consistent status enum for content import sourcesMatches the other status enums (
Active/Deactivated) and keeps model state constrained. No further changes needed.src/Articles/Types/UpdateArticleRequestState.php (1)
5-9: Article update state enum looks good
Published/Draftstring-backed enum is a clear, type-safe representation for update state; consistent with the rest of the Articles types.src/AiContent/Types/ContentImportSourceSyncBehavior.php (1)
5-10: File does not exist in current repository stateThe file
src/AiContent/Types/ContentImportSourceSyncBehavior.phpreferenced in this review does not exist in the repository. If this file is being added in the current change, verify against Intercom API documentation: the official docs indicate the sync_behavior field accepts"api","automated", and"manual". The enum case should useAutomated = "automated"rather thanAutomatic = "automatic"to match the primary API value. Additionally, confirm whetherUpdateContentImportSourceRequestSyncBehaviorexists or needs to be created with aligned naming.src/AiContent/Requests/DeleteExternalPageRequest.php (1)
7-40: DeleteExternalPageRequest DTO looks consistent and correctConstructor, typed property, getter, and fluent setter are straightforward and match patterns used elsewhere in the SDK. No issues from a correctness standpoint.
src/AiContent/Requests/GetExternalPageRequest.php (1)
7-40: GetExternalPageRequest mirrors delete request and is fineThis DTO is structurally identical to DeleteExternalPageRequest (single required pageId with getter/setter), which keeps the API surface consistent. No changes requested.
src/AiContent/Requests/GetContentImportSourceRequest.php (1)
7-40: GetContentImportSourceRequest is straightforward and aligned with othersSingle required sourceId field plus getter/setter follows the same pattern as other AiContent request types. Implementation is clear and correct.
src/AiContent/Requests/DeleteContentImportSourceRequest.php (1)
7-40: DeleteContentImportSourceRequest matches the established request patternThe delete DTO cleanly mirrors the corresponding “get” request, using a typed sourceId with simple accessor methods. No issues spotted.
src/Calls/Requests/ShowCallTranscriptRequest.php (1)
7-40: ShowCallTranscriptRequest is well-formedSimple ID wrapper (callId) with a typed property and fluent API; matches other request DTOs and should work cleanly with the JSON/HTTP layer.
src/Conversations/Requests/DeleteConversationRequest.php (1)
7-40: DeleteConversationRequest DTO is consistent and type-safeTyped
conversationIdfield with simple ctor/getter/setter matches the rest of the Conversations request surface and should integrate cleanly with the client.src/AwayStatusReasons/AwayStatusReasonsClient.php (1)
69-106: LGTM!The error handling logic is well-structured:
- Successfully handles 2xx responses with JSON deserialization
- Properly wraps JSON parsing errors in IntercomException
- Correctly handles HTTP request exceptions with appropriate status codes
- Falls through to a final exception for non-2xx responses
The baseUrl resolution chain (per-call options → instance options → default environment) provides good flexibility.
src/Articles/Types/ArticleSearchResponseData.php (1)
14-79: ArticleSearchResponseData DTO looks consistentThe articles/highlights arrays, constructor, and accessors all line up with the JsonProperty/ArrayType annotations and expected usage as a search payload wrapper. No issues spotted.
src/Conversations/Requests/FindConversationRequest.php (1)
9-88: FindConversationRequest structure is straightforward and coherentRequired
conversationIdplus optionaldisplayAs/includeTranslationsare clearly modeled, and the constructor/getter/setter set is consistent. Nothing problematic here.src/Contacts/Types/ContactsMergeLeadInUserResponse.php (1)
18-148: ContactsMergeLeadInUserResponse wiring looks correctThe
enabledPushMessagingflag is correctly documented, mapped viaJsonProperty('enabled_push_messaging'), initialized in the constructor, and exposed through simple accessors. This fits cleanly with the existingContacttrait fields.src/Admins/Requests/ConfigureAwayAdminRequest.php (1)
8-116: ConfigureAwayAdminRequest DTO looks soundThe required/optional fields for configuring away mode are clearly modeled, and the JsonProperty mappings for
away_mode_enabled,away_mode_reassign, andaway_status_reason_idlook correct. No changes needed from this file alone.src/AiContent/Types/ExternalPagesList.php (1)
15-130: ExternalPagesList list wrapper looks correctThe
type,pages,totalCount, anddatafields are all correctly annotated and initialized, and the list structure matches other list DTOs in this PR. Nothing to change here.src/AiContent/Types/ContentImportSourcesList.php (1)
10-131: DTO wiring for content import sources list looks consistent.JsonProperty / ArrayType mappings, constructor, and accessors all line up with the documented API shape; nothing problematic stands out.
src/Articles/Types/ArticleSearchResponse.php (1)
9-130: Article search response DTO is aligned with existing pagination patterns.JSON mappings, constructor, and getters/setters are coherent and match the expected shape for a list-style search result.
src/Calls/Types/ListCallsWithTranscriptsResponse.php (1)
9-77: ListCallsWithTranscriptsResponse wiring looks correct.The JsonProperty/ArrayType configuration and the simple constructor/accessors are consistent with the rest of the calls models.
src/Calls/CallsClient.php (1)
24-336: CallsClient implementation is consistent with the new core client pattern.Option merging, request construction, success/error handling, and JSON deserialization all mirror the other generated clients; behavior for list/show/recording/transcript/search endpoints looks coherent with the documented types.
src/AiContent/Types/ExternalPage.php (1)
8-379: ExternalPage model cleanly matches the described AI content schema.JSON attributes, constructor wiring, and the full set of getters/setters are consistent and straightforward; nothing problematic stands out.
src/AiContent/Requests/CreateExternalPageRequest.php (1)
8-218: CreateExternalPageRequest DTO aligns well with ExternalPage and API docs.Required vs optional fields, JsonProperty mappings, and fluent setters are all coherent and match the expected payload structure.
src/Conversations/Requests/UpdateConversationRequest.php (1)
12-77: DTO structure and JSON mapping look soundConstructor, nullable fields, and
JsonProperty/ArrayTypeusage are consistent with the rest of the SDK; nothing blocking here from a typing or wiring perspective.src/AiContent/Requests/UpdateExternalPageRequest.php (1)
8-80: External page update request is well‑modeledRequired fields, optional booleans/IDs, and
JsonPropertymappings all line up with the described API; usingpageIdas a non‑serialized path parameter is appropriate.src/AiContent/AiContentClient.php (1)
79-332: AiContentClient methods are consistent and correctly wiredThe content import source and external page methods share consistent option merging, base URL resolution, and exception handling, and the return types (void for source delete, entity for others) match their documented behavior. No functional issues stand out.
src/Calls/Traits/Call.php (1)
47-529: Call trait data model looks correct and internally consistentJSON property names, union date handling, and the fluent getters/setters all align; this should integrate cleanly into the call types that
usethe trait.src/Contacts/ContactsClient.php (2)
1-92: LGTM - Class structure and imports are well organized.The class follows a clean dependency injection pattern with
RawClientand configurable options. PHPDoc annotations provide good type hints for the options array.
142-179: Consistent error handling pattern.The try-catch structure properly handles JSON deserialization errors, Guzzle request exceptions, and generic PSR client exceptions. The pattern correctly propagates error details through custom exception types.
src/Contacts/Types/Contact.php (2)
17-386: Well-structured data model with comprehensive field coverage.The Contact class properly maps all Intercom API fields with appropriate JSON property mappings, nullable types, and a flexible constructor pattern. The use of nested types (ContactTags, ContactNotes, etc.) maintains clean separation of concerns.
1153-1159: LGTM - String representation delegates to JSON serialization.The
__toString()implementation properly delegates to the parent'stoJson()method, providing consistent JSON output for debugging and logging.src/Contacts/Traits/Contact.php (1)
62-1097: The files referenced in this review do not exist in the current codebase.This review comment references
src/Contacts/Traits/Contact.phpandsrc/Contacts/Types/Contact.php, but neither file exists in the repository. The actual codebase structure does not contain asrc/Contacts/directory.This review appears to be for a different pull request, branch, or codebase state. It cannot be addressed as written because the files being reviewed are not present.
Likely an incorrect or invalid review comment.
| Exceptions are handled by [Guzzle](https://github.com/guzzle/guzzle). | ||
| The Intercom API may return an unsuccessful HTTP response, for example when a resource is not found (404). | ||
| If you want to catch errors you can wrap your API call into a try/catch: | ||
| The SDK defaults to a 30 second timeout. Use the `timeout` option to configure this behavior. |
There was a problem hiding this comment.
Minor: Use hyphenated compound adjective.
Per static analysis, "30 second" should be hyphenated when used as a compound modifier.
-The SDK defaults to a 30 second timeout. Use the `timeout` option to configure this behavior.
+The SDK defaults to a 30-second timeout. Use the `timeout` option to configure this behavior.🧰 Tools
🪛 LanguageTool
[grammar] ~182-~182: Use a hyphen to join words.
Context: ... ### Timeouts The SDK defaults to a 30 second timeout. Use the timeout option...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In README.md around line 182, the phrase "30 second timeout" is using an
unhyphenated compound adjective; change it to "30-second timeout" so the
compound modifier is hyphenated when placed before the noun.
| /** | ||
| * @var 'content_import_source' $type Always external_page | ||
| */ | ||
| #[JsonProperty('type')] | ||
| private string $type; | ||
|
|
There was a problem hiding this comment.
Fix copy/paste typo in ContentImportSource $type docblock
The $type docblock says:
@var 'content_import_source' $type Always external_page
Given the class name and value type, this should describe content_import_source, not external_page. Consider updating to something like:
- /**
- * @var 'content_import_source' $type Always external_page
- */
+ /**
+ * @var 'content_import_source' $type Always content_import_source.
+ */to avoid confusion with the ExternalPage types.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @var 'content_import_source' $type Always external_page | |
| */ | |
| #[JsonProperty('type')] | |
| private string $type; | |
| /** | |
| * @var 'content_import_source' $type Always content_import_source. | |
| */ | |
| #[JsonProperty('type')] | |
| private string $type; |
🤖 Prompt for AI Agents
In src/AiContent/Types/ContentImportSource.php around lines 13 to 18, the
docblock for the $type property contains a copy/paste typo referencing
"external_page"; update the @var description to correctly indicate the property
represents 'content_import_source' (e.g., "@var 'content_import_source' $type
Always content_import_source") so the docblock matches the class and value
semantics.
| enum UpdateContentImportSourceRequestSyncBehavior: string | ||
| { | ||
| case Api = "api"; | ||
| case Automated = "automated"; | ||
| case Manual = "manual"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.php" | xargs grep -l "ContentImportSourceSyncBehavior"Repository: asknicely/intercom-php
Length of output: 50
🏁 Script executed:
rg "ContentImportSourceSyncBehavior" -A 5 -B 2Repository: asknicely/intercom-php
Length of output: 48
🌐 Web query:
Intercom AI Content API sync behavior enum values api automated manual
💡 Result:
Enum values for the AI Content API field sync_behavior:
- "api" — external pages are created/updated via the API (used when managing content programmatically). You cannot change the value to or from "api" via update. [1]
- "automatic" (aka sometimes shown as "automated" in some examples/clients) — Intercom will automatically sync the source on its schedule. [1][2]
- "manual" — the source is only synced when a manual sync is triggered. [1]
Sources: Intercom Developers API reference for Content Import Sources (sync_behavior enum: "api", "automatic", "manual"). [1] Example/client docs showing the alternate "automated" spelling in request/response examples. [2]
Change Automated case to Automatic to match API specification
The enum case Automated = "automated" does not match the Intercom AI Content API specification, which defines the sync behavior value as "automatic" (not "automated"). While some client examples may show "automated", the official API documentation for the sync_behavior field uses "automatic". Update the enum case name and value to Automatic = "automatic" to ensure correct serialization/deserialization with the API.
🤖 Prompt for AI Agents
In src/AiContent/Types/UpdateContentImportSourceRequestSyncBehavior.php around
lines 5 to 10, the enum defines case Automated = "automated" but the API expects
"automatic"; change the enum case name and value to Automatic = "automatic" so
serialization/deserialization matches the Intercom AI Content API specification.
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->conversationIds = $values['conversationIds']; | ||
| } |
There was a problem hiding this comment.
Add validation for required constructor parameter and enforce maximum limit.
Two issues:
- The constructor directly accesses
$values['conversationIds']without checking if the key exists, which will trigger an undefined array key warning or error in PHP 8+. - The comment on Line 12 states "Maximum 20" conversation IDs, but this limit is not enforced.
Apply this diff to add validation:
public function __construct(
array $values,
) {
+ if (!isset($values['conversationIds'])) {
+ throw new \InvalidArgumentException('Missing required field: conversationIds');
+ }
+ if (!is_array($values['conversationIds'])) {
+ throw new \InvalidArgumentException('Field conversationIds must be an array');
+ }
+ if (count($values['conversationIds']) > 20) {
+ throw new \InvalidArgumentException('Field conversationIds cannot contain more than 20 items');
+ }
$this->conversationIds = $values['conversationIds'];
}🤖 Prompt for AI Agents
In src/Calls/Requests/ListCallsWithTranscriptsRequest.php around lines 22 to 26,
the constructor currently reads $values['conversationIds'] directly; update it
to first verify the key exists and is an array, throwing an
InvalidArgumentException (or similar) if missing or not an array, then enforce
the documented maximum of 20 conversation IDs by checking
count($values['conversationIds']) <= 20 and throwing if exceeded; also
optionally validate elements are non-empty strings/ints as needed before
assigning to $this->conversationIds.
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->callId = $values['callId']; | ||
| } |
There was a problem hiding this comment.
Add validation for callId in the constructor
The constructor assumes $values['callId'] exists and is a string. If the key is missing or not a string, you’ll get a notice and potentially a TypeError when assigning to the typed property.
Defensive validation here would make failures clearer and avoid noisy PHP notices.
- public function __construct(
- array $values,
- ) {
- $this->callId = $values['callId'];
- }
+ public function __construct(
+ array $values,
+ ) {
+ if (
+ !\array_key_exists('callId', $values)
+ || !\is_string($values['callId'])
+ || \trim($values['callId']) === ''
+ ) {
+ throw new \InvalidArgumentException('callId is required and must be a non-empty string.');
+ }
+
+ $this->callId = $values['callId'];
+ }🤖 Prompt for AI Agents
In src/Calls/Requests/ShowCallRecordingRequest.php around lines 19 to 23, the
constructor blindly reads $values['callId'] which may be missing or not a
string; add defensive validation: check that the key exists (isset or
array_key_exists) and that its value is a string, and if not throw a clear
InvalidArgumentException (or TypeError) with a descriptive message; only assign
$this->callId after the checks pass.
| /** | ||
| * @var ?array<array<string, mixed>> $transcript The call transcript if available, otherwise an empty array. | ||
| */ | ||
| #[JsonProperty('transcript'), ArrayType([['string' => 'mixed']])] | ||
| private ?array $transcript; | ||
|
|
There was a problem hiding this comment.
Clarify transcript docblock to match nullable behavior
The comment says “The call transcript if available, otherwise an empty array.” but the property is nullable and is initialized from $values['transcript'] ?? null, so null is a valid and likely default value.
To avoid confusion for consumers and static analysis, consider aligning the wording with the actual behavior, e.g.:
- /**
- * @var ?array<array<string, mixed>> $transcript The call transcript if available, otherwise an empty array.
- */
+ /**
+ * @var ?array<array<string, mixed>> $transcript The call transcript if available, or null when not present.
+ */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @var ?array<array<string, mixed>> $transcript The call transcript if available, otherwise an empty array. | |
| */ | |
| #[JsonProperty('transcript'), ArrayType([['string' => 'mixed']])] | |
| private ?array $transcript; | |
| /** | |
| * @var ?array<array<string, mixed>> $transcript The call transcript if available, or null when not present. | |
| */ | |
| #[JsonProperty('transcript'), ArrayType([['string' => 'mixed']])] | |
| private ?array $transcript; |
🤖 Prompt for AI Agents
In src/Calls/Types/ListCallsWithTranscriptsResponseDataItem.php around lines 15
to 20, the PHPDoc wrongly says "otherwise an empty array" while the property is
nullable; update the docblock to reflect nullable behavior (e.g., "An array of
associative arrays representing the call transcript, or null if not available")
so it matches the ?array type and the initialization from $values['transcript']
?? null.
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->contactId = $values['contactId']; | ||
| } |
There was a problem hiding this comment.
Add validation for required array key.
The constructor directly accesses $values['contactId'] without checking if the key exists. This will cause an undefined array key error if the key is missing or misspelled.
Apply this diff to add validation:
public function __construct(
array $values,
) {
+ if (!isset($values['contactId'])) {
+ throw new \InvalidArgumentException('contactId is required');
+ }
$this->contactId = $values['contactId'];
}Alternatively, if the parent class or calling code guarantees the key exists, document this assumption in the PHPDoc.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function __construct( | |
| array $values, | |
| ) { | |
| $this->contactId = $values['contactId']; | |
| } | |
| public function __construct( | |
| array $values, | |
| ) { | |
| if (!isset($values['contactId'])) { | |
| throw new \InvalidArgumentException('contactId is required'); | |
| } | |
| $this->contactId = $values['contactId']; | |
| } |
🤖 Prompt for AI Agents
In src/Contacts/Requests/BlockContactRequest.php around lines 19 to 23, the
constructor directly reads $values['contactId'] which can trigger an undefined
array key; update the constructor to validate the presence (and optionally type)
of 'contactId' before assignment — e.g., check array_key_exists('contactId',
$values) (or isset) and if missing throw a clear InvalidArgumentException with a
descriptive message; then assign $this->contactId = $values['contactId'];
alternatively, if the key is guaranteed by callers, add a PHPDoc comment on the
constructor stating that 'contactId' is required and its expected type.
| /** | ||
| * @param string $value | ||
| */ | ||
| public function setContactId(string $value): self | ||
| { | ||
| $this->contactId = $value; | ||
| return $this; | ||
| } |
There was a problem hiding this comment.
Add missing return type to PHPDoc.
The setter returns self for fluent interface, but the PHPDoc is missing the @return annotation.
Apply this diff to complete the documentation:
/**
* @param string $value
+ * @return self
*/
public function setContactId(string $value): self📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @param string $value | |
| */ | |
| public function setContactId(string $value): self | |
| { | |
| $this->contactId = $value; | |
| return $this; | |
| } | |
| /** | |
| * @param string $value | |
| * @return self | |
| */ | |
| public function setContactId(string $value): self | |
| { | |
| $this->contactId = $value; | |
| return $this; | |
| } |
🤖 Prompt for AI Agents
In src/Contacts/Requests/BlockContactRequest.php around lines 33 to 40, the
PHPDoc for setContactId documents the parameter but omits the @return
annotation; update the docblock to include "@return self" (or "@return $this")
so it correctly reflects the fluent interface return type for the setter.
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->contactId = $values['contactId']; | ||
| } |
There was a problem hiding this comment.
Add validation for required constructor parameter.
The constructor directly accesses $values['contactId'] without checking if the key exists. In PHP 8+, this will trigger an undefined array key warning or error if the key is missing.
Apply this diff to add validation:
public function __construct(
array $values,
) {
+ if (!isset($values['contactId'])) {
+ throw new \InvalidArgumentException('Missing required field: contactId');
+ }
$this->contactId = $values['contactId'];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function __construct( | |
| array $values, | |
| ) { | |
| $this->contactId = $values['contactId']; | |
| } | |
| public function __construct( | |
| array $values, | |
| ) { | |
| if (!isset($values['contactId'])) { | |
| throw new \InvalidArgumentException('Missing required field: contactId'); | |
| } | |
| $this->contactId = $values['contactId']; | |
| } |
🤖 Prompt for AI Agents
In src/Contacts/Requests/DeleteContactRequest.php around lines 19 to 23, the
constructor accesses $values['contactId'] directly which can trigger an
undefined array key error; update the constructor to validate that the
'contactId' key exists and is non-empty (and optionally of the expected type,
e.g. string or int), and if not present/invalid throw an
InvalidArgumentException with a clear message; after validation assign the value
to $this->contactId. Ensure the validation covers both missing key and
empty/null values.
| public function __construct( | ||
| array $values, | ||
| ) { | ||
| $this->externalId = $values['externalId']; | ||
| } |
There was a problem hiding this comment.
Add validation for required constructor parameter.
The constructor directly accesses $values['externalId'] without checking if the key exists. In PHP 8+, this will trigger an undefined array key warning or error if the key is missing.
Apply this diff to add validation:
public function __construct(
array $values,
) {
+ if (!isset($values['externalId'])) {
+ throw new \InvalidArgumentException('Missing required field: externalId');
+ }
$this->externalId = $values['externalId'];
}🤖 Prompt for AI Agents
In src/Contacts/Requests/ShowContactByExternalIdRequest.php around lines 19 to
23, the constructor accesses $values['externalId'] directly which can cause an
undefined array key error; update the constructor to validate the presence of
the externalId key (use array_key_exists or isset) and that it is non-empty (and
optionally the expected type), and if missing/invalid throw a clear
InvalidArgumentException (or a domain-specific exception) with a helpful message
instead of accessing the array element directly.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.