Conversation
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
Overall this looks good, Sabs! In the future, work on your exception handling and refactoring, but well done, Sabs!
| "SQLALCHEMY_DATABASE_URI") | ||
| # app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get( | ||
| # "SQLALCHEMY_DATABASE_URI") | ||
| app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get("RENDER_DATABASE_URI") |
| def make_dict(self): | ||
| return dict( | ||
| id = self.goal_id, | ||
| title = self.title) No newline at end of file |
| task_id = db.Column(db.Integer, primary_key=True, autoincrement=True) | ||
| title = db.Column(db.String) | ||
| description = db.Column(db.String) | ||
| completed_at = db.Column(db.DateTime, nullable=True, default=None) |
There was a problem hiding this comment.
The default for nullable is True, so we do not need to explicitly set the attribute to True here!
| load_dotenv() | ||
|
|
||
| tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks") | ||
| goals_bp = Blueprint("goals", __name__, url_prefix="/goals") |
There was a problem hiding this comment.
Overall, it's ok to have all of our routes here in one file because this is a pretty small project, but as you start building more and larger scale APIs, it's a good idea to separate your routes into their own files. For example, you could have a task_routes.py and a goal_routes.py as well. If you wanted to take it a step further, you can also have a helper_functions.py file that could be used to house any helper functions that apply to all routes (validate_model for example).
|
|
||
| if not goal: | ||
| abort(make_response({"message":f"goal {goal_id} not found"}, 404)) | ||
| return goal |
There was a problem hiding this comment.
This is a great start! As you refactor, make note of any places where you have code that looks really similar. For example, your validate_task and your validate_goal methods are so similar. You could refactor them into a single validate_model that looks something like this:
def validate_model(cls, model_id):
try:
model_id = int(model_id)
except:
abort(make_response({"message":f"{cls.__name__} {model_id} invalid"}, 400))
model = cls.query.get(model_id)
if not model:
abort(make_response({"message":f"{cls.__name__} {model_id} not found"}, 404))
return model
|
|
||
| if "description" not in request_body or "title" not in request_body: | ||
| abort(make_response({"details": "Invalid data"}, 400)) | ||
|
|
There was a problem hiding this comment.
While this exception handling can work, a more appropriate structure would be to use a try/catch block here. This allows you to catch a variety of errors as opposed to having to specify each thing that might be an issue. For example, we could replace the conditional here with:
try:
new_task = Task.from_dict(request_body)
except KeyError as error:
abort(make_response({"details": "Invalid data"}, 400))
This will attempt to make the new_task and if for whatever reason it can't, it will throw the error. While this functions the same way as your conditional, it's just a little more streamlined and easy to read!
| @tasks_bp.route("/<task_id>/mark_complete", methods =["PATCH"]) | ||
| def mark_complete(task_id): | ||
| task = validate_task(task_id) | ||
| task.completed_at = date.today() |
There was a problem hiding this comment.
Your patch methods and really any method where you plan on including any information in the request body is a good candidate for error handling! This allows you to make sure the user is attaching the correct information to the route's request body!
|
|
||
| if "title" not in request_body: | ||
| abort(make_response({"details": "Invalid data"}, 400)) | ||
|
|
There was a problem hiding this comment.
Same as before. Let's look at using a try/except block here for the error handling!
2 participants
No description provided.