Commit 785ee558 authored by James Edwards-Jones's avatar James Edwards-Jones

GithubService configured by url instead of owner and repo name

API URL also determined from GitHub project URL
parent 40f62bae
...@@ -41,14 +41,12 @@ module ServiceParams ...@@ -41,14 +41,12 @@ module ServiceParams
:notify, :notify,
:notify_only_broken_pipelines, :notify_only_broken_pipelines,
:notify_only_default_branch, :notify_only_default_branch,
:owner,
:password, :password,
:priority, :priority,
:project_key, :project_key,
:project_url, :project_url,
:recipients, :recipients,
:restrict_to_branch, :restrict_to_branch,
:repository_name,
:room, :room,
:send_from_committer_email, :send_from_committer_email,
:server, :server,
......
...@@ -4,7 +4,8 @@ module EE ...@@ -4,7 +4,8 @@ module EE
:jenkins_url, :jenkins_url,
:multiproject_enabled, :multiproject_enabled,
:pass_unstable, :pass_unstable,
:project_name :project_name,
:repository_url
].freeze ].freeze
def allowed_service_params def allowed_service_params
......
...@@ -2,12 +2,10 @@ class GithubService < Service ...@@ -2,12 +2,10 @@ class GithubService < Service
include Gitlab::Routing include Gitlab::Routing
include ActionView::Helpers::UrlHelper include ActionView::Helpers::UrlHelper
prop_accessor :token, :api_url, :owner, :repository_name prop_accessor :token, :repository_url
validates :token, presence: true, if: :activated? validates :token, presence: true, if: :activated?
validates :api_url, url: true, allow_blank: true validates :repository_url, url: true, allow_blank: true
validates :owner, presence: true, if: :activated?
validates :repository_name, presence: true, if: :activated?
default_value_for :pipeline_events, true default_value_for :pipeline_events, true
...@@ -32,9 +30,7 @@ class GithubService < Service ...@@ -32,9 +30,7 @@ class GithubService < Service
def fields def fields
[ [
{ type: 'text', name: "token", required: true, placeholder: "e.g. 8d3f016698e...", help: 'Create a <a href="https://github.com/settings/tokens">personal access token</a> with <code>repo:status</code> access granted and paste it here.'.html_safe }, { type: 'text', name: "token", required: true, placeholder: "e.g. 8d3f016698e...", help: 'Create a <a href="https://github.com/settings/tokens">personal access token</a> with <code>repo:status</code> access granted and paste it here.'.html_safe },
{ type: 'text', name: "owner", required: true, help: 'The username or organization where the GitHub repository belongs. This can be found in the repository URL: https://github.com/<strong>owner</strong>/repository'.html_safe }, { type: 'text', name: "repository_url", title: 'Repository URL', required: true, placeholder: 'e.g. https://github.com/owner/repository' }
{ type: 'text', name: "repository_name", required: true, help: 'This can be found in the repository URL: https://github.com/owner/<strong>repository</strong>'.html_safe },
{ type: 'text', name: "api_url", placeholder: "https://api.github.com", help: 'Leave blank when using GitHub.com or use <code>https://YOUR-HOSTNAME/api/v3/</code> for GitHub Enterprise'.html_safe }
] ]
end end
...@@ -80,8 +76,32 @@ class GithubService < Service ...@@ -80,8 +76,32 @@ class GithubService < Service
{ success: true, result: result } { success: true, result: result }
end end
def api_url
if github_host == 'github.com'
'https://api.github.com'
else
"#{repository_uri.scheme}://#{github_host}/api/v3"
end
end
def owner
repository_uri.path.split('/')[1]
end
def repository_name
repository_uri.path.split('/')[2]
end
private private
def github_host
repository_uri.host
end
def repository_uri
URI.parse(repository_url)
end
def disabled? def disabled?
project.disabled_services.include?(to_param) project.disabled_services.include?(to_param)
end end
......
...@@ -36,8 +36,7 @@ describe 'User activates GitHub Service' do ...@@ -36,8 +36,7 @@ describe 'User activates GitHub Service' do
def fill_in_details def fill_in_details
check('Active') check('Active')
fill_in "Token", with: "aaaaaaaaaa" fill_in "Token", with: "aaaaaaaaaa"
fill_in "Owner", with: "h5bp" fill_in "Repository URL", with: 'https://github.com/h5bp/html5-boilerplate'
fill_in "Repository name", with: "html5-boilerplate"
end end
it 'activates service' do it 'activates service' do
...@@ -46,13 +45,6 @@ describe 'User activates GitHub Service' do ...@@ -46,13 +45,6 @@ describe 'User activates GitHub Service' do
expect(page).to have_content('GitHub activated.') expect(page).to have_content('GitHub activated.')
end end
it 'allows API URL to be set' do
fill_in "Api url", with: "https://api.github.com"
click_button('Save')
expect(page).to have_content('GitHub activated.')
end
context 'with pipelines', :js do context 'with pipelines', :js do
let(:pipeline) { create(:ci_pipeline) } let(:pipeline) { create(:ci_pipeline) }
let(:project) { create(:project, pipelines: [pipeline])} let(:project) { create(:project, pipelines: [pipeline])}
......
...@@ -4,19 +4,18 @@ describe GithubService do ...@@ -4,19 +4,18 @@ describe GithubService do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: project) } let(:pipeline) { create(:ci_pipeline, project: project) }
let(:pipeline_sample_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } let(:pipeline_sample_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) }
let(:api_url) { '' }
let(:owner) { 'my-user' } let(:owner) { 'my-user' }
let(:token) { 'aaaaaaaaa' } let(:token) { 'aaaaaaaaa' }
let(:repository_name) { 'my-project' } let(:repository_name) { 'my-project' }
let(:base_url) { 'https://github.com' }
let(:repository_url) { "#{base_url}/#{owner}/#{repository_name}" }
let(:service_params) do let(:service_params) do
{ {
active: true, active: true,
project: project, project: project,
properties: { properties: {
token: token, token: token,
api_url: api_url, repository_url: repository_url
owner: owner,
repository_name: repository_name
} }
} }
end end
...@@ -139,6 +138,10 @@ describe GithubService do ...@@ -139,6 +138,10 @@ describe GithubService do
context 'with custom api endpoint' do context 'with custom api endpoint' do
let(:api_url) { 'https://my.code.repo' } let(:api_url) { 'https://my.code.repo' }
before do
allow(subject).to receive(:api_url).and_return(api_url)
end
it 'hands custom api url to StatusNotifier' do it 'hands custom api url to StatusNotifier' do
allow(notifier).to receive(:notify) allow(notifier).to receive(:notify)
expect(GithubService::StatusNotifier).to receive(:new).with(anything, anything, api_endpoint: api_url) expect(GithubService::StatusNotifier).to receive(:new).with(anything, anything, api_endpoint: api_url)
......
...@@ -680,23 +680,11 @@ module API ...@@ -680,23 +680,11 @@ module API
type: String, type: String,
desc: 'GitHub API token with repo:status OAuth scope' desc: 'GitHub API token with repo:status OAuth scope'
}, },
{
required: false,
name: :api_url,
type: String,
desc: 'GitHub instance API URL, defaults to https://api.github.com'
},
{
required: true,
name: :owner,
type: String,
desc: 'Owner or organization of the GitHub repo'
},
{ {
required: true, required: true,
name: :repository_name, name: :repository_url,
type: String, type: String,
desc: "GitHub repository name" desc: "GitHub repository URL"
} }
], ],
'jenkins' => [ 'jenkins' => [
......
...@@ -568,23 +568,11 @@ module API ...@@ -568,23 +568,11 @@ module API
type: String, type: String,
desc: 'GitHub API token with repo:status OAuth scope' desc: 'GitHub API token with repo:status OAuth scope'
}, },
{
required: false,
name: :api_url,
type: String,
desc: 'GitHub instance API URL, defaults to https://api.github.com'
},
{
required: true,
name: :owner,
type: String,
desc: 'Owner or organization of the GitHub repo'
},
{ {
required: true, required: true,
name: :repository_name, name: :repository_name,
type: String, type: String,
desc: "GitHub repository name" desc: "GitHub repository URL"
} }
], ],
'jenkins' => [ 'jenkins' => [
......
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