Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Karis - Edges - API-muncher #44

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Karis - Edges - API-muncher #44

wants to merge 26 commits into from

Conversation

kimj42
Copy link

@kimj42 kimj42 commented Nov 6, 2018 •
edited
Loading

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, and how did you try querying the API? Explored the documentation and played around with postman. Also tried the queries in pry to look at the JSON.
What is an API Wrapper? Why is it in lib? How would your project change if you needed to interact with more than one API (aka more than just the Edamam API)? API wrapper holds the keys and sends the requests to the actual API. It will also send back the response back to the controller. If there was more than one API I guess I would have more than one wrapper and several Keys.
Describe your API wrapper, the methods you created in it, and why you decided to create those methods/how they were helpful The API wrapper returns the all 50 instances of the Recipe class if the search word is valid. It also sends a new request to return the specific instance of the Recipe for the show page. I created those methods to get back the appropriate JSON for the controller. One for index and one for show.
What was an edge case or failure case test you wrote for your API Wrapper? What was a nominal case? An edge case test I wrote is for when the search word is invalid like it's empty or the search word is "asdf". A nominal case was when the word is "cookie".
How does VCR aid in testing an API? VCR prevents sending more API calls.
What is the Heroku URL of your deployed application? https://karis-api-muncher.herokuapp.com/

@kimj42
created new rails app
2e9d5c2
@kimj42
created index new show method for controller
031fed9
@kimj42
added a show method and added route
f9b8959
@kimj42
fixed routes issue and added form to new view
64efb2d
@kimj42
list of all recipe names for the search word shows up in index page
b853a62
@kimj42
show page for single recipe works now
3fc276f
@kimj42
added styling to root pg
fae564b
@kimj42
basic styling to root pg but needs to be responsive
ebb128e
@kimj42
tried to work on pagination, added gem, but need to work on this on a…
bf577f6 
… branch
@kimj42
pagination works for 5 items per list but it seems most have 10 items…
b04ef5d 
… so it is not showing more pages.
@kimj42
made link to external website work in show page
2493949
@kimj42
external link pops up as new tab
103a130
@kimj42
made 50 items show up in index page with pagenation
944958c
@kimj42
used bootstrap to style list of food images, not perfect but works
96db008
@kimj42
images lined up well but image size not all the same, could not get s…
d9e4604 
…earch bar to work for index and show page
@kimj42
made site work without new and create method but search bar still not…
18a6253 
… working for index and show page
@kimj42
made errors for entering invalid recipe name show up in root page
9e26141
@kimj42
shows error msg for user that tries to manually write one recipe id
cb67bef
@kimj42
added testing for recipe.rb
8d8ec8b
@kimj42
used cassette and hid token and id for api wrapper testing
982a6fe
@kimj42
added more tests
582748e
@kimj42
fixed form not working in index and show page. completed all testing
c8a1810
@kimj42
added testing for recipes controller
cf6a5c9
@kimj42
trying to troubleshoot heroku deployment issue with concurrent ruby
e0e9c8f
@kimj42
bundle updated
fed11eb
@kimj42
still trying to troubleshoot
fc1f1d4
@droberts-sea
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
Rails fundamentals (RESTful routing, use of named paths) see inline
Semantic HTML yes
Errors are reported to the user yes - great work!
API Wrapper to handle the API requests yes
Controller testing yes
Lib testing yes - see inline
Search Functionality yes
List Functionality yes
Show individual item functionality yes
Styling
List view shows 10 items at a time and/or has pagination yes
Attractive and usable UI good start
API Features
The App attributes Edamam This is really important! When you're designing code for money, using someone else's work without attribution is a good way to end up in legal trouble.
The VCR cassettes do not contain the API key yes
External Resources
Link to deployed app on Heroku yes
Overall Great work overall! The core functionality of this site works well, the code is well tested, and I'm especially happy with your approach to error handling. It is clear to me that the learning goals for this assignment were met. Keep up the hard work!

droberts-sea
droberts-sea reviewed Nov 12, 2018
View reviewed changes
lib/edamam_api_wrapper.rb
private

def self.create_recipe(recipe)
unless recipe.nil?
Copy link

Choose a reason for hiding this comment

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

I like that you've broken this functionality out as separate methods and made them private. Good organization!

lib/edamam_api_wrapper.rb
data = HTTParty.get(url)
# binding.pry
h = data[0]
return create_recipe(h)
Copy link

Choose a reason for hiding this comment

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

Why h?

config/routes.rb

get 'recipes', to: 'recipes#index', as: 'recipes_index'

get 'recipe/:recipe', to: 'recipes#show', as: 'recipe_show'
Copy link

Choose a reason for hiding this comment

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

Could you use resources for the index and show pages?

test/lib/edamam_api_wrapper_test.rb
describe "create recipe " do
it "does not create an instance of Recipe if recipe is nil" do
VCR.use_cassette("recipes") do
recipe = nil
Copy link

Choose a reason for hiding this comment

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

Since this is a private method, you don't have to test it! See POODR ch 4.

test/lib/edamam_api_wrapper_test.rb
describe "show recipe" do
it "shows one instance of the recipe for valid recipe id" do
VCR.use_cassette("recipes") do
all_recipes = EdamamApiWrapper.search("cookie")
Copy link

Choose a reason for hiding this comment

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

What if the recipe ID is invalid?

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

@droberts-sea droberts-sea droberts-sea 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
@kimj42 @droberts-sea