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

Ampers - Sara S - api-muncher #28

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

Conversation

CheerOnMars
Copy link

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 looked over the documentation, I tried to show all the data it would return, even if it didn't wind up on my final product.
Describe your API Wrapper. How did you decide on the methods you created? I have three methods on my API method. One to search recipes, one to get a recipe, and one to show a recipe. Only the search and show methods require calling the API with HTTParty. I decided on these three because they seemed pretty discrete.
Describe an edge case or failure case test you wrote for your API Wrapper. One of my tests makes sure it will raise an error if there's an invalid recipe it.
Explain how VCR aids in testing an API. VCR allows you to not have to call the API for every test because it stores the retrieved information in a cassette.
What is the Heroku URL of your deployed application? adies-recipes.herokuapp.com
Provide a link to the Trello board you used https://trello.com/b/XssCTarz/apimuncher

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits, and good commit messages
Comprehension questions Check
General
Rails fundamentals (RESTful routing, use of named paths) Check
Semantic HTML Well done
Errors are reported to the user No report if there are no search results, no message if the URI for the show action is invalid.
API Wrapper to handle the API requests Check
Controller testing Check, but see my notes, you're not testing the search functionality. You are also missing testing the pagination.
Lib testing Check
Search Functionality Check
List Functionality Check
Show individual item functionality (link to original recipe opens in new tab) Check, but it doesn't open the original recipe in a new tab, minor quibble.
Styling
Responsive layout The search results are responsive as is the show page.
List View shows 10 items at a time/pagination Check
The app is styled to create an attractive user interface It looks nice, simple and effective
API Features
The App attributes Edaman Check
The VCR casettes do not contain the API key Check
External Resources
Link to Trello Board Check, but I can't access it
Link to deployed app on Heroku Check
Overall Nicely done, you hit most of the learning goals. You need to look more carefully at your controller testing, and do some error handling using flash notices to give the user feedback.


describe RecipesController do
describe 'index' do
it "should get index" do

Choose a reason for hiding this comment

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

There are no tests here for:

  1. Searching for a food that has results
  2. Searching for an item that has no results


<body>
<section>
<% if flash[:status] %>

Choose a reason for hiding this comment

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

You're not using these flash notices!

begin
@recipe = EdamamApiWrapper.show_recipe(params[:id])
rescue ArgumentError => error
redirect_to root_path

Choose a reason for hiding this comment

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

Having a flash notice here would be appropriate.

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