Skip to content

Sharks-Xiomara R.#93

Open
xiomaraR wants to merge 4 commits intoada-c17:masterfrom
xiomaraR:master
Open

Sharks-Xiomara R.#93
xiomaraR wants to merge 4 commits intoada-c17:masterfrom
xiomaraR:master

Conversation

@xiomaraR
Copy link
Copy Markdown

Hey Claire, I'm still working on finishing wave 6 and deploying to Heroku. My VsCode crashed yesterday and was having technical difficulties

Copy link
Copy Markdown

@spitsfire spitsfire left a comment

Choose a reason for hiding this comment

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

Great job, Xiomara! You did a great job separating out helper functions and helper methods! Most of your route return statements were very consistent, but there were a few that could be altered. Consistent returns in an API is key.

Your logic and routes were very clean and organized! Woo hoo!

Comment thread app/goal_routes.py
@@ -0,0 +1,102 @@
from flask import Blueprint, jsonify, make_response, request
from requests import session
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This session is not the same as db.session. They serve completely separate purposes, so we can get rid of this import completely

Suggested change
from requests import session

Comment thread app/goal_routes.py
# Create goal


@goal_bp.route("", methods=["POST"])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread app/goal_routes.py
for goal in goals:
goals_response.append(goal.to_json())

return jsonify(goals_response), 200
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Try to stay consistent with your return statements here. Most of them are using make_response(jsonify()), so let's repeat that everywhere for predictability and consistency.

Suggested change
return jsonify(goals_response), 200
return make_response(jsonify(goals_response), 200)

Comment thread app/goal_routes.py
@goal_bp.route("/<id>", methods=["GET"])
def get_one_goal(id):
goal = validate_goal(id)
return jsonify({"goal": goal.to_json()}), 200
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
return jsonify({"goal": goal.to_json()}), 200
return make_response(jsonify({"goal": goal.to_json()}), 200)

Comment thread app/goal_routes.py
# Update goal


@goal_bp.route("/<id>", methods=["PUT"])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread app/routes.py


# Get One Task: One Saved Task
@task_bp.route("/<id>", methods=["GET"])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread app/routes.py

# Get Tasks

@task_bp.route("", methods=["POST", "GET"])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread app/routes.py
Comment on lines +113 to +123
def post_message_to_slack(task):
send_msg_path = "https://slack.com/api/chat.postMessage"
confirm_message = f"You completed the task {task.title}!"
query_params = {
"channel": "task-notifications",
"text": confirm_message
}
headers = {
"Authorization": os.environ.get("slack_token")
}
requests.post(send_msg_path, params=query_params, headers=headers)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's move this to the helpers.py file!

Comment thread app/routes.py
# Mark Complete


@task_bp.route("/<id>/mark_complete", methods=["PATCH"])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread app/routes.py


# Mark Incomplete
@task_bp.route("/<id>/mark_incomplete", methods=["PATCH"])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread app/goal_routes.py
goal = validate_goal(id)
request_body = request.get_json()

for id in request_body["id"]:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here is where I think the last wave is failing. Double check what the correct key is in the tests! id is for the goal, not the list of tasks

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