Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: restore array parameter support in CatalogItemsV20220401Api #833

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

burakaktna
Copy link

Description

This PR fixes the breaking change introduced in v5.10.3 (PR #700) that affected parameter type handling in searchCatalogItemsRequest. The fix restores support for both string and array inputs while maintaining the validation logic.

Change Details

  • Added getArraySize helper method to handle both array and string inputs consistently
  • Fixed parameter validation to support both input types
  • Maintained existing query parameter handling logic
  • Restored backwards compatibility without breaking changes

Related Issue

Fixes #831

Background

PR #700 introduced stricter type checking that unintentionally broke backwards compatibility by enforcing string-only parameters. This change restores the ability to handle both string and array inputs while maintaining the same validation rules.

Testing

  • Supports both string and comma-separated inputs
  • Maintains array handling in query parameters
  • Preserves validation limits (1 for marketplace_ids, 20 for others)
  • Backwards compatible with existing implementations

The changes resolve the TypeError: explode(): Argument #2 ($string) must be of type string, array given

- Add getArraySize helper method
- Maintain array handling in query parameters
- Fix backwards compatibility from PR jlevers#700

Fixes jlevers#831
@iajrz
Copy link
Collaborator

iajrz commented Dec 17, 2024

Hi! I will look into this; want to take the opportunity to entreat you to move to v7: dependencies on this branch that allow compatibility with the deprecated php7.3 have security implications.

@burakaktna
Copy link
Author

Hi! I will look into this; want to take the opportunity to entreat you to move to v7: dependencies on this branch that allow compatibility with the deprecated php7.3 have security implications.

Thanks for looking into this! You're absolutely right about v7, and we're actually planning that upgrade. However, we're part of a large organization with significant project dependencies and tightly coupled systems using this version's SDK. A rapid migration isn't feasible for us right now, though it's definitely on our roadmap.

That's why I opened this PR - to fix the issue introduced in PR #700 by ensuring both input types work as originally intended. As noted in that PR, breaking backwards compatibility was specifically something to be avoided. This change maintains that goal by supporting both string and array inputs, helping others who might be affected by the same constraint.

Would really appreciate if you could review this contribution - it provides the dual functionality needed while teams plan their v7 migrations properly.

@iajrz iajrz merged commit a56356c into jlevers:v5.0 Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants