Improve user experience around slash commands in instant comments

1 files changed, 96 insertions, 37 deletions

diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js
index 17aa70ff3f1..24335614e09 100644
--- a/spec/javascripts/notes_spec.js
+++ b/spec/javascripts/notes_spec.js
@@ -13,6 +13,23 @@ import '~/notes';
   window.gl = window.gl || {};
   gl.utils = gl.utils || {};
 
+  const htmlEscape = (comment) => {
+    const escapedString = comment.replace(/["&'<>]/g, (a) => {
+      const escapedToken = {
+        '&': '&amp;',
+        '<': '&lt;',
+        '>': '&gt;',
+        '"': '&quot;',
+        "'": '&#x27;',
+        '`': '&#x60;'
+      }[a];
+
+      return escapedToken;
+    });
+
+    return escapedString;
+  };
+
   describe('Notes', function() {
     const FLASH_TYPE_ALERT = 'alert';
     var commentsTemplate = 'issues/issue_with_comment.html.raw';
@@ -445,11 +462,17 @@ import '~/notes';
     });
 
     describe('getFormData', () => {
-      it('should return form metadata object from form reference', () => {
+      let $form;
+      let sampleComment;
+
+      beforeEach(() => {
         this.notes = new Notes('', []);
 
-        const $form = $('form');
-        const sampleComment = 'foobar';
+        $form = $('form');
+        sampleComment = 'foobar';
+      });
+
+      it('should return form metadata object from form reference', () => {
         $form.find('textarea.js-note-text').val(sampleComment);
         const { formData, formContent, formAction } = this.notes.getFormData($form);
 
@@ -457,6 +480,18 @@ import '~/notes';
         expect(formContent).toEqual(sampleComment);
         expect(formAction).toEqual($form.attr('action'));
       });
+
+      it('should return form metadata with sanitized formContent from form reference', () => {
+        spyOn(_, 'escape').and.callFake(htmlEscape);
+
+        sampleComment = '<script>alert("Boom!");</script>';
+        $form.find('textarea.js-note-text').val(sampleComment);
+
+        const { formContent } = this.notes.getFormData($form);
+
+        expect(_.escape).toHaveBeenCalledWith(sampleComment);
+        expect(formContent).toEqual('&lt;script&gt;alert(&quot;Boom!&quot;);&lt;/script&gt;');
+      });
     });
 
     describe('hasSlashCommands', () => {
@@ -512,30 +547,42 @@ import '~/notes';
       });
     });
 
+    describe('getSlashCommandDescription', () => {
+      const availableSlashCommands = [
+        { name: 'close', description: 'Close this issue', params: [] },
+        { name: 'title', description: 'Change title', params: [{}] },
+        { name: 'estimate', description: 'Set time estimate', params: [{}] }
+      ];
+
+      beforeEach(() => {
+        this.notes = new Notes();
+      });
+
+      it('should return executing slash command description when note has single slash command', () => {
+        const sampleComment = '/close';
+        expect(this.notes.getSlashCommandDescription(sampleComment, availableSlashCommands)).toBe('Applying command to close this issue');
+      });
+
+      it('should return generic multiple slash command description when note has multiple slash commands', () => {
+        const sampleComment = '/close\n/title [Duplicate] Issue foobar';
+        expect(this.notes.getSlashCommandDescription(sampleComment, availableSlashCommands)).toBe('Applying multiple commands');
+      });
+
+      it('should return generic slash command description when available slash commands list is not populated', () => {
+        const sampleComment = '/close\n/title [Duplicate] Issue foobar';
+        expect(this.notes.getSlashCommandDescription(sampleComment)).toBe('Applying command');
+      });
+    });
+
     describe('createPlaceholderNote', () => {
       const sampleComment = 'foobar';
       const uniqueId = 'b1234-a4567';
       const currentUsername = 'root';
       const currentUserFullname = 'Administrator';
+      const currentUserAvatar = 'avatar_url';
 
       beforeEach(() => {
         this.notes = new Notes('', []);
-        spyOn(_, 'escape').and.callFake((comment) => {
-          const escapedString = comment.replace(/["&'<>]/g, (a) => {
-            const escapedToken = {
-              '&': '&amp;',
-              '<': '&lt;',
-              '>': '&gt;',
-              '"': '&quot;',
-              "'": '&#x27;',
-              '`': '&#x60;'
-            }[a];
-
-            return escapedToken;
-          });
-
-          return escapedString;
-        });
       });
 
       it('should return constructed placeholder element for regular note based on form contents', () => {
@@ -544,46 +591,59 @@ import '~/notes';
           uniqueId,
           isDiscussionNote: false,
           currentUsername,
-          currentUserFullname
+          currentUserFullname,
+          currentUserAvatar,
         });
         const $tempNoteHeader = $tempNote.find('.note-header');
 
         expect($tempNote.prop('nodeName')).toEqual('LI');
         expect($tempNote.attr('id')).toEqual(uniqueId);
+        expect($tempNote.hasClass('being-posted')).toBeTruthy();
+        expect($tempNote.hasClass('fade-in-half')).toBeTruthy();
         $tempNote.find('.timeline-icon > a, .note-header-info > a').each(function() {
           expect($(this).attr('href')).toEqual(`/${currentUsername}`);
         });
+        expect($tempNote.find('.timeline-icon .avatar').attr('src')).toEqual(currentUserAvatar);
         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 p').text().trim()).toEqual(sampleComment);
       });
 
-      it('should escape HTML characters from note based on form contents', () => {
-        const commentWithHtml = '<script>alert("Boom!");</script>';
+      it('should return constructed placeholder element for discussion note based on form contents', () => {
         const $tempNote = this.notes.createPlaceholderNote({
-          formContent: commentWithHtml,
+          formContent: sampleComment,
           uniqueId,
-          isDiscussionNote: false,
+          isDiscussionNote: true,
           currentUsername,
           currentUserFullname
         });
 
-        expect(_.escape).toHaveBeenCalledWith(commentWithHtml);
-        expect($tempNote.find('.note-body .note-text p').html()).toEqual('&lt;script&gt;alert("Boom!");&lt;/script&gt;');
+        expect($tempNote.prop('nodeName')).toEqual('LI');
+        expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeTruthy();
       });
+    });
 
-      it('should return constructed placeholder element for discussion note based on form contents', () => {
-        const $tempNote = this.notes.createPlaceholderNote({
-          formContent: sampleComment,
+    describe('createPlaceholderSystemNote', () => {
+      const sampleCommandDescription = 'Applying command to close this issue';
+      const uniqueId = 'b1234-a4567';
+
+      beforeEach(() => {
+        this.notes = new Notes('', []);
+        spyOn(_, 'escape').and.callFake(htmlEscape);
+      });
+
+      it('should return constructed placeholder element for system note based on form contents', () => {
+        const $tempNote = this.notes.createPlaceholderSystemNote({
+          formContent: sampleCommandDescription,
           uniqueId,
-          isDiscussionNote: true,
-          currentUsername,
-          currentUserFullname
         });
 
         expect($tempNote.prop('nodeName')).toEqual('LI');
-        expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeTruthy();
+        expect($tempNote.attr('id')).toEqual(uniqueId);
+        expect($tempNote.hasClass('being-posted')).toBeTruthy();
+        expect($tempNote.hasClass('fade-in-half')).toBeTruthy();
+        expect($tempNote.find('.timeline-content i').text().trim()).toEqual(sampleCommandDescription);
       });
     });
 
@@ -595,7 +655,7 @@ import '~/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);
+        expect($('.flash-alert').is(':visible')).toBeTruthy();
       });
     });
 
@@ -605,13 +665,12 @@ import '~/notes';
         this.notes = new Notes();
       });
 
-      it('removes all the associated flash messages', () => {
+      it('hides visible flash message', () => {
         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);
+        expect($('.flash-alert').is(':visible')).toBeFalsy();
       });
     });
   });