Merge branch '277083-save-formatting-changes-in-separate-commit' into 'master'

Use a separate commit for formatting changes

See merge request gitlab-org/gitlab!49052

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

doc/user/project/static_site_editor/index.md

@@ -53,10 +53,15 @@ click of a button:
 
 ![Static Site Editor](img/wysiwyg_editor_v13_3.png)
 
-When an editor submits their changes, in the background, GitLab automatically
-creates a new branch, commits their changes, and opens a merge request. The
-editor lands directly on the merge request, and then they can assign it to
-a colleague for review.
+When an editor submits their changes, these are the following actions that GitLab
+performs automatically in the background:
+
+1. Creates a new branch.
+1. Commits their changes.
+   1. Fixes formatting according to the [Handbook Markdown Style Guide](https://about.gitlab.com/handbook/markdown-guide/) style guide and add them through another commit.
+1. Opens a merge request against the default branch.
+
+The editor can then navigate to the merge request to assign it to a colleague for review.
 
 ## Set up your project
 

locale/gitlab.pot

@@ -26234,6 +26234,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 ""
 
@@ -26252,6 +26255,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', () => {