GRAM-4868: fix stale array-param test calls for v2 ids parameter - #313
Merged
Merged
Conversation
getActivities()/getDeals() now type $ids as string[] (array, style form, explode false) following the api-docs schema change from type: string to type: array. Update the two v2 test calls that were still passing a comma-joined string, which throws a TypeError in count($ids) with the regenerated SDK.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are narrowly scoped test updates that correctly align with the regenerated v2 SDK parameter types while preserving the expected query serialization.
Pull request overview
Updates the v2 functional tests to match the regenerated PHP SDK method signatures after the OpenAPI ids query parameter was corrected from string to array (comma-separated on the wire), preventing runtime TypeError failures in request-building code that uses count($ids).
Changes:
- Update
DealsApiTestto passidsas an array (['1', '2']) instead of a comma-joined string. - Update
ActivitiesApiTestto passidsas an array (['1', '2', '3']) instead of a comma-joined string. - Keep query-string assertions unchanged (wire format remains comma-separated via collection serialization).
File summaries
| File | Description |
|---|---|
| test/versions/v2/Deals/DealsApiTest.php | Pass ids as an array to match the v2 SDK signature and avoid count() on string. |
| test/versions/v2/Activities/ActivitiesApiTest.php | Pass ids as an array to match the v2 SDK signature and avoid count() on string. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
siirimangus
approved these changes
Sep 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
api-docscommit 6515fe5 (GRAM-4868) changed the v2ids/include_fields/custom_fieldsquery parameters fromtype: stringtotype: array(style: form, explode: false), which is the correct OpenAPI shape (matches the existingdeal_idspattern) and produces the same comma-separated wire format.$idsparameter type fromstringtostring[].ActivitiesApiTest::it lists activitiesandDealsApiTest::it lists dealswere still passing a comma-joined string ('1,2,3'), which throwsTypeError: count(): Argument #1 ($value) must be of type Countable|array, string givenagainst the regenerated SDK (ActivitiesApi::getActivitiesRequest,DealsApi::getDealsRequestdocount($ids) > 100).Fix
Pass arrays instead of comma-joined strings for
ids, matching the new generated signature. Expected query string assertions are unchanged sinceObjectSerializer::serializeCollection($ids, 'form', true)still joins with a comma on the wire.Test plan
vendor/bin/pestlocally against this branch (PHP 8.3): 19 passed, 12 skipped, 0 failed (previously 2 failed with the reportedTypeError).