Move view_file into files controller

src/api/.rubocop_todo.yml

@@ -194,7 +194,7 @@ Metrics/BlockNesting:
     - 'app/models/bs_request_action.rb'
     - 'lib/xpath_engine.rb'
 
-# Offense count: 85
+# Offense count: 86
 # Configuration parameters: CountComments, Max, CountAsOne.
 Metrics/ClassLength:
   Exclude:
@@ -209,6 +209,7 @@ Metrics/ClassLength:
     - 'app/controllers/webui/kiwi/images_controller.rb'
     - 'app/controllers/webui/package_controller.rb'
     - 'app/controllers/webui/packages/build_log_controller.rb'
+    - 'app/controllers/webui/packages/files_controller.rb'
     - 'app/controllers/webui/patchinfo_controller.rb'
     - 'app/controllers/webui/project_controller.rb'
     - 'app/controllers/webui/packages/binaries_controller.rb'

src/api/app/components/diff_list_component.rb

@@ -32,13 +32,13 @@ def source_file(filename)
     return nil unless @source_package
     return nil unless @source_package.file_exists?(filename, { rev: @source_rev, expand: 1 }.compact)
 
-    package_view_file_path(@source_package.project, @source_package, filename, rev: @source_rev, expand: 1)
+    project_package_file_path(@source_package.project, @source_package, filename, rev: @source_rev, expand: 1)
   end
 
   def target_file(filename)
     return nil unless @target_package
     return nil unless @target_package.file_exists?(filename, expand: 1)
 
-    package_view_file_path(@target_package.project, @target_package, filename, expand: 1)
+    project_package_file_path(@target_package.project, @target_package, filename, expand: 1)
   end
 end

src/api/app/components/sourcediff_tab_component.rb

@@ -19,7 +19,9 @@ def file_view_path(filename, sourcediff)
     return if sourcediff['files'][filename]['state'] == 'deleted'
 
     diff_params = diff_data(@action[:type], sourcediff)
-    package_view_file_path(diff_params.merge(filename: filename))
+    diff_params[:project_name] = diff_params[:project]
+    diff_params[:package_name] = diff_params[:package]
+    project_package_file_path(diff_params.merge(filename: filename))
   end
 
   def release_info

src/api/app/controllers/webui/kiwi/images_controller.rb

@@ -23,7 +23,7 @@ def import_from_package
           unless package.save
             errors = package.kiwi_image.nested_error_messages.merge(title: "Kiwi File '#{kiwi_file}' has errors:")
 
-            redirect_to package_view_file_path(project: package.project, package: package, filename: kiwi_file), error: errors
+            redirect_to project_package_file_path(project_name: package.project, package_name: package, filename: kiwi_file), error: errors
             return
           end
         end

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

@@ -281,40 +281,6 @@ def trigger_services
     redirect_to package_show_path(@project, @package)
   end
 
-  def view_file
-    @filename = params[:filename] || params[:file] || ''
-    if binary_file?(@filename) # We don't want to display binary files
-      flash[:error] = "Unable to display binary file #{@filename}"
-      redirect_back(fallback_location: { action: :show, project: @project, package: @package })
-      return
-    end
-    @rev = params[:rev]
-    @expand = params[:expand]
-    @addeditlink = false
-    if User.possibly_nobody.can_modify?(@package) && @rev.blank? && @package.scmsync.blank?
-      files = @package.dir_hash({ rev: @rev, expand: @expand }.compact).elements('entry')
-      files.each do |file|
-        if file['name'] == @filename
-          @addeditlink = editable_file?(@filename, file['size'].to_i)
-          break
-        end
-      end
-    end
-    begin
-      @file = @package.source_file(@filename, fetch_from_params(:rev, :expand))
-    rescue Backend::NotFoundError
-      flash[:error] = "File not found: #{@filename}"
-      redirect_to action: :show, package: @package, project: @project
-      return
-    rescue Backend::Error => e
-      flash[:error] = "Error: #{e}"
-      redirect_back(fallback_location: { action: :show, project: @project, package: @package })
-      return
-    end
-
-    render(template: 'webui/package/simple_file_view') && return if @spider_bot
-  end
-
   def abort_build
     authorize @package, :update?
 
@@ -603,12 +569,4 @@ def get_diff(project, package, options = {})
     end
     true
   end
-
-  def fetch_from_params(*arr)
-    opts = {}
-    arr.each do |k|
-      opts[k] = params[k] if params[k].present?
-    end
-    opts
-  end
 end

src/api/app/controllers/webui/packages/files_controller.rb

@@ -1,10 +1,31 @@
 module Webui
   module Packages
     class FilesController < Packages::MainController
+      include Webui::PackageHelper
+
       before_action :set_project
       before_action :set_package
-      before_action :set_filename, only: %i[update destroy]
-      after_action :verify_authorized
+      before_action :set_filename, only: %i[show update destroy]
+      before_action :ensure_existence, only: :show
+      before_action :ensure_viewable, only: :show
+      before_action :set_file, only: :show
+
+      after_action :verify_authorized, except: :show
+
+      def show
+        @rev = params[:rev]
+        @expand = params[:expand]
+        @addeditlink = false
+
+        if User.possibly_nobody.can_modify?(@package) && @rev.blank? && @package.scmsync.blank?
+          files = @package.dir_hash({ rev: @rev, expand: @expand }.compact).elements('entry')
+          if (file = files.find { |f| f['name'] == @filename }.presence)
+            @addeditlink = editable_file?(@filename, file['size'].to_i)
+          end
+        end
+
+        render(template: 'webui/packages/files/simple_show') && return if @spider_bot
+      end
 
       def new
         authorize @package, :update?
@@ -93,6 +114,27 @@
       def set_filename
         @filename = params[:filename]
       end
+
+      def ensure_existence
+        return if @package.file_exists?(@filename, params.slice(:rev, :expand).permit!.to_h)
+
+        flash[:error] = "File not found: #{@filename}"
+        redirect_to package_show_path(project: @project, package: @package)
+      end
+
+      def ensure_viewable
+        return unless binary_file?(@filename) # We don't want to display binary files
+
+        flash[:error] = "Unable to display binary file #{@filename}"
+        redirect_back(fallback_location: package_show_path(project: @project, package: @package))
+      end
+
+      def set_file
+        @file = @package.source_file(@filename, params.slice(:rev, :expand).permit!.to_h)
+      rescue Backend::Error => e
+        flash[:error] = "Error: #{e}"
+        redirect_back(fallback_location: package_show_path(project: @project, package: @package))
+      end
     end
   end
 end

src/api/app/views/webui/package/_breadcrumb_items.html.haml

@@ -25,6 +25,3 @@
       = link_to 'Repository State', project_package_repository_binaries_path(@project, @package_name, @repository)
     %li.breadcrumb-item.active{ 'aria-current' => 'page' }
       Statistics
-  - if current_page?(package_view_file_path(@project, @package))
-    %li.breadcrumb-item.active.text-word-break-all{ 'aria-current' => 'page' }
-      = truncate(@filename, length: 50)

src/api/app/views/webui/package/_file.html.haml

@@ -1,7 +1,8 @@
 - can_be_removed = removable_file?(file_name: file['name'], package: package) && can_modify
 %tr{ id: "file-#{valid_xml_id(file['name'])}" }
   %td.text-word-break-all
-    - link_opts = { action: :view_file, project: project, package: package, filename: file['name'], expand: expand }
+    - link_opts = { action: :show, controller: 'webui/packages/files', project_name: project,
+                    package_name: package, filename: file['name'], expand: expand }
     - unless is_current_rev
       - link_opts[:rev] = srcmd5
     = link_to_if(viewable_file?(file['name'], file['size'].to_i), nbsp(file['name']), link_opts)

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

@@ -33,7 +33,7 @@
             - revision = calculate_revision_on_state(@rev, @files[filename]['state'])
             .mb-2
               = render partial: 'revision_diff_detail',
-              locals: { file_view_path: package_view_file_path(project: @project, package: @package, rev: revision, filename: filename),
+              locals: { file_view_path: project_package_file_path(project_name: @project, package_name: @package, rev: revision, filename: filename),
                 filename: filename,
                 file: @files[filename],
                 index: index,

src/api/app/views/webui/packages/files/_breadcrumb_items.html.haml

@@ -1,3 +1,7 @@
 = render partial: 'webui/package/breadcrumb_items'
-%li.breadcrumb-item.active{ 'aria-current' => 'page' }
-  Add File
+- if action_name == 'new'
+  %li.breadcrumb-item.active{ 'aria-current' => 'page' }
+    Add File
+- if action_name == 'show'
+  %li.breadcrumb-item.active.text-word-break-all{ 'aria-current' => 'page' }
+    = truncate(@filename, length: 50)

src/api/app/views/webui/packages/files/show.html.haml

@@ -0,0 +1,30 @@
+- @pagetitle = "File #{truncate(@filename, length: 50)} of Package #{truncate(@package.name, length: 50)}"
+- content_for(:content_for_head, javascript_include_tag('webui/cm2/codemirror-file'))
+
+.card.mb-3
+  = render partial: 'webui/package/tabs', locals: { project: @project, package: @package }
+  .card-body
+    %h3.text-word-break-all
+      File #{@filename} of Package #{@package}
+      - if @rev
+        (Revision #{@rev})
+    - if @rev
+      %p
+        Currently displaying revision
+        %i= @rev
+        ,
+        = link_to('Show latest', project: @project, package: @package, filename: @filename, expand: @expand)
+
+    - if @addeditlink
+      - if @filename.ends_with?('.changes')
+        %p
+          %a.changes-link{ href: '#', onclick: 'addChangesEntryTemplate(); return false;',
+                           data: { packagername: User.session!.realname, packageremail: User.session!.email } }
+            Insert changes entry template
+
+      -# TODO: Provide a comments field through a callback
+      = render(partial: 'webui/shared/editor', locals: { text: @file, mode: guess_code_class(@filename),
+        save: { url: project_package_file_path(@project, @package, @filename), method: :put,
+        data: { project: @project.name, package: @package.name, submit: 'file', comment: '', filename: @filename, rev: @rev } } })
+    - else
+      = render(partial: 'webui/shared/editor', locals: { text: @file, mode: guess_code_class(@filename), style: { read_only: true } })

src/api/app/views/webui/packages/files/simple_show.html.haml

@@ -0,0 +1,7 @@
+- @pagetitle = "File #{@filename} of Package #{@package}"
+
+.card.mb-3
+  = render partial: 'webui/package/tabs', locals: { project: @project, package: @package }
+  .card-body
+    %h3= @pagetitle
+    %pre.border.border-dotted.bg-body-secondary.p-2.mb-0= force_utf8_and_transform_nonprintables(@file)

src/api/config/routes/webui_routes.rb

@@ -94,7 +94,8 @@
         post 'package/save_person/:project/:package' => :save_person, constraints: cons, as: 'package_save_person'
         post 'package/save_group/:project/:package' => :save_group, constraints: cons, as: 'package_save_group'
         post 'package/remove_role/:project/:package' => :remove_role, constraints: cons, as: 'package_remove_role'
-        get 'package/view_file/:project/:package/(:filename)' => :view_file, constraints: cons, as: 'package_view_file'
+        # For backward compatibility
+        get 'package/view_file/:project/:package/:filename', to: redirect('/projects/%{project}/packages/%{package}/files/%{filename}'), constraints: cons
         defaults format: 'js' do
           post 'package/trigger_rebuild/:project/:package' => :trigger_rebuild, constraints: cons, as: 'package_trigger_rebuild'
           get 'package/abort_build/:project/:package' => :abort_build, constraints: cons, as: 'package_abort_build'
@@ -281,7 +282,7 @@
         resources :deletions, controller: 'webui/requests/deletions', only: %i[new create], constraints: cons
         resources :devel_project_changes, controller: 'webui/requests/devel_project_changes', only: %i[new create], constraints: cons
         resources :submissions, controller: 'webui/requests/submissions', only: %i[new create], constraints: cons
-        resources :files, controller: 'webui/packages/files', only: %i[new create update destroy], constraints: cons, param: :filename, format: false
+        resources :files, controller: 'webui/packages/files', only: %i[new create show update destroy], constraints: cons, param: :filename, format: false, defaults: { format: 'html' }
         put 'toggle_watched_item', controller: 'webui/watched_items', constraints: cons
         resource :badge, controller: 'webui/packages/badge', only: [:show], constraints: cons.merge(format: :svg)
         resources :repositories, only: [], param: :name do