diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index f5005ec2c949d3183a776e0f210ac55b883d00e1..62961b529fdbd9deeb9d862079bf4cc4e377c2ea 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -46,6 +46,26 @@ var NoteList = {
".js-note-delete",
NoteList.removeNote);
+ // show the edit note form
+ $(document).on("click",
+ ".js-note-edit",
+ NoteList.showEditNoteForm);
+
+ // cancel note editing
+ $(document).on("click",
+ ".note-edit-cancel",
+ NoteList.cancelNoteEdit);
+
+ // delete note attachment
+ $(document).on("click",
+ ".js-note-attachment-delete",
+ NoteList.deleteNoteAttachment);
+
+ // update the note after editing
+ $(document).on("ajax:complete",
+ "form.edit_note",
+ NoteList.updateNote);
+
// reset main target form after submit
$(document).on("ajax:complete",
".js-main-target-form",
@@ -53,12 +73,12 @@ var NoteList = {
$(document).on("click",
- ".js-choose-note-attachment-button",
- NoteList.chooseNoteAttachment);
+ ".js-choose-note-attachment-button",
+ NoteList.chooseNoteAttachment);
$(document).on("click",
- ".js-show-outdated-discussion",
- function(e) { $(this).next('.outdated-discussion').show(); e.preventDefault() });
+ ".js-show-outdated-discussion",
+ function(e) { $(this).next('.outdated-discussion').show(); e.preventDefault() });
},
@@ -97,8 +117,8 @@ var NoteList = {
/**
* Called when clicking the "Choose File" button.
- *
- * Opesn the file selection dialog.
+ *
+ * Opens the file selection dialog.
*/
chooseNoteAttachment: function() {
var form = $(this).closest("form");
@@ -133,7 +153,7 @@ var NoteList = {
/**
* Called in response to "cancel" on a diff note form.
- *
+ *
* Shows the reply button again.
* Removes the form and if necessary it's temporary row.
*/
@@ -176,6 +196,59 @@ var NoteList = {
NoteList.updateVotes();
},
+ /**
+ * Called in response to clicking the edit note link
+ *
+ * Replaces the note text with the note edit form
+ * Adds a hidden div with the original content of the note to fill the edit note form with
+ * if the user cancels
+ */
+ showEditNoteForm: function(e) {
+ e.preventDefault();
+ var note = $(this).closest(".note");
+ note.find(".note-text").hide();
+
+ // Show the attachment delete link
+ note.find(".js-note-attachment-delete").show();
+
+ var form = note.find(".note-edit-form");
+ form.show();
+
+
+ var textarea = form.find("textarea");
+ var p = $("<p></p>").text(textarea.val());
+ var hidden_div = $('<div class="note-original-content"></div>').append(p);
+ form.append(hidden_div);
+ hidden_div.hide();
+ textarea.focus();
+ },
+
+ /**
+ * Called in response to clicking the cancel button when editing a note
+ *
+ * Resets and hides the note editing form
+ */
+ cancelNoteEdit: function(e) {
+ e.preventDefault();
+ var note = $(this).closest(".note");
+ NoteList.resetNoteEditing(note);
+ },
+
+
+ /**
+ * Called in response to clicking the delete attachment link
+ *
+ * Removes the attachment wrapper view, including image tag if it exists
+ * Resets the note editing form
+ */
+ deleteNoteAttachment: function() {
+ var note = $(this).closest(".note");
+ note.find(".note-attachment").remove();
+ NoteList.resetNoteEditing(note);
+ NoteList.rewriteTimestamp(note.find(".note-last-update"));
+ },
+
+
/**
* Called when clicking on the "reply" button for a diff line.
*
@@ -426,5 +499,65 @@ var NoteList = {
votes.find(".upvotes").text(votes.find(".upvotes").text().replace(/\d+/, upvotes));
votes.find(".downvotes").text(votes.find(".downvotes").text().replace(/\d+/, downvotes));
}
+ },
+
+ /**
+ * Called in response to the edit note form being submitted
+ *
+ * Updates the current note field.
+ * Hides the edit note form
+ */
+ updateNote: function(e, xhr, settings) {
+ response = JSON.parse(xhr.responseText);
+ if (response.success) {
+ var note_li = $("#note_" + response.id);
+ var note_text = note_li.find(".note-text");
+ note_text.html(response.note).show();
+
+ var note_form = note_li.find(".note-edit-form");
+ note_form.hide();
+ note_form.find(".btn-save").enableButton();
+
+ // Update the "Edited at xxx label" on the note to show it's just been updated
+ NoteList.rewriteTimestamp(note_li.find(".note-last-update"));
+ }
+ },
+
+ /**
+ * Called in response to the 'cancel note' link clicked, or after deleting a note attachment
+ *
+ * Hides the edit note form and shows the note
+ * Resets the edit note form textarea with the original content of the note
+ */
+ resetNoteEditing: function(note) {
+ note.find(".note-text").show();
+
+ // Hide the attachment delete link
+ note.find(".js-note-attachment-delete").hide();
+
+ // Put the original content of the note back into the edit form textarea
+ var form = note.find(".note-edit-form");
+ var original_content = form.find(".note-original-content");
+ form.find("textarea").val(original_content.text());
+ original_content.remove();
+
+ note.find(".note-edit-form").hide();
+ },
+
+ /**
+ * Utility function to generate new timestamp text for a note
+ *
+ */
+ rewriteTimestamp: function(element) {
+ // Strip all newlines from the existing timestamp
+ var ts = element.text().replace(/\n/g, ' ').trim();
+
+ // If the timestamp already has '(Edited xxx ago)' text, remove it
+ ts = ts.replace(new RegExp("\\(Edited [A-Za-z0-9 ]+\\)$", "gi"), "");
+
+ // Append "(Edited just now)"
+ ts = (ts + " <small>(Edited just now)</small>");
+
+ element.html(ts);
}
};
diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss
index d4bb4872ac741c350fe9b2d89787f74207d52bda..bae1ac3aa9a91023578c4bb83990025cc8b3e736 100644
--- a/app/assets/stylesheets/sections/notes.scss
+++ b/app/assets/stylesheets/sections/notes.scss
@@ -325,3 +325,32 @@ ul.notes {
float: left;
}
}
+
+.note-edit-form {
+ display: none;
+
+ .note_text {
+ border: 1px solid #DDD;
+ box-shadow: none;
+ font-size: 14px;
+ height: 80px;
+ width: 98.6%;
+ }
+
+ .form-actions {
+ padding-left: 20px;
+
+ .btn-save {
+ float: left;
+ }
+
+ .note-form-option {
+ float: left;
+ padding: 2px 0 0 25px;
+ }
+ }
+}
+
+.js-note-attachment-delete {
+ display: none;
+}
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index fbd0f01e5d436d57ad78771ebae1e43657634675..a3ec4cca59de85d3061ceab135c3d96dd157e174 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -28,4 +28,11 @@ module NotesHelper
def loading_new_notes?
params[:loading_new].present?
end
+
+ def note_timestamp(note)
+ # Shows the created at time and the updated at time if different
+ ts = "#{time_ago_in_words(note.created_at)} ago"
+ ts << content_tag(:small, " (Edited #{time_ago_in_words(note.updated_at)} ago)") if note.updated_at != note.created_at
+ ts.html_safe
+ end
end
diff --git a/app/views/notes/_note.html.haml b/app/views/notes/_note.html.haml
index 88c450d61f936b5e547adb14e1836f029f5633b4..6b08aa5afe85415fb05510db29ff8798cb9d8430 100644
--- a/app/views/notes/_note.html.haml
+++ b/app/views/notes/_note.html.haml
@@ -6,13 +6,14 @@
Link here
- if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project)
- = link_to project_note_path(@project, note), title: "Remove comment", method: :delete, confirm: 'Are you sure you want to remove comment?', remote: true, class: "danger js-note-delete" do
+ = link_to "javascript:;", title: "Edit comment", class: "js-note-edit" do
+ %i.icon-edit
+ = link_to project_note_path(@project, note), title: "Remove comment", method: :delete, confirm: 'Are you sure you want to remove this comment?', remote: true, class: "danger js-note-delete" do
%i.icon-trash.cred
= image_tag gravatar_icon(note.author.email), class: "avatar s32", alt: ''
= link_to_member(@project, note.author, avatar: false)
%span.note-last-update
- = time_ago_in_words(note.updated_at)
- ago
+ = note_timestamp(note)
- if note.upvote?
%span.vote.upvote.label.label-success
@@ -25,13 +26,37 @@
.note-body
- = preserve do
- = markdown(note.note)
+ .note-text
+ = preserve do
+ = markdown(note.note)
+
+ .note-edit-form
+ = form_for note, url: project_note_path(@project, note), method: :put, remote: true do |f|
+ = f.text_area :note, class: 'note_text js-note-text js-gfm-input turn-on'
+
+ .form-actions
+ = f.submit 'Save changes', class: "btn btn-primary btn-save"
+
+ .note-form-option
+ %a.choose-btn.btn.btn-small.js-choose-note-attachment-button
+ %i.icon-paper-clip
+ %span Choose File ...
+
+ %span.file_name.js-attachment-filename File name...
+ = f.file_field :attachment, class: "js-note-attachment-input hide"
+
+ = link_to 'Cancel', "javascript:;", class: "btn btn-cancel note-edit-cancel"
+
+
- if note.attachment.url
- - if note.attachment.image?
- = image_tag note.attachment.url, class: 'note-image-attach'
- .attachment.pull-right
- = link_to note.attachment.secure_url, target: "_blank" do
- %i.icon-paper-clip
- = note.attachment_identifier
+ .note-attachment
+ - if note.attachment.image?
+ = image_tag note.attachment.url, class: 'note-image-attach'
+ .attachment.pull-right
+ = link_to note.attachment.secure_url, target: "_blank" do
+ %i.icon-paper-clip
+ = note.attachment_identifier
+ = link_to delete_attachment_project_note_path(@project, note),
+ title: "Delete this attachment", method: :delete, remote: true, confirm: 'Are you sure you want to remove the attachment?', class: "danger js-note-attachment-delete" do
+ %i.icon-trash.cred
.clear
diff --git a/spec/factories.rb b/spec/factories.rb
index b596f80fa9ed34db5f84ac73a9714c3e4d66fca6..bd2ec6abf62f2aeb40dda5556ddabe63773d90ba 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -1,3 +1,5 @@
+include ActionDispatch::TestProcess
+
FactoryGirl.define do
sequence :sentence, aliases: [:title, :content] do
Faker::Lorem.sentence
@@ -120,6 +122,7 @@ FactoryGirl.define do
factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note]
factory :note_on_merge_request, traits: [:on_merge_request]
factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff]
+ factory :note_on_merge_request_with_attachment, traits: [:on_merge_request, :with_attachment]
trait :on_commit do
project factory: :project_with_code
@@ -141,6 +144,10 @@ FactoryGirl.define do
noteable_id 1
noteable_type "Issue"
end
+
+ trait :with_attachment do
+ attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png") }
+ end
end
factory :event do
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index 24f5437efff4de6a632fa6cdb3958bda4bd67952..d7bc66dd9c879e63f32585dca71e765c1d01846b 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/spec/features/notes_on_merge_requests_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
describe "On a merge request", js: true do
let!(:project) { create(:project_with_code) }
let!(:merge_request) { create(:merge_request, project: project) }
+ let!(:note) { create(:note_on_merge_request_with_attachment, project: project) }
before do
login_as :user
@@ -72,6 +73,71 @@ describe "On a merge request", js: true do
should_not have_css(".note")
end
end
+
+ describe "when editing a note", js: true do
+ it "should contain the hidden edit form" do
+ within("#note_#{note.id}") { should have_css(".note-edit-form", visible: false) }
+ end
+
+ describe "editing the note" do
+ before do
+ find('.note').hover
+ find(".js-note-edit").click
+ end
+
+ it "should show the note edit form and hide the note body" do
+ within("#note_#{note.id}") do
+ find(".note-edit-form", visible: true).should be_visible
+ find(".note-text", visible: false).should_not be_visible
+ end
+ end
+
+ it "should reset the edit note form textarea with the original content of the note if cancelled" do
+ find('.note').hover
+ find(".js-note-edit").click
+
+ within(".note-edit-form") do
+ fill_in "note[note]", with: "Some new content"
+ find(".btn-cancel").click
+ find(".js-note-text", visible: false).text.should == note.note
+ end
+ end
+
+ it "appends the edited at time to the note" do
+ find('.note').hover
+ find(".js-note-edit").click
+
+ within(".note-edit-form") do
+ fill_in "note[note]", with: "Some new content"
+ find(".btn-save").click
+ end
+
+ within("#note_#{note.id}") do
+ should have_css(".note-last-update small")
+ find(".note-last-update small").text.should match(/Edited just now/)
+ end
+ end
+ end
+
+ describe "deleting an attachment" do
+ before do
+ find('.note').hover
+ find(".js-note-edit").click
+ end
+
+ it "shows the delete link" do
+ within(".note-attachment") do
+ should have_css(".js-note-attachment-delete")
+ end
+ end
+
+ it "removes the attachment div and resets the edit form" do
+ find(".js-note-attachment-delete").click
+ should_not have_css(".note-attachment")
+ find(".note-edit-form", visible: false).should_not be_visible
+ end
+ end
+ end
end
describe "On a merge request diff", js: true, focus: true do
diff --git a/spec/fixtures/dk.png b/spec/fixtures/dk.png
new file mode 100644
index 0000000000000000000000000000000000000000..87ce25e877ab8a9602b77af019f191e6749003f9
Binary files /dev/null and b/spec/fixtures/dk.png differ