Redmine

Toshi MARUYAMA wrote:

Sorry, skipping is need in both view and Repository#fetch_changesets.

I've spent some time reviewing and testing this. I don't think we need to change Repository.fetch_changesets (the class method) at all, only the Repository#show method in the repositories controller needs to be modified.
The reason for this lies in the situation that Repository.fetch_changesets is already scoped to iterate only over projects which are active (as in STATUS_ACTIVE, as opposed to STATUS_CLOSED and STATUS_ARCHIVED) as a result of the use of the active scope (defined in source:/trunk/app/models/project.rb@11784#L89):

Project.active.has_module(:repository).all.each do |project|
 ...
end

I think by the way that the change I proposed in the description can be optimized by changing the order of the conditions (source:/trunk/app/controllers/repositories_controller.rb@11784#L114):

Index: app/controllers/repositories_controller.rb
===================================================================
--- app/controllers/repositories_controller.rb  (revision 11784)
+++ app/controllers/repositories_controller.rb  (working copy)
@@ -111,7 +111,7 @@
   end

   def show
-    @repository.fetch_changesets if Setting.autofetch_changesets? && @path.empty?
+    @repository.fetch_changesets if @project.active? && Setting.autofetch_changesets? && @path.empty?

     @entries = @repository.entries(@path, @rev)
     @changeset = @repository.find_changeset_by_name(@rev)

I've did a quick scan for any affected tests but couldn't find any. I do think that it would be good if this controller behavior is tested before it gets committed.

Patch committed with tests in r11838, thanks.