-
Notifications
You must be signed in to change notification settings - Fork 43
TaskList-Pipes-Mariana #29
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
base: master
Are you sure you want to change the base?
Conversation
Task ListWhat We're Looking For
|
| get '/tasks/:id/edit', to: 'tasks#edit', as: 'edit_task' | ||
| patch '/tasks/:id', to: 'tasks#update' | ||
| delete '/tasks/:id', to: 'tasks#destroy' | ||
| post '/tasks/:id/checkout', to: 'tasks#checkout', as: 'checkout_task' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we don't have a checkout method in TasksController so I think this route should probably be removed.
| def create #Has access to user data, from the form | ||
| @task = Task.create(title: params[:task][:title], description: params[:task][:description], duedate: params[:task][:duedate], complete: false) | ||
| # task.save | ||
| redirect_to('/tasks') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally here we would use redirect_to tasks_path instead. It should work the same way as this line does, but is more flexible if we for some reason need to change the URL that maps to the tasks#index action.
| <%= link_to "Añadir Tarea", new_task_path %> | ||
| </il> | ||
| <il> | ||
| <%= link_to "Todas las tareas", tasks_path %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section of links to "Añadir Tarea" and "Todas las tareas" is repeated at the top of every view. We can DRY up this code by moving it into the app/views/layouts/application.html.erb layout template.
In this case we would put it above the line with <%= yield %> so that it appears above all of the content for a given page.
| <% @tasks.each do |task| %> | ||
| <li> | ||
| <h3> | ||
| <% if task.complete %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if statement can probably be DRY'd up somewhat.
The link_to that we're creating in both cases is identical except for the ID that it will have. Since that is the case we can probably use a ternary statement to figure out the correct ID and then write the link_to code once:
<%= link_to task.title, task_path(task.id), id: task.complete ? "Complete" : "NoComplete" %>
Task List
Congratulations! You're submitting your assignment!
Comprehension Questions