Merge pull request #14974 from hennevogel/refactoring/graceful-not-found

Handle Project/Package not found more gracefully

src/api/app/controllers/concerns/webui/rescue_handler.rb

@@ -21,11 +21,12 @@ module Webui::RescueHandler
       end
     end
 
-    rescue_from Package::Errors::ScmsyncReadOnly do |exception|
+    rescue_from Project::Errors::UnknownObjectError, Package::Errors::UnknownObjectError, Package::Errors::ReadSourceAccessError, Package::Errors::ScmsyncReadOnly do |exception|
+      message = exception.message || exception.default_message
       if request.xhr?
-        render json: { error: exception.default_message }, status: exception.status
+        head :not_found
       else
-        flash[:error] = exception.default_message
+        flash[:error] = message
         redirect_back(fallback_location: root_path)
       end
     end

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

@@ -45,8 +45,9 @@ def handle_unverified_request
 
   def set_project
     # We've started to use project_name for new routes...
-    @project = ::Project.find_by(name: params[:project_name] || params[:project])
-    raise ActiveRecord::RecordNotFound unless @project
+    project_name = params[:project_name] || params[:project]
+    @project = ::Project.find_by(name: project_name)
+    raise Project::Errors::UnknownObjectError, "Project not found: #{project_name}" unless @project
   end
 
   def require_login

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

@@ -12,17 +12,6 @@
       expect(assigns(:commits)).to eq([commit])
     end
 
-    it 'assigns @project' do
-      get :commits, params: { project: project, format: 'atom' }
-      expect(assigns(:project)).to eq(project)
-    end
-
-    it 'fails if project is not existent' do
-      expect do
-        get :commits, params: { project: 'DoesNotExist', format: 'atom' }
-      end.to raise_error ActiveRecord::RecordNotFound
-    end
-
     it 'renders the rss template' do
       get :commits, params: { project: project, format: 'atom' }
       expect(response).to render_template('webui/feeds/commits')

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

@@ -499,11 +499,11 @@
     end
 
     it 'without a repository param' do
-      expect { post :remove_path_from_target, params: { project: user } }.to raise_error ActiveRecord::RecordNotFound
+      expect { post :remove_path_from_target, params: { project: user.home_project } }.to raise_error ActiveRecord::RecordNotFound
     end
 
     it 'with a repository param but without a path param' do
-      expect { post :remove_path_from_target, params: { repository: repo_for_user_home.id, project: user } }.to raise_error ActiveRecord::RecordNotFound
+      expect { post :remove_path_from_target, params: { repository: repo_for_user_home.id, project: user.home_project } }.to raise_error ActiveRecord::RecordNotFound
     end
 
     context 'with a repository and path' do
@@ -774,14 +774,6 @@
         end
       end
     end
-
-    context 'with non existing project' do
-      before do
-        login admin_user
-      end
-
-      it { expect { post :move_path, params: { project: 'non:existent:project' } }.to raise_error ActiveRecord::RecordNotFound }
-    end
   end
 
   describe 'GET #monitor' do

src/api/spec/controllers/webui/projects/maintenance_incident_requests_controller_spec.rb

@@ -10,10 +10,6 @@
       request.env['HTTP_REFERER'] = root_url # Needed for the redirect_to :back
     end
 
-    it 'without an existent project will raise an exception' do
-      expect { post :create, params: { project_name: 'non:existent:project' } }.to raise_error(ActiveRecord::RecordNotFound)
-    end
-
     context 'without a proper action for the maintenance project' do
       before do
         post :create, params: { project_name: maintenance_project, description: 'Fake description for a request' }

src/api/spec/controllers/webui/projects/meta_controller_spec.rb

@@ -17,12 +17,6 @@
       login user
     end
 
-    context 'with a nonexistent project' do
-      let(:post_save_meta) { post :update, params: { project_name: 'nonexistent_project' }, xhr: true }
-
-      it { expect { post_save_meta }.to raise_error(ActiveRecord::RecordNotFound) }
-    end
-
     context 'with a valid project' do
       context 'without a valid meta' do
         before do

src/api/spec/controllers/webui/projects/project_configuration_controller_spec.rb

@@ -69,13 +69,5 @@
       it { expect(response.status).to eq(302) }
       it { expect(response).to redirect_to(root_path) }
     end
-
-    context 'with a non existing project' do
-      let(:post_update) { post :update, params: { project_name: 'non:existing:project', config: 'save config' } }
-
-      it 'raise a RecordNotFound Exception' do
-        expect { post_update }.to raise_error ActiveRecord::RecordNotFound
-      end
-    end
   end
 end

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

@@ -101,10 +101,11 @@ def create
 
   describe 'set_project before filter' do
     context 'with invalid project parameter' do
-      it 'raises an ActiveRecord::RecordNotFound exception' do
-        expect do
-          get :edit, params: { id: 1, project: 'invalid' }
-        end.to raise_error(ActiveRecord::RecordNotFound)
+      it 'redirects back' do
+        from projects_path
+        get :edit, params: { id: 1, project: 'invalid' }
+        expect(flash[:error]).to eq('Project not found: invalid')
+        expect(response).to redirect_to projects_url
       end
     end
 
@@ -122,10 +123,11 @@ def create
     let(:project) { create(:project) }
 
     context 'with invalid package parameter' do
-      it 'raises an Package::Errors::UnknownObjectError exception' do
-        expect do
-          get :create, params: { project: project, package: 'invalid' }
-        end.to raise_error(Package::Errors::UnknownObjectError)
+      it 'redirects back' do
+        from project_show_path(project: project)
+        get :create, params: { project: project, package: 'invalid' }
+        expect(flash[:error]).to eq("Package not found: #{project.name}/invalid")
+        expect(response).to redirect_to project_show_url(project: project)
       end
     end