Merge branch 'ph/placeholderNoteRendering' into 'master'

Renders placeholder notes markdown as rendered HTML

See merge request gitlab-org/gitlab!65840

app/assets/javascripts/lib/utils/text_utility.js

@@ -421,3 +421,61 @@ export const isValidSha1Hash = (str) => {
 export function insertFinalNewline(content, endOfLine = '\n') {
   return content.slice(-endOfLine.length) !== endOfLine ? `${content}${endOfLine}` : content;
 }
+
+export const markdownConfig = {
+  // allowedTags from GitLab's inline HTML guidelines
+  // https://docs.gitlab.com/ee/user/markdown.html#inline-html
+  ALLOWED_TAGS: [
+    'a',
+    'abbr',
+    'b',
+    'blockquote',
+    'br',
+    'code',
+    'dd',
+    'del',
+    'div',
+    'dl',
+    'dt',
+    'em',
+    'h1',
+    'h2',
+    'h3',
+    'h4',
+    'h5',
+    'h6',
+    'hr',
+    'i',
+    'img',
+    'ins',
+    'kbd',
+    'li',
+    'ol',
+    'p',
+    'pre',
+    'q',
+    'rp',
+    'rt',
+    'ruby',
+    's',
+    'samp',
+    'span',
+    'strike',
+    'strong',
+    'sub',
+    'summary',
+    'sup',
+    'table',
+    'tbody',
+    'td',
+    'tfoot',
+    'th',
+    'thead',
+    'tr',
+    'tt',
+    'ul',
+    'var',
+  ],
+  ALLOWED_ATTR: ['class', 'style', 'href', 'src'],
+  ALLOW_DATA_ATTR: false,
+};

app/assets/javascripts/notebook/cells/markdown.vue

@@ -3,7 +3,7 @@
 import katex from 'katex';
 import marked from 'marked';
 import { sanitize } from '~/lib/dompurify';
-import { hasContent } from '~/lib/utils/text_utility';
+import { hasContent, markdownConfig } from '~/lib/utils/text_utility';
 import Prompt from './prompt.vue';
 
 const renderer = new marked.Renderer();
@@ -140,63 +140,7 @@ export default {
     markdown() {
       renderer.attachments = this.cell.attachments;
 
-      return sanitize(marked(this.cell.source.join('').replace(/\\/g, '\\\\')), {
-        // allowedTags from GitLab's inline HTML guidelines
-        // https://docs.gitlab.com/ee/user/markdown.html#inline-html
-        ALLOWED_TAGS: [
-          'a',
-          'abbr',
-          'b',
-          'blockquote',
-          'br',
-          'code',
-          'dd',
-          'del',
-          'div',
-          'dl',
-          'dt',
-          'em',
-          'h1',
-          'h2',
-          'h3',
-          'h4',
-          'h5',
-          'h6',
-          'hr',
-          'i',
-          'img',
-          'ins',
-          'kbd',
-          'li',
-          'ol',
-          'p',
-          'pre',
-          'q',
-          'rp',
-          'rt',
-          'ruby',
-          's',
-          'samp',
-          'span',
-          'strike',
-          'strong',
-          'sub',
-          'summary',
-          'sup',
-          'table',
-          'tbody',
-          'td',
-          'tfoot',
-          'th',
-          'thead',
-          'tr',
-          'tt',
-          'ul',
-          'var',
-        ],
-        ALLOWED_ATTR: ['class', 'style', 'href', 'src'],
-        ALLOW_DATA_ATTR: false,
-      });
+      return sanitize(marked(this.cell.source.join('').replace(/\\/g, '\\\\')), markdownConfig);
     },
   },
 };

app/assets/javascripts/notes/components/noteable_note.vue

@@ -13,6 +13,7 @@ import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_
 import eventHub from '../event_hub';
 import noteable from '../mixins/noteable';
 import resolvable from '../mixins/resolvable';
+import { renderMarkdown } from '../utils';
 import {
   getStartLineNumber,
   getEndLineNumber,
@@ -300,7 +301,7 @@ export default {
       this.isRequesting = true;
       this.oldContent = this.note.note_html;
       // eslint-disable-next-line vue/no-mutating-props
-      this.note.note_html = escape(noteText);
+      this.note.note_html = renderMarkdown(noteText);
 
       this.updateNote(data)
         .then(() => {

app/assets/javascripts/notes/stores/actions.js

@@ -312,25 +312,23 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
   $('.notes-form .flash-container').hide(); // hide previous flash notification
   commit(types.REMOVE_PLACEHOLDER_NOTES); // remove previous placeholders
 
-  if (replyId) {
-    if (hasQuickActions) {
-      placeholderText = utils.stripQuickActions(placeholderText);
-    }
+  if (hasQuickActions) {
+    placeholderText = utils.stripQuickActions(placeholderText);
+  }
 
-    if (placeholderText.length) {
-      commit(types.SHOW_PLACEHOLDER_NOTE, {
-        noteBody: placeholderText,
-        replyId,
-      });
-    }
+  if (placeholderText.length) {
+    commit(types.SHOW_PLACEHOLDER_NOTE, {
+      noteBody: placeholderText,
+      replyId,
+    });
+  }
 
-    if (hasQuickActions) {
-      commit(types.SHOW_PLACEHOLDER_NOTE, {
-        isSystemNote: true,
-        noteBody: utils.getQuickActionText(note),
-        replyId,
-      });
-    }
+  if (hasQuickActions) {
+    commit(types.SHOW_PLACEHOLDER_NOTE, {
+      isSystemNote: true,
+      noteBody: utils.getQuickActionText(note),
+      replyId,
+    });
   }
 
   const processQuickActions = (res) => {
@@ -400,9 +398,7 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
   };
 
   const removePlaceholder = (res) => {
-    if (replyId) {
-      commit(types.REMOVE_PLACEHOLDER_NOTES);
-    }
+    commit(types.REMOVE_PLACEHOLDER_NOTES);
 
     return res;
   };

app/assets/javascripts/notes/stores/getters.js

@@ -279,7 +279,7 @@ export const getDiscussion = (state) => (discussionId) =>
 export const commentsDisabled = (state) => state.commentsDisabled;
 
 export const suggestionsCount = (state, getters) =>
-  Object.values(getters.notesById).filter((n) => n.suggestions.length).length;
+  Object.values(getters.notesById).filter((n) => n.suggestions?.length).length;
 
 export const hasDrafts = (state, getters, rootState, rootGetters) =>
   Boolean(rootGetters['batchComments/hasDrafts']);

app/assets/javascripts/notes/utils.js

 /* eslint-disable @gitlab/require-i18n-strings */
+import marked from 'marked';
+import { sanitize } from '~/lib/dompurify';
+import { markdownConfig } from '~/lib/utils/text_utility';
 
 /**
  * Tracks snowplow event when User toggles timeline view
@@ -10,3 +13,7 @@ export const trackToggleTimelineView = (enabled) => ({
   label: 'Status',
   property: enabled,
 });
+
+export const renderMarkdown = (rawMarkdown) => {
+  return sanitize(marked(rawMarkdown), markdownConfig);
+};

app/assets/javascripts/vue_shared/components/notes/placeholder_note.vue

@@ -16,12 +16,15 @@
  *   :note="{body: 'This is a note'}"
  *   />
  */
+import { GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
 import { mapGetters } from 'vuex';
+import { renderMarkdown } from '~/notes/utils';
 import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
 import userAvatarLink from '../user_avatar/user_avatar_link.vue';
 
 export default {
   name: 'PlaceholderNote',
+  directives: { SafeHtml },
   components: {
     userAvatarLink,
     TimelineEntryItem,
@@ -34,6 +37,9 @@ export default {
   },
   computed: {
     ...mapGetters(['getUserData']),
+    renderedNote() {
+      return renderMarkdown(this.note.body);
+    },
   },
 };
 </script>
@@ -57,9 +63,7 @@ export default {
         </div>
       </div>
       <div class="note-body">
-        <div class="note-text md">
-          <p>{{ note.body }}</p>
-        </div>
+        <div v-safe-html="renderedNote" class="note-text md"></div>
       </div>
     </div>
   </timeline-entry-item>

spec/features/merge_request/batch_comments_spec.rb

@@ -13,6 +13,8 @@ RSpec.describe 'Merge request > Batch comments', :js do
   end
 
   before do
+    stub_feature_flags(paginated_notes: false)
+
     project.add_maintainer(user)
 
     sign_in(user)

spec/frontend/notes/components/noteable_note_spec.js

 import { mount } from '@vue/test-utils';
-import { escape } from 'lodash';
 import Vue from 'vue';
 import Vuex from 'vuex';
 
@@ -263,7 +262,9 @@ describe('issue_note', () => {
 
       await waitForPromises();
       expect(alertSpy).not.toHaveBeenCalled();
-      expect(wrapper.vm.note.note_html).toBe(escape(noteBody));
+      expect(wrapper.vm.note.note_html).toBe(
+        '<p><img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7"></p>\n',
+      );
     });
   });
 
@@ -291,7 +292,7 @@ describe('issue_note', () => {
       await wrapper.vm.$nextTick();
       let noteBodyProps = noteBody.props();
 
-      expect(noteBodyProps.note.note_html).toBe(updatedText);
+      expect(noteBodyProps.note.note_html).toBe(`<p>${updatedText}</p>\n`);
 
       noteBody.vm.$emit('cancelForm');
       await wrapper.vm.$nextTick();

spec/frontend/vue_shared/components/notes/__snapshots__/placeholder_note_spec.js.snap

@@ -55,6 +55,8 @@ exports[`Issue placeholder note component matches snapshot 1`] = `
         <p>
           Foo
         </p>
+        
+
       </div>
     </div>
   </div>