From 63b58b9491badf4fe5fa79a32c6ad77be2bf3c25 Mon Sep 17 00:00:00 2001 From: Sato Hiroyuki Date: Wed, 24 Apr 2013 15:06:31 +0000 Subject: [PATCH 1/7] Reducing database access. --- app/helpers/graph_helper.rb | 3 +-- app/models/network/graph.rb | 11 ++++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/helpers/graph_helper.rb b/app/helpers/graph_helper.rb index ca7d823a45..71a07d6cad 100644 --- a/app/helpers/graph_helper.rb +++ b/app/helpers/graph_helper.rb @@ -4,8 +4,7 @@ module GraphHelper refs += commit.refs.collect{|r|r.name}.join(" ") if commit.refs # append note count - notes = @project.notes.for_commit_id(commit.id) - refs += "[#{notes.count}]" if notes.any? + refs += "[#{@graph.notes[commit.id]}]" if @graph.notes[commit.id] > 0 refs end diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index 0fe7765b9e..ea7d188cc3 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -2,7 +2,7 @@ require "grit" module Network class Graph - attr_reader :days, :commits, :map + attr_reader :days, :commits, :map, :notes def self.max_count @max_count ||= 650 @@ -16,10 +16,19 @@ module Network @commits = collect_commits @days = index_commits + @notes = collect_notes end protected + def collect_notes + h = Hash.new(0) + @project.notes.where('noteable_type = ?' ,"Commit").group('notes.commit_id').select('notes.commit_id, count(notes.id) as note_count').each do |item| + h[item["commit_id"]] = item["note_count"] + end + h + end + # Get commits from repository # def collect_commits From d7edceec739cbfd87044e63033430f487c8799c4 Mon Sep 17 00:00:00 2001 From: Joshua Gigg Date: Wed, 24 Apr 2013 17:33:27 +0200 Subject: [PATCH 2/7] Allow vertical scrolling on Network graph --- app/assets/stylesheets/sections/graph.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/sections/graph.scss b/app/assets/stylesheets/sections/graph.scss index 7da00719b3..58c2c20321 100644 --- a/app/assets/stylesheets/sections/graph.scss +++ b/app/assets/stylesheets/sections/graph.scss @@ -13,7 +13,8 @@ background: #f1f1f1; cursor: move; height: 500px; - overflow: hidden; + overflow-y: scroll; + overflow-x: hidden; } } From 85c468ec480a1541de36da07fc5fb4ca73c9ad5e Mon Sep 17 00:00:00 2001 From: Sato Hiroyuki Date: Wed, 24 Apr 2013 16:49:01 +0000 Subject: [PATCH 3/7] Render graph partially. --- app/assets/javascripts/branch-graph.js.coffee | 90 ++++++++++++------- 1 file changed, 59 insertions(+), 31 deletions(-) diff --git a/app/assets/javascripts/branch-graph.js.coffee b/app/assets/javascripts/branch-graph.js.coffee index 2a668de278..0ce067b961 100644 --- a/app/assets/javascripts/branch-graph.js.coffee +++ b/app/assets/javascripts/branch-graph.js.coffee @@ -9,6 +9,7 @@ class BranchGraph @offsetY = 20 @unitTime = 30 @unitSpace = 10 + @prev_start = -1 @load() load: -> @@ -24,10 +25,18 @@ class BranchGraph prepareData: (@days, @commits) -> @collectParents() + @graphHeight = $(@element).height() + @graphWidth = $(@element).width() + ch = Math.max(@graphHeight, @offsetY + @unitTime * @mtime + 150) + cw = Math.max(@graphWidth, @offsetX + @unitSpace * @mspace + 300) + @r = Raphael(@element.get(0), cw, ch) + @top = @r.set() + @barHeight = Math.max(@graphHeight, @unitTime * @days.length + 320) for c in @commits c.isParent = true if c.id of @parents @preparedCommits[c.id] = c + @markCommit(c) @collectColors() @@ -49,18 +58,12 @@ class BranchGraph k++ buildGraph: -> - graphHeight = $(@element).height() - graphWidth = $(@element).width() - ch = Math.max(graphHeight, @offsetY + @unitTime * @mtime + 150) - cw = Math.max(graphWidth, @offsetX + @unitSpace * @mspace + 300) - @r = r = Raphael(@element.get(0), cw, ch) - top = r.set() + r = @r cuday = 0 cumonth = "" - barHeight = Math.max(graphHeight, @unitTime * @days.length + 320) - r.rect(0, 0, 26, barHeight).attr fill: "#222" - r.rect(26, 0, 20, barHeight).attr fill: "#444" + r.rect(0, 0, 26, @barHeight).attr fill: "#222" + r.rect(26, 0, 20, @barHeight).attr fill: "#444" for day, mm in @days if cuday isnt day[0] @@ -81,23 +84,41 @@ class BranchGraph ) cumonth = day[1] - for commit in @commits - x = @offsetX + @unitSpace * (@mspace - commit.space) - y = @offsetY + @unitTime * commit.time + @renderPartialGraph() - @drawDot(x, y, commit) - - @drawLines(x, y, commit) - - @appendLabel(x, y, commit.refs) if commit.refs - - @appendAnchor(top, commit, x, y) - - @markCommit(x, y, commit, graphHeight) - - top.toFront() @bindEvents() + renderPartialGraph: -> + start = Math.floor((@element.scrollTop() - @offsetY) / @unitTime) - 10 + start = 0 if start < 0 + end = start + 40 + end = @commits.length if @commits.length < end + + if @prev_start == -1 or Math.abs(@prev_start - start) > 10 + i = start + + @prev_start = start + + while i < end + commit = @commits[i] + i += 1 + + if commit.hasDrawn isnt true + x = @offsetX + @unitSpace * (@mspace - commit.space) + y = @offsetY + @unitTime * commit.time + + @drawDot(x, y, commit) + + @drawLines(x, y, commit) + + @appendLabel(x, y, commit) + + @appendAnchor(x, y, commit) + + commit.hasDrawn = true + + @top.toFront() + bindEvents: -> drag = {} element = @element @@ -114,9 +135,10 @@ class BranchGraph $(window).on "mousemove", dragger $(window).on - mouseup: -> + mouseup: => $(window).off "mousemove", dragger - keydown: (event) -> + @renderPartialGraph() + keydown: (event) => # left element.scrollLeft element.scrollLeft() - 50 if event.keyCode is 37 # top @@ -125,17 +147,20 @@ class BranchGraph element.scrollLeft element.scrollLeft() + 50 if event.keyCode is 39 # bottom element.scrollTop element.scrollTop() + 50 if event.keyCode is 40 + @renderPartialGraph() + + appendLabel: (x, y, commit) -> + return unless commit.refs - appendLabel: (x, y, refs) -> r = @r - shortrefs = refs + shortrefs = commit.refs # Truncate if longer than 15 chars shortrefs = shortrefs.substr(0, 15) + "…" if shortrefs.length > 17 text = r.text(x + 4, y, shortrefs).attr( "text-anchor": "start" font: "10px Monaco, monospace" fill: "#FFF" - title: refs + title: commit.refs ) textbox = text.getBBox() # Create rectangle based on the size of the textbox @@ -156,8 +181,9 @@ class BranchGraph # Set text to front text.toFront() - appendAnchor: (top, commit, x, y) -> + appendAnchor: (x, y, commit) -> r = @r + top = @top options = @options anchor = r.circle(x, y, 10).attr( fill: "#000" @@ -240,16 +266,18 @@ class BranchGraph stroke: color "stroke-width": 2) - markCommit: (x, y, commit, graphHeight) -> + markCommit: (commit) -> if commit.id is @options.commit_id r = @r + x = @offsetX + @unitSpace * (@mspace - commit.space) + y = @offsetY + @unitTime * commit.time r.path(["M", x + 5, y, "L", x + 15, y + 4, "L", x + 15, y - 4, "Z"]).attr( fill: "#000" "fill-opacity": .5 stroke: "none" ) # Displayed in the center - @element.scrollTop(y - graphHeight / 2) + @element.scrollTop(y - @graphHeight / 2) Raphael::commitTooltip = (x, y, commit) -> boxWidth = 300 From b36b40ca40c1a380e74ac5384ef40938323307a8 Mon Sep 17 00:00:00 2001 From: Sato Hiroyuki Date: Thu, 25 Apr 2013 21:53:30 +0900 Subject: [PATCH 4/7] Fix minor bug about line overlap. --- app/models/network/graph.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index ea7d188cc3..27072836cb 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -190,7 +190,7 @@ module Network l.spaces << space # Also add space to parent l.parents(@map).each do |parent| - if parent.space > 0 + if 0 < parent.space && parent.space < space parent.spaces << space end end From 963ec23407760bcd86ac989916140b1e8f6d5c0c Mon Sep 17 00:00:00 2001 From: Sato Hiroyuki Date: Thu, 25 Apr 2013 22:05:22 +0900 Subject: [PATCH 5/7] Add test related to a bug of network graph. --- features/project/network.feature | 10 ++++++---- features/steps/project/project_network_graph.rb | 9 +++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/features/project/network.feature b/features/project/network.feature index a6cbd2c478..538124a4c5 100644 --- a/features/project/network.feature +++ b/features/project/network.feature @@ -11,14 +11,16 @@ Feature: Project Network Graph And page should have "master" on graph @javascript - Scenario: I should switch ref to "stable" + Scenario: I should switch "branch" and "tag" When I switch ref to "stable" - Then page should have network graph - And page should select "stable" in select box + Then page should select "stable" in select box And page should have "stable" on graph + When I switch ref to "v2.1.0" + Then page should select "v2.1.0" in select box + And page should have "v2.1.0" on graph @javascript - Scenario: I should looking for a commit by SHA of "v2.1.0" + Scenario: I should looking for a commit by SHA 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 diff --git a/features/steps/project/project_network_graph.rb b/features/steps/project/project_network_graph.rb index cf5fa751cc..763b4de2ab 100644 --- a/features/steps/project/project_network_graph.rb +++ b/features/steps/project/project_network_graph.rb @@ -30,10 +30,19 @@ class ProjectNetworkGraph < Spinach::FeatureSteps sleep 2 end + When 'I switch ref to "v2.1.0"' do + page.select 'v2.1.0', :from => 'ref' + sleep 2 + end + And 'page should select "stable" in select box' do page.should have_selector '#ref_chzn span', :text => "stable" end + And 'page should select "v2.1.0" in select box' do + page.should have_selector '#ref_chzn span', :text => "v2.1.0" + end + And 'page should have "stable" on graph' do within '.graph' do page.should have_content 'stable' From 05c2c15cd1672a46e2b31e2ba1e71b872e8993e3 Mon Sep 17 00:00:00 2001 From: Sato Hiroyuki Date: Thu, 25 Apr 2013 19:58:25 +0000 Subject: [PATCH 6/7] Fix Gitlab::Git::Repository#commit returns wrong commit, if commit_id is "tag name". --- lib/extracts_path.rb | 4 +--- lib/gitlab/git/repository.rb | 11 ++++++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 009c5fcada..1b7c698d0a 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -98,9 +98,7 @@ module ExtractsPath @ref, @path = extract_ref(@id) - # It is used "@project.repository.commits(@ref, @path, 1, 0)", - # because "@project.repository.commit(@ref)" returns wrong commit when @ref is tag name. - @commit = @project.repository.commits(@ref, @path, 1, 0).first + @commit = @project.repository.commit(@ref) @tree = Tree.new(@project.repository, @commit.id, @ref, @path) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index ddead51d44..0218f2fe0e 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -49,7 +49,16 @@ module Gitlab def commit(commit_id = nil) commit = if commit_id - repo.commit(commit_id) + # Find repo.refs first, + # because if commit_id is "tag name", + # repo.commit(commit_id) returns wrong commit sha + # that is git tag object sha. + ref = repo.refs.find {|r| r.name == commit_id} + if ref + ref.commit + else + repo.commit(commit_id) + end else repo.commits(root_ref).first end From 1ae9c697349f1b001c077c27149987177d264524 Mon Sep 17 00:00:00 2001 From: Sato Hiroyuki Date: Thu, 25 Apr 2013 22:10:09 +0000 Subject: [PATCH 7/7] Draging network graph is replaced by vertical scrolling. --- app/assets/javascripts/branch-graph.js.coffee | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/branch-graph.js.coffee b/app/assets/javascripts/branch-graph.js.coffee index 0ce067b961..c12e672d66 100644 --- a/app/assets/javascripts/branch-graph.js.coffee +++ b/app/assets/javascripts/branch-graph.js.coffee @@ -122,22 +122,11 @@ class BranchGraph bindEvents: -> drag = {} element = @element - dragger = (event) -> - element.scrollLeft drag.sl - (event.clientX - drag.x) - element.scrollTop drag.st - (event.clientY - drag.y) - element.on mousedown: (event) -> - drag = - x: event.clientX - y: event.clientY - st: element.scrollTop() - sl: element.scrollLeft() - $(window).on "mousemove", dragger + $(element).scroll (event) => + @renderPartialGraph() $(window).on - mouseup: => - $(window).off "mousemove", dragger - @renderPartialGraph() keydown: (event) => # left element.scrollLeft element.scrollLeft() - 50 if event.keyCode is 37