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

&& Brittany Jones Ampers #26

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

Conversation

brittanyrjones
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 through the Recipe Search documentation and started experimenting with different queries and params in Postman. I also spent a lot of time on google.
Describe your API Wrapper. How did you decide on the methods you created? My API wrapper has 3 methods. There is a .search_recipes method that takes in a search term and returns results and there is a .find_recipe method that uses the uri of a specific recipe to get detailed information about that recipe. And then I have a method for the search that creates Recipe objects out of JSON.
Describe an edge case or failure case test you wrote for your API Wrapper. I tested what happens if a fake app_key or app_id is passed, and I also tested what happens if someone gives a fake uri to find a recipe.
Explain how VCR aids in testing an API. Having recorded API calls has 2 big benefits. It allows you to run your tests offline or even if the API is down, and it reduces the number of calls to the API.
What is the Heroku URL of your deployed application? https://bj-api-muncher.herokuapp.com/
Provide a link to the Trello board you used https://trello.com/b/gp3aS7GP/welcome-board

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits, good commit messages
Comprehension questions Check
General
Rails fundamentals (RESTful routing, use of named paths) The index action shouldn't be on the /recipes/index path, instead it should be on /recipes, and the show action should be on the path: /recipes/:id.
Semantic HTML Your HTML is broken. You're missing an open body element.
Errors are reported to the user Searches with no results have some feedback, erroneous URIs for the show action just crash the page.
API Wrapper to handle the API requests Check
Controller testing Check, missing tests for searches
Lib testing Check
Search Functionality Check
List Functionality Check
Show individual item functionality (link to original recipe opens in new tab) Check
Styling
Responsive layout Nope
List View shows 10 items at a time/pagination MISSING
The app is styled to create an attractive user interface Minimal Styling
API Features
The App attributes Edaman Check
The VCR cassettes do not contain the API key **You have the wrong keys being filtered in test_helper.rb.
External Resources
Link to Trello Board Check, but I don't have acces
Link to deployed app on Heroku Check
Overall You hit most of the learning goals. You need to review your routes, and testing. I see you also didn't get finished with the pagination, although that's lower priority. I left notes in your code and let me know if you have questions.

gem 'foundation-rails', '6.4.1.2'
group :development do
gem 'better_errors'
gem 'pry-rails'

Choose a reason for hiding this comment

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

This gem should also be under :test


url = URL + "r=#{uri}&app_id=#{app_id}&app_key=#{app_key}"
result = HTTParty.get(url)
if result && result[0]

Choose a reason for hiding this comment

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

This should be something like:

if result && result[0] && result.code == 200

When the request fails, you're crashing

<%= stylesheet_link_tag "application" %>
<%= csrf_meta_tags %>
</head>
<h1><%= link_to "Snack Time", root_path, alt: "link to home page" %></h1>

Choose a reason for hiding this comment

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

You've put this code outside the body element.


describe RecipesController do
describe "index" do
it "can return list of matching recipes" do

Choose a reason for hiding this comment

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

No testing of searches

end
end

describe "self.find_recipe" do

Choose a reason for hiding this comment

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

No attempt to find recipes with bad URIs

end

it "will send bad_request if message sent to fake recipe page" do
VCR.use_cassette("create_action_bad") do

Choose a reason for hiding this comment

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

This test fails due to a bug in your wrapper, see my note there.

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