Commit, network graph refactoring

app/assets/stylesheets/highlight.black.css.scss

@@ -11,7 +11,7 @@
   .cm { color: #888888 } /* Comment.Multiline */
   .cp { color: #cc0000; font-weight: bold } /* Comment.Preproc */
   .c1 { color: #888888 } /* Comment.Single */
-  .cs { color: #cc0000; font-weight: bold; background-color: auto } /* Comment.Special */
+  .cs { color: #cc0000; font-weight: bold; background-color: transparent } /* Comment.Special */
   .gd { color: #000000; background-color: #ffdddd } /* Generic.Deleted */
   .ge { font-style: italic } /* Generic.Emph */
   .gr { color: #aa0000 } /* Generic.Error */
@@ -30,7 +30,7 @@
   .highlight .kt{color:#458;font-weight:bold;} /* Keyword.Type */
   .m { color: #0000DD; font-weight: bold } /* Literal.Number */
   .p { color: #eee;  }
-  .s { color: #dd2200; background-color: auto } /* Literal.String */
+  .s { color: #dd2200; background-color: transparent } /* Literal.String */
   .highlight .na{color:#008080;} /* Name.Attribute */
   .highlight .nb{color:#0086B3;} /* Name.Builtin */
   .highlight .nc{color:#4d3;font-weight:bold;} /* Name.Class */
@@ -48,9 +48,9 @@
   .mh { color: #0000DD; font-weight: bold } /* Literal.Number.Hex */
   .highlight .mi {color:#099;} /* Literal.Number.Integer */
   .mo { color: #0000DD; font-weight: bold } /* Literal.Number.Oct */
-  .sb { color: #dd2200; background-color: auto } /* Literal.String.Backtick */
+  .sb { color: #dd2200; background-color: transparent; } /* Literal.String.Backtick */
   .highlight .sc{color:#d14;} /* Literal.String.Char */
-  .sd { color: #dd2200; background-color: auto } /* Literal.String.Doc */
+  .sd { color: #dd2200; background-color: transparent; } /* Literal.String.Doc */
   .highlight .s2{color:orange;} /* Literal.String.Double */
   .highlight .se{color:orange;} /* Literal.String.Escape */
   .highlight .sh{color:orange;} /* Literal.String.Heredoc */

app/controllers/commits_controller.rb

@@ -13,12 +13,7 @@ class CommitsController < ApplicationController
   def index
     @repo = project.repo
     @limit, @offset = (params[:limit] || 20), (params[:offset] || 0)
-
-    @commits = if params[:path]
-                 @repo.log(@ref, params[:path], :max_count => @limit, :skip => @offset)
-               else
-                 @repo.commits(@ref, @limit, @offset)
-               end
+    @commits = @project.commits(@ref, params[:path], @limit, @offset)
 
     respond_to do |format|
       format.html # index.html.erb
@@ -28,7 +23,7 @@ class CommitsController < ApplicationController
   end
 
   def show
-    @commit = project.repo.commits(params[:id]).first
+    @commit = project.commit(params[:id])
     @notes = project.commit_notes(@commit).fresh.limit(20)
     @note = @project.build_commit_note(@commit)
 

app/controllers/projects_controller.rb

@@ -86,31 +86,7 @@ class ProjectsController < ApplicationController
   end
 
   def graph
-    @repo = project.repo
-    commits = Grit::Commit.find_all(@repo, nil, {:max_count => 650})
-    ref_cache = {}
-    commits.collect! do |commit|
-      add_refs(commit, ref_cache)
-      GraphCommit.new(commit)
-    end
-
-    days = GraphCommit.index_commits(commits)
-    @days_json = days.compact.collect{|d| [d.day, d.strftime("%b")] }.to_json
-    @commits_json = commits.collect do |c|
-      h = {}
-      h[:parents] = c.parents.collect do |p|
-        [p.id,0,0]
-      end
-      h[:author] = c.author.name.force_encoding("UTF-8")
-      h[:time] = c.time
-      h[:space] = c.space
-      h[:refs] = c.refs.collect{|r|r.name}.join(" ") unless c.refs.nil?
-      h[:id] = c.sha
-      h[:date] = c.date
-      h[:message] = c.message.force_encoding("UTF-8")
-      h[:login] = c.author.email
-      h
-    end.to_json
+    @days_json, @commits_json = GraphCommit.to_graph(project)
   end
 
   def destroy
@@ -123,17 +99,6 @@ class ProjectsController < ApplicationController
 
   protected
 
-  def add_refs(commit, ref_cache)
-    if ref_cache.empty?
-      @repo.refs.each do |ref|
-        ref_cache[ref.commit.id] ||= []
-        ref_cache[ref.commit.id] << ref
-      end
-    end
-    commit.refs = ref_cache[commit.id] if ref_cache.include? commit.id
-    commit.refs ||= []
-  end
-
   def project
     @project ||= Project.find_by_code(params[:id])
   end

app/helpers/dashboard_helper.rb

@@ -2,7 +2,7 @@ module DashboardHelper
   def dashboard_feed_path(project, object)
     case object.class.name.to_s
     when "Issue" then project_issue_path(project, project.issues.find(object.id))
-    when "Grit::Commit" then project_commit_path(project, project.repo.commits(object.id).first)
+    when "Commit" then project_commit_path(project, project.repo.commits(object.id).first)
     when "Note"
       then
       note = object

app/models/commit.rb

 class Commit
+  attr_accessor :commit
+  attr_accessor :head
+
+  delegate :message,
+    :committed_date,
+    :parents,
+    :sha,
+    :date,
+    :author,
+    :message,
+    :diffs,
+    :tree,
+    :id,
+    :to => :commit
+
+  def initialize(raw_commit, head = nil)
+    @commit = raw_commit
+    @head = head
+  end
+
+  def safe_message
+    message.force_encoding(Encoding::UTF_8)
+  end
+
+  def created_at
+    committed_date
+  end
+
+  def author_email
+    author.email.force_encoding(Encoding::UTF_8)
+  end
+
+  def author_name
+    author.name.force_encoding(Encoding::UTF_8)
+  end
 end

app/models/repository.rb

@@ -64,16 +64,17 @@ class Repository
   end
 
   def commit(commit_id = nil)
-    if commit_id
+    commit = if commit_id
                repo.commits(commit_id).first
              else
                repo.commits.first
              end
+    Commit.new(commit) if commit
   end
 
   def fresh_commits(n = 10)
     commits = heads.map do |h|
-      repo.commits(h.name, n).each { |c| c.head = h }
+      repo.commits(h.name, n).map { |c| Commit.new(c, h) }
     end.flatten.uniq { |c| c.id }
 
     commits.sort! do |x, y|
@@ -85,7 +86,7 @@ class Repository
 
   def commits_since(date)
     commits = heads.map do |h|
-      repo.log(h.name, nil, :since => date).each { |c| c.head = h }
+      repo.log(h.name, nil, :since => date).each { |c| Commit.new(c, h) }
     end.flatten.uniq { |c| c.id }
 
     commits.sort! do |x, y|
@@ -94,4 +95,14 @@ class Repository
 
     commits
   end
+
+  def commits(ref, path = nil, limit = nil, offset = nil)
+    if path
+      repo.log(ref, path, :max_count => limit, :skip => offset)
+    elsif limit && offset
+      repo.commits(ref, limit, offset)
+    else
+      repo.commits(ref)
+    end.map{ |c| Commit.new(c) } 
+  end
 end

app/views/projects/_feed.html.haml

@@ -10,6 +10,6 @@
   .right
     - klass = update.class.to_s.split("::").last.downcase
     %span.tag{ :class => klass }= klass
-    - if update.kind_of?(Grit::Commit)
+    - if update.kind_of?(Commit)
       %span.tag.commit= update.head.name
 

app/views/refs/_tree_item.html.haml

 - file = params[:path] ? File.join(params[:path], content.name) : content.name
-- content_commit = @project.repo.log(@commit.id, file, :max_count => 1).last
+- content_commit = @project.commits(@commit.id, file, 1).last
 - return unless content_commit
 %tr{ :class => "tree-item", :url => tree_file_project_ref_path(@project, @ref, file) }
   %td.tree-item-file-name

config/initializers/gitlabhq/20_grit_ext.rb

@@ -7,9 +7,5 @@ Grit::Blob.class_eval do
   include Utils::Colorize
 end
 
-Grit::Commit.class_eval do
-  include CommitExt
-end
-
 Grit::Git.git_timeout = GIT_OPTS["git_timeout"]
 Grit::Git.git_max_size = GIT_OPTS["git_max_size"]

lib/commit_ext.rb

-module CommitExt
-  attr_accessor :head
-  attr_accessor :refs
-
-  def safe_message
-    message.force_encoding(Encoding::UTF_8)
-  end
-
-  def created_at
-    committed_date
-  end
-
-  def author_email
-    author.email.force_encoding(Encoding::UTF_8)
-  end
-
-  def author_name
-    author.name.force_encoding(Encoding::UTF_8)
-  end
-end

lib/graph_commit.rb

@@ -2,14 +2,22 @@ require "grit"
 
 class GraphCommit
   attr_accessor :time, :space
-  def initialize(commit)
-    @_commit = commit
-    @time = -1
-    @space = 0
-  end
+  attr_accessor :refs
 
-  def method_missing(m, *args, &block)
-    @_commit.send(m, *args, &block)
+  def self.to_graph(project)
+    @repo = project.repo
+    commits = Grit::Commit.find_all(@repo, nil, {:max_count => 650})
+
+    ref_cache = {}
+
+    commits.map! {|c| GraphCommit.new(Commit.new(c))}
+    commits.each { |commit| commit.add_refs(ref_cache, @repo) }
+
+    days = GraphCommit.index_commits(commits)
+    @days_json = days.compact.collect{|d| [d.day, d.strftime("%b")] }.to_json
+    @commits_json = commits.map(&:to_graph_hash).to_json
+
+    return @days_json, @commits_json
   end
 
   # Method is adding time and space on the
@@ -72,4 +80,41 @@ class GraphCommit
     marks.compact.max
   end
 
+
+  def initialize(commit)
+    @_commit = commit
+    @time = -1
+    @space = 0
+  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]  = author.name.force_encoding("UTF-8")
+    h[:time]    = time
+    h[:space]   = space
+    h[:refs]    = refs.collect{|r|r.name}.join(" ") unless refs.nil?
+    h[:id]      = sha
+    h[:date]    = date
+    h[:message] = message.force_encoding("UTF-8")
+    h[:login]   = author.email
+    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

spec/requests/commits_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe "Commits" do
   let(:project) { Factory :project }
-  let!(:commit) { project.repo.commits.first }
+  let!(:commit) { project.commit }
   before do
     login_as :user
     project.add_access(@user, :read)
@@ -48,11 +48,11 @@ describe "Commits" do
 
   describe "GET /commits/:id" do
     before do
-      visit project_commit_path(project, commit)
+      visit project_commit_path(project, commit.id)
     end
 
     it "should have valid path" do
-      current_path.should == project_commit_path(project, commit)
+      current_path.should == project_commit_path(project, commit.id)
     end
   end
 end

spec/requests/projects_security_spec.rb

@@ -55,12 +55,12 @@ describe "Projects" do
     end
 
     describe "GET /project_code/commit" do
-      it { project_commit_path(@project, @project.commit).should be_allowed_for @u1 }
-      it { project_commit_path(@project, @project.commit).should be_allowed_for @u3 }
-      it { project_commit_path(@project, @project.commit).should be_denied_for :admin }
-      it { project_commit_path(@project, @project.commit).should be_denied_for @u2 }
-      it { project_commit_path(@project, @project.commit).should be_denied_for :user }
-      it { project_commit_path(@project, @project.commit).should be_denied_for :visitor }
+      it { project_commit_path(@project, @project.commit.id).should be_allowed_for @u1 }
+      it { project_commit_path(@project, @project.commit.id).should be_allowed_for @u3 }
+      it { project_commit_path(@project, @project.commit.id).should be_denied_for :admin }
+      it { project_commit_path(@project, @project.commit.id).should be_denied_for @u2 }
+      it { project_commit_path(@project, @project.commit.id).should be_denied_for :user }
+      it { project_commit_path(@project, @project.commit.id).should be_denied_for :visitor }
     end
 
     describe "GET /project_code/team" do

spec/requests/projects_spec.rb

@@ -79,6 +79,7 @@ describe "Projects" do
     end
 
     it "should beahave like activities page" do
+      save_and_open_page
       within ".project-update"  do
         page.should have_content("master")
         page.should have_content(@project.commit.author.name)