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

Leaves - Mariya #41

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

Leaves - Mariya #41

wants to merge 9 commits into from

Conversation

M-Burr
Copy link

@M-Burr M-Burr commented Oct 7, 2019

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 deals with the business logic of the program. It also reads & modifies the database & allows us to do so using Ruby code.
Describe in your own words what the Controller is doing in Rails The controller manages interactions between the model and the viewer.
Describe in your own words what the View is doing in Rails The View provides the visuals that the user can actually see and interact with.
Describe an edge-case controller test you wrote I wrote an edge case for the controller actions having invalid ids.
What is the purpose of using strong params? (i.e. the params method in the controller) Strong params add enhanced security & prevent outside users from modifying your forms (i.e., the keys in the param hash). Strong params should be stored in private methods.
How are Rails migrations related to Rails models? Migrations allow you to create & modify
Describe one area of Rails that are still unclear on I have some confusion about routes.

@beccaelenzil
Copy link

beccaelenzil commented Oct 10, 2019

Task List

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage with no extraneous files checked in check
Answered comprehension questions check
Successfully handles: Index, Show check
Index & Show tests pass check
Successfully handles: New, Create check
New & Create tests pass I wasn't able to run tests, but they look good.
Successfully handles: Edit, Update check
Tests for Edit & Update test valid & invalid task ids check
Successfully handles: Destroy, Task Complete The ways to mark a task complete is a little funny. What does the check mark do? When you mark a task complete, you need to refresh in order for the view to change to indicate the task is complete with a line through it.
Tests for Destroy & Task Complete include tests for valid and invalid task ids check
Routes follow RESTful conventions check
Uses named routes (like _path) check
Overall Good job overall. You hit the learning goals of using partials, using strong params, and following Rails conventions. There's one issue -- the completed action doesn't work quite as expected -- you need to refresh to see the strikethrough and the function of the checkbox is not clear.

end
end

def completed

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.

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