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

Earth- Ana C14 #33

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

Earth- Ana C14 #33

wants to merge 16 commits into from

Conversation

anakp07
Copy link

@anakp07 anakp07 commented Nov 3, 2020

Task List

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe in your own words what the Model is doing in Rails It is our database
Describe in your own words what the Controller is doing in Rails It is how View and Model communicate with each other.
Describe in your own words what the View is doing in Rails It renders HTML from the controller.
Describe an edge-case controller test you wrote Expecting the path to redirect if an invalid ID is entered into 'update'.
What is the purpose of using strong params? (i.e. the params method in the controller) To dry up the code.
How are Rails migrations related to Rails models? Rail migrations change aspects of our models.
Describe one area of Rails that are still unclear on Testing, partial views and strong parameter usage.

Copy link
Contributor

@dHelmgren dHelmgren left a comment

Choose a reason for hiding this comment

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

Task List

Major Learning Goals/Code Review

Criteria yes/no, and optionally any details/lines of code to reference
At least 6 commits with meaningful commit messages ✔️
Routes follow RESTful conventions ✔️
Uses named routes (like _path) ✔️
Creates Models and migrations ✔️
Creates styled views ✔️
Handles errors like nonexistant tasks ✔️
Uses form_with to render forms in Rails ✔️

Functional Requirements/Manual Testing

Functional Requirement yes/no
Successfully handles index & show ✔️
index & show tests pass ✔️
Successfully handles: New, Create ✔️
New, Create tests pass ✔️
Successfully handles: Edit, Update ✔️
Edit, Update tests pass with valid & invalid task ids ✔️
Successfully handles: Destroy, Task Complete ✔️
Tests for Destroy & Task Complete include tests for valid and invalid task ids ✔️

Overall Feedback

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 5+ in Code Review && 6+ in Functional Requirements ✔️
Yellow (Approaches Standards) 3+ in Code Review && 5+ in Functional Requirements, or the instructor judges that this project needs special attention
Red (Not at Standard) 0-2 in Code Review or 0-4 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging, or the instructor judges that this project needs special attention

Something went funky with your submission, so some of my comments were based on a weird old version of the code you had submitted. IDK what exactly was going on, but in the future please QUADRUPLE CHECK that you are cloning and doing the PR agains C14 :D

@task.update(completed_at: nil)
redirect_to tasks_path
return
end
Copy link
Contributor

Choose a reason for hiding this comment

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

oops! keep an eye on your indentation!

Comment on lines +52 to +62
elsif @task.update(
name: params[:task][:name],
description: params[:task][:description],
completed_at: params[:task][:completed_at]
)
redirect_to root_path
return
else
render :edit
return
end
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like your indentation got messed up here

Suggested change
elsif @task.update(
name: params[:task][:name],
description: params[:task][:description],
completed_at: params[:task][:completed_at]
)
redirect_to root_path
return
else
render :edit
return
end
elsif @task.update(
name: params[:task][:name],
description: params[:task][:description],
completed_at: params[:task][:completed_at]
)
redirect_to root_path
return
else
render :edit
return
end

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