Commit ff55398a authored by Sean McGivern's avatar Sean McGivern

DRY up diff_for_path actions

1. Move render method to a concern, not a helper.
2. Let DiffHelper#diff_options automatically add the path option.
3. Move more instance var definitions to before filters.
parent 6a46926f
module DiffForPath
extend ActiveSupport::Concern
def render_diff_for_path(diffs, diff_refs, project)
diff_file = safe_diff_files(diffs, diff_refs: diff_refs, repository: project.repository).first
return render_404 unless diff_file
diff_commit = commit_for_diff(diff_file)
blob = diff_file.blob(diff_commit)
@expand_all = true
locals = {
diff_file: diff_file,
diff_commit: diff_commit,
diff_refs: diff_refs,
blob: blob,
project: project
}
render json: { html: view_to_html_string('projects/diffs/_content', locals) }
end
end
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
# Not to be confused with CommitsController, plural. # Not to be confused with CommitsController, plural.
class Projects::CommitController < Projects::ApplicationController class Projects::CommitController < Projects::ApplicationController
include CreatesCommit include CreatesCommit
include DiffForPath
include DiffHelper include DiffHelper
# Authorize # Authorize
...@@ -11,29 +12,14 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -11,29 +12,14 @@ class Projects::CommitController < Projects::ApplicationController
before_action :authorize_update_build!, only: [:cancel_builds, :retry_builds] before_action :authorize_update_build!, only: [:cancel_builds, :retry_builds]
before_action :authorize_read_commit_status!, only: [:builds] before_action :authorize_read_commit_status!, only: [:builds]
before_action :commit before_action :commit
before_action :define_show_vars, only: [:show, :builds] before_action :define_commit_vars, only: [:show, :diff_for_path, :builds]
before_action :define_status_vars, only: [:show, :builds]
before_action :define_note_vars, only: [:show, :diff_for_path]
before_action :authorize_edit_tree!, only: [:revert, :cherry_pick] before_action :authorize_edit_tree!, only: [:revert, :cherry_pick]
def show def show
apply_diff_view_cookie! apply_diff_view_cookie!
@grouped_diff_notes = commit.notes.grouped_diff_notes
@notes = commit.notes.non_diff_notes.fresh
Banzai::NoteRenderer.render(
@grouped_diff_notes.values.flatten + @notes,
@project,
current_user,
)
@note = @project.build_commit_note(commit)
@noteable = @commit
@comments_target = {
noteable_type: 'Commit',
commit_id: @commit.id
}
respond_to do |format| respond_to do |format|
format.html format.html
format.diff { render text: @commit.to_diff } format.diff { render text: @commit.to_diff }
...@@ -42,21 +28,7 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -42,21 +28,7 @@ class Projects::CommitController < Projects::ApplicationController
end end
def diff_for_path def diff_for_path
return git_not_found! unless commit render_diff_for_path(@diffs, @commit.diff_refs, @project)
opts = diff_options
opts[:ignore_whitespace_change] = true if params[:format] == 'diff'
diffs = commit.diffs(opts.merge(paths: [params[:path]]))
diff_refs = [commit.parent || commit, commit]
@comments_target = {
noteable_type: 'Commit',
commit_id: @commit.id
}
@grouped_diff_notes = {}
render_diff_for_path(diffs, diff_refs, @project)
end end
def builds def builds
...@@ -132,7 +104,7 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -132,7 +104,7 @@ class Projects::CommitController < Projects::ApplicationController
@ci_builds ||= Ci::Build.where(pipeline: pipelines) @ci_builds ||= Ci::Build.where(pipeline: pipelines)
end end
def define_show_vars def define_commit_vars
return git_not_found! unless commit return git_not_found! unless commit
opts = diff_options opts = diff_options
...@@ -140,7 +112,28 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -140,7 +112,28 @@ class Projects::CommitController < Projects::ApplicationController
@diffs = commit.diffs(opts) @diffs = commit.diffs(opts)
@notes_count = commit.notes.count @notes_count = commit.notes.count
end
def define_note_vars
@grouped_diff_notes = commit.notes.grouped_diff_notes
@notes = commit.notes.non_diff_notes.fresh
Banzai::NoteRenderer.render(
@grouped_diff_notes.values.flatten + @notes,
@project,
current_user,
)
@note = @project.build_commit_note(commit)
@noteable = @commit
@comments_target = {
noteable_type: 'Commit',
commit_id: @commit.id
}
end
def define_status_vars
@statuses = CommitStatus.where(pipeline: pipelines) @statuses = CommitStatus.where(pipeline: pipelines)
@builds = Ci::Build.where(pipeline: pipelines) @builds = Ci::Build.where(pipeline: pipelines)
end end
......
require 'addressable/uri' require 'addressable/uri'
class Projects::CompareController < Projects::ApplicationController class Projects::CompareController < Projects::ApplicationController
include DiffForPath
include DiffHelper include DiffHelper
# Authorize # Authorize
before_action :require_non_empty_project before_action :require_non_empty_project
before_action :authorize_download_code! before_action :authorize_download_code!
before_action :assign_ref_vars, only: [:index, :show, :diff_for_path] before_action :define_ref_vars, only: [:index, :show, :diff_for_path]
before_action :define_diff_vars, only: [:show, :diff_for_path]
before_action :merge_request, only: [:index, :show] before_action :merge_request, only: [:index, :show]
def index def index
end end
def show def show
compare = CompareService.new. end
execute(@project, @head_ref, @project, @start_ref)
if compare
@commits = Commit.decorate(compare.commits, @project)
@start_commit = @project.commit(@start_ref) def diff_for_path
@commit = @project.commit(@head_ref) return render_404 unless @compare
@base_commit = @project.merge_base_commit(@start_ref, @head_ref)
@diffs = compare.diffs(diff_options) render_diff_for_path(@diffs, @diff_refs, @project)
@diff_refs = Gitlab::Diff::DiffRefs.new( end
base_sha: @base_commit.try(:sha),
start_sha: @start_commit.try(:sha),
head_sha: @commit.try(:sha)
)
@diff_notes_disabled = true def create
@grouped_diff_notes = {} redirect_to namespace_project_compare_path(@project.namespace, @project,
params[:from], params[:to])
end end
private
def define_ref_vars
@start_ref = Addressable::URI.unescape(params[:from])
@ref = @head_ref = Addressable::URI.unescape(params[:to])
end end
def diff_for_path def define_diff_vars
compare = CompareService.new. @compare = CompareService.new.execute(@project, @head_ref, @project, @start_ref)
execute(@project, @head_ref, @project, @start_ref)
return render_404 unless compare if @compare
@commits = Commit.decorate(@compare.commits, @project)
@start_commit = @project.commit(@start_ref) @start_commit = @project.commit(@start_ref)
@commit = @project.commit(@head_ref) @commit = @project.commit(@head_ref)
@base_commit = @project.merge_base_commit(@start_ref, @head_ref) @base_commit = @project.merge_base_commit(@start_ref, @head_ref)
diffs = compare.diffs(diff_options.merge(paths: [params[:path]]))
diff_refs = Gitlab::Diff::DiffRefs.new( @diffs = @compare.diffs(diff_options)
@diff_refs = Gitlab::Diff::DiffRefs.new(
base_sha: @base_commit.try(:sha), base_sha: @base_commit.try(:sha),
start_sha: @start_commit.try(:sha), start_sha: @start_commit.try(:sha),
head_sha: @commit.try(:sha) head_sha: @commit.try(:sha)
...@@ -53,20 +54,7 @@ class Projects::CompareController < Projects::ApplicationController ...@@ -53,20 +54,7 @@ class Projects::CompareController < Projects::ApplicationController
@diff_notes_disabled = true @diff_notes_disabled = true
@grouped_diff_notes = {} @grouped_diff_notes = {}
render_diff_for_path(diffs, diff_refs, @project)
end
def create
redirect_to namespace_project_compare_path(@project.namespace, @project,
params[:from], params[:to])
end end
private
def assign_ref_vars
@start_ref = Addressable::URI.unescape(params[:from])
@ref = @head_ref = Addressable::URI.unescape(params[:to])
end end
def merge_request def merge_request
......
class Projects::MergeRequestsController < Projects::ApplicationController class Projects::MergeRequestsController < Projects::ApplicationController
include ToggleSubscriptionAction include ToggleSubscriptionAction
include DiffForPath
include DiffHelper include DiffHelper
include IssuableActions include IssuableActions
include ToggleAwardEmoji include ToggleAwardEmoji
...@@ -102,10 +103,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -102,10 +103,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
define_commit_vars define_commit_vars
diffs = @merge_request.diffs(diff_options.merge(paths: [params[:path]])) diffs = @merge_request.diffs(diff_options)
diff_refs = @merge_request.diff_refs
render_diff_for_path(diffs, diff_refs, @merge_request.project) render_diff_for_path(diffs, @merge_request.diff_refs, @merge_request.project)
end end
def commits def commits
......
...@@ -12,26 +12,6 @@ module DiffHelper ...@@ -12,26 +12,6 @@ module DiffHelper
@expand_all || params[:expand].present? @expand_all || params[:expand].present?
end end
def render_diff_for_path(diffs, diff_refs, project)
diff_file = safe_diff_files(diffs, diff_refs).first
return render_404 unless diff_file
diff_commit = commit_for_diff(diff_file)
blob = project.repository.blob_for_diff(diff_commit, diff_file)
@expand_all = true
locals = {
diff_file: diff_file,
diff_commit: diff_commit,
diff_refs: diff_refs,
blob: blob,
project: project
}
render json: { html: view_to_html_string('projects/diffs/_content', locals) }
end
def diff_view def diff_view
diff_views = %w(inline parallel) diff_views = %w(inline parallel)
...@@ -43,7 +23,9 @@ module DiffHelper ...@@ -43,7 +23,9 @@ module DiffHelper
end end
def diff_options def diff_options
Commit.max_diff_options.merge(ignore_whitespace_change: hide_whitespace?) default_options = Commit.max_diff_options
default_options[:paths] = [params[:path]] if params[:path]
default_options.merge(ignore_whitespace_change: hide_whitespace?)
end end
def safe_diff_files(diffs, diff_refs: nil, repository: nil) def safe_diff_files(diffs, diff_refs: nil, repository: nil)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment