Skip to content
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

Accelerate-Christina S #10

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

Peacegypsy
Copy link

Somehow all my passing tests are not all passing. I had all passing tests thru wave-05. Not sure what happened.

@@ -31,7 +31,7 @@ def client(app):
@pytest.fixture
def one_task(app):
new_task = Task(
title="Go on my daily walk 🏞", description="Notice something new every day", completed_at=None)
title="Go on my daily walk 🏞",description="Notice something new every day",completed_at=None)
Copy link

Choose a reason for hiding this comment

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

I would put the space back. It's affecting a few of your tests passing.

Suggested change
title="Go on my daily walk 🏞",description="Notice something new every day",completed_at=None)
title="Go on my daily walk 🏞", description="Notice something new every day", completed_at=None)

for task in goal.tasks:
single_task= {
"id": task.task_id,
"goal_id": goal.goal_id,
Copy link

Choose a reason for hiding this comment

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

Here I would add a check if the task has a goal id. Something like
if task.goal_id: dict["goal_id"] = task.goal_id]
so its only added to the dictionary if there is a goal_id

Comment on lines +3 to +4
from sqlalchemy import ForeignKey, update
from sqlalchemy.orm import relationship
Copy link

Choose a reason for hiding this comment

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

remove these imports

Comment on lines +3 to +4
from sqlalchemy.orm import relationship
from sqlalchemy import ForeignKey, update
Copy link

Choose a reason for hiding this comment

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

remove these imports

title = db.Column(db.VARCHAR(50))
description = db.Column(db.String)
completed_at = db.Column(db.DateTime, nullable=True, default=None)
is_complete = db.Column(db.Boolean, default=False)
Copy link

Choose a reason for hiding this comment

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

I think adding this here creates confusion and is why some of your tests for mark complete/incomplete are failing. I would remove this line and just add an instance method for self.completed_at. It could be here in the Task model or in routes.py.

Suggested change
is_complete = db.Column(db.Boolean, default=False)

an example of how that could look
def build_dict_from_task(task): is_complete = True if task.completed_at else False dict = { "id": task.task_id, "title": task.title, "description": task.description, "is_complete": is_complete }

{"id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": bool(task.completed_at)
Copy link

@tgoslee tgoslee Jun 19, 2021

Choose a reason for hiding this comment

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

if this is marked incomplete then "is_complete" would equal False

{"id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": bool(task.completed_at)
Copy link

@tgoslee tgoslee Jun 19, 2021

Choose a reason for hiding this comment

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

if this is marked complete then "is_complete" would equal to True

Comment on lines +27 to +53
def handle_goals():
if request.method == "GET":
goals = Goal.query.all()
goal_response = []
for goal in goals:
goal_response.append({
"title": goal.title,
"id": goal.goal_id
})
return jsonify(goal_response), 200

elif request.method == "POST":
request_body = request.get_json()
title = request_body.get("title")
if not title:
return jsonify({"details": "Invalid data"}), 400

new_goal = Goal(title=request_body["title"])
db.session.add(new_goal)
db.session.commit()

commited_goal = {"goal":
{"id": new_goal.goal_id,
"title": new_goal.title
}}

return (commited_goal), 201
Copy link

Choose a reason for hiding this comment

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

💃🏽

"title": goal.title
}
}
return selected_goal
Copy link

Choose a reason for hiding this comment

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

just to be consistent I would add

Suggested change
return selected_goal
return jsonify(selected_goal), 200

if request.method == "GET":
title_query = request.args.get("title")
if title_query:
tasks = Task.query.filter(Task.title.ilike(f'%{title_query}%'))
Copy link

Choose a reason for hiding this comment

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

ouu interesting what made you choose ilike() instead of filter_by()

Comment on lines +181 to +203
if request.method == "GET":
if task == None:
return make_response("No matching task found", 404)
if request.method == "GET":
if task.goal_id == None:
selected_task = {"task":
{"task_id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": task.is_complete
}
}
return selected_task
else:
return make_response ({
"task": {
"id": task.task_id,
"goal_id": task.goal_id,
"title": task.title,
"description": task.description,
"is_complete": task.is_complete
}
})
Copy link

Choose a reason for hiding this comment

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

good check

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