-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add task service for get task API #12
base: feat-get-todo-api-view-and-validator
Are you sure you want to change the base?
Add task service for get task API #12
Conversation
074fa71
to
f91df5a
Compare
bf89c2a
to
81848f5
Compare
createdAt: datetime | None = None | ||
updatedAt: datetime | None = None | ||
createdBy: UserDTO | None = None | ||
updatedBy: UserDTO | None = None |
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.
These values can ever be none?
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.
Yes. This label DTO can be used in other places also
In get tasks API, we only need to send name and color of the label
from todo.dto.task_dto import TaskDTO | ||
|
||
|
||
class GetTasksResponse(PaginatedResponse): |
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.
why not TasksResponse
?
createdAt: datetime | None = None | ||
updatedAt: datetime | None = None | ||
createdBy: UserDTO | None = None | ||
updatedBy: UserDTO | None = None |
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.
In what scenario, all these fields will be none?
next: str | None = None | ||
prev: str | None = None |
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.
The datatype will be string or number?
id: str | ||
displayId: str |
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.
what is the difference b/w id and displayId?
displayId: str | ||
title: str | ||
description: str | None = None | ||
priority: TaskPriority | None = None |
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.
Why priority can be none?
title: str | ||
description: str | None = None | ||
priority: TaskPriority | None = None | ||
status: TaskStatus | None = None |
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.
Why status can be none?
updatedBy: UserDTO | None = None | ||
|
||
class Config: | ||
json_encoders = {TaskPriority: lambda x: x.name} |
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.
What is it doing?
@classmethod | ||
def get_tasks(cls, page, limit) -> GetTasksResponse: |
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.
Why is it a class method?
what is cls?
links_data.prev = f"{cls.tasks_api_base_url}?page={page-1}&limit={limit}" | ||
|
||
if tasks_count > tasks_skip_count + limit: | ||
links_data.next = f"{cls.tasks_api_base_url}?page={page+1}&limit={limit}" |
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.
Can we create an util function which will create the next and prev link based on given input?
if tasks_count > tasks_skip_count + limit: | ||
links_data.next = f"{cls.tasks_api_base_url}?page={page+1}&limit={limit}" | ||
|
||
if links_data.prev is not None or links_data.next is not None: |
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.
if links_data.prev is not None or links_data.next is not None: | |
if not links_data.prev or not links_data.next: |
response.links = links_data | ||
return response | ||
|
||
@classmethod |
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.
Why is it a class method?
Date: 20 Dec 2024
Developer Name: Samarpan Harit
Important
THIS PR MUST BE MERGED AFTER
Issue Ticket Number
Description
SYSTEM
username. When we add authentication, we'll be using the correct use details hereSuccess Response
URL - GET http://127.0.0.1:8000/v1/tasks?page=1&limit=1Response Code: 200
Error Response (Incorrect query params)
URL - GET http://127.0.0.1:8000/v1/tasks?page=1&limit=-1Response Code: 400
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Success Response
Error Response (Incorrect query params)
Test Coverage
Coverage report
Additional Notes
The CI is failing right now because there an issue in the workflow. I have raised a PR to fix this. Once it's merged the CI should pass