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 - Samantha #30

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

Leaves - Samantha #30

wants to merge 16 commits into from

Conversation

iamsammysam
Copy link

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 the part that "talks" to the database, it knows what is on the database and communicates back with the controller.
Describe in your own words what the Controller is doing in Rails It receives events from the outside world through the router and passes data to the "view".
Describe in your own words what the View is doing in Rails It turns data into HTML and communicates with the client (similar to a CLI).
Describe an edge-case controller test you wrote I wrote tests to redirect the user for trying to update or find an invalid task or id for example.
What is the purpose of using strong params? (i.e. the params method in the controller) It is a security feature that ensures users can't update sensitive attributes without permission.
How are Rails migrations related to Rails models? By adding a model and a correspondent migration to it, that migration provides instructions to the database on how to store the data the model is describing.
Describe one area of Rails that are still unclear on I've tried to use RESTful routing and Strong Params to make my code "DRY" and more efficient but I couldn't apply those concepts without breaking the code.

end

# Wave 4 - mark task as complete
def complete

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. Currently when you click the complete button on a completed event, it marks the task as incomplete.


<ul>
<% @tasks.each do |task| %>
<li><h3><%= link_to task.name, task_path(task) %></h3>

Choose a reason for hiding this comment

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

Consider adding some styling (such as a strikethrough) for a completed event.

post complete_task_path(existing_task.id)
}.wont_change "Task.count"

expect(Task.find_by(id: existing_task.id).name).must_equal "completed task"

Choose a reason for hiding this comment

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

Consider edge cases with an invalid id.

@beccaelenzil
Copy link

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 check
Successfully handles: Edit, Update check
Tests for Edit & Update test valid & invalid task ids check
Successfully handles: Destroy, Task Complete complete button toggles the completion status
Tests for Destroy & Task Complete include tests for valid and invalid task ids check -- see comment
Routes follow RESTful conventions check
Uses named routes (like _path) check
Overall Good job on Task List overall. You hit all of the learning goals of following Rails conventions. It would've been nice to see practice using strong params and view partials. There is one bug where the completed button toggles the completion status. This is a bit surprising as the user.

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