Adapt routes and views - #21
Conversation
| # GET /comments.json | ||
| def index | ||
| @comments = Comment.all | ||
| @comments = Comment.all.sort_by { |c| c.dependency.name } |
There was a problem hiding this comment.
We should actually sort this by created_at, imho 🤔
There was a problem hiding this comment.
And whatever we decide for the sorting criteria, it belongs to the model, not
the controller.
Also, wouldn't #sort_by do the sort in Ruby? If so, we should let PostgreSQL
do the work for us IMO (Comment.order name: :asc, Comment.order created_at: :asc…)
| <div class="field"> | ||
| <%= form.label :dependency_id %> | ||
| <%= form.text_field :dependency_id %> | ||
| <%= form.select :dependency_id, Dependency.all.collect {|d| [ d.name, d.id ] } %> |
There was a problem hiding this comment.
I believe there's a Rails helper for that.
There was a problem hiding this comment.
- Line is too long;
- I'd favor
#mapover#collect(but as said there might be an helper to
remove/simplify this part); - Does our coding style allow this syntax for the block? Shouldn't this be:
{ |d| [d.name, d.id] }
| puts "#{Dependency.count} created" | ||
| puts "#{Comment.count} created" | ||
|
|
||
| puts "Happy Growing !" No newline at end of file |
There was a problem hiding this comment.
This file shouldn't be here.
| # GET /comments.json | ||
| def index | ||
| @comments = Comment.all | ||
| @comments = Comment.all.sort_by { |c| c.dependency.name } |
There was a problem hiding this comment.
And whatever we decide for the sorting criteria, it belongs to the model, not
the controller.
Also, wouldn't #sort_by do the sort in Ruby? If so, we should let PostgreSQL
do the work for us IMO (Comment.order name: :asc, Comment.order created_at: :asc…)
| <div class="field"> | ||
| <%= form.label :dependency_id %> | ||
| <%= form.text_field :dependency_id %> | ||
| <%= form.select :dependency_id, Dependency.all.collect {|d| [ d.name, d.id ] } %> |
There was a problem hiding this comment.
- Line is too long;
- I'd favor
#mapover#collect(but as said there might be an helper to
remove/simplify this part); - Does our coding style allow this syntax for the block? Shouldn't this be:
{ |d| [d.name, d.id] }
| <% @dependencies.each do |dependency| %> | ||
| <tr> | ||
| <td><%= dependency.name %></td> | ||
| <td><%= "#{dependency.comments.count} comments" %></td> |
There was a problem hiding this comment.
<%= "#{dependency.comments.count} comments" %>should be:
<%= dependency.comments.count %> comments| <br> | ||
|
|
||
| <%= link_to 'New Dependency', new_dependency_path %> | ||
| <%= link_to "See Comments", comments_path %> No newline at end of file |
9db287f to
d56f90b
Compare
d56f90b to
c0d7f95
Compare
| <% @dependencies.each do |dependency| %> | ||
| <tr> | ||
| <td><%= dependency.name %></td> | ||
| <td><%= dependency.comments.count %> comments</td> |
There was a problem hiding this comment.
Use I18n or remove this line
c0d7f95 to
3f30003
Compare
| <div class="field"> | ||
| <%= form.label :dependency_id %> | ||
| <%= form.text_field :dependency_id %> | ||
| <%= select_tag(:dependency_id, |
There was a problem hiding this comment.
Please use form.select instead of select_tag, specially in rails 5.2 with the new form_with method.
3f30003 to
c3721ad
Compare
3 participants
Basic set up to navigate between dependencies and comments