fix(tasks): mirror parent_id to Project body on create when absent - #574
prasad-albert wants to merge 3 commits into
Conversation
|
Looks good - no bugs or correctness issues found. |
| The location where the property task is performed. | ||
| parent_id : str | ||
| The ID of the parent project. | ||
| The ID of the parent inventory item. |
There was a problem hiding this comment.
waiting on confirmation of what exactly parentId is here? inventory or project
There was a problem hiding this comment.
The agent worker workaround mirrors parent_id → project for PropertyTask.This PR does two things that point the other way:
In create(), it mirrors parent_id → Project body only for BatchTask (isinstance(task, BatchTask)), not PropertyTask.
BaseTask carries both parent_id and project inherited by every task type, so both interpretations are structurally possible. The two views cannot both be correct:
If this change is right(PropertyTask.parent_id is an inventory item, only BatchTask.parent_id is a project): then the worker's PropertyTask mirror is actively wrong. It is stuffing an inventory id into the Project entity link. That mirror should be removed, not kept, and this is the correct fix for the BatchTask side.
If the worker is right (PropertyTask.parent_id is a project), then this change is incomplete (it should also mirror PropertyTask) and its docstring change is wrong.
I believe this needs confirmation from the task API owner or a real create test for both task types before merge
There was a problem hiding this comment.
confirmed with @siddhant-albertinvent -- parentId is always project ID irrespective of task type.
There was a problem hiding this comment.
actually, the tests are failing and they expect parent ID to be inventory ID for Property Tasks
cc: @siddhant-albertinvent
When a BatchTask is created with parent_id set but project omitted, the API requires the Project entity in the request body for ACL resolution. Without it the server crashes at runtime. Auto-populate Project from parent_id in that case. Also corrects the PropertyTask docstring: parent_id refers to the parent inventory item, not a project.
parentId in the query param and Project.id in the payload must always match, regardless of task type. Also reverts an incorrect PropertyTask docstring edit (parent_id is the parent project, not inventory item).
2efd859 to
cde27ad
Compare
|
Looks good - no bugs or correctness issues found. |
Only inject Project into the create payload when it is missing and parent_id is a PRO-prefixed project ID, preserving the inventory parent_id + explicit project pattern used by property tasks.
|
Looks good - no bugs or correctness issues found. |
|
Bump: this has been open since June with prior reviews from prakashchandrahv and sumedhb1995. @prasad-albert is this still wanted? If yes, happy to rebase it onto current main and re-request review. |
What
General and property task creates with
parent_idset to a project ID now automatically include theProjectentity link in the POST body when callers omitproject, so tasks are linked to the project on create.Why
Staging verification showed that without
Projectin the request body, General and Property tasks could be created with only a queryparentIdbut were not associated with the project. Batch tasks already worked without this (the API backfillsProjectfrom the query param).How
Mirror
parent_idintoProject.idonly when:Projectis not already in the serialized payload (preserves property tasks that useparent_id=inventory_idwith an explicitproject), andparent_idstarts withPRO(project ID, not inventory).Testing
uv run ruff format .uv run ruff check . --fixuv run pytest tests/collections/test_tasks.py -vSDK Changes
Task creates with
parent_idset to a project ID no longer require callers to also setprojectfor General and Property tasks to be linked to that project.