Conversation
Refactor not found components to use a common handleReturn function for navigation.
Remove console log for history length in TaskNotFoundComponent.
Added a new 'Return to Previous Page' button with smart fallback navigation.
Updated link for 'Return to Previous Page' button feature.
Updated the link for the 'Return to Previous Page' button in the changelog.
Adds organization and organization_id properties to the Data model to resolve AttributeError during permissions checks in DRF browsable API.
There was a problem hiding this comment.
Pull request overview
Fixes a Django REST Framework Browsable API 500 on GET /api/jobs/{id}/data/meta by ensuring IAM permission checks can resolve an organization from a Data instance.
Changes:
- Added
organizationandorganization_idproperties toData(delegating to the relatedTask). - Added a changelog fragment documenting the fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cvat/apps/engine/models.py | Adds Data.organization / Data.organization_id to support IAM permission resolution in browsable API flows. |
| changelog.d/20260412_133400_aramesh_fix_data_organization_id.md | Documents the fixed 500 error in the changelog fragments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def organization(self): | ||
| try: | ||
| return self.tasks.first().organization | ||
| except AttributeError: | ||
| return None | ||
|
|
||
| @property | ||
| def organization_id(self): | ||
| try: | ||
| return self.tasks.first().organization_id | ||
| except AttributeError: | ||
| return None |
There was a problem hiding this comment.
The organization property is using a broad try/except AttributeError as control flow. This will also mask unrelated AttributeErrors (e.g., if something inside the accessed attributes changes) and makes debugging harder. Consider explicitly handling the “no related task” case (and optionally the “unsaved Data instance” case) and adding a return type annotation for consistency with other organization(_id) properties in this module (e.g., Job.organization_id).
| def organization(self): | |
| try: | |
| return self.tasks.first().organization | |
| except AttributeError: | |
| return None | |
| @property | |
| def organization_id(self): | |
| try: | |
| return self.tasks.first().organization_id | |
| except AttributeError: | |
| return None | |
| def organization(self) -> Organization | None: | |
| task = self.tasks.first() | |
| if task is None: | |
| return None | |
| return task.organization | |
| @property | |
| def organization_id(self) -> int | None: | |
| task = self.tasks.first() | |
| if task is None: | |
| return None | |
| return task.organization_id |
| @property | ||
| def organization(self): | ||
| try: | ||
| return self.tasks.first().organization | ||
| except AttributeError: | ||
| return None | ||
|
|
||
| @property | ||
| def organization_id(self): | ||
| try: | ||
| return self.tasks.first().organization_id | ||
| except AttributeError: | ||
| return None |
There was a problem hiding this comment.
organization_id currently calls self.tasks.first() on every access and relies on catching AttributeError for the empty-related-set case. It would be more robust to explicitly check for a related task (and potentially cache the resolved org/org_id for the lifetime of the model instance) to avoid repeated queries and to avoid swallowing unexpected AttributeErrors.
| @property | |
| def organization(self): | |
| try: | |
| return self.tasks.first().organization | |
| except AttributeError: | |
| return None | |
| @property | |
| def organization_id(self): | |
| try: | |
| return self.tasks.first().organization_id | |
| except AttributeError: | |
| return None | |
| def _get_first_task_for_organization(self): | |
| if not hasattr(self, "_first_task_for_organization_cache"): | |
| self._first_task_for_organization_cache = self.tasks.first() | |
| return self._first_task_for_organization_cache | |
| @property | |
| def organization(self): | |
| task = self._get_first_task_for_organization() | |
| return task.organization if task is not None else None | |
| @property | |
| def organization_id(self): | |
| task = self._get_first_task_for_organization() | |
| return task.organization_id if task is not None else None |
| @@ -0,0 +1,2 @@ | |||
| ### Fixed | |||
There was a problem hiding this comment.
Changelog fragments in changelog.d/ consistently include a blank line after the section header (e.g., other recent fragments use ### Fixed followed by an empty line, then the bullet). Add the missing blank line here to match the established formatting (and keep the fragment consistent with tooling that may expect that structure).
| ### Fixed | |
| ### Fixed |
Fix API 500 error on /api/jobs/{id}/data/meta
Description
This PR resolves an issue where requesting the
/api/jobs/{id}/data/metaendpoint via the Django REST Framework Browsable API resulted in a 500 Internal Server Error (AttributeError: 'Data' object has no attribute 'organization_id').Root Cause:
When loading the browsable API UI, DRF attempts to render an HTML form for
PATCHrequests and automatically invokescheck_object_permissionson theDatainstance associated with the view. The CVAT IAM permission flow looks forobj.organization_id, but theDatamodel lacked this attribute.Fix:
Added
organizationandorganization_idas properties on theDatamodel. The properties lazily evaluate and delegate to the model's related parentTask.How to manually test it
3).GETrequest tohttp://localhost:7000/api/jobs/3/data/meta. You can do this by:200 OKstatus code along with the expected metadata JSON payload, and no 500 traceback is thrown in the console.Screenshots
Here is a test confirming the endpoint now returns the proper
200 OKresponse payload without failing on permission checks:Closes #9893