WIP: Download patch with code comments for unresolved discussionsdm-download-discussions-as-patch

13 files changed, 466 insertions, 12 deletions

diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index ad6e7cf501d..a4d988cf49c 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -1,5 +1,6 @@
 <script>
 import { mapActions, mapGetters } from 'vuex';
+import Icon from '~/vue_shared/components/icon.vue';
 import resolveSvg from 'icons/_icon_resolve_discussion.svg';
 import resolvedSvg from 'icons/_icon_status_success_solid.svg';
 import mrIssueSvg from 'icons/_icon_mr_issue.svg';
@@ -9,6 +10,9 @@ import discussionNavigation from '../mixins/discussion_navigation';
 import tooltip from '../../vue_shared/directives/tooltip';
 
 export default {
+  components: {
+    Icon,
+  },
   directives: {
     tooltip,
   },
@@ -80,10 +84,10 @@ export default {
         </span>
       </div>
       <div
-        v-if="resolveAllDiscussionsIssuePath && !allResolved"
         class="btn-group"
         role="group">
         <a
+          v-if="resolveAllDiscussionsIssuePath && !allResolved"
           v-tooltip
           :href="resolveAllDiscussionsIssuePath"
           :title="s__('Resolve all discussions in new issue')"
@@ -91,12 +95,19 @@ export default {
           class="new-issue-for-discussion btn btn-default discussion-create-issue-btn">
           <span v-html="mrIssueSvg"></span>
         </a>
-      </div>
-      <div
-        v-if="isLoggedIn && !allResolved"
-        class="btn-group"
-        role="group">
         <button
+          v-if="!allResolved"
+          v-tooltip
+          data-target="#modal_download_as_patch"
+          data-toggle="modal"
+          :title="s__('Download unresolved discussions as code comments')"
+          data-container="body"
+          class="btn btn-default"
+          type="button">
+          <icon name="download" />
+        </button>
+        <button
+          v-if="isLoggedIn && !allResolved"
           v-tooltip
           title="Jump to first unresolved discussion"
           data-container="body"
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 97b131687d3..3efb4ab9f21 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -503,7 +503,7 @@
   display: none;
 }
 
-#modal_merge_info .modal-dialog {
+.merge-request-modal .modal-dialog {
   .dark {
     margin-right: 40px;
   }
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index dfb69de650b..0881d168ef9 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -77,6 +77,26 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     end
   end
 
+  def discussions
+    respond_to do |format|
+      format.json do
+        super
+      end
+
+      format.patch do
+        return render_404 unless commit = commit_with_unresolved_discussions
+
+        send_git_patch @project.repository, commit.diff_refs
+      end
+
+      format.diff do
+        return render_404 unless commit = commit_with_unresolved_discussions
+
+        send_git_diff @project.repository, commit.diff_refs
+      end
+    end
+  end
+
   def commits
     # Get commits from repository
     # or from cache if already merged
@@ -353,4 +373,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     # Also see https://gitlab.com/gitlab-org/gitlab-ce/issues/42441
     Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42438')
   end
+
+  def commit_with_unresolved_discussions
+    @commit_with_unresolved_discussions ||= Discussions::CommitWithUnresolvedDiscussionsService.new(project, current_user).execute(merge_request)
+  end
 end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 49c36ad9d3f..726c35c9b93 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -464,7 +464,7 @@ class Commit
     # We don't want to do anything for `Commit` model, so this is empty.
   end
 
-  WIP_REGEX = /\A\s*(((?i)(\[WIP\]|WIP:|WIP)\s|WIP$))|(fixup!|squash!)\s/.freeze
+  WIP_REGEX = /\A\s*(((?i)(\[WIP\]|WIP:|WIP)\s|WIP$))|(fixup!|squash!|FIXME:)\s/.freeze
 
   def work_in_progress?
     !!(title =~ WIP_REGEX)
diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb
index 00dec6bb92b..1c6a709af12 100644
--- a/app/models/legacy_diff_note.rb
+++ b/app/models/legacy_diff_note.rb
@@ -43,6 +43,10 @@ class LegacyDiffNote < Note
     self.line_code
   end
 
+  def position
+    @position ||= diff_file.position_for_line_code(self.line_code) if active?
+  end
+
   # Check if this note is part of an "active" discussion
   #
   # This will always return true for anything except MergeRequest noteables,
diff --git a/app/services/discussions/commit_with_unresolved_discussions_service.rb b/app/services/discussions/commit_with_unresolved_discussions_service.rb
new file mode 100644
index 00000000000..550146125e3
--- /dev/null
+++ b/app/services/discussions/commit_with_unresolved_discussions_service.rb
@@ -0,0 +1,61 @@
+module Discussions
+  class CommitWithUnresolvedDiscussionsService < Discussions::BaseService
+    def execute(merge_request)
+      @merge_request = merge_request
+
+      cached_commit || create_commit
+    end
+
+    private
+
+    def update_actions
+      @update_actions ||= UpdateActions.new(@merge_request)
+    end
+
+    def cache_key
+      @cache_key ||= [
+        'merge-request',
+        @merge_request.id,
+        'update-actions',
+        update_actions.cache_key
+      ]
+    end
+
+    def cached_commit
+      return if Rails.env.development?
+
+      commit_sha = Rails.cache.read(cache_key)
+      commit_sha && project.commit(commit_sha)
+    end
+
+    def create_commit
+      return unless @merge_request.source_branch_exists?
+
+      commit_sha = repository.multi_action(
+        @merge_request.author,
+        branch_name: nil, # We just want a commit, not a branch
+        message: commit_message,
+        start_branch_name: @merge_request.source_branch,
+        start_project: @merge_request.source_project,
+        actions: update_actions.actions
+      )
+      return unless commit_sha
+
+      Rails.cache.write(cache_key, commit_sha)
+
+      project.commit(commit_sha)
+    end
+
+    def commit_message
+      <<~MSG
+        FIXME: Add code comments for unresolved discussions
+
+        This patch adds a code comment for every unresolved discussion on the
+        latest version of the diff of #{@merge_request.to_reference(full: true)}.
+
+        This commit was automatically generated by GitLab. Don't forget to
+        remove it from the merge request source branch before it is merged.
+      MSG
+    end
+  end
+end
diff --git a/app/services/discussions/commit_with_unresolved_discussions_service/commenter.rb b/app/services/discussions/commit_with_unresolved_discussions_service/commenter.rb
new file mode 100644
index 00000000000..1caaf5c30d7
--- /dev/null
+++ b/app/services/discussions/commit_with_unresolved_discussions_service/commenter.rb
@@ -0,0 +1,101 @@
+module Discussions
+  class CommitWithUnresolvedDiscussionsService
+    class Commenter
+      # Commenters can be strings or lambdas.
+      # Use a string for single-line comments: each line will be prefixed
+      # Use a lambda for multi-line comments: it will be called with the full text
+      COMMENTERS = {
+        double_slash: '// ', # Substitution applied to each line
+        double_stroke: '-- ', # Substitution applied to each line
+        pound:        '# ',
+        percent:      '% ',
+        quote:        "'",
+        bang:         '!',
+        semicolon:    ';',
+        xml:          lambda { |text| "<!--\n#{text}-->" }
+      }.freeze
+
+      # Maps Rouge lexer (language) tags to commenters.
+      COMMENT_TYPES_BY_LANG = {
+        default: :double_slash,
+
+        actionscript: :double_slash,
+        c: :double_slash,
+        csharp: :double_slash,
+        d: :double_slash,
+        cpp: :double_slash,
+        fsharp: :double_slash,
+        go: :double_slash,
+        php: :double_slash,
+        java: :double_slash,
+        kotlin: :double_slash,
+        javascript: :double_slash,
+        objective_c: :double_slash,
+        rust: :double_slash,
+        scala: :double_slash,
+        swift: :double_slash,
+        sass: :double_slash,
+
+        markdown: :xml,
+        html: :xml,
+        xml: :xml,
+
+        shell: :pound,
+        perl: :pound,
+        python: :pound,
+        powershell: :pound,
+        r: :pound,
+        ruby: :pound,
+        make: :pound,
+        elixir: :pound,
+        yaml: :pound,
+
+        tex: :percent,
+        prolog: :percent,
+        matlab: :percent,
+        erlang: :percent,
+
+        vb: :quote,
+
+        fortran: :bang,
+
+        lisp: :semicolon,
+        common_lisp: :semicolon,
+        clojure: :semicolon,
+        scheme: :semicolon,
+
+        haskell: :double_stroke,
+        sql: :double_stroke,
+        lua: :double_stroke,
+      }.freeze
+
+      attr_reader :format
+
+      def initialize(comment_type = :default)
+        @format = COMMENTERS[comment_type] || COMMENTERS[:default]
+      end
+
+      def self.for_lang(lang)
+        comment_type = COMMENT_TYPES_BY_LANG[lang] || COMMENT_TYPES_BY_LANG[:default]
+        new(comment_type)
+      end
+
+      def self.for_blob(blob)
+        lexer = Gitlab::Highlight.new(blob.name, blob.data, repository: blob.project.repository).lexer
+        for_lang(lexer.tag.to_sym)
+      end
+
+      def apply(text)
+        # Lambdas are used for multi-line comments, strings for single-line. See comment by the COMMENTERS constant.
+        if format.respond_to?(:call)
+          format.call(text) << "\n"
+        else
+          # With single line comments, we insert an extra newline at the end to
+          # create some distance between the comment and the next line of code.
+          text = "#{text}\n"
+          text.gsub(/^/, format)
+        end
+      end
+    end
+  end
+end
diff --git a/app/services/discussions/commit_with_unresolved_discussions_service/inserter.rb b/app/services/discussions/commit_with_unresolved_discussions_service/inserter.rb
new file mode 100644
index 00000000000..9fb29a9e1f4
--- /dev/null
+++ b/app/services/discussions/commit_with_unresolved_discussions_service/inserter.rb
@@ -0,0 +1,35 @@
+module Discussions
+  class CommitWithUnresolvedDiscussionsService
+    class Inserter
+      attr_reader :blob
+
+      def initialize(blob)
+        @blob = blob
+      end
+
+      def insert(insertions)
+        commenter = Commenter.for_blob(blob)
+
+        line_offset = 0
+        insertions.sort.each do |insertion|
+          insertion_length = insertion.insert(lines, commenter, offset: line_offset)
+          line_offset += insertion_length
+        end
+
+        lines.join
+      end
+
+      private
+
+      def lines
+        @lines ||= begin
+          blob.load_all_data!
+
+          content = blob.data
+          content += "\n" unless content.end_with?("\n")
+          content.lines
+        end
+      end
+    end
+  end
+end
diff --git a/app/services/discussions/commit_with_unresolved_discussions_service/insertion.rb b/app/services/discussions/commit_with_unresolved_discussions_service/insertion.rb
new file mode 100644
index 00000000000..f94d05bd806
--- /dev/null
+++ b/app/services/discussions/commit_with_unresolved_discussions_service/insertion.rb
@@ -0,0 +1,120 @@
+module Discussions
+  class CommitWithUnresolvedDiscussionsService
+    class Insertion
+      include Comparable
+
+      attr_accessor :discussion, :override_preceding_lines
+
+      def initialize(discussion)
+        @discussion = discussion
+      end
+
+      def insert(target_lines, commenter, offset: 0)
+        line_index = line + offset
+
+        insertion_lines = text(target_lines[0..line_index-1], commenter).lines
+        target_lines.insert(line_index, *insertion_lines)
+
+        insertion_lines.length
+      end
+
+      def line
+        position.removed? ? discussion.diff_line.new_pos - 1 : position.new_line
+      end
+
+      def path
+        position.file_path
+      end
+
+      def <=>(other)
+        sort_key <=> other.sort_key
+      end
+
+      def sort_key
+        [
+          line,
+          # Comments related to a preceding line should show up before comments
+          # related to a deleted
+          # line at the same location.
+          position.removed? ? 1 : 0,
+          discussion.created_at
+        ]
+      end
+
+      private
+
+      def text(preceding_lines, commenter)
+        text = base_text.dup
+
+        text = commenter.apply(base_text) if commenter
+
+        # We determine the indentation level of the insertion based the actual
+        # preceding lines, or the original preceding lines in case of deletion.
+        preceding_lines = removed_diff_lines if position.removed?
+
+        text.gsub!(/^/, indentation(preceding_lines)) if preceding_lines
+
+        text
+      end
+
+      def base_text
+        sections = []
+
+        # When the discussion is on a deleted line, we include the preceding 5
+        # deleted lines in the comment.
+        sections << removed_lines_text if position.removed?
+
+        discussion.notes.each_with_index do |note, i|
+          sections << note_text(note, started: i == 0)
+        end
+
+        text = sections.join("\n\n")
+
+        text << "\n" unless text.end_with?("\n")
+        text
+      end
+
+      def position
+        discussion.position
+      end
+
+      def removed_diff_lines
+        @removed_diff_lines ||=
+          discussion
+            .truncated_diff_lines(highlight: false)
+            .reverse[0, 5]
+            .take_while(&:removed?)
+            .reverse
+            .map { |l| l.text[1..-1] } # Drop the - prefixes
+      end
+
+      def removed_lines_text
+        heading = "Old #{"line".pluralize(removed_diff_lines.count)}:"
+        indented_removed_lines = removed_diff_lines.join("\n").strip_heredoc.gsub(/^/, '  ')
+
+        [heading, indented_removed_lines].join("\n")
+      end
+
+      def note_text(note, started: false)
+        byline = "FIXME: #{note.author.name} (#{note.author.to_reference}) "
+        byline << (started ? "started a discussion on the preceding line:" : "commented:")
+        byline << " (Resolved by #{note.resolved_by.try(:name)})" if note.resolved?
+
+        comment = note.note.gsub(/^/, '  ')
+
+        [byline, comment].join("\n")
+      end
+
+      def indentation(preceding_lines)
+        # Find closest preceding non-blank line
+        preceding_line = preceding_lines.reverse.find(&:present?)
+        # Fall back on preceding line
+        preceding_line ||= preceding_lines.last
+
+        return unless preceding_line
+
+        preceding_line[/\A[\t ]*/]
+      end
+    end
+  end
+end
diff --git a/app/services/discussions/commit_with_unresolved_discussions_service/update_actions.rb b/app/services/discussions/commit_with_unresolved_discussions_service/update_actions.rb
new file mode 100644
index 00000000000..9e48b9d94ee
--- /dev/null
+++ b/app/services/discussions/commit_with_unresolved_discussions_service/update_actions.rb
@@ -0,0 +1,50 @@
+module Discussions
+  class CommitWithUnresolvedDiscussionsService
+    class UpdateActions
+      attr_reader :merge_request
+
+      def initialize(merge_request)
+        @merge_request = merge_request
+      end
+
+      def actions
+        insertions
+          .group_by(&:path)
+          .map do |path, insertions|
+            action_for(path, insertions)
+          end
+      end
+
+      def cache_key
+        @cache_key ||= [
+          @merge_request.diff_head_sha,
+          @merge_request.notes.diff_notes.count,
+          @merge_request.notes.diff_notes.maximum(:updated_at)
+        ]
+      end
+
+      private
+
+      def discussions
+        @discussions ||= @merge_request
+          .notes.diff_notes.inc_relations_for_view.fresh.discussions
+          .select { |d| d.on_text? && d.active? && !d.resolved? && !d.diff_file.deleted_file? && d.position }
+      end
+
+      def insertions
+        discussions.map { |d| Insertion.new(d) }
+      end
+
+      def action_for(path, insertions)
+        blob = @merge_request.project.repository.blob_at(@merge_request.diff_head_sha, path)
+        content = Inserter.new(blob).insert(insertions)
+
+        {
+          action: :update,
+          file_path: path,
+          content: content
+        }
+      end
+    end
+  end
+end
diff --git a/app/views/projects/merge_requests/_download_as_patch_modal.html.haml b/app/views/projects/merge_requests/_download_as_patch_modal.html.haml
new file mode 100644
index 00000000000..721cc8e65c2
--- /dev/null
+++ b/app/views/projects/merge_requests/_download_as_patch_modal.html.haml
@@ -0,0 +1,44 @@
+#modal_download_as_patch.merge-request-modal.modal{ tabindex: '-1' }
+  .modal-dialog.modal-lg
+    .modal-content
+      .modal-header
+        %h3.modal-title Download unresolved discussions as code comments
+        %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') }
+          %span{ "aria-hidden": true } &times;
+      .modal-body
+        %p
+          %strong Step 1.
+          Fetch and check out the branch for this merge request
+        = clipboard_button(target: "pre#download-as-patch-1", title: "Copy commands to clipboard")
+        %pre.dark#download-as-patch-1
+          - if @merge_request.for_fork?
+            :preserve
+              git fetch #{h default_url_to_repo(@merge_request.source_project)} #{h @merge_request.source_branch}
+              git checkout -b #{h @merge_request.source_project_path}-#{h @merge_request.source_branch} FETCH_HEAD
+          - else
+            :preserve
+              git fetch origin
+              git checkout -b #{h @merge_request.source_branch} origin/#{h @merge_request.source_branch}
+
+        %p
+          %strong Step 2.
+          Download patch that will add a code comment for every unresolved discussion on the latest version of the diff, and apply it to your working copy
+
+        - url = discussions_project_merge_request_url(@project, @merge_request, format: :patch)
+        - if @project.public?
+          = clipboard_button(target: "pre#download-as-patch-2", title: "Copy commands to clipboard")
+          %pre.dark#download-as-patch-2
+            :preserve
+              curl #{h url} | git apply
+        - else
+          - filename = "merge-request-#{@merge_request.iid}-discussions.patch"
+          %p
+            = link_to %Q{Download patch as "#{filename}"}, url, class: "btn", download: filename
+          = clipboard_button(target: "pre#download-as-patch-2", title: "Copy commands to clipboard")
+          %pre.dark#download-as-patch-2
+            :preserve
+              git apply #{h filename}
+
+        %p
+          %strong Step 3.
+          Find the newly added 'FIXME' comments and update the code as appropriate
diff --git a/app/views/projects/merge_requests/_how_to_merge.html.haml b/app/views/projects/merge_requests/_how_to_merge_modal.html.haml
index 15499c89ffb..d06bed9c15c 100644
--- a/app/views/projects/merge_requests/_how_to_merge.html.haml
+++ b/app/views/projects/merge_requests/_how_to_merge_modal.html.haml
@@ -1,4 +1,4 @@
-#modal_merge_info.modal{ tabindex: '-1' }
+#modal_merge_info.merge-request-modal.modal{ tabindex: '-1' }
   .modal-dialog.modal-lg
     .modal-content
       .modal-header
@@ -19,6 +19,7 @@
             :preserve
               git fetch origin
               git checkout -b #{h @merge_request.source_branch} origin/#{h @merge_request.source_branch}
+
         %p
           %strong Step 2.
           Review the changes locally
@@ -38,6 +39,7 @@
               git fetch origin
               git checkout origin/#{h @merge_request.target_branch}
               git merge --no-ff #{h @merge_request.source_branch}
+
         %p
           %strong Step 4.
           Push the result of the merge to GitLab
@@ -48,6 +50,7 @@
         - unless @merge_request.can_be_merged_by?(current_user)
           %p
             Note that pushing to GitLab requires write access to this repository.
+
         %p
           %strong Tip:
           = succeed '.' do
diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml
index b23baa22d8b..bf5e5fb25c0 100644
--- a/app/views/projects/merge_requests/show.html.haml
+++ b/app/views/projects/merge_requests/show.html.haml
@@ -6,15 +6,16 @@
 - page_description @merge_request.description
 - page_card_attributes @merge_request.card_attributes
 
+= render "projects/merge_requests/download_as_patch_modal"
+- if @merge_request.source_branch_exists?
+  = render "projects/merge_requests/how_to_merge_modal"
+
 .merge-request{ data: { mr_action: j(params[:tab].presence || 'show'), url: merge_request_path(@merge_request, format: :json), project_path: project_path(@merge_request.project) } }
   = render "projects/merge_requests/mr_title"
 
   .merge-request-details.issuable-details{ data: { id: @merge_request.project.id } }
     = render "projects/merge_requests/mr_box"
 
-    - if @merge_request.source_branch_exists?
-      = render "projects/merge_requests/how_to_merge"
-
     -# haml-lint:disable InlineJavaScript
     :javascript
       window.gl = window.gl || {};