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

Jackie - Ampers - Api muncher #23

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

Conversation

Jackiesan
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 to understand the responses. I also tested different syntax options in the api documentation to compare the responses.
Describe your API Wrapper. How did you decide on the methods you created? I created 2 methods in the api wrapper. The first method is responsible for returning an array of hits (recipes). The second is responsible for returning one specific recipe which is used in the show method of the controller.
Describe an edge case or failure case test you wrote for your API Wrapper. I wrote a controller test that asserts that there is a redirection to root path if the recipe uri does not exist for the show method.
Explain how VCR aids in testing an API. VCR records the response of an api call the first time that the test is run. That information is stored in a cassette which can then be used to replay for future testing of that response. This is helpful because it means that the tests will not make new calls when they are run.
What is the Heroku URL of your deployed application? https://seattle-muncher.herokuapp.com/
Provide a link to the Trello board you used https://trello.com/b/VUu9CwwQ/recipe

@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) Check,
Semantic HTML See my notes, avoid using div elements so much
Errors are reported to the user No error messages for bad URIs on the show action or empty search results
API Wrapper to handle the API requests Check
Controller testing Check,
Lib testing Check, missing an edge-case on the find-recipe action.
Search Functionality Check
List Functionality Check
Show individual item functionality (link to original recipe opens in new tab) Check
Styling
Responsive layout On the index results yes, on the show page, no.
List View shows 10 items at a time/pagination Check, nice styling there
The app is styled to create an attractive user interface Well done
API Features
The App attributes Edaman Check
The VCR casettes do not contain the API key Check
External Resources
Link to Trello Board Check
Link to deployed app on Heroku Check
Overall Well done, you hit all the learning goal. I did leave some notes in your code on small matters. Let me know if you have any questions.

<h1 class="muncher"><%= link_to "Seattle Muncher", root_path%></h1>

<%= yield %>
<div class="footer">

Choose a reason for hiding this comment

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

wouldn't a footer tag work better?


<div class="container">
<%= form_tag recipes_path, method: :get do %>
<div class="row">

Choose a reason for hiding this comment

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

toooooo maaaaany div elements

must_respond_with :success
end

it "can get recipes index" do

Choose a reason for hiding this comment

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

No test for empty search results?

end
end

it "returns the accurate information of a recipe" do

Choose a reason for hiding this comment

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

What about bad URIs on the find recipe action?

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