Emily's API Muncher #87
Conversation
Baseline requirements completed
working with basic functionality
Retakestyling
tests written
There was a problem hiding this comment.
@eabrash Hello Emily, very nice job overall with your API-Muncher project. It's a beautiful site with all the required functionality. You also did a good job with testing.
I've included some comments in the pull request here, and a summary in your Progress report at: https://docs.google.com/document/d/1SL6czRQKGmbms1JTES6Kjl6qNbk-YWhW9KfBDl9vFME/edit
| end | ||
|
|
||
| def show | ||
| @page = 'nonhomepage' |
There was a problem hiding this comment.
What about if the show page is missing some of those parameters? Like the from parameter is missing?
|
|
||
| get 'recipes/show/:id' => 'recipes#show', as: 'recipes_show' | ||
|
|
||
| # The priority is based upon order of creation: first created -> highest priority. |
There was a problem hiding this comment.
This needs to be cleaned up a bit, the paths are all over the place for one type of thing. For someone looking at your routes from the outside they would have trouble figuring out which route did which thing.
There was a problem hiding this comment.
Thanks for this great feedback Chris - you are totally right that the routes were messy and difficult to follow. I've put in a little more time cleaning them up, but would love to talk with you about best practices regarding the relationship between route names and controllers/controller methods. Maybe we can discuss this in our next 1:1 meeting :)
| if recipe_hash['label'] == nil || recipe_hash['uri'] == nil || recipe_hash['label'] == "" || recipe_hash['uri'] == "" || recipe_hash['ingredients'] == nil || recipe_hash['ingredients'].length == 0 || recipe_hash['source'] == nil || recipe_hash['source'] == "" | ||
| raise ArgumentError | ||
| end | ||
|
|
There was a problem hiding this comment.
Good check for missing required arguments.
| }]}) | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
You could probably do the ArgumentError testing with a loop like we did in class.
|
|
||
| config.filter_sensitive_data("<EDAMAM_CLIENT_SECRET>") do | ||
| ENV['EDAMAM_CLIENT_SECRET'] | ||
| end |
There was a problem hiding this comment.
Good work hiding your keys from github and the cassets!
@CheezItMan @PilgrimMemoirs