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

Amethyst - Hannah Watkins #132

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Conversation

Hannah1Watkins
Copy link

No description provided.

@@ -1,5 +1,12 @@
from app import db


class Goal(db.Model):

Choose a reason for hiding this comment

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

Great job completing Task List Hannah! Your code looks clean and easily readable. I want to point out the good variable naming and your code is DRY. Great use of helper methods in your models! Excellent work using blue prints & creating RESTful CRUD routes for each model.

Comment on lines +8 to +12
def to_dict(self):
return{
"id": self.goal_id,
"title": self.title,
}

Choose a reason for hiding this comment

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

Great job creating this reusable helper function

Comment on lines +9 to +10
goal_id = db.Column(db.Integer, db.ForeignKey("goal.goal_id"), nullable=True)
goal = db.relationship("Goal", back)

Choose a reason for hiding this comment

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

Nice approach creating this relationship to your Goal model.

Comment on lines +35 to +38
from app.routes import tasks_bp
from app.routes import goals_bp
app.register_blueprint(tasks_bp)
app.register_blueprint(goals_bp)

Choose a reason for hiding this comment

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

Great work registering these blueprints ✅

from flask import Blueprint, jsonify, request

Choose a reason for hiding this comment

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

👍🏾 These imports help a great deal when it comes to our request & response cycle

Comment on lines +17 to +21
tasks = Task.query.order_by(Task.title.asc()).all()
elif sort == "desc":
tasks = Task.query.order_by(Task.title.desc()).all()
else:

Choose a reason for hiding this comment

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

Great job with this query param logic

)

db.session.add(new_task)

Choose a reason for hiding this comment

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

Great job using db.session.<method-name>. Session gives us access to the follow:

  • db is our app’s instance of SQLAlchemy.
  • session represents our active database connection.
  • By referencing db.session we can use SQLAlchemy’s methods to perform tasks like:
    • committing a change to a model
    • storing a new record of a model
    • deleting a record.
  • By referencing db.session.add() you are able to use the SQLAlchemy method to store a new record of the task model

"is_complete": new_task.completed_at != None
}
}), 201

Choose a reason for hiding this comment

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

Usually a resource creation relates to a 201 response code 👍🏾

def test_get_tasks_for_specific_goal_no_goal(client):
# Act
response = client.get("/goals/1/tasks")
response_body = response.get_json()

# Assert
assert response.status_code == 404
assert response_body == {"message": "Goal not found"}

Choose a reason for hiding this comment

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

Nice work with this test completion! This assertion is a great way to validate functionality is working as expected

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