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

tor s - octos - api-muncher #41

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

Conversation

torshimizu
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 read the docs, tried Postman, tried things in atom, realized I didn't have what I needed, reread the docs then retried things in Postman.
Describe your API Wrapper. How did you decide on the methods you created? I had a method to get a list of recipes and to get one recipe. I decided on two methods because of the nature of Edamam. Edamam takes a different parameter for a single recipe search, so I needed a different method to account for that.
Describe an edge case or failure case test you wrote for your API Wrapper. If Edemam sent back a 404, I had to rescue the error. I tried my best to test for this, but it was difficult to know how to get Edamam to send an error. I also added a test for Edemam not sending back any entries.
Explain how VCR aids in testing an API. VCR lets you save queries for indexing so that you do not use up limited requests when testing.
What is the Heroku URL of your deployed application? https://octo-muncher.herokuapp.com/
Provide a link to the Trello board you used https://trello.com/b/gAcjE6d6/muncher

…rapper - hoping this will resolve heroku issue also
@torshimizu
Copy link
Author

If it's not too much to ask, could mine please, please be looked at later? I really didn't get to finish/polish up things and I wasn't the most comfortable submitting what I had.

Please and thank you!


post favorites_path, params: recipe_data

must_respond_with :success

Choose a reason for hiding this comment

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

This should be a redirect as it redirects back.

<%= csrf_meta_tags %>
</head>

<body class='grid-y'>

Choose a reason for hiding this comment

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

Foundation xy-grid? neat

end
# =======================

it 'must return ok for no recipes' do

Choose a reason for hiding this comment

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

Maybe also test the flash notices?

end


it 'redirects to root when an error is received' do

Choose a reason for hiding this comment

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

This can be removed

auth_hash = request.env['omniauth.auth']

if auth_hash['uid']
user = User.build_from_google(auth_hash)

Choose a reason for hiding this comment

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

🥇

@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, I'm happy to look at this again if you get additional features done, but you already have the requirements.
General
Rails fundamentals (RESTful routing, use of named paths) Your show action isn't RESTful, to follow the pattern it should be /recipes/:id,
Semantic HTML Check
Errors are reported to the user Check
API Wrapper to handle the API requests Check
Controller testing Check
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 Check, both in search results and show page
List View shows 10 items at a time/pagination Check
The app is styled to create an attractive user interface Nicely done
API Features
The App attributes Edaman Check
The VCR casettes do not contain the API key
External Resources Check
Link to Trello Board Check, although I don't have access
Link to deployed app on Heroku Nicely done
Overall Nice work on the Google OAuth. If you get the chance show it to Zheng in the Ampers as she's working on the same and did Amazon OAuth. You hit all the learning goals and some nice extras with favorites and Google OAuth. Excellent job!

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