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

Laura #30

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

Laura #30

wants to merge 30 commits into from

Conversation

lauracodecreations
Copy link

@lauracodecreations lauracodecreations commented Nov 5, 2018

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, and how did you try querying the API? I used the Edamam documentation and tested the key and values in Postman
What is an API Wrapper? Why is it in lib? How would your project change if you needed to interact with more than one API (aka more than just the Edamam API)? An API wrapper makes API request and transforms the incoming data into a Ruby hash data. It is in 'lib' folder because it is data for our application. If I need to interact with more than API, I will make a new Wrapper for it.
Describe your API wrapper, the methods you created in it, and why you decided to create those methods/how they were helpful I have three methods in my API wrapper: list_recipes, find_recipes, and format_uri. I decided to create these methods because they list recipes and find recipes need to be two different API calls. The format uri is useful because it formats the request to search for a recipe in the API
What was an edge case or failure case test you wrote for your API Wrapper? What was a nominal case? not yet
How does VCR aid in testing an API? not yet
What is the Heroku URL of your deployed application? have a H10 error https://cheflaura.herokuapp.com/

@dHelmgren
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good
Comprehension questions Good
General
Rails fundamentals (RESTful routing, use of named paths) See comments
Semantic HTML could be better, see comments
Errors are reported to the user No, see comments
API Wrapper to handle the API requests yes
Controller testing Needs more, see comments
Lib testing Missing
Search Functionality yes
List Functionality yes
Show individual item functionality yes
Styling
List view shows 10 items at a time and/or has pagination 10 at a time.
Attractive and usable UI A little too bear bones. No way to go back to the home page.
API Features
The App attributes Edamam not on the index page.
The VCR cassettes do not contain the API key No cassettes.
External Resources
Link to deployed app on Heroku Broken.
Overall Unfortunately, I don't think you've met the learning goals for this assignment. I would have liked to see your tests, and see you using VCR too before I saw styling. What you have here is a really good start, and I'd like to make sure that you have the support you need to see where you can improve in the future.

Rails.application.routes.draw do
root 'recipes#home'
get 'index', to: 'recipes#index', as: 'recipes_index'
get ':uri/show', to: 'recipes#show', as: 'recipes_show'

Choose a reason for hiding this comment

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

Not sure why :uri is before show here. Could you use resources for the index and show pages?

return CGI.escape(r)
end

def self.find_recipe(uri)

Choose a reason for hiding this comment

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

nice work finding a way to keep this method DRY

test "should get index" do
get recepies_index_url
assert_response :success
end

Choose a reason for hiding this comment

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

This test doesn't cover all your cases.

  • What happens if no search term is provided?
  • What if the search returns no results?

It might also be worthwhile to add some tests around the paging parameters:

  • What happens if they're absent?
  • Do you get different results when they change?
  • What if you send a bogus value, like a negative page number?

<%= javascript_include_tag 'application', 'data-turbolinks-track': 'reload' %>
</head>

<body>

Choose a reason for hiding this comment

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

You should probably have a header that can take the user back to the main page.

<div class="card" style="width: 18rem;">
<img class="card-img-top" src="<%=recipe.image_url%>" alt="Card image cap">
<div class="card-body">
<h5 class="card-title"<%=recipe.label%></h5>

Choose a reason for hiding this comment

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

Can't see this text on my screen.

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