Skip to content

Conversation

@thenora
Copy link

@thenora thenora commented May 5, 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 The model holds the database to communicate with the controller.
Describe in your own words what the Controller is doing in Rails the controller contains the "business logic" about how the program works.
Describe in your own words what the View is doing in Rails The view tells us how to display the information to the browser.
Describe an edge-case controller test you wrote an edge case for the update function was "will redirect to the root page if given an invalid id"
What is the purpose of using strong params? (i.e. the params method in the controller) strong params let us setup once for our controller, then we don't need to specify each field individually. It saves time while keeping things secure.
How are Rails migrations related to Rails models? I don't understand the question. Is this about databases?
Describe one area of Rails that are still unclear on See previous question.

@beccaelenzil
Copy link

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 No, but not required
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 ✔️
Successfully handles: Destroy, Task Complete ✔️

Overall Feedback

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 5+ in Code Review && 5+ in Functional Requirements ✔️

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Descriptive/Readable
Concise
Logical/Organized

Copy link

@beccaelenzil beccaelenzil left a comment

Choose a reason for hiding this comment

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

Great work on this project! It's clear that the learning goals for your first rails project were met. Keep up the hard work.

patch '/tasks/:id', to: 'tasks#update'
delete '/tasks/:id', to: 'tasks#destroy'

patch '/tasks/:id/mark_done', to: 'tasks#mark_done', as: 'mark_done'

Choose a reason for hiding this comment

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

Consider how making two separate methods, for instance mark_complete and mark_incomplete would make this action idempotent.

id = Task.last.id
expect {
delete task_path(id)
}.must_change "Task.count"

Choose a reason for hiding this comment

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

Consider an even more precise test.

Suggested change
}.must_change "Task.count"
expect {
delete task_path(id)
}.will_differ "Task.count", -1

expect {
delete task_path(id)
}.must_change "Task.count"

Choose a reason for hiding this comment

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

Remember to test for a redirect


it "can update an existing task" do
# Your code here
Task.create(name: "Get Shit Done", description: "You gotta werk.")

Choose a reason for hiding this comment

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

Good work fleshing out this test.

@@ -0,0 +1,14 @@
<%= form_with model: @task, class: 'create-form' do |f| %>

Choose a reason for hiding this comment

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

Nice use of partial views.

@@ -0,0 +1,3 @@
<h2>Add a Task</h2>

Choose a reason for hiding this comment

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

Consider adding this to the partial by adding to the locals hash.

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