Skip to content

Pine - Kayla Huddleston#76

Open
khuddleup wants to merge 2 commits intoAda-C16:masterfrom
khuddleup:master
Open

Pine - Kayla Huddleston#76
khuddleup wants to merge 2 commits intoAda-C16:masterfrom
khuddleup:master

Conversation

@khuddleup
Copy link

No description provided.

@khuddleup
Copy link
Author

I just realized that my code has a "PATCH" endpoint for handeling a single goal..BUT THERE IS NO CODE for that end point. LMAOOOOO. MY BAD!

Copy link

@alope107 alope107 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job Kayla! This was a huge project and you nailed it!

One thing you should try to do for future projects is to make smaller, more frequent commits. This helps keep your code organized, safe from computer failure, and easier to collaborate with.

No worries about the unimplemented PATCH! After seeing your comment I went back and looked my implementation. I had accidentally done almost the exact same thing 😂

Very well done, this project is solidly green.

@@ -1,6 +1,15 @@
from flask import current_app
from app import db
from sqlalchemy.orm import backref

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need this import statement. The backref on line 9 is just a named parameter for the relationship function instead of an object we need to import.

title = db.Column(db.String)
description = db.Column(db.String)
completed_at = db.Column(db.DateTime, nullable=True)
goal_id =db.Column(db.Integer, db.ForeignKey('goal.goal_id'), nullable=True)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice foreign key relationship!

Comment on lines +14 to +33
if self.goal_id == None:
if not self.completed_at:
self.completed_at = None
else:
self.completed_at = True
return {
"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": True if self.completed_at else False
}
else:
task_dict = {
"id": self.task_id,
"goal_id": self.goal_id,
"title": self.title,
"description": self.description,
"is_complete": self.completed_at is not None
}
return task_dict

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely works! For an extra challenge, see if you can think of how to re-write this with less duplication of code. It might be helpful to start off with a dictionary with all the shared values, and then only add the additional values you need in the conditionals.

"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": True if self.completed_at else False

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of truthiness and the ternary operator!

Comment on lines +21 to +30
if request.args.get('sort') == 'asc':
tasks = Task.query.order_by(Task.title.asc()).all()
elif request.args.get('sort') == 'desc':
tasks = Task.query.order_by(Task.title.desc()).all()

#WV1: No Saved Tasks, One Saved Tasks
else:
tasks = Task.query.all()
for task in tasks:
tasks_response.append(task.to_dict())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome condensed code here!

if completion_status == "mark_incomplete":
task.completed_at = None
db.session.commit()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding logic here to send back an error code if the user attempts to pass in an invalid completion_status

Comment on lines +112 to +113
for goal in goals:
goal_response.append(goal.to_dict())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! As an additional challenge, can you think of how you would do this in a list comprehension? Either way is fine, it's just a matter of personal style.


@goal_bp.route("/<goal_id>", methods=["GET", "DELETE", "PUT", "PATCH"])
def handle_single_goal(goal_id):
goal_id = int(goal_id)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding validation here so an error response is made if the user passes in a non-integer goal_id

Comment on lines +167 to +173
for task in goal.tasks:
task_list.append(task.to_dict())
return {
"id": goal.goal_id,
"title": goal.title,
"tasks": task_list
}, 200

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

Comment on lines +81 to +90
assert response.status_code == 200
assert "goal" in response_body
assert response_body == {
"goal": {
"id": 1,
"title": "Updated Goal Title"
}
}
goal = Goal.query.get(1)
assert goal.title == "Updated Goal Title"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job asserting that both the response is correct and the DB is updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants