diff options
author | Robert Speicher <robert@gitlab.com> | 2016-01-26 03:48:56 +0300 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2016-01-26 04:05:49 +0300 |
commit | 80814cbd04169f82aff9736a3c243eea85328cef (patch) | |
tree | 23503ff14429697e03b09704e03cbc38cb417d95 | |
parent | 6881ba0e796fe22dd1d47bd1a7d42e532f18077a (diff) |
Merge branch 'fix-import-redirect-loop' into 'master'
Fix import redirect loop
Fixes #11864
See merge request !2606
-rw-r--r-- | app/controllers/projects/imports_controller.rb | 20 | ||||
-rw-r--r-- | spec/controllers/projects/imports_controller_spec.rb | 109 |
2 files changed, 124 insertions, 5 deletions
diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb index 8d8035ef5ff..07f355c35b1 100644 --- a/app/controllers/projects/imports_controller.rb +++ b/app/controllers/projects/imports_controller.rb @@ -1,8 +1,8 @@ class Projects::ImportsController < Projects::ApplicationController # Authorize before_action :authorize_admin_project! - before_action :require_no_repo, except: :show - before_action :redirect_if_progress, except: :show + before_action :require_no_repo, only: [:new, :create] + before_action :redirect_if_progress, only: [:new, :create] def new end @@ -24,11 +24,11 @@ class Projects::ImportsController < Projects::ApplicationController end def show - if @project.repository_exists? || @project.import_finished? + if @project.import_finished? if continue_params redirect_to continue_params[:to], notice: continue_params[:notice] else - redirect_to project_path(@project), notice: "The project was successfully forked." + redirect_to namespace_project_path(@project.namespace, @project), notice: finished_notice end elsif @project.import_failed? redirect_to new_namespace_project_import_path(@project.namespace, @project) @@ -36,6 +36,7 @@ class Projects::ImportsController < Projects::ApplicationController if continue_params && continue_params[:notice_now] flash.now[:notice] = continue_params[:notice_now] end + # Render end end @@ -44,6 +45,7 @@ class Projects::ImportsController < Projects::ApplicationController def continue_params continue_params = params[:continue] + if continue_params continue_params.permit(:to, :notice, :notice_now) else @@ -51,8 +53,16 @@ class Projects::ImportsController < Projects::ApplicationController end end + def finished_notice + if @project.forked? + 'The project was successfully forked.' + else + 'The project was successfully imported.' + end + end + def require_no_repo - if @project.repository_exists? && !@project.import_in_progress? + if @project.repository_exists? redirect_to(namespace_project_path(@project.namespace, @project)) end end diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb new file mode 100644 index 00000000000..85d1d1e0524 --- /dev/null +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -0,0 +1,109 @@ +require 'spec_helper' + +describe Projects::ImportsController do + let(:user) { create(:user) } + + describe 'GET #show' do + context 'when repository does not exists' do + let(:project) { create(:empty_project) } + + before do + sign_in(user) + project.team << [user, :master] + end + + it 'renders template' do + get :show, namespace_id: project.namespace.to_param, project_id: project.to_param + + expect(response).to render_template :show + end + + it 'sets flash.now if params is present' do + get :show, namespace_id: project.namespace.to_param, project_id: project.to_param, continue: { notice_now: 'Started' } + + expect(flash.now[:notice]).to eq 'Started' + end + end + + context 'when repository exists' do + let(:project) { create(:project_empty_repo, import_url: 'https://github.com/vim/vim.git') } + + before do + sign_in(user) + project.team << [user, :master] + end + + context 'when import is in progress' do + before do + project.update_attribute(:import_status, :started) + end + + it 'renders template' do + get :show, namespace_id: project.namespace.to_param, project_id: project.to_param + + expect(response).to render_template :show + end + + it 'sets flash.now if params is present' do + get :show, namespace_id: project.namespace.to_param, project_id: project.to_param, continue: { notice_now: 'In progress' } + + expect(flash.now[:notice]).to eq 'In progress' + end + end + + context 'when import failed' do + before do + project.update_attribute(:import_status, :failed) + end + + it 'redirects to new_namespace_project_import_path' do + get :show, namespace_id: project.namespace.to_param, project_id: project.to_param + + expect(response).to redirect_to new_namespace_project_import_path(project.namespace, project) + end + end + + context 'when import finished' do + before do + project.update_attribute(:import_status, :finished) + end + + context 'when project is a fork' do + it 'redirects to namespace_project_path' do + allow_any_instance_of(Project).to receive(:forked?).and_return(true) + + get :show, namespace_id: project.namespace.to_param, project_id: project.to_param + + expect(flash[:notice]).to eq 'The project was successfully forked.' + expect(response).to redirect_to namespace_project_path(project.namespace, project) + end + end + + context 'when project is external' do + it 'redirects to namespace_project_path' do + get :show, namespace_id: project.namespace.to_param, project_id: project.to_param + + expect(flash[:notice]).to eq 'The project was successfully imported.' + expect(response).to redirect_to namespace_project_path(project.namespace, project) + end + end + + context 'when continue params is present' do + let(:params) do + { + to: namespace_project_path(project.namespace, project), + notice: 'Finished' + } + end + + it 'redirects to params[:to]' do + get :show, namespace_id: project.namespace.to_param, project_id: project.to_param, continue: params + + expect(flash[:notice]).to eq params[:notice] + expect(response).to redirect_to params[:to] + end + end + end + end + end +end |