Fix missing .original-note-content and trailing new line edge case
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/32449
This commit is contained in:
Eric Eastwood
parent
319cab4115
commit
f21a315590
5 changed files with 156 additions and 75 deletions
|
|
@ -306,7 +306,7 @@ const normalizeNewlines = function(str) {
|
|||
}
|
||||
|
||||
const $note = $notesList.find(`#note_${noteEntity.id}`);
|
||||
if (this.isNewNote(noteEntity)) {
|
||||
if (Notes.isNewNote(noteEntity, this.note_ids)) {
|
||||
this.note_ids.push(noteEntity.id);
|
||||
|
||||
const $newNote = Notes.animateAppendNote(noteEntity.html, $notesList);
|
||||
|
|
@ -319,7 +319,7 @@ const normalizeNewlines = function(str) {
|
|||
return this.updateNotesCount(1);
|
||||
}
|
||||
// The server can send the same update multiple times so we need to make sure to only update once per actual update.
|
||||
else if (this.isUpdatedNote(noteEntity, $note)) {
|
||||
else if (Notes.isUpdatedNote(noteEntity, $note)) {
|
||||
const isEditing = $note.hasClass('is-editing');
|
||||
const initialContent = normalizeNewlines(
|
||||
$note.find('.original-note-content').text().trim()
|
||||
|
|
@ -347,23 +347,6 @@ const normalizeNewlines = function(str) {
|
|||
}
|
||||
};
|
||||
|
||||
/*
|
||||
Check if note does not exists on page
|
||||
*/
|
||||
|
||||
Notes.prototype.isNewNote = function(noteEntity) {
|
||||
return $.inArray(noteEntity.id, this.note_ids) === -1;
|
||||
};
|
||||
|
||||
Notes.prototype.isUpdatedNote = function(noteEntity, $note) {
|
||||
// There can be CRLF vs LF mismatches if we don't sanitize and compare the same way
|
||||
const sanitizedNoteNote = normalizeNewlines(noteEntity.note);
|
||||
const currentNoteText = normalizeNewlines(
|
||||
$note.find('.original-note-content').text().trim()
|
||||
);
|
||||
return sanitizedNoteNote !== currentNoteText;
|
||||
};
|
||||
|
||||
Notes.prototype.isParallelView = function() {
|
||||
return Cookies.get('diff_view') === 'parallel';
|
||||
};
|
||||
|
|
@ -376,7 +359,7 @@ const normalizeNewlines = function(str) {
|
|||
|
||||
Notes.prototype.renderDiscussionNote = function(noteEntity, $form) {
|
||||
var discussionContainer, form, row, lineType, diffAvatarContainer;
|
||||
if (!this.isNewNote(noteEntity)) {
|
||||
if (!Notes.isNewNote(noteEntity, this.note_ids)) {
|
||||
return;
|
||||
}
|
||||
this.note_ids.push(noteEntity.id);
|
||||
|
|
@ -1137,6 +1120,25 @@ const normalizeNewlines = function(str) {
|
|||
return $form;
|
||||
};
|
||||
|
||||
/**
|
||||
* Check if note does not exists on page
|
||||
*/
|
||||
Notes.isNewNote = function(noteEntity, noteIds) {
|
||||
return $.inArray(noteEntity.id, noteIds) === -1;
|
||||
};
|
||||
|
||||
/**
|
||||
* Check if $note already contains the `noteEntity` content
|
||||
*/
|
||||
Notes.isUpdatedNote = function(noteEntity, $note) {
|
||||
// There can be CRLF vs LF mismatches if we don't sanitize and compare the same way
|
||||
const sanitizedNoteEntityText = normalizeNewlines(noteEntity.note.trim());
|
||||
const currentNoteText = normalizeNewlines(
|
||||
$note.find('.original-note-content').text().trim()
|
||||
);
|
||||
return sanitizedNoteEntityText !== currentNoteText;
|
||||
};
|
||||
|
||||
Notes.checkMergeRequestStatus = function() {
|
||||
if (gl.utils.getPagePath(1) === 'merge_requests') {
|
||||
gl.mrWidget.checkStatus();
|
||||
|
|
|
|||
|
|
@ -1,3 +1 @@
|
|||
.original-note-content.hidden{ data: { post_url: note_url(note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } }
|
||||
#{note.note}
|
||||
%textarea.hidden.js-task-list-field.original-task-list{ data: {update_url: note_url(note) } }= note.note
|
||||
|
|
|
|||
|
|
@ -43,6 +43,8 @@
|
|||
.note-text.md
|
||||
= note.redacted_note_html
|
||||
= edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago')
|
||||
.original-note-content.hidden{ data: { post_url: note_url(note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } }
|
||||
#{note.note}
|
||||
- if note_editable
|
||||
= render 'shared/notes/edit', note: note
|
||||
.note-awards
|
||||
|
|
|
|||
|
|
@ -18,58 +18,93 @@ feature 'Issue notes polling', :feature, :js do
|
|||
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) }
|
||||
context 'when from own user' 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)
|
||||
before do
|
||||
login_as(user)
|
||||
visit namespace_project_issue_path(project.namespace, project, issue)
|
||||
end
|
||||
|
||||
it 'has .original-note-content to compare against' do
|
||||
expect(page).to have_selector("#note_#{existing_note.id}", text: note_text)
|
||||
expect(page).to have_selector("#note_#{existing_note.id} .original-note-content", visible: false)
|
||||
|
||||
update_note(existing_note, updated_text)
|
||||
|
||||
expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
|
||||
expect(page).to have_selector("#note_#{existing_note.id} .original-note-content", visible: false)
|
||||
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
|
||||
|
||||
it 'displays the updated content' do
|
||||
expect(page).to have_selector("#note_#{existing_note.id}", text: note_text)
|
||||
context 'when from another user' do
|
||||
let(:user1) { create(:user) }
|
||||
let(:user2) { create(:user) }
|
||||
let(:note_text) { "Hello World" }
|
||||
let(:updated_text) { "Bye World" }
|
||||
let!(:existing_note) { create(:note, noteable: issue, project: project, author: user1, note: note_text) }
|
||||
|
||||
update_note(existing_note, updated_text)
|
||||
before do
|
||||
login_as(user2)
|
||||
visit namespace_project_issue_path(project.namespace, project, issue)
|
||||
end
|
||||
|
||||
expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
|
||||
end
|
||||
it 'has .original-note-content to compare against' do
|
||||
expect(page).to have_selector("#note_#{existing_note.id}", text: note_text)
|
||||
expect(page).to have_selector("#note_#{existing_note.id} .original-note-content", visible: false)
|
||||
|
||||
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
|
||||
update_note(existing_note, updated_text)
|
||||
|
||||
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)
|
||||
expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
|
||||
expect(page).to have_selector("#note_#{existing_note.id} .original-note-content", visible: false)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -99,8 +99,6 @@ import '~/notes';
|
|||
|
||||
notes = jasmine.createSpyObj('notes', [
|
||||
'refresh',
|
||||
'isNewNote',
|
||||
'isUpdatedNote',
|
||||
'collapseLongCommitList',
|
||||
'updateNotesCount',
|
||||
'putConflictEditWarningInPlace'
|
||||
|
|
@ -110,13 +108,15 @@ import '~/notes';
|
|||
notes.updatedNotesTrackingMap = {};
|
||||
|
||||
spyOn(gl.utils, 'localTimeAgo');
|
||||
spyOn(Notes, 'isNewNote').and.callThrough();
|
||||
spyOn(Notes, 'isUpdatedNote').and.callThrough();
|
||||
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);
|
||||
|
||||
expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, $notesList);
|
||||
|
|
@ -125,7 +125,8 @@ import '~/notes';
|
|||
|
||||
describe('when note was edited', () => {
|
||||
it('should call .animateUpdateNote', () => {
|
||||
notes.isUpdatedNote.and.returnValue(true);
|
||||
Notes.isNewNote.and.returnValue(false);
|
||||
Notes.isUpdatedNote.and.returnValue(true);
|
||||
const $note = $('<div>');
|
||||
$notesList.find.and.returnValue($note);
|
||||
Notes.prototype.renderNote.call(notes, note, null, $notesList);
|
||||
|
|
@ -135,7 +136,8 @@ import '~/notes';
|
|||
|
||||
describe('while editing', () => {
|
||||
it('should update textarea if nothing has been touched', () => {
|
||||
notes.isUpdatedNote.and.returnValue(true);
|
||||
Notes.isNewNote.and.returnValue(false);
|
||||
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>
|
||||
|
|
@ -147,7 +149,8 @@ import '~/notes';
|
|||
});
|
||||
|
||||
it('should call .putConflictEditWarningInPlace', () => {
|
||||
notes.isUpdatedNote.and.returnValue(true);
|
||||
Notes.isNewNote.and.returnValue(false);
|
||||
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>
|
||||
|
|
@ -161,6 +164,47 @@ import '~/notes';
|
|||
});
|
||||
});
|
||||
|
||||
describe('isUpdatedNote', () => {
|
||||
it('should consider same note text as the same', () => {
|
||||
const result = Notes.isUpdatedNote(
|
||||
{
|
||||
note: 'initial'
|
||||
},
|
||||
$(`<div>
|
||||
<div class="original-note-content">initial</div>
|
||||
</div>`)
|
||||
);
|
||||
|
||||
expect(result).toEqual(false);
|
||||
});
|
||||
|
||||
it('should consider same note with trailing newline as the same', () => {
|
||||
const result = Notes.isUpdatedNote(
|
||||
{
|
||||
note: 'initial\n'
|
||||
},
|
||||
$(`<div>
|
||||
<div class="original-note-content">initial\n</div>
|
||||
</div>`)
|
||||
);
|
||||
|
||||
expect(result).toEqual(false);
|
||||
});
|
||||
|
||||
it('should consider different notes as different', () => {
|
||||
const result = Notes.isUpdatedNote(
|
||||
{
|
||||
note: 'foo'
|
||||
},
|
||||
$(`<div>
|
||||
<div class="original-note-content">bar</div>
|
||||
</div>`)
|
||||
);
|
||||
|
||||
expect(result).toEqual(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('renderDiscussionNote', () => {
|
||||
let discussionContainer;
|
||||
let note;
|
||||
|
|
@ -180,15 +224,15 @@ import '~/notes';
|
|||
row = jasmine.createSpyObj('row', ['prevAll', 'first', 'find']);
|
||||
|
||||
notes = jasmine.createSpyObj('notes', [
|
||||
'isNewNote',
|
||||
'isParallelView',
|
||||
'updateNotesCount',
|
||||
]);
|
||||
notes.note_ids = [];
|
||||
|
||||
spyOn(gl.utils, 'localTimeAgo');
|
||||
spyOn(Notes, 'isNewNote');
|
||||
spyOn(Notes, 'animateAppendNote');
|
||||
notes.isNewNote.and.returnValue(true);
|
||||
Notes.isNewNote.and.returnValue(true);
|
||||
notes.isParallelView.and.returnValue(false);
|
||||
row.prevAll.and.returnValue(row);
|
||||
row.first.and.returnValue(row);
|
||||
|
|
|
|||
Loading…
Reference in a new issue