From b8c9257fb1dc70cd65a14cb7dec62455ea54e394 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Mon, 2 Mar 2015 15:05:23 -0800
Subject: [PATCH] Fix bug where editing a comment with "+1" or "-1" would cause
a server error
Closes #1151
---
CHANGELOG | 1 +
app/controllers/projects/notes_controller.rb | 8 +++++++-
app/helpers/notes_helper.rb | 6 +++---
app/views/projects/notes/_edit_form.html.haml | 1 +
app/views/projects/notes/_form.html.haml | 2 +-
features/project/commits/comments.feature | 6 ++++++
features/project/issues/issues.feature | 9 +++++++++
features/steps/shared/note.rb | 17 +++++++++++++++++
8 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index 6bd93b8cd4b..e26d4ab690e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
Please view this file on the master branch, on stable branches it's out of date.
v 7.9.0 (unreleased)
+ - Fix bug that caused a server error when editing a comment to "+1" or "-1" (Stan Hu)
- Move labels/milestones tabs to sidebar
- Upgrade Rails gem to version 4.1.9.
- Improve error messages for file edit failures
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 2f1d631c14a..868629a0bc4 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -3,10 +3,10 @@ class Projects::NotesController < Projects::ApplicationController
before_filter :authorize_read_note!
before_filter :authorize_write_note!, only: [:create]
before_filter :authorize_admin_note!, only: [:update, :destroy]
+ before_filter :find_current_user_notes, except: [:destroy, :delete_attachment]
def index
current_fetched_at = Time.now.to_i
- @notes = NotesFinder.new.execute(project, current_user, params)
notes_json = { notes: [], last_fetched_at: current_fetched_at }
@@ -116,4 +116,10 @@ class Projects::NotesController < Projects::ApplicationController
:attachment, :line_code, :commit_id
)
end
+
+ private
+
+ def find_current_user_notes
+ @notes = NotesFinder.new.execute(project, current_user, params)
+ end
end
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 92ecb2abe4d..ab44fa6ee43 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -4,9 +4,9 @@ module NotesHelper
(@noteable.class.name == note.noteable_type && !note.for_diff_line?)
end
- def note_target_fields
- hidden_field_tag(:target_type, @target_type) +
- hidden_field_tag(:target_id, @target_id)
+ def note_target_fields(note)
+ hidden_field_tag(:target_type, note.noteable.class.name.underscore) +
+ hidden_field_tag(:target_id, note.noteable.id)
end
def link_to_commit_diff_line_note(note)
diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml
index b51ca795418..acb3991d294 100644
--- a/app/views/projects/notes/_edit_form.html.haml
+++ b/app/views/projects/notes/_edit_form.html.haml
@@ -1,5 +1,6 @@
.note-edit-form
= form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true do |f|
+ = note_target_fields(note)
= render layout: 'projects/md_preview', locals: { preview_class: "note-text" } do
= render 'projects/zen', f: f, attr: :note,
classes: 'note_text js-note-text'
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index 4476337cb10..be96c302143 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -1,5 +1,5 @@
= form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form common-note-form gfm-form" }, authenticity_token: true do |f|
- = note_target_fields
+ = note_target_fields(@note)
= f.hidden_field :commit_id
= f.hidden_field :line_code
= f.hidden_field :noteable_id
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
--
GitLab