Remove joinSentences in favor of HTML <br>

- joinSentences has a strong leaning towards English
and wouldn't work simply in other languages.
- Also clean up a few specs

app/assets/javascripts/ide/components/commit_sidebar/form.vue

 <script>
 import { mapState, mapActions, mapGetters } from 'vuex';
-import { GlModal } from '@gitlab/ui';
+import { GlModal, GlSafeHtmlDirective } from '@gitlab/ui';
 import { n__, __ } from '~/locale';
 import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import CommitMessageField from './message_field.vue';
@@ -18,6 +18,9 @@ export default {
     SuccessMessage,
     GlModal,
   },
+  directives: {
+    SafeHtml: GlSafeHtmlDirective,
+  },
   data() {
     return {
       isCompact: true,
@@ -189,7 +192,7 @@ export default {
           :action-cancel="{ text: __('Cancel') }"
           @ok="forceCreateNewBranch"
         >
-          {{ lastCommitError.message }}
+          <div v-safe-html="lastCommitError.messageHTML"></div>
         </gl-modal>
       </form>
     </transition>

app/assets/javascripts/ide/lib/errors.js

+import { escape } from 'lodash';
 import { __ } from '~/locale';
-import { joinSentences } from '~/lib/utils/text_utility';
 
 const CODEOWNERS_REGEX = /Push.*protected branches.*CODEOWNERS/;
 const BRANCH_CHANGED_REGEX = /changed.*since.*start.*edit/;
 
 export const createUnexpectedCommitError = () => ({
   title: __('Unexpected error'),
-  message: __('Could not commit. An unexpected error occurred.'),
+  messageHTML: __('Could not commit. An unexpected error occurred.'),
   canCreateBranch: false,
 });
 
 export const createCodeownersCommitError = message => ({
   title: __('CODEOWNERS rule violation'),
-  message,
+  messageHTML: escape(message),
   canCreateBranch: true,
 });
 
 export const createBranchChangedCommitError = message => ({
   title: __('Branch changed'),
-  message: joinSentences(message, __('Would you like to create a new branch?')),
+  messageHTML: `${escape(message)}<br/><br/>${__('Would you like to create a new branch?')}`,
   canCreateBranch: true,
 });
 

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

@@ -399,24 +399,3 @@ export const truncateNamespace = (string = '') => {
  * @returns {Boolean}
  */
 export const hasContent = obj => isString(obj) && obj.trim() !== '';
-
-/**
- * Joins the given sentences by adding periods if necessary.
- *
- * @param  {...string} sentences
- */
-export const joinSentences = (...sentences) =>
-  sentences.reduce((acc, sentence) => {
-    if (!sentence?.trim()) {
-      return acc;
-    } else if (!acc) {
-      return sentence;
-    } else if (/[.!?]\s*$/.test(acc)) {
-      const endsWithSpace = /\s$/.test(acc);
-      const sep = endsWithSpace ? '' : ' ';
-
-      return `${acc}${sep}${sentence}`;
-    }
-
-    return `${acc}. ${sentence}`;
-  }, '');

spec/frontend/ide/components/commit_sidebar/form_spec.js

@@ -270,7 +270,7 @@ describe('IDE commit form', () => {
         jest.spyOn(vm.$refs.commitErrorModal, 'show');
 
         const error = createError();
-        vm.$store.state.commit.commitError = error;
+        store.state.commit.commitError = error;
 
         await vm.$nextTick();
 
@@ -279,7 +279,9 @@ describe('IDE commit form', () => {
           actionCancel: { text: 'Cancel' },
           ...props,
         });
-        expect(document.body).toHaveText(error.message);
+        // Because of the legacy 'mountComponent' approach here, the only way to
+        // test the text of the modal is by viewing the content of the modal added to the document.
+        expect(document.body).toHaveText(error.messageHTML);
       });
     });
 
@@ -289,7 +291,7 @@ describe('IDE commit form', () => {
       });
 
       it('updates commit action and commits', async () => {
-        vm.$store.state.commit.commitError = createCodeownersCommitError('test message');
+        store.state.commit.commitError = createCodeownersCommitError('test message');
 
         await vm.$nextTick();
 

spec/frontend/ide/lib/errors_spec.js

@@ -5,9 +5,9 @@ import {
   parseCommitError,
 } from '~/ide/lib/errors';
 
-const TEST_MESSAGE = 'Test message';
-const TEST_MESSAGE_WITH_SENTENCE = 'Test message.';
-const TEST_MESSAGE_WITH_SENTENCE_AND_SPACE = 'Test message. ';
+const TEST_SPECIAL = '&special<';
+const TEST_SPECIAL_ESCAPED = '&amp;special&lt;';
+const TEST_MESSAGE = 'Test message.';
 const CODEOWNERS_MESSAGE =
   'Push to protected branches that contain changes to files matching CODEOWNERS is not allowed';
 const CHANGED_MESSAGE = 'Things changed since you started editing';
@@ -25,7 +25,15 @@ describe('~/ide/lib/errors', () => {
     it('uses given message', () => {
       expect(createCodeownersCommitError(TEST_MESSAGE)).toEqual({
         title: 'CODEOWNERS rule violation',
-        message: TEST_MESSAGE,
+        messageHTML: TEST_MESSAGE,
+        canCreateBranch: true,
+      });
+    });
+
+    it('escapes special chars', () => {
+      expect(createCodeownersCommitError(TEST_SPECIAL)).toEqual({
+        title: 'CODEOWNERS rule violation',
+        messageHTML: TEST_SPECIAL_ESCAPED,
         canCreateBranch: true,
       });
     });
@@ -33,14 +41,13 @@ describe('~/ide/lib/errors', () => {
 
   describe('createBranchChangedCommitError', () => {
     it.each`
-      message                                 | expectedMessage
-      ${TEST_MESSAGE}                         | ${`${TEST_MESSAGE}. Would you like to create a new branch?`}
-      ${TEST_MESSAGE_WITH_SENTENCE}           | ${`${TEST_MESSAGE}. Would you like to create a new branch?`}
-      ${TEST_MESSAGE_WITH_SENTENCE_AND_SPACE} | ${`${TEST_MESSAGE}. Would you like to create a new branch?`}
+      message         | expectedMessage
+      ${TEST_MESSAGE} | ${`${TEST_MESSAGE}<br/><br/>Would you like to create a new branch?`}
+      ${TEST_SPECIAL} | ${`${TEST_SPECIAL_ESCAPED}<br/><br/>Would you like to create a new branch?`}
     `('uses given message="$message"', ({ message, expectedMessage }) => {
       expect(createBranchChangedCommitError(message)).toEqual({
         title: 'Branch changed',
-        message: expectedMessage,
+        messageHTML: expectedMessage,
         canCreateBranch: true,
       });
     });

spec/frontend/ide/stores/modules/commit/actions_spec.js

@@ -545,10 +545,10 @@ describe('IDE commit module actions', () => {
         await store.dispatch('commit/commitChanges').catch(() => {});
 
         expect(store.commit.mock.calls).toEqual([
-          expect.arrayContaining(['commit/CLEAR_ERROR']),
-          expect.arrayContaining(['commit/UPDATE_LOADING', true]),
-          expect.arrayContaining(['commit/UPDATE_LOADING', false]),
-          expect.arrayContaining(['commit/SET_ERROR', createUnexpectedCommitError()]),
+          ['commit/CLEAR_ERROR', undefined, undefined],
+          ['commit/UPDATE_LOADING', true, undefined],
+          ['commit/UPDATE_LOADING', false, undefined],
+          ['commit/SET_ERROR', createUnexpectedCommitError(), undefined],
         ]);
       });
     });

spec/frontend/lib/utils/text_utility_spec.js

@@ -325,17 +325,4 @@ describe('text_utility', () => {
       expect(textUtils.hasContent(txt)).toEqual(result);
     });
   });
-
-  describe('joinSentences', () => {
-    it.each`
-      input                                 | output
-      ${[]}                                 | ${''}
-      ${['Lorem ipsum']}                    | ${'Lorem ipsum'}
-      ${['Lorem ipsum', null, 'Dolar sit']} | ${'Lorem ipsum. Dolar sit'}
-      ${['Lorem ipsum!', 'Dolar sit']}      | ${'Lorem ipsum! Dolar sit'}
-      ${['Lorem ipsum? ', 'Dolar sit']}     | ${'Lorem ipsum? Dolar sit'}
-    `('joins the sentences with periods ($input)', ({ input, output }) => {
-      expect(textUtils.joinSentences(...input)).toBe(output);
-    });
-  });
 });