Fix ordering of commits in the network graph.
- We upgraded `rugged` to 0.25.1.1 in !10286 for %9.1 - Prior to this upgrade, the default sort order for commits returned by `Gitlab::Git::Repository#find_commits` was `Rugged::SORT_DATE`, which the graph relied on. - While upgrading `rugged`, the MR also changed this default to `Rugged::SORT_NONE`, which broke commit ordering in the graph. - This commit adds an option to `Gitlab::Git::Repository#find_commits` to sort by date, and changes the graph builder `Network::Graph` so it explictly requests the `:date` sort order
This commit is contained in:
parent
3c6fad6429
commit
a7e67604b3
|
@ -107,7 +107,8 @@ module Network
|
||||||
def find_commits(skip = 0)
|
def find_commits(skip = 0)
|
||||||
opts = {
|
opts = {
|
||||||
max_count: self.class.max_count,
|
max_count: self.class.max_count,
|
||||||
skip: skip
|
skip: skip,
|
||||||
|
order: :date
|
||||||
}
|
}
|
||||||
|
|
||||||
opts[:ref] = @commit.id if @filter_ref
|
opts[:ref] = @commit.id if @filter_ref
|
||||||
|
|
|
@ -494,7 +494,9 @@ module Gitlab
|
||||||
# :contains is the commit contained by the refs from which to begin (SHA1 or name)
|
# :contains is the commit contained by the refs from which to begin (SHA1 or name)
|
||||||
# :max_count is the maximum number of commits to fetch
|
# :max_count is the maximum number of commits to fetch
|
||||||
# :skip is the number of commits to skip
|
# :skip is the number of commits to skip
|
||||||
# :order is the commits order and allowed value is :date(default) or :topo
|
# :order is the commits order and allowed value is :none (default), :date, or :topo
|
||||||
|
# commit ordering types are documented here:
|
||||||
|
# http://www.rubydoc.info/github/libgit2/rugged/Rugged#SORT_NONE-constant)
|
||||||
#
|
#
|
||||||
def find_commits(options = {})
|
def find_commits(options = {})
|
||||||
actual_options = options.dup
|
actual_options = options.dup
|
||||||
|
@ -522,11 +524,8 @@ module Gitlab
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
if actual_options[:order] == :topo
|
sort_type = rugged_sort_type(actual_options[:order])
|
||||||
walker.sorting(Rugged::SORT_TOPO)
|
walker.sorting(sort_type)
|
||||||
else
|
|
||||||
walker.sorting(Rugged::SORT_NONE)
|
|
||||||
end
|
|
||||||
|
|
||||||
commits = []
|
commits = []
|
||||||
offset = actual_options[:skip]
|
offset = actual_options[:skip]
|
||||||
|
@ -1273,6 +1272,18 @@ module Gitlab
|
||||||
def gitaly_ref_client
|
def gitaly_ref_client
|
||||||
@gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(self)
|
@gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(self)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Returns the `Rugged` sorting type constant for a given
|
||||||
|
# sort type key. Valid keys are `:none`, `:topo`, and `:date`
|
||||||
|
def rugged_sort_type(key)
|
||||||
|
@rugged_sort_types ||= {
|
||||||
|
none: Rugged::SORT_NONE,
|
||||||
|
topo: Rugged::SORT_TOPO,
|
||||||
|
date: Rugged::SORT_DATE
|
||||||
|
}
|
||||||
|
|
||||||
|
@rugged_sort_types.fetch(key, Rugged::SORT_NONE)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1031,6 +1031,35 @@ describe Gitlab::Git::Repository, seed_helper: true do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#find_commits' do
|
||||||
|
it 'should return a return a collection of commits' do
|
||||||
|
commits = repository.find_commits
|
||||||
|
|
||||||
|
expect(commits).not_to be_empty
|
||||||
|
expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) )
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'while applying a sort order based on the `order` option' do
|
||||||
|
it "allows ordering topologically (no parents shown before their children)" do
|
||||||
|
expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_TOPO)
|
||||||
|
|
||||||
|
repository.find_commits(order: :topo)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "allows ordering by date" do
|
||||||
|
expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_DATE)
|
||||||
|
|
||||||
|
repository.find_commits(order: :date)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "applies no sorting by default" do
|
||||||
|
expect_any_instance_of(Rugged::Walker).to receive(:sorting).with(Rugged::SORT_NONE)
|
||||||
|
|
||||||
|
repository.find_commits
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '#branches with deleted branch' do
|
describe '#branches with deleted branch' do
|
||||||
before(:each) do
|
before(:each) do
|
||||||
ref = double()
|
ref = double()
|
||||||
|
|
|
@ -9,4 +9,25 @@ describe Network::Graph, models: true do
|
||||||
|
|
||||||
expect(graph.notes).to eq( { note_on_commit.commit_id => 1 } )
|
expect(graph.notes).to eq( { note_on_commit.commit_id => 1 } )
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "#commits" do
|
||||||
|
let(:graph) { described_class.new(project, 'refs/heads/master', project.repository.commit, nil) }
|
||||||
|
|
||||||
|
it "returns a list of commits" do
|
||||||
|
commits = graph.commits
|
||||||
|
|
||||||
|
expect(commits).not_to be_empty
|
||||||
|
expect(commits).to all( be_kind_of(Network::Commit) )
|
||||||
|
end
|
||||||
|
|
||||||
|
it "sorts the commits by commit date (descending)" do
|
||||||
|
# Remove duplicate timestamps because they make it harder to
|
||||||
|
# assert that the commits are sorted as expected.
|
||||||
|
commits = graph.commits.uniq(&:date)
|
||||||
|
sorted_commits = commits.sort_by(&:date).reverse
|
||||||
|
|
||||||
|
expect(commits).not_to be_empty
|
||||||
|
expect(commits.map(&:id)).to eq(sorted_commits.map(&:id))
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue