-
Notifications
You must be signed in to change notification settings - Fork 14
Implement post /tasks/{task id}/labels/ api to add a label to a task #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,4 +93,32 @@ | |
5. To fix lint issues | ||
``` | ||
ruff check --fix | ||
``` | ||
``` | ||
|
||
## API Documentation | ||
|
||
### Tasks | ||
|
||
#### Add Label to Task | ||
``` | ||
POST /v1/tasks/{task_id}/labels | ||
|
||
Add a label to an existing task. | ||
|
||
Request Body: | ||
{ | ||
"label_id": "string" // ID of the label to add | ||
} | ||
|
||
Responses: | ||
200 OK - Label added successfully | ||
400 Bad Request - Invalid input (e.g., invalid label ID format) | ||
404 Not Found - Task or label not found | ||
500 Internal Server Error - Unexpected server error | ||
|
||
Example: | ||
POST /v1/tasks/123/labels | ||
{ | ||
"label_id": "456" | ||
} | ||
``` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Add missing trailing newline. Files should end with a single newline character as indicated by the static analysis. Add a trailing newline at the end of the file: }
In README.md at line 124, the file is missing a trailing newline character at
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,12 @@ | ||||||||||||||||||||||
from rest_framework import serializers | ||||||||||||||||||||||
from todo.models.common.pyobjectid import PyObjectId | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
class AddLabelSerializer(serializers.Serializer): | ||||||||||||||||||||||
label_id = serializers.CharField(required=True) | ||||||||||||||||||||||
|
||||||||||||||||||||||
def validate_label_id(self, value): | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
return PyObjectId(value) | ||||||||||||||||||||||
except Exception: | ||||||||||||||||||||||
raise serializers.ValidationError("Invalid label ID format") | ||||||||||||||||||||||
Comment on lines
+8
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve exception handling specificity. The generic Use more specific exception handling: def validate_label_id(self, value):
try:
return PyObjectId(value)
- except Exception:
+ except (ValueError, TypeError):
raise serializers.ValidationError("Invalid label ID format") This way, only expected validation errors are caught, and any unexpected exceptions will bubble up for proper debugging. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -221,3 +221,45 @@ def create_task(cls, dto: CreateTaskDTO) -> CreateTaskResponse: | |||||||||||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
@staticmethod | ||||||||||||||||||||||||||||||||||||||
def add_label_to_task(task_id: PyObjectId, label_id: PyObjectId) -> None: | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix the missing import for PyObjectId. The static analysis correctly identifies that Add this import at the top of the file with the other imports: +from todo.models.common.pyobjectid import PyObjectId 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.11.9)226-226: Undefined name (F821) 226-226: Undefined name (F821) 🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
Add a label to a task. | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Args: | ||||||||||||||||||||||||||||||||||||||
task_id: The ID of the task | ||||||||||||||||||||||||||||||||||||||
label_id: The ID of the label to add | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Raises: | ||||||||||||||||||||||||||||||||||||||
ValueError: If the task or label doesn't exist | ||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||
from todo.repositories.task_repository import TaskRepository | ||||||||||||||||||||||||||||||||||||||
from todo.repositories.label_repository import LabelRepository | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+237
to
+238
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Move imports to module level. Imports inside functions are generally an anti-pattern and can hurt performance. These repositories are already imported at the module level. Remove these local imports since - from todo.repositories.task_repository import TaskRepository
- from todo.repositories.label_repository import LabelRepository
- 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
# Check if task exists | ||||||||||||||||||||||||||||||||||||||
task = TaskRepository.get_by_id(task_id) | ||||||||||||||||||||||||||||||||||||||
if not task: | ||||||||||||||||||||||||||||||||||||||
raise ValueError(ApiErrorResponse( | ||||||||||||||||||||||||||||||||||||||
statusCode=404, | ||||||||||||||||||||||||||||||||||||||
message="Task not found", | ||||||||||||||||||||||||||||||||||||||
errors=[{"detail": f"Task with ID {task_id} does not exist"}] | ||||||||||||||||||||||||||||||||||||||
)) | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+243
to
+247
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Maintain consistency with existing error response patterns. The error response structure here doesn't match the pattern used elsewhere in this file. The existing pattern uses Update the error response to match the existing pattern: - raise ValueError(ApiErrorResponse(
- statusCode=404,
- message="Task not found",
- errors=[{"detail": f"Task with ID {task_id} does not exist"}]
- ))
+ raise ValueError(
+ ApiErrorResponse(
+ statusCode=404,
+ message="Task not found",
+ errors=[
+ ApiErrorDetail(
+ source={ApiErrorSource.PARAMETER: "task_id"},
+ title="Task not found",
+ detail=f"Task with ID {task_id} does not exist",
+ )
+ ],
+ )
+ ) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
# Check if label exists | ||||||||||||||||||||||||||||||||||||||
label = LabelRepository.get_by_id(label_id) | ||||||||||||||||||||||||||||||||||||||
if not label: | ||||||||||||||||||||||||||||||||||||||
raise ValueError(ApiErrorResponse( | ||||||||||||||||||||||||||||||||||||||
statusCode=404, | ||||||||||||||||||||||||||||||||||||||
message="Label not found", | ||||||||||||||||||||||||||||||||||||||
errors=[{"detail": f"Label with ID {label_id} does not exist"}] | ||||||||||||||||||||||||||||||||||||||
)) | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+252
to
+256
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Maintain consistency with existing error response patterns. Same issue as above - the error response structure should match the existing pattern. Update the error response to match the existing pattern: - raise ValueError(ApiErrorResponse(
- statusCode=404,
- message="Label not found",
- errors=[{"detail": f"Label with ID {label_id} does not exist"}]
- ))
+ raise ValueError(
+ ApiErrorResponse(
+ statusCode=404,
+ message="Label not found",
+ errors=[
+ ApiErrorDetail(
+ source={ApiErrorSource.PARAMETER: "label_id"},
+ title="Label not found",
+ detail=f"Label with ID {label_id} does not exist",
+ )
+ ],
+ )
+ ) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
# Initialize labels list if None | ||||||||||||||||||||||||||||||||||||||
if task.labels is None: | ||||||||||||||||||||||||||||||||||||||
task.labels = [] | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
# Add label if not already present | ||||||||||||||||||||||||||||||||||||||
if label_id not in task.labels: | ||||||||||||||||||||||||||||||||||||||
task.labels.append(label_id) | ||||||||||||||||||||||||||||||||||||||
TaskRepository.update(task) | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+263
to
+265
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Convert PyObjectId to string before storing in labels list. The Convert the - if label_id not in task.labels:
- task.labels.append(label_id)
+ label_id_str = str(label_id)
+ if label_id_str not in task.labels:
+ task.labels.append(label_id_str) 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,11 +6,13 @@ | |
|
||
from todo.serializers.get_tasks_serializer import GetTaskQueryParamsSerializer | ||
from todo.serializers.create_task_serializer import CreateTaskSerializer | ||
from todo.serializers.add_label_serializer import AddLabelSerializer | ||
from todo.services.task_service import TaskService | ||
from todo.dto.task_dto import CreateTaskDTO | ||
from todo.dto.responses.error_response import ApiErrorResponse, ApiErrorDetail, ApiErrorSource | ||
from todo.dto.responses.create_task_response import CreateTaskResponse | ||
from todo.constants.messages import ApiErrors | ||
from todo.models.common.pyobjectid import PyObjectId | ||
|
||
class TaskView(APIView): | ||
def get(self, request: Request): | ||
|
@@ -59,6 +61,51 @@ def post(self, request: Request): | |
data=fallback_response.model_dump(mode="json"), status=status.HTTP_500_INTERNAL_SERVER_ERROR | ||
) | ||
|
||
def post_label(self, request: Request, task_id: str): | ||
""" | ||
Add a label to a task. | ||
|
||
Args: | ||
request: HTTP request containing label data | ||
task_id: ID of the task to add the label to | ||
|
||
Returns: | ||
Response: HTTP response with success or error details | ||
""" | ||
try: | ||
task_id = PyObjectId(task_id) | ||
except Exception: | ||
return Response( | ||
data=ApiErrorResponse( | ||
statusCode=400, | ||
message="Invalid task ID format", | ||
errors=[{"detail": "The provided task ID is not valid"}] | ||
).model_dump(mode="json"), | ||
status=status.HTTP_400_BAD_REQUEST | ||
) | ||
Comment on lines
+75
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainUse more specific exception handling for ObjectId conversion. The broad try:
task_id = PyObjectId(task_id)
- except Exception:
+ except (ValueError, TypeError, InvalidId):
return Response( Note: You may need to import Let me verify what specific exceptions PyObjectId raises: 🏁 Script executed: #!/bin/bash
# Description: Check PyObjectId implementation to understand what exceptions it raises
# Expected: Find the specific exceptions that PyObjectId constructor can raise
ast-grep --pattern $'class PyObjectId {
$$$
def __init__($$$) {
$$$
}
$$$
}'
# Also search for any exception handling patterns in existing PyObjectId usage
rg -A 5 -B 5 "PyObjectId\(" Length of output: 1738 🏁 Script executed: #!/bin/bash
# Display the implementation of PyObjectId to determine which exceptions it raises
cat todo/models/common/pyobjectid.py Length of output: 430 Use specific exception classes for ObjectId conversion To avoid masking other errors, catch only the exceptions raised by the ObjectId constructor: • File:
🤖 Prompt for AI Agents
|
||
|
||
serializer = AddLabelSerializer(data=request.data) | ||
if not serializer.is_valid(): | ||
return self._handle_validation_errors(serializer.errors) | ||
|
||
try: | ||
TaskService.add_label_to_task(task_id, serializer.validated_data["label_id"]) | ||
return Response(status=status.HTTP_200_OK) | ||
|
||
except ValueError as e: | ||
if isinstance(e.args[0], ApiErrorResponse): | ||
error_response = e.args[0] | ||
return Response(data=error_response.model_dump(mode="json"), status=error_response.statusCode) | ||
|
||
fallback_response = ApiErrorResponse( | ||
statusCode=500, | ||
message=ApiErrors.UNEXPECTED_ERROR_OCCURRED, | ||
errors=[{"detail": str(e) if settings.DEBUG else ApiErrors.INTERNAL_SERVER_ERROR}], | ||
) | ||
return Response( | ||
data=fallback_response.model_dump(mode="json"), status=status.HTTP_500_INTERNAL_SERVER_ERROR | ||
) | ||
|
||
def _handle_validation_errors(self, errors): | ||
formatted_errors = [] | ||
for field, messages in errors.items(): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Fix markdown formatting issues.
The static analysis correctly identifies several markdown formatting issues that should be addressed for consistency and readability.
Apply these formatting fixes:
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
102-102: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents