List Notes and add Add note button.
#59
Conversation
|
Need to find out why deleting comments returns 500 sometimes. Not sure if its related to my machine only or not. |
|
Those 500 errors might be related to #16, which I thought I fixed by setting the ActiveRecord connection pool size. |
app.rb
Outdated
| if @record.notes.create!(body: params[:body]) | ||
| flash[:success] = "Note added successfully." | ||
| else | ||
| flash[:danger] = "Something went wrong!" |
There was a problem hiding this comment.
Change message to "Failed to create Note".
views/db/advisories/show.erb
Outdated
|
|
||
| <%= partial(:notes, notes: @advisory.notes) %> | ||
|
|
||
| <%= partial(:add_note, parent: @advisory) %> |
There was a problem hiding this comment.
You could probably merge _add_note.erb into _notes.erb.
views/_notes.erb
Outdated
| console.error('Error:', error); | ||
| }); | ||
| } | ||
| </script> |
There was a problem hiding this comment.
This JavaScript should probably be extracted into public/javascript/notes.js so it gets cached by the browser.
views/_notes.erb
Outdated
| @@ -0,0 +1,43 @@ | |||
| <% if notes.present? %> | |||
There was a problem hiding this comment.
Use unless notes.empty?. I'm trying to avoid using ActiveSupport core-exts as much as possible. In the future I may switch from ActiveRecord to Sequel::Model.
views/_notes.erb
Outdated
| console.error('Error:', error); | ||
| }); | ||
| } | ||
| </script> |
There was a problem hiding this comment.
Also this JavaScript should be wrapped in a function that is passed to ready(...);.
const Notes = {
init() {
...
},
delete(noteId, button) {
...
}
};
ready(Notes.init);
views/_notes.erb
Outdated
|
|
||
| <script> | ||
| const deleteButtons = document.querySelectorAll('.delete-note'); | ||
| const path = '<%= request.path_info %>' |
There was a problem hiding this comment.
Could we use document.location?
views/_notes.erb
Outdated
| <% end %> | ||
| <% end %> | ||
|
|
||
| <%= partial(:add_note) %> |
There was a problem hiding this comment.
I would just merge _add_note.erb directly into _notes.erb. I don't think you'd ever want to render _add_note.erb separately from _notes.erb?
views/_notes.erb
Outdated
| </div> | ||
| </form> | ||
| </div> | ||
|
|
#42 #43