feat(tasks): add support for task batches (batchId) - #263
Conversation
There was a problem hiding this comment.
🟡 Not ready to approve
The new batch feature isn’t fully wired through the consolidated vendor task helper methods (and corresponding tests), leaving the PR incomplete relative to #255.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Updates the Tasks API resource to reflect the upstream Crowdin/Crowdin Enterprise changes described in #255, including task-batch support (batchId) and the consolidated vendor task creation form.
Changes:
- Adds
batchIdsupport to task listing and several task creation helpers (general task forms) for both Crowdin and Enterprise resources. - Consolidates Crowdin vendor task helpers by removing per-vendor create-form variants and introducing generic vendor-task helpers that take a free-form
vendoridentifier. - Extends task patch enums/tests to cover new editable fields (
batchId,resetScope) and simplifies/removes obsolete vendor-specific enums and tests.
File summaries
| File | Description |
|---|---|
| crowdin_api/api_resources/tasks/resource.py | Adds batchId to list/create helpers, removes per-vendor helpers, introduces consolidated vendor helpers, and exposes patch support. |
| crowdin_api/api_resources/tasks/enums.py | Adds patch paths for batchId/resetScope and removes enums tied to the deprecated per-vendor create forms. |
| crowdin_api/api_resources/tasks/tests/test_tasks_resources.py | Updates tests for the new params/enums and new consolidated vendor helper methods. |
Review details
Suppressed comments (5)
crowdin_api/api_resources/tasks/resource.py:367
add_vendor_task()still doesn't forwardbatchIdinto the request body; even after adding the parameter, it will be ignored unless it's included inrequest_data.
"deadline": deadline,
"dateFrom": dateFrom,
"dateTo": dateTo,
},
crowdin_api/api_resources/tasks/resource.py:410
add_vendor_by_string_ids_task()should includebatchIdin the request body when provided; otherwise the new parameter (and API feature) has no effect.
"deadline": deadline,
"dateFrom": dateFrom,
"dateTo": dateTo,
},
crowdin_api/api_resources/tasks/resource.py:463
add_vendor_pending_task()currently ignoresbatchIdeven if added to the signature; it needs to be forwarded into the request body (ideally only when non-null).
"vendor": vendor,
"title": title,
"description": description,
crowdin_api/api_resources/tasks/tests/test_tasks_resources.py:439
- Batch support is being added, but the vendor-task tests don’t cover passing a non-null
batchIdthrough the helper into the request body. AddingbatchIdto one of the parametrized cases would lock in the behavior.
This issue also appears on line 600 of the same file.
"deadline": datetime(year=1988, month=9, day=26),
"dateFrom": datetime(year=1988, month=1, day=4),
"dateTo": datetime(year=2015, month=10, day=13),
},
{
crowdin_api/api_resources/tasks/tests/test_tasks_resources.py:607
add_vendor_pending_taskshould also have at least one test case that passes a non-nullbatchIdto ensure the field is actually forwarded to the API request.
{
"title": "title",
"precedingTaskId": 1,
"description": "description",
"vendor": "acclaro",
"assignees": [{"id": 1, "wordsCount": 2}],
"deadline": datetime(year=1988, month=9, day=26),
},
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| deadline: Optional[datetime] = None, | ||
| startedAt: Optional[datetime] = None, | ||
| dateFrom: Optional[datetime] = None, | ||
| dateTo: Optional[datetime] = None, | ||
| ): |
There was a problem hiding this comment.
Checked against Crowdin's official API docs (api.projects.tasks.post request body schemas for both crowdin and enterprise): batchId is only present on the general Task Create Form (source/string ids), not on the Vendor Task Create Form or Vendor Pending Task Create Form (neither Crowdin nor Enterprise). So add_vendor_task() intentionally doesn't accept batchId — adding it would send a field the API doesn't recognize for that form.
While verifying this, found the opposite issue: EnterpriseTasksResource.add_vendor_task() / add_vendor_by_string_ids_task() had batchId incorrectly added (not present in the Enterprise Vendor Task Create Form schema either). Removed it there for consistency with the docs and with the non-enterprise vendor methods.
| deadline: Optional[datetime] = None, | ||
| startedAt: Optional[datetime] = None, | ||
| dateFrom: Optional[datetime] = None, | ||
| dateTo: Optional[datetime] = None, | ||
| ): |
There was a problem hiding this comment.
Same as the reply on the sibling comment: per Crowdin's API docs, batchId isn't part of the Vendor Task Create Form (by string ids) either — only the general Task Create Form forms support it. No change needed here.
| projectId: Optional[int] = None, | ||
| description: Optional[str] = None, | ||
| assignees: Optional[Iterable[CrowdinTaskAssignee]] = None, | ||
| deadline: Optional[datetime] = None, | ||
| ): |
There was a problem hiding this comment.
Checked against the docs: batchId isn't part of the Pending Task Create Form or Vendor Pending Task Create Form (neither Crowdin nor Enterprise) — it's only supported on the general Task Create Form. So add_vendor_pending_task() correctly omits it.
Closes #255