diff --git a/changelogs/unreleased/330818-services-api-return-404-for-unsaved-services.yml b/changelogs/unreleased/330818-services-api-return-404-for-unsaved-services.yml new file mode 100644 index 0000000000000000000000000000000000000000..8df02746b373c58066b95f1bd571c477e3161d28 --- /dev/null +++ b/changelogs/unreleased/330818-services-api-return-404-for-unsaved-services.yml @@ -0,0 +1,5 @@ +--- +title: Fix services API returning non-existing services +merge_request: 61646 +author: +type: fixed diff --git a/lib/api/services.rb b/lib/api/services.rb index fb45337cf055748c074335d776f6fc3cebcbb89e..8a7abe721ddcbe0ae33f2fedecbcc83bf0fe9797 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -125,8 +125,11 @@ def service_attributes(service) requires :service_slug, type: String, values: SERVICES.keys, desc: 'The name of the service' end get ":id/services/:service_slug" do - service = user_project.find_or_initialize_service(params[:service_slug].underscore) - present service, with: Entities::ProjectService + integration = user_project.find_or_initialize_service(params[:service_slug].underscore) + + not_found!('Service') unless integration&.persisted? + + present integration, with: Entities::ProjectService end end diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index e52e1bf4695df5abb820e09cc97583acfcec18dd..1f859622760f9df8f9305ba4271b3088b86ec10c 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -114,21 +114,61 @@ describe "GET /projects/:id/services/#{service.dasherize}" do include_context service - # inject some properties into the service - let!(:initialized_service) { initialize_service(service) } + let!(:initialized_service) { initialize_service(service, active: true) } + + 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 get api("/projects/#{project.id}/services/#{dashed_service}") expect(response).to have_gitlab_http_status(:unauthorized) 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) + expect(initialized_service).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 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 project.add_developer(user2) get api("/projects/#{project.id}/services/#{dashed_service}", user2) diff --git a/spec/support/shared_contexts/services_shared_context.rb b/spec/support/shared_contexts/services_shared_context.rb index 576261030a7e57c2b66d87a5eaac847685bb1946..34c92367efa0d14cb173983a04490c21949f1893 100644 --- a/spec/support/shared_contexts/services_shared_context.rb +++ b/spec/support/shared_contexts/services_shared_context.rb @@ -49,8 +49,9 @@ stub_jira_service_test if service == 'jira' end - def initialize_service(service) + def initialize_service(service, attrs = {}) service_item = project.find_or_initialize_service(service) + service_item.attributes = attrs service_item.properties = service_attrs service_item.save! service_item