Merge branch 'master' of https://gitlab.com/gitlab-org/gitlab-ce into 39543-milestone-page-list-redesign

8 files changed, 111 insertions, 23 deletions

diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index abc283d7aa9..6484a713f8e 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -7,6 +7,7 @@ class Projects::ArtifactsController < Projects::ApplicationController
   before_action :authorize_read_build!
   before_action :authorize_update_build!, only: [:keep]
   before_action :extract_ref_name_and_path
+  before_action :set_request_format, only: [:file]
   before_action :validate_artifacts!
   before_action :entry, only: [:file]
 
@@ -101,4 +102,12 @@ class Projects::ArtifactsController < Projects::ApplicationController
 
     render_404 unless @entry.exists?
   end
+
+  def set_request_format
+    request.format = :html if set_request_format?
+  end
+
+  def set_request_format?
+    request.format != :json
+  end
 end
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 7c65f1f5dfe..ebc61264b39 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -3,10 +3,11 @@ class Projects::BlobController < Projects::ApplicationController
   include ExtractsPath
   include CreatesCommit
   include RendersBlob
+  include NotesHelper
   include ActionView::Helpers::SanitizeHelper
-
   prepend_before_action :authenticate_user!, only: [:edit]
 
+  before_action :set_request_format, only: [:edit, :show, :update]
   before_action :require_non_empty_project, except: [:new, :create]
   before_action :authorize_download_code!
 
@@ -92,6 +93,7 @@ class Projects::BlobController < Projects::ApplicationController
     @lines = Gitlab::Highlight.highlight(@blob.path, @blob.data, repository: @repository).lines
 
     @form = UnfoldForm.new(params)
+
     @lines = @lines[@form.since - 1..@form.to - 1].map(&:html_safe)
 
     if @form.bottom?
@@ -102,11 +104,50 @@ class Projects::BlobController < Projects::ApplicationController
       @match_line = "@@ -#{line}+#{line} @@"
     end
 
-    render layout: false
+    # We can keep only 'render_diff_lines' from this conditional when
+    # https://gitlab.com/gitlab-org/gitlab-ce/issues/44988 is done
+    if rendered_for_merge_request?
+      render_diff_lines
+    else
+      render layout: false
+    end
   end
 
   private
 
+  # Converts a String array to Gitlab::Diff::Line array
+  def render_diff_lines
+    @lines.map! do |line|
+      # These are marked as context lines but are loaded from blobs.
+      # We also have context lines loaded from diffs in other places.
+      diff_line = Gitlab::Diff::Line.new(line, 'context', nil, nil, nil)
+      diff_line.rich_text = line
+      diff_line
+    end
+
+    add_match_line
+
+    render json: @lines
+  end
+
+  def add_match_line
+    return unless @form.unfold?
+
+    if @form.bottom? && @form.to < @blob.lines.size
+      old_pos = @form.to - @form.offset
+      new_pos = @form.to
+    elsif @form.since != 1
+      old_pos = new_pos = @form.since
+    end
+
+    # Match line is not needed when it reaches the top limit or bottom limit of the file.
+    return unless new_pos
+
+    @match_line = Gitlab::Diff::Line.new(@match_line, 'match', nil, old_pos, new_pos)
+
+    @form.bottom? ? @lines.push(@match_line) : @lines.unshift(@match_line)
+  end
+
   def blob
     @blob ||= @repository.blob_at(@commit.id, @path)
 
@@ -188,6 +229,18 @@ class Projects::BlobController < Projects::ApplicationController
       .last_for_path(@repository, @ref, @path).sha
   end
 
+  # In Rails 4.2 if params[:format] is empty, Rails set it to :html
+  # But since Rails 5.0 the framework now looks for an extension.
+  # E.g. for `blob/master/CHANGELOG.md` in Rails 4 the format would be `:html`, but in Rails 5 on it'd be `:md`
+  # This before_action explicitly sets the `:html` format for all requests unless `:format` is set by a client e.g. by JS for XHR requests.
+  def set_request_format
+    request.format = :html if set_request_format?
+  end
+
+  def set_request_format?
+    params[:id].present? && params[:format].blank? && request.format != "json"
+  end
+
   def show_html
     environment_params = @repository.branch_exists?(@ref) ? { ref: @ref } : { commit: @commit }
     @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index b7b36f770f5..cd7250b10fc 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -31,7 +31,10 @@ class Projects::BranchesController < Projects::ApplicationController
           end
         end
 
-        render
+        # https://gitlab.com/gitlab-org/gitlab-ce/issues/48097
+        Gitlab::GitalyClient.allow_n_plus_1_calls do
+          render
+        end
       end
       format.json do
         branches = BranchesFinder.new(@repository, params).execute
diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb
index 7b7cb52d7ed..9e495061f4e 100644
--- a/app/controllers/projects/commits_controller.rb
+++ b/app/controllers/projects/commits_controller.rb
@@ -9,6 +9,7 @@ class Projects::CommitsController < Projects::ApplicationController
   before_action :assign_ref_vars
   before_action :authorize_download_code!
   before_action :set_commits
+  before_action :set_request_format, only: :show
 
   def show
     @merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened
@@ -61,6 +62,19 @@ class Projects::CommitsController < Projects::ApplicationController
     @commits = prepare_commits_for_rendering(@commits)
   end
 
+  # Rails 5 sets request.format from the extension.
+  # Explicitly set to :html.
+  def set_request_format
+    request.format = :html if set_request_format?
+  end
+
+  # Rails 5 sets request.format from extension.
+  # In this case if the ref ends with `.atom`, it's expected to be the html response,
+  # not the atom one. So explicitly set request.format as :html to act like rails4.
+  def set_request_format?
+    request.format.to_s == "text/html" || @commits.ref.ends_with?("atom")
+  end
+
   def whitelist_query_limiting
     Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42330')
   end
diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb
index 8e86af43fee..78b9d53a780 100644
--- a/app/controllers/projects/discussions_controller.rb
+++ b/app/controllers/projects/discussions_controller.rb
@@ -21,7 +21,7 @@ class Projects::DiscussionsController < Projects::ApplicationController
 
   def show
     render json: {
-      discussion_html: view_to_html_string('discussions/_diff_with_notes', discussion: discussion, expanded: true)
+      truncated_diff_lines: discussion.try(:truncated_diff_lines)
     }
   end
 
@@ -29,11 +29,6 @@ class Projects::DiscussionsController < Projects::ApplicationController
 
   def render_discussion
     if serialize_notes?
-      # TODO - It is not needed to serialize notes when resolving
-      # or unresolving discussions. We should remove this behavior
-      # passing a parameter to DiscussionEntity to return an empty array
-      # for notes.
-      # Check issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/42853
       prepare_notes_for_rendering(discussion.notes, merge_request)
       render_json_with_discussions_serializer
     else
@@ -44,7 +39,7 @@ class Projects::DiscussionsController < Projects::ApplicationController
   def render_json_with_discussions_serializer
     render json:
       DiscussionSerializer.new(project: project, noteable: discussion.noteable, current_user: current_user, note_entity:  ProjectNoteEntity)
-      .represent(discussion, context: self)
+      .represent(discussion, context: self, render_truncated_diff_lines: true)
   end
 
   # Legacy method used to render discussions notes when not using Vue on views.
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 35c36c725e2..7c897b2d86c 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -4,6 +4,7 @@ class Projects::IssuesController < Projects::ApplicationController
   include IssuableActions
   include ToggleAwardEmoji
   include IssuableCollections
+  include IssuesCalendar
   include SpammableActions
 
   prepend_before_action :authenticate_user!, only: [:new]
@@ -40,14 +41,7 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def calendar
-    @issues = @issuables
-                  .non_archived
-                  .with_due_date
-                  .limit(100)
-
-    respond_to do |format|
-      format.ics { response.headers['Content-Disposition'] = 'inline' }
-    end
+    render_issues_calendar(@issuables)
   end
 
   def new
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb
index fe8525a488c..48e02581d54 100644
--- a/app/controllers/projects/merge_requests/diffs_controller.rb
+++ b/app/controllers/projects/merge_requests/diffs_controller.rb
@@ -9,17 +9,21 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
   before_action :define_diff_comment_vars
 
   def show
-    @environment = @merge_request.environments_for(current_user).last
-
-    render json: { html: view_to_html_string("projects/merge_requests/diffs/_diffs") }
+    render_diffs
   end
 
   def diff_for_path
-    render_diff_for_path(@diffs)
+    render_diffs
   end
 
   private
 
+  def render_diffs
+    @environment = @merge_request.environments_for(current_user).last
+
+    render json: DiffsSerializer.new(current_user: current_user).represent(@diffs, additional_attributes)
+  end
+
   def define_diff_vars
     @merge_request_diffs = @merge_request.merge_request_diffs.viewable.order_id_desc
     @compare = commit || find_merge_request_diff_compare
@@ -63,6 +67,19 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
     end
   end
 
+  def additional_attributes
+    {
+      environment: @environment,
+      merge_request: @merge_request,
+      merge_request_diff: @merge_request_diff,
+      merge_request_diffs: @merge_request_diffs,
+      start_version: @start_version,
+      start_sha: @start_sha,
+      commit: @commit,
+      latest_diff: @merge_request_diff&.latest?
+    }
+  end
+
   def define_diff_comment_vars
     @new_diff_note_attrs = {
       noteable_type: 'MergeRequest',
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index b452bfd7e6f..a7c5f858c42 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -42,6 +42,9 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
         @noteable = @merge_request
         @commits_count = @merge_request.commits_count
 
+        # TODO cleanup- Fatih Simon Create an issue to remove these after the refactoring
+        # we no longer render notes here. I see it will require a small frontend refactoring,
+        # since we gather some data from this collection.
         @discussions = @merge_request.discussions
         @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes), @noteable)
 
@@ -115,7 +118,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
       end
 
       format.json do
-        render json: @merge_request.to_json(include: { milestone: {}, assignee: { only: [:name, :username], methods: [:avatar_url] }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short])
+        render json: serializer.represent(@merge_request, serializer: 'basic')
       end
     end
   rescue ActiveRecord::StaleObjectError