Skip to content

Cedar - Alie Ibarra#85

Open
alieibarra wants to merge 14 commits intoAda-C16:masterfrom
alieibarra:master
Open

Cedar - Alie Ibarra#85
alieibarra wants to merge 14 commits intoAda-C16:masterfrom
alieibarra:master

Conversation

@alieibarra
Copy link

No description provided.

Copy link

@kaidamasaki kaidamasaki left a comment

Choose a reason for hiding this comment

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

Well done!

I left a few minor notes but your code looks really good overall! Congrats!

PS: Great job committing regularly!

Comment on lines +15 to +18
if not self.completed_at:
is_complete = False
else:
is_complete = True

Choose a reason for hiding this comment

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

I think you probably meant to clean this up since the code below (line 20) handles this on its own.

Comment on lines +23 to +27
# if not self.completed_at:
# is_complete = False
# else:
# is_complete = True

Choose a reason for hiding this comment

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

Style: It's good practice to clean up commented out code.

goals_bp = Blueprint("goals", __name__, url_prefix="/goals")

#-----------------
#HELPER FUNCTIONS

Choose a reason for hiding this comment

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

Nice helper functions. 😃

request_body = request.get_json()

if "title" not in request_body or "description" not in request_body or "completed_at" not in request_body:
return make_response(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.

If you're returning a dictionary you don't need to call jsonify or make_response Flask's default return behavior will work just fine.

Suggested change
return make_response(jsonify({"details" : "Invalid data"}), 400)
return {"details" : "Invalid data"}, 400

When you return a dictionary Flask automatically makes a JSON response. (This isn't true for a list which is why we need jsonify sometimes.)

if completion_status == "mark_incomplete":
task.completed_at = None

#task_dict["task"] = task.to_dict()

Choose a reason for hiding this comment

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

You didn't commit before turning from here.

Things will look right in your response but won't be saved between requests.

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