From 2e1bcb2abff6f78f028064299125480cbf3c2c2a Mon Sep 17 00:00:00 2001 From: Toshi MARUYAMA Date: Sun, 2 Jan 2011 09:45:05 +0000 Subject: [PATCH] Changing revision label and identifier at SCM adapter level (#3724, #6092) Contributed by Yuya Nishihara. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4613 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/repositories_controller.rb | 3 ++ app/helpers/application_helper.rb | 4 +- app/helpers/repositories_helper.rb | 14 +++-- app/models/changeset.rb | 22 +++++++- app/models/repository/git.rb | 10 ++++ .../repositories/_dir_list_content.rhtml | 2 +- app/views/repositories/_revisions.rhtml | 6 +-- app/views/repositories/annotate.rhtml | 2 +- app/views/repositories/diff.rhtml | 2 +- app/views/repositories/revision.rhtml | 12 ++--- lib/redmine/scm/adapters/abstract_adapter.rb | 13 ++++- lib/redmine/scm/adapters/git_adapter.rb | 7 +++ test/unit/changeset_test.rb | 5 ++ test/unit/repository_bazaar_test.rb | 2 +- test/unit/repository_git_test.rb | 28 +++++++++- test/unit/repository_subversion_test.rb | 53 ++++++++++++++++++- 16 files changed, 161 insertions(+), 24 deletions(-) diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index a71f7bf45..6195c6409 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -174,6 +174,9 @@ class RepositoriesController < ApplicationController @diff = @repository.diff(@path, @rev, @rev_to) show_error_not_found unless @diff end + + @changeset = @repository.find_changeset_by_name(@rev) + @changeset_to = @rev_to ? @repository.find_changeset_by_name(@rev_to) : nil end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index e1048ea22..3cdd3add5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -104,8 +104,10 @@ module ApplicationHelper # * :text - Link text (default to the formatted revision) def link_to_revision(revision, project, options={}) text = options.delete(:text) || format_revision(revision) + rev = revision.respond_to?(:identifier) ? revision.identifier : revision - link_to(text, {:controller => 'repositories', :action => 'revision', :id => project, :rev => revision}, :title => l(:label_revision_id, revision)) + link_to(text, {:controller => 'repositories', :action => 'revision', :id => project, :rev => rev}, + :title => l(:label_revision_id, format_revision(revision))) end # Generates a link to a project if active diff --git a/app/helpers/repositories_helper.rb b/app/helpers/repositories_helper.rb index 19a86e00c..63ea52238 100644 --- a/app/helpers/repositories_helper.rb +++ b/app/helpers/repositories_helper.rb @@ -18,8 +18,12 @@ require 'iconv' module RepositoriesHelper - def format_revision(txt) - txt.to_s[0,8] + def format_revision(revision) + if revision.respond_to? :format_identifier + revision.format_identifier + else + revision.to_s + end end def truncate_at_line_break(text, length = 255) @@ -87,7 +91,7 @@ module RepositoriesHelper :action => 'show', :id => @project, :path => path_param, - :rev => @changeset.revision) + :rev => @changeset.identifier) output << "
  • #{text}
    • " output << render_changes_tree(s) elsif c = tree[file][:c] @@ -97,13 +101,13 @@ module RepositoriesHelper :action => 'entry', :id => @project, :path => path_param, - :rev => @changeset.revision) unless c.action == 'D' + :rev => @changeset.identifier) unless c.action == 'D' text << " - #{c.revision}" unless c.revision.blank? text << ' (' + link_to('diff', :controller => 'repositories', :action => 'diff', :id => @project, :path => path_param, - :rev => @changeset.revision) + ') ' if c.action == 'M' + :rev => @changeset.identifier) + ') ' if c.action == 'M' text << ' ' + content_tag('span', c.from_path, :class => 'copied-from') unless c.from_path.blank? output << "
  • #{text}
    • " end diff --git a/app/models/changeset.rb b/app/models/changeset.rb index ca4daace8..e49113ebf 100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@ -23,10 +23,10 @@ class Changeset < ActiveRecord::Base has_many :changes, :dependent => :delete_all has_and_belongs_to_many :issues - acts_as_event :title => Proc.new {|o| "#{l(:label_revision)} #{o.revision}" + (o.short_comments.blank? ? '' : (': ' + o.short_comments))}, + acts_as_event :title => Proc.new {|o| "#{l(:label_revision)} #{o.format_identifier}" + (o.short_comments.blank? ? '' : (': ' + o.short_comments))}, :description => :long_comments, :datetime => :committed_on, - :url => Proc.new {|o| {:controller => 'repositories', :action => 'revision', :id => o.repository.project, :rev => o.revision}} + :url => Proc.new {|o| {:controller => 'repositories', :action => 'revision', :id => o.repository.project, :rev => o.identifier}} acts_as_searchable :columns => 'comments', :include => {:repository => :project}, @@ -47,6 +47,15 @@ class Changeset < ActiveRecord::Base def revision=(r) write_attribute :revision, (r.nil? ? nil : r.to_s) end + + # Returns the identifier of this changeset; depending on repository backends + def identifier + if repository.class.respond_to? :changeset_identifier + repository.class.changeset_identifier self + else + revision.to_s + end + end def comments=(comment) write_attribute(:comments, Changeset.normalize_comments(comment)) @@ -56,6 +65,15 @@ class Changeset < ActiveRecord::Base self.commit_date = date super end + + # Returns the readable identifier + def format_identifier + if repository.class.respond_to? :format_changeset_identifier + repository.class.format_changeset_identifier self + else + identifier + end + end def committer=(arg) write_attribute(:committer, self.class.to_utf8(arg.to_s)) diff --git a/app/models/repository/git.rb b/app/models/repository/git.rb index 473eb07e2..9349f3c11 100644 --- a/app/models/repository/git.rb +++ b/app/models/repository/git.rb @@ -29,6 +29,16 @@ class Repository::Git < Repository 'Git' end + # Returns the identifier for the given git changeset + def self.changeset_identifier(changeset) + changeset.scmid + end + + # Returns the readable identifier for the given git changeset + def self.format_changeset_identifier(changeset) + changeset.revision[0, 8] + end + def branches scm.branches end diff --git a/app/views/repositories/_dir_list_content.rhtml b/app/views/repositories/_dir_list_content.rhtml index c5bd53ea7..66574f1c8 100644 --- a/app/views/repositories/_dir_list_content.rhtml +++ b/app/views/repositories/_dir_list_content.rhtml @@ -17,7 +17,7 @@ <%= (entry.size ? number_to_human_size(entry.size) : "?") unless entry.is_dir? %> <% changeset = @project.repository.changesets.find_by_revision(entry.lastrev.identifier) if entry.lastrev && entry.lastrev.identifier %> -<%= link_to_revision(changeset.revision, @project) if changeset %> +<%= link_to_revision(changeset, @project) if changeset %> <%= distance_of_time_in_words(entry.lastrev.time, Time.now) if entry.lastrev && entry.lastrev.time %> <%= changeset.nil? ? h(entry.lastrev.author.to_s.split('<').first) : changeset.author if entry.lastrev %> <%=h truncate(changeset.comments, :length => 50) unless changeset.nil? %> diff --git a/app/views/repositories/_revisions.rhtml b/app/views/repositories/_revisions.rhtml index 26fb5b699..92c6fb535 100644 --- a/app/views/repositories/_revisions.rhtml +++ b/app/views/repositories/_revisions.rhtml @@ -13,9 +13,9 @@ <% line_num = 1 %> <% revisions.each do |changeset| %> -<%= link_to_revision(changeset.revision, project) %> -<%= radio_button_tag('rev', changeset.revision, (line_num==1), :id => "cb-#{line_num}", :onclick => "$('cbto-#{line_num+1}').checked=true;") if show_diff && (line_num < revisions.size) %> -<%= radio_button_tag('rev_to', changeset.revision, (line_num==2), :id => "cbto-#{line_num}", :onclick => "if ($('cb-#{line_num}').checked==true) {$('cb-#{line_num-1}').checked=true;}") if show_diff && (line_num > 1) %> +<%= link_to_revision(changeset, project) %> +<%= radio_button_tag('rev', changeset.identifier, (line_num==1), :id => "cb-#{line_num}", :onclick => "$('cbto-#{line_num+1}').checked=true;") if show_diff && (line_num < revisions.size) %> +<%= radio_button_tag('rev_to', changeset.identifier, (line_num==2), :id => "cbto-#{line_num}", :onclick => "if ($('cb-#{line_num}').checked==true) {$('cb-#{line_num-1}').checked=true;}") if show_diff && (line_num > 1) %> <%= format_time(changeset.committed_on) %> <%=h changeset.author %> <%= textilizable(truncate_at_line_break(changeset.comments)) %> diff --git a/app/views/repositories/annotate.rhtml b/app/views/repositories/annotate.rhtml index a18e9bbac..498507148 100644 --- a/app/views/repositories/annotate.rhtml +++ b/app/views/repositories/annotate.rhtml @@ -19,7 +19,7 @@ <%= line_num %> - <%= (revision.identifier ? link_to(format_revision(revision.identifier), :action => 'revision', :id => @project, :rev => revision.identifier) : format_revision(revision.revision)) if revision %> + <%= (revision.identifier ? link_to_revision(revision, @project) : format_revision(revision)) if revision %> <%= h(revision.author.to_s.split('<').first) if revision %> 
    <%= line %>
    diff --git a/app/views/repositories/diff.rhtml b/app/views/repositories/diff.rhtml index 24f92a540..e2323549e 100644 --- a/app/views/repositories/diff.rhtml +++ b/app/views/repositories/diff.rhtml @@ -1,4 +1,4 @@ -

    <%= l(:label_revision) %> <%= format_revision(@rev_to) + ':' if @rev_to %><%= format_revision(@rev) %> <%=h @path %>

    +

    <%= l(:label_revision) %> <%= format_revision(@changeset_to) + ':' if @changeset_to %><%= format_revision(@changeset) %> <%=h @path %>

    <% form_tag({:path => to_path_param(@path)}, :method => 'get') do %> diff --git a/app/views/repositories/revision.rhtml b/app/views/repositories/revision.rhtml index 92597dff7..483e358de 100644 --- a/app/views/repositories/revision.rhtml +++ b/app/views/repositories/revision.rhtml @@ -1,25 +1,25 @@
    « <% unless @changeset.previous.nil? -%> - <%= link_to_revision(@changeset.previous.revision, @project, :text => l(:label_previous)) %> + <%= link_to_revision(@changeset.previous, @project, :text => l(:label_previous)) %> <% else -%> <%= l(:label_previous) %> <% end -%> | <% unless @changeset.next.nil? -%> - <%= link_to_revision(@changeset.next.revision, @project, :text => l(:label_next)) %> + <%= link_to_revision(@changeset.next, @project, :text => l(:label_next)) %> <% else -%> <%= l(:label_next) %> <% end -%> »  <% form_tag({:controller => 'repositories', :action => 'revision', :id => @project, :rev => nil}, :method => :get) do %> - <%= text_field_tag 'rev', @rev[0,8], :size => 8 %> + <%= text_field_tag 'rev', @rev, :size => 8 %> <%= submit_tag 'OK', :name => nil %> <% end %>
    -

    <%= l(:label_revision) %> <%= format_revision(@changeset.revision) %>

    +

    <%= l(:label_revision) %> <%= format_revision(@changeset) %>

    <% if @changeset.scmid %>ID: <%= @changeset.scmid %>
    <% end %> <%= authoring(@changeset.committed_on, @changeset.author) %>

    @@ -45,7 +45,7 @@
  • <%= l(:label_deleted) %>
    • -

    <%= link_to(l(:label_view_diff), :action => 'diff', :id => @project, :path => "", :rev => @changeset.revision) if @changeset.changes.any? %>

    +

    <%= link_to(l(:label_view_diff), :action => 'diff', :id => @project, :path => "", :rev => @changeset.identifier) if @changeset.changes.any? %>

    <%= render_changeset_changes %> @@ -56,4 +56,4 @@ <%= stylesheet_link_tag "scm" %> <% end %> -<% html_title("#{l(:label_revision)} #{@changeset.revision}") -%> +<% html_title("#{l(:label_revision)} #{format_revision(@changeset)}") -%> diff --git a/lib/redmine/scm/adapters/abstract_adapter.rb b/lib/redmine/scm/adapters/abstract_adapter.rb index a3ca61e23..f98e90958 100644 --- a/lib/redmine/scm/adapters/abstract_adapter.rb +++ b/lib/redmine/scm/adapters/abstract_adapter.rb @@ -271,7 +271,8 @@ module Redmine end class Revision - attr_accessor :identifier, :scmid, :name, :author, :time, :message, :paths, :revision, :branch + attr_accessor :scmid, :name, :author, :time, :message, :paths, :revision, :branch + attr_writer :identifier def initialize(attributes={}) self.identifier = attributes[:identifier] @@ -285,6 +286,16 @@ module Redmine self.branch = attributes[:branch] end + # Returns the identifier of this revision; see also Changeset model + def identifier + (@identifier || revision).to_s + end + + # Returns the readable identifier. + def format_identifier + identifier + end + def save(repo) Changeset.transaction do changeset = Changeset.new( diff --git a/lib/redmine/scm/adapters/git_adapter.rb b/lib/redmine/scm/adapters/git_adapter.rb index 7901f23d6..cd8fd95d4 100644 --- a/lib/redmine/scm/adapters/git_adapter.rb +++ b/lib/redmine/scm/adapters/git_adapter.rb @@ -264,6 +264,13 @@ module Redmine return nil if $? && $?.exitstatus != 0 cat end + + class Revision < Redmine::Scm::Adapters::Revision + # Returns the readable identifier + def format_identifier + identifier[0,8] + end + end end end end diff --git a/test/unit/changeset_test.rb b/test/unit/changeset_test.rb index 9265fe9c5..2f0415d3a 100644 --- a/test/unit/changeset_test.rb +++ b/test/unit/changeset_test.rb @@ -218,4 +218,9 @@ class ChangesetTest < ActiveSupport::TestCase c.comments = File.read("#{RAILS_ROOT}/test/fixtures/encoding/iso-8859-1.txt") assert_equal "Texte encod en ISO-8859-1.", c.comments end + + def test_identifier + c = Changeset.find_by_revision('1') + assert_equal c.revision, c.identifier + end end diff --git a/test/unit/repository_bazaar_test.rb b/test/unit/repository_bazaar_test.rb index bd1c9a9b4..5cd7da13c 100644 --- a/test/unit/repository_bazaar_test.rb +++ b/test/unit/repository_bazaar_test.rb @@ -77,7 +77,7 @@ class RepositoryBazaarTest < ActiveSupport::TestCase def test_annotate annotate = @repository.scm.annotate('doc-mkdir.txt') assert_equal 17, annotate.lines.size - assert_equal 1, annotate.revisions[0].identifier + assert_equal '1', annotate.revisions[0].identifier assert_equal 'jsmith@', annotate.revisions[0].author assert_equal 'mkdir', annotate.lines[0] end diff --git a/test/unit/repository_git_test.rb b/test/unit/repository_git_test.rb index acf4f174a..cec091760 100644 --- a/test/unit/repository_git_test.rb +++ b/test/unit/repository_git_test.rb @@ -18,7 +18,7 @@ require File.expand_path('../../test_helper', __FILE__) class RepositoryGitTest < ActiveSupport::TestCase - fixtures :projects + fixtures :projects, :repositories, :enabled_modules, :users, :roles # No '..' in the repository path REPOSITORY_PATH = RAILS_ROOT.gsub(%r{config\/\.\.}, '') + '/tmp/test/git_repository' @@ -62,6 +62,32 @@ class RepositoryGitTest < ActiveSupport::TestCase @repository.fetch_changesets assert_equal 15, @repository.changesets.count end + + def test_identifier + @repository.fetch_changesets + @repository.reload + c = @repository.changesets.find_by_revision('7234cb2750b63f47bff735edc50a1c0a433c2518') + assert_equal c.scmid, c.identifier + end + + def test_format_identifier + @repository.fetch_changesets + @repository.reload + c = @repository.changesets.find_by_revision('7234cb2750b63f47bff735edc50a1c0a433c2518') + assert_equal c.format_identifier, '7234cb27' + end + + def test_activities + @repository.fetch_changesets + @repository.reload + f = Redmine::Activity::Fetcher.new(User.anonymous, :project => Project.find(1)) + f.scope = ['changesets'] + events = f.events + assert_kind_of Array, events + eve = events[-9] + assert eve.event_title.include?('7234cb27:') + assert_equal eve.event_url[:rev], '7234cb2750b63f47bff735edc50a1c0a433c2518' + end else puts "Git test repository NOT FOUND. Skipping unit tests !!!" def test_fake; assert true end diff --git a/test/unit/repository_subversion_test.rb b/test/unit/repository_subversion_test.rb index 903cdd049..5034c30cf 100644 --- a/test/unit/repository_subversion_test.rb +++ b/test/unit/repository_subversion_test.rb @@ -18,7 +18,7 @@ require File.expand_path('../../test_helper', __FILE__) class RepositorySubversionTest < ActiveSupport::TestCase - fixtures :projects, :repositories + fixtures :projects, :repositories, :enabled_modules, :users, :roles def setup @project = Project.find(1) @@ -88,6 +88,57 @@ class RepositorySubversionTest < ActiveSupport::TestCase assert_equal 1, entries.size, 'Expect a single entry' assert_equal 'README.txt', entries.first.name end + + def test_identifier + @repository.fetch_changesets + @repository.reload + c = @repository.changesets.find_by_revision('1') + assert_equal c.revision, c.identifier + end + + def test_identifier_nine_digit + c = Changeset.new(:repository => @repository, :committed_on => Time.now, + :revision => '123456789', :comments => 'test') + assert_equal c.identifier, c.revision + end + + def test_format_identifier + @repository.fetch_changesets + @repository.reload + c = @repository.changesets.find_by_revision('1') + assert_equal c.format_identifier, c.revision + end + + def test_format_identifier_nine_digit + c = Changeset.new(:repository => @repository, :committed_on => Time.now, + :revision => '123456789', :comments => 'test') + assert_equal c.format_identifier, c.revision + end + + def test_activities + @repository.fetch_changesets + @repository.reload + f = Redmine::Activity::Fetcher.new(User.anonymous, :project => Project.find(1)) + f.scope = ['changesets'] + events = f.events + assert_kind_of Array, events + eve = events[-9] + assert eve.event_title.include?('1:') + assert_equal eve.event_url[:rev], '1' + end + + def test_activities_nine_digit + c = Changeset.new(:repository => @repository, :committed_on => Time.now, + :revision => '123456789', :comments => 'test') + assert( c.save ) + f = Redmine::Activity::Fetcher.new(User.anonymous, :project => Project.find(1)) + f.scope = ['changesets'] + events = f.events + assert_kind_of Array, events + eve = events[-11] + assert eve.event_title.include?('123456789:') + assert_equal eve.event_url[:rev], '123456789' + end else puts "Subversion test repository NOT FOUND. Skipping unit tests !!!" def test_fake; assert true end -- 2.39.5