Conversation
…ws-index and views-show
… and page and after editing takes you to the show page. Tests for Wave 3 passed.
…thod and passed those tests for Wave 4
…index view and show view, update routes
beccaelenzil
left a comment
There was a problem hiding this comment.
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 | ✔️ Consider using resources |
Uses named routes (like _path) |
✔️ |
| Creates Models and migrations | ✔️ |
| Creates styled views | This was not required, but something to consider for next time! |
| 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 | Add functionality to remark incomplete |
| Tests for Destroy & Task Complete include tests for valid and invalid task ids | ✔️ |
Overall Feedback
Good work on task list! You successfully followed rails conventions to create your first app in rails. I've left a few inline comments for your to review. On your next project I encourage you to make use of additional tools and features including strong params, resources, and partial views. Keep up the hard work!
| 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 |
Code Style Bonus Awards
Was the code particularly impressive in code style for any of these reasons (or more...?)
| Quality | Yes? |
|---|---|
| Perfect Indentation | ✅ |
| Descriptive/Readable | ✅ |
| Logical/Organized | ✅ |
| @@ -0,0 +1,16 @@ | |||
| <%# in app/views/edit.html.erb %> | |||
There was a problem hiding this comment.
Not that this code is largely repeated for the new view. Consider using a partial view.
| end | ||
| end | ||
|
|
||
| def toggle_complete |
There was a problem hiding this comment.
Consider how making two separate methods, for instance mark_complete and mark_incomplete would make this action idempotent.
| else | ||
| @task.update(completed_at: Time.now) | ||
| redirect_to tasks_path | ||
| return |
There was a problem hiding this comment.
You should also include capability to unmark a task complete.
| head :not_found | ||
| return | ||
| else | ||
| @task.update(completed_at: Time.now) |
There was a problem hiding this comment.
| @task.update(completed_at: Time.now) | |
| if @task.complete_at.nil? | |
| @task.update(completed_at: Time.now) | |
| else | |
| @task.update(completed_at: nil) | |
| end |
This will require some conditionals in your views additionally.
| <%= f.label :completed_at %> | ||
| <%= f.text_field :completed_at %> |
There was a problem hiding this comment.
Consider whether this value should be input in the form or set as a default to nil.
| expect { | ||
| patch toggle_complete_path(task.id) | ||
|
|
||
| # Assert | ||
| }.must_change "Task.count" |
There was a problem hiding this comment.
I am unsure why this test passes. Take a look. We should expect that marking complete does not change the number of tasks in the database.
Task List
Congratulations! You're submitting your assignment!
Comprehension Questions