Update books.py #32
Update books.py #32
Conversation
![review-copilot-bot[bot]](https://avatars.githubusercontent.com/u/137213780?s=60&v=4){kind=small}
| @@ -49,11 +49,11 @@ def add_new_book(): | |||
|
|
|||
|
|
|||
| def get_by_title(book_title): | |||
| resp = token_validator(request.headers.get('Authorization')) | |||
| resp = token_validator() | |||
There was a problem hiding this comment.
The token validation response handling should include tests to verify proper error responses for expired and invalid tokens
| @@ -65,7 +65,7 @@ def get_by_title(book_title): | |||
| } | |||
| return Response(json.dumps(responseObject), 200, mimetype="application/json") | |||
| else: | |||
| return Response(error_message_helper("Book not found!"), 404, mimetype="application/json") | |||
| return Response(error_message_helper(), 404, mimetype="application/json") | |||
There was a problem hiding this comment.
Add test coverage for the 404 error case when book is not found
| return Response(error_message_helper(), 404, mimetype="application/json") |
There was a problem hiding this comment.
The error handling for non-existent books is consistent, but should be tested for both vulnerable and non-vulnerable paths
| @@ -49,11 +49,11 @@ def add_new_book(): | |||
|
|
|||
|
|
|||
| def get_by_title(book_title): | |||
| resp = token_validator(request.headers.get('Authorization')) | |||
| resp = token_validator() | |||
There was a problem hiding this comment.
token_validator() is called without any arguments but based on the error handling it seems to expect an authorization token parameter
| if "expired" in resp: | ||
| return Response(error_message_helper(resp), 401, mimetype="application/json") | ||
| return Response(error_message_helper(), 401, mimetype="application/json") |
There was a problem hiding this comment.
error_message_helper() is called without a message parameter which deviates from the helper function's intended usage pattern
| elif "Invalid token" in resp: | ||
| return Response(error_message_helper(resp), 401, mimetype="application/json") | ||
| return Response(error_message_helper(), 401, mimetype="application/json") |
There was a problem hiding this comment.
The error message for invalid token case should be passed to error_message_helper() to provide more specific feedback
| @@ -65,7 +65,7 @@ def get_by_title(book_title): | |||
| } | |||
| return Response(json.dumps(responseObject), 200, mimetype="application/json") | |||
| else: | |||
| return Response(error_message_helper("Book not found!"), 404, mimetype="application/json") | |||
| return Response(error_message_helper(), 404, mimetype="application/json") | |||
There was a problem hiding this comment.
Missing message parameter in error_message_helper() call for 404 response
| return Response(error_message_helper(), 404, mimetype="application/json") |
There was a problem hiding this comment.
The 404 error message should include details about which book was not found
No description provided.