From 55294ce60aeb416da285c273ef2d25572679a6c1 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Mon, 15 May 2017 15:07:43 +0000 Subject: Improve slash command stripping, escape temporary note contents --- spec/javascripts/notes_spec.js | 46 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) (limited to 'spec/javascripts/notes_spec.js') diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index be4605a5b89..8243a9c991a 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -377,7 +377,7 @@ import '~/notes'; }); it('should return true when comment begins with a slash command', () => { - const sampleComment = '/wip \n/milestone %1.0 \n/merge \n/unassign Merging this'; + const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign Merging this'; const hasSlashCommands = this.notes.hasSlashCommands(sampleComment); expect(hasSlashCommands).toBeTruthy(); @@ -401,10 +401,18 @@ import '~/notes'; describe('stripSlashCommands', () => { it('should strip slash commands from the comment which begins with a slash command', () => { this.notes = new Notes(); - const sampleComment = '/wip \n/milestone %1.0 \n/merge \n/unassign Merging this'; + const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign Merging this'; const stripedComment = this.notes.stripSlashCommands(sampleComment); - expect(stripedComment).not.toBe(sampleComment); + expect(stripedComment).toBe(''); + }); + + it('should strip slash commands from the comment but leaves plain comment if it is present', () => { + this.notes = new Notes(); + const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign\nMerging this'; + const stripedComment = this.notes.stripSlashCommands(sampleComment); + + expect(stripedComment).toBe('Merging this'); }); it('should NOT strip string that has slashes within', () => { @@ -424,6 +432,22 @@ import '~/notes'; beforeEach(() => { this.notes = new Notes('', []); + spyOn(_, 'escape').and.callFake((comment) => { + const escapedString = comment.replace(/["&'<>]/g, (a) => { + const escapedToken = { + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', + '`': '`' + }[a]; + + return escapedToken; + }); + + return escapedString; + }); }); it('should return constructed placeholder element for regular note based on form contents', () => { @@ -444,7 +468,21 @@ import '~/notes'; expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeFalsy(); expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(currentUserFullname); expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${currentUsername}`); - expect($tempNote.find('.note-body .note-text').text().trim()).toEqual(sampleComment); + expect($tempNote.find('.note-body .note-text p').text().trim()).toEqual(sampleComment); + }); + + it('should escape HTML characters from note based on form contents', () => { + const commentWithHtml = ''; + const $tempNote = this.notes.createPlaceholderNote({ + formContent: commentWithHtml, + uniqueId, + isDiscussionNote: false, + currentUsername, + currentUserFullname + }); + + expect(_.escape).toHaveBeenCalledWith(commentWithHtml); + expect($tempNote.find('.note-body .note-text p').html()).toEqual('<script>alert("Boom!");</script>'); }); it('should return constructed placeholder element for discussion note based on form contents', () => { -- cgit v1.2.1 From 1fe6beae47d99271b50c473082c4bac6daa0d0a3 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Thu, 20 Apr 2017 16:07:06 +0200 Subject: Notes: Warning message should go away once resolved Flash error messages disappear(`fadeOut`) automatically after any successful request --- spec/javascripts/notes_spec.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'spec/javascripts/notes_spec.js') diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index be4605a5b89..ae59ee787a8 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -14,6 +14,7 @@ import '~/notes'; gl.utils = gl.utils || {}; describe('Notes', function() { + const FLASH_TYPE_ALERT = 'alert'; var commentsTemplate = 'issues/issue_with_comment.html.raw'; preloadFixtures(commentsTemplate); @@ -460,5 +461,33 @@ import '~/notes'; expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeTruthy(); }); }); + + describe('appendFlash', () => { + beforeEach(() => { + this.notes = new Notes(); + }); + + it('shows a flash message', () => { + this.notes.addFlash('Error message', FLASH_TYPE_ALERT, this.notes.parentTimeline); + + expect(document.querySelectorAll('.flash-alert').length).toBe(1); + }); + }); + + describe('clearFlash', () => { + beforeEach(() => { + $(document).off('ajax:success'); + this.notes = new Notes(); + }); + + it('removes all the associated flash messages', () => { + this.notes.addFlash('Error message 1', FLASH_TYPE_ALERT, this.notes.parentTimeline); + this.notes.addFlash('Error message 2', FLASH_TYPE_ALERT, this.notes.parentTimeline); + + this.notes.clearFlash(); + + expect(document.querySelectorAll('.flash-alert').length).toBe(0); + }); + }); }); }).call(window); -- cgit v1.2.1 From f21a31559031c283cbc9cf99a11e98fa519ca0e5 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 18 May 2017 02:46:44 -0500 Subject: Fix missing .original-note-content and trailing new line edge case Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/32449 --- spec/javascripts/notes_spec.js | 60 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 8 deletions(-) (limited to 'spec/javascripts/notes_spec.js') diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 87745ea9817..7f12dea5277 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -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 = $('
'); $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 = $(`
initial
@@ -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 = $(`
initial
@@ -161,6 +164,47 @@ import '~/notes'; }); }); + describe('isUpdatedNote', () => { + it('should consider same note text as the same', () => { + const result = Notes.isUpdatedNote( + { + note: 'initial' + }, + $(`
+
initial
+
`) + ); + + expect(result).toEqual(false); + }); + + it('should consider same note with trailing newline as the same', () => { + const result = Notes.isUpdatedNote( + { + note: 'initial\n' + }, + $(`
+
initial\n
+
`) + ); + + expect(result).toEqual(false); + }); + + it('should consider different notes as different', () => { + const result = Notes.isUpdatedNote( + { + note: 'foo' + }, + $(`
+
bar
+
`) + ); + + 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); -- cgit v1.2.1 From 4b9c952d358b55aecd19fa3c3c6cedc91d823bba Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Thu, 18 May 2017 18:05:01 +0000 Subject: Fix ability to edit diff notes multiple times --- spec/javascripts/notes_spec.js | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) (limited to 'spec/javascripts/notes_spec.js') diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 7f12dea5277..d3494aaa94f 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -79,6 +79,47 @@ import '~/notes'; }); }); + describe('updateNote', () => { + let sampleComment; + let noteEntity; + let $form; + let $notesContainer; + + beforeEach(() => { + this.notes = new Notes('', []); + window.gon.current_username = 'root'; + window.gon.current_user_fullname = 'Administrator'; + sampleComment = 'foo'; + noteEntity = { + id: 1234, + html: `
  • +
    ${sampleComment}
    +
    • `, + note: sampleComment, + valid: true + }; + $form = $('form.js-main-target-form'); + $notesContainer = $('ul.main-notes-list'); + $form.find('textarea.js-note-text').val(sampleComment); + }); + + it('updates note and resets edit form', () => { + const deferred = $.Deferred(); + spyOn($, 'ajax').and.returnValue(deferred.promise()); + spyOn(this.notes, 'revertNoteEditForm'); + + $('.js-comment-button').click(); + deferred.resolve(noteEntity); + + const $targetNote = $notesContainer.find(`#note_${noteEntity.id}`); + const updatedNote = Object.assign({}, noteEntity); + updatedNote.note = 'bar'; + this.notes.updateNote(updatedNote, $targetNote); + + expect(this.notes.revertNoteEditForm).toHaveBeenCalledWith($targetNote); + }); + }); + describe('renderNote', () => { let notes; let note; -- cgit v1.2.1