Vote #73861
完了Disable autofetching of repository changesets if projects are closed
0%
説明
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)
journals
--------------------------------------------------------------------------------
I think it is better to skip in Repository#fetch_changesets.
<pre><code class="diff">
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
</code></pre>
--------------------------------------------------------------------------------
Sorry, Repository#fetch_changesets is class method, so note-2 is wrong.
This is correct code.
<pre><code class="diff">
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
</code></pre>
--------------------------------------------------------------------------------
Sorry, skipping is need in both view and Repository#fetch_changesets.
--------------------------------------------------------------------------------
+1
--------------------------------------------------------------------------------
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):
<pre><code class="ruby">
Project.active.has_module(:repository).all.each do |project|
...
end
</code></pre>
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):
<pre><code class="diff">
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)
</code></pre>
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.
--------------------------------------------------------------------------------