Merge pull request #3109 from hiroponz/improve-network-graph

Improve network graph

app/assets/javascripts/branch-graph.js

@@ -132,17 +132,31 @@
             });
 
           } else if (c.space < this.commits[i].space) {
-            r.path([
+            if (y == psy) {
-                "M", x - 5, y,
+                r.path([
-                "l-5-2,0,4,5,-2",
+                    "M", x - 5, y,
-                "L", x - 10, y,
+                    "l-5,-2,0,4,5,-2",
-                "L", x - 15, psy,
+                    "L", x - 10, y,
-                "L", cx + 5, psy,
+                    "L", x - 15, psy,
-                "L", cx, cy])
+                    "L", cx + 5, psy,
-            .attr({
+                    "L", cx, cy])
-              stroke: this.colors[this.commits[i].space], 
+                .attr({
-              "stroke-width": 2
+                  stroke: this.colors[this.commits[i].space], 
-            });
+                  "stroke-width": 2
+                });
+            } else {
+                r.path([
+                    "M", x - 3, y - 6,
+                    "l-4,-3,4,-2,0,5",
+                    "L", x - 5, y - 10,
+                    "L", x - 10, psy,
+                    "L", cx + 5, psy,
+                    "L", cx, cy])
+                .attr({
+                  stroke: this.colors[this.commits[i].space], 
+                  "stroke-width": 2
+                });
+            }
           } else {
             r.path([
                 "M", x - 3, y + 6,
@@ -306,15 +320,16 @@
   
 }(this);
 Raphael.fn.commitTooltip = function(x, y, commit){
-  var nameText, idText, messageText
+  var icon, nameText, idText, messageText
     , boxWidth = 300
     , boxHeight = 200;
   
-  nameText = this.text(x, y + 10, commit.author.name);
+  icon = this.image(commit.author.icon, x, y, 20, 20);
+  nameText = this.text(x + 25, y + 10, commit.author.name);
   idText = this.text(x, y + 35, commit.id);
   messageText = this.text(x, y + 50, commit.message);
   
-  textSet = this.set(nameText, idText, messageText).attr({
+  textSet = this.set(icon, nameText, idText, messageText).attr({
     "text-anchor": "start",
     "font": "12px Monaco, monospace"
   });

app/controllers/graph_controller.rb

@@ -1,5 +1,6 @@
 class GraphController < ProjectResourceController
   include ExtractsPath
+  include ApplicationHelper
 
   # Authorize
   before_filter :authorize_read_project!
@@ -20,7 +21,10 @@ class GraphController < ProjectResourceController
     respond_to do |format|
       format.html
       format.json do
-        graph = Gitlab::Graph::JsonBuilder.new(project, @ref, @commit)
+        graph = Graph::JsonBuilder.new(project, @ref, @commit)
+        graph.commits.each do |c|
+          c.icon = gravatar_icon(c.author.email)
+        end
         render :json => graph.to_json
       end
     end

app/controllers/projects_controller.rb

@@ -1,5 +1,3 @@
-require Rails.root.join('lib', 'gitlab', 'graph', 'json_builder')
-
 class ProjectsController < ProjectResourceController
   skip_before_filter :project, only: [:new, :create]
   skip_before_filter :repository, only: [:new, :create]

app/models/graph/commit.rb

@@ -0,0 +1,59 @@
+require "grit"
+
+module Graph
+  class Commit
+    include ActionView::Helpers::TagHelper
+
+    attr_accessor :time, :spaces, :refs, :parent_spaces, :icon
+
+    def initialize(commit)
+      @_commit = commit
+      @time = -1
+      @spaces = []
+      @parent_spaces = []
+    end
+
+    def method_missing(m, *args, &block)
+      @_commit.send(m, *args, &block)
+    end
+
+    def to_graph_hash
+      h = {}
+      h[:parents] = self.parents.collect do |p|
+        [p.id,0,0]
+      end
+      h[:author]  = {
+        name: author.name,
+        email: author.email,
+        icon: icon
+      }
+      h[:time]    = time
+      h[:space]   = spaces.first
+      h[:parent_spaces]   = parent_spaces
+      h[:refs]    = refs.collect{|r|r.name}.join(" ") unless refs.nil?
+      h[:id]      = sha
+      h[:date]    = date
+      h[:message] = message
+      h
+    end
+
+    def add_refs(ref_cache, repo)
+      if ref_cache.empty?
+        repo.refs.each do |ref|
+          ref_cache[ref.commit.id] ||= []
+          ref_cache[ref.commit.id] << ref
+        end
+      end
+      @refs = ref_cache[@_commit.id] if ref_cache.include?(@_commit.id)
+      @refs ||= []
+    end
+
+    def space
+      if @spaces.size > 0
+        @spaces.first
+      else
+        0
+      end
+    end
+  end
+end

app/models/graph/json_builder.rb

@@ -0,0 +1,291 @@
+require "grit"
+
+module Graph
+  class JsonBuilder
+    attr_accessor :days, :commits, :ref_cache, :repo
+
+    def self.max_count
+      @max_count ||= 650
+    end
+
+    def initialize project, ref, commit
+      @project = project
+      @ref = ref
+      @commit = commit
+      @repo = project.repo
+      @ref_cache = {}
+
+      @commits = collect_commits
+      @days = index_commits
+    end
+
+    def to_json(*args)
+      {
+        days: @days.compact.map { |d| [d.day, d.strftime("%b")] },
+        commits: @commits.map(&:to_graph_hash)
+      }.to_json(*args)
+    end
+
+  protected
+
+    # Get commits from repository
+    #
+    def collect_commits
+
+      @commits = Grit::Commit.find_all(repo, nil, {date_order: true, max_count: self.class.max_count, skip: to_commit}).dup
+
+      # Decorate with app/models/commit.rb
+      @commits.map! { |commit| Commit.new(commit) }
+
+      # Decorate with lib/gitlab/graph/commit.rb
+      @commits.map! { |commit| Graph::Commit.new(commit) }
+
+      # add refs to each commit
+      @commits.each { |commit| commit.add_refs(ref_cache, repo) }
+
+      @commits
+    end
+
+    # Method is adding time and space on the
+    # list of commits. As well as returns date list
+    # corelated with time set on commits.
+    #
+    # @param [Array<Graph::Commit>] commits to index
+    #
+    # @return [Array<TimeDate>] list of commit dates corelated with time on commits
+    def index_commits
+      days, times = [], []
+      map = {}
+
+      commits.reverse.each_with_index do |c,i|
+        c.time = i
+        days[i] = c.committed_date
+        map[c.id] = c
+        times[i] = c
+      end
+
+      @_reserved = {}
+      days.each_index do |i|
+        @_reserved[i] = []
+      end
+
+      commits_sort_by_ref.each do |commit|
+        if map.include? commit.id then
+          place_chain(map[commit.id], map)
+        end
+      end
+
+      # find parent spaces for not overlap lines
+      times.each do |c|
+        c.parent_spaces.concat(find_free_parent_spaces(c, map, times))
+      end
+
+      days
+    end
+
+    # Skip count that the target commit is displayed in center.
+    def to_commit
+      commits = Grit::Commit.find_all(repo, nil, {date_order: true})
+      commit_index = commits.index do |c|
+        c.id == @commit.id
+      end
+
+      if commit_index && (self.class.max_count / 2 < commit_index) then
+        # get max index that commit is displayed in the center.
+        commit_index - self.class.max_count / 2
+      else
+        0
+      end
+    end
+
+    def commits_sort_by_ref
+      commits.sort do |a,b|
+        if include_ref?(a)
+          -1
+        elsif include_ref?(b)
+          1
+        else
+          b.committed_date <=> a.committed_date
+        end
+      end
+    end
+
+    def include_ref?(commit)
+      heads = commit.refs.select do |ref|
+        ref.is_a?(Grit::Head) or ref.is_a?(Grit::Remote) or ref.is_a?(Grit::Tag)
+      end
+
+      heads.map! do |head|
+        head.name
+      end
+
+      heads.include?(@ref)
+    end
+
+    def find_free_parent_spaces(commit, map, times)
+      spaces = []
+
+      commit.parents.each do |p|
+        if map.include?(p.id) then
+          parent = map[p.id]
+
+          range = if commit.time < parent.time then
+                    commit.time..parent.time
+                  else
+                    parent.time..commit.time
+                  end
+
+          space = if commit.space >= parent.space then
+                    find_free_parent_space(range, parent.space, -1, commit.space, times)
+                  else
+                    find_free_parent_space(range, commit.space, -1, parent.space, times)
+                  end
+
+          mark_reserved(range, space)
+          spaces << space
+        end
+      end
+
+      spaces
+    end
+
+    def find_free_parent_space(range, space_base, space_step, space_default, times)
+      if is_overlap?(range, times, space_default) then
+        find_free_space(range, space_step, space_base, space_default)
+      else
+        space_default
+      end
+    end
+
+    def is_overlap?(range, times, overlap_space)
+      range.each do |i|
+        if i != range.first &&
+          i != range.last &&
+          times[i].spaces.include?(overlap_space) then
+
+          return true;
+        end
+      end
+
+      false
+    end
+
+    # Add space mark on commit and its parents
+    #
+    # @param [Graph::Commit] the commit object.
+    # @param [Hash<String,Graph::Commit>] map of commits
+    def place_chain(commit, map, parent_time = nil)
+      leaves = take_left_leaves(commit, map)
+      if leaves.empty?
+        return
+      end
+
+      time_range = leaves.last.time..leaves.first.time
+      space_base = get_space_base(leaves, map)
+      space = find_free_space(time_range, 2, space_base)
+      leaves.each do |l|
+        l.spaces << space
+        # Also add space to parent
+        l.parents.each do |p|
+          if map.include?(p.id)
+            parent = map[p.id]
+            if parent.space > 0
+              parent.spaces << space
+            end
+          end
+        end
+      end
+
+      # and mark it as reserved
+      min_time = leaves.last.time
+      parents = leaves.last.parents.collect
+      parents.each do |p|
+        if map.include? p.id
+          parent = map[p.id]
+          if parent.time < min_time
+            min_time = parent.time
+          end
+        end
+      end
+
+      if parent_time.nil?
+        max_time = leaves.first.time
+      else
+        max_time = parent_time - 1
+      end
+      mark_reserved(min_time..max_time, space)
+
+      # Visit branching chains
+      leaves.each do |l|
+        parents = l.parents.collect.select{|p| map.include? p.id and map[p.id].space.zero?}
+        for p in parents
+          place_chain(map[p.id], map, l.time)
+        end
+      end
+    end
+
+    def get_space_base(leaves, map)
+      space_base = 1
+      if leaves.last.parents.size > 0
+        first_parent = leaves.last.parents.first
+        if map.include?(first_parent.id)
+          first_p = map[first_parent.id]
+          if first_p.space > 0
+            space_base = first_p.space
+          end
+        end
+      end
+      space_base
+    end
+
+    def mark_reserved(time_range, space)
+      for day in time_range
+        @_reserved[day].push(space)
+      end
+    end
+
+    def find_free_space(time_range, space_step, space_base = 1, space_default = nil)
+      space_default ||= space_base
+
+      reserved = []
+      for day in time_range
+        reserved += @_reserved[day]
+      end
+      reserved.uniq!
+
+      space = space_default
+      while reserved.include?(space) do
+        space += space_step
+        if space < space_base then
+          space_step *= -1
+          space = space_base + space_step
+        end
+      end
+
+      space
+    end
+
+    # Takes most left subtree branch of commits
+    # which don't have space mark yet.
+    #
+    # @param [Graph::Commit] the commit object.
+    # @param [Hash<String,Graph::Commit>] map of commits
+    #
+    # @return [Array<Graph::Commit>] list of branch commits
+    def take_left_leaves(commit, map)
+      leaves = []
+      leaves.push(commit) if commit.space.zero?
+
+      while true
+        return leaves if commit.parents.count.zero?
+        return leaves unless map.include? commit.parents.first.id
+
+        commit = map[commit.parents.first.id]
+
+        return leaves unless commit.space.zero?
+
+        leaves.push(commit)
+      end
+    end
+  end
+end

features/project/network.feature

@@ -7,3 +7,19 @@ Feature: Project Network Graph
   @javascript
   Scenario: I should see project network
     Then page should have network graph
+    And page should select "master" in select box
+    And page should have "master" on graph
+
+  @javascript
+  Scenario: I should switch ref to "stable"
+    When I switch ref to "stable"
+    Then page should have network graph
+    And page should select "stable" in select box
+    And page should have "stable" on graph
+
+  @javascript
+  Scenario: I should looking for a commit by SHA of "v2.1.0"
+    When I looking for a commit by SHA of "v2.1.0"
+    Then page should have network graph
+    And page should select "master" in select box
+    And page should have "v2.1.0" on graph

features/steps/project/project_network_graph.rb

@@ -4,16 +4,51 @@ class ProjectNetworkGraph < Spinach::FeatureSteps
 
   Then 'page should have network graph' do
     page.should have_content "Project Network Graph"
-    within ".graph" do
+    page.should have_selector ".graph"
-      page.should have_content "master"
-    end
   end
 
-  And 'I visit project "Shop" network page' do
+  When 'I visit project "Shop" network page' do
     # Stub Graph::JsonBuilder max_size to speed up test (10 commits vs. 650)
-    Gitlab::Graph::JsonBuilder.stub(max_count: 10)
+    Graph::JsonBuilder.stub(max_count: 10)
 
     project = Project.find_by_name("Shop")
     visit project_graph_path(project, "master")
   end
+
+  And 'page should select "master" in select box' do
+    page.should have_selector '#ref_chzn span', :text => "master"
+  end
+
+  And 'page should have "master" on graph' do
+    within '.graph' do
+      page.should have_content 'master'
+    end
+  end
+
+  And 'I switch ref to "stable"' do
+    page.select 'stable', :from => 'ref'
+  end
+
+  And 'page should select "stable" in select box' do
+    page.should have_selector '#ref_chzn span', :text => "stable"
+  end
+
+  And 'page should have "stable" on graph' do
+    within '.graph' do
+      page.should have_content 'stable'
+    end
+  end
+
+  And 'I looking for a commit by SHA of "v2.1.0"' do
+    within ".content .search" do
+      fill_in 'q', :with => '98d6492'
+      find('button').click
+    end
+  end
+
+  And 'page should have "v2.1.0" on graph' do
+    within '.graph' do
+      page.should have_content 'v2.1.0'
+    end
+  end
 end

features/steps/shared/paths.rb

@@ -143,7 +143,7 @@ module SharedPaths
 
   Given "I visit my project's network page" do
     # Stub Graph::JsonBuilder max_size to speed up test (10 commits vs. 650)
-    Gitlab::Graph::JsonBuilder.stub(max_count: 10)
+    Graph::JsonBuilder.stub(max_count: 10)
 
     visit project_graph_path(@project, root_ref)
   end

lib/gitlab/graph/commit.rb

@@ -1,52 +0,0 @@
-require "grit"
-
-module Gitlab
-  module Graph
-    class Commit
-      include ActionView::Helpers::TagHelper
-
-      attr_accessor :time, :space, :refs, :parent_spaces
-
-      def initialize(commit)
-        @_commit = commit
-        @time = -1
-        @space = 0
-        @parent_spaces = []
-      end
-
-      def method_missing(m, *args, &block)
-        @_commit.send(m, *args, &block)
-      end
-
-      def to_graph_hash
-        h = {}
-        h[:parents] = self.parents.collect do |p|
-          [p.id,0,0]
-        end
-        h[:author]  = {
-          name: author.name, 
-          email: author.email
-        }
-        h[:time]    = time
-        h[:space]   = space
-        h[:parent_spaces]   = parent_spaces
-        h[:refs]    = refs.collect{|r|r.name}.join(" ") unless refs.nil?
-        h[:id]      = sha
-        h[:date]    = date
-        h[:message] = message
-        h
-      end
-
-      def add_refs(ref_cache, repo)
-        if ref_cache.empty?
-          repo.refs.each do |ref|
-            ref_cache[ref.commit.id] ||= []
-            ref_cache[ref.commit.id] << ref
-          end
-        end
-        @refs = ref_cache[@_commit.id] if ref_cache.include?(@_commit.id)
-        @refs ||= []
-      end
-    end
-  end
-end

lib/gitlab/graph/json_builder.rb

@@ -1,268 +0,0 @@
-require "grit"
-
-module Gitlab
-  module Graph
-    class JsonBuilder
-      attr_accessor :days, :commits, :ref_cache, :repo
-
-      def self.max_count
-        @max_count ||= 650
-      end
-
-      def initialize project, ref, commit
-        @project = project
-        @ref = ref
-        @commit = commit
-        @repo = project.repo
-        @ref_cache = {}
-
-        @commits = collect_commits
-        @days = index_commits
-      end
-
-      def to_json(*args)
-        {
-          days: @days.compact.map { |d| [d.day, d.strftime("%b")] },
-          commits: @commits.map(&:to_graph_hash)
-        }.to_json(*args)
-      end
-
-    protected
-
-      # Get commits from repository
-      #
-      def collect_commits
-
-        @commits = Grit::Commit.find_all(repo, nil, {topo_order: true, max_count: self.class.max_count, skip: to_commit}).dup
-
-        # Decorate with app/models/commit.rb
-        @commits.map! { |commit| ::Commit.new(commit) }
-
-        # Decorate with lib/gitlab/graph/commit.rb
-        @commits.map! { |commit| Gitlab::Graph::Commit.new(commit) }
-
-        # add refs to each commit
-        @commits.each { |commit| commit.add_refs(ref_cache, repo) }
-
-        @commits
-      end
-
-      # Method is adding time and space on the
-      # list of commits. As well as returns date list
-      # corelated with time set on commits.
-      #
-      # @param [Array<Graph::Commit>] commits to index
-      #
-      # @return [Array<TimeDate>] list of commit dates corelated with time on commits
-      def index_commits
-        days, times = [], []
-        map = {}
-
-        commits.reverse.each_with_index do |c,i|
-          c.time = i
-          days[i] = c.committed_date
-          map[c.id] = c
-          times[i] = c
-        end
-
-        @_reserved = {}
-        days.each_index do |i|
-          @_reserved[i] = []
-        end
-
-        commits_sort_by_ref.each do |commit|
-          if map.include? commit.id then
-            place_chain(map[commit.id], map)
-          end
-        end
-
-        # find parent spaces for not overlap lines
-        times.each do |c|
-          c.parent_spaces.concat(find_free_parent_spaces(c, map, times))
-        end
-
-        days
-      end
-
-      # Skip count that the target commit is displayed in center.
-      def to_commit
-        commits = Grit::Commit.find_all(repo, nil, {topo_order: true})
-        commit_index = commits.index do |c|
-          c.id == @commit.id
-        end
-
-        if commit_index && (self.class.max_count / 2 < commit_index) then
-          # get max index that commit is displayed in the center.
-          commit_index - self.class.max_count / 2
-        else
-          0
-        end
-      end
-
-      def commits_sort_by_ref
-        commits.sort do |a,b|
-          if include_ref?(a)
-            -1
-          elsif include_ref?(b)
-            1
-          else
-            b.committed_date <=> a.committed_date
-          end
-        end
-      end
-
-      def include_ref?(commit)
-        heads = commit.refs.select do |ref|
-          ref.is_a?(Grit::Head) or ref.is_a?(Grit::Remote) or ref.is_a?(Grit::Tag)
-        end
-
-        heads.map! do |head|
-          head.name
-        end
-
-        heads.include?(@ref)
-      end
-
-      def find_free_parent_spaces(commit, map, times)
-        spaces = []
-
-        commit.parents.each do |p|
-          if map.include?(p.id) then
-            parent = map[p.id]
-
-            range = if commit.time < parent.time then
-                      commit.time..parent.time
-                    else
-                      parent.time..commit.time
-                    end
-
-            space = if commit.space >= parent.space then
-                      find_free_parent_space(range, parent.space, 1, commit.space, times)
-                    else
-                      find_free_parent_space(range, parent.space, -1, parent.space, times)
-                    end
-
-            mark_reserved(range, space)
-            spaces << space
-          end
-        end
-
-        spaces
-      end
-
-      def find_free_parent_space(range, space_base, space_step, space_default, times)
-        if is_overlap?(range, times, space_default) then
-          find_free_space(range, space_base, space_step)
-        else
-          space_default
-        end
-      end
-
-      def is_overlap?(range, times, overlap_space)
-        range.each do |i|
-          if i != range.first &&
-            i != range.last &&
-            times[i].space == overlap_space then
-
-            return true;
-          end
-        end
-
-        false
-      end
-
-      # Add space mark on commit and its parents
-      #
-      # @param [Graph::Commit] the commit object.
-      # @param [Hash<String,Graph::Commit>] map of commits
-      def place_chain(commit, map, parent_time = nil)
-        leaves = take_left_leaves(commit, map)
-        if leaves.empty?
-          return
-        end
-        # and mark it as reserved
-        min_time = leaves.last.time
-        max_space = 1
-        parents = leaves.last.parents.collect
-        parents.each do |p|
-          if map.include? p.id
-            parent = map[p.id]
-            if parent.time < min_time
-              min_time = parent.time
-            end
-            if max_space < parent.space then
-              max_space = parent.space
-            end
-          end
-        end
-        if parent_time.nil?
-          max_time = leaves.first.time
-        else
-          max_time = parent_time - 1
-        end
-
-        time_range = leaves.last.time..leaves.first.time
-        space = find_free_space(time_range, max_space, 2)
-        leaves.each{|l| l.space = space}
-
-        mark_reserved(min_time..max_time, space)
-
-        # Visit branching chains
-        leaves.each do |l|
-          parents = l.parents.collect.select{|p| map.include? p.id and map[p.id].space.zero?}
-          for p in parents
-            place_chain(map[p.id], map, l.time)
-          end
-        end
-      end
-
-      def mark_reserved(time_range, space)
-        for day in time_range
-          @_reserved[day].push(space)
-        end
-      end
-
-      def find_free_space(time_range, space_base, space_step)
-        reserved = []
-        for day in time_range
-          reserved += @_reserved[day]
-        end
-        reserved.uniq!
-
-        space = space_base
-        while reserved.include?(space) do
-          space += space_step
-          if space <= 0 then
-            space_step *= -1
-            space = space_base + space_step
-          end
-        end
-
-        space
-      end
-
-      # Takes most left subtree branch of commits
-      # which don't have space mark yet.
-      #
-      # @param [Graph::Commit] the commit object.
-      # @param [Hash<String,Graph::Commit>] map of commits
-      #
-      # @return [Array<Graph::Commit>] list of branch commits
-      def take_left_leaves(commit, map)
-        leaves = []
-        leaves.push(commit) if commit.space.zero?
-
-        while true
-          return leaves if commit.parents.count.zero?
-          return leaves unless map.include? commit.parents.first.id
-
-          commit = map[commit.parents.first.id]
-
-          return leaves unless commit.space.zero?
-
-          leaves.push(commit)
-        end
-      end
-    end
-  end
-end