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 - Maddie Shields #37

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

Ampers - Maddie Shields #37

wants to merge 14 commits into from

Conversation

madaleines
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 used Postman with JSON to see the format to access the data
Describe your API Wrapper. How did you decide on the methods you created? I searched for the query and showed an individual recipe
Describe an edge case or failure case test you wrote for your API Wrapper. If the word doesn't exist was one. If it does exist. If the query matches the search phrase
Explain how VCR aids in testing an API. To help limit the amount of times I use my access tokens and also show the recorded versions that work.
What is the Heroku URL of your deployed application? https://mvs-muncher.herokuapp.com/
Provide a link to the Trello board you used https://trello.com/b/v5OKz7Pd/recipe-api-consumer

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Reasonable number of commits and good commit messages
Comprehension questions Check
General
Rails fundamentals (RESTful routing, use of named paths) Check
Semantic HTML Check, although the HTML is minimal.
Errors are reported to the user If the URI is wrong the show action crashes. There are no messages when there are no search results.
API Wrapper to handle the API requests Check
Controller testing MISSING
Lib testing Missing a negative case for the show action. You also are missing the configuration for VCR in test_helper.rb
Search Functionality Check
List Functionality Check
Show individual item functionality (link to original recipe opens in new tab) Check, although the link to the original recipe is missing
Styling
Responsive layout Nope
List View shows 10 items at a time/pagination Check
The app is styled to create an attractive user interface Minimal interface
API Features
The App attributes Edaman MISSING
The VCR cassettes do not contain the API key Without the configuration for VCR in test_helper.rb you aren't saving cassettes!
External Resources
Link to Trello Board Check, but I don't have access
Link to deployed app on Heroku Check
Overall You got it working, as noted you still need to work on testing and making sure VCR is configured.

<ul>
<% @recipes.each do |recipe| %>
<li>
<%= image_tag(recipe.image) %>

Choose a reason for hiding this comment

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

Making the image a clickable link would improve the user experience.

@@ -0,0 +1,18 @@
<h1> Results for "<%= @query %>"</h1>
<div class="">

Choose a reason for hiding this comment

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

What purpose does this div element serve?

end

describe "show_recipe" do
it "can show a single recipe" do

Choose a reason for hiding this comment

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

What about the negative case here? What about if the URI is wrong?


class ActiveSupport::TestCase
# Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.
fixtures :all

Choose a reason for hiding this comment

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

VCR configuration?

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