From 28fd7b6c5df43b2f29557e813055deb438791c67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 22 Jan 2018 19:28:10 +0100 Subject: Add auto_devops_domain to ApplicationSettings model --- ...162010_add_auto_devops_domain_to_application_settings.rb | 13 +++++++++++++ db/schema.rb | 3 ++- 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20180122162010_add_auto_devops_domain_to_application_settings.rb diff --git a/db/migrate/20180122162010_add_auto_devops_domain_to_application_settings.rb b/db/migrate/20180122162010_add_auto_devops_domain_to_application_settings.rb new file mode 100644 index 00000000000..7e16cb83087 --- /dev/null +++ b/db/migrate/20180122162010_add_auto_devops_domain_to_application_settings.rb @@ -0,0 +1,13 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddAutoDevopsDomainToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def change + add_column :application_settings, :auto_devops_domain, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index a0901833c3d..a683a60df45 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180113220114) do +ActiveRecord::Schema.define(version: 20180122162010) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -155,6 +155,7 @@ ActiveRecord::Schema.define(version: 20180113220114) do t.integer "gitaly_timeout_medium", default: 30, null: false t.integer "gitaly_timeout_fast", default: 10, null: false t.boolean "authorized_keys_enabled", default: true, null: false + t.string "auto_devops_domain" end create_table "audit_events", force: :cascade do |t| -- cgit v1.2.1 From 147f0428c00e7a10e908438a99a1558c24be41f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 22 Jan 2018 19:29:15 +0100 Subject: Add validation for auto_devops_domain --- app/models/application_setting.rb | 5 +++++ spec/models/application_setting_spec.rb | 40 +++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 80bda7f22ff..0dee6df525d 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -117,6 +117,11 @@ class ApplicationSetting < ActiveRecord::Base validates :repository_storages, presence: true validate :check_repository_storages + validates :auto_devops_domain, + allow_blank: true, + hostname: { allow_numeric_hostname: true, require_valid_tld: true }, + if: :auto_devops_enabled? + validates :enabled_git_access_protocol, inclusion: { in: %w(ssh http), allow_blank: true, allow_nil: true } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index ef480e7a80a..0d7b98229a4 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -114,6 +114,46 @@ describe ApplicationSetting do it { expect(setting.repository_storages).to eq(['default']) } end + context 'auto_devops_domain setting' do + context 'when auto_devops_enabled? is true' do + before do + setting.update(auto_devops_enabled: true) + end + + context 'with a valid value' do + before do + setting.update(auto_devops_domain: 'domain.com') + end + + it 'is valid' do + expect(setting).to be_valid + end + end + + context 'with an invalid value' do + before do + setting.update(auto_devops_domain: 'definitelynotahostname') + end + + it 'is invalid' do + expect(setting).to be_invalid + end + end + end + + context 'when auto_devops_enabled? is false' do + before do + setting.update(auto_devops_enabled: false) + end + + it 'can be blank' do + setting.update(auto_devops_domain: '') + + expect(setting).to be_valid + end + end + end + context 'circuitbreaker settings' do [:circuitbreaker_failure_count_threshold, :circuitbreaker_check_interval, -- cgit v1.2.1 From 57060295031f1ad2d5f058889561a68ede04d2f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 22 Jan 2018 19:31:03 +0100 Subject: Expose auto_devops_domain in admin settings view --- app/helpers/application_settings_helper.rb | 1 + app/views/admin/application_settings/_form.html.haml | 7 ++++++- spec/features/admin/admin_settings_spec.rb | 10 ++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 8ef561d90e6..6f07428975d 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -148,6 +148,7 @@ module ApplicationSettingsHelper :akismet_enabled, :authorized_keys_enabled, :auto_devops_enabled, + :auto_devops_domain, :circuitbreaker_access_retries, :circuitbreaker_check_interval, :circuitbreaker_failure_count_threshold, diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index ba4ca88a8a9..d77d99180fe 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -249,7 +249,12 @@ .help-block It will automatically build, test, and deploy applications based on a predefined CI/CD configuration = link_to icon('question-circle'), help_page_path('topics/autodevops/index.md') - + .form-group + = f.label :auto_devops_domain, class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :auto_devops_domain, class: 'form-control', placeholder: 'domain.com' + .help-block + = s_("Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages.") .form-group .col-sm-offset-2.col-sm-10 .checkbox diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 1218ea52227..cfc6697c79a 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -47,6 +47,16 @@ feature 'Admin updates settings' do expect(page).to have_content "Application settings saved successfully" end + scenario 'Change AutoDevOps settings' do + check 'Enabled Auto DevOps (Beta) for projects by default' + fill_in 'Auto devops domain', with: 'domain.com' + click_button 'Save' + + expect(current_application_settings.auto_devops_enabled?).to be true + expect(current_application_settings.auto_devops_domain).to eq('domain.com') + expect(page).to have_content "Application settings saved successfully" + end + scenario 'Change Slack Notifications Service template settings' do first(:link, 'Service Templates').click click_link 'Slack notifications' -- cgit v1.2.1 From 8f0a14843a630fec2eec22920ce922d5548ddec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 22 Jan 2018 19:31:35 +0100 Subject: Expose instance AutoDevOps domain in Project --- app/models/project.rb | 10 +++++++++- spec/models/project_spec.rb | 36 +++++++++++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index c0f7b30ceb0..87616716c7c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1604,7 +1604,15 @@ class Project < ActiveRecord::Base def auto_devops_variables return [] unless auto_devops_enabled? - auto_devops&.variables || [] + variables = [] + + if current_application_settings.auto_devops_domain.present? + variables << { key: 'AUTO_DEVOPS_DOMAIN', + value: current_application_settings.auto_devops_domain, + public: true } + end + + auto_devops&.variables || variables end def append_or_update_attribute(name, value) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 987be8e8b46..dc0825aad73 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2976,18 +2976,40 @@ describe Project do subject { project.auto_devops_variables } - context 'when enabled in settings' do + context 'when enabled in instance settings' do before do stub_application_setting(auto_devops_enabled: true) end + context 'when domain is empty' do + before do + stub_application_setting(auto_devops_domain: nil) + end + + it 'variables does not include AUTO_DEVOPS_DOMAIN' do + is_expected.not_to include(domain_variable) + end + end + + context 'when domain is configured' do + before do + stub_application_setting(auto_devops_domain: 'example.com') + end + + it 'variables includes AUTO_DEVOPS_DOMAIN' do + is_expected.to include(domain_variable) + end + end + end + + context 'when explicitely enabled' do context 'when domain is empty' do before do create(:project_auto_devops, project: project, domain: nil) end - it 'variables are empty' do - is_expected.to be_empty + it 'variables does not include AUTO_DEVOPS_DOMAIN' do + is_expected.not_to include(domain_variable) end end @@ -2996,11 +3018,15 @@ describe Project do create(:project_auto_devops, project: project, domain: 'example.com') end - it "variables are not empty" do - is_expected.not_to be_empty + it 'variables includes AUTO_DEVOPS_DOMAIN' do + is_expected.to include(domain_variable) end end end + + def domain_variable + { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true } + end end describe '#latest_successful_builds_for' do -- cgit v1.2.1 From f5591e4c901cea77f57e2b406e8e818d29919dfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 22 Jan 2018 20:25:22 +0100 Subject: Add CHANGELOG entry --- .../38175-add-domain-field-to-auto-devops-application-setting.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/38175-add-domain-field-to-auto-devops-application-setting.yml diff --git a/changelogs/unreleased/38175-add-domain-field-to-auto-devops-application-setting.yml b/changelogs/unreleased/38175-add-domain-field-to-auto-devops-application-setting.yml new file mode 100644 index 00000000000..475e1dc12b5 --- /dev/null +++ b/changelogs/unreleased/38175-add-domain-field-to-auto-devops-application-setting.yml @@ -0,0 +1,5 @@ +--- +title: Add Auto DevOps Domain application setting +merge_request: 16604 +author: +type: changed -- cgit v1.2.1 From 6028f9eecfad1190e99c830b3367b7ccb9fb5c81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 22 Jan 2018 20:37:02 +0100 Subject: Refactor auto_devops_domain setting specs --- spec/models/application_setting_spec.rb | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 0d7b98229a4..ae2d34750a7 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -120,6 +120,12 @@ describe ApplicationSetting do setting.update(auto_devops_enabled: true) end + it 'can be blank' do + setting.update(auto_devops_domain: '') + + expect(setting).to be_valid + end + context 'with a valid value' do before do setting.update(auto_devops_domain: 'domain.com') @@ -140,18 +146,6 @@ describe ApplicationSetting do end end end - - context 'when auto_devops_enabled? is false' do - before do - setting.update(auto_devops_enabled: false) - end - - it 'can be blank' do - setting.update(auto_devops_domain: '') - - expect(setting).to be_valid - end - end end context 'circuitbreaker settings' do -- cgit v1.2.1 From 62b7c3ba4aa1a354ac5380400c707a381ef0e5e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 22 Jan 2018 21:10:41 +0100 Subject: Short-circuit Project#auto_devops_variables This makes Project#auto_devops_variables more performant by evaluating the application settings only if necessary. --- app/models/project.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 87616716c7c..97f5e678021 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1604,15 +1604,13 @@ class Project < ActiveRecord::Base def auto_devops_variables return [] unless auto_devops_enabled? - variables = [] - - if current_application_settings.auto_devops_domain.present? - variables << { key: 'AUTO_DEVOPS_DOMAIN', - value: current_application_settings.auto_devops_domain, - public: true } - end - - auto_devops&.variables || variables + auto_devops&.variables || if current_application_settings.auto_devops_domain.present? + [{ key: 'AUTO_DEVOPS_DOMAIN', + value: current_application_settings.auto_devops_domain, + public: true }] + else + [] + end end def append_or_update_attribute(name, value) -- cgit v1.2.1 From 5126b1c5d3ba71a3ddad7e59142e46344b4c0eda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 30 Jan 2018 22:02:44 +0000 Subject: Update auto_devops_domain help block with i18n --- app/views/admin/application_settings/_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index d77d99180fe..e49fbafa61d 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -254,7 +254,7 @@ .col-sm-10 = f.text_field :auto_devops_domain, class: 'form-control', placeholder: 'domain.com' .help-block - = s_("Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages.") + = s_("AdminSettings|Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages.") .form-group .col-sm-offset-2.col-sm-10 .checkbox -- cgit v1.2.1 From a31539847fd431e18053d0ea5a007dc38134b3f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 1 Feb 2018 23:29:11 +0100 Subject: Add specs for .auto_devops_warning_message --- app/helpers/auto_devops_helper.rb | 19 ++++++++++++------ spec/helpers/auto_devops_helper_spec.rb | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/app/helpers/auto_devops_helper.rb b/app/helpers/auto_devops_helper.rb index d72457efec0..898ad2d304b 100644 --- a/app/helpers/auto_devops_helper.rb +++ b/app/helpers/auto_devops_helper.rb @@ -9,21 +9,28 @@ module AutoDevopsHelper end def auto_devops_warning_message(project) - missing_domain = !project.auto_devops&.has_domain? - missing_service = !project.deployment_platform&.active? - - if missing_service + if missing_service? params = { kubernetes: link_to('Kubernetes cluster', project_clusters_path(project)) } - if missing_domain + if missing_domain? _('Auto Review Apps and Auto Deploy need a domain name and a %{kubernetes} to work correctly.') % params else _('Auto Review Apps and Auto Deploy need a %{kubernetes} to work correctly.') % params end - elsif missing_domain + elsif missing_domain? _('Auto Review Apps and Auto Deploy need a domain name to work correctly.') end end + + private + + def missing_domain? + !(project.auto_devops&.has_domain? || current_application_settings.auto_devops_domain.present?) + end + + def missing_service? + !project.deployment_platform&.active? + end end diff --git a/spec/helpers/auto_devops_helper_spec.rb b/spec/helpers/auto_devops_helper_spec.rb index 5e272af6073..1fcbad5875d 100644 --- a/spec/helpers/auto_devops_helper_spec.rb +++ b/spec/helpers/auto_devops_helper_spec.rb @@ -82,4 +82,39 @@ describe AutoDevopsHelper do it { is_expected.to eq(false) } end end + + describe '.auto_devops_warning_message' do + subject { helper.auto_devops_warning_message(project) } + + context 'when the service is missing' do + before do + allow(helper).to receive(:missing_service?).and_return(true) + end + + context 'when the domain is missing' do + before do + allow(helper).to receive(:missing_domain?).and_return(true) + end + + it { is_expected.to match(/Auto Review Apps and Auto Deploy need a domain name and a .* to work correctly./) } + end + + context 'when the domain is not missing' do + before do + allow(helper).to receive(:missing_domain?).and_return(false) + end + + it { is_expected.to match(/Auto Review Apps and Auto Deploy need a .* to work correctly./) } + end + end + + context 'when the domain is missing' do + before do + allow(helper).to receive(:missing_service?).and_return(false) + allow(helper).to receive(:missing_domain?).and_return(true) + end + + it { is_expected.to eq('Auto Review Apps and Auto Deploy need a domain name to work correctly.') } + end + end end -- cgit v1.2.1 From e6487168ea1e0016a76e3e62f156990dd4412679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 1 Feb 2018 23:59:14 +0100 Subject: Read the AutoDevOps instance domain in ProjectAutoDevOps --- app/models/project.rb | 8 +----- app/models/project_auto_devops.rb | 10 ++++++-- spec/models/project_auto_devops_spec.rb | 43 ++++++++++++++++++++++++++++++--- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index cbcf8b80e8e..ef67b86d3e8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1609,13 +1609,7 @@ class Project < ActiveRecord::Base def auto_devops_variables return [] unless auto_devops_enabled? - auto_devops&.variables || if current_application_settings.auto_devops_domain.present? - [{ key: 'AUTO_DEVOPS_DOMAIN', - value: current_application_settings.auto_devops_domain, - public: true }] - else - [] - end + (auto_devops || ProjectAutoDevops.new)&.variables end def append_or_update_attribute(name, value) diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index 9a52edbff8e..f23310e854a 100644 --- a/app/models/project_auto_devops.rb +++ b/app/models/project_auto_devops.rb @@ -1,4 +1,6 @@ class ProjectAutoDevops < ActiveRecord::Base + include Gitlab::CurrentSettings + belongs_to :project scope :enabled, -> { where(enabled: true) } @@ -6,13 +8,17 @@ class ProjectAutoDevops < ActiveRecord::Base validates :domain, allow_blank: true, hostname: { allow_numeric_hostname: true } + def instance_domain + current_application_settings.auto_devops_domain + end + def has_domain? - domain.present? + domain.present? || instance_domain.present? end def variables variables = [] - variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain, public: true } if domain.present? + variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain || instance_domain, public: true } if has_domain? variables end end diff --git a/spec/models/project_auto_devops_spec.rb b/spec/models/project_auto_devops_spec.rb index 12069575866..de90080b4b8 100644 --- a/spec/models/project_auto_devops_spec.rb +++ b/spec/models/project_auto_devops_spec.rb @@ -18,7 +18,21 @@ describe ProjectAutoDevops do context 'when domain is empty' do let(:auto_devops) { build_stubbed(:project_auto_devops, project: project, domain: '') } - it { expect(auto_devops).not_to have_domain } + context 'when there is an instance domain specified' do + before do + stub_application_setting(auto_devops_domain: 'example.com') + end + + it { expect(auto_devops).to have_domain } + end + + context 'when there is no instance domain specified' do + before do + stub_application_setting(auto_devops_domain: nil) + end + + it { expect(auto_devops).not_to have_domain } + end end end @@ -29,9 +43,32 @@ describe ProjectAutoDevops do let(:domain) { 'example.com' } it 'returns AUTO_DEVOPS_DOMAIN' do - expect(auto_devops.variables).to include( - { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true }) + expect(auto_devops.variables).to include(domain_variable) end end + + context 'when domain is not defined' do + let(:domain) { nil } + + context 'when there is an instance domain specified' do + before do + stub_application_setting(auto_devops_domain: 'example.com') + end + + it { expect(auto_devops.variables).to include(domain_variable) } + end + + context 'when there is no instance domain specified' do + before do + stub_application_setting(auto_devops_domain: nil) + end + + it { expect(auto_devops.variables).not_to include(domain_variable) } + end + end + + def domain_variable + { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true } + end end end -- cgit v1.2.1 From e5ee1d6a29510125ae836720efd7f9127cff3032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 2 Feb 2018 00:25:01 +0100 Subject: Build AutoDevops instead of using ProjectAutoDevops#new --- app/models/project.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index ef67b86d3e8..c9f47fa4349 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1609,7 +1609,7 @@ class Project < ActiveRecord::Base def auto_devops_variables return [] unless auto_devops_enabled? - (auto_devops || ProjectAutoDevops.new)&.variables + (auto_devops || build_auto_devops)&.variables end def append_or_update_attribute(name, value) -- cgit v1.2.1 From c120b7a1fe575766c000e8e49387e1ef05671cda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 2 Feb 2018 01:21:56 +0100 Subject: Fix AutoDevOpsHelper helper methods --- app/helpers/auto_devops_helper.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/helpers/auto_devops_helper.rb b/app/helpers/auto_devops_helper.rb index 898ad2d304b..fa65ddf6b03 100644 --- a/app/helpers/auto_devops_helper.rb +++ b/app/helpers/auto_devops_helper.rb @@ -9,28 +9,28 @@ module AutoDevopsHelper end def auto_devops_warning_message(project) - if missing_service? + if missing_service?(project) params = { kubernetes: link_to('Kubernetes cluster', project_clusters_path(project)) } - if missing_domain? + if missing_domain?(project) _('Auto Review Apps and Auto Deploy need a domain name and a %{kubernetes} to work correctly.') % params else _('Auto Review Apps and Auto Deploy need a %{kubernetes} to work correctly.') % params end - elsif missing_domain? + elsif missing_domain?(project) _('Auto Review Apps and Auto Deploy need a domain name to work correctly.') end end private - def missing_domain? + def missing_domain?(project) !(project.auto_devops&.has_domain? || current_application_settings.auto_devops_domain.present?) end - def missing_service? + def missing_service?(project) !project.deployment_platform&.active? end end -- cgit v1.2.1 From 3443f3eb125186679afcbca2d58943e288691f2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 2 Feb 2018 19:54:00 +0100 Subject: Rename AutoDevopsHelper helper methods --- app/helpers/auto_devops_helper.rb | 10 +++++----- spec/helpers/auto_devops_helper_spec.rb | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/helpers/auto_devops_helper.rb b/app/helpers/auto_devops_helper.rb index fa65ddf6b03..4a7b7ff763b 100644 --- a/app/helpers/auto_devops_helper.rb +++ b/app/helpers/auto_devops_helper.rb @@ -9,28 +9,28 @@ module AutoDevopsHelper end def auto_devops_warning_message(project) - if missing_service?(project) + if missing_auto_devops_service?(project) params = { kubernetes: link_to('Kubernetes cluster', project_clusters_path(project)) } - if missing_domain?(project) + if missing_auto_devops_domain?(project) _('Auto Review Apps and Auto Deploy need a domain name and a %{kubernetes} to work correctly.') % params else _('Auto Review Apps and Auto Deploy need a %{kubernetes} to work correctly.') % params end - elsif missing_domain?(project) + elsif missing_auto_devops_domain?(project) _('Auto Review Apps and Auto Deploy need a domain name to work correctly.') end end private - def missing_domain?(project) + def missing_auto_devops_domain?(project) !(project.auto_devops&.has_domain? || current_application_settings.auto_devops_domain.present?) end - def missing_service?(project) + def missing_auto_devops_service?(project) !project.deployment_platform&.active? end end diff --git a/spec/helpers/auto_devops_helper_spec.rb b/spec/helpers/auto_devops_helper_spec.rb index 1fcbad5875d..1950c2b129b 100644 --- a/spec/helpers/auto_devops_helper_spec.rb +++ b/spec/helpers/auto_devops_helper_spec.rb @@ -88,12 +88,12 @@ describe AutoDevopsHelper do context 'when the service is missing' do before do - allow(helper).to receive(:missing_service?).and_return(true) + allow(helper).to receive(:missing_auto_devops_service?).and_return(true) end context 'when the domain is missing' do before do - allow(helper).to receive(:missing_domain?).and_return(true) + allow(helper).to receive(:missing_auto_devops_domain?).and_return(true) end it { is_expected.to match(/Auto Review Apps and Auto Deploy need a domain name and a .* to work correctly./) } @@ -101,7 +101,7 @@ describe AutoDevopsHelper do context 'when the domain is not missing' do before do - allow(helper).to receive(:missing_domain?).and_return(false) + allow(helper).to receive(:missing_auto_devops_domain?).and_return(false) end it { is_expected.to match(/Auto Review Apps and Auto Deploy need a .* to work correctly./) } @@ -110,8 +110,8 @@ describe AutoDevopsHelper do context 'when the domain is missing' do before do - allow(helper).to receive(:missing_service?).and_return(false) - allow(helper).to receive(:missing_domain?).and_return(true) + allow(helper).to receive(:missing_auto_devops_service?).and_return(false) + allow(helper).to receive(:missing_auto_devops_domain?).and_return(true) end it { is_expected.to eq('Auto Review Apps and Auto Deploy need a domain name to work correctly.') } -- cgit v1.2.1 From effb96b98511711fe8797f46fe1add68b968694f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 2 Feb 2018 23:06:13 +0100 Subject: Refactor AutoDevopsHelper.missing_auto_devops_domain? --- app/helpers/auto_devops_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/auto_devops_helper.rb b/app/helpers/auto_devops_helper.rb index 4a7b7ff763b..16451993e93 100644 --- a/app/helpers/auto_devops_helper.rb +++ b/app/helpers/auto_devops_helper.rb @@ -27,7 +27,7 @@ module AutoDevopsHelper private def missing_auto_devops_domain?(project) - !(project.auto_devops&.has_domain? || current_application_settings.auto_devops_domain.present?) + !(project.auto_devops || project.build_auto_devops)&.has_domain? end def missing_auto_devops_service?(project) -- cgit v1.2.1 From 56bcb3e8c87bf25cb33ed02832c146e966cc4d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 3 Feb 2018 01:56:11 +0100 Subject: Use Gitlab::CurrentSettings instead of current_application_settings --- app/models/project_auto_devops.rb | 4 +--- spec/features/admin/admin_settings_spec.rb | 4 ++-- spec/models/project_auto_devops_spec.rb | 8 ++++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index f23310e854a..728e64e8fe3 100644 --- a/app/models/project_auto_devops.rb +++ b/app/models/project_auto_devops.rb @@ -1,6 +1,4 @@ class ProjectAutoDevops < ActiveRecord::Base - include Gitlab::CurrentSettings - belongs_to :project scope :enabled, -> { where(enabled: true) } @@ -9,7 +7,7 @@ class ProjectAutoDevops < ActiveRecord::Base validates :domain, allow_blank: true, hostname: { allow_numeric_hostname: true } def instance_domain - current_application_settings.auto_devops_domain + Gitlab::CurrentSettings.auto_devops_domain end def has_domain? diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 3da2f4e2fdb..39b213988f0 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -52,8 +52,8 @@ feature 'Admin updates settings' do fill_in 'Auto devops domain', with: 'domain.com' click_button 'Save' - expect(current_application_settings.auto_devops_enabled?).to be true - expect(current_application_settings.auto_devops_domain).to eq('domain.com') + expect(Gitlab::CurrentSettings.auto_devops_enabled?).to be true + expect(Gitlab::CurrentSettings.auto_devops_domain).to eq('domain.com') expect(page).to have_content "Application settings saved successfully" end diff --git a/spec/models/project_auto_devops_spec.rb b/spec/models/project_auto_devops_spec.rb index de90080b4b8..296b91a771c 100644 --- a/spec/models/project_auto_devops_spec.rb +++ b/spec/models/project_auto_devops_spec.rb @@ -20,7 +20,7 @@ describe ProjectAutoDevops do context 'when there is an instance domain specified' do before do - stub_application_setting(auto_devops_domain: 'example.com') + allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return('example.com') end it { expect(auto_devops).to have_domain } @@ -28,7 +28,7 @@ describe ProjectAutoDevops do context 'when there is no instance domain specified' do before do - stub_application_setting(auto_devops_domain: nil) + allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return(nil) end it { expect(auto_devops).not_to have_domain } @@ -52,7 +52,7 @@ describe ProjectAutoDevops do context 'when there is an instance domain specified' do before do - stub_application_setting(auto_devops_domain: 'example.com') + allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return('example.com') end it { expect(auto_devops.variables).to include(domain_variable) } @@ -60,7 +60,7 @@ describe ProjectAutoDevops do context 'when there is no instance domain specified' do before do - stub_application_setting(auto_devops_domain: nil) + allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return(nil) end it { expect(auto_devops.variables).not_to include(domain_variable) } -- cgit v1.2.1 From 5291c0bb51ae19109e09ff0ee7fca6f118288923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 6 Feb 2018 15:30:55 +0100 Subject: Use domain.presence instead of domain to avoid empty strings --- app/models/project_auto_devops.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index 728e64e8fe3..112ed7ed434 100644 --- a/app/models/project_auto_devops.rb +++ b/app/models/project_auto_devops.rb @@ -16,7 +16,7 @@ class ProjectAutoDevops < ActiveRecord::Base def variables variables = [] - variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain || instance_domain, public: true } if has_domain? + variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain.presence || instance_domain, public: true } if has_domain? variables end end -- cgit v1.2.1