Commit ef3be00a authored by Adam Niedzielski's avatar Adam Niedzielski

Defer saving project services to the database if there are no user changes

parent 0f48abf2
...@@ -10,8 +10,7 @@ class Projects::ServicesController < Projects::ApplicationController ...@@ -10,8 +10,7 @@ class Projects::ServicesController < Projects::ApplicationController
layout "project_settings" layout "project_settings"
def index def index
@project.build_missing_services @services = @project.find_or_initialize_services
@services = @project.services.visible.reload
end end
def edit def edit
...@@ -46,6 +45,6 @@ class Projects::ServicesController < Projects::ApplicationController ...@@ -46,6 +45,6 @@ class Projects::ServicesController < Projects::ApplicationController
private private
def service def service
@service ||= @project.services.find { |service| service.to_param == params[:id] } @service ||= @project.find_or_initialize_service(params[:id])
end end
end end
...@@ -76,7 +76,6 @@ class Project < ActiveRecord::Base ...@@ -76,7 +76,6 @@ class Project < ActiveRecord::Base
has_many :boards, before_add: :validate_board_limit, dependent: :destroy has_many :boards, before_add: :validate_board_limit, dependent: :destroy
# Project services # Project services
has_many :services
has_one :campfire_service, dependent: :destroy has_one :campfire_service, dependent: :destroy
has_one :drone_ci_service, dependent: :destroy has_one :drone_ci_service, dependent: :destroy
has_one :emails_on_push_service, dependent: :destroy has_one :emails_on_push_service, dependent: :destroy
...@@ -748,27 +747,32 @@ class Project < ActiveRecord::Base ...@@ -748,27 +747,32 @@ class Project < ActiveRecord::Base
update_column(:has_external_wiki, services.external_wikis.any?) update_column(:has_external_wiki, services.external_wikis.any?)
end end
def build_missing_services def find_or_initialize_services
services_templates = Service.where(template: true) services_templates = Service.where(template: true)
Service.available_services_names.each do |service_name| Service.available_services_names.map do |service_name|
service = find_service(services, service_name) service = find_service(services, service_name)
# If service is available but missing in db if service
if service.nil? service
else
# We should check if template for the service exists # We should check if template for the service exists
template = find_service(services_templates, service_name) template = find_service(services_templates, service_name)
if template.nil? if template.nil?
# If no template, we should create an instance. Ex `create_gitlab_ci_service` # If no template, we should create an instance. Ex `build_gitlab_ci_service`
public_send("create_#{service_name}_service") public_send("build_#{service_name}_service")
else else
Service.create_from_template(self.id, template) Service.build_from_template(id, template)
end end
end end
end end
end end
def find_or_initialize_service(name)
find_or_initialize_services.find { |service| service.to_param == name }
end
def create_labels def create_labels
Label.templates.each do |label| Label.templates.each do |label|
params = label.attributes.except('id', 'template', 'created_at', 'updated_at') params = label.attributes.except('id', 'template', 'created_at', 'updated_at')
......
...@@ -222,11 +222,11 @@ class Service < ActiveRecord::Base ...@@ -222,11 +222,11 @@ class Service < ActiveRecord::Base
] ]
end end
def self.create_from_template(project_id, template) def self.build_from_template(project_id, template)
service = template.dup service = template.dup
service.template = false service.template = false
service.project_id = project_id service.project_id = project_id
service if service.save service
end end
private private
......
...@@ -95,7 +95,7 @@ module Projects ...@@ -95,7 +95,7 @@ module Projects
unless @project.gitlab_project_import? unless @project.gitlab_project_import?
@project.create_wiki unless skip_wiki? @project.create_wiki unless skip_wiki?
@project.build_missing_services create_services_from_active_templates(@project)
@project.create_labels @project.create_labels
end end
...@@ -135,5 +135,12 @@ module Projects ...@@ -135,5 +135,12 @@ module Projects
@project @project
end end
def create_services_from_active_templates(project)
Service.where(template: true, active: true).each do |template|
service = Service.build_from_template(project.id, template)
service.save!
end
end
end end
end end
...@@ -28,5 +28,6 @@ ...@@ -28,5 +28,6 @@
%td.hidden-xs %td.hidden-xs
= service.description = service.description
%td.light %td.light
= time_ago_in_words service.updated_at - if service.updated_at.present?
ago = time_ago_in_words service.updated_at
ago
---
title: Defer saving project services to the database if there are no user changes
merge_request: 6958
author:
...@@ -81,25 +81,10 @@ module API ...@@ -81,25 +81,10 @@ module API
end end
def project_service def project_service
@project_service ||= begin @project_service ||= user_project.find_or_initialize_service(params[:service_slug].underscore)
underscored_service = params[:service_slug].underscore
if Service.available_services_names.include?(underscored_service)
user_project.build_missing_services
service_method = "#{underscored_service}_service"
send_service(service_method)
end
end
@project_service || not_found!("Service") @project_service || not_found!("Service")
end end
def send_service(service_method)
user_project.send(service_method)
end
def service_attributes def service_attributes
@service_attributes ||= project_service.fields.inject([]) do |arr, hash| @service_attributes ||= project_service.fields.inject([]) do |arr, hash|
arr << hash[:name].to_sym arr << hash[:name].to_sym
......
...@@ -496,9 +496,6 @@ describe Project, models: true do ...@@ -496,9 +496,6 @@ describe Project, models: true do
end end
it 'returns nil and does not query services when there is no external issue tracker' do it 'returns nil and does not query services when there is no external issue tracker' do
project.build_missing_services
project.reload
expect(project).not_to receive(:services) expect(project).not_to receive(:services)
expect(project.external_issue_tracker).to eq(nil) expect(project.external_issue_tracker).to eq(nil)
...@@ -506,9 +503,6 @@ describe Project, models: true do ...@@ -506,9 +503,6 @@ describe Project, models: true do
it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do
ext_project.reload # Factory returns a project with changed attributes ext_project.reload # Factory returns a project with changed attributes
ext_project.build_missing_services
ext_project.reload
expect(ext_project).to receive(:services).once.and_call_original expect(ext_project).to receive(:services).once.and_call_original
2.times { expect(ext_project.external_issue_tracker).to be_a_kind_of(RedmineService) } 2.times { expect(ext_project.external_issue_tracker).to be_a_kind_of(RedmineService) }
......
...@@ -53,7 +53,7 @@ describe Service, models: true do ...@@ -53,7 +53,7 @@ describe Service, models: true do
describe "Template" do describe "Template" do
describe "for pushover service" do describe "for pushover service" do
let(:service_template) do let!(:service_template) do
PushoverService.create( PushoverService.create(
template: true, template: true,
properties: { properties: {
...@@ -66,13 +66,9 @@ describe Service, models: true do ...@@ -66,13 +66,9 @@ describe Service, models: true do
let(:project) { create(:project) } let(:project) { create(:project) }
describe 'is prefilled for projects pushover service' do describe 'is prefilled for projects pushover service' do
before do
service_template
project.build_missing_services
end
it "has all fields prefilled" do it "has all fields prefilled" do
service = project.pushover_service service = project.find_or_initialize_service('pushover')
expect(service.template).to eq(false) expect(service.template).to eq(false)
expect(service.device).to eq('MyDevice') expect(service.device).to eq('MyDevice')
expect(service.sound).to eq('mic') expect(service.sound).to eq('mic')
......
...@@ -56,8 +56,7 @@ describe API::API, api: true do ...@@ -56,8 +56,7 @@ describe API::API, api: true do
# inject some properties into the service # inject some properties into the service
before do before do
project.build_missing_services service_object = project.find_or_initialize_service(service)
service_object = project.send(service_method)
service_object.properties = service_attrs service_object.properties = service_attrs
service_object.save service_object.save
end end
......
...@@ -10,13 +10,6 @@ describe Projects::CreateService, services: true do ...@@ -10,13 +10,6 @@ describe Projects::CreateService, services: true do
} }
end end
it 'creates services on Project creation' do
project = create_project(@user, @opts)
project.reload
expect(project.services).not_to be_empty
end
it 'creates labels on Project creation if there are templates' do it 'creates labels on Project creation if there are templates' do
Label.create(title: "bug", template: true) Label.create(title: "bug", template: true)
project = create_project(@user, @opts) project = create_project(@user, @opts)
...@@ -137,6 +130,19 @@ describe Projects::CreateService, services: true do ...@@ -137,6 +130,19 @@ describe Projects::CreateService, services: true do
expect(project.namespace).to eq(@user.namespace) expect(project.namespace).to eq(@user.namespace)
end end
end end
context 'when there is an active service template' do
before do
create(:service, project: nil, template: true, active: true)
end
it 'creates a service from this template' do
project = create_project(@user, @opts)
project.reload
expect(project.services.count).to eq 1
end
end
end end
def create_project(user, opts) def create_project(user, opts)
......
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