Feature #13945
closed
Disable autofetching of repository changesets if projects are closed
0%
Description
When Setting.autofetch_changesets == true repository changesets are automatically fetched (and added) when Repository#show is executed.
This behavior feels a bit weird if a project is closed and thus read-only.
It can be disabled by adding a new condition to the if construct in source:/trunk/app/controllers/repositories_controller.rb@11784#L114, like:
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 Setting.autofetch_changesets? && @path.empty? && @project.active?
@entries = @repository.entries(@path, @rev)
@changeset = @repository.find_changeset_by_name(@rev)
Updated by Toshi MARUYAMA over 11 years ago
I think it is better to skip in Repository#fetch_changesets.
diff --git a/app/models/repository.rb b/app/models/repository.rb
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -327,6 +327,10 @@ class Repository < ActiveRecord::Base
# Can be called periodically by an external script
# eg. ruby script/runner "Repository.fetch_changesets"
def self.fetch_changesets
+ unless @project.active?
+ logger.warn "#{@project.name} is not active"
+ return
+ end
Project.active.has_module(:repository).all.each do |project|
project.repositories.each do |repository|
begin
Updated by Toshi MARUYAMA over 11 years ago
Sorry, Repository#fetch_changesets is class method, so note-2 is wrong.
This is correct code.
diff --git a/app/models/repository.rb b/app/models/repository.rb
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -328,6 +328,10 @@ class Repository < ActiveRecord::Base
# eg. ruby script/runner "Repository.fetch_changesets"
def self.fetch_changesets
Project.active.has_module(:repository).all.each do |project|
+ unless project.active?
+ logger.warn "#{project.name} is not active"
+ next
+ end
project.repositories.each do |repository|
begin
repository.fetch_changesets
Updated by Toshi MARUYAMA over 11 years ago
Sorry, skipping is need in both view and Repository#fetch_changesets.
Updated by
Updated by Mischa The Evil over 11 years ago
- Description updated (diff)
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.
Updated by Jean-Philippe Lang over 11 years ago
- Status changed from New to Closed
- Resolution set to Fixed
Patch committed with tests in r11838, thanks.