Skip to content

Conversation

@cdvx
Copy link
Owner

@cdvx cdvx commented Aug 29, 2018

Hello, please review my PR.
I implemented all the feedback you gave, just left with Documenting and writing the Proper tests.
thanks.

return Response(json.dumps(['Question not Found']),
status=404, mimetype='application/json')
return jsonify({f'Question {questionId}': 'Has not been added yet'}), 404
return jsonify({f'Question {questionId}': 'Has not been added yet'}), 200

Choose a reason for hiding this comment

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

This return message could be improved to "there are no questions or question doesn't exist" since its "if" statement is to query and check if there are any questions in the database.

'topic': question[1],
'body': question[2]
}
return jsonify(temp), 200

Choose a reason for hiding this comment

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

the returned json should come with the all answers provided so far for the question according to the notes in the doc.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks, I implemented that in a new connection branch, about to send a PR

request_data = request.get_json()
if (valid_question(request_data)):
print(request_data)

Choose a reason for hiding this comment

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

Remove print statements


def valid_question(questionObject):
if 'topic' in questionObject and 'body' in questionObject:
if 'topic' in questionObject.keys() and 'body' in questionObject.keys():

Choose a reason for hiding this comment

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

You can shift these validator functions to a different file and import them to use them in the views so that the routes.py has routes only

@cdvx cdvx merged commit 9fa2564 into Challenge-3 Aug 30, 2018
@cdvx cdvx deleted the Update-Delete-Prefer branch August 31, 2018 17:55
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.

3 participants