Adopt specs to require_package in all controllers

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

@@ -9,6 +9,11 @@ class Webui::PatchinfoController < Webui::WebuiController
   before_action :require_login, except: [:show]
   before_action :set_patchinfo, only: [:show, :edit]
 
+  rescue_from Package::UnknownObjectError do
+    flash[:error] = "Patchinfo '#{elide(params[:package])}' not found in project '#{elide(params[:project])}'"
+    redirect_to project_show_path(project: @project)
+  end
+
   def show
     @pkg_names = @project.packages.pluck(:name)
     @pkg_names.delete('patchinfo')
@@ -147,7 +152,7 @@ def require_exists
     return if @package && @package.patchinfo
 
     # FIXME: should work for remote packages
-    flash[:error] = "Patchinfo not found for #{elide(params[:project])}"
+    flash[:error] = "Patchinfo not found for #{@project.name}"
     redirect_to(controller: 'package', action: 'show', project: @project, package: @package)
   end
 

src/api/spec/controllers/webui/attribute_controller_spec.rb

@@ -3,20 +3,6 @@
 RSpec.describe Webui::AttributeController do
   let!(:user) { create(:confirmed_user, :with_home) }
 
-  describe 'GET index' do
-    it 'shows an error message when package does not exist' do
-      get :index, params: { project: user.home_project, package: 'Pok' }
-      expect(response).to redirect_to(project_show_path(user.home_project))
-      expect(flash[:error]).to eq('Package Pok not found')
-    end
-
-    it 'shows an error message when project does not exist' do
-      get :index, params: { project: 'Does:Not:Exist' }
-      expect(response).to redirect_to(projects_path)
-      expect(flash[:error]).to eq('Project not found: Does:Not:Exist')
-    end
-  end
-
   describe 'GET #new' do
     before do
       login user

src/api/spec/controllers/webui/package_controller_spec.rb

@@ -149,12 +149,6 @@
   end
 
   describe 'GET #show' do
-    context 'require_package before_action' do
-      context 'with an invalid package' do
-        it { expect { get :show, params: { project: user.home_project, package: 'no_package' } }.to raise_error(ActiveRecord::RecordNotFound) }
-      end
-    end
-
     context 'with a valid package' do
       before do
         get :show, params: { project: user.home_project, package: source_package.name }
@@ -175,16 +169,6 @@
       it { expect(assigns(:more_info)).to include('service daemon error:') }
     end
 
-    context 'with a package without sourceaccess' do
-      before do
-        create(:sourceaccess_flag, project: user.home_project)
-        get :show, params: { project: user.home_project, package: source_package.name }
-      end
-
-      it { expect(flash[:error]).to eq("You don't have access to the sources of this package: \"#{source_package}\"") }
-      it { expect(response).to redirect_to(project_show_path(user.home_project)) }
-    end
-
     context 'revision handling' do
       let(:package_with_revisions) do
         create(:package_with_revisions, name: 'rev_package', revision_count: 3, project: user.home_project)
@@ -274,17 +258,6 @@ def remove_file_post
       login(user)
     end
 
-    context 'without source access' do
-      before do
-        package.add_flag('sourceaccess', 'disable')
-        package.save
-        get :revisions, params: { project: project, package: package }
-      end
-
-      it { expect(flash[:error]).to eq("You don't have access to the sources of this package: \"#{elided_package_name}\"") }
-      it { expect(response).to redirect_to(project_show_path(project: project.name)) }
-    end
-
     context 'with source access' do
       before do
         get :revisions, params: { project: project, package: package }
@@ -295,14 +268,6 @@ def remove_file_post
         package.destroy
       end
 
-      it 'sets the project' do
-        expect(assigns(:project)).to eq(project)
-      end
-
-      it 'sets the package' do
-        expect(assigns(:package)).to eq(package)
-      end
-
       context 'when not passing the rev parameter' do
         let(:package_with_revisions) { create(:package_with_revisions, name: "package_with_#{revision_count}_revisions", revision_count: revision_count, project: project) }
         let(:revision_count) { 25 }
@@ -497,12 +462,6 @@ def remove_file_post
       it { expect(response).to have_http_status(:bad_request) }
     end
 
-    context 'with an unexistent package' do
-      let(:post_save_meta) { post :save_meta, params: { project: source_project, package: 'blah', meta: valid_meta } }
-
-      it { expect { post_save_meta }.to raise_error(ActiveRecord::RecordNotFound) }
-    end
-
     context 'when connection with the backend fails' do
       before do
         allow_any_instance_of(Package).to receive(:update_from_xml).and_raise(Backend::Error, 'fake message')

src/api/spec/controllers/webui/packages/build_reason_controller_spec.rb

@@ -1,62 +1,53 @@
 require 'rails_helper'
 require 'webmock/rspec'
-# WARNING: If you change owner tests make sure you uncomment this line
-# and start a test backend. Some of the Owner methods
-# require real backend answers for projects/packages.
-# CONFIG['global_write_through'] = true
 
-RSpec.describe Webui::Packages::BuildReasonController, :vcr do
+RSpec.describe Webui::Packages::BuildReasonController do
   describe 'GET #index' do
     let(:user) { create(:confirmed_user, :with_home, login: 'tom') }
-    let(:source_project) { user.home_project }
-    let(:package) { create(:package, name: 'package', project: source_project) }
-    let(:repo_for_source_project) do
-      repo = create(:repository, project: source_project, architectures: ['i586'])
-      source_project.store
-      repo
-    end
-
+    let(:project) { user.home_project }
+    let(:package) { create(:package, name: 'package', project: project) }
+    let(:repository) { create(:repository, project: project, architectures: ['i586']) }
     let(:valid_request_params) do
       {
         package_name: package.name,
-        project: source_project.name,
-        repository: repo_for_source_project.name,
-        arch: repo_for_source_project.architectures.first.name
+        project: project.name,
+        repository: repository.name,
+        arch: repository.architectures.first.name
       }
     end
 
     context 'without a valid respository' do
       before do
-        get :index, params: { package_name: package, project: source_project, repository: 'fake_repo', arch: 'i586' }
+        get :index, params: { package_name: package, project: project, repository: 'fake_repo', arch: 'i586' }
       end
 
       it { expect(flash[:error]).not_to be_empty }
 
       it {
-        expect(response).to redirect_to(project_package_repository_binaries_path(package_name: package, project_name: source_project,
+        expect(response).to redirect_to(project_package_repository_binaries_path(package_name: package, project_name: project,
                                                                                  repository_name: 'fake_repo'))
       }
     end
 
     context 'without a valid architecture' do
       before do
         login(user)
-        get :index, params: { package_name: package, project: source_project, repository: repo_for_source_project.name, arch: 'i58' }
+        get :index, params: { package_name: package, project: project, repository: repository.name, arch: 'i58' }
       end
 
       it { expect(flash[:error]).not_to be_empty }
 
       it 'redirects to package_binaries_path' do
         expect(response).to redirect_to(project_package_repository_binaries_path(package_name: package,
-                                                                                 project_name: source_project,
-                                                                                 repository_name: repo_for_source_project.name))
+                                                                                 project_name: project,
+                                                                                 repository_name: repository.name))
       end
     end
 
     context 'for packages without a build reason' do
       before do
-        path = "#{CONFIG['source_url']}/build/#{source_project.name}/#{repo_for_source_project.name}/" \
-               "#{repo_for_source_project.architectures.first.name}/#{package.name}/_reason"
+        path = "#{CONFIG['source_url']}/build/#{project.name}/#{repository.name}/" \
+               "#{repository.architectures.first.name}/#{package.name}/_reason"
         stub_request(:get, path).and_return(body:
         %(<reason>\n <explain/>  <time/>  <oldsource/>  </reason>))
 
@@ -67,15 +58,15 @@
 
       it 'redirects to package_binaries_path' do
         expect(response).to redirect_to(project_package_repository_binaries_path(package_name: package,
-                                                                                 project_name: source_project,
-                                                                                 repository_name: repo_for_source_project.name))
+                                                                                 project_name: project,
+                                                                                 repository_name: repository.name))
       end
     end
 
     context 'for valid requests' do
       before do
-        path = "#{CONFIG['source_url']}/build/#{source_project.name}/#{repo_for_source_project.name}/" \
-               "#{repo_for_source_project.architectures.first.name}/#{package.name}/_reason"
+        path = "#{CONFIG['source_url']}/build/#{project.name}/#{repository.name}/" \
+               "#{repository.architectures.first.name}/#{package.name}/_reason"
         stub_request(:get, path).and_return(body:
         %(<reason>\n  <explain>source change</explain>  <time>1496387771</time>  <oldsource>1de56fdc419ea4282e35bd388285d370</oldsource></reason>))