Add latest changes from gitlab-org/gitlab@master
This commit is contained in:
parent
8957ace315
commit
8c55809579
|
@ -33,7 +33,7 @@ class Service < ApplicationRecord
|
|||
has_one :service_hook
|
||||
|
||||
validates :project_id, presence: true, unless: -> { template? || instance? }
|
||||
validates :project_id, absence: true, if: -> { instance? }
|
||||
validates :project_id, absence: true, if: -> { template? || instance? }
|
||||
validates :type, presence: true
|
||||
validates :template, uniqueness: { scope: :type }, if: -> { template? }
|
||||
validates :instance, uniqueness: { scope: :type }, if: -> { instance? }
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Validate absence of project_id if service is a template
|
||||
merge_request: 26563
|
||||
author:
|
||||
type: other
|
|
@ -0,0 +1,19 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class DeleteTemplateProjectServices < ActiveRecord::Migration[6.0]
|
||||
DOWNTIME = false
|
||||
|
||||
def up
|
||||
# In 12.9 an ActiveRecord validation for services not being a template and
|
||||
# attached to a project at the same time is introduced. This migration cleans up invalid data.
|
||||
execute <<~SQL
|
||||
DELETE
|
||||
FROM services
|
||||
WHERE TEMPLATE = TRUE AND project_id IS NOT NULL
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
# This migration cannot be reversed.
|
||||
end
|
||||
end
|
|
@ -30,9 +30,9 @@ describe Admin::ServicesController do
|
|||
|
||||
describe "#update" do
|
||||
let(:project) { create(:project) }
|
||||
let!(:service) do
|
||||
let!(:service_template) do
|
||||
RedmineService.create(
|
||||
project: project,
|
||||
project: nil,
|
||||
active: false,
|
||||
template: true,
|
||||
properties: {
|
||||
|
@ -44,9 +44,9 @@ describe Admin::ServicesController do
|
|||
end
|
||||
|
||||
it 'calls the propagation worker when service is active' do
|
||||
expect(PropagateServiceTemplateWorker).to receive(:perform_async).with(service.id)
|
||||
expect(PropagateServiceTemplateWorker).to receive(:perform_async).with(service_template.id)
|
||||
|
||||
put :update, params: { id: service.id, service: { active: true } }
|
||||
put :update, params: { id: service_template.id, service: { active: true } }
|
||||
|
||||
expect(response).to have_gitlab_http_status(:found)
|
||||
end
|
||||
|
@ -54,7 +54,7 @@ describe Admin::ServicesController do
|
|||
it 'does not call the propagation worker when service is not active' do
|
||||
expect(PropagateServiceTemplateWorker).not_to receive(:perform_async)
|
||||
|
||||
put :update, params: { id: service.id, service: { properties: {} } }
|
||||
put :update, params: { id: service_template.id, service: { properties: {} } }
|
||||
|
||||
expect(response).to have_gitlab_http_status(:found)
|
||||
end
|
||||
|
|
|
@ -4,11 +4,6 @@ FactoryBot.define do
|
|||
factory :service do
|
||||
project
|
||||
type { 'Service' }
|
||||
|
||||
trait :instance do
|
||||
project { nil }
|
||||
instance { true }
|
||||
end
|
||||
end
|
||||
|
||||
factory :custom_issue_tracker_service, class: 'CustomIssueTrackerService' do
|
||||
|
@ -69,6 +64,7 @@ FactoryBot.define do
|
|||
factory :jira_service do
|
||||
project
|
||||
active { true }
|
||||
type { 'JiraService' }
|
||||
|
||||
transient do
|
||||
create_data { true }
|
||||
|
@ -159,4 +155,14 @@ FactoryBot.define do
|
|||
IssueTrackerService.set_callback(:validation, :before, :handle_properties)
|
||||
end
|
||||
end
|
||||
|
||||
trait :template do
|
||||
project { nil }
|
||||
template { true }
|
||||
end
|
||||
|
||||
trait :instance do
|
||||
project { nil }
|
||||
instance { true }
|
||||
end
|
||||
end
|
||||
|
|
|
@ -27,7 +27,7 @@ describe Gitlab::UsageData do
|
|||
create(:service, project: projects[1], type: 'SlackService', active: true)
|
||||
create(:service, project: projects[2], type: 'SlackService', active: true)
|
||||
create(:service, project: projects[2], type: 'MattermostService', active: false)
|
||||
create(:service, project: projects[2], type: 'MattermostService', active: true, template: true)
|
||||
create(:service, :template, type: 'MattermostService', active: true)
|
||||
create(:service, project: projects[2], type: 'CustomIssueTrackerService', active: true)
|
||||
create(:project_error_tracking_setting, project: projects[0])
|
||||
create(:project_error_tracking_setting, project: projects[1], enabled: false)
|
||||
|
|
|
@ -0,0 +1,21 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
require Rails.root.join('db', 'migrate', '20200305151736_delete_template_project_services.rb')
|
||||
|
||||
describe DeleteTemplateProjectServices, :migration do
|
||||
let(:services) { table(:services) }
|
||||
let(:project) { table(:projects).create!(namespace_id: 1) }
|
||||
|
||||
before do
|
||||
services.create!(template: true, project_id: project.id)
|
||||
services.create!(template: true)
|
||||
services.create!(template: false, project_id: project.id)
|
||||
end
|
||||
|
||||
it 'deletes services when template and attached to a project' do
|
||||
expect { migrate! }.to change { services.where(template: true, project_id: project.id).count }.from(1).to(0)
|
||||
.and not_change { services.where(template: true, project_id: nil).count }
|
||||
.and not_change { services.where(template: false).where.not(project_id: nil).count }
|
||||
end
|
||||
end
|
|
@ -28,17 +28,22 @@ describe Service do
|
|||
expect(build(:service, instance: true)).to be_invalid
|
||||
end
|
||||
|
||||
it 'validates absence of project_id if template', :aggregate_failures do
|
||||
expect(build(:service, template: true)).to validate_absence_of(:project_id)
|
||||
expect(build(:service, template: false)).not_to validate_absence_of(:project_id)
|
||||
end
|
||||
|
||||
it 'validates service is template or instance' do
|
||||
expect(build(:service, project_id: nil, template: true, instance: true)).to be_invalid
|
||||
end
|
||||
|
||||
context 'with an existing service template' do
|
||||
before do
|
||||
create(:service, type: 'Service', template: true)
|
||||
create(:service, :template)
|
||||
end
|
||||
|
||||
it 'validates only one service template per type' do
|
||||
expect(build(:service, type: 'Service', template: true)).to be_invalid
|
||||
expect(build(:service, :template)).to be_invalid
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -192,7 +197,7 @@ describe Service do
|
|||
|
||||
context 'when data are stored in separated fields' do
|
||||
let(:template) do
|
||||
create(:jira_service, data_params.merge(properties: {}, title: title, description: description, template: true))
|
||||
create(:jira_service, :template, data_params.merge(properties: {}, title: title, description: description))
|
||||
end
|
||||
|
||||
it_behaves_like 'service creation from a template'
|
||||
|
|
Loading…
Reference in New Issue