-
Notifications
You must be signed in to change notification settings - Fork 111
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
C17/Bahareh/Sharks #108
base: master
Are you sure you want to change the base?
C17/Bahareh/Sharks #108
Conversation
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.
good job, Bahareh! One thing to consider is staying consistent as you can with your route responses. Sometimes you send through just a dictionary, sometimes you jsonify the dictionary, sometimes the status code isn't there, etc.
Double check your routes so that they are formatted consistently and that they have predictable behavior.
def to_json(self): | ||
if self.completed_at: | ||
is_completed=True | ||
else: | ||
is_completed=False | ||
task_dict= { | ||
"id": self.task_id, | ||
"title": self.title, | ||
"description": self.description, | ||
"is_complete": is_completed, | ||
} | ||
if self.goal_id: | ||
task_dict["goal_id"] = self.goal_id | ||
return task_dict | ||
|
||
def to_json_without_des(self): | ||
if self.completed_at: | ||
is_completed=True | ||
else: | ||
is_completed=False | ||
return { | ||
"id": self.task_id, | ||
"title": self.title, | ||
"description": "", | ||
"is_complete": is_completed, | ||
} |
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.
👍 ohhh cool! Interesting way to separate out which tasks need which attributes! I wonder if we could combine these two methods back together with an if statement, though. Something to think about!
goal_list_bp = Blueprint("goal_list", __name__,url_prefix="/goals") |
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.
one thing we could do to help with too much in one file is by separating these two blueprints out into their own files. This can help bigger teams work on different tasks in a project without stepping on each other's toes.
try: | ||
task_id = int(task_id) | ||
except: | ||
abort(make_response({"message":f"task {task_id} invalid"}, 400)) | ||
|
||
task = Task.query.get(task_id) | ||
|
||
if not task: | ||
abort(make_response({"message":f"task {task_id} not found"}, 404)) | ||
|
||
return task |
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.
we could put this in a separate file to keep our routes file tidier
|
||
|
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.
be careful with too much blank lines! it can make code harder to read
|
||
|
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.
) |
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.
new_goal = Goal(title=request_body["title"], | |
) | |
new_goal = Goal(title=request_body["title"]) |
"id": goal.goal_id, | ||
"title": goal.title, | ||
}} |
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.
don't forget your status code!
|
||
for task in tasks: | ||
task_list.append(task.to_json()) | ||
return make_response(jsonify({ | ||
"id": goal.goal_id, | ||
"title": goal.title, | ||
"tasks": task_list |
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.
just as we made a to_json
method for tasks, we could do the same for goals
|
||
|
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.
|
||
#Post tasks to a goal | ||
@goal_list_bp.route("/<goal_id>/tasks", methods=["POST"]) |
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.
👍
No description provided.