From 5c6314d46828780ea18361cfabbffcc87cd6b017 Mon Sep 17 00:00:00 2001 From: Siddharth Asthana Date: Tue, 6 Jul 2021 21:53:00 +0530 Subject: [PATCH 1/2] Force user to re-enter integration password Changelog: changed EE: true --- app/models/integrations/bamboo.rb | 3 ++- app/models/integrations/jenkins.rb | 2 +- app/models/integrations/jira.rb | 2 +- app/models/integrations/teamcity.rb | 3 ++- .../projects/services_controller_spec.rb | 9 +++++--- spec/fixtures/trace/sample_trace | 2 +- spec/models/integrations/bamboo_spec.rb | 10 ++++----- spec/models/integrations/jenkins_spec.rb | 12 +++++----- spec/models/integrations/jira_spec.rb | 22 +++++++++---------- spec/models/integrations/teamcity_spec.rb | 10 ++++----- 10 files changed, 40 insertions(+), 35 deletions(-) diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index 590be52151ced4..ee989f3d4bb8b5 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -18,7 +18,8 @@ class Bamboo < BaseCi attr_accessor :response - before_update :reset_password + after_save :compose_service_hook, if: :activated? + before_validation :reset_password def reset_password if bamboo_url_changed? && !password_touched? diff --git a/app/models/integrations/jenkins.rb b/app/models/integrations/jenkins.rb index 55fc60990f38e3..e5c1d5ad0d7af4 100644 --- a/app/models/integrations/jenkins.rb +++ b/app/models/integrations/jenkins.rb @@ -8,7 +8,7 @@ class Jenkins < BaseCi prop_accessor :jenkins_url, :project_name, :username, :password - before_update :reset_password + before_validation :reset_password validates :jenkins_url, presence: true, addressable_url: true, if: :activated? validates :project_name, presence: true, if: :activated? diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index 1dc5c0db9e3b62..9a3c2d675e0e03 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -33,7 +33,7 @@ class Jira < BaseIssueTracker data_field :username, :password, :url, :api_url, :jira_issue_transition_automatic, :jira_issue_transition_id, :project_key, :issues_enabled, :vulnerabilities_enabled, :vulnerabilities_issuetype - before_update :reset_password + before_validation :reset_password after_commit :update_deployment_type, on: [:create, :update], if: :update_deployment_type? enum comment_detail: { diff --git a/app/models/integrations/teamcity.rb b/app/models/integrations/teamcity.rb index 135c304b57eac3..70dcc5fd0df49a 100644 --- a/app/models/integrations/teamcity.rb +++ b/app/models/integrations/teamcity.rb @@ -18,7 +18,8 @@ class Teamcity < BaseCi attr_accessor :response - before_update :reset_password + after_save :compose_service_hook, if: :activated? + before_validation :reset_password class << self def to_param diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index baf3bde83bdf03..50d88b5923f323 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -241,9 +241,12 @@ def do_put context 'when update succeeds' do let(:integration_params) { { url: 'http://example.com' } } - it 'returns JSON response with no errors' do - expect(response).to be_successful - expect(json_response).to include('active' => true, 'errors' => {}) + it 'returns JSON response errors when password field is left blank' do + expect(response).not_to be_successful + expect(json_response).to include( + 'active' => true, + 'errors' => { 'url' => ['must be a valid URL', %{can't be blank}] } + ) end end diff --git a/spec/fixtures/trace/sample_trace b/spec/fixtures/trace/sample_trace index ebd2853e558985..ad01c113062b5f 100644 --- a/spec/fixtures/trace/sample_trace +++ b/spec/fixtures/trace/sample_trace @@ -2592,7 +2592,7 @@ TeamcityService should not validate that :username cannot be empty/falsy should not validate that :password cannot be empty/falsy Callbacks - before_update :reset_password + before_validation :reset_password saves password if new url is set together with password when no password was previously set when a password was previously set resets password if url changed diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index 73ebf404828bd5..cdae438bb6c36e 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -74,18 +74,18 @@ end describe 'Callbacks' do - describe 'before_update :reset_password' do + describe 'before_validation :reset_password' do context 'when a password was previously set' do it 'resets password if url changed' do integration.bamboo_url = 'http://gitlab1.com' - integration.save! + integration.valid? expect(integration.password).to be_nil end it 'does not reset password if username changed' do integration.username = 'some_name' - integration.save! + integration.valid? expect(integration.password).to eq('password') end @@ -93,7 +93,7 @@ it "does not reset password if new url is set together with password, even if it's the same password" do integration.bamboo_url = 'http://gitlab_edited.com' integration.password = 'password' - integration.save! + integration.valid? expect(integration.password).to eq('password') expect(integration.bamboo_url).to eq('http://gitlab_edited.com') @@ -105,7 +105,7 @@ integration.bamboo_url = 'http://gitlab_edited.com' integration.password = 'password' - integration.save! + integration.valid? expect(integration.password).to eq('password') expect(integration.bamboo_url).to eq('http://gitlab_edited.com') diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index 9eb2a7fc09839c..67e75de56e1487 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -200,21 +200,21 @@ it 'resets password if url changed' do jenkins_integration.jenkins_url = 'http://jenkins-edited.example.com/' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to be_nil end it 'resets password if username is blank' do jenkins_integration.username = '' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to be_nil end it 'does not reset password if username changed' do jenkins_integration.username = 'some_name' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to eq('password') end @@ -222,7 +222,7 @@ it 'does not reset password if new url is set together with password, even if it\'s the same password' do jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/' jenkins_integration.password = 'password' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to eq('password') expect(jenkins_integration.jenkins_url).to eq('http://jenkins_edited.example.com/') @@ -231,7 +231,7 @@ it 'resets password if url changed, even if setter called multiple times' do jenkins_integration.jenkins_url = 'http://jenkins1.example.com/' jenkins_integration.jenkins_url = 'http://jenkins1.example.com/' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to be_nil end @@ -251,7 +251,7 @@ it 'saves password if new url is set together with password' do jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/' jenkins_integration.password = 'password' - jenkins_integration.save! + jenkins_integration.valid? expect(jenkins_integration.password).to eq('password') expect(jenkins_integration.jenkins_url).to eq('http://jenkins_edited.example.com/') diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 6ca72d68bbb910..c8e23df147f8d4 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -358,7 +358,7 @@ it 'resets password if url changed' do integration integration.url = 'http://jira_edited.example.com' - integration.save! + integration.valid? expect(integration.reload.url).to eq('http://jira_edited.example.com') expect(integration.password).to be_nil @@ -366,7 +366,7 @@ it 'does not reset password if url "changed" to the same url as before' do integration.url = 'http://jira.example.com' - integration.save! + integration.valid? expect(integration.reload.url).to eq('http://jira.example.com') expect(integration.password).not_to be_nil @@ -374,7 +374,7 @@ it 'resets password if url not changed but api url added' do integration.api_url = 'http://jira_edited.example.com/rest/api/2' - integration.save! + integration.valid? expect(integration.reload.api_url).to eq('http://jira_edited.example.com/rest/api/2') expect(integration.password).to be_nil @@ -383,7 +383,7 @@ it 'does not reset password if new url is set together with password, even if it\'s the same password' do integration.url = 'http://jira_edited.example.com' integration.password = password - integration.save! + integration.valid? expect(integration.password).to eq(password) expect(integration.url).to eq('http://jira_edited.example.com') @@ -392,14 +392,14 @@ it 'resets password if url changed, even if setter called multiple times' do integration.url = 'http://jira1.example.com/rest/api/2' integration.url = 'http://jira1.example.com/rest/api/2' - integration.save! + integration.valid? expect(integration.password).to be_nil end it 'does not reset password if username changed' do integration.username = 'some_name' - integration.save! + integration.valid? expect(integration.reload.password).to eq(password) end @@ -407,7 +407,7 @@ it 'does not reset password if password changed' do integration.url = 'http://jira_edited.example.com' integration.password = 'new_password' - integration.save! + integration.valid? expect(integration.reload.password).to eq('new_password') end @@ -415,7 +415,7 @@ it 'does not reset password if the password is touched and same as before' do integration.url = 'http://jira_edited.example.com' integration.password = password - integration.save! + integration.valid? expect(integration.reload.password).to eq(password) end @@ -432,14 +432,14 @@ it 'resets password if api url changed' do integration.api_url = 'http://jira_edited.example.com/rest/api/2' - integration.save! + integration.valid? expect(integration.password).to be_nil end it 'does not reset password if url changed' do integration.url = 'http://jira_edited.example.com' - integration.save! + integration.valid? expect(integration.password).to eq(password) end @@ -462,7 +462,7 @@ it 'saves password if new url is set together with password' do integration.url = 'http://jira_edited.example.com/rest/api/2' integration.password = 'password' - integration.save! + integration.valid? expect(integration.reload.password).to eq('password') expect(integration.reload.url).to eq('http://jira_edited.example.com/rest/api/2') end diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index d425357aef0c73..17f3d1377be73d 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -76,18 +76,18 @@ describe 'Callbacks' do let(:teamcity_integration) { integration } - describe 'before_update :reset_password' do + describe 'before_validation :reset_password' do context 'when a password was previously set' do it 'resets password if url changed' do teamcity_integration.teamcity_url = 'http://gitlab1.com' - teamcity_integration.save! + teamcity_integration.valid? expect(teamcity_integration.password).to be_nil end it 'does not reset password if username changed' do teamcity_integration.username = 'some_name' - teamcity_integration.save! + teamcity_integration.valid? expect(teamcity_integration.password).to eq('password') end @@ -95,7 +95,7 @@ it "does not reset password if new url is set together with password, even if it's the same password" do teamcity_integration.teamcity_url = 'http://gitlab_edited.com' teamcity_integration.password = 'password' - teamcity_integration.save! + teamcity_integration.valid? expect(teamcity_integration.password).to eq('password') expect(teamcity_integration.teamcity_url).to eq('http://gitlab_edited.com') @@ -107,7 +107,7 @@ teamcity_integration.teamcity_url = 'http://gitlab_edited.com' teamcity_integration.password = 'password' - teamcity_integration.save! + teamcity_integration.valid? expect(teamcity_integration.password).to eq('password') expect(teamcity_integration.teamcity_url).to eq('http://gitlab_edited.com') -- GitLab From 04aa183df08a6601ddd04bc7b53a0d48cb6388b8 Mon Sep 17 00:00:00 2001 From: Markus Koller Date: Tue, 20 Jul 2021 17:40:36 +0200 Subject: [PATCH 2/2] Fix failing specs --- app/models/integrations/bamboo.rb | 1 - app/models/integrations/jira.rb | 5 +- app/models/integrations/teamcity.rb | 1 - .../issue_detail_entity_spec.rb | 2 +- .../jira_serializers/issue_entity_spec.rb | 2 +- .../admin/integrations_controller_spec.rb | 10 ++-- .../settings/integrations_controller_spec.rb | 10 ++-- .../projects/services_controller_spec.rb | 24 +++++++-- spec/factories/integrations.rb | 2 +- spec/models/integrations/bamboo_spec.rb | 15 +++--- spec/models/integrations/jenkins_spec.rb | 8 +-- spec/models/integrations/jira_spec.rb | 54 ++++++++++--------- spec/models/integrations/teamcity_spec.rb | 8 +-- 13 files changed, 84 insertions(+), 58 deletions(-) diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index ee989f3d4bb8b5..1a7cbaa34c7102 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -18,7 +18,6 @@ class Bamboo < BaseCi attr_accessor :response - after_save :compose_service_hook, if: :activated? before_validation :reset_password def reset_password diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index 9a3c2d675e0e03..745654a87be9ea 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -65,7 +65,10 @@ def data_fields end def reset_password - data_fields.password = nil if reset_password? + return unless reset_password? + + data_fields.password = nil + properties.delete('password') if properties end def set_default_data diff --git a/app/models/integrations/teamcity.rb b/app/models/integrations/teamcity.rb index 70dcc5fd0df49a..3f868b57597f56 100644 --- a/app/models/integrations/teamcity.rb +++ b/app/models/integrations/teamcity.rb @@ -18,7 +18,6 @@ class Teamcity < BaseCi attr_accessor :response - after_save :compose_service_hook, if: :activated? before_validation :reset_password class << self diff --git a/ee/spec/serializers/integrations/jira_serializers/issue_detail_entity_spec.rb b/ee/spec/serializers/integrations/jira_serializers/issue_detail_entity_spec.rb index 60c8674a95d8b1..29c00a25ba5b4e 100644 --- a/ee/spec/serializers/integrations/jira_serializers/issue_detail_entity_spec.rb +++ b/ee/spec/serializers/integrations/jira_serializers/issue_detail_entity_spec.rb @@ -131,7 +131,7 @@ context 'with only url' do before do stub_jira_integration_test - jira_integration.update!(api_url: nil) + jira_integration.update!(api_url: nil, password: 'password') end it 'returns URLs with the web url' do diff --git a/ee/spec/serializers/integrations/jira_serializers/issue_entity_spec.rb b/ee/spec/serializers/integrations/jira_serializers/issue_entity_spec.rb index ba56b5ebdf49ec..59b18c684e2412 100644 --- a/ee/spec/serializers/integrations/jira_serializers/issue_entity_spec.rb +++ b/ee/spec/serializers/integrations/jira_serializers/issue_entity_spec.rb @@ -95,7 +95,7 @@ context 'with only url' do before do stub_jira_integration_test - jira_integration.update!(api_url: nil) + jira_integration.update!(api_url: nil, password: 'password') end it 'returns URLs with the web url' do diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index 5a68bb2749b5ec..0d68cd5a64d2f7 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -43,15 +43,15 @@ stub_jira_integration_test allow(PropagateIntegrationWorker).to receive(:perform_async) - put :update, params: { id: integration.class.to_param, service: { url: url } } + put :update, params: { id: integration.class.to_param, service: params } end context 'valid params' do - let(:url) { 'https://jira.gitlab-example.com' } + let(:params) { { url: 'https://jira.gitlab-example.com', password: 'password' } } it 'updates the integration' do expect(response).to have_gitlab_http_status(:found) - expect(integration.reload.url).to eq(url) + expect(integration.reload).to have_attributes(params) end it 'calls to PropagateIntegrationWorker' do @@ -60,12 +60,12 @@ end context 'invalid params' do - let(:url) { 'invalid' } + let(:params) { { url: 'invalid', password: 'password' } } it 'does not update the integration' do expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:edit) - expect(integration.reload.url).not_to eq(url) + expect(integration.reload).not_to have_attributes(params) end it 'does not call to PropagateIntegrationWorker' do diff --git a/spec/controllers/groups/settings/integrations_controller_spec.rb b/spec/controllers/groups/settings/integrations_controller_spec.rb index ef8f9f69710358..931e726850a8d6 100644 --- a/spec/controllers/groups/settings/integrations_controller_spec.rb +++ b/spec/controllers/groups/settings/integrations_controller_spec.rb @@ -69,25 +69,25 @@ group.add_owner(user) stub_jira_integration_test - put :update, params: { group_id: group, id: integration.class.to_param, service: { url: url } } + put :update, params: { group_id: group, id: integration.class.to_param, service: params } end context 'valid params' do - let(:url) { 'https://jira.gitlab-example.com' } + let(:params) { { url: 'https://jira.gitlab-example.com', password: 'password' } } it 'updates the integration' do expect(response).to have_gitlab_http_status(:found) - expect(integration.reload.url).to eq(url) + expect(integration.reload).to have_attributes(params) end end context 'invalid params' do - let(:url) { 'invalid' } + let(:params) { { url: 'invalid', password: 'password' } } it 'does not update the integration' do expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:edit) - expect(integration.reload.url).not_to eq(url) + expect(integration.reload).not_to have_attributes(params) end end end diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 50d88b5923f323..82d7d5825f3058 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -239,25 +239,39 @@ def do_put end context 'when update succeeds' do + let(:integration_params) { { url: 'http://example.com', password: 'password' } } + + it 'returns success response' do + expect(response).to be_successful + expect(json_response).to include( + 'active' => true, + 'errors' => {} + ) + end + end + + context 'when update fails with missing password' do let(:integration_params) { { url: 'http://example.com' } } - it 'returns JSON response errors when password field is left blank' do + it 'returns JSON response errors' do expect(response).not_to be_successful expect(json_response).to include( 'active' => true, - 'errors' => { 'url' => ['must be a valid URL', %{can't be blank}] } + 'errors' => { + 'password' => ["can't be blank"] + } ) end end - context 'when update fails' do - let(:integration_params) { { url: '' } } + context 'when update fails with invalid URL' do + let(:integration_params) { { url: '', password: 'password' } } it 'returns JSON response with errors' do expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response).to include( 'active' => true, - 'errors' => { 'url' => ['must be a valid URL', %(can't be blank)] } + 'errors' => { 'url' => ['must be a valid URL', "can't be blank"] } ) end end diff --git a/spec/factories/integrations.rb b/spec/factories/integrations.rb index ed8a562b331e4d..a5a17ca4058da1 100644 --- a/spec/factories/integrations.rb +++ b/spec/factories/integrations.rb @@ -53,7 +53,7 @@ transient do create_data { true } url { 'https://jira.example.com' } - api_url { nil } + api_url { '' } username { 'jira_username' } password { 'jira_password' } jira_issue_transition_automatic { false } diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index cdae438bb6c36e..60ff6685c3d0f3 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -12,6 +12,7 @@ subject(:integration) do described_class.create!( + active: true, project: project, properties: { bamboo_url: bamboo_url, @@ -78,23 +79,23 @@ context 'when a password was previously set' do it 'resets password if url changed' do integration.bamboo_url = 'http://gitlab1.com' - integration.valid? + expect(integration).not_to be_valid expect(integration.password).to be_nil end it 'does not reset password if username changed' do integration.username = 'some_name' - integration.valid? + expect(integration).to be_valid expect(integration.password).to eq('password') end it "does not reset password if new url is set together with password, even if it's the same password" do integration.bamboo_url = 'http://gitlab_edited.com' integration.password = 'password' - integration.valid? + expect(integration).to be_valid expect(integration.password).to eq('password') expect(integration.bamboo_url).to eq('http://gitlab_edited.com') end @@ -105,10 +106,12 @@ integration.bamboo_url = 'http://gitlab_edited.com' integration.password = 'password' - integration.valid? + integration.save! - expect(integration.password).to eq('password') - expect(integration.bamboo_url).to eq('http://gitlab_edited.com') + expect(integration.reload).to have_attributes( + bamboo_url: 'http://gitlab_edited.com', + password: 'password' + ) end end end diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index 67e75de56e1487..9286d026290f60 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -251,10 +251,12 @@ it 'saves password if new url is set together with password' do jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/' jenkins_integration.password = 'password' - jenkins_integration.valid? + jenkins_integration.save! - expect(jenkins_integration.password).to eq('password') - expect(jenkins_integration.jenkins_url).to eq('http://jenkins_edited.example.com/') + expect(jenkins_integration.reload).to have_attributes( + jenkins_url: 'http://jenkins_edited.example.com/', + password: 'password' + ) end end end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index c8e23df147f8d4..d555ff379c6232 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -280,7 +280,7 @@ expect(integration.jira_tracker_data.deployment_server?).to be_truthy - integration.update!(api_url: 'http://another.url') + integration.update!(api_url: 'http://another.url', password: password) integration.jira_tracker_data.reload expect(integration.jira_tracker_data.deployment_cloud?).to be_truthy @@ -301,13 +301,13 @@ end it 'calls serverInfo for url' do - integration.update!(url: 'http://first.url') + integration.update!(url: 'http://first.url', password: password) expect(WebMock).to have_requested(:get, /serverInfo/) end it 'calls serverInfo for api_url' do - integration.update!(api_url: 'http://another.url') + integration.update!(api_url: 'http://another.url', password: password) expect(WebMock).to have_requested(:get, /serverInfo/) end @@ -358,33 +358,33 @@ it 'resets password if url changed' do integration integration.url = 'http://jira_edited.example.com' - integration.valid? - expect(integration.reload.url).to eq('http://jira_edited.example.com') + expect(integration).not_to be_valid + expect(integration.url).to eq('http://jira_edited.example.com') expect(integration.password).to be_nil end it 'does not reset password if url "changed" to the same url as before' do integration.url = 'http://jira.example.com' - integration.valid? - expect(integration.reload.url).to eq('http://jira.example.com') + expect(integration).to be_valid + expect(integration.url).to eq('http://jira.example.com') expect(integration.password).not_to be_nil end it 'resets password if url not changed but api url added' do integration.api_url = 'http://jira_edited.example.com/rest/api/2' - integration.valid? - expect(integration.reload.api_url).to eq('http://jira_edited.example.com/rest/api/2') + expect(integration).not_to be_valid + expect(integration.api_url).to eq('http://jira_edited.example.com/rest/api/2') expect(integration.password).to be_nil end it 'does not reset password if new url is set together with password, even if it\'s the same password' do integration.url = 'http://jira_edited.example.com' integration.password = password - integration.valid? + expect(integration).to be_valid expect(integration.password).to eq(password) expect(integration.url).to eq('http://jira_edited.example.com') end @@ -392,32 +392,32 @@ it 'resets password if url changed, even if setter called multiple times' do integration.url = 'http://jira1.example.com/rest/api/2' integration.url = 'http://jira1.example.com/rest/api/2' - integration.valid? + expect(integration).not_to be_valid expect(integration.password).to be_nil end it 'does not reset password if username changed' do integration.username = 'some_name' - integration.valid? - expect(integration.reload.password).to eq(password) + expect(integration).to be_valid + expect(integration.password).to eq(password) end it 'does not reset password if password changed' do integration.url = 'http://jira_edited.example.com' integration.password = 'new_password' - integration.valid? - expect(integration.reload.password).to eq('new_password') + expect(integration).to be_valid + expect(integration.password).to eq('new_password') end it 'does not reset password if the password is touched and same as before' do integration.url = 'http://jira_edited.example.com' integration.password = password - integration.valid? - expect(integration.reload.password).to eq(password) + expect(integration).to be_valid + expect(integration.password).to eq(password) end end @@ -432,22 +432,23 @@ it 'resets password if api url changed' do integration.api_url = 'http://jira_edited.example.com/rest/api/2' - integration.valid? + expect(integration).not_to be_valid expect(integration.password).to be_nil end it 'does not reset password if url changed' do integration.url = 'http://jira_edited.example.com' - integration.valid? + expect(integration).to be_valid expect(integration.password).to eq(password) end it 'resets password if api url set to empty' do - integration.update!(api_url: '') + integration.api_url = '' - expect(integration.reload.password).to be_nil + expect(integration).not_to be_valid + expect(integration.password).to be_nil end end end @@ -462,9 +463,12 @@ it 'saves password if new url is set together with password' do integration.url = 'http://jira_edited.example.com/rest/api/2' integration.password = 'password' - integration.valid? - expect(integration.reload.password).to eq('password') - expect(integration.reload.url).to eq('http://jira_edited.example.com/rest/api/2') + integration.save! + + expect(integration.reload).to have_attributes( + url: 'http://jira_edited.example.com/rest/api/2', + password: 'password' + ) end end end @@ -492,7 +496,7 @@ context 'when data are stored in both properties and separated fields' do let(:properties) { data_params } let(:integration) do - create(:jira_integration, :without_properties_callback, active: false, properties: properties).tap do |integration| + create(:jira_integration, :without_properties_callback, properties: properties).tap do |integration| create(:jira_tracker_data, data_params.merge(integration: integration)) end end diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index 17f3d1377be73d..0713141ea08cd0 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -107,10 +107,12 @@ teamcity_integration.teamcity_url = 'http://gitlab_edited.com' teamcity_integration.password = 'password' - teamcity_integration.valid? + teamcity_integration.save! - expect(teamcity_integration.password).to eq('password') - expect(teamcity_integration.teamcity_url).to eq('http://gitlab_edited.com') + expect(teamcity_integration.reload).to have_attributes( + teamcity_url: 'http://gitlab_edited.com', + password: 'password' + ) end end end -- GitLab