MR: Handle broken diff ex. in case its too huge

Dmitriy Zaporozhets
1 parent d40b9ce2
Showing 3 changed files with 38 additions and 19 deletions Show diff stats
app/models/merge_request.rb
app/views/layouts/_project_menu.html.haml
app/views/merge_requests/show/_diffs.html.haml
@@ -3,6 +3,8 @@ require File.join(Rails.root, &quot;app/models/commit&quot;)
 class MergeRequest < ActiveRecord::Base
   include Upvote
  
+  BROKEN_DIFF = "--broken-diff"
+
   UNCHECKED = 1
   CAN_BE_MERGED = 2
   CANNOT_BE_MERGED = 3
@@ -108,14 +110,25 @@ class MergeRequest &lt; ActiveRecord::Base
   def reloaded_diffs
     if open? && unmerged_diffs.any?
       self.st_diffs = unmerged_diffs
-      save
+      self.save
     end
-    diffs
+
+  rescue Grit::Git::GitTimeout
+    self.st_diffs = [BROKEN_DIFF]
+    self.save
+  end
+
+  def broken_diffs?
+    diffs == [BROKEN_DIFF]
+  end
+
+  def valid_diffs?
+    !broken_diffs?
   end
  
   def unmerged_diffs
     commits = project.repo.commits_between(target_branch, source_branch).map {|c| Commit.new(c)}
-    diffs = project.repo.diff(commits.first.prev_commit.id, commits.last.id) rescue []
+    diffs = project.repo.diff(commits.first.prev_commit.id, commits.last.id)
   end
  
   def last_commit
@@ -13,24 +13,25 @@
       %li{:class => tab_class(:network)}
         = link_to "Network", graph_project_path(@project)
  
-    - if @project.issues_enabled
-      %li{:class => tab_class(:issues)}
-        = link_to  project_issues_filter_path(@project)  do
-          Issues
-          %span.count= @project.issues.opened.count
+  - if @project.issues_enabled
+    %li{:class => tab_class(:issues)}
+      = link_to  project_issues_filter_path(@project)  do
+        Issues
+        %span.count= @project.issues.opened.count
  
+  - if @project.repo_exists?
     - if @project.merge_requests_enabled
       %li{:class => tab_class(:merge_requests)}
         = link_to project_merge_requests_path(@project) do
           Merge Requests
           %span.count= @project.merge_requests.opened.count
  
-    - if @project.wall_enabled
-      %li{:class =>  tab_class(:wall)}
-        = link_to wall_project_path(@project) do
-          Wall
+  - if @project.wall_enabled
+    %li{:class =>  tab_class(:wall)}
+      = link_to wall_project_path(@project) do
+        Wall
  
-    - if @project.wiki_enabled
-      %li{:class =>  tab_class(:wiki)}
-        = link_to project_wiki_path(@project, :index) do
-          Wiki
+  - if @project.wiki_enabled
+    %li{:class =>  tab_class(:wiki)}
+      = link_to project_wiki_path(@project, :index) do
+        Wiki
-= render "commits/diffs", :diffs => @diffs
-- if @diffs.empty?
-  %p.cgray Nothing to merge
+- if @merge_request.valid_diffs?
+  = render "commits/diffs", :diffs => @diffs
+- elsif @merge_request.broken_diffs?
+  %h4.nothing_here_message 
+    Can't load diff.
+    You can #{link_to "download MR patch", raw_project_merge_request_path(@project, @merge_request), :class => "vlink"} instead.
+- else
+  %h4.nothing_here_message Nothing to merge
...	...	@@ -3,6 +3,8 @@ require File.join(Rails.root, "app/models/commit")
3	3	class MergeRequest < ActiveRecord::Base
4	4	include Upvote
5	5
	6	+ BROKEN_DIFF = "--broken-diff"
	7	+
6	8	UNCHECKED = 1
7	9	CAN_BE_MERGED = 2
8	10	CANNOT_BE_MERGED = 3
...	...	@@ -108,14 +110,25 @@ class MergeRequest < ActiveRecord::Base
108	110	def reloaded_diffs
109	111	if open? && unmerged_diffs.any?
110	112	self.st_diffs = unmerged_diffs
111		- save
	113	+ self.save
112	114	end
113		- diffs
	115	+
	116	+ rescue Grit::Git::GitTimeout
	117	+ self.st_diffs = [BROKEN_DIFF]
	118	+ self.save
	119	+ end
	120	+
	121	+ def broken_diffs?
	122	+ diffs == [BROKEN_DIFF]
	123	+ end
	124	+
	125	+ def valid_diffs?
	126	+ !broken_diffs?
114	127	end
115	128
116	129	def unmerged_diffs
117	130	commits = project.repo.commits_between(target_branch, source_branch).map {\|c\| Commit.new(c)}
118		- diffs = project.repo.diff(commits.first.prev_commit.id, commits.last.id) rescue []
	131	+ diffs = project.repo.diff(commits.first.prev_commit.id, commits.last.id)
119	132	end
120	133
121	134	def last_commit
...	...
...	...	@@ -13,24 +13,25 @@
13	13	%li{:class => tab_class(:network)}
14	14	= link_to "Network", graph_project_path(@project)
15	15
16		- - if @project.issues_enabled
17		- %li{:class => tab_class(:issues)}
18		- = link_to project_issues_filter_path(@project) do
19		- Issues
20		- %span.count= @project.issues.opened.count
	16	+ - if @project.issues_enabled
	17	+ %li{:class => tab_class(:issues)}
	18	+ = link_to project_issues_filter_path(@project) do
	19	+ Issues
	20	+ %span.count= @project.issues.opened.count
21	21
	22	+ - if @project.repo_exists?
22	23	- if @project.merge_requests_enabled
23	24	%li{:class => tab_class(:merge_requests)}
24	25	= link_to project_merge_requests_path(@project) do
25	26	Merge Requests
26	27	%span.count= @project.merge_requests.opened.count
27	28
28		- - if @project.wall_enabled
29		- %li{:class => tab_class(:wall)}
30		- = link_to wall_project_path(@project) do
31		- Wall
	29	+ - if @project.wall_enabled
	30	+ %li{:class => tab_class(:wall)}
	31	+ = link_to wall_project_path(@project) do
	32	+ Wall
32	33
33		- - if @project.wiki_enabled
34		- %li{:class => tab_class(:wiki)}
35		- = link_to project_wiki_path(@project, :index) do
36		- Wiki
	34	+ - if @project.wiki_enabled
	35	+ %li{:class => tab_class(:wiki)}
	36	+ = link_to project_wiki_path(@project, :index) do
	37	+ Wiki
...	...
1		-= render "commits/diffs", :diffs => @diffs
2		-- if @diffs.empty?
3		- %p.cgray Nothing to merge
	1	+- if @merge_request.valid_diffs?
	2	+ = render "commits/diffs", :diffs => @diffs
	3	+- elsif @merge_request.broken_diffs?
	4	+ %h4.nothing_here_message
	5	+ Can't load diff.
	6	+ You can #{link_to "download MR patch", raw_project_merge_request_path(@project, @merge_request), :class => "vlink"} instead.
	7	+- else
	8	+ %h4.nothing_here_message Nothing to merge
...	...