66fa4b09ed
Fix server error when editing a note to "+1" or "-1" ### Summary If a user edits a comment with "+1" or "-1" in the beginning, the POST returns an Internal Server error. (issue #1151). This merge request resolves that error. ### Steps to reproduce 1. Comment on an issue with "Test comment". 2. Edit the issue. 3. Write "+1" and click "Save Comment". ### Expected behavior The edited note should be saved and refreshed. Any previous upvotes/downvotes from the user should contain a strikethrough. ### Observed behavior Internal Error ### Relevant logs ``` Started PUT "/avocode/avocode-manager/notes/4996" for 185.33.136.107 at 2015-02-28 17:11:53 +0100 Processing by Projects::NotesController#update as JS Parameters: {"utf8"=>"✓", "authenticity_token"=>"*removed*", "note"=>{"note"=>"+1\r\n\r\nYes"}, "commit"=>"Save Comment", "project_id"=>"avocode/avocode-manager", "id"=>"4996"} Completed 500 Internal Server Error in 86ms ActionView::Template::Error (undefined method `each' for nil:NilClass): 28: %span.note-last-update 29: = note_timestamp(note) 30: 31: - if note.superceded?(@notes) 32: - if note.upvote? 33: %span.vote.upvote.label.label-gray.strikethrough 34: %i.fa.fa-thumbs-up app/models/note.rb:495:in `superceded?' app/views/projects/notes/_note.html.haml:31:in `_app_views_projects_notes__note_html_haml___812277000516355462_69988235638820' app/controllers/projects/notes_controller.rb:71:in `note_to_html' app/controllers/projects/notes_controller.rb:103:in `render_note_json' app/controllers/projects/notes_controller.rb:39:in `block (2 levels) in update' app/controllers/projects/notes_controller.rb:38:in `update' ``` ### Fix It turns out no tests were present for the "Edit Issue" functionality. I added spinach tests to exercise this and reproduced the error. Most of the routes in `notes_controller.rb` appear to render all notes for the given discussion. `_form.html.haml` needs the full list of notes commented by the user to add strikethroughs for older upvotes/downvotes. However, only the `index` route appeared to obtain this information. The fix is to add a `before_filter` to obtain all the user's notes beforehand, except in the delete case where this information is not needed. Things to watch: `NotesFinder` needs `target_type` and `target_id` to determine what to do. I'm not sure if there is a conscious effort to phase these keywords out in favor of `noteable_type` and `noteable_id`. See merge request !360 |
||
---|---|---|
.. | ||
commits | ||
issues | ||
source | ||
active_tab.feature | ||
archived.feature | ||
create.feature | ||
deploy_keys.feature | ||
fork.feature | ||
forked_merge_requests.feature | ||
graph.feature | ||
hooks.feature | ||
merge_requests.feature | ||
network_graph.feature | ||
project.feature | ||
redirects.feature | ||
service.feature | ||
shortcuts.feature | ||
snippets.feature | ||
star.feature | ||
team_management.feature | ||
wiki.feature |