Skip to content

Jan Edrozo | Carets#32

Open
JNEdrozo wants to merge 38 commits intoAda-C8:masterfrom
JNEdrozo:master
Open

Jan Edrozo | Carets#32
JNEdrozo wants to merge 38 commits intoAda-C8:masterfrom
JNEdrozo:master

Conversation

@JNEdrozo
Copy link

@JNEdrozo JNEdrozo commented Nov 6, 2017

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, how did you try querying the API? I reviewed the Edamam documentation and tested request paths in Postman (occasionally using awesome print to confirm responses).
Describe your API Wrapper. How did you decide on the methods you created? I have 2 self methods (show_recipe and list_recipes) so I could retrieve a list of recipes (based on search term) as well as retrieve info for a specific recipe (using its uri).
Describe an edge case or failure case test you wrote for your API Wrapper. A fail case would be for a bad request (e.g. a search term that does not have any recipe results like a number), and I tested to make sure it returns an empty array
Explain how VCR aids in testing an API. VCR aids in testing because it reduces the number of API calls we make since we can reuse API responses stored in the cassettes.
What is the Heroku URL of your deployed application? https://recipoogle.herokuapp.com/

JNEdrozo added 30 commits October 31, 2017 15:52
@JNEdrozo
Initial Set Up with edamam_api_wrapper.rb
2c12cf7
@JNEdrozo
edamam_api_wrapper.rb#list_recipes (need to fix the image and source …
63a7127 
…hash symbol syntax)
@JNEdrozo
Updated edamam_api_wrapper#list_recipes method and Recipe.rb; also ad…
f0e0781 
…ded the dotenv gem to read the app_id and key in the .env file
@JNEdrozo
Recipe_test.rb: Added initialize tests
185632b
@JNEdrozo
Added basic self#list_recipes for 2 basic tests (mirrored from SlackAPI)
8cf7b19
@JNEdrozo
Generated a Controller (recipes)
81ff255
@JNEdrozo
Edamam_api_wrapper_test.rb (added test fr wrong app id and key)
1ababf6
@JNEdrozo
Updated routes.rb with 3 get routes (index, search, show)
a617edf
@JNEdrozo
Added recipe_payload.json sample text and updated api_wrapper #list_r…
fc02d5a 
…ecipes to simplify options hash readability
@JNEdrozo
added will_paginate
25f94bd
@JNEdrozo
Added recipes show link in index page
ae6714f
@JNEdrozo
Added EdamamApiWrapper#show_recipe(uri%23conversion)
532e008
@JNEdrozo
Updated Recipe#show view with basic wireframe elements
d0ec531
@JNEdrozo
Added Home page with footer and Edamam attribution link
d0fb276
@JNEdrozo
Added render_404 to show (if recipe is nil)
0bcfae6
@JNEdrozo
removed config.enable_dependency_loading = true from production.rb
ae9ab74
@JNEdrozo
Raind rails db:migrate to create schema(to troubleshoot heroku)
6172421
@JNEdrozo
Added required='required' to search form's text_field_tag to ensure u…
c1c5203 
…ser enters a search term (does not exclude spaces)
@JNEdrozo
Fixed RecipesController#index method to prevent empty string searches…
6d90a54 
… (switched condition order)
@JNEdrozo
Updated RecipeController tests (still need to fix show test and id)
1975845
@JNEdrozo
Correction: RecipeController#show (removed render_404 condition)
1806a97
@JNEdrozo
Updated index styling so recipes are boxes rather than a list
8b89016
@JNEdrozo
Added Gale Tip on EdamamApiWrapper issue in application.rb and produc…
6bbcf61 
…tion.rb
@JNEdrozo
Removed production.rb typo in production.rb file
f1843b7
@JNEdrozo
Moved httparty gem outside of test and dev
b1f2f5b
@JNEdrozo
Application.rb(removed config.autoload_paths << Rails.root.join('lib'))
85a50fa
@JNEdrozo
Moved gem awesome_print out of development and test group and consoli…
8d13417 
…dated development group in gemfile
@JNEdrozo
Review basic tests for controller and lib
183e6c2
@JNEdrozo
Styling: Moved footer position to bottom of page
8308b13
@JNEdrozo
Added a favicon to application.html.erb
4d8a2bd
@CheezItMan
Copy link

CheezItMan commented Nov 6, 2017

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Very good commits!
Comprehension questions Check
General
Rails fundamentals (RESTful routing, use of named paths) Check
Semantic HTML Good semantic HTML, I think some of the div elements could be removed.
Errors are reported to the user You have flash notices, but did not put them in the views!
API Wrapper to handle the API requests Check
Controller testing Well done here
Lib testing No testing on the show recipe method.
Search Functionality Check
List Functionality Check
Show individual item functionality (link to original recipe opens in new tab) Check, but not in a new tab
Styling
Foundation Styling for responsive layout Check
List View shows 10 items at a time/pagination Check
The app is styled to create an attractive user interface Very nicely styled here.
API Features
The App attributes Edaman Check
The VCR casettes do not contain the API key Check
External Resources
Link to deployed app on Heroku Well done
Overall Very nicely done, you missed a couple of tests in your wrapper, but I think that was just an oversight. Well done!

CheezItMan
CheezItMan reviewed Nov 6, 2017
View reviewed changes
test/lib/edamam_api_wrapper_test.rb
end

describe "self#list_recipes" do
it "Can list a group of channels" do
Copy link

@CheezItMan CheezItMan Nov 6, 2017

Choose a reason for hiding this comment

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

Something's missing here!

test/lib/edamam_api_wrapper_test.rb
end
end
end
end
Copy link

@CheezItMan CheezItMan Nov 6, 2017

Choose a reason for hiding this comment

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

Tests for the show_recipe method?

test/controllers/recipes_controller_test.rb
@@ -0,0 +1,68 @@
require "test_helper"

describe RecipesController do
Copy link

@CheezItMan CheezItMan Nov 6, 2017

Choose a reason for hiding this comment

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

Good set of tests here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CheezItMan CheezItMan CheezItMan left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JNEdrozo @CheezItMan