Commit 80814cbd authored by Robert Speicher's avatar Robert Speicher Committed by Robert Speicher

Merge branch 'fix-import-redirect-loop' into 'master'

Fix import redirect loop

Fixes #11864 

See merge request !2606
parent 6881ba0e
class Projects::ImportsController < Projects::ApplicationController class Projects::ImportsController < Projects::ApplicationController
# Authorize # Authorize
before_action :authorize_admin_project! before_action :authorize_admin_project!
before_action :require_no_repo, except: :show before_action :require_no_repo, only: [:new, :create]
before_action :redirect_if_progress, except: :show before_action :redirect_if_progress, only: [:new, :create]
def new def new
end end
...@@ -24,11 +24,11 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -24,11 +24,11 @@ class Projects::ImportsController < Projects::ApplicationController
end end
def show def show
if @project.repository_exists? || @project.import_finished? if @project.import_finished?
if continue_params if continue_params
redirect_to continue_params[:to], notice: continue_params[:notice] redirect_to continue_params[:to], notice: continue_params[:notice]
else else
redirect_to project_path(@project), notice: "The project was successfully forked." redirect_to namespace_project_path(@project.namespace, @project), notice: finished_notice
end end
elsif @project.import_failed? elsif @project.import_failed?
redirect_to new_namespace_project_import_path(@project.namespace, @project) redirect_to new_namespace_project_import_path(@project.namespace, @project)
...@@ -36,6 +36,7 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -36,6 +36,7 @@ class Projects::ImportsController < Projects::ApplicationController
if continue_params && continue_params[:notice_now] if continue_params && continue_params[:notice_now]
flash.now[:notice] = continue_params[:notice_now] flash.now[:notice] = continue_params[:notice_now]
end end
# Render # Render
end end
end end
...@@ -44,6 +45,7 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -44,6 +45,7 @@ class Projects::ImportsController < Projects::ApplicationController
def continue_params def continue_params
continue_params = params[:continue] continue_params = params[:continue]
if continue_params if continue_params
continue_params.permit(:to, :notice, :notice_now) continue_params.permit(:to, :notice, :notice_now)
else else
...@@ -51,8 +53,16 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -51,8 +53,16 @@ class Projects::ImportsController < Projects::ApplicationController
end end
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 def require_no_repo
if @project.repository_exists? && !@project.import_in_progress? if @project.repository_exists?
redirect_to(namespace_project_path(@project.namespace, @project)) redirect_to(namespace_project_path(@project.namespace, @project))
end end
end end
......
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment