Skip to content

Cheetahs - Alexis Miller #125

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

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

Conversation

LexElena22
Copy link

No description provided.

Copy link

@jbieniosek jbieniosek left a comment

Choose a reason for hiding this comment

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

Great work on this project Alexis! This project is green based on the informal LP that we set up on 11/9, reducing the requirements to Waves 1-3.

description = db.Column(db.String(2600))
completed_at = db.Column(db.DateTime, nullable=True)

def to_dict(self):

Choose a reason for hiding this comment

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

👍


#Validate
def validate_task(task_id):

Choose a reason for hiding this comment

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

Great helper!


if "title" not in request_body or "description" not in request_body:
return jsonify({"details": "Invalid data"}), 400

Choose a reason for hiding this comment

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

👍

task_response.append(task.to_dict())

return jsonify(task_response)

Choose a reason for hiding this comment

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

Even though Flask will add the 200 for you, I recommend being explicit about the code everywhere, and not just in non 200 returns. Consistent formats make code easier to read.

Suggested change
return jsonify(task_response)
return jsonify(task_response), 200

task = validate_task(task_id)

return make_response({"task": task.to_dict()})

Choose a reason for hiding this comment

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

The function make_response isn't incorrect here, but I notice that some functions return jsonify and others return make_response. I recommend picking one format and using it throughout your code - having different formats that do the same thing can make it harder for someone reading your code to realize that the behavior is the same.

task = validate_task(task_id)

task.completed_at = date.today()

Choose a reason for hiding this comment

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

💯

# *****************************************************************
# **Complete test with assertion about response body***************
# *****************************************************************

assert response_body == {"message" : "Task ID '1' not found"}

Choose a reason for hiding this comment

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

👍

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