Commit 1a4d1b05 authored by Stan Hu's avatar Stan Hu

Fix Fogbugz Importer not working

This stopped working in GitLab 11.11 when we upgraded to Rails 5.1.
Rails 5 changed ActionController::Parameters to return an Object instead
of a Hash. The old behavior was deprecated in Rails 5 but finally
removed in Rails 5.1

Since the controller wasn't updated properly, the callback endpoint
quietly failed with the message, "Could not connect to FogBugz, check
your url".

To fix this, we need to call `to_h` on the `import_params` to access the
Hash. We also need to do this for the user map and permit specific keys.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/33530
parent 3017d2a5
...@@ -11,7 +11,7 @@ class Import::FogbugzController < Import::BaseController ...@@ -11,7 +11,7 @@ class Import::FogbugzController < Import::BaseController
def callback def callback
begin begin
res = Gitlab::FogbugzImport::Client.new(import_params.symbolize_keys) res = Gitlab::FogbugzImport::Client.new(import_params.to_h.symbolize_keys)
rescue rescue
# If the URI is invalid various errors can occur # If the URI is invalid various errors can occur
return redirect_to new_import_fogbugz_path, alert: _('Could not connect to FogBugz, check your URL') return redirect_to new_import_fogbugz_path, alert: _('Could not connect to FogBugz, check your URL')
...@@ -26,7 +26,7 @@ class Import::FogbugzController < Import::BaseController ...@@ -26,7 +26,7 @@ class Import::FogbugzController < Import::BaseController
end end
def create_user_map def create_user_map
user_map = params[:users] user_map = user_map_params.to_h[:users]
unless user_map.is_a?(Hash) && user_map.all? { |k, v| !v[:name].blank? } unless user_map.is_a?(Hash) && user_map.all? { |k, v| !v[:name].blank? }
flash.now[:alert] = _('All users must have a name.') flash.now[:alert] = _('All users must have a name.')
...@@ -99,6 +99,10 @@ class Import::FogbugzController < Import::BaseController ...@@ -99,6 +99,10 @@ class Import::FogbugzController < Import::BaseController
params.permit(:uri, :email, :password) params.permit(:uri, :email, :password)
end end
def user_map_params
params.permit(users: %w(name email gitlab_user))
end
def verify_fogbugz_import_enabled def verify_fogbugz_import_enabled
render_404 unless fogbugz_import_enabled? render_404 unless fogbugz_import_enabled?
end end
......
---
title: Fix Fogbugz Importer not working
merge_request: 29383
author:
type: fixed
...@@ -11,6 +11,44 @@ describe Import::FogbugzController do ...@@ -11,6 +11,44 @@ describe Import::FogbugzController do
sign_in(user) sign_in(user)
end end
describe 'POST #callback' do
let(:token) { FFaker::Lorem.characters(8) }
let(:uri) { 'https://example.com' }
let(:xml_response) { %Q(<?xml version=\"1.0\" encoding=\"UTF-8\"?><response><token><![CDATA[#{token}]]></token></response>) }
it 'attempts to contact Fogbugz server' do
stub_request(:post, "https://example.com/api.asp").to_return(status: 200, body: xml_response, headers: {})
post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword' }
expect(session[:fogbugz_token]).to eq(token)
expect(session[:fogbugz_uri]).to eq(uri)
expect(response).to redirect_to(new_user_map_import_fogbugz_path)
end
end
describe 'POST #create_user_map' do
let(:user_map) do
{
"2" => {
"name" => "Test User",
"email" => "testuser@example.com",
"gitlab_user" => "3"
}
}
end
it 'stores the user map in the session' do
client = double(user_map: {})
expect(controller).to receive(:client).and_return(client)
post :create_user_map, params: { users: user_map }
expect(session[:fogbugz_user_map]).to eq(user_map)
expect(response).to redirect_to(status_import_fogbugz_path)
end
end
describe 'GET status' do describe 'GET status' do
before do before do
@repo = OpenStruct.new(name: 'vim') @repo = OpenStruct.new(name: 'vim')
......
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