Skip to content

Earth - Lina#48

Open
lina5147 wants to merge 23 commits intoAda-C14:masterfrom
lina5147:master
Open

Earth - Lina#48
lina5147 wants to merge 23 commits intoAda-C14:masterfrom
lina5147:master

Conversation

@lina5147
Copy link

@lina5147 lina5147 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 Model represents the data. It is were data is stored for the controller to retrieve from.
Describe in your own words what the Controller is doing in Rails The controller coordinates activity. It receives HTTP requests, communicates with model to gather data, and passes data to the view to be rendered as HTML.
Describe in your own words what the View is doing in Rails View is helps display Model data. It turns the data into HTML.
Describe an edge-case controller test you wrote An edge case I wrote would be for the mark complete method, where I tested for the situation that if the chosen id was not valid, it should then redirect to the index page.
What is the purpose of using strong params? (i.e. the params method in the controller) Using strong params will provide help reduce error when passing data from params directly into our methods formulated from our forms.
How are Rails migrations related to Rails models? Rails migrations help makes changes to the database schema in models.
Describe one area of Rails that are still unclear on I am still struggling to formulate tests for the controller actions.

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.

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 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 ✔️
Edit, Update tests pass with valid & invalid task ids ✔️
Successfully handles: Destroy, Task Complete These methods are written well and tested well, but the functionality is never given to the user
Tests for Destroy & Task Complete include tests for valid and invalid task ids ✔️

Overall Feedback

Good work on this project. You've created your first web app in rails, nice work! It is clear that the learning goals around using rails conventions, strong params, and custom routes were met. On upcoming project I enourage you to explore styling and use partial views. I've left a few in line comments for you to review. 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
Concise
Logical/Organized

Comment on lines +13 to +14
patch '/tasks/:id/complete', to: 'tasks#mark_complete', as: 'complete_task'
patch '/tasks/:id/incomplete', to: 'tasks#unmark_complete', as: 'incomplete_task'

Choose a reason for hiding this comment

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

Great use of two separate methods to make this action idempotent.

# For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html

root 'tasks#index'
get '/tasks', to: 'tasks#index', as: 'tasks'

Choose a reason for hiding this comment

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

Consider using resources

@@ -0,0 +1,22 @@
<h2>EDIT A TASK</h2>

<%= form_with model: @task, class: 'create-task' do |f| %>

Choose a reason for hiding this comment

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

Consider using a partial view for edit and new views.

Comment on lines +16 to +19
<div>
<%= f.label :completed_at %>:
<%= f.text_field :completed_at %>
</div>

Choose a reason for hiding this comment

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

Consider leaving this off from your form as it would make sense that new task has set nil for completed_at, and you have a custom method to change this (mark_complete and mark_uncomplete).

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