Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/controllers/commit_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ def show
end
end

format.patch
format.diff { render text: @commit.to_diff }
format.patch { render text: @commit.to_patch }
end
end
end
12 changes: 5 additions & 7 deletions app/controllers/merge_requests_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class MergeRequestsController < ProjectResourceController
before_filter :module_enabled
before_filter :merge_request, only: [:edit, :update, :destroy, :show, :commits, :diffs, :automerge, :automerge_check, :raw]
before_filter :validates_merge_request, only: [:show, :diffs, :raw]
before_filter :merge_request, only: [:edit, :update, :destroy, :show, :commits, :diffs, :automerge, :automerge_check]
before_filter :validates_merge_request, only: [:show, :diffs]
before_filter :define_show_vars, only: [:show, :diffs]

# Allow read any merge_request
Expand All @@ -16,7 +16,6 @@ class MergeRequestsController < ProjectResourceController
# Allow destroy merge_request
before_filter :authorize_admin_merge_request!, only: [:destroy]


def index
@merge_requests = MergeRequestsLoadContext.new(project, current_user, params).execute
end
Expand All @@ -25,11 +24,10 @@ def show
respond_to do |format|
format.html
format.js
end
end

def raw
send_file @merge_request.to_raw
format.diff { render text: @merge_request.to_diff }
format.patch { render text: @merge_request.to_patch }
end
end

def diffs
Expand Down
15 changes: 15 additions & 0 deletions app/models/commit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,19 @@ def prev_commit_id
def parents_count
parents && parents.count || 0
end

# Shows the diff between the commit's parent and the commit.
#
# Cuts out the header and stats from #to_patch and returns only the diff.
def to_diff
# see Grit::Commit#show
patch = to_patch

# discard lines before the diff
lines = patch.split("\n")
while !lines.first.start_with?("diff --git") do
lines.shift
end
lines.join("\n")
end
end
26 changes: 14 additions & 12 deletions app/models/merge_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,20 +202,22 @@ def automerge!(current_user)
false
end

def to_raw
FileUtils.mkdir_p(Rails.root.join("tmp", "patches"))
patch_path = Rails.root.join("tmp", "patches", "merge_request_#{self.id}.patch")

from = commits.last.id
to = source_branch

project.repo.git.run('', "format-patch" , " > #{patch_path.to_s}", {}, ["#{from}..#{to}", "--stdout"])

patch_path
end

def mr_and_commit_notes
commit_ids = commits.map(&:id)
Note.where("(noteable_type = 'MergeRequest' AND noteable_id = :mr_id) OR (noteable_type = 'Commit' AND noteable_id IN (:commit_ids))", mr_id: id, commit_ids: commit_ids)
end

# Returns the raw diff for this merge request
#
# see "git diff"
def to_diff
project.repo.git.native(:diff, {timeout: 30, raise: true}, "#{target_branch}...#{source_branch}")
end

# Returns the commit as a series of email patches.
#
# see "git format-patch"
def to_patch
project.repo.git.format_patch({timeout: 30, raise: true, stdout: true}, "#{target_branch}..#{source_branch}")
end
end
1 change: 0 additions & 1 deletion app/views/commit/show.patch.erb

This file was deleted.

11 changes: 8 additions & 3 deletions app/views/commits/_commit_box.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,14 @@
%span.btn.disabled.grouped
%i.icon-comment
= @notes_count
= link_to project_commit_path(@project, @commit, format: :patch), class: "btn small grouped" do
%i.icon-download-alt
Get Patch
.left.btn-group
%a.btn.small.grouped.dropdown-toggle{ data: {toggle: :dropdown} }
%i.icon-download-alt
Download as
%span.caret
%ul.dropdown-menu
%li= link_to "Email Patches", project_commit_path(@project, @commit, format: :patch)
%li= link_to "Plain Diff", project_commit_path(@project, @commit, format: :diff)
= link_to project_tree_path(@project, @commit), class: "browse-button primary grouped" do
%strong Browse Code »
%h3.commit-title.page_title
Expand Down
6 changes: 4 additions & 2 deletions app/views/merge_requests/show/_diffs.html.haml
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
- if @merge_request.valid_diffs?
= render "commits/diffs", diffs: @diffs
- elsif @merge_request.broken_diffs?
%h4.nothing_here_message
%h4.nothing_here_message
Can't load diff.
You can #{link_to "download MR patch", raw_project_merge_request_path(@project, @merge_request), class: "vlink"} instead.
You can
= link_to "download it", project_merge_request_path(@project, @merge_request), format: :diff, class: "vlink"
instead.
- else
%h4.nothing_here_message Nothing to merge
11 changes: 8 additions & 3 deletions app/views/merge_requests/show/_mr_title.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@
= "MERGED"
- if can?(current_user, :modify_merge_request, @merge_request)
- if @merge_request.open?
= link_to raw_project_merge_request_path(@project, @merge_request), class: "btn grouped" do
%i.icon-download-alt
Get Patch
.left.btn-group
%a.btn.grouped.dropdown-toggle{ data: {toggle: :dropdown} }
%i.icon-download-alt
Download as
%span.caret
%ul.dropdown-menu
%li= link_to "Email Patches", project_merge_request_path(@project, @merge_request, format: :patch)
%li= link_to "Plain Diff", project_merge_request_path(@project, @merge_request, format: :diff)

= link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {closed: true }, status_only: true), method: :put, class: "btn grouped danger", title: "Close merge request"

Expand Down
3 changes: 2 additions & 1 deletion config/initializers/mime_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@
# Mime::Type.register "text/richtext", :rtf
# Mime::Type.register_alias "text/html", :iphone

Mime::Type.register_alias 'text/plain', :patch
Mime::Type.register_alias "text/plain", :diff
Mime::Type.register_alias "text/plain", :patch
3 changes: 1 addition & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,11 @@
end
end

resources :merge_requests do
resources :merge_requests, constraints: {id: /\d+/} do
member do
get :diffs
get :automerge
get :automerge_check
get :raw
end

collection do
Expand Down
74 changes: 74 additions & 0 deletions spec/controllers/commit_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
require 'spec_helper'

describe CommitController do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:commit) { project.last_commit_for("master") }

before do
sign_in(user)

project.add_access(user, :read, :admin)
end

describe "#show" do
shared_examples "export as" do |format|
it "should generally work" do
get :show, project_id: project.code, id: commit.id, format: format

expect(response).to be_success
end

it "should generate it" do
Commit.any_instance.should_receive(:"to_#{format}")

get :show, project_id: project.code, id: commit.id, format: format
end

it "should render it" do
get :show, project_id: project.code, id: commit.id, format: format

expect(response.body).to eq(commit.send(:"to_#{format}"))
end

it "should not escape Html" do
Commit.any_instance.stub(:"to_#{format}").and_return('HTML entities &<>" ')

get :show, project_id: project.code, id: commit.id, format: format

expect(response.body).to_not include('&amp;')
expect(response.body).to_not include('&gt;')
expect(response.body).to_not include('&lt;')
expect(response.body).to_not include('&quot;')
end
end

describe "as diff" do
include_examples "export as", :diff
let(:format) { :diff }

it "should really only be a git diff" do
get :show, project_id: project.code, id: commit.id, format: format

expect(response.body).to start_with("diff --git")
end
end

describe "as patch" do
include_examples "export as", :patch
let(:format) { :patch }

it "should really be a git email patch" do
get :show, project_id: project.code, id: commit.id, format: format

expect(response.body).to start_with("From #{commit.id}")
end

it "should contain a git diff" do
get :show, project_id: project.code, id: commit.id, format: format

expect(response.body).to match(/^diff --git/)
end
end
end
end
84 changes: 84 additions & 0 deletions spec/controllers/merge_requests_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
require 'spec_helper'

describe MergeRequestsController do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:merge_request) { create(:merge_request_with_diffs, project: project) }

before do
sign_in(user)

project.add_access(user, :read, :admin)
end

describe "#show" do
shared_examples "export as" do |format|
it "should generally work" do
get :show, project_id: project.code, id: merge_request.id, format: format

expect(response).to be_success
end

it "should generate it" do
MergeRequest.any_instance.should_receive(:"to_#{format}")

get :show, project_id: project.code, id: merge_request.id, format: format
end

it "should render it" do
get :show, project_id: project.code, id: merge_request.id, format: format

expect(response.body).to eq(merge_request.send(:"to_#{format}"))
end

it "should not escape Html" do
MergeRequest.any_instance.stub(:"to_#{format}").and_return('HTML entities &<>" ')

get :show, project_id: project.code, id: merge_request.id, format: format

expect(response.body).to_not include('&amp;')
expect(response.body).to_not include('&gt;')
expect(response.body).to_not include('&lt;')
expect(response.body).to_not include('&quot;')
end
end

describe "as diff" do
include_examples "export as", :diff
let(:format) { :diff }

it "should really only be a git diff" do
get :show, project_id: project.code, id: merge_request.id, format: format

expect(response.body).to start_with("diff --git")
end
end

describe "as patch" do
include_examples "export as", :patch
let(:format) { :patch }

it "should really be a git email patch with commit" do
get :show, project_id: project.code, id: merge_request.id, format: format

expect(response.body[0..100]).to start_with("From #{merge_request.commits.last.id}")
end

it "should contain as many patches as there are commits" do
get :show, project_id: project.code, id: merge_request.id, format: format

patch_count = merge_request.commits.count
merge_request.commits.each_with_index do |commit, patch_num|
expect(response.body).to match(/^From #{commit.id}/)
expect(response.body).to match(/^Subject: \[PATCH #{patch_num}\/#{patch_count}\]/)
end
end

it "should contain git diffs" do
get :show, project_id: project.code, id: merge_request.id, format: format

expect(response.body).to match(/^diff --git/)
end
end
end
end
15 changes: 15 additions & 0 deletions spec/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,22 @@
closed true
end

# pick 3 commits "at random" (from bcf03b5d~3 to bcf03b5d)
trait :with_diffs do
target_branch "bcf03b5d~3"
source_branch "bcf03b5d"
st_commits do
[Commit.new(project.repo.commit('bcf03b5d')),
Commit.new(project.repo.commit('bcf03b5d~1')),
Commit.new(project.repo.commit('bcf03b5d~2'))]
end
st_diffs do
project.repo.diff("bcf03b5d~3", "bcf03b5d")
end
end

factory :closed_merge_request, traits: [:closed]
factory :merge_request_with_diffs, traits: [:with_diffs]
end

factory :note do
Expand Down
11 changes: 6 additions & 5 deletions spec/routing/project_routing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@
# diffs_project_merge_request GET /:project_id/merge_requests/:id/diffs(.:format) merge_requests#diffs
# automerge_project_merge_request GET /:project_id/merge_requests/:id/automerge(.:format) merge_requests#automerge
# automerge_check_project_merge_request GET /:project_id/merge_requests/:id/automerge_check(.:format) merge_requests#automerge_check
# raw_project_merge_request GET /:project_id/merge_requests/:id/raw(.:format) merge_requests#raw
# branch_from_project_merge_requests GET /:project_id/merge_requests/branch_from(.:format) merge_requests#branch_from
# branch_to_project_merge_requests GET /:project_id/merge_requests/branch_to(.:format) merge_requests#branch_to
# project_merge_requests GET /:project_id/merge_requests(.:format) merge_requests#index
Expand All @@ -231,10 +230,6 @@
get("/gitlabhq/merge_requests/1/automerge_check").should route_to('merge_requests#automerge_check', project_id: 'gitlabhq', id: '1')
end

it "to #raw" do
get("/gitlabhq/merge_requests/1/raw").should route_to('merge_requests#raw', project_id: 'gitlabhq', id: '1')
end

it "to #branch_from" do
get("/gitlabhq/merge_requests/branch_from").should route_to('merge_requests#branch_from', project_id: 'gitlabhq')
end
Expand All @@ -243,6 +238,11 @@
get("/gitlabhq/merge_requests/branch_to").should route_to('merge_requests#branch_to', project_id: 'gitlabhq')
end

it "to #show" do
get("/gitlabhq/merge_requests/1.diff").should route_to('merge_requests#show', project_id: 'gitlabhq', id: '1', format: 'diff')
get("/gitlabhq/merge_requests/1.patch").should route_to('merge_requests#show', project_id: 'gitlabhq', id: '1', format: 'patch')
end

it_behaves_like "RESTful project resources" do
let(:controller) { 'merge_requests' }
end
Expand Down Expand Up @@ -285,6 +285,7 @@
describe CommitController, "routing" do
it "to #show" do
get("/gitlabhq/commit/4246fb").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fb')
get("/gitlabhq/commit/4246fb.diff").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fb', format: 'diff')
get("/gitlabhq/commit/4246fb.patch").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fb', format: 'patch')
get("/gitlabhq/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5").should route_to('commit#show', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5')
end
Expand Down