From 8e6b6528543818a779e1df2b38150eb422145fb0 Mon Sep 17 00:00:00 2001
From: Simon Knox <psimyn@gmail.com>
Date: Tue, 2 Apr 2019 19:27:38 +1100
Subject: Fix webpack dev-server crash due to memory limit

Remove unneeded var for webpack-prod call in gitlab-ci
---
 .gitlab-ci.yml | 3 +--
 package.json   | 6 +++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ab38c87039e..7e38952358b 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -852,8 +852,6 @@ qa:selectors:
 .qa-frontend-node: &qa-frontend-node
   <<: *dedicated-no-docs-no-db-pull-cache-job
   stage: test
-  variables:
-    NODE_OPTIONS: --max_old_space_size=3584
   cache:
     key: "$CI_JOB_NAME"
     paths:
@@ -1145,3 +1143,4 @@ schedule:review-performance:
   <<: *review-schedules-only
   script:
     - wait_for_job_to_be_done "schedule:review-deploy"
+
diff --git a/package.json b/package.json
index 90fcfe01438..a83d5e233bf 100644
--- a/package.json
+++ b/package.json
@@ -2,7 +2,7 @@
   "private": true,
   "scripts": {
     "clean": "rm -rf public/assets tmp/cache/*-loader",
-    "dev-server": "nodemon -w 'config/webpack.config.js' --exec 'webpack-dev-server --config config/webpack.config.js'",
+    "dev-server": "NODE_OPTIONS=\"--max-old-space-size=3584\" nodemon -w 'config/webpack.config.js' --exec 'webpack-dev-server --config config/webpack.config.js'",
     "eslint": "eslint --max-warnings 0 --report-unused-disable-directives --ext .js,.vue .",
     "eslint-fix": "eslint --max-warnings 0 --report-unused-disable-directives --ext .js,.vue --fix .",
     "eslint-report": "eslint --max-warnings 0 --ext .js,.vue --format html --output-file ./eslint-report.html --no-inline-config .",
@@ -20,8 +20,8 @@
     "stylelint-file": "node node_modules/stylelint/bin/stylelint.js",
     "stylelint-create-utility-map": "node scripts/frontend/stylelint/stylelint-utility-map.js",
     "test": "yarn jest && yarn karma",
-    "webpack": "webpack --config config/webpack.config.js",
-    "webpack-prod": "NODE_ENV=production webpack --config config/webpack.config.js"
+    "webpack": "NODE_OPTIONS=\"--max-old-space-size=3584\" webpack --config config/webpack.config.js",
+    "webpack-prod": "NODE_OPTIONS=\"--max-old-space-size=3584\" NODE_ENV=production webpack --config config/webpack.config.js"
   },
   "dependencies": {
     "@babel/core": "^7.2.2",
-- 
cgit v1.2.1


From 1c8e99be48551b8b04791166886d2b2ac86d83d3 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Thu, 28 Mar 2019 18:07:27 +0700
Subject: Ignore merge if the status of the merge request pipeline is stale

Merge request pipeline is meant for ensuring target branch's pipeline
green. We should not let maintainers merge a merge request if the head
pipeline of the merge request doesn't fulfill the criteria.
---
 spec/requests/api/merge_requests_spec.rb | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 4259fda7f04..73d4072b9d1 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -1353,7 +1353,12 @@ describe API::MergeRequests do
     end
 
     it 'returns 405 if the build failed for a merge request that requires success' do
-      allow_any_instance_of(MergeRequest).to receive(:mergeable_ci_state?).and_return(false)
+      project.update!(only_allow_merge_if_pipeline_succeeds: true)
+
+      create(:ci_pipeline,
+             :failed,
+             sha: merge_request.diff_head_sha,
+             merge_requests_as_head_pipeline: [merge_request])
 
       put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
 
-- 
cgit v1.2.1


From e1850718034bceaf9ef521d21592cb1d8d84cb6a Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Thu, 4 Apr 2019 08:07:36 +0000
Subject: Revert "Merge branch 'fix/missing-border' into 'master'"

This reverts merge request !26242
---
 app/assets/stylesheets/pages/notes.scss      | 1 -
 changelogs/unreleased/fix-missing-border.yml | 5 -----
 2 files changed, 6 deletions(-)
 delete mode 100644 changelogs/unreleased/fix-missing-border.yml

diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index 0c334e919de..fd07415a52f 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -44,7 +44,6 @@ $note-form-margin-left: 72px;
     border: 1px solid $border-color;
     border-radius: $border-radius-default;
     margin: $gl-padding 0;
-    overflow: auto;
 
     &.system-note,
     &.note-form {
diff --git a/changelogs/unreleased/fix-missing-border.yml b/changelogs/unreleased/fix-missing-border.yml
deleted file mode 100644
index 21728223cb8..00000000000
--- a/changelogs/unreleased/fix-missing-border.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Fixes missing border color in discussion card component
-merge_request: 26242
-author: Farhad Yasir
-type: fixed
-- 
cgit v1.2.1


From ec85debaf51067cc78d54188ec1eef94342d5a8b Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Tue, 2 Apr 2019 12:52:28 +0100
Subject: Speed up avatar URLs with object storage

With object storage enabled, calling `#filename` on an upload does this:

1. Call the `#filename` method on the CarrierWave object.
2. Generate the URL for that object.
3. If the uploader isn't public, do so by generating an authenticated
   URL, including signing that request.

That's all correct behaviour, but for the case where we use `#filename`,
it's typically to generate a GitLab URL. That URL doesn't need to be
signed because we do our own auth.

Signing the URLs can be very expensive, especially in batch (say, we
need to get the avatar URLs for 150 users in one request). It's all
unnecessary work. If we used the `RecordsUploads` concern, we have
already recorded a `path` in the database. That `path` is actually
generated from CarrierWave's `#filename` at upload time, so we don't
need to recompute it - we can just use it and strip off the prefix if
it's available.

On a sample users autocomplete URL, at least 10% of the time before this
change went to signing URLs. After this change, we spend no time in URL
signing, and still get the correct results.
---
 app/uploaders/records_uploads.rb                    | 4 ++++
 changelogs/unreleased/stop-signing-avatar-paths.yml | 5 +++++
 spec/uploaders/records_uploads_spec.rb              | 9 +++++++++
 3 files changed, 18 insertions(+)
 create mode 100644 changelogs/unreleased/stop-signing-avatar-paths.yml

diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb
index 9a243e07936..00b51f92b12 100644
--- a/app/uploaders/records_uploads.rb
+++ b/app/uploaders/records_uploads.rb
@@ -46,6 +46,10 @@ module RecordsUploads
       File.join(store_dir, filename.to_s)
     end
 
+    def filename
+      upload&.path ? File.basename(upload.path) : super
+    end
+
     private
 
     # rubocop: disable CodeReuse/ActiveRecord
diff --git a/changelogs/unreleased/stop-signing-avatar-paths.yml b/changelogs/unreleased/stop-signing-avatar-paths.yml
new file mode 100644
index 00000000000..2c2493f0f21
--- /dev/null
+++ b/changelogs/unreleased/stop-signing-avatar-paths.yml
@@ -0,0 +1,5 @@
+---
+title: Speed up generation of avatar URLs when using object storage
+merge_request:
+author:
+type: performance
diff --git a/spec/uploaders/records_uploads_spec.rb b/spec/uploaders/records_uploads_spec.rb
index 3592a11360d..ab98976ec27 100644
--- a/spec/uploaders/records_uploads_spec.rb
+++ b/spec/uploaders/records_uploads_spec.rb
@@ -94,4 +94,13 @@ describe RecordsUploads do
       expect { uploader.remove! }.to change { Upload.count }.from(1).to(0)
     end
   end
+
+  describe '#filename' do
+    it 'gets the filename from the path recorded in the database, not CarrierWave' do
+      uploader.store!(upload_fixture('rails_sample.jpg'))
+      expect_any_instance_of(GitlabUploader).not_to receive(:filename)
+
+      expect(uploader.filename).to eq('rails_sample.jpg')
+    end
+  end
 end
-- 
cgit v1.2.1


From fee7036394c881aa5a1f9851b3531267104e8c1d Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 4 Apr 2019 13:26:01 +0200
Subject: Exempt release automation MRs from Danger rules

---
 Dangerfile                  | 33 ++++++++++++++++++---------------
 lib/gitlab/danger/helper.rb |  5 +++++
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/Dangerfile b/Dangerfile
index 95dd48aae9e..3e8cb456003 100644
--- a/Dangerfile
+++ b/Dangerfile
@@ -1,16 +1,19 @@
 danger.import_plugin('danger/plugins/helper.rb')
-danger.import_dangerfile(path: 'danger/metadata')
-danger.import_dangerfile(path: 'danger/changes_size')
-danger.import_dangerfile(path: 'danger/changelog')
-danger.import_dangerfile(path: 'danger/specs')
-danger.import_dangerfile(path: 'danger/gemfile')
-danger.import_dangerfile(path: 'danger/database')
-danger.import_dangerfile(path: 'danger/documentation')
-danger.import_dangerfile(path: 'danger/frozen_string')
-danger.import_dangerfile(path: 'danger/commit_messages')
-danger.import_dangerfile(path: 'danger/duplicate_yarn_dependencies')
-danger.import_dangerfile(path: 'danger/prettier')
-danger.import_dangerfile(path: 'danger/eslint')
-danger.import_dangerfile(path: 'danger/roulette')
-danger.import_dangerfile(path: 'danger/single_codebase')
-danger.import_dangerfile(path: 'danger/gitlab_ui_wg')
+
+unless helper.release_automation?
+  danger.import_dangerfile(path: 'danger/metadata')
+  danger.import_dangerfile(path: 'danger/changes_size')
+  danger.import_dangerfile(path: 'danger/changelog')
+  danger.import_dangerfile(path: 'danger/specs')
+  danger.import_dangerfile(path: 'danger/gemfile')
+  danger.import_dangerfile(path: 'danger/database')
+  danger.import_dangerfile(path: 'danger/documentation')
+  danger.import_dangerfile(path: 'danger/frozen_string')
+  danger.import_dangerfile(path: 'danger/commit_messages')
+  danger.import_dangerfile(path: 'danger/duplicate_yarn_dependencies')
+  danger.import_dangerfile(path: 'danger/prettier')
+  danger.import_dangerfile(path: 'danger/eslint')
+  danger.import_dangerfile(path: 'danger/roulette')
+  danger.import_dangerfile(path: 'danger/single_codebase')
+  danger.import_dangerfile(path: 'danger/gitlab_ui_wg')
+end
diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb
index ac65cf74808..d347f3c13a4 100644
--- a/lib/gitlab/danger/helper.rb
+++ b/lib/gitlab/danger/helper.rb
@@ -7,6 +7,7 @@ require_relative 'teammate'
 module Gitlab
   module Danger
     module Helper
+      RELEASE_TOOLS_BOT = 'gitlab-release-tools-bot'
       ROULETTE_DATA_URL = URI.parse('https://about.gitlab.com/roulette.json').freeze
 
       # Returns a list of all files that have been added, modified or renamed.
@@ -40,6 +41,10 @@ module Gitlab
         ENV['CI_PROJECT_NAME'] == 'gitlab-ee' || File.exist?('../../CHANGELOG-EE.md')
       end
 
+      def release_automation?
+        gitlab.mr_author == RELEASE_TOOLS_BOT
+      end
+
       def project_name
         ee? ? 'gitlab-ee' : 'gitlab-ce'
       end
-- 
cgit v1.2.1


From e540c0d71e00c4ce031b94cf11ec3de905e87da7 Mon Sep 17 00:00:00 2001
From: Oswaldo Ferreira <oswaldo@gitlab.com>
Date: Thu, 4 Apr 2019 13:08:34 +0000
Subject: Fixed test specs

- added suggestions to mock data
- fixed props to be not required
---
 .../diffs/components/diff_line_note_form.vue       |   6 +-
 .../javascripts/notes/components/note_form.vue     |  44 +++++++-
 .../vue_shared/components/lib/utils/diff_utils.js  |  20 ++++
 .../vue_shared/components/markdown/field.vue       |   5 +-
 .../vue_shared/components/markdown/header.vue      |   2 +-
 .../components/markdown/suggestion_diff.vue        |  42 +++-----
 .../components/markdown/suggestion_diff_row.vue    |  32 ++++++
 .../vue_shared/components/markdown/suggestions.vue |  38 +------
 app/controllers/concerns/preview_markdown.rb       |   2 +-
 app/serializers/issue_entity.rb                    |   2 +-
 app/serializers/merge_request_widget_entity.rb     |   2 +-
 app/serializers/suggestion_entity.rb               |   2 +
 app/serializers/suggestion_serializer.rb           |   9 ++
 app/services/concerns/suggestible.rb               |   7 ++
 app/services/preview_markdown_service.rb           |  28 ++++--
 .../shared/form_elements/_description.html.haml    |   2 +-
 app/views/shared/notes/_form.html.haml             |   2 +-
 .../osw-support-multi-line-suggestions.yml         |   5 +
 .../img/multi-line-suggestion-preview.png          | Bin 0 -> 61692 bytes
 .../img/multi-line-suggestion-syntax.png           | Bin 0 -> 29753 bytes
 doc/user/discussions/index.md                      |  18 ++++
 lib/banzai/suggestions_parser.rb                   |  16 ---
 spec/controllers/projects_controller_spec.rb       |  10 ++
 .../user_suggests_changes_on_diff_spec.rb          | 111 +++++++++++++++++++--
 .../markdown/suggestion_diff_row_spec.js           |  98 ++++++++++++++++++
 spec/javascripts/notes/mock_data.js                |   6 +-
 .../vue_shared/components/markdown/header_spec.js  |   2 +-
 .../components/markdown/suggestion_diff_spec.js    |  66 ++++++++----
 .../components/markdown/suggestions_spec.js        | 109 +++++++++-----------
 spec/lib/banzai/suggestions_parser_spec.rb         |  32 ------
 spec/lib/gitlab/diff/suggestion_spec.rb            |  87 ++++++++++++++--
 spec/lib/gitlab/diff/suggestions_parser_spec.rb    |  61 +++++++++++
 spec/models/suggestion_spec.rb                     |  16 +++
 spec/serializers/suggestion_entity_spec.rb         |   3 +-
 spec/services/preview_markdown_service_spec.rb     |  73 +++++++++++---
 spec/services/suggestions/apply_service_spec.rb    |  64 ++++++++++--
 36 files changed, 755 insertions(+), 267 deletions(-)
 create mode 100644 app/assets/javascripts/vue_shared/components/lib/utils/diff_utils.js
 create mode 100644 app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_row.vue
 create mode 100644 app/serializers/suggestion_serializer.rb
 create mode 100644 changelogs/unreleased/osw-support-multi-line-suggestions.yml
 create mode 100644 doc/user/discussions/img/multi-line-suggestion-preview.png
 create mode 100644 doc/user/discussions/img/multi-line-suggestion-syntax.png
 delete mode 100644 lib/banzai/suggestions_parser.rb
 create mode 100644 spec/frontend/vue_shared/components/markdown/suggestion_diff_row_spec.js
 delete mode 100644 spec/lib/banzai/suggestions_parser_spec.rb

diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
index bb66ab36283..41670b45798 100644
--- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
@@ -48,10 +48,13 @@ export default {
         noteableType: this.noteableType,
         noteTargetLine: this.noteTargetLine,
         diffViewType: this.diffViewType,
-        diffFile: this.getDiffFileByHash(this.diffFileHash),
+        diffFile: this.diffFile,
         linePosition: this.linePosition,
       };
     },
+    diffFile() {
+      return this.getDiffFileByHash(this.diffFileHash);
+    },
   },
   mounted() {
     if (this.isLoggedIn) {
@@ -102,6 +105,7 @@ export default {
       :line-code="line.line_code"
       :line="line"
       :help-page-path="helpPagePath"
+      :diff-file="diffFile"
       save-button-title="Comment"
       class="diff-comment-form"
       @handleFormUpdateAddToReview="addToReview"
diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue
index 57d6b181bd7..471323bfc83 100644
--- a/app/assets/javascripts/notes/components/note_form.vue
+++ b/app/assets/javascripts/notes/components/note_form.vue
@@ -61,6 +61,11 @@ export default {
       required: false,
       default: null,
     },
+    diffFile: {
+      type: Object,
+      required: false,
+      default: null,
+    },
     helpPagePath: {
       type: String,
       required: false,
@@ -102,9 +107,42 @@ export default {
       }
       return '#';
     },
+    diffParams() {
+      if (this.diffFile) {
+        return {
+          filePath: this.diffFile.file_path,
+          refs: this.diffFile.diff_refs,
+        };
+      } else if (this.note && this.note.position) {
+        return {
+          filePath: this.note.position.new_path,
+          refs: this.note.position,
+        };
+      } else if (this.discussion && this.discussion.diff_file) {
+        return {
+          filePath: this.discussion.diff_file.file_path,
+          refs: this.discussion.diff_file.diff_refs,
+        };
+      }
+
+      return null;
+    },
     markdownPreviewPath() {
       const notable = this.getNoteableDataByProp('preview_note_path');
-      return mergeUrlParams({ preview_suggestions: true }, notable);
+
+      const previewSuggestions = this.line && this.diffParams;
+      const params = previewSuggestions
+        ? {
+            preview_suggestions: previewSuggestions,
+            line: this.line.new_line,
+            file_path: this.diffParams.filePath,
+            base_sha: this.diffParams.refs.base_sha,
+            start_sha: this.diffParams.refs.start_sha,
+            head_sha: this.diffParams.refs.head_sha,
+          }
+        : {};
+
+      return mergeUrlParams(params, notable);
     },
     markdownDocsPath() {
       return this.getNotesDataByProp('markdownDocsPath');
@@ -234,8 +272,8 @@ export default {
           placeholder="Write a comment or drag your files here…"
           @keydown.meta.enter="handleKeySubmit()"
           @keydown.ctrl.enter="handleKeySubmit()"
-          @keydown.up="editMyLastNote()"
-          @keydown.esc="cancelHandler(true)"
+          @keydown.exact.up="editMyLastNote()"
+          @keydown.exact.esc="cancelHandler(true)"
           @input="onInput"
         ></textarea>
       </markdown-field>
diff --git a/app/assets/javascripts/vue_shared/components/lib/utils/diff_utils.js b/app/assets/javascripts/vue_shared/components/lib/utils/diff_utils.js
new file mode 100644
index 00000000000..d1aba99ac22
--- /dev/null
+++ b/app/assets/javascripts/vue_shared/components/lib/utils/diff_utils.js
@@ -0,0 +1,20 @@
+/* eslint-disable import/prefer-default-export */
+
+function trimFirstCharOfLineContent(text) {
+  if (!text) {
+    return text;
+  }
+
+  return text.replace(/^( |\+|-)/, '');
+}
+
+function cleanSuggestionLine(line = {}) {
+  return {
+    ...line,
+    text: trimFirstCharOfLineContent(line.text),
+  };
+}
+
+export function selectDiffLines(lines) {
+  return lines.filter(line => line.type !== 'match').map(line => cleanSuggestionLine(line));
+}
diff --git a/app/assets/javascripts/vue_shared/components/markdown/field.vue b/app/assets/javascripts/vue_shared/components/markdown/field.vue
index eccf73e227c..0f3b3568414 100644
--- a/app/assets/javascripts/vue_shared/components/markdown/field.vue
+++ b/app/assets/javascripts/vue_shared/components/markdown/field.vue
@@ -76,6 +76,7 @@ export default {
       hasSuggestion: false,
       markdownPreviewLoading: false,
       previewMarkdown: false,
+      suggestions: this.note.suggestions || [],
     };
   },
   computed: {
@@ -109,9 +110,6 @@ export default {
       }
       return lineNumber;
     },
-    suggestions() {
-      return this.note.suggestions || [];
-    },
     lineType() {
       return this.line ? this.line.type : '';
     },
@@ -175,6 +173,7 @@ export default {
         this.referencedCommands = data.references.commands;
         this.referencedUsers = data.references.users;
         this.hasSuggestion = data.references.suggestions && data.references.suggestions.length;
+        this.suggestions = data.references.suggestions;
       }
 
       this.$nextTick()
diff --git a/app/assets/javascripts/vue_shared/components/markdown/header.vue b/app/assets/javascripts/vue_shared/components/markdown/header.vue
index cc6ecdb0395..a5a5b2ef415 100644
--- a/app/assets/javascripts/vue_shared/components/markdown/header.vue
+++ b/app/assets/javascripts/vue_shared/components/markdown/header.vue
@@ -38,7 +38,7 @@ export default {
       ].join('\n');
     },
     mdSuggestion() {
-      return ['```suggestion', `{text}`, '```'].join('\n');
+      return ['```suggestion:-0+0', `{text}`, '```'].join('\n');
     },
   },
   mounted() {
diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue
index a351ca62c94..2eb4ec12a4a 100644
--- a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue
+++ b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue
@@ -1,24 +1,14 @@
 <script>
 import SuggestionDiffHeader from './suggestion_diff_header.vue';
+import SuggestionDiffRow from './suggestion_diff_row.vue';
+import { selectDiffLines } from '../lib/utils/diff_utils';
 
 export default {
   components: {
     SuggestionDiffHeader,
+    SuggestionDiffRow,
   },
   props: {
-    newLines: {
-      type: Array,
-      required: true,
-    },
-    fromContent: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    fromLine: {
-      type: Number,
-      required: true,
-    },
     suggestion: {
       type: Object,
       required: true,
@@ -33,6 +23,11 @@ export default {
       required: true,
     },
   },
+  computed: {
+    lines() {
+      return selectDiffLines(this.suggestion.diff_lines);
+    },
+  },
   methods: {
     applySuggestion(callback) {
       this.$emit('apply', { suggestionId: this.suggestion.id, callback });
@@ -52,22 +47,11 @@ export default {
     />
     <table class="mb-3 md-suggestion-diff js-syntax-highlight code">
       <tbody>
-        <!-- Old Line -->
-        <tr class="line_holder old">
-          <td class="diff-line-num old_line qa-old-diff-line-number old">{{ fromLine }}</td>
-          <td class="diff-line-num new_line old"></td>
-          <td class="line_content old">
-            <span>{{ fromContent }}</span>
-          </td>
-        </tr>
-        <!-- New Line(s) -->
-        <tr v-for="(line, key) of newLines" :key="key" class="line_holder new">
-          <td class="diff-line-num old_line new"></td>
-          <td class="diff-line-num new_line qa-new-diff-line-number new">{{ line.lineNumber }}</td>
-          <td class="line_content new">
-            <span>{{ line.content }}</span>
-          </td>
-        </tr>
+        <suggestion-diff-row
+          v-for="(line, index) of lines"
+          :key="`${index}-${line.text}`"
+          :line="line"
+        />
       </tbody>
     </table>
   </div>
diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_row.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_row.vue
new file mode 100644
index 00000000000..cafd3a515ea
--- /dev/null
+++ b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_row.vue
@@ -0,0 +1,32 @@
+<script>
+export default {
+  name: 'SuggestionDiffRow',
+  props: {
+    line: {
+      type: Object,
+      required: true,
+    },
+  },
+  computed: {
+    lineType() {
+      return this.line.type;
+    },
+  },
+};
+</script>
+
+<template>
+  <tr class="line_holder" :class="lineType">
+    <td class="diff-line-num old_line" :class="lineType">
+      {{ line.old_line }}
+    </td>
+    <td class="diff-line-num new_line" :class="lineType">
+      {{ line.new_line }}
+    </td>
+    <td class="line_content" :class="lineType">
+      <span v-if="line.text">{{ line.text }}</span>
+      <!-- TODO: replace this hack with zero-width whitespace when we have rich_text from BE -->
+      <span v-else>&#8203;</span>
+    </td>
+  </tr>
+</template>
diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue
index 177d78cb904..8d3705e1e4a 100644
--- a/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue
+++ b/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue
@@ -6,16 +6,6 @@ import Flash from '~/flash';
 export default {
   components: { SuggestionDiff },
   props: {
-    fromLine: {
-      type: Number,
-      required: false,
-      default: 0,
-    },
-    fromContent: {
-      type: String,
-      required: false,
-      default: '',
-    },
     lineType: {
       type: String,
       required: false,
@@ -71,41 +61,19 @@ export default {
 
       suggestionElements.forEach((suggestionEl, i) => {
         const suggestionParentEl = suggestionEl.parentElement;
-        const newLines = this.extractNewLines(suggestionParentEl);
-        const diffComponent = this.generateDiff(newLines, i);
+        const diffComponent = this.generateDiff(i);
         diffComponent.$mount(suggestionParentEl);
       });
 
       this.isRendered = true;
     },
-    extractNewLines(suggestionEl) {
-      // extracts the suggested lines from the markdown
-      // calculates a line number for each line
-
-      const newLines = suggestionEl.querySelectorAll('.line');
-      const fromLine = this.suggestions.length ? this.suggestions[0].from_line : this.fromLine;
-      const lines = [];
-
-      newLines.forEach((line, i) => {
-        const content = `${line.innerText}\n`;
-        const lineNumber = fromLine + i;
-        lines.push({ content, lineNumber });
-      });
-
-      return lines;
-    },
-    generateDiff(newLines, suggestionIndex) {
-      // generates the diff <suggestion-diff /> component
-      // all `suggestion` markdown will be swapped out by this component
-
+    generateDiff(suggestionIndex) {
       const { suggestions, disabled, helpPagePath } = this;
       const suggestion =
         suggestions && suggestions[suggestionIndex] ? suggestions[suggestionIndex] : {};
-      const fromContent = suggestion.from_content || this.fromContent;
-      const fromLine = suggestion.from_line || this.fromLine;
       const SuggestionDiffComponent = Vue.extend(SuggestionDiff);
       const suggestionDiff = new SuggestionDiffComponent({
-        propsData: { newLines, fromLine, fromContent, disabled, suggestion, helpPagePath },
+        propsData: { disabled, suggestion, helpPagePath },
       });
 
       suggestionDiff.$on('apply', ({ suggestionId, callback }) => {
diff --git a/app/controllers/concerns/preview_markdown.rb b/app/controllers/concerns/preview_markdown.rb
index f72d25fc54c..2a9729b6ffd 100644
--- a/app/controllers/concerns/preview_markdown.rb
+++ b/app/controllers/concerns/preview_markdown.rb
@@ -20,7 +20,7 @@ module PreviewMarkdown
       body: view_context.markdown(result[:text], markdown_params),
       references: {
         users: result[:users],
-        suggestions: result[:suggestions],
+        suggestions: SuggestionSerializer.new.represent_diff(result[:suggestions]),
         commands: view_context.markdown(result[:commands])
       }
     }
diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb
index c3f7d4651fb..914ad628a99 100644
--- a/app/serializers/issue_entity.rb
+++ b/app/serializers/issue_entity.rb
@@ -42,6 +42,6 @@ class IssueEntity < IssuableEntity
   end
 
   expose :preview_note_path do |issue|
-    preview_markdown_path(issue.project, quick_actions_target_type: 'Issue', quick_actions_target_id: issue.iid)
+    preview_markdown_path(issue.project, target_type: 'Issue', target_id: issue.iid)
   end
 end
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index d673f8ae896..4831eb32c96 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -235,7 +235,7 @@ class MergeRequestWidgetEntity < IssuableEntity
   end
 
   expose :preview_note_path do |merge_request|
-    preview_markdown_path(merge_request.project, quick_actions_target_type: 'MergeRequest', quick_actions_target_id: merge_request.iid)
+    preview_markdown_path(merge_request.project, target_type: 'MergeRequest', target_id: merge_request.iid)
   end
 
   expose :merge_commit_path do |merge_request|
diff --git a/app/serializers/suggestion_entity.rb b/app/serializers/suggestion_entity.rb
index 4d0d4da10be..2dd62e19e29 100644
--- a/app/serializers/suggestion_entity.rb
+++ b/app/serializers/suggestion_entity.rb
@@ -3,6 +3,8 @@
 class SuggestionEntity < API::Entities::Suggestion
   include RequestAwareEntity
 
+  unexpose :from_line, :to_line, :from_content, :to_content
+  expose :diff_lines, using: DiffLineEntity
   expose :current_user do
     expose :can_apply do |suggestion|
       Ability.allowed?(current_user, :apply_suggestion, suggestion)
diff --git a/app/serializers/suggestion_serializer.rb b/app/serializers/suggestion_serializer.rb
new file mode 100644
index 00000000000..010344f9fcd
--- /dev/null
+++ b/app/serializers/suggestion_serializer.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class SuggestionSerializer < BaseSerializer
+  entity SuggestionEntity
+
+  def represent_diff(resource)
+    represent(resource, { only: [:diff_lines] })
+  end
+end
diff --git a/app/services/concerns/suggestible.rb b/app/services/concerns/suggestible.rb
index 0b9822b1909..0cba9bf1b8a 100644
--- a/app/services/concerns/suggestible.rb
+++ b/app/services/concerns/suggestible.rb
@@ -2,10 +2,17 @@
 
 module Suggestible
   extend ActiveSupport::Concern
+  include Gitlab::Utils::StrongMemoize
 
   # This translates into limiting suggestion changes to `suggestion:-100+100`.
   MAX_LINES_CONTEXT = 100.freeze
 
+  def diff_lines
+    strong_memoize(:diff_lines) do
+      Gitlab::Diff::SuggestionDiff.new(self).diff_lines
+    end
+  end
+
   def fetch_from_content
     diff_file.new_blob_lines_between(from_line, to_line).join
   end
diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb
index c1655c38095..7386530f45f 100644
--- a/app/services/preview_markdown_service.rb
+++ b/app/services/preview_markdown_service.rb
@@ -17,7 +17,7 @@ class PreviewMarkdownService < BaseService
   private
 
   def explain_quick_actions(text)
-    return text, [] unless %w(Issue MergeRequest Commit).include?(commands_target_type)
+    return text, [] unless %w(Issue MergeRequest Commit).include?(target_type)
 
     quick_actions_service = QuickActions::InterpretService.new(project, current_user)
     quick_actions_service.explain(text, find_commands_target)
@@ -30,22 +30,34 @@ class PreviewMarkdownService < BaseService
   end
 
   def find_suggestions(text)
-    return [] unless params[:preview_suggestions]
+    return [] unless preview_sugestions?
 
-    Banzai::SuggestionsParser.parse(text)
+    position = Gitlab::Diff::Position.new(new_path: params[:file_path],
+                                          new_line: params[:line].to_i,
+                                          base_sha: params[:base_sha],
+                                          head_sha: params[:head_sha],
+                                          start_sha: params[:start_sha])
+
+    Gitlab::Diff::SuggestionsParser.parse(text, position: position, project: project)
+  end
+
+  def preview_sugestions?
+    params[:preview_suggestions] &&
+      target_type == 'MergeRequest' &&
+      Ability.allowed?(current_user, :download_code, project)
   end
 
   def find_commands_target
     QuickActions::TargetService
       .new(project, current_user)
-      .execute(commands_target_type, commands_target_id)
+      .execute(target_type, target_id)
   end
 
-  def commands_target_type
-    params[:quick_actions_target_type]
+  def target_type
+    params[:target_type]
   end
 
-  def commands_target_id
-    params[:quick_actions_target_id]
+  def target_id
+    params[:target_id]
   end
 end
diff --git a/app/views/shared/form_elements/_description.html.haml b/app/views/shared/form_elements/_description.html.haml
index 25df2fe5cd6..b11cb8a3076 100644
--- a/app/views/shared/form_elements/_description.html.haml
+++ b/app/views/shared/form_elements/_description.html.haml
@@ -5,7 +5,7 @@
 - supports_quick_actions = model.new_record?
 
 - if supports_quick_actions
-  - preview_url = preview_markdown_path(project, quick_actions_target_type: model.class.name)
+  - preview_url = preview_markdown_path(project, target_type: model.class.name)
 - else
   - preview_url = preview_markdown_path(project)
 
diff --git a/app/views/shared/notes/_form.html.haml b/app/views/shared/notes/_form.html.haml
index 6a1eea85fde..d91bc6e57c9 100644
--- a/app/views/shared/notes/_form.html.haml
+++ b/app/views/shared/notes/_form.html.haml
@@ -1,7 +1,7 @@
 - supports_autocomplete = local_assigns.fetch(:supports_autocomplete, true)
 - supports_quick_actions = note_supports_quick_actions?(@note)
 - if supports_quick_actions
-  - preview_url = preview_markdown_path(@project, quick_actions_target_type: @note.noteable_type, quick_actions_target_id: @note.noteable_id)
+  - preview_url = preview_markdown_path(@project, target_type: @note.noteable_type, target_id: @note.noteable_id)
 - else
   - preview_url = preview_markdown_path(@project)
 
diff --git a/changelogs/unreleased/osw-support-multi-line-suggestions.yml b/changelogs/unreleased/osw-support-multi-line-suggestions.yml
new file mode 100644
index 00000000000..8c8206c3822
--- /dev/null
+++ b/changelogs/unreleased/osw-support-multi-line-suggestions.yml
@@ -0,0 +1,5 @@
+---
+title: Support multi-line suggestions
+merge_request: 25211
+author:
+type: added
diff --git a/doc/user/discussions/img/multi-line-suggestion-preview.png b/doc/user/discussions/img/multi-line-suggestion-preview.png
new file mode 100644
index 00000000000..4288d0ba034
Binary files /dev/null and b/doc/user/discussions/img/multi-line-suggestion-preview.png differ
diff --git a/doc/user/discussions/img/multi-line-suggestion-syntax.png b/doc/user/discussions/img/multi-line-suggestion-syntax.png
new file mode 100644
index 00000000000..df0c99b84ef
Binary files /dev/null and b/doc/user/discussions/img/multi-line-suggestion-syntax.png differ
diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md
index 23b9604a456..bf41fdecd10 100644
--- a/doc/user/discussions/index.md
+++ b/doc/user/discussions/index.md
@@ -344,6 +344,24 @@ and push the suggested change directly into the codebase in the merge request's
 Custom commit messages will be introduced by
 [#54404](https://gitlab.com/gitlab-org/gitlab-ce/issues/54404).
 
+### Multi-line suggestions
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/53310) in GitLab 11.10.
+
+Reviewers can also suggest changes to
+multiple lines with a single suggestion within Merge Request diff discussions.
+
+![Multi-line suggestion syntax](img/multi-line-suggestion-syntax.png)
+
+In the example above, the suggestion covers three lines above and four lines below the commented diff line.
+It'd change from 3 lines _above_ to 4 lines _below_ the commented Diff line.
+
+![Multi-line suggestion preview](img/multi-line-suggestion-preview.png)
+
+NOTE: **Note:**
+Suggestions covering multiple lines are limited to 100 lines _above_ and 100 lines _below_ 
+the commented diff line, allowing up to 200 changed lines per suggestion.
+
 ## Start a discussion by replying to a standard comment
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/30299) in GitLab 11.9
diff --git a/lib/banzai/suggestions_parser.rb b/lib/banzai/suggestions_parser.rb
deleted file mode 100644
index 0d7f751bfc1..00000000000
--- a/lib/banzai/suggestions_parser.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-# frozen_string_literal: true
-
-# TODO: Delete when https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/26107
-# exchange this parser by `Gitlab::Diff::SuggestionsParser`.
-module Banzai
-  module SuggestionsParser
-    # Returns the content of each suggestion code block.
-    #
-    def self.parse(text)
-      html = Banzai.render(text, project: nil, no_original_data: true)
-      doc = Nokogiri::HTML(html)
-
-      doc.search('pre.suggestion').map { |node| node.text }
-    end
-  end
-end
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 356d606d5c5..56d38b9475e 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -703,6 +703,16 @@ describe ProjectsController do
       expect(JSON.parse(response.body).keys).to match_array(%w(body references))
     end
 
+    context 'when not authorized' do
+      let(:private_project) { create(:project, :private) }
+
+      it 'returns 404' do
+        post :preview_markdown, params: { namespace_id: private_project.namespace, id: private_project, text: '*Markdown* text' }
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
     context 'state filter on references' do
       let(:issue) { create(:issue, :closed, project: public_project) }
       let(:merge_request) { create(:merge_request, :closed, target_project: public_project) }
diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb
index c19e299097e..1b5dd6945e0 100644
--- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb
+++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb
@@ -6,6 +6,14 @@ describe 'User comments on a diff', :js do
   include MergeRequestDiffHelpers
   include RepoHelpers
 
+  def expect_suggestion_has_content(element, expected_changing_content, expected_suggested_content)
+    changing_content = element.all(:css, '.line_holder.old').map(&:text)
+    suggested_content = element.all(:css, '.line_holder.new').map(&:text)
+
+    expect(changing_content).to eq(expected_changing_content)
+    expect(suggested_content).to eq(expected_suggested_content)
+  end
+
   let(:project) { create(:project, :repository) }
   let(:merge_request) do
     create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
@@ -33,8 +41,18 @@ describe 'User comments on a diff', :js do
       page.within('.diff-discussions') do
         expect(page).to have_button('Apply suggestion')
         expect(page).to have_content('Suggested change')
-        expect(page).to have_content('	url = https://github.com/gitlabhq/gitlab-shell.git')
-        expect(page).to have_content('# change to a comment')
+      end
+
+      page.within('.md-suggestion-diff') do
+        expected_changing_content = [
+          "6 url = https://github.com/gitlabhq/gitlab-shell.git"
+        ]
+
+        expected_suggested_content = [
+          "6 # change to a comment"
+        ]
+
+        expect_suggestion_has_content(page, expected_changing_content, expected_suggested_content)
       end
     end
 
@@ -64,7 +82,7 @@ describe 'User comments on a diff', :js do
       click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']"))
 
       page.within('.js-discussion-note-form') do
-        fill_in('note_note', with: "```suggestion\n# change to a comment\n```\n```suggestion\n# or that\n```")
+        fill_in('note_note', with: "```suggestion\n# change to a comment\n```\n```suggestion:-2\n# or that\n# heh\n```")
         click_button('Comment')
       end
 
@@ -74,11 +92,90 @@ describe 'User comments on a diff', :js do
         suggestion_1 = page.all(:css, '.md-suggestion-diff')[0]
         suggestion_2 = page.all(:css, '.md-suggestion-diff')[1]
 
-        expect(suggestion_1).to have_content('	url = https://github.com/gitlabhq/gitlab-shell.git')
-        expect(suggestion_1).to have_content('# change to a comment')
+        suggestion_1_expected_changing_content = [
+          "6 url = https://github.com/gitlabhq/gitlab-shell.git"
+        ]
+        suggestion_1_expected_suggested_content = [
+          "6 # change to a comment"
+        ]
+
+        suggestion_2_expected_changing_content = [
+          "4 [submodule \"gitlab-shell\"]",
+          "5 path = gitlab-shell",
+          "6 url = https://github.com/gitlabhq/gitlab-shell.git"
+        ]
+        suggestion_2_expected_suggested_content = [
+          "4 # or that",
+          "5 # heh"
+        ]
+
+        expect_suggestion_has_content(suggestion_1,
+                                      suggestion_1_expected_changing_content,
+                                      suggestion_1_expected_suggested_content)
+
+        expect_suggestion_has_content(suggestion_2,
+                                      suggestion_2_expected_changing_content,
+                                      suggestion_2_expected_suggested_content)
+      end
+    end
+  end
+
+  context 'multi-line suggestions' do
+    it 'suggestion is presented' do
+      click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']"))
+
+      page.within('.js-discussion-note-form') do
+        fill_in('note_note', with: "```suggestion:-3+5\n# change to a\n# comment\n# with\n# broken\n# lines\n```")
+        click_button('Comment')
+      end
 
-        expect(suggestion_2).to have_content('	url = https://github.com/gitlabhq/gitlab-shell.git')
-        expect(suggestion_2).to have_content('# or that')
+      wait_for_requests
+
+      page.within('.diff-discussions') do
+        expect(page).to have_button('Apply suggestion')
+        expect(page).to have_content('Suggested change')
+      end
+
+      page.within('.md-suggestion-diff') do
+        expected_changing_content = [
+          "3 url = git://github.com/randx/six.git",
+          "4 [submodule \"gitlab-shell\"]",
+          "5 path = gitlab-shell",
+          "6 url = https://github.com/gitlabhq/gitlab-shell.git",
+          "7 [submodule \"gitlab-grack\"]",
+          "8 path = gitlab-grack",
+          "9 url = https://gitlab.com/gitlab-org/gitlab-grack.git"
+        ]
+
+        expected_suggested_content = [
+          "3 # change to a",
+          "4 # comment",
+          "5 # with",
+          "6 # broken",
+          "7 # lines"
+        ]
+
+        expect_suggestion_has_content(page, expected_changing_content, expected_suggested_content)
+      end
+    end
+
+    it 'suggestion is appliable' do
+      click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']"))
+
+      page.within('.js-discussion-note-form') do
+        fill_in('note_note', with: "```suggestion:-3+5\n# change to a\n# comment\n# with\n# broken\n# lines\n```")
+        click_button('Comment')
+      end
+
+      wait_for_requests
+
+      page.within('.diff-discussions') do
+        expect(page).not_to have_content('Applied')
+
+        click_button('Apply suggestion')
+        wait_for_requests
+
+        expect(page).to have_content('Applied')
       end
     end
   end
diff --git a/spec/frontend/vue_shared/components/markdown/suggestion_diff_row_spec.js b/spec/frontend/vue_shared/components/markdown/suggestion_diff_row_spec.js
new file mode 100644
index 00000000000..866d6eb05c6
--- /dev/null
+++ b/spec/frontend/vue_shared/components/markdown/suggestion_diff_row_spec.js
@@ -0,0 +1,98 @@
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import SuggestionDiffRow from '~/vue_shared/components/markdown/suggestion_diff_row.vue';
+
+const oldLine = {
+  can_receive_suggestion: false,
+  line_code: null,
+  meta_data: null,
+  new_line: null,
+  old_line: 5,
+  rich_text: '-oldtext',
+  text: '-oldtext',
+  type: 'old',
+};
+
+const newLine = {
+  can_receive_suggestion: false,
+  line_code: null,
+  meta_data: null,
+  new_line: 6,
+  old_line: null,
+  rich_text: '-newtext',
+  text: '-newtext',
+  type: 'new',
+};
+
+describe(SuggestionDiffRow.name, () => {
+  let wrapper;
+
+  const factory = (options = {}) => {
+    const localVue = createLocalVue();
+
+    wrapper = shallowMount(SuggestionDiffRow, {
+      localVue,
+      ...options,
+    });
+  };
+
+  const findOldLineWrapper = () => wrapper.find('.old_line');
+  const findNewLineWrapper = () => wrapper.find('.new_line');
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it('renders correctly', () => {
+    factory({
+      propsData: {
+        line: oldLine,
+      },
+    });
+
+    expect(wrapper.is('.line_holder')).toBe(true);
+  });
+
+  describe('when passed line has type old', () => {
+    beforeEach(() => {
+      factory({
+        propsData: {
+          line: oldLine,
+        },
+      });
+    });
+
+    it('has old class when line has type old', () => {
+      expect(wrapper.find('td').classes()).toContain('old');
+    });
+
+    it('has old line number rendered', () => {
+      expect(findOldLineWrapper().text()).toBe('5');
+    });
+
+    it('has no new line number rendered', () => {
+      expect(findNewLineWrapper().text()).toBe('');
+    });
+  });
+
+  describe('when passed line has type new', () => {
+    beforeEach(() => {
+      factory({
+        propsData: {
+          line: newLine,
+        },
+      });
+    });
+
+    it('has new class when line has type new', () => {
+      expect(wrapper.find('td').classes()).toContain('new');
+    });
+
+    it('has no old line number rendered', () => {
+      expect(findOldLineWrapper().text()).toBe('');
+    });
+
+    it('has no new line number rendered', () => {
+      expect(findNewLineWrapper().text()).toBe('6');
+    });
+  });
+});
diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js
index 348743081eb..1df5cf9ef68 100644
--- a/spec/javascripts/notes/mock_data.js
+++ b/spec/javascripts/notes/mock_data.js
@@ -44,8 +44,7 @@ export const noteableDataMock = {
   milestone: null,
   milestone_id: null,
   moved_to_id: null,
-  preview_note_path:
-    '/gitlab-org/gitlab-ce/preview_markdown?quick_actions_target_id=98&quick_actions_target_type=Issue',
+  preview_note_path: '/gitlab-org/gitlab-ce/preview_markdown?target_id=98&target_type=Issue',
   project_id: 2,
   state: 'opened',
   time_estimate: 0,
@@ -347,8 +346,7 @@ export const loggedOutnoteableData = {
   },
   noteable_note_url: '/group/project/merge_requests/1#note_1',
   create_note_path: '/gitlab-org/gitlab-ce/notes?target_id=98&target_type=issue',
-  preview_note_path:
-    '/gitlab-org/gitlab-ce/preview_markdown?quick_actions_target_id=98&quick_actions_target_type=Issue',
+  preview_note_path: '/gitlab-org/gitlab-ce/preview_markdown?target_id=98&target_type=Issue',
 };
 
 export const collapseNotesMock = [
diff --git a/spec/javascripts/vue_shared/components/markdown/header_spec.js b/spec/javascripts/vue_shared/components/markdown/header_spec.js
index e733a95288e..d4be2451f0b 100644
--- a/spec/javascripts/vue_shared/components/markdown/header_spec.js
+++ b/spec/javascripts/vue_shared/components/markdown/header_spec.js
@@ -98,7 +98,7 @@ describe('Markdown field header component', () => {
   it('renders suggestion template', () => {
     vm.lineContent = 'Some content';
 
-    expect(vm.mdSuggestion).toEqual('```suggestion\n{text}\n```');
+    expect(vm.mdSuggestion).toEqual('```suggestion:-0+0\n{text}\n```');
   });
 
   it('does not render suggestion button if `canSuggest` is set to false', () => {
diff --git a/spec/javascripts/vue_shared/components/markdown/suggestion_diff_spec.js b/spec/javascripts/vue_shared/components/markdown/suggestion_diff_spec.js
index f87c2a92f47..ea74cb9eb21 100644
--- a/spec/javascripts/vue_shared/components/markdown/suggestion_diff_spec.js
+++ b/spec/javascripts/vue_shared/components/markdown/suggestion_diff_spec.js
@@ -1,21 +1,50 @@
 import Vue from 'vue';
 import SuggestionDiffComponent from '~/vue_shared/components/markdown/suggestion_diff.vue';
+import { selectDiffLines } from '~/vue_shared/components/lib/utils/diff_utils';
 
 const MOCK_DATA = {
   canApply: true,
-  newLines: [
-    { content: 'Line 1\n', lineNumber: 1 },
-    { content: 'Line 2\n', lineNumber: 2 },
-    { content: 'Line 3\n', lineNumber: 3 },
-  ],
-  fromLine: 1,
-  fromContent: 'Old content',
   suggestion: {
     id: 1,
+    diff_lines: [
+      {
+        can_receive_suggestion: false,
+        line_code: null,
+        meta_data: null,
+        new_line: null,
+        old_line: 5,
+        rich_text: '-test',
+        text: '-test',
+        type: 'old',
+      },
+      {
+        can_receive_suggestion: true,
+        line_code: null,
+        meta_data: null,
+        new_line: 5,
+        old_line: null,
+        rich_text: '+new test',
+        text: '+new test',
+        type: 'new',
+      },
+      {
+        can_receive_suggestion: true,
+        line_code: null,
+        meta_data: null,
+        new_line: 5,
+        old_line: null,
+        rich_text: '+new test2',
+        text: '+new test2',
+        type: 'new',
+      },
+    ],
   },
   helpPagePath: 'path_to_docs',
 };
 
+const lines = selectDiffLines(MOCK_DATA.suggestion.diff_lines);
+const newLines = lines.filter(line => line.type === 'new');
+
 describe('Suggestion Diff component', () => {
   let vm;
 
@@ -39,30 +68,23 @@ describe('Suggestion Diff component', () => {
     });
 
     it('renders the oldLineNumber', () => {
-      const fromLine = vm.$el.querySelector('.qa-old-diff-line-number').innerHTML;
+      const fromLine = vm.$el.querySelector('.old_line').innerHTML;
 
-      expect(parseInt(fromLine, 10)).toBe(vm.fromLine);
+      expect(parseInt(fromLine, 10)).toBe(lines[0].old_line);
     });
 
     it('renders the oldLineContent', () => {
       const fromContent = vm.$el.querySelector('.line_content.old').innerHTML;
 
-      expect(fromContent.includes(vm.fromContent)).toBe(true);
-    });
-
-    it('renders the contents of newLines', () => {
-      const newLines = vm.$el.querySelectorAll('.line_holder.new');
-
-      newLines.forEach((line, i) => {
-        expect(newLines[i].innerHTML.includes(vm.newLines[i].content)).toBe(true);
-      });
+      expect(fromContent.includes(lines[0].text)).toBe(true);
     });
 
-    it('renders a line number for each line', () => {
-      const newLineNumbers = vm.$el.querySelectorAll('.qa-new-diff-line-number');
+    it('renders new lines', () => {
+      const newLinesElements = vm.$el.querySelectorAll('.line_holder.new');
 
-      newLineNumbers.forEach((line, i) => {
-        expect(newLineNumbers[i].innerHTML.includes(vm.newLines[i].lineNumber)).toBe(true);
+      newLinesElements.forEach((line, i) => {
+        expect(newLinesElements[i].innerHTML.includes(newLines[i].new_line)).toBe(true);
+        expect(newLinesElements[i].innerHTML.includes(newLines[i].text)).toBe(true);
       });
     });
   });
diff --git a/spec/javascripts/vue_shared/components/markdown/suggestions_spec.js b/spec/javascripts/vue_shared/components/markdown/suggestions_spec.js
index 33be63a3a1e..b7de40b4831 100644
--- a/spec/javascripts/vue_shared/components/markdown/suggestions_spec.js
+++ b/spec/javascripts/vue_shared/components/markdown/suggestions_spec.js
@@ -2,46 +2,52 @@ import Vue from 'vue';
 import SuggestionsComponent from '~/vue_shared/components/markdown/suggestions.vue';
 
 const MOCK_DATA = {
-  fromLine: 1,
-  fromContent: 'Old content',
-  suggestions: [],
+  suggestions: [
+    {
+      id: 1,
+      appliable: true,
+      applied: false,
+      current_user: {
+        can_apply: true,
+      },
+      diff_lines: [
+        {
+          can_receive_suggestion: false,
+          line_code: null,
+          meta_data: null,
+          new_line: null,
+          old_line: 5,
+          rich_text: '-test',
+          text: '-test',
+          type: 'old',
+        },
+        {
+          can_receive_suggestion: true,
+          line_code: null,
+          meta_data: null,
+          new_line: 5,
+          old_line: null,
+          rich_text: '+new test',
+          text: '+new test',
+          type: 'new',
+        },
+      ],
+    },
+  ],
   noteHtml: `
+      <div class="suggestion">
+      <div class="line">-oldtest</div>
+    </div>  
     <div class="suggestion">
-      <div class="line">Suggestion 1</div>
+      <div class="line">+newtest</div>
     </div>    
-    
-    <div class="suggestion">
-      <div class="line">Suggestion 2</div>
-    </div>
   `,
   isApplied: false,
   helpPagePath: 'path_to_docs',
 };
 
-const generateLine = content => {
-  const line = document.createElement('div');
-  line.className = 'line';
-  line.innerHTML = content;
-
-  return line;
-};
-
-const generateMockLines = () => {
-  const line1 = generateLine('Line 1');
-  const line2 = generateLine('Line 2');
-  const line3 = generateLine('- Line 3');
-  const container = document.createElement('div');
-
-  container.appendChild(line1);
-  container.appendChild(line2);
-  container.appendChild(line3);
-
-  return container;
-};
-
 describe('Suggestion component', () => {
   let vm;
-  let extractedLines;
   let diffTable;
 
   beforeEach(done => {
@@ -51,8 +57,7 @@ describe('Suggestion component', () => {
       propsData: MOCK_DATA,
     }).$mount();
 
-    extractedLines = vm.extractNewLines(generateMockLines());
-    diffTable = vm.generateDiff(extractedLines).$mount().$el;
+    diffTable = vm.generateDiff(0).$mount().$el;
 
     spyOn(vm, 'renderSuggestions');
     vm.renderSuggestions();
@@ -70,32 +75,8 @@ describe('Suggestion component', () => {
 
     it('renders suggestions', () => {
       expect(vm.renderSuggestions).toHaveBeenCalled();
-      expect(vm.$el.innerHTML.includes('Suggestion 1')).toBe(true);
-      expect(vm.$el.innerHTML.includes('Suggestion 2')).toBe(true);
-    });
-  });
-
-  describe('extractNewLines', () => {
-    it('extracts suggested lines', () => {
-      const expectedReturn = [
-        { content: 'Line 1\n', lineNumber: 1 },
-        { content: 'Line 2\n', lineNumber: 2 },
-        { content: '- Line 3\n', lineNumber: 3 },
-      ];
-
-      expect(vm.extractNewLines(generateMockLines())).toEqual(expectedReturn);
-    });
-
-    it('increments line number for each extracted line', () => {
-      expect(extractedLines[0].lineNumber).toEqual(1);
-      expect(extractedLines[1].lineNumber).toEqual(2);
-      expect(extractedLines[2].lineNumber).toEqual(3);
-    });
-
-    it('returns empty array if no lines are found', () => {
-      const el = document.createElement('div');
-
-      expect(vm.extractNewLines(el)).toEqual([]);
+      expect(vm.$el.innerHTML.includes('oldtest')).toBe(true);
+      expect(vm.$el.innerHTML.includes('newtest')).toBe(true);
     });
   });
 
@@ -109,17 +90,17 @@ describe('Suggestion component', () => {
     });
 
     it('generates a diff table that contains contents the suggested lines', () => {
-      extractedLines.forEach((line, i) => {
-        expect(diffTable.innerHTML.includes(extractedLines[i].content)).toBe(true);
+      MOCK_DATA.suggestions[0].diff_lines.forEach(line => {
+        const text = line.text.substring(1);
+
+        expect(diffTable.innerHTML.includes(text)).toBe(true);
       });
     });
 
     it('generates a diff table with the correct line number for each suggested line', () => {
-      const lines = diffTable.getElementsByClassName('qa-new-diff-line-number');
+      const lines = diffTable.querySelectorAll('.old_line');
 
-      expect([...lines][0].innerHTML).toBe('1');
-      expect([...lines][1].innerHTML).toBe('2');
-      expect([...lines][2].innerHTML).toBe('3');
+      expect(parseInt([...lines][0].innerHTML, 10)).toBe(5);
     });
   });
 });
diff --git a/spec/lib/banzai/suggestions_parser_spec.rb b/spec/lib/banzai/suggestions_parser_spec.rb
deleted file mode 100644
index 79658d710ce..00000000000
--- a/spec/lib/banzai/suggestions_parser_spec.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Banzai::SuggestionsParser do
-  describe '.parse' do
-    it 'returns a list of suggestion contents' do
-      markdown = <<-MARKDOWN.strip_heredoc
-        ```suggestion
-          foo
-          bar
-        ```
-
-        ```
-          nothing
-        ```
-
-        ```suggestion
-          xpto
-          baz
-        ```
-
-        ```thing
-          this is not a suggestion, it's a thing
-        ```
-      MARKDOWN
-
-      expect(described_class.parse(markdown)).to eq(["  foo\n  bar",
-                                                     "  xpto\n  baz"])
-    end
-  end
-end
diff --git a/spec/lib/gitlab/diff/suggestion_spec.rb b/spec/lib/gitlab/diff/suggestion_spec.rb
index 71fd25df698..d7ca0e0a522 100644
--- a/spec/lib/gitlab/diff/suggestion_spec.rb
+++ b/spec/lib/gitlab/diff/suggestion_spec.rb
@@ -10,6 +10,16 @@ describe Gitlab::Diff::Suggestion do
                                             lines_above: above,
                                             lines_below: below)
     end
+
+    it 'returns diff lines with correct line numbers' do
+      diff_lines = suggestion.diff_lines
+
+      expect(diff_lines).to all(be_a(Gitlab::Diff::Line))
+
+      expected_diff_lines.each_with_index do |expected_line, index|
+        expect(diff_lines[index].to_hash).to include(expected_line)
+      end
+    end
   end
 
   let(:merge_request) { create(:merge_request) }
@@ -48,6 +58,18 @@ describe Gitlab::Diff::Suggestion do
       let(:expected_above) { line - 1 }
       let(:expected_below) { below }
       let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) }
+      let(:expected_diff_lines) do
+        [
+          { old_pos: 1, new_pos: 1, type: 'old', text: "-require 'fileutils'" },
+          { old_pos: 2, new_pos: 1, type: 'old', text: "-require 'open3'" },
+          { old_pos: 3, new_pos: 1, type: 'old', text: "-" },
+          { old_pos: 4, new_pos: 1, type: 'old', text: "-module Popen" },
+          { old_pos: 5, new_pos: 1, type: 'old', text: "-  extend self" },
+          { old_pos: 6, new_pos: 1, type: 'old', text: "-" },
+          { old_pos: 7, new_pos: 1, type: 'new', text: "+# parsed suggestion content" },
+          { old_pos: 7, new_pos: 2, type: 'new', text: "+# with comments" }
+        ]
+      end
 
       it_behaves_like 'correct suggestion raw content'
     end
@@ -59,6 +81,47 @@ describe Gitlab::Diff::Suggestion do
       let(:expected_below) { below }
       let(:expected_above) { above }
       let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) }
+      let(:expected_diff_lines) do
+        [
+          { old_pos: 4, new_pos: 4, type: "match", text: "@@ -4 +4" },
+          { old_pos: 4, new_pos: 4, type: "old", text: "-module Popen" },
+          { old_pos: 5, new_pos: 4, type: "old", text: "-  extend self" },
+          { old_pos: 6, new_pos: 4, type: "old", text: "-" },
+          { old_pos: 7, new_pos: 4, type: "old", text: "-  def popen(cmd, path=nil)" },
+          { old_pos: 8, new_pos: 4, type: "old", text: "-    unless cmd.is_a?(Array)" },
+          { old_pos: 9, new_pos: 4, type: "old", text: "-      raise RuntimeError, \"System commands must be given as an array of strings\"" },
+          { old_pos: 10, new_pos: 4, type: "old", text: "-    end" },
+          { old_pos: 11, new_pos: 4, type: "old", text: "-" },
+          { old_pos: 12, new_pos: 4, type: "old", text: "-    path ||= Dir.pwd" },
+          { old_pos: 13, new_pos: 4, type: "old", text: "-" },
+          { old_pos: 14, new_pos: 4, type: "old", text: "-    vars = {" },
+          { old_pos: 15, new_pos: 4, type: "old", text: "-      \"PWD\" => path" },
+          { old_pos: 16, new_pos: 4, type: "old", text: "-    }" },
+          { old_pos: 17, new_pos: 4, type: "old", text: "-" },
+          { old_pos: 18, new_pos: 4, type: "old", text: "-    options = {" },
+          { old_pos: 19, new_pos: 4, type: "old", text: "-      chdir: path" },
+          { old_pos: 20, new_pos: 4, type: "old", text: "-    }" },
+          { old_pos: 21, new_pos: 4, type: "old", text: "-" },
+          { old_pos: 22, new_pos: 4, type: "old", text: "-    unless File.directory?(path)" },
+          { old_pos: 23, new_pos: 4, type: "old", text: "-      FileUtils.mkdir_p(path)" },
+          { old_pos: 24, new_pos: 4, type: "old", text: "-    end" },
+          { old_pos: 25, new_pos: 4, type: "old", text: "-" },
+          { old_pos: 26, new_pos: 4, type: "old", text: "-    @cmd_output = \"\"" },
+          { old_pos: 27, new_pos: 4, type: "old", text: "-    @cmd_status = 0" },
+          { old_pos: 28, new_pos: 4, type: "old", text: "-" },
+          { old_pos: 29, new_pos: 4, type: "old", text: "-    Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|" },
+          { old_pos: 30, new_pos: 4, type: "old", text: "-      @cmd_output << stdout.read" },
+          { old_pos: 31, new_pos: 4, type: "old", text: "-      @cmd_output << stderr.read" },
+          { old_pos: 32, new_pos: 4, type: "old", text: "-      @cmd_status = wait_thr.value.exitstatus" },
+          { old_pos: 33, new_pos: 4, type: "old", text: "-    end" },
+          { old_pos: 34, new_pos: 4, type: "old", text: "-" },
+          { old_pos: 35, new_pos: 4, type: "old", text: "-    return @cmd_output, @cmd_status" },
+          { old_pos: 36, new_pos: 4, type: "old", text: "-  end" },
+          { old_pos: 37, new_pos: 4, type: "old", text: "-end" },
+          { old_pos: 38, new_pos: 4, type: "new", text: "+# parsed suggestion content" },
+          { old_pos: 38, new_pos: 5, type: "new", text: "+# with comments" }
+        ]
+      end
 
       it_behaves_like 'correct suggestion raw content'
     end
@@ -70,17 +133,19 @@ describe Gitlab::Diff::Suggestion do
       let(:expected_below) { below }
       let(:expected_above) { above }
       let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) }
-
-      it_behaves_like 'correct suggestion raw content'
-    end
-
-    context 'when no extra lines (single-line suggestion)' do
-      let(:line) { 5 }
-      let(:above) { 0 }
-      let(:below) { 0 }
-      let(:expected_below) { below }
-      let(:expected_above) { above }
-      let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) }
+      let(:expected_diff_lines) do
+        [
+          { old_pos: 3, new_pos: 3, type: "match", text: "@@ -3 +3" },
+          { old_pos: 3, new_pos: 3, type: "old", text: "-" },
+          { old_pos: 4, new_pos: 3, type: "old", text: "-module Popen" },
+          { old_pos: 5, new_pos: 3, type: "old", text: "-  extend self" },
+          { old_pos: 6, new_pos: 3, type: "old", text: "-" },
+          { old_pos: 7, new_pos: 3, type: "old", text: "-  def popen(cmd, path=nil)" },
+          { old_pos: 8, new_pos: 3, type: "old", text: "-    unless cmd.is_a?(Array)" },
+          { old_pos: 9, new_pos: 3, type: "new", text: "+# parsed suggestion content" },
+          { old_pos: 9, new_pos: 4, type: "new", text: "+# with comments" }
+        ]
+      end
 
       it_behaves_like 'correct suggestion raw content'
     end
diff --git a/spec/lib/gitlab/diff/suggestions_parser_spec.rb b/spec/lib/gitlab/diff/suggestions_parser_spec.rb
index 1119ea04995..1f2af42f6e7 100644
--- a/spec/lib/gitlab/diff/suggestions_parser_spec.rb
+++ b/spec/lib/gitlab/diff/suggestions_parser_spec.rb
@@ -69,5 +69,66 @@ describe Gitlab::Diff::SuggestionsParser do
                                                   lines_below: 0)
       end
     end
+
+    context 'multi-line suggestions' do
+      let(:markdown) do
+        <<-MARKDOWN.strip_heredoc
+          ```suggestion:-2+1
+            # above and below
+          ```
+
+          ```
+            nothing
+          ```
+
+          ```suggestion:-3
+            # only above
+          ```
+
+          ```suggestion:+3
+            # only below
+          ```
+
+          ```thing
+            this is not a suggestion, it's a thing
+          ```
+        MARKDOWN
+      end
+
+      it 'returns a list of Gitlab::Diff::Suggestion' do
+        expect(subject).to all(be_a(Gitlab::Diff::Suggestion))
+        expect(subject.size).to eq(3)
+      end
+
+      it 'suggestion with above and below param has correct data' do
+        from_line = position.new_line - 2
+        to_line = position.new_line + 1
+
+        expect(subject.first.to_hash).to include(from_content: blob_lines_data(from_line, to_line),
+                                                 to_content: "  # above and below\n",
+                                                 lines_above: 2,
+                                                 lines_below: 1)
+      end
+
+      it 'suggestion with above param has correct data' do
+        from_line = position.new_line - 3
+        to_line = position.new_line
+
+        expect(subject.second.to_hash).to eq(from_content: blob_lines_data(from_line, to_line),
+                                             to_content: "  # only above\n",
+                                             lines_above: 3,
+                                             lines_below: 0)
+      end
+
+      it 'suggestion with below param has correct data' do
+        from_line = position.new_line
+        to_line = position.new_line + 3
+
+        expect(subject.third.to_hash).to eq(from_content: blob_lines_data(from_line, to_line),
+                                            to_content: "  # only below\n",
+                                            lines_above: 0,
+                                            lines_below: 3)
+      end
+    end
   end
 end
diff --git a/spec/models/suggestion_spec.rb b/spec/models/suggestion_spec.rb
index cafc725dddb..8d4e9070b19 100644
--- a/spec/models/suggestion_spec.rb
+++ b/spec/models/suggestion_spec.rb
@@ -21,6 +21,22 @@ describe Suggestion do
     end
   end
 
+  describe '#diff_lines' do
+    let(:suggestion) { create(:suggestion, :content_from_repo) }
+
+    it 'returns parsed diff lines' do
+      expected_diff_lines = Gitlab::Diff::SuggestionDiff.new(suggestion).diff_lines
+      diff_lines = suggestion.diff_lines
+
+      expect(diff_lines.size).to eq(expected_diff_lines.size)
+      expect(diff_lines).to all(be_a(Gitlab::Diff::Line))
+
+      expected_diff_lines.each_with_index do |expected_line, index|
+        expect(diff_lines[index].to_hash).to eq(expected_line.to_hash)
+      end
+    end
+  end
+
   describe '#appliable?' do
     context 'when note does not support suggestions' do
       it 'returns false' do
diff --git a/spec/serializers/suggestion_entity_spec.rb b/spec/serializers/suggestion_entity_spec.rb
index d38fc2b132b..d282a7f9c7a 100644
--- a/spec/serializers/suggestion_entity_spec.rb
+++ b/spec/serializers/suggestion_entity_spec.rb
@@ -13,8 +13,7 @@ describe SuggestionEntity do
   subject { entity.as_json }
 
   it 'exposes correct attributes' do
-    expect(subject).to include(:id, :from_line, :to_line, :appliable,
-                               :applied, :from_content, :to_content)
+    expect(subject.keys).to match_array([:id, :appliable, :applied, :diff_lines, :current_user])
   end
 
   it 'exposes current user abilities' do
diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb
index 85515d548a7..a1d31464e07 100644
--- a/spec/services/preview_markdown_service_spec.rb
+++ b/spec/services/preview_markdown_service_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe PreviewMarkdownService do
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, :repository) }
 
   before do
     project.add_developer(user)
@@ -20,23 +20,72 @@ describe PreviewMarkdownService do
   end
 
   describe 'suggestions' do
-    let(:params) { { text: "```suggestion\nfoo\n```", preview_suggestions: preview_suggestions } }
+    let(:merge_request) do
+      create(:merge_request, target_project: project, source_project: project)
+    end
+    let(:text) { "```suggestion\nfoo\n```" }
+    let(:params) do
+      suggestion_params.merge(text: text,
+                              target_type: 'MergeRequest',
+                              target_id: merge_request.iid)
+    end
     let(:service) { described_class.new(project, user, params) }
 
     context 'when preview markdown param is present' do
-      let(:preview_suggestions) { true }
+      let(:path) { "files/ruby/popen.rb" }
+      let(:line) { 10 }
+      let(:diff_refs) { merge_request.diff_refs }
+
+      let(:suggestion_params) do
+        {
+          preview_suggestions: true,
+          file_path: path,
+          line: line,
+          base_sha: diff_refs.base_sha,
+          start_sha: diff_refs.start_sha,
+          head_sha: diff_refs.head_sha
+        }
+      end
+
+      it 'returns suggestions referenced in text' do
+        position = Gitlab::Diff::Position.new(new_path: path,
+                                              new_line: line,
+                                              diff_refs: diff_refs)
+
+        expect(Gitlab::Diff::SuggestionsParser)
+          .to receive(:parse)
+          .with(text, position: position, project: merge_request.project)
+          .and_call_original
 
-      it 'returns users referenced in text' do
         result = service.execute
 
-        expect(result[:suggestions]).to eq(['foo'])
+        expect(result[:suggestions]).to all(be_a(Gitlab::Diff::Suggestion))
+      end
+
+      context 'when user is not authorized' do
+        let(:another_user) { create(:user) }
+        let(:service) { described_class.new(project, another_user, params) }
+
+        before do
+          project.add_guest(another_user)
+        end
+
+        it 'returns no suggestions' do
+          result = service.execute
+
+          expect(result[:suggestions]).to be_empty
+        end
       end
     end
 
     context 'when preview markdown param is not present' do
-      let(:preview_suggestions) { false }
+      let(:suggestion_params) do
+        {
+          preview_suggestions: false
+        }
+      end
 
-      it 'returns users referenced in text' do
+      it 'returns suggestions referenced in text' do
         result = service.execute
 
         expect(result[:suggestions]).to eq([])
@@ -49,8 +98,8 @@ describe PreviewMarkdownService do
     let(:params) do
       {
         text: "Please do it\n/assign #{user.to_reference}",
-        quick_actions_target_type: 'Issue',
-        quick_actions_target_id: issue.id
+        target_type: 'Issue',
+        target_id: issue.id
       }
     end
     let(:service) { described_class.new(project, user, params) }
@@ -72,7 +121,7 @@ describe PreviewMarkdownService do
     let(:params) do
       {
         text: "My work\n/estimate 2y",
-        quick_actions_target_type: 'MergeRequest'
+        target_type: 'MergeRequest'
       }
     end
     let(:service) { described_class.new(project, user, params) }
@@ -96,8 +145,8 @@ describe PreviewMarkdownService do
     let(:params) do
       {
         text: "My work\n/tag v1.2.3 Stable release",
-        quick_actions_target_type: 'Commit',
-        quick_actions_target_id: commit.id
+        target_type: 'Commit',
+        target_id: commit.id
       }
     end
     let(:service) { described_class.new(project, user, params) }
diff --git a/spec/services/suggestions/apply_service_spec.rb b/spec/services/suggestions/apply_service_spec.rb
index 80b5dcac6c7..7732767137c 100644
--- a/spec/services/suggestions/apply_service_spec.rb
+++ b/spec/services/suggestions/apply_service_spec.rb
@@ -51,6 +51,10 @@ describe Suggestions::ApplyService do
                                diff_refs: merge_request.diff_refs)
   end
 
+  let(:diff_note) do
+    create(:diff_note_on_merge_request, noteable: merge_request, position: position, project: project)
+  end
+
   let(:suggestion) do
     create(:suggestion, :content_from_repo, note: diff_note,
                                             to_content: "      raise RuntimeError, 'Explosion'\n      # explosion?\n")
@@ -108,12 +112,6 @@ describe Suggestions::ApplyService do
                                target_project: project)
       end
 
-      let!(:diff_note) do
-        create(:diff_note_on_merge_request, noteable: merge_request,
-                                            position: position,
-                                            project: project)
-      end
-
       before do
         project.add_maintainer(user)
       end
@@ -192,11 +190,6 @@ describe Suggestions::ApplyService do
           CONTENT
         end
 
-        let(:merge_request) do
-          create(:merge_request, source_project: project,
-                                 target_project: project)
-        end
-
         def create_suggestion(diff, old_line: nil, new_line: nil, from_content:, to_content:, path:)
           position = Gitlab::Diff::Position.new(old_path: path,
                                                 new_path: path,
@@ -291,6 +284,55 @@ describe Suggestions::ApplyService do
           expect(suggestion_2_diff.strip).to eq(expected_suggestion_2_diff.strip)
         end
       end
+
+      context 'multi-line suggestion' do
+        let(:expected_content) do
+          <<~CONTENT
+            require 'fileutils'
+            require 'open3'
+
+            module Popen
+              extend self
+
+            # multi
+            # line
+
+                vars = {
+                  "PWD" => path
+                }
+
+                options = {
+                  chdir: path
+                }
+
+                unless File.directory?(path)
+                  FileUtils.mkdir_p(path)
+                end
+
+                @cmd_output = ""
+                @cmd_status = 0
+
+                Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+                  @cmd_output << stdout.read
+                  @cmd_output << stderr.read
+                  @cmd_status = wait_thr.value.exitstatus
+                end
+
+                return @cmd_output, @cmd_status
+              end
+            end
+          CONTENT
+        end
+
+        let(:suggestion) do
+          create(:suggestion, :content_from_repo, note: diff_note,
+                                                  lines_above: 2,
+                                                  lines_below: 3,
+                                                  to_content: "# multi\n# line\n")
+        end
+
+        it_behaves_like 'successfully creates commit and updates suggestion'
+      end
     end
 
     context 'fork-project' do
-- 
cgit v1.2.1


From 7af20f8e510ed389ba39995038d10369a93afe77 Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Thu, 4 Apr 2019 14:02:42 +0100
Subject: Fix an order-dependent spec failure in
 spec/migrations/schedule_sync_issuables_state_id_spec.rb

---
 spec/migrations/schedule_runners_token_encryption_spec.rb | 2 +-
 spec/migrations/schedule_sync_issuables_state_id_spec.rb  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/migrations/schedule_runners_token_encryption_spec.rb b/spec/migrations/schedule_runners_token_encryption_spec.rb
index 376d2795277..97ff6c128f3 100644
--- a/spec/migrations/schedule_runners_token_encryption_spec.rb
+++ b/spec/migrations/schedule_runners_token_encryption_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20181121111200_schedule_runners_token_encryption')
 
-describe ScheduleRunnersTokenEncryption, :migration do
+describe ScheduleRunnersTokenEncryption, :migration, :sidekiq do
   let(:settings) { table(:application_settings) }
   let(:namespaces) { table(:namespaces) }
   let(:projects) { table(:projects) }
diff --git a/spec/migrations/schedule_sync_issuables_state_id_spec.rb b/spec/migrations/schedule_sync_issuables_state_id_spec.rb
index bf974d60b24..bc94f8820bd 100644
--- a/spec/migrations/schedule_sync_issuables_state_id_spec.rb
+++ b/spec/migrations/schedule_sync_issuables_state_id_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20190214112022_schedule_sync_issuables_state_id.rb')
 
-describe ScheduleSyncIssuablesStateId, :migration do
+describe ScheduleSyncIssuablesStateId, :migration, :sidekiq do
   let(:namespaces) { table(:namespaces) }
   let(:projects) { table(:projects) }
   let(:merge_requests) { table(:merge_requests) }
-- 
cgit v1.2.1


From 435d98c9cee0d32305a6fd20995f41849749e8eb Mon Sep 17 00:00:00 2001
From: Jan Provaznik <jprovaznik@gitlab.com>
Date: Thu, 4 Apr 2019 13:50:31 +0000
Subject: Monitor GraphQL with Prometheus

Extends graphql's platform tracing class to observe duration
of graphql methods.

In graphql 1.8.11 is added prometheus class but it's not very useful
for us because it uses prometheus_exporter to export results.
---
 app/graphql/gitlab_schema.rb                 |  1 +
 changelogs/unreleased/graphql-prometheus.yml |  5 ++++
 lib/gitlab/graphql/tracing.rb                | 43 ++++++++++++++++++++++++++++
 spec/lib/gitlab/graphql/tracing_spec.rb      | 33 +++++++++++++++++++++
 4 files changed, 82 insertions(+)
 create mode 100644 changelogs/unreleased/graphql-prometheus.yml
 create mode 100644 lib/gitlab/graphql/tracing.rb
 create mode 100644 spec/lib/gitlab/graphql/tracing_spec.rb

diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index 06d26309b5b..ecc34eacc7d 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/app/graphql/gitlab_schema.rb
@@ -5,6 +5,7 @@ class GitlabSchema < GraphQL::Schema
   use Gitlab::Graphql::Authorize
   use Gitlab::Graphql::Present
   use Gitlab::Graphql::Connections
+  use Gitlab::Graphql::Tracing
 
   query(Types::QueryType)
 
diff --git a/changelogs/unreleased/graphql-prometheus.yml b/changelogs/unreleased/graphql-prometheus.yml
new file mode 100644
index 00000000000..180577f3aec
--- /dev/null
+++ b/changelogs/unreleased/graphql-prometheus.yml
@@ -0,0 +1,5 @@
+---
+title: Added prometheus monitoring to GraphQL
+merge_request:
+author:
+type: added
diff --git a/lib/gitlab/graphql/tracing.rb b/lib/gitlab/graphql/tracing.rb
new file mode 100644
index 00000000000..6b505e4262b
--- /dev/null
+++ b/lib/gitlab/graphql/tracing.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    class Tracing < GraphQL::Tracing::PlatformTracing
+      self.platform_keys = {
+        'lex' => 'graphql.lex',
+        'parse' => 'graphql.parse',
+        'validate' => 'graphql.validate',
+        'analyze_query' => 'graphql.analyze',
+        'analyze_multiplex' => 'graphql.analyze',
+        'execute_multiplex' => 'graphql.execute',
+        'execute_query' => 'graphql.execute',
+        'execute_query_lazy' => 'graphql.execute',
+        'execute_field' => 'graphql.execute',
+        'execute_field_lazy' => 'graphql.execute'
+      }
+
+      def platform_field_key(type, field)
+        "#{type.name}.#{field.name}"
+      end
+
+      def platform_trace(platform_key, key, data, &block)
+        start = Gitlab::Metrics::System.monotonic_time
+
+        yield
+      ensure
+        duration = Gitlab::Metrics::System.monotonic_time - start
+
+        graphql_duration_seconds.observe({ platform_key: platform_key, key: key }, duration)
+      end
+
+      private
+
+      def graphql_duration_seconds
+        @graphql_duration_seconds ||= Gitlab::Metrics.histogram(
+          :graphql_duration_seconds,
+          'GraphQL execution time'
+        )
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/graphql/tracing_spec.rb b/spec/lib/gitlab/graphql/tracing_spec.rb
new file mode 100644
index 00000000000..7300a9a572e
--- /dev/null
+++ b/spec/lib/gitlab/graphql/tracing_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Graphql::Tracing do
+  let(:graphql_duration_seconds_histogram) { double('Gitlab::Metrics::NullMetric') }
+
+  it 'updates graphql histogram with expected labels' do
+    query = 'query { users { id } }'
+    tracer = described_class.new
+
+    allow(tracer)
+      .to receive(:graphql_duration_seconds)
+      .and_return(graphql_duration_seconds_histogram)
+
+    expect_metric('graphql.lex', 'lex')
+    expect_metric('graphql.parse', 'parse')
+    expect_metric('graphql.validate', 'validate')
+    expect_metric('graphql.analyze', 'analyze_multiplex')
+    expect_metric('graphql.execute', 'execute_query_lazy')
+    expect_metric('graphql.execute', 'execute_multiplex')
+
+    GitlabSchema.execute(query, context: { tracers: [tracer] })
+  end
+
+  private
+
+  def expect_metric(platform_key, key)
+    expect(graphql_duration_seconds_histogram)
+      .to receive(:observe)
+      .with({ platform_key: platform_key, key: key }, be > 0.0)
+  end
+end
-- 
cgit v1.2.1


From 17bee986bc971cc7d04c4b767cc026577eb56c6a Mon Sep 17 00:00:00 2001
From: Gosia Ksionek <mksionek@gitlab.com>
Date: Thu, 4 Apr 2019 14:19:57 +0000
Subject: Add cr remarks

Chnage method used in model to make it
more efficient database-wise
Add additional spec
---
 app/models/group.rb                                |   7 +-
 .../unreleased/38564-cant-leave-subgroup.yml       |   5 +
 spec/models/group_spec.rb                          |  26 +++++
 spec/policies/group_member_policy_spec.rb          | 105 +++++++++++++++++++++
 4 files changed, 139 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/unreleased/38564-cant-leave-subgroup.yml
 create mode 100644 spec/policies/group_member_policy_spec.rb

diff --git a/app/models/group.rb b/app/models/group.rb
index c77586c4cdc..ac66815705c 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -228,22 +228,21 @@ class Group < Namespace
   def has_owner?(user)
     return false unless user
 
-    members_with_parents.owners.where(user_id: user).any?
+    members_with_parents.owners.exists?(user_id: user)
   end
 
   def has_maintainer?(user)
     return false unless user
 
-    members_with_parents.maintainers.where(user_id: user).any?
+    members_with_parents.maintainers.exists?(user_id: user)
   end
 
   # @deprecated
   alias_method :has_master?, :has_maintainer?
 
   # Check if user is a last owner of the group.
-  # Parent owners are ignored for nested groups.
   def last_owner?(user)
-    owners.include?(user) && owners.size == 1
+    has_owner?(user) && members_with_parents.owners.size == 1
   end
 
   def ldap_synced?
diff --git a/changelogs/unreleased/38564-cant-leave-subgroup.yml b/changelogs/unreleased/38564-cant-leave-subgroup.yml
new file mode 100644
index 00000000000..a6397062343
--- /dev/null
+++ b/changelogs/unreleased/38564-cant-leave-subgroup.yml
@@ -0,0 +1,5 @@
+---
+title: Allow removing last owner from subgroup if parent group has owners
+merge_request: 26718
+author:
+type: changed
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 2c6abddca17..b2ffd5812ab 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -364,6 +364,32 @@ describe Group do
     it { expect(group.has_maintainer?(nil)).to be_falsey }
   end
 
+  describe '#last_owner?' do
+    before do
+      @members = setup_group_members(group)
+    end
+
+    it { expect(group.last_owner?(@members[:owner])).to be_truthy }
+
+    context 'with two owners' do
+      before do
+        create(:group_member, :owner, group: group)
+      end
+
+      it { expect(group.last_owner?(@members[:owner])).to be_falsy }
+    end
+
+    context 'with owners from a parent', :postgresql do
+      before do
+        parent_group = create(:group)
+        create(:group_member, :owner, group: parent_group)
+        group.update(parent: parent_group)
+      end
+
+      it { expect(group.last_owner?(@members[:owner])).to be_falsy }
+    end
+  end
+
   describe '#lfs_enabled?' do
     context 'LFS enabled globally' do
       before do
diff --git a/spec/policies/group_member_policy_spec.rb b/spec/policies/group_member_policy_spec.rb
new file mode 100644
index 00000000000..7bd7184cffe
--- /dev/null
+++ b/spec/policies/group_member_policy_spec.rb
@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GroupMemberPolicy do
+  let(:guest) { create(:user) }
+  let(:owner) { create(:user) }
+  let(:group) { create(:group, :private) }
+
+  before do
+    group.add_guest(guest)
+    group.add_owner(owner)
+  end
+
+  let(:member_related_permissions) do
+    [:update_group_member, :destroy_group_member]
+  end
+
+  let(:membership) { current_user.members.first }
+
+  subject { described_class.new(current_user, membership) }
+
+  def expect_allowed(*permissions)
+    permissions.each { |p| is_expected.to be_allowed(p) }
+  end
+
+  def expect_disallowed(*permissions)
+    permissions.each { |p| is_expected.not_to be_allowed(p) }
+  end
+
+  context 'with guest user' do
+    let(:current_user) { guest }
+
+    it do
+      expect_disallowed(:member_related_permissions)
+    end
+  end
+
+  context 'with one owner' do
+    let(:current_user) { owner }
+
+    it do
+      expect_disallowed(:destroy_group_member)
+      expect_disallowed(:update_group_member)
+    end
+  end
+
+  context 'with more than one owner' do
+    let(:current_user) { owner }
+
+    before do
+      group.add_owner(create(:user))
+    end
+
+    it do
+      expect_allowed(:destroy_group_member)
+      expect_allowed(:update_group_member)
+    end
+  end
+
+  context 'with the group parent', :postgresql do
+    let(:current_user) { create :user }
+    let(:subgroup) { create(:group, :private, parent: group)}
+
+    before do
+      group.add_owner(owner)
+      subgroup.add_owner(current_user)
+    end
+
+    it do
+      expect_allowed(:destroy_group_member)
+      expect_allowed(:update_group_member)
+    end
+  end
+
+  context 'without group parent' do
+    let(:current_user) { create :user }
+    let(:subgroup) { create(:group, :private)}
+
+    before do
+      subgroup.add_owner(current_user)
+    end
+
+    it do
+      expect_disallowed(:destroy_group_member)
+      expect_disallowed(:update_group_member)
+    end
+  end
+
+  context 'without group parent with two owners' do
+    let(:current_user) { create :user }
+    let(:other_user) { create :user }
+    let(:subgroup) { create(:group, :private)}
+
+    before do
+      subgroup.add_owner(current_user)
+      subgroup.add_owner(other_user)
+    end
+
+    it do
+      expect_allowed(:destroy_group_member)
+      expect_allowed(:update_group_member)
+    end
+  end
+end
-- 
cgit v1.2.1