diff options
author | Marin Jankovski <marin@gitlab.com> | 2015-03-03 21:57:00 +0000 |
---|---|---|
committer | Marin Jankovski <marin@gitlab.com> | 2015-03-03 21:57:00 +0000 |
commit | 66fa4b09edd5e347882aa2d5646b5faeeac3b37d (patch) | |
tree | 784e2bbcd919b98672c900c636fb98a2adc0af9d /features | |
parent | 8c47a72a4ed3df2327104e029307b5d804525886 (diff) | |
parent | b8c9257fb1dc70cd65a14cb7dec62455ea54e394 (diff) | |
download | gitlab-ce-66fa4b09edd5e347882aa2d5646b5faeeac3b37d.tar.gz |
Merge branch 'fix-edit-note-with-votes' into 'master'
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
Diffstat (limited to 'features')
-rw-r--r-- | features/project/commits/comments.feature | 6 | ||||
-rw-r--r-- | features/project/issues/issues.feature | 9 | ||||
-rw-r--r-- | features/steps/shared/note.rb | 17 |
3 files changed, 32 insertions, 0 deletions
diff --git a/features/project/commits/comments.feature b/features/project/commits/comments.feature index afcf0fdbb07..c41075d7ad4 100644 --- a/features/project/commits/comments.feature +++ b/features/project/commits/comments.feature @@ -41,3 +41,9 @@ Feature: Project Commits Comments Given I leave a comment like "XML attached" And I delete a comment Then I should not see a comment saying "XML attached" + + @javascript + Scenario: I can edit a comment with +1 + Given I leave a comment like "XML attached" + And I edit the last comment with a +1 + Then I should see +1 in the description diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index 28ea44530fe..283979204db 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -139,6 +139,15 @@ Feature: Project Issues And I leave a comment with task markdown Then I should not see task checkboxes in the comment + @javascript + Scenario: Issue notes should be editable with +1 + Given project "Shop" has "Tasks-open" open issue with task markdown + When I visit issue page "Tasks-open" + And I leave a comment with a header containing "Comment with a header" + Then The comment with the header should not have an ID + And I edit the last comment with a +1 + Then I should see +1 in the description + # Task status in issues list Scenario: Issues list should display task status diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index 45773056953..583746d4475 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -135,4 +135,21 @@ module SharedNote 'li.note div.timeline-content input[type="checkbox"]' ) end + + step 'I edit the last comment with a +1' do + find(".note").hover + find('.js-note-edit').click + + within(".current-note-edit-form") do + fill_in 'note[note]', with: '+1 Awesome!' + click_button 'Save Comment' + sleep 0.05 + end + end + + step 'I should see +1 in the description' do + within(".note") do + page.should have_content("+1 Awesome!") + end + end end |