feat(ai): add AI Translate Strings endpoint support#240
feat(ai): add AI Translate Strings endpoint support#240Tharun-bot wants to merge 4 commits intocrowdin:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #240 +/- ##
==========================================
- Coverage 99.39% 99.38% -0.01%
==========================================
Files 182 182
Lines 8433 8444 +11
Branches 187 188 +1
==========================================
+ Hits 8381 8391 +10
Misses 36 36
- Partials 16 17 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds SDK support for invoking Crowdin’s “AI Translate Strings” endpoint from the AI resource layer, with a corresponding unit test to validate the outgoing request shape.
Changes:
- Added
ai_translate_strings()to the AI resource implementation. - Added a unit test covering the happy-path request for the new endpoint.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
crowdin_api/api_resources/ai/resource.py |
Introduces the new resource method that builds the request payload and calls the requester. |
crowdin_api/api_resources/ai/tests/test_ai_resources.py |
Adds a test that asserts the HTTP method/path and payload passed to the requester. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return self.requester.request( | ||
| method="post", | ||
| path=f"ai/{aiId}/translate-strings", | ||
| post_data=post_data, |
There was a problem hiding this comment.
APIRequester.request() does not accept a post_data keyword (it only supports request_data), so this will raise a TypeError at runtime when the request is executed. Please rename post_data to request_data here (and keep the dict as-is).
| post_data=post_data, | |
| request_data=post_data, |
| AI Translate Strings. | ||
|
|
||
| Link to API docs: | ||
| https://developer.crowdin.com/api/v2/#tag/AI/operation/api.ai.translations.generate |
There was a problem hiding this comment.
The documentation link in this docstring doesn’t match the endpoint being called (ai/{aiId}/translate-strings) and also uses the non-enterprise base URL. Please update it to the correct "AI Translate Strings" operation URL for this resource (enterprise vs. non-enterprise) so readers aren’t sent to the wrong API docs.
| https://developer.crowdin.com/api/v2/#tag/AI/operation/api.ai.translations.generate | |
| https://support.crowdin.com/developer/enterprise/api/v2/#tag/AI/operation/api.ai.translate-strings.enterprise.generate |
| targetLanguageIds, | ||
| stringIds=None, |
There was a problem hiding this comment.
targetLanguageIds and stringIds are missing type hints here, while other resource methods typically annotate these as Optional[Iterable[str]] / Optional[Iterable[int]]. Adding explicit types (and making stringIds an Optional[...]) will improve IDE/autocomplete support and make the public API clearer.
| targetLanguageIds, | |
| stringIds=None, | |
| targetLanguageIds: Optional[Iterable[str]], | |
| stringIds: Optional[Iterable[int]] = None, |
| def ai_translate_strings( | ||
| self, | ||
| aiId: int, | ||
| projectId: int, | ||
| targetLanguageIds, | ||
| stringIds=None, | ||
| ): | ||
| """ | ||
| AI Translate Strings. | ||
|
|
||
| Link to API docs: | ||
| https://developer.crowdin.com/api/v2/#tag/AI/operation/api.ai.translations.generate | ||
| """ | ||
| post_data = { | ||
| "projectId": projectId, | ||
| "targetLanguageIds": targetLanguageIds, | ||
| } | ||
| if stringIds is not None: | ||
| post_data["stringIds"] = stringIds | ||
|
|
||
| return self.requester.request( | ||
| method="post", | ||
| path=f"ai/{aiId}/translate-strings", | ||
| post_data=post_data, |
There was a problem hiding this comment.
PR description says this adds support for the Crowdin API "AI Translate Strings" endpoint (linked under the non-enterprise docs), but the implementation is currently only on EnterpriseAIResource and uses the enterprise-style path (no users/{userId} prefix). Please confirm whether this endpoint is enterprise-only; if it’s part of the standard API as well, the method (and tests) likely need to be added to AIResource with the correct path/params, or the PR description should be updated to reflect enterprise scope.
| m_request.assert_called_once_with( | ||
| method="post", | ||
| path="ai/1/translate-strings", | ||
| post_data={ | ||
| "projectId": 42, | ||
| "targetLanguageIds": ["uk", "fr"], | ||
| "stringIds": [101, 102], | ||
| }, | ||
| ) |
There was a problem hiding this comment.
This test asserts the request is called with post_data=..., but APIRequester.request() uses request_data for payloads. Once the implementation is fixed to use request_data, the test should be updated accordingly so it matches the real requester API.
| resource = self.get_resource(base_absolut_url) | ||
| assert resource.ai_translate_strings( | ||
| aiId=1, | ||
| projectId=42, | ||
| targetLanguageIds=["uk", "fr"], | ||
| stringIds=[101, 102], | ||
| ) == "response" |
There was a problem hiding this comment.
The implementation conditionally includes stringIds only when provided, but the tests only cover the case where stringIds is set. Consider adding a second assertion covering the stringIds=None/omitted case to lock in the payload behavior for that branch.
|
@Tharun-bot thanks for the contribution! Please address the comments above |
Description
Adds support for the AI Translate Strings endpoint introduced in the Crowdin API.
Changes
ai_translate_strings()method to the AI resourceReferences