Commit 47f78a2c authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge pull request #4440 from jackbot/editable-notes

Editable notes
parents 315d4cc8 6fa06893
...@@ -46,6 +46,26 @@ var NoteList = { ...@@ -46,6 +46,26 @@ var NoteList = {
".js-note-delete", ".js-note-delete",
NoteList.removeNote); 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 // reset main target form after submit
$(document).on("ajax:complete", $(document).on("ajax:complete",
".js-main-target-form", ".js-main-target-form",
...@@ -53,12 +73,12 @@ var NoteList = { ...@@ -53,12 +73,12 @@ var NoteList = {
$(document).on("click", $(document).on("click",
".js-choose-note-attachment-button", ".js-choose-note-attachment-button",
NoteList.chooseNoteAttachment); NoteList.chooseNoteAttachment);
$(document).on("click", $(document).on("click",
".js-show-outdated-discussion", ".js-show-outdated-discussion",
function(e) { $(this).next('.outdated-discussion').show(); e.preventDefault() }); function(e) { $(this).next('.outdated-discussion').show(); e.preventDefault() });
}, },
...@@ -97,8 +117,8 @@ var NoteList = { ...@@ -97,8 +117,8 @@ var NoteList = {
/** /**
* Called when clicking the "Choose File" button. * Called when clicking the "Choose File" button.
* *
* Opesn the file selection dialog. * Opens the file selection dialog.
*/ */
chooseNoteAttachment: function() { chooseNoteAttachment: function() {
var form = $(this).closest("form"); var form = $(this).closest("form");
...@@ -133,7 +153,7 @@ var NoteList = { ...@@ -133,7 +153,7 @@ var NoteList = {
/** /**
* Called in response to "cancel" on a diff note form. * Called in response to "cancel" on a diff note form.
* *
* Shows the reply button again. * Shows the reply button again.
* Removes the form and if necessary it's temporary row. * Removes the form and if necessary it's temporary row.
*/ */
...@@ -176,6 +196,59 @@ var NoteList = { ...@@ -176,6 +196,59 @@ var NoteList = {
NoteList.updateVotes(); 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. * Called when clicking on the "reply" button for a diff line.
* *
...@@ -426,5 +499,65 @@ var NoteList = { ...@@ -426,5 +499,65 @@ var NoteList = {
votes.find(".upvotes").text(votes.find(".upvotes").text().replace(/\d+/, upvotes)); votes.find(".upvotes").text(votes.find(".upvotes").text().replace(/\d+/, upvotes));
votes.find(".downvotes").text(votes.find(".downvotes").text().replace(/\d+/, downvotes)); 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);
} }
}; };
...@@ -325,3 +325,32 @@ ul.notes { ...@@ -325,3 +325,32 @@ ul.notes {
float: left; 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;
}
...@@ -38,6 +38,32 @@ class NotesController < ProjectResourceController ...@@ -38,6 +38,32 @@ class NotesController < ProjectResourceController
end end
end end
def update
@note = @project.notes.find(params[:id])
return access_denied! unless can?(current_user, :admin_note, @note)
@note.update_attributes(params[:note])
respond_to do |format|
format.js do
render js: { success: @note.valid?, id: @note.id, note: view_context.markdown(@note.note) }.to_json
end
format.html do
redirect_to :back
end
end
end
def delete_attachment
@note = @project.notes.find(params[:id])
@note.remove_attachment!
@note.update_attribute(:attachment, nil)
respond_to do |format|
format.js { render nothing: true }
end
end
def preview def preview
render text: view_context.markdown(params[:note]) render text: view_context.markdown(params[:note])
end end
......
...@@ -28,4 +28,11 @@ module NotesHelper ...@@ -28,4 +28,11 @@ module NotesHelper
def loading_new_notes? def loading_new_notes?
params[:loading_new].present? params[:loading_new].present?
end 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 end
...@@ -20,22 +20,24 @@ ...@@ -20,22 +20,24 @@
= nav_tab :scope, 'joined' do = nav_tab :scope, 'joined' do
= link_to "Joined", projects_dashboard_path(scope: 'joined') = link_to "Joined", projects_dashboard_path(scope: 'joined')
%p.light Filter by label: - if @labels.any?
%ul.bordered-list %p.light Filter by label:
- @labels.each do |label| %ul.bordered-list
%li{ class: (label.name == params[:label]) ? 'active' : 'light' } - @labels.each do |label|
= link_to projects_dashboard_path(scope: params[:scope], label: label.name) do %li{ class: (label.name == params[:label]) ? 'active' : 'light' }
%i.icon-tag = link_to projects_dashboard_path(scope: params[:scope], label: label.name) do
= label.name %i.icon-tag
= label.name
.span9 .span9
= form_tag projects_dashboard_path, method: 'get' do - if @projects.any?
%fieldset.dashboard-search-filter = form_tag projects_dashboard_path, method: 'get' do
= hidden_field_tag "scope", params[:scope] %fieldset.dashboard-search-filter
= search_field_tag "search", params[:search], { id: 'dashboard_projects_search', placeholder: 'Search', class: 'left input-xxlarge'} = hidden_field_tag "scope", params[:scope]
= button_tag type: 'submit', class: 'btn' do = search_field_tag "search", params[:search], { id: 'dashboard_projects_search', placeholder: 'Search', class: 'left input-xxlarge'}
%i.icon-search = button_tag type: 'submit', class: 'btn' do
%i.icon-search
%ul.bordered-list %ul.bordered-list
- @projects.each do |project| - @projects.each do |project|
......
...@@ -6,13 +6,14 @@ ...@@ -6,13 +6,14 @@
Link here Link here
&nbsp; &nbsp;
- if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project) - 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 "#", 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 %i.icon-trash.cred
= image_tag gravatar_icon(note.author.email), class: "avatar s32", alt: '' = image_tag gravatar_icon(note.author.email), class: "avatar s32", alt: ''
= link_to_member(@project, note.author, avatar: false) = link_to_member(@project, note.author, avatar: false)
%span.note-last-update %span.note-last-update
= time_ago_in_words(note.updated_at) = note_timestamp(note)
ago
- if note.upvote? - if note.upvote?
%span.vote.upvote.label.label-success %span.vote.upvote.label.label-success
...@@ -25,13 +26,37 @@ ...@@ -25,13 +26,37 @@
.note-body .note-body
= preserve do .note-text
= markdown(note.note) = 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 ...
&nbsp;
%span.file_name.js-attachment-filename File name...
= f.file_field :attachment, class: "js-note-attachment-input hide"
= link_to 'Cancel', "#", class: "btn btn-cancel note-edit-cancel"
- if note.attachment.url - if note.attachment.url
- if note.attachment.image? .note-attachment
= image_tag note.attachment.url, class: 'note-image-attach' - if note.attachment.image?
.attachment.pull-right = image_tag note.attachment.url, class: 'note-image-attach'
= link_to note.attachment.secure_url, target: "_blank" do .attachment.pull-right
%i.icon-paper-clip = link_to note.attachment.secure_url, target: "_blank" do
= note.attachment_identifier %i.icon-paper-clip
.clear = 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
\ No newline at end of file
...@@ -319,7 +319,10 @@ Gitlab::Application.routes.draw do ...@@ -319,7 +319,10 @@ Gitlab::Application.routes.draw do
end end
end end
resources :notes, only: [:index, :create, :destroy] do resources :notes, only: [:index, :create, :destroy, :update] do
member do
delete :delete_attachment
end
collection do collection do
post :preview post :preview
end end
......
include ActionDispatch::TestProcess
FactoryGirl.define do FactoryGirl.define do
sequence :sentence, aliases: [:title, :content] do sequence :sentence, aliases: [:title, :content] do
Faker::Lorem.sentence Faker::Lorem.sentence
...@@ -120,6 +122,7 @@ FactoryGirl.define do ...@@ -120,6 +122,7 @@ FactoryGirl.define do
factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note] factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note]
factory :note_on_merge_request, traits: [:on_merge_request] 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_diff, traits: [:on_merge_request, :on_diff]
factory :note_on_merge_request_with_attachment, traits: [:on_merge_request, :with_attachment]
trait :on_commit do trait :on_commit do
project factory: :project_with_code project factory: :project_with_code
...@@ -141,6 +144,10 @@ FactoryGirl.define do ...@@ -141,6 +144,10 @@ FactoryGirl.define do
noteable_id 1 noteable_id 1
noteable_type "Issue" noteable_type "Issue"
end end
trait :with_attachment do
attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png") }
end
end end
factory :event do factory :event do
......
...@@ -3,6 +3,7 @@ require 'spec_helper' ...@@ -3,6 +3,7 @@ require 'spec_helper'
describe "On a merge request", js: true do describe "On a merge request", js: true do
let!(:project) { create(:project_with_code) } let!(:project) { create(:project_with_code) }
let!(:merge_request) { create(:merge_request, project: project) } let!(:merge_request) { create(:merge_request, project: project) }
let!(:note) { create(:note_on_merge_request_with_attachment, project: project) }
before do before do
login_as :user login_as :user
...@@ -72,6 +73,71 @@ describe "On a merge request", js: true do ...@@ -72,6 +73,71 @@ describe "On a merge request", js: true do
should_not have_css(".note") should_not have_css(".note")
end end
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 end
describe "On a merge request diff", js: true, focus: true do describe "On a merge request diff", js: true, focus: true do
......
...@@ -262,7 +262,7 @@ end ...@@ -262,7 +262,7 @@ end
# project_snippet GET /:project_id/snippets/:id(.:format) snippets#show # project_snippet GET /:project_id/snippets/:id(.:format) snippets#show
# PUT /:project_id/snippets/:id(.:format) snippets#update # PUT /:project_id/snippets/:id(.:format) snippets#update
# DELETE /:project_id/snippets/:id(.:format) snippets#destroy # DELETE /:project_id/snippets/:id(.:format) snippets#destroy
describe Projects::SnippetsController, "routing" do describe SnippetsController, "routing" do
it "to #raw" do it "to #raw" do
get("/gitlabhq/snippets/1/raw").should route_to('projects/snippets#raw', project_id: 'gitlabhq', id: '1') get("/gitlabhq/snippets/1/raw").should route_to('projects/snippets#raw', project_id: 'gitlabhq', id: '1')
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment