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.

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

Hayden - Edges (C10) - API Muncher #39

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

Conversation

haydenwalls
Copy link

@haydenwalls haydenwalls commented Nov 5, 2018

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? I poked around thru their dox and also played with it in postman
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)? an apiwrapper is kind of analogous to an activerecord model, but for accessing an external db / service. lib is where miscellaneous code files go. i did interact with more than one API and i just wrote another wrapper for it.
Describe your API wrapper, the methods you created in it, and why you decided to create those methods/how they were helpful i have a search method and a recipe show method. i created them because thats literally what the project asked for. they helped make my project functional. the methods use CONSTANTS and some user specified args to format the proper URLs in the methods.
What was an edge case or failure case test you wrote for your API Wrapper? What was a nominal case? I haven't finished any of the testing yet :/ I def want to test weird input like negative integers, strings for the page number, blank input, etc.
How does VCR aid in testing an API? records an API response so you can replay the data again and again instead of wasting expensive API calls
What is the Heroku URL of your deployed application? http://fooddd.herokuapp.com/ (be gentle with my baby!!)

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Not very many commits and your commit messages are mostly ok, sorry about that... isn't very descriptive.
Comprehension questions Check, No promises with your baby, you should teach her to fend for herself!
General
Rails fundamentals (RESTful routing, use of named paths) Somewhat, some of your routes like /favs and /recipe aren't really RESTful.
Semantic HTML Pretty good
Errors are reported to the user Check
API Wrapper to handle the API requests Check
Controller testing MISSING
Lib testing MISSING
Search Functionality Check
List Functionality Check
Show individual item functionality Check, but an invalid URI crashes the app.
Styling
List view shows 10 items at a time and/or has pagination Check
Attractive and usable UI Everything on the search results page is a little too big, like King Kong is a little too big.
API Features
The App attributes Edamam MISSING
The VCR cassettes do not contain the API key NO TESTING
External Resources
Link to deployed app on Heroku Check
Overall It looks like you tried to get Google OAuth working, but it's not working on Heroku. I do like the saved recent searches. However you should get the project requirements done first before going the extra mile. I strongly suggest reviewing testing an API, both in the controller and testing the lib folder. You hit the learning goals except for testing.

ENV["RAILS_ENV"] = "test"
require File.expand_path("../../config/environment", __FILE__)
require "rails/test_help"
require "minitest/rails"require "minitest/reporters" # for Colorized output

Choose a reason for hiding this comment

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

You have a typo here, put a newline between rails" and require

<%= javascript_include_tag 'application', 'data-turbolinks-track': 'reload' %>

<style>
body {

Choose a reason for hiding this comment

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

This should go in your stylesheet.


<body>
<p class="logo"><%= link_to "Food.", root_path %></p>
<ul class="login">

Choose a reason for hiding this comment

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

I would put this stuff in a header or other container tag.

data = HTTParty.get(url)
end

if data.include?("Error")

Choose a reason for hiding this comment

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

Good that you're checking things here.


private

def self.recipe_squeezer(hits)

Choose a reason for hiding this comment

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

This method name isn't really very descriptive of what it does.

encoded_url = URI.encode(url)
data = HTTParty.get(encoded_url)

if data.include?("Error")

Choose a reason for hiding this comment

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

if data.body is [], this is going to crash.

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.

2 participants