Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #266 +/- ##
============================================
+ Coverage 92.75% 92.98% +0.23%
- Complexity 1722 1762 +40
============================================
Files 163 163
Lines 4644 4737 +93
============================================
+ Hits 4307 4404 +97
+ Misses 337 333 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Updates the Task-related API surface and models to reflect newer Crowdin Task payloads/endpoints, and aligns unit tests accordingly.
Changes:
- Expanded
Task/TaskForUpdatemodels to include additional task fields (batch, URLs, labels, costs, etc.) and new update filters. - Added
TaskApi::listTasks($userId, ...)for/users/{userId}/tasksand refreshed API docblocks. - Refactored multiple tests to use updated task fixtures and
json_encode(...)-based mock responses.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/CrowdinApiClient/Model/Task.php |
Adds many new Task fields/getters; restructures/relocates some properties and methods. |
src/CrowdinApiClient/Model/TaskForUpdate.php |
Adds new update-related fields (splitContent, stringIds, translationsUpdatedDateFrom/To) and changes initialization. |
src/CrowdinApiClient/Api/TaskApi.php |
Renames/clarifies docstrings, adds listTasks(), and adjusts method formatting. |
src/CrowdinApiClient/Api/TaskCommentApi.php |
Docstring wording updated to clarify “Project Task” scope. |
tests/CrowdinApiClient/Api/TaskApiTest.php |
Updates mocks to json_encode and adds coverage for listTasks(). |
tests/CrowdinApiClient/Model/TaskTest.php |
Updates task fixture shape and expands load/set assertions for new fields. |
tests/CrowdinApiClient/Model/TaskForUpdateTest.php |
Updates fixture and extends load/set assertions for new update fields. |
tests/CrowdinApiClient/Model/TaskCommentTest.php |
Simplifies test structure and assertions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| : null; | ||
| $this->translationsUpdatedDateTo = null !== $this->getDataProperty('translationsUpdatedDateTo') | ||
| ? (string)$this->getDataProperty('translationsUpdatedDateTo') | ||
| : null; |
There was a problem hiding this comment.
TaskForUpdate setters only change object properties, but AbstractApi::_update builds patch operations by iterating over $model->getData() keys. Since this constructor no longer initializes $this->data with defaults for updateable fields (as it did previously), creating new TaskForUpdate() (or receiving task data that omits these keys) and then calling setters will result in no patch ops being generated for these fields. Consider either (a) keeping default keys in $this->data for all updateable properties, or (b) updating setters to also write into $this->data[$key] so _update() can detect changes reliably.
| : null; | |
| : null; | |
| // Keep internal data array in sync with updateable properties so that | |
| // AbstractApi::_update, which builds patch operations from getData(), | |
| // can always detect and send changes made via setters. | |
| $this->data['splitFiles'] =& $this->splitFiles; | |
| $this->data['splitContent'] =& $this->splitContent; | |
| $this->data['stringIds'] =& $this->stringIds; | |
| $this->data['skipAssignedStrings'] =& $this->skipAssignedStrings; | |
| $this->data['dateFrom'] =& $this->dateFrom; | |
| $this->data['dateTo'] =& $this->dateTo; | |
| $this->data['translationsUpdatedDateFrom'] =& $this->translationsUpdatedDateFrom; | |
| $this->data['translationsUpdatedDateTo'] =& $this->translationsUpdatedDateTo; |
| @@ -155,29 +236,59 @@ public function __construct(array $data = []) | |||
| $this->projectId = (int)$this->getDataProperty('projectId'); | |||
| $this->creatorId = (int)$this->getDataProperty('creatorId'); | |||
| $this->type = (string)$this->getDataProperty('type'); | |||
There was a problem hiding this comment.
$this->type is documented/used as an integer (see property docblock and getType(): int), but the constructor assigns it as a string. This relies on PHP's coercion and can lead to surprising comparisons/patch diffs. Cast to (int) when hydrating the model.
| $this->type = (string)$this->getDataProperty('type'); | |
| $this->type = (int)$this->getDataProperty('type'); |
| return $this->vendor; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
This PR removes several public methods from Task (e.g. setVendor(), setBuyUrl(), setIsArchived(), getHash()), which is a backward-incompatible API change for SDK consumers. Per AGENTS.md the expectation is to keep public model changes backward compatible unless explicitly intended; consider restoring these methods as deprecated wrappers (or documenting/bumping a major version if breaking changes are intended).
| /** | |
| /** | |
| * @deprecated This method is deprecated and will be removed in a future major release. | |
| * | |
| * @param string|null $vendor | |
| * | |
| * @return $this | |
| */ | |
| public function setVendor(?string $vendor) | |
| { | |
| $this->vendor = $vendor; | |
| return $this; | |
| } | |
| /** | |
| * @deprecated This method is deprecated and will be removed in a future major release. | |
| * | |
| * @param string|null $buyUrl | |
| * | |
| * @return $this | |
| */ | |
| public function setBuyUrl(?string $buyUrl) | |
| { | |
| $this->buyUrl = $buyUrl; | |
| return $this; | |
| } | |
| /** | |
| * @deprecated This method is deprecated and will be removed in a future major release. | |
| * | |
| * @param bool $isArchived | |
| * | |
| * @return $this | |
| */ | |
| public function setIsArchived(bool $isArchived) | |
| { | |
| $this->isArchived = $isArchived; | |
| return $this; | |
| } | |
| /** | |
| * @deprecated This method is deprecated and will be removed in a future major release. | |
| * | |
| * @return string|null | |
| */ | |
| public function getHash(): ?string | |
| { | |
| return $this->hash; | |
| } | |
| /** |
No description provided.