Commit ee890f2b2a66be925746f2238dc462a74b0fd219
Exists in
master
and in
4 other branches
Merge branch 'master' into 6-0-dev
Conflicts: app/views/dashboard/projects.html.haml app/views/layouts/_head_panel.html.haml config/routes.rb
Showing
15 changed files
with
324 additions
and
24 deletions
Show diff stats
app/assets/javascripts/notes.js
... | ... | @@ -56,6 +56,26 @@ var NoteList = { |
56 | 56 | ".js-note-delete", |
57 | 57 | NoteList.removeNote); |
58 | 58 | |
59 | + // show the edit note form | |
60 | + $(document).on("click", | |
61 | + ".js-note-edit", | |
62 | + NoteList.showEditNoteForm); | |
63 | + | |
64 | + // cancel note editing | |
65 | + $(document).on("click", | |
66 | + ".note-edit-cancel", | |
67 | + NoteList.cancelNoteEdit); | |
68 | + | |
69 | + // delete note attachment | |
70 | + $(document).on("click", | |
71 | + ".js-note-attachment-delete", | |
72 | + NoteList.deleteNoteAttachment); | |
73 | + | |
74 | + // update the note after editing | |
75 | + $(document).on("ajax:complete", | |
76 | + "form.edit_note", | |
77 | + NoteList.updateNote); | |
78 | + | |
59 | 79 | // reset main target form after submit |
60 | 80 | $(document).on("ajax:complete", |
61 | 81 | ".js-main-target-form", |
... | ... | @@ -63,12 +83,12 @@ var NoteList = { |
63 | 83 | |
64 | 84 | |
65 | 85 | $(document).on("click", |
66 | - ".js-choose-note-attachment-button", | |
67 | - NoteList.chooseNoteAttachment); | |
86 | + ".js-choose-note-attachment-button", | |
87 | + NoteList.chooseNoteAttachment); | |
68 | 88 | |
69 | 89 | $(document).on("click", |
70 | - ".js-show-outdated-discussion", | |
71 | - function(e) { $(this).next('.outdated-discussion').show(); e.preventDefault() }); | |
90 | + ".js-show-outdated-discussion", | |
91 | + function(e) { $(this).next('.outdated-discussion').show(); e.preventDefault() }); | |
72 | 92 | }, |
73 | 93 | |
74 | 94 | |
... | ... | @@ -107,8 +127,8 @@ var NoteList = { |
107 | 127 | |
108 | 128 | /** |
109 | 129 | * Called when clicking the "Choose File" button. |
110 | - * | |
111 | - * Opesn the file selection dialog. | |
130 | + * | |
131 | + * Opens the file selection dialog. | |
112 | 132 | */ |
113 | 133 | chooseNoteAttachment: function() { |
114 | 134 | var form = $(this).closest("form"); |
... | ... | @@ -143,7 +163,7 @@ var NoteList = { |
143 | 163 | |
144 | 164 | /** |
145 | 165 | * Called in response to "cancel" on a diff note form. |
146 | - * | |
166 | + * | |
147 | 167 | * Shows the reply button again. |
148 | 168 | * Removes the form and if necessary it's temporary row. |
149 | 169 | */ |
... | ... | @@ -187,6 +207,59 @@ var NoteList = { |
187 | 207 | }, |
188 | 208 | |
189 | 209 | /** |
210 | + * Called in response to clicking the edit note link | |
211 | + * | |
212 | + * Replaces the note text with the note edit form | |
213 | + * Adds a hidden div with the original content of the note to fill the edit note form with | |
214 | + * if the user cancels | |
215 | + */ | |
216 | + showEditNoteForm: function(e) { | |
217 | + e.preventDefault(); | |
218 | + var note = $(this).closest(".note"); | |
219 | + note.find(".note-text").hide(); | |
220 | + | |
221 | + // Show the attachment delete link | |
222 | + note.find(".js-note-attachment-delete").show(); | |
223 | + | |
224 | + var form = note.find(".note-edit-form"); | |
225 | + form.show(); | |
226 | + | |
227 | + | |
228 | + var textarea = form.find("textarea"); | |
229 | + var p = $("<p></p>").text(textarea.val()); | |
230 | + var hidden_div = $('<div class="note-original-content"></div>').append(p); | |
231 | + form.append(hidden_div); | |
232 | + hidden_div.hide(); | |
233 | + textarea.focus(); | |
234 | + }, | |
235 | + | |
236 | + /** | |
237 | + * Called in response to clicking the cancel button when editing a note | |
238 | + * | |
239 | + * Resets and hides the note editing form | |
240 | + */ | |
241 | + cancelNoteEdit: function(e) { | |
242 | + e.preventDefault(); | |
243 | + var note = $(this).closest(".note"); | |
244 | + NoteList.resetNoteEditing(note); | |
245 | + }, | |
246 | + | |
247 | + | |
248 | + /** | |
249 | + * Called in response to clicking the delete attachment link | |
250 | + * | |
251 | + * Removes the attachment wrapper view, including image tag if it exists | |
252 | + * Resets the note editing form | |
253 | + */ | |
254 | + deleteNoteAttachment: function() { | |
255 | + var note = $(this).closest(".note"); | |
256 | + note.find(".note-attachment").remove(); | |
257 | + NoteList.resetNoteEditing(note); | |
258 | + NoteList.rewriteTimestamp(note.find(".note-last-update")); | |
259 | + }, | |
260 | + | |
261 | + | |
262 | + /** | |
190 | 263 | * Called when clicking on the "reply" button for a diff line. |
191 | 264 | * |
192 | 265 | * Shows the note form below the notes. |
... | ... | @@ -436,5 +509,65 @@ var NoteList = { |
436 | 509 | votes.find(".upvotes").text(votes.find(".upvotes").text().replace(/\d+/, upvotes)); |
437 | 510 | votes.find(".downvotes").text(votes.find(".downvotes").text().replace(/\d+/, downvotes)); |
438 | 511 | } |
512 | + }, | |
513 | + | |
514 | + /** | |
515 | + * Called in response to the edit note form being submitted | |
516 | + * | |
517 | + * Updates the current note field. | |
518 | + * Hides the edit note form | |
519 | + */ | |
520 | + updateNote: function(e, xhr, settings) { | |
521 | + response = JSON.parse(xhr.responseText); | |
522 | + if (response.success) { | |
523 | + var note_li = $("#note_" + response.id); | |
524 | + var note_text = note_li.find(".note-text"); | |
525 | + note_text.html(response.note).show(); | |
526 | + | |
527 | + var note_form = note_li.find(".note-edit-form"); | |
528 | + note_form.hide(); | |
529 | + note_form.find(".btn-save").enableButton(); | |
530 | + | |
531 | + // Update the "Edited at xxx label" on the note to show it's just been updated | |
532 | + NoteList.rewriteTimestamp(note_li.find(".note-last-update")); | |
533 | + } | |
534 | + }, | |
535 | + | |
536 | + /** | |
537 | + * Called in response to the 'cancel note' link clicked, or after deleting a note attachment | |
538 | + * | |
539 | + * Hides the edit note form and shows the note | |
540 | + * Resets the edit note form textarea with the original content of the note | |
541 | + */ | |
542 | + resetNoteEditing: function(note) { | |
543 | + note.find(".note-text").show(); | |
544 | + | |
545 | + // Hide the attachment delete link | |
546 | + note.find(".js-note-attachment-delete").hide(); | |
547 | + | |
548 | + // Put the original content of the note back into the edit form textarea | |
549 | + var form = note.find(".note-edit-form"); | |
550 | + var original_content = form.find(".note-original-content"); | |
551 | + form.find("textarea").val(original_content.text()); | |
552 | + original_content.remove(); | |
553 | + | |
554 | + note.find(".note-edit-form").hide(); | |
555 | + }, | |
556 | + | |
557 | + /** | |
558 | + * Utility function to generate new timestamp text for a note | |
559 | + * | |
560 | + */ | |
561 | + rewriteTimestamp: function(element) { | |
562 | + // Strip all newlines from the existing timestamp | |
563 | + var ts = element.text().replace(/\n/g, ' ').trim(); | |
564 | + | |
565 | + // If the timestamp already has '(Edited xxx ago)' text, remove it | |
566 | + ts = ts.replace(new RegExp("\\(Edited [A-Za-z0-9 ]+\\)$", "gi"), ""); | |
567 | + | |
568 | + // Append "(Edited just now)" | |
569 | + ts = (ts + " <small>(Edited just now)</small>"); | |
570 | + | |
571 | + element.html(ts); | |
439 | 572 | } |
440 | 573 | }; | ... | ... |
app/assets/stylesheets/sections/header.scss
app/assets/stylesheets/sections/notes.scss
... | ... | @@ -325,3 +325,32 @@ ul.notes { |
325 | 325 | float: left; |
326 | 326 | } |
327 | 327 | } |
328 | + | |
329 | +.note-edit-form { | |
330 | + display: none; | |
331 | + | |
332 | + .note_text { | |
333 | + border: 1px solid #DDD; | |
334 | + box-shadow: none; | |
335 | + font-size: 14px; | |
336 | + height: 80px; | |
337 | + width: 98.6%; | |
338 | + } | |
339 | + | |
340 | + .form-actions { | |
341 | + padding-left: 20px; | |
342 | + | |
343 | + .btn-save { | |
344 | + float: left; | |
345 | + } | |
346 | + | |
347 | + .note-form-option { | |
348 | + float: left; | |
349 | + padding: 2px 0 0 25px; | |
350 | + } | |
351 | + } | |
352 | +} | |
353 | + | |
354 | +.js-note-attachment-delete { | |
355 | + display: none; | |
356 | +} | ... | ... |
app/controllers/projects/notes_controller.rb
... | ... | @@ -38,6 +38,32 @@ class Projects::NotesController < Projects::ApplicationController |
38 | 38 | end |
39 | 39 | end |
40 | 40 | |
41 | + def update | |
42 | + @note = @project.notes.find(params[:id]) | |
43 | + return access_denied! unless can?(current_user, :admin_note, @note) | |
44 | + | |
45 | + @note.update_attributes(params[:note]) | |
46 | + | |
47 | + respond_to do |format| | |
48 | + format.js do | |
49 | + render js: { success: @note.valid?, id: @note.id, note: view_context.markdown(@note.note) }.to_json | |
50 | + end | |
51 | + format.html do | |
52 | + redirect_to :back | |
53 | + end | |
54 | + end | |
55 | + end | |
56 | + | |
57 | + def delete_attachment | |
58 | + @note = @project.notes.find(params[:id]) | |
59 | + @note.remove_attachment! | |
60 | + @note.update_attribute(:attachment, nil) | |
61 | + | |
62 | + respond_to do |format| | |
63 | + format.js { render nothing: true } | |
64 | + end | |
65 | + end | |
66 | + | |
41 | 67 | def preview |
42 | 68 | render text: view_context.markdown(params[:note]) |
43 | 69 | end | ... | ... |
app/helpers/notes_helper.rb
... | ... | @@ -28,4 +28,11 @@ module NotesHelper |
28 | 28 | def loading_new_notes? |
29 | 29 | params[:loading_new].present? |
30 | 30 | end |
31 | + | |
32 | + def note_timestamp(note) | |
33 | + # Shows the created at time and the updated at time if different | |
34 | + ts = "#{time_ago_in_words(note.created_at)} ago" | |
35 | + ts << content_tag(:small, " (Edited #{time_ago_in_words(note.updated_at)} ago)") if note.updated_at != note.created_at | |
36 | + ts.html_safe | |
37 | + end | |
31 | 38 | end | ... | ... |
app/views/layouts/_head_panel.html.haml
app/views/projects/notes/_note.html.haml
... | ... | @@ -6,13 +6,14 @@ |
6 | 6 | Link here |
7 | 7 | |
8 | 8 | - if(note.author_id == current_user.id) || can?(current_user, :admin_note, @project) |
9 | - = 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 | |
9 | + = link_to "#", title: "Edit comment", class: "js-note-edit" do | |
10 | + %i.icon-edit | |
11 | + = 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 | |
10 | 12 | %i.icon-trash.cred |
11 | 13 | = image_tag gravatar_icon(note.author_email), class: "avatar s32" |
12 | 14 | = link_to_member(@project, note.author, avatar: false) |
13 | 15 | %span.note-last-update |
14 | - = time_ago_in_words(note.updated_at) | |
15 | - ago | |
16 | + = note_timestamp(note) | |
16 | 17 | |
17 | 18 | - if note.upvote? |
18 | 19 | %span.vote.upvote.label.label-success |
... | ... | @@ -25,13 +26,37 @@ |
25 | 26 | |
26 | 27 | |
27 | 28 | .note-body |
28 | - = preserve do | |
29 | - = markdown(note.note) | |
29 | + .note-text | |
30 | + = preserve do | |
31 | + = markdown(note.note) | |
32 | + | |
33 | + .note-edit-form | |
34 | + = form_for note, url: project_note_path(@project, note), method: :put, remote: true do |f| | |
35 | + = f.text_area :note, class: 'note_text js-note-text js-gfm-input turn-on' | |
36 | + | |
37 | + .form-actions | |
38 | + = f.submit 'Save changes', class: "btn btn-primary btn-save" | |
39 | + | |
40 | + .note-form-option | |
41 | + %a.choose-btn.btn.btn-small.js-choose-note-attachment-button | |
42 | + %i.icon-paper-clip | |
43 | + %span Choose File ... | |
44 | + | |
45 | + %span.file_name.js-attachment-filename File name... | |
46 | + = f.file_field :attachment, class: "js-note-attachment-input hide" | |
47 | + | |
48 | + = link_to 'Cancel', "#", class: "btn btn-cancel note-edit-cancel" | |
49 | + | |
50 | + | |
30 | 51 | - if note.attachment.url |
31 | - - if note.attachment.image? | |
32 | - = image_tag note.attachment.url, class: 'note-image-attach' | |
33 | - .attachment.pull-right | |
34 | - = link_to note.attachment.secure_url, target: "_blank" do | |
35 | - %i.icon-paper-clip | |
36 | - = note.attachment_identifier | |
52 | + .note-attachment | |
53 | + - if note.attachment.image? | |
54 | + = image_tag note.attachment.url, class: 'note-image-attach' | |
55 | + .attachment.pull-right | |
56 | + = link_to note.attachment.secure_url, target: "_blank" do | |
57 | + %i.icon-paper-clip | |
58 | + = note.attachment_identifier | |
59 | + = link_to delete_attachment_project_note_path(@project, note), | |
60 | + 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 | |
61 | + %i.icon-trash.cred | |
37 | 62 | .clear | ... | ... |
app/views/snippets/_blob.html.haml
... | ... | @@ -6,6 +6,7 @@ |
6 | 6 | .btn-group.tree-btn-group.pull-right |
7 | 7 | - if @snippet.author == current_user |
8 | 8 | = link_to "Edit", edit_snippet_path(@snippet), class: "btn btn-tiny", title: 'Edit Snippet' |
9 | + = link_to "Delete", snippet_path(@snippet), method: :delete, confirm: "Are you sure?", class: "btn btn-tiny", title: 'Delete Snippet' | |
9 | 10 | = link_to "Raw", raw_snippet_path(@snippet), class: "btn btn-tiny", target: "_blank" |
10 | 11 | .file_content.code |
11 | 12 | - unless @snippet.content.empty? | ... | ... |
app/views/snippets/show.html.haml
1 | 1 | %h3.page_title |
2 | 2 | - if @snippet.private? |
3 | - %i.icon-lock.cgreen | |
3 | + %i{:class => "icon-lock cgreen has_bottom_tooltip", "data-original-title" => "Private snippet"} | |
4 | 4 | - else |
5 | - %i.icon-globe.cblue | |
5 | + %i{:class => "icon-globe cblue has_bottom_tooltip", "data-original-title" => "Public snippet"} | |
6 | 6 | |
7 | 7 | = @snippet.title |
8 | 8 | ... | ... |
config/gitlab.yml.example
... | ... | @@ -18,6 +18,7 @@ production: &base |
18 | 18 | host: localhost |
19 | 19 | port: 80 |
20 | 20 | https: false |
21 | + # WARNING: This feature is no longer supported | |
21 | 22 | # Uncomment and customize to run in non-root path |
22 | 23 | # Note that ENV['RAILS_RELATIVE_URL_ROOT'] in config/puma.rb may need to be changed |
23 | 24 | # relative_url_root: /gitlab | ... | ... |
config/routes.rb
... | ... | @@ -284,12 +284,16 @@ Gitlab::Application.routes.draw do |
284 | 284 | end |
285 | 285 | end |
286 | 286 | |
287 | - resources :notes, only: [:index, :create, :destroy] do | |
287 | + resources :notes, only: [:index, :create, :destroy, :update] do | |
288 | + member do | |
289 | + delete :delete_attachment | |
290 | + end | |
291 | + | |
288 | 292 | collection do |
289 | 293 | post :preview |
290 | 294 | end |
291 | 295 | end |
292 | - end | |
296 | + end | |
293 | 297 | end |
294 | 298 | |
295 | 299 | root to: "dashboard#show" | ... | ... |
spec/factories.rb
1 | +include ActionDispatch::TestProcess | |
2 | + | |
1 | 3 | FactoryGirl.define do |
2 | 4 | sequence :sentence, aliases: [:title, :content] do |
3 | 5 | Faker::Lorem.sentence |
... | ... | @@ -127,6 +129,7 @@ FactoryGirl.define do |
127 | 129 | factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note] |
128 | 130 | factory :note_on_merge_request, traits: [:on_merge_request] |
129 | 131 | factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff] |
132 | + factory :note_on_merge_request_with_attachment, traits: [:on_merge_request, :with_attachment] | |
130 | 133 | |
131 | 134 | trait :on_commit do |
132 | 135 | project factory: :project_with_code |
... | ... | @@ -148,6 +151,10 @@ FactoryGirl.define do |
148 | 151 | noteable_id 1 |
149 | 152 | noteable_type "Issue" |
150 | 153 | end |
154 | + | |
155 | + trait :with_attachment do | |
156 | + attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png") } | |
157 | + end | |
151 | 158 | end |
152 | 159 | |
153 | 160 | factory :event do | ... | ... |
spec/features/notes_on_merge_requests_spec.rb
... | ... | @@ -3,6 +3,7 @@ require 'spec_helper' |
3 | 3 | describe "On a merge request", js: true do |
4 | 4 | let!(:project) { create(:project_with_code) } |
5 | 5 | let!(:merge_request) { create(:merge_request, project: project) } |
6 | + let!(:note) { create(:note_on_merge_request_with_attachment, project: project) } | |
6 | 7 | |
7 | 8 | before do |
8 | 9 | login_as :user |
... | ... | @@ -72,6 +73,71 @@ describe "On a merge request", js: true do |
72 | 73 | should_not have_css(".note") |
73 | 74 | end |
74 | 75 | end |
76 | + | |
77 | + describe "when editing a note", js: true do | |
78 | + it "should contain the hidden edit form" do | |
79 | + within("#note_#{note.id}") { should have_css(".note-edit-form", visible: false) } | |
80 | + end | |
81 | + | |
82 | + describe "editing the note" do | |
83 | + before do | |
84 | + find('.note').hover | |
85 | + find(".js-note-edit").click | |
86 | + end | |
87 | + | |
88 | + it "should show the note edit form and hide the note body" do | |
89 | + within("#note_#{note.id}") do | |
90 | + find(".note-edit-form", visible: true).should be_visible | |
91 | + find(".note-text", visible: false).should_not be_visible | |
92 | + end | |
93 | + end | |
94 | + | |
95 | + it "should reset the edit note form textarea with the original content of the note if cancelled" do | |
96 | + find('.note').hover | |
97 | + find(".js-note-edit").click | |
98 | + | |
99 | + within(".note-edit-form") do | |
100 | + fill_in "note[note]", with: "Some new content" | |
101 | + find(".btn-cancel").click | |
102 | + find(".js-note-text", visible: false).text.should == note.note | |
103 | + end | |
104 | + end | |
105 | + | |
106 | + it "appends the edited at time to the note" do | |
107 | + find('.note').hover | |
108 | + find(".js-note-edit").click | |
109 | + | |
110 | + within(".note-edit-form") do | |
111 | + fill_in "note[note]", with: "Some new content" | |
112 | + find(".btn-save").click | |
113 | + end | |
114 | + | |
115 | + within("#note_#{note.id}") do | |
116 | + should have_css(".note-last-update small") | |
117 | + find(".note-last-update small").text.should match(/Edited just now/) | |
118 | + end | |
119 | + end | |
120 | + end | |
121 | + | |
122 | + describe "deleting an attachment" do | |
123 | + before do | |
124 | + find('.note').hover | |
125 | + find(".js-note-edit").click | |
126 | + end | |
127 | + | |
128 | + it "shows the delete link" do | |
129 | + within(".note-attachment") do | |
130 | + should have_css(".js-note-attachment-delete") | |
131 | + end | |
132 | + end | |
133 | + | |
134 | + it "removes the attachment div and resets the edit form" do | |
135 | + find(".js-note-attachment-delete").click | |
136 | + should_not have_css(".note-attachment") | |
137 | + find(".note-edit-form", visible: false).should_not be_visible | |
138 | + end | |
139 | + end | |
140 | + end | |
75 | 141 | end |
76 | 142 | |
77 | 143 | describe "On a merge request diff", js: true, focus: true do | ... | ... |
1.12 KB
spec/routing/project_routing_spec.rb
... | ... | @@ -237,7 +237,7 @@ end |
237 | 237 | # project_snippet GET /:project_id/snippets/:id(.:format) snippets#show |
238 | 238 | # PUT /:project_id/snippets/:id(.:format) snippets#update |
239 | 239 | # DELETE /:project_id/snippets/:id(.:format) snippets#destroy |
240 | -describe Projects::SnippetsController, "routing" do | |
240 | +describe SnippetsController, "routing" do | |
241 | 241 | it "to #raw" do |
242 | 242 | get("/gitlabhq/snippets/1/raw").should route_to('projects/snippets#raw', project_id: 'gitlabhq', id: '1') |
243 | 243 | end | ... | ... |