Skip to content

feat: Task API update#266

Open
bhdnb wants to merge 1 commit intomasterfrom
task-api-update
Open

feat: Task API update#266
bhdnb wants to merge 1 commit intomasterfrom
task-api-update

Conversation

@bhdnb
Copy link
Collaborator

@bhdnb bhdnb commented Mar 24, 2026

No description provided.

@bhdnb bhdnb self-assigned this Mar 24, 2026
@bhdnb bhdnb marked this pull request as draft March 24, 2026 15:21
@codecov
Copy link

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.98%. Comparing base (8e24d5c) to head (029ae20).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bhdnb bhdnb linked an issue Mar 24, 2026 that may be closed by this pull request
@bhdnb bhdnb force-pushed the task-api-update branch from 3016118 to 029ae20 Compare March 24, 2026 15:29
@bhdnb bhdnb marked this pull request as ready for review March 24, 2026 15:32
@bhdnb bhdnb requested a review from andrii-bodnar March 24, 2026 15:32
@andrii-bodnar andrii-bodnar requested a review from Copilot March 24, 2026 15:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 / TaskForUpdate models to include additional task fields (batch, URLs, labels, costs, etc.) and new update filters.
  • Added TaskApi::listTasks($userId, ...) for /users/{userId}/tasks and 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;
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
: 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;

Copilot uses AI. Check for mistakes.
@@ -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');
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$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.

Suggested change
$this->type = (string)$this->getDataProperty('type');
$this->type = (int)$this->getDataProperty('type');

Copilot uses AI. Check for mistakes.
return $this->vendor;
}

/**
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
/**
/**
* @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;
}
/**

Copilot uses AI. Check for mistakes.
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.

Tasks API updates

2 participants