Merge pull request #4262 from eduardoj/fulldiff_feature

Allow users to show the full diff of large diffs

src/api/app/assets/stylesheets/webui/application.css.erb

@@ -32,4 +32,6 @@
  *= require webui/application/jobhistory.css.scss
  *= require webui/application/kiwi_image.css.scss
  *= require webui/application/image_templates
+ *= require webui/application/flash
+ *= require webui/application/box-extensions
 */

src/api/app/assets/stylesheets/webui/application/box-extensions.scss

@@ -0,0 +1,3 @@
+.box-invisible {
+  padding: 0px 0 10px 0;
+}

src/api/app/assets/stylesheets/webui/application/flash.scss

@@ -0,0 +1,21 @@
+.ui-state-info {
+  background-color: #bfd7ff;
+  border: 1px solid #052d6d;
+  color: #000;
+
+  a:link, a:visited {
+    color: #000;
+    text-decoration: underline;
+  }
+
+  a:active, a:hover {
+    color: #000;
+    text-decoration: none;
+  }
+}
+
+.flash-message {
+  p {
+      margin: 10px;
+  }
+}

src/api/app/assets/stylesheets/webui/application/style.scss

@@ -420,10 +420,3 @@ div.dataTables_wrapper:after {
 #subheader #breadcrump {
   line-height: 1.3em;
 }
-
-// flash messages
-.flash-message {
-  p {
-      margin: 10px;
-  }
-}

src/api/app/controllers/webui/package_controller.rb

@@ -450,17 +450,22 @@ def rdiff
     @last_req = find_last_req
 
     @rev = params[:rev] || @last_rev
+    @linkrev = params[:linkrev]
 
     options = {}
     [:orev, :opackage, :oproject, :linkrev, :olinkrev].each do |k|
       options[k] = params[k] if params[k].present?
     end
     options[:rev] = @rev if @rev
+    options[:filelimit] = 0 if params[:full_diff]
+    options[:tarlimit] = 0 if params[:full_diff]
     return unless get_diff(@project.name, @package.name, options)
 
     # we only look at [0] because this is a generic function for multi diffs - but we're sure we get one
     filenames = sorted_filenames_from_sourcediff(@rdiff)[0]
+
     @files = filenames['files']
+    @not_full_diff = @files.any? { |file| file[1]['diff']['shown'] }
     @filenames = filenames['filenames']
   end
 

src/api/app/controllers/webui/request_controller.rb

@@ -79,7 +79,9 @@ def modify_review
   end
 
   def show
-    @req = @bs_request.webui_infos
+    diff_limit = params[:full_diff] ? 0 : nil
+
+    @req = @bs_request.webui_infos(filelimit: diff_limit, tarlimit: diff_limit)
     @id = @req['id']
     @number = @req['number']
     @state = @req['state'].to_s
@@ -97,6 +99,9 @@ def show
     @history = @bs_request.history_elements
     @actions = @req['actions']
 
+    # print a hint that the diff is not fully shown (this only needs to be verified for submit actions)
+    @not_full_diff = BsRequest.truncated_diffs?(@req)
+
     # retrieve a list of all package maintainers that are assigned to at least one target package
     @package_maintainers = get_target_package_maintainers(@actions) || []
 

src/api/app/mixins/request_source_diff.rb

@@ -71,7 +71,8 @@ def diff_for_source(spkg, options = {})
         end
 
         path = Package.source_path(action.source_project, spkg)
-        query[:filelimit] = 10000
+        query[:filelimit] = options[:filelimit] ? options[:filelimit].to_i : 10000
+        query[:tarlimit] = options[:tarlimit] ? options[:tarlimit].to_i : 10000
 
         if !provided_in_other_action && !action.updatelink
           # do show the same diff multiple times, so just diff unexpanded so we see possible link changes instead

src/api/app/models/bs_request.rb

@@ -905,7 +905,7 @@ def webui_infos(opts = {})
 
     result['my_open_reviews'], result['other_open_reviews'] = reviews_for_user_and_others(User.current)
 
-    result['actions'] = webui_actions(opts[:diffs])
+    result['actions'] = webui_actions(opts)
     result
   end
 
@@ -1033,9 +1033,10 @@ def notify
     end
   end
 
-  def webui_actions(with_diff = true)
+  def webui_actions(opts = {})
     # TODO: Fix!
     actions = []
+    with_diff = opts.delete(:diffs)
     bs_request_actions.each do |xml|
       action = { type: xml.action_type }
 
@@ -1056,7 +1057,7 @@ def webui_actions(with_diff = true)
       case xml.action_type # All further stuff depends on action type...
       when :submit then
         action[:name] = "Submit #{action[:spkg]}"
-        action[:sourcediff] = xml.webui_infos if with_diff
+        action[:sourcediff] = xml.webui_infos(opts) if with_diff
         creator = User.find_by_login(self.creator)
         target_package = Package.find_by_project_and_name(action[:tprj], action[:tpkg])
         action[:creator_is_target_maintainer] = true if creator.has_local_role?(Role.hashed['maintainer'], target_package)
@@ -1160,6 +1161,17 @@ def update_cache
     # rubocop:enable Rails/SkipsModelValidations
   end
 
+  def self.truncated_diffs?(request)
+    request['actions'].select { |action| action[:type] == :submit && action[:sourcediff] }.each do |action|
+      action[:sourcediff].each do |sourcediff|
+        # the 'shown' attribute is only set if the backend truncated the diff
+        return true if sourcediff['files'].any? { |file| file[1]['diff']['shown'] }
+      end
+    end
+
+    false
+  end
+
   private
 
   def raisepriority(new_priority)

src/api/app/models/bs_request_action.rb

@@ -302,9 +302,12 @@ def sourcediff(_opts = {})
     ''
   end
 
-  def webui_infos
+  def webui_infos(opts = {})
     begin
-      sd = sourcediff(view: 'xml', withissues: true)
+      opts[:view] = 'xml'
+      opts[:withissues] = true
+
+      sd = sourcediff(opts)
     rescue DiffError, Project::UnknownObjectError, Package::UnknownObjectError => e
       return [{ error: e.message }]
     end

src/api/app/views/webui/package/rdiff.html.erb

@@ -10,6 +10,11 @@
     <h3>Changes of Revision <%= @rev %></h3>
 <% end %>
 
+<% if @not_full_diff %>
+  <%= render partial: 'webui/shared/truncated_diff_hint', locals: { type: :package } %>
+  <br />
+<% end %>
+
 <%= render(partial: 'shared/sourcediff', locals: { filenames: @filenames, files: @files, source: { project: @project, package: @package, rev: @rev }, editor_width: '915px' }) %>
 
 <% unless @last_req.blank? %>

src/api/app/views/webui/request/show.html.erb

@@ -72,6 +72,12 @@
   </div>
 </div>
 
+<% if @not_full_diff %>
+  <div class="grid_16 alpha omega box-invisible">
+    <%= render partial: 'webui/shared/truncated_diff_hint', locals: { type: :request } %>
+  </div>
+<% end %>
+
 <div class="grid_16 alpha omega box box-shadow">
 
   <div class="box-header header-tabs">

src/api/app/views/webui/shared/_truncated_diff_hint.html.erb

@@ -0,0 +1,11 @@
+<div id="truncated_diff_hint" class="ui-state-info ui-corner-all ui-widget-shadow padding-10pt">
+  <span class="ui-icon ui-icon-info">
+    We truncated the diff of some files because they were too big. If you want to see the full diff for
+    every file,
+    <% if type == :request %>
+      <%= link_to 'click here', request_show_path(number: @number, full_diff: true) %>.
+    <% elsif type == :package %>
+      <%= link_to 'click here', package_rdiff_path(project: @project, package: @package, linkrev: @linkrev, rev: @rev, full_diff: true) %>.
+    <% end %>
+  </span>
+</div>

src/api/lib/backend/api/sources/package.rb

@@ -111,10 +111,11 @@ def self.write_link(project_name, package_name, user_login, content)
         # @option options [String] :linkrev Use the revision of the linked package.
         # @option options [String] :olinkrev Use the origin revision of the linked package.
         # @option options [String] :expand Expand sources.
+        # @option options [String] :filelimit Sets the maximum lines of the diff which will be returned (0 = all lines)
         # @return [String]
         def self.source_diff(project_name, package_name, options = {})
           post(['/source/:project/:package', project_name, package_name], defaults: { cmd: :diff, view: :xml, withissues: 1 },
-               params: options, accepted: [:rev, :orev, :opackage, :oproject, :linkrev, :olinkrev, :expand])
+               params: options, accepted: [:rev, :orev, :opackage, :oproject, :linkrev, :olinkrev, :expand, :filelimit, :tarlimit])
         end
 
         # Runs the command rebuild for that package