Commit df1c933e authored by Jacob Schatz's avatar Jacob Schatz

Merge branch '30458-real-time-note-edits' into 'master'

Add real-time note edits 🐿

Closes #30458

See merge request !10837
parents e14ca539 cf34b07e
...@@ -34,9 +34,9 @@ GLForm.prototype.setupForm = function() { ...@@ -34,9 +34,9 @@ GLForm.prototype.setupForm = function() {
gl.GfmAutoComplete.setup(this.form.find('.js-gfm-input')); gl.GfmAutoComplete.setup(this.form.find('.js-gfm-input'));
new DropzoneInput(this.form); new DropzoneInput(this.form);
autosize(this.textarea); autosize(this.textarea);
}
// form and textarea event listeners // form and textarea event listeners
this.addEventListeners(); this.addEventListeners();
}
gl.text.init(this.form); gl.text.init(this.form);
// hide discard button // hide discard button
this.form.find('.js-note-discard').hide(); this.form.find('.js-note-discard').hide();
......
This diff is collapsed.
...@@ -67,7 +67,7 @@ ul.notes { ...@@ -67,7 +67,7 @@ ul.notes {
} }
} }
&.is-editting { &.is-editing {
.note-header, .note-header,
.note-text, .note-text,
.edited-text { .edited-text {
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
= render 'projects/notes/hints' = render 'projects/notes/hints'
.note-form-actions.clearfix .note-form-actions.clearfix
.settings-message.note-edit-warning.js-edit-warning .settings-message.note-edit-warning.js-finish-edit-warning
Finish editing this message first! Finish editing this message first!
= submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-button' = submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-button'
%button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' } %button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' }
......
...@@ -2,7 +2,11 @@ ...@@ -2,7 +2,11 @@
- return if note.cross_reference_not_visible_for?(current_user) - return if note.cross_reference_not_visible_for?(current_user)
- note_editable = note_editable?(note) - note_editable = note_editable?(note)
%li.timeline-entry{ id: dom_id(note), class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable, note_id: note.id} } %li.timeline-entry{ id: dom_id(note),
class: ["note", "note-row-#{note.id}", ('system-note' if note.system)],
data: { author_id: note.author.id,
editable: note_editable,
note_id: note.id } }
.timeline-entry-inner .timeline-entry-inner
.timeline-icon .timeline-icon
- if note.system - if note.system
......
---
title: Update note edits in real-time
merge_request:
author:
...@@ -62,6 +62,8 @@ describe "GitLab Flavored Markdown", feature: true do ...@@ -62,6 +62,8 @@ describe "GitLab Flavored Markdown", feature: true do
project: project, project: project,
title: "fix #{@other_issue.to_reference}", title: "fix #{@other_issue.to_reference}",
description: "ask #{fred.to_reference} for details") description: "ask #{fred.to_reference} for details")
@note = create(:note_on_issue, noteable: @issue, project: @issue.project, note: "Hello world")
end end
it "renders subject in issues#index" do it "renders subject in issues#index" do
...@@ -81,14 +83,6 @@ describe "GitLab Flavored Markdown", feature: true do ...@@ -81,14 +83,6 @@ describe "GitLab Flavored Markdown", feature: true do
expect(page).to have_link(fred.to_reference) expect(page).to have_link(fred.to_reference)
end end
it "renders updated subject once edited somewhere else in issues#show" do
visit namespace_project_issue_path(project.namespace, project, @issue)
@issue.update(title: "fix #{@other_issue.to_reference} and update")
wait_for_vue_resource
expect(page).to have_text("fix #{@other_issue.to_reference} and update")
end
end end
describe "for merge requests" do describe "for merge requests" do
......
...@@ -4,14 +4,77 @@ feature 'Issue notes polling', :feature, :js do ...@@ -4,14 +4,77 @@ feature 'Issue notes polling', :feature, :js do
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :public) }
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
describe 'creates' do
before do before do
visit namespace_project_issue_path(project.namespace, project, issue) visit namespace_project_issue_path(project.namespace, project, issue)
end end
it 'should display the new comment' do it 'displays the new comment' do
note = create(:note, noteable: issue, project: project, note: 'Looks good!') note = create(:note, noteable: issue, project: project, note: 'Looks good!')
page.execute_script('notes.refresh();') page.execute_script('notes.refresh();')
expect(page).to have_selector("#note_#{note.id}", text: 'Looks good!') expect(page).to have_selector("#note_#{note.id}", text: 'Looks good!')
end end
end
describe 'updates' do
let(:user) { create(:user) }
let(:note_text) { "Hello World" }
let(:updated_text) { "Bye World" }
let!(:existing_note) { create(:note, noteable: issue, project: project, author: user, note: note_text) }
before do
login_as(user)
visit namespace_project_issue_path(project.namespace, project, issue)
end
it 'displays the updated content' do
expect(page).to have_selector("#note_#{existing_note.id}", text: note_text)
update_note(existing_note, updated_text)
expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
end
it 'when editing but have not changed anything, and an update comes in, show the updated content in the textarea' do
find("#note_#{existing_note.id} .js-note-edit").click
expect(page).to have_field("note[note]", with: note_text)
update_note(existing_note, updated_text)
expect(page).to have_field("note[note]", with: updated_text)
end
it 'when editing but you changed some things, and an update comes in, show a warning' do
find("#note_#{existing_note.id} .js-note-edit").click
expect(page).to have_field("note[note]", with: note_text)
find("#note_#{existing_note.id} .js-note-text").set('something random')
update_note(existing_note, updated_text)
expect(page).to have_selector(".alert")
end
it 'when editing but you changed some things, an update comes in, and you press cancel, show the updated content' do
find("#note_#{existing_note.id} .js-note-edit").click
expect(page).to have_field("note[note]", with: note_text)
find("#note_#{existing_note.id} .js-note-text").set('something random')
update_note(existing_note, updated_text)
find("#note_#{existing_note.id} .note-edit-cancel").click
expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
end
end
def update_note(note, new_text)
note.update(note: new_text)
page.execute_script('notes.refresh();')
end
end end
/* eslint-disable space-before-function-paren, no-unused-expressions, no-var, object-shorthand, comma-dangle, max-len */ /* eslint-disable space-before-function-paren, no-unused-expressions, no-var, object-shorthand, comma-dangle, max-len */
/* global Notes */ /* global Notes */
require('~/notes'); import 'vendor/autosize';
require('vendor/autosize'); import '~/gl_form';
require('~/gl_form'); import '~/lib/utils/text_utility';
require('~/lib/utils/text_utility'); import '~/render_gfm';
import '~/render_math';
import '~/notes';
(function() { (function() {
window.gon || (window.gon = {}); window.gon || (window.gon = {});
...@@ -80,35 +82,78 @@ require('~/lib/utils/text_utility'); ...@@ -80,35 +82,78 @@ require('~/lib/utils/text_utility');
beforeEach(() => { beforeEach(() => {
note = { note = {
id: 1,
discussion_html: null, discussion_html: null,
valid: true, valid: true,
html: '<div></div>', note: 'heya',
html: '<div>heya</div>',
}; };
$notesList = jasmine.createSpyObj('$notesList', ['find']); $notesList = jasmine.createSpyObj('$notesList', [
'find',
'append',
]);
notes = jasmine.createSpyObj('notes', [ notes = jasmine.createSpyObj('notes', [
'refresh', 'refresh',
'isNewNote', 'isNewNote',
'isUpdatedNote',
'collapseLongCommitList', 'collapseLongCommitList',
'updateNotesCount', 'updateNotesCount',
'putConflictEditWarningInPlace'
]); ]);
notes.taskList = jasmine.createSpyObj('tasklist', ['init']); notes.taskList = jasmine.createSpyObj('tasklist', ['init']);
notes.note_ids = []; notes.note_ids = [];
notes.updatedNotesTrackingMap = {};
spyOn(window, '$').and.returnValue($notesList);
spyOn(gl.utils, 'localTimeAgo'); spyOn(gl.utils, 'localTimeAgo');
spyOn(Notes, 'animateAppendNote'); spyOn(Notes, 'animateAppendNote').and.callThrough();
spyOn(Notes, 'animateUpdateNote').and.callThrough();
});
describe('when adding note', () => {
it('should call .animateAppendNote', () => {
notes.isNewNote.and.returnValue(true); notes.isNewNote.and.returnValue(true);
Notes.prototype.renderNote.call(notes, note, null, $notesList);
Notes.prototype.renderNote.call(notes, note); expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, $notesList);
});
}); });
it('should query for the notes list', () => { describe('when note was edited', () => {
expect(window.$).toHaveBeenCalledWith('ul.main-notes-list'); it('should call .animateUpdateNote', () => {
notes.isUpdatedNote.and.returnValue(true);
const $note = $('<div>');
$notesList.find.and.returnValue($note);
Notes.prototype.renderNote.call(notes, note, null, $notesList);
expect(Notes.animateUpdateNote).toHaveBeenCalledWith(note.html, $note);
}); });
it('should call .animateAppendNote', () => { describe('while editing', () => {
expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, $notesList); it('should update textarea if nothing has been touched', () => {
notes.isUpdatedNote.and.returnValue(true);
const $note = $(`<div class="is-editing">
<div class="original-note-content">initial</div>
<textarea class="js-note-text">initial</textarea>
</div>`);
$notesList.find.and.returnValue($note);
Notes.prototype.renderNote.call(notes, note, null, $notesList);
expect($note.find('.js-note-text').val()).toEqual(note.note);
});
it('should call .putConflictEditWarningInPlace', () => {
notes.isUpdatedNote.and.returnValue(true);
const $note = $(`<div class="is-editing">
<div class="original-note-content">initial</div>
<textarea class="js-note-text">different</textarea>
</div>`);
$notesList.find.and.returnValue($note);
Notes.prototype.renderNote.call(notes, note, null, $notesList);
expect(notes.putConflictEditWarningInPlace).toHaveBeenCalledWith(note, $note);
});
});
}); });
}); });
...@@ -147,14 +192,12 @@ require('~/lib/utils/text_utility'); ...@@ -147,14 +192,12 @@ require('~/lib/utils/text_utility');
}); });
describe('Discussion root note', () => { describe('Discussion root note', () => {
let $notesList;
let body; let body;
beforeEach(() => { beforeEach(() => {
body = jasmine.createSpyObj('body', ['attr']); body = jasmine.createSpyObj('body', ['attr']);
discussionContainer = { length: 0 }; discussionContainer = { length: 0 };
spyOn(window, '$').and.returnValues(discussionContainer, body, $notesList);
$form.closest.and.returnValues(row, $form); $form.closest.and.returnValues(row, $form);
$form.find.and.returnValues(discussionContainer); $form.find.and.returnValues(discussionContainer);
body.attr.and.returnValue(''); body.attr.and.returnValue('');
...@@ -162,12 +205,8 @@ require('~/lib/utils/text_utility'); ...@@ -162,12 +205,8 @@ require('~/lib/utils/text_utility');
Notes.prototype.renderDiscussionNote.call(notes, note, $form); Notes.prototype.renderDiscussionNote.call(notes, note, $form);
}); });
it('should query for the notes list', () => {
expect(window.$.calls.argsFor(2)).toEqual(['ul.main-notes-list']);
});
it('should call Notes.animateAppendNote', () => { it('should call Notes.animateAppendNote', () => {
expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.discussion_html, $notesList); expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.discussion_html, $('.main-notes-list'));
}); });
}); });
...@@ -175,16 +214,12 @@ require('~/lib/utils/text_utility'); ...@@ -175,16 +214,12 @@ require('~/lib/utils/text_utility');
beforeEach(() => { beforeEach(() => {
discussionContainer = { length: 1 }; discussionContainer = { length: 1 };
spyOn(window, '$').and.returnValues(discussionContainer); $form.closest.and.returnValues(row, $form);
$form.closest.and.returnValues(row); $form.find.and.returnValues(discussionContainer);
Notes.prototype.renderDiscussionNote.call(notes, note, $form); Notes.prototype.renderDiscussionNote.call(notes, note, $form);
}); });
it('should query foor the discussion container', () => {
expect(window.$).toHaveBeenCalledWith(`.notes[data-discussion-id="${note.discussion_id}"]`);
});
it('should call Notes.animateAppendNote', () => { it('should call Notes.animateAppendNote', () => {
expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, discussionContainer); expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, discussionContainer);
}); });
...@@ -193,35 +228,45 @@ require('~/lib/utils/text_utility'); ...@@ -193,35 +228,45 @@ require('~/lib/utils/text_utility');
describe('animateAppendNote', () => { describe('animateAppendNote', () => {
let noteHTML; let noteHTML;
let $note;
let $notesList; let $notesList;
let $resultantNote;
beforeEach(() => { beforeEach(() => {
noteHTML = '<div></div>'; noteHTML = '<div></div>';
$note = jasmine.createSpyObj('$note', ['addClass', 'renderGFM', 'removeClass']);
$notesList = jasmine.createSpyObj('$notesList', ['append']); $notesList = jasmine.createSpyObj('$notesList', ['append']);
spyOn(window, '$').and.returnValue($note); $resultantNote = Notes.animateAppendNote(noteHTML, $notesList);
spyOn(window, 'setTimeout').and.callThrough(); });
$note.addClass.and.returnValue($note);
$note.renderGFM.and.returnValue($note);
Notes.animateAppendNote(noteHTML, $notesList); it('should have `fade-in` class', () => {
expect($resultantNote.hasClass('fade-in')).toEqual(true);
}); });
it('should init the note jquery object', () => { it('should append note to the notes list', () => {
expect(window.$).toHaveBeenCalledWith(noteHTML); expect($notesList.append).toHaveBeenCalledWith($resultantNote);
});
}); });
it('should call addClass', () => { describe('animateUpdateNote', () => {
expect($note.addClass).toHaveBeenCalledWith('fade-in'); let noteHTML;
let $note;
let $updatedNote;
beforeEach(() => {
noteHTML = '<div></div>';
$note = jasmine.createSpyObj('$note', [
'replaceWith'
]);
$updatedNote = Notes.animateUpdateNote(noteHTML, $note);
}); });
it('should call renderGFM', () => {
expect($note.renderGFM).toHaveBeenCalledWith(); it('should have `fade-in` class', () => {
expect($updatedNote.hasClass('fade-in')).toEqual(true);
}); });
it('should append note to the notes list', () => { it('should call replaceWith on $note', () => {
expect($notesList.append).toHaveBeenCalledWith($note); expect($note.replaceWith).toHaveBeenCalledWith($updatedNote);
}); });
}); });
}); });
......
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