Commit 70a459eb authored by Luke Duncalfe's avatar Luke Duncalfe Committed by Heinrich Lee Yu

Fix services API returning nonexistent services

parent 09f5face
---
title: Fix services API returning non-existing services
merge_request: 61646
author:
type: fixed
...@@ -125,8 +125,11 @@ module API ...@@ -125,8 +125,11 @@ module API
requires :service_slug, type: String, values: SERVICES.keys, desc: 'The name of the service' requires :service_slug, type: String, values: SERVICES.keys, desc: 'The name of the service'
end end
get ":id/services/:service_slug" do get ":id/services/:service_slug" do
service = user_project.find_or_initialize_service(params[:service_slug].underscore) integration = user_project.find_or_initialize_service(params[:service_slug].underscore)
present service, with: Entities::ProjectService
not_found!('Service') unless integration&.persisted?
present integration, with: Entities::ProjectService
end end
end end
......
...@@ -114,21 +114,61 @@ RSpec.describe API::Services do ...@@ -114,21 +114,61 @@ RSpec.describe API::Services do
describe "GET /projects/:id/services/#{service.dasherize}" do describe "GET /projects/:id/services/#{service.dasherize}" do
include_context service include_context service
# inject some properties into the service let!(:initialized_service) { initialize_service(service, active: true) }
let!(:initialized_service) { initialize_service(service) }
let_it_be(:project2) do
create(:project, creator_id: user.id, namespace: user.namespace)
end
def deactive_service!
return initialized_service.update!(active: false) unless initialized_service.is_a?(PrometheusService)
# PrometheusService sets `#active` itself within a `before_save`:
initialized_service.manual_configuration = false
initialized_service.save!
end
it 'returns authentication error when unauthenticated' do it 'returns authentication error when unauthenticated' do
get api("/projects/#{project.id}/services/#{dashed_service}") get api("/projects/#{project.id}/services/#{dashed_service}")
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:unauthorized)
end end
it "returns all properties of service #{service}" do it "returns all properties of active service #{service}" do
get api("/projects/#{project.id}/services/#{dashed_service}", user) get api("/projects/#{project.id}/services/#{dashed_service}", user)
expect(initialized_service).to be_active
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response['properties'].keys).to match_array(service_instance.api_field_names) expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
end end
it "returns all properties of inactive service #{service}" do
deactive_service!
get api("/projects/#{project.id}/services/#{dashed_service}", user)
expect(initialized_service).not_to be_active
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
end
it "returns not found if service does not exist" do
get api("/projects/#{project2.id}/services/#{dashed_service}", user)
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response['message']).to eq('404 Service Not Found')
end
it "returns not found if service exists but is in `Project#disabled_services`" do
expect_next_found_instance_of(Project) do |project|
expect(project).to receive(:disabled_services).at_least(:once).and_return([service])
end
get api("/projects/#{project.id}/services/#{dashed_service}", user)
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response['message']).to eq('404 Service Not Found')
end
it "returns error when authenticated but not a project owner" do it "returns error when authenticated but not a project owner" do
project.add_developer(user2) project.add_developer(user2)
get api("/projects/#{project.id}/services/#{dashed_service}", user2) get api("/projects/#{project.id}/services/#{dashed_service}", user2)
......
...@@ -49,8 +49,9 @@ Integration.available_services_names.each do |service| ...@@ -49,8 +49,9 @@ Integration.available_services_names.each do |service|
stub_jira_service_test if service == 'jira' stub_jira_service_test if service == 'jira'
end end
def initialize_service(service) def initialize_service(service, attrs = {})
service_item = project.find_or_initialize_service(service) service_item = project.find_or_initialize_service(service)
service_item.attributes = attrs
service_item.properties = service_attrs service_item.properties = service_attrs
service_item.save! service_item.save!
service_item service_item
......
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