Use a separate commit for formatting changes

When the Static Site Editor generates markdown from
the WYSIWYG editor, it does it following formatting
preferences that may be different from the original
preferences in the edited file

As a result, when the user creates a merge request
with the Static Site Editor, the merge request contains
many changes that result from generating markdown
with different formatting preferences. For example
the file may use a different syntax to set text as
bold from the one used in the editor

This change moves those formatting changes to a
separate commit

app/assets/javascripts/static_site_editor/components/edit_area.vue

@@ -60,6 +60,7 @@ export default {
   },
   data() {
     return {
+      formattedMarkdown: null,
       parsedSource: parseSourceFile(this.preProcess(true, this.content)),
       editorMode: EDITOR_TYPES.wysiwyg,
       hasMatter: false,
@@ -140,10 +141,14 @@ export default {
     onSubmit() {
       const preProcessedContent = this.preProcess(false, this.parsedSource.content());
       this.$emit('submit', {
+        formattedMarkdown: this.formattedMarkdown,
         content: preProcessedContent,
         images: this.$options.imageRepository.getAll(),
       });
     },
+    onEditorLoad({ formattedMarkdown }) {
+      this.formattedMarkdown = formattedMarkdown;
+    },
   },
 };
 </script>
@@ -167,6 +172,7 @@ export default {
       @modeChange="onModeChange"
       @input="onInputChange"
       @uploadImage="onUploadImage"
+      @load="onEditorLoad"
     />
     <unsaved-changes-confirm-dialog :modified="isSaveable" />
     <publish-toolbar

app/assets/javascripts/static_site_editor/constants.js

@@ -15,6 +15,14 @@ export const LOAD_CONTENT_ERROR = __(
   'An error ocurred while loading your content. Please try again.',
 );
 
+export const DEFAULT_FORMATTING_CHANGES_COMMIT_MESSAGE = s__(
+  'StaticSiteEditor|Automatic formatting changes',
+);
+
+export const DEFAULT_FORMATTING_CHANGES_COMMIT_DESCRIPTION = s__(
+  'StaticSiteEditor|Markdown formatting preferences introduced by the Static Site Editor',
+);
+
 export const DEFAULT_HEADING = s__('StaticSiteEditor|Static site editor');
 
 export const TRACKING_ACTION_CREATE_COMMIT = 'create_commit';

app/assets/javascripts/static_site_editor/graphql/resolvers/submit_content_changes.js

@@ -4,7 +4,17 @@ import savedContentMetaQuery from '../queries/saved_content_meta.query.graphql';
 
 const submitContentChangesResolver = (
   _,
-  { input: { project: projectId, username, sourcePath, content, images, mergeRequestMeta } },
+  {
+    input: {
+      project: projectId,
+      username,
+      sourcePath,
+      content,
+      images,
+      mergeRequestMeta,
+      formattedMarkdown,
+    },
+  },
   { cache },
 ) => {
   return submitContentChanges({
@@ -14,6 +24,7 @@ const submitContentChangesResolver = (
     content,
     images,
     mergeRequestMeta,
+    formattedMarkdown,
   }).then(savedContentMeta => {
     const data = produce(savedContentMeta, draftState => {
       return {

app/assets/javascripts/static_site_editor/pages/home.vue

@@ -53,6 +53,7 @@ export default {
     return {
       content: null,
       images: null,
+      formattedMarkdown: null,
       submitChangesError: null,
       isSavingChanges: false,
     };
@@ -79,9 +80,10 @@ export default {
     onDismissError() {
       this.submitChangesError = null;
     },
-    onPrepareSubmit({ content, images }) {
+    onPrepareSubmit({ formattedMarkdown, content, images }) {
       this.content = content;
       this.images = images;
+      this.formattedMarkdown = formattedMarkdown;
 
       this.isSavingChanges = true;
       this.$refs.editMetaModal.show();
@@ -110,6 +112,7 @@ export default {
               username: this.appData.username,
               sourcePath: this.appData.sourcePath,
               content: this.content,
+              formattedMarkdown: this.formattedMarkdown,
               images: this.images,
               mergeRequestMeta,
             },

app/assets/javascripts/static_site_editor/services/submit_content_changes.js

@@ -12,6 +12,8 @@ import {
   TRACKING_ACTION_CREATE_MERGE_REQUEST,
   USAGE_PING_TRACKING_ACTION_CREATE_COMMIT,
   USAGE_PING_TRACKING_ACTION_CREATE_MERGE_REQUEST,
+  DEFAULT_FORMATTING_CHANGES_COMMIT_MESSAGE,
+  DEFAULT_FORMATTING_CHANGES_COMMIT_DESCRIPTION,
 } from '../constants';
 
 const createBranch = (projectId, branch) =>
@@ -47,7 +49,15 @@ const createImageActions = (images, markdown) => {
   return actions;
 };
 
-const commitContent = (projectId, message, branch, sourcePath, content, images) => {
+const createUpdateSourceFileAction = (sourcePath, content) => [
+  convertObjectPropsToSnakeCase({
+    action: 'update',
+    filePath: sourcePath,
+    content,
+  }),
+];
+
+const commit = (projectId, message, branch, actions) => {
   Tracking.event(document.body.dataset.page, TRACKING_ACTION_CREATE_COMMIT);
   Api.trackRedisCounterEvent(USAGE_PING_TRACKING_ACTION_CREATE_COMMIT);
 
@@ -56,14 +66,7 @@ const commitContent = (projectId, message, branch, sourcePath, content, images)
     convertObjectPropsToSnakeCase({
       branch,
       commitMessage: message,
-      actions: [
-        convertObjectPropsToSnakeCase({
-          action: 'update',
-          filePath: sourcePath,
-          content,
-        }),
-        ...createImageActions(images, content),
-      ],
+      actions,
     }),
   ).catch(() => {
     throw new Error(SUBMIT_CHANGES_COMMIT_ERROR);
@@ -100,6 +103,7 @@ const submitContentChanges = ({
   content,
   images,
   mergeRequestMeta,
+  formattedMarkdown,
 }) => {
   const branch = generateBranchName(username);
   const { title: mergeRequestTitle, description: mergeRequestDescription } = mergeRequestMeta;
@@ -107,10 +111,25 @@ const submitContentChanges = ({
 
   return createBranch(projectId, branch)
     .then(({ data: { web_url: url } }) => {
+      const message = `${DEFAULT_FORMATTING_CHANGES_COMMIT_MESSAGE}\n\n${DEFAULT_FORMATTING_CHANGES_COMMIT_DESCRIPTION}`;
+
       Object.assign(meta, { branch: { label: branch, url } });
 
-      return commitContent(projectId, mergeRequestTitle, branch, sourcePath, content, images);
+      return formattedMarkdown
+        ? commit(
+            projectId,
+            message,
+            branch,
+            createUpdateSourceFileAction(sourcePath, formattedMarkdown),
+          )
+        : meta;
     })
+    .then(() =>
+      commit(projectId, mergeRequestTitle, branch, [
+        ...createUpdateSourceFileAction(sourcePath, content),
+        ...createImageActions(images, content),
+      ]),
+    )
     .then(({ data: { short_id: label, web_url: url } }) => {
       Object.assign(meta, { commit: { label, url } });
 

app/assets/javascripts/vue_shared/components/rich_content_editor/rich_content_editor.vue

@@ -105,6 +105,8 @@ export default {
       registerHTMLToMarkdownRenderer(editorApi);
 
       this.addListeners(editorApi);
+
+      this.$emit('load', { formattedMarkdown: editorApi.getMarkdown() });
     },
     onOpenAddImageModal() {
       this.$refs.addImageModal.show();

changelogs/unreleased/277083-save-formatting-changes-in-separate-commit.yml

+---
+title: Use a separate commit to store formatting changes in the Static Site Editor
+merge_request: 49052
+author:
+type: changed

locale/gitlab.pot

@@ -26222,6 +26222,9 @@ msgstr ""
 msgid "StaticSiteEditor|An error occurred while submitting your changes."
 msgstr ""
 
+msgid "StaticSiteEditor|Automatic formatting changes"
+msgstr ""
+
 msgid "StaticSiteEditor|Branch could not be created."
 msgstr ""
 
@@ -26240,6 +26243,9 @@ msgstr ""
 msgid "StaticSiteEditor|Incompatible file content"
 msgstr ""
 
+msgid "StaticSiteEditor|Markdown formatting preferences introduced by the Static Site Editor"
+msgstr ""
+
 msgid "StaticSiteEditor|Return to site"
 msgstr ""
 

spec/frontend/__mocks__/@toast-ui/vue-editor/index.js

+export const mockEditorApi = {
+  eventManager: {
+    addEventType: jest.fn(),
+    listen: jest.fn(),
+    removeEventHandler: jest.fn(),
+  },
+  getMarkdown: jest.fn(),
+};
+
 export const Editor = {
   props: {
     initialValue: {
@@ -18,14 +27,6 @@ export const Editor = {
     },
   },
   created() {
-    const mockEditorApi = {
-      eventManager: {
-        addEventType: jest.fn(),
-        listen: jest.fn(),
-        removeEventHandler: jest.fn(),
-      },
-    };
-
     this.$emit('load', mockEditorApi);
   },
   render(h) {

spec/frontend/static_site_editor/components/edit_area_spec.js

@@ -250,4 +250,17 @@ describe('~/static_site_editor/components/edit_area.vue', () => {
       expect(wrapper.emitted('submit').length).toBe(1);
     });
   });
+
+  describe('when RichContentEditor component triggers load event', () => {
+    it('stores formatted markdown provided in the event data', () => {
+      const data = { formattedMarkdown: 'formatted markdown' };
+
+      findRichContentEditor().vm.$emit('load', data);
+
+      // We can access the formatted markdown when submitting changes
+      findPublishToolbar().vm.$emit('submit');
+
+      expect(wrapper.emitted('submit')[0][0]).toMatchObject(data);
+    });
+  });
 });

spec/frontend/static_site_editor/pages/home_spec.js

@@ -235,6 +235,7 @@ describe('static_site_editor/pages/home', () => {
 
   describe('when submitting changes succeeds', () => {
     const newContent = `new ${content}`;
+    const formattedMarkdown = `formatted ${content}`;
 
     beforeEach(() => {
       mutateMock.mockResolvedValueOnce(hasSubmittedChangesMutationPayload).mockResolvedValueOnce({
@@ -243,7 +244,12 @@ describe('static_site_editor/pages/home', () => {
         },
       });
 
-      buildWrapper({ content: newContent, images });
+      buildWrapper();
+
+      findEditMetaModal().vm.show = jest.fn();
+
+      findEditArea().vm.$emit('submit', { content: newContent, images, formattedMarkdown });
+
       findEditMetaModal().vm.$emit('primary', mergeRequestMeta);
 
       return wrapper.vm.$nextTick();
@@ -266,6 +272,7 @@ describe('static_site_editor/pages/home', () => {
         variables: {
           input: {
             content: newContent,
+            formattedMarkdown,
             project,
             sourcePath,
             username,

spec/frontend/static_site_editor/services/submit_content_changes_spec.js

@@ -11,6 +11,8 @@ import {
   TRACKING_ACTION_CREATE_MERGE_REQUEST,
   USAGE_PING_TRACKING_ACTION_CREATE_COMMIT,
   USAGE_PING_TRACKING_ACTION_CREATE_MERGE_REQUEST,
+  DEFAULT_FORMATTING_CHANGES_COMMIT_MESSAGE,
+  DEFAULT_FORMATTING_CHANGES_COMMIT_DESCRIPTION,
 } from '~/static_site_editor/constants';
 import generateBranchName from '~/static_site_editor/services/generate_branch_name';
 import submitContentChanges from '~/static_site_editor/services/submit_content_changes';
@@ -81,6 +83,36 @@ describe('submitContentChanges', () => {
     );
   });
 
+  describe('committing markdown formatting changes', () => {
+    const formattedMarkdown = `formatted ${content}`;
+    const commitPayload = {
+      branch,
+      commit_message: `${DEFAULT_FORMATTING_CHANGES_COMMIT_MESSAGE}\n\n${DEFAULT_FORMATTING_CHANGES_COMMIT_DESCRIPTION}`,
+      actions: [
+        {
+          action: 'update',
+          file_path: sourcePath,
+          content: formattedMarkdown,
+        },
+      ],
+    };
+
+    it('commits markdown formatting changes in a separate commit', () => {
+      return submitContentChanges(buildPayload({ formattedMarkdown })).then(() => {
+        expect(Api.commitMultiple).toHaveBeenCalledWith(projectId, commitPayload);
+      });
+    });
+
+    it('does not commit markdown formatting changes when there are none', () => {
+      return submitContentChanges(buildPayload()).then(() => {
+        expect(Api.commitMultiple.mock.calls).toHaveLength(1);
+        expect(Api.commitMultiple.mock.calls[0][1]).not.toMatchObject({
+          actions: commitPayload.actions,
+        });
+      });
+    });
+  });
+
   it('commits the content changes to the branch when creating branch succeeds', () => {
     return submitContentChanges(buildPayload()).then(() => {
       expect(Api.commitMultiple).toHaveBeenCalledWith(projectId, {

spec/frontend/vue_shared/components/rich_content_editor/rich_content_editor_spec.js

 import { shallowMount } from '@vue/test-utils';
+import { mockEditorApi } from '@toast-ui/vue-editor';
 import RichContentEditor from '~/vue_shared/components/rich_content_editor/rich_content_editor.vue';
 import AddImageModal from '~/vue_shared/components/rich_content_editor/modals/add_image/add_image_modal.vue';
 import InsertVideoModal from '~/vue_shared/components/rich_content_editor/modals/insert_video_modal.vue';
@@ -114,10 +115,17 @@ describe('Rich Content Editor', () => {
   });
 
   describe('when editor is loaded', () => {
+    const formattedMarkdown = 'formatted markdown';
+
     beforeEach(() => {
+      mockEditorApi.getMarkdown.mockReturnValueOnce(formattedMarkdown);
       buildWrapper();
     });
 
+    afterEach(() => {
+      mockEditorApi.getMarkdown.mockReset();
+    });
+
     it('adds the CUSTOM_EVENTS.openAddImageModal custom event listener', () => {
       expect(addCustomEventListener).toHaveBeenCalledWith(
         wrapper.vm.editorApi,
@@ -137,6 +145,11 @@ describe('Rich Content Editor', () => {
     it('registers HTML to markdown renderer', () => {
       expect(registerHTMLToMarkdownRenderer).toHaveBeenCalledWith(wrapper.vm.editorApi);
     });
+
+    it('emits load event with the markdown formatted by Toast UI', () => {
+      expect(mockEditorApi.getMarkdown).toHaveBeenCalled();
+      expect(wrapper.emitted('load')[0]).toEqual([{ formattedMarkdown }]);
+    });
   });
 
   describe('when editor is destroyed', () => {