diff options
Diffstat (limited to 'spec/models')
20 files changed, 393 insertions, 543 deletions
diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 2efab3076d8..62a75bde63b 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -1,3 +1,4 @@ +# encoding: utf-8 # frozen_string_literal: true require 'spec_helper' diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 67cd939b4c6..917a65ddf21 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -22,72 +22,42 @@ describe Ci::BuildMetadata do describe '#update_timeout_state' do subject { metadata } - shared_examples 'sets timeout' do |source, timeout| - it 'sets project_timeout_source' do - expect { subject.update_timeout_state }.to change { subject.reload.timeout_source }.to(source) + context 'when runner is not assigned to the job' do + it "doesn't change timeout value" do + expect { subject.update_timeout_state }.not_to change { subject.reload.timeout } end - it 'sets project timeout' do - expect { subject.update_timeout_state }.to change { subject.reload.timeout }.to(timeout) + it "doesn't change timeout_source value" do + expect { subject.update_timeout_state }.not_to change { subject.reload.timeout_source } end end - context 'when project timeout is set' do - context 'when runner is assigned to the job' do - before do - build.update!(runner: runner) - end - - context 'when runner timeout is not set' do - let(:runner) { create(:ci_runner, maximum_timeout: nil) } - - it_behaves_like 'sets timeout', 'project_timeout_source', 2000 - end - - context 'when runner timeout is lower than project timeout' do - let(:runner) { create(:ci_runner, maximum_timeout: 1900) } - - it_behaves_like 'sets timeout', 'runner_timeout_source', 1900 - end - - context 'when runner timeout is higher than project timeout' do - let(:runner) { create(:ci_runner, maximum_timeout: 2100) } - - it_behaves_like 'sets timeout', 'project_timeout_source', 2000 - end + context 'when runner is assigned to the job' do + before do + build.update(runner: runner) end - context 'when job timeout is set' do - context 'when job timeout is higher than project timeout' do - let(:build) { create(:ci_build, pipeline: pipeline, options: { job_timeout: 3000 }) } + context 'when runner timeout is lower than project timeout' do + let(:runner) { create(:ci_runner, maximum_timeout: 1900) } - it_behaves_like 'sets timeout', 'job_timeout_source', 3000 + it 'sets runner timeout' do + expect { subject.update_timeout_state }.to change { subject.reload.timeout }.to(1900) end - context 'when job timeout is lower than project timeout' do - let(:build) { create(:ci_build, pipeline: pipeline, options: { job_timeout: 1000 }) } - - it_behaves_like 'sets timeout', 'job_timeout_source', 1000 + it 'sets runner_timeout_source' do + expect { subject.update_timeout_state }.to change { subject.reload.timeout_source }.to('runner_timeout_source') end end - context 'when both runner and job timeouts are set' do - before do - build.update(runner: runner) - end - - context 'when job timeout is higher than runner timeout' do - let(:build) { create(:ci_build, pipeline: pipeline, options: { job_timeout: 3000 }) } - let(:runner) { create(:ci_runner, maximum_timeout: 2100) } + context 'when runner timeout is higher than project timeout' do + let(:runner) { create(:ci_runner, maximum_timeout: 2100) } - it_behaves_like 'sets timeout', 'runner_timeout_source', 2100 + it 'sets project timeout' do + expect { subject.update_timeout_state }.to change { subject.reload.timeout }.to(2000) end - context 'when job timeout is lower than runner timeout' do - let(:build) { create(:ci_build, pipeline: pipeline, options: { job_timeout: 1900 }) } - let(:runner) { create(:ci_runner, maximum_timeout: 2100) } - - it_behaves_like 'sets timeout', 'job_timeout_source', 1900 + it 'sets project_timeout_source' do + expect { subject.update_timeout_state }.to change { subject.reload.timeout_source }.to('project_timeout_source') end end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index f984f6ba0ce..d9461ee8581 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -131,4 +131,41 @@ describe Clusters::Applications::Ingress do expect(values).to include('podAnnotations') end end + + describe '#values' do + let(:project) { build(:project) } + let(:cluster) { build(:cluster, projects: [project]) } + + context 'when ingress_modsecurity is enabled' do + before do + stub_feature_flags(ingress_modsecurity: true) + + allow(subject).to receive(:cluster).and_return(cluster) + end + + it 'includes modsecurity module enablement' do + expect(subject.values).to include("enable-modsecurity: 'true'") + end + + it 'includes modsecurity core ruleset enablement' do + expect(subject.values).to include("enable-owasp-modsecurity-crs: 'true'") + end + end + + context 'when ingress_modsecurity is disabled' do + before do + stub_feature_flags(ingress_modsecurity: false) + + allow(subject).to receive(:cluster).and_return(cluster) + end + + it 'excludes modsecurity module enablement' do + expect(subject.values).not_to include('enable-modsecurity') + end + + it 'excludes modsecurity core ruleset enablement' do + expect(subject.values).not_to include('enable-owasp-modsecurity-crs') + end + end + end end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index f78a089bc2e..cad705ee594 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -58,7 +58,7 @@ describe Group, 'Routable' do end end - context '.find_by_full_path' do + shared_examples_for '.find_by_full_path' do let!(:nested_group) { create(:group, parent: group) } context 'without any redirect routes' do @@ -117,6 +117,24 @@ describe Group, 'Routable' do end end + describe '.find_by_full_path' do + context 'with routable_two_step_lookup feature' do + before do + stub_feature_flags(routable_two_step_lookup: true) + end + + it_behaves_like '.find_by_full_path' + end + + context 'without routable_two_step_lookup feature' do + before do + stub_feature_flags(routable_two_step_lookup: false) + end + + it_behaves_like '.find_by_full_path' + end + end + describe '.where_full_path_in' do context 'without any paths' do it 'returns an empty relation' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 65cc1a4bd6b..fde1b096c76 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -650,35 +650,9 @@ describe MergeRequest do end end - describe '#discussions_diffs' do + describe '#preload_discussions_diff_highlight' do let(:merge_request) { create(:merge_request) } - shared_examples 'discussions diffs collection' do - it 'initializes Gitlab::DiscussionsDiff::FileCollection with correct data' do - note_diff_file = diff_note.note_diff_file - - expect(Gitlab::DiscussionsDiff::FileCollection) - .to receive(:new) - .with([note_diff_file]) - .and_call_original - - result = merge_request.discussions_diffs - - expect(result).to be_a(Gitlab::DiscussionsDiff::FileCollection) - end - - it 'eager loads relations' do - result = merge_request.discussions_diffs - - recorder = ActiveRecord::QueryRecorder.new do - result.first.diff_note - result.first.diff_note.project - end - - expect(recorder.count).to be_zero - end - end - context 'with commit diff note' do let(:other_merge_request) { create(:merge_request) } @@ -690,15 +664,40 @@ describe MergeRequest do create(:diff_note_on_commit, project: other_merge_request.project) end - it_behaves_like 'discussions diffs collection' + it 'preloads diff highlighting' do + expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection| + note_diff_file = diff_note.note_diff_file + + expect(collection) + .to receive(:load_highlight) + .with([note_diff_file.id]).and_call_original + end + + merge_request.preload_discussions_diff_highlight + end end context 'with merge request diff note' do - let!(:diff_note) do + let!(:unresolved_diff_note) do create(:diff_note_on_merge_request, project: merge_request.project, noteable: merge_request) end - it_behaves_like 'discussions diffs collection' + let!(:resolved_diff_note) do + create(:diff_note_on_merge_request, :resolved, project: merge_request.project, noteable: merge_request) + end + + it 'preloads diff highlighting' do + expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection| + note_diff_file = unresolved_diff_note.note_diff_file + + expect(collection) + .to receive(:load_highlight) + .with([note_diff_file.id]) + .and_call_original + end + + merge_request.preload_discussions_diff_highlight + end end end diff --git a/spec/models/milestone_release_spec.rb b/spec/models/milestone_release_spec.rb index 28cec7bbc17..d6f73275977 100644 --- a/spec/models/milestone_release_spec.rb +++ b/spec/models/milestone_release_spec.rb @@ -14,29 +14,23 @@ describe MilestoneRelease do it { is_expected.to belong_to(:release) } end - context 'when trying to create the same record in milestone_releases twice' do - it 'is not committing on the second time' do - create(:milestone_release, milestone: milestone, release: release) - - expect do - subject.save! - end.to raise_error(ActiveRecord::RecordNotUnique) - end - end - describe 'validations' do - subject(:milestone_release) { build(:milestone_release, milestone: milestone, release: release) } + it { is_expected.to validate_uniqueness_of(:milestone_id).scoped_to(:release_id) } context 'when milestone and release do not have the same project' do it 'is not valid' do - milestone_release.release = build(:release, project: create(:project)) - + other_project = create(:project) + release = build(:release, project: other_project) + milestone_release = described_class.new(milestone: milestone, release: release) expect(milestone_release).not_to be_valid end end context 'when milestone and release have the same project' do - it { is_expected.to be_valid } + it 'is valid' do + milestone_release = described_class.new(milestone: milestone, release: release) + expect(milestone_release).to be_valid + end end end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 0c4952eebd7..64030f5b92a 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -55,20 +55,20 @@ describe Milestone do end end - describe 'milestone_releases' do + describe 'milestone_release' do let(:milestone) { build(:milestone, project: project) } context 'when it is tied to a release for another project' do it 'creates a validation error' do other_project = create(:project) - milestone.releases << build(:release, project: other_project) + milestone.release = build(:release, project: other_project) expect(milestone).not_to be_valid end end context 'when it is tied to a release for the same project' do it 'is valid' do - milestone.releases << build(:release, project: project) + milestone.release = build(:release, project: project) expect(milestone).to be_valid end end @@ -78,8 +78,7 @@ describe Milestone do describe "Associations" do it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:issues) } - it { is_expected.to have_many(:releases) } - it { is_expected.to have_many(:milestone_releases) } + it { is_expected.to have_one(:release) } end let(:project) { create(:project, :public) } diff --git a/spec/models/project_services/bugzilla_service_spec.rb b/spec/models/project_services/bugzilla_service_spec.rb index e25d87f61d6..74c85a13c88 100644 --- a/spec/models/project_services/bugzilla_service_spec.rb +++ b/spec/models/project_services/bugzilla_service_spec.rb @@ -48,7 +48,7 @@ describe BugzillaService do create(:bugzilla_service, :without_properties_callback, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in separated fields' do @@ -56,7 +56,7 @@ describe BugzillaService do create(:bugzilla_service, title: title, description: description, properties: access_params) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in both properties and separated fields' do @@ -65,7 +65,7 @@ describe BugzillaService do create(:bugzilla_service, :without_properties_callback, title: title, description: description, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when no title & description are set' do diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb index 8359bc6807a..5259357a254 100644 --- a/spec/models/project_services/custom_issue_tracker_service_spec.rb +++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb @@ -62,7 +62,7 @@ describe CustomIssueTrackerService do create(:custom_issue_tracker_service, :without_properties_callback, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in separated fields' do @@ -70,7 +70,7 @@ describe CustomIssueTrackerService do create(:custom_issue_tracker_service, title: title, description: description, properties: access_params) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in both properties and separated fields' do @@ -79,7 +79,7 @@ describe CustomIssueTrackerService do create(:custom_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when no title & description are set' do diff --git a/spec/models/project_services/data_fields_spec.rb b/spec/models/project_services/data_fields_spec.rb deleted file mode 100644 index 146db0ae227..00000000000 --- a/spec/models/project_services/data_fields_spec.rb +++ /dev/null @@ -1,138 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe DataFields do - let(:url) { 'http://url.com' } - let(:username) { 'username_one' } - let(:properties) do - { url: url, username: username } - end - - shared_examples 'data fields' do - describe '#arg' do - it 'returns an argument correctly' do - expect(service.url).to eq(url) - end - end - - describe '{arg}_changed?' do - it 'returns false when the property has not been assigned a new value' do - service.username = 'new_username' - service.validate - expect(service.url_changed?).to be_falsy - end - - it 'returns true when the property has been assigned a different value' do - service.url = "http://example.com" - service.validate - expect(service.url_changed?).to be_truthy - end - - it 'returns true when the property has been assigned a different value twice' do - service.url = "http://example.com" - service.url = "http://example.com" - service.validate - expect(service.url_changed?).to be_truthy - end - - it 'returns false when the property has been re-assigned the same value' do - service.url = 'http://url.com' - service.validate - expect(service.url_changed?).to be_falsy - end - end - - describe '{arg}_touched?' do - it 'returns false when the property has not been assigned a new value' do - service.username = 'new_username' - service.validate - expect(service.url_changed?).to be_falsy - end - - it 'returns true when the property has been assigned a different value' do - service.url = "http://example.com" - service.validate - expect(service.url_changed?).to be_truthy - end - - it 'returns true when the property has been assigned a different value twice' do - service.url = "http://example.com" - service.url = "http://example.com" - service.validate - expect(service.url_changed?).to be_truthy - end - - it 'returns true when the property has been re-assigned the same value' do - service.url = 'http://url.com' - expect(service.url_touched?).to be_truthy - end - - it 'returns false when the property has been re-assigned the same value' do - service.url = 'http://url.com' - service.validate - expect(service.url_changed?).to be_falsy - end - end - end - - context 'when data are stored in data_fields' do - let(:service) do - create(:jira_service, url: url, username: username) - end - - it_behaves_like 'data fields' - - describe '{arg}_was?' do - it 'returns nil' do - service.url = 'http://example.com' - service.validate - expect(service.url_was).to be_nil - end - end - end - - context 'when data are stored in properties' do - let(:service) { create(:jira_service, :without_properties_callback, properties: properties) } - - it_behaves_like 'data fields' - - describe '{arg}_was?' do - it 'returns nil when the property has not been assigned a new value' do - service.username = 'new_username' - service.validate - expect(service.url_was).to be_nil - end - - it 'returns initial value when the property has been assigned a different value' do - service.url = 'http://example.com' - service.validate - expect(service.url_was).to eq('http://url.com') - end - - it 'returns initial value when the property has been re-assigned the same value' do - service.url = 'http://url.com' - service.validate - expect(service.url_was).to eq('http://url.com') - end - end - end - - context 'when data are stored in both properties and data_fields' do - let(:service) do - create(:jira_service, :without_properties_callback, active: false, properties: properties).tap do |service| - create(:jira_tracker_data, properties.merge(service: service)) - end - end - - it_behaves_like 'data fields' - - describe '{arg}_was?' do - it 'returns nil' do - service.url = 'http://example.com' - service.validate - expect(service.url_was).to be_nil - end - end - end -end diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index 4f3736ca65b..0c4fc290a13 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -65,7 +65,7 @@ describe GitlabIssueTrackerService do create(:gitlab_issue_tracker_service, :without_properties_callback, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in separated fields' do @@ -73,7 +73,7 @@ describe GitlabIssueTrackerService do create(:gitlab_issue_tracker_service, title: title, description: description, properties: access_params) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in both properties and separated fields' do @@ -82,7 +82,7 @@ describe GitlabIssueTrackerService do create(:gitlab_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when no title & description are set' do diff --git a/spec/models/project_services/issue_tracker_data_spec.rb b/spec/models/project_services/issue_tracker_data_spec.rb index db617cf0abb..aaab654f874 100644 --- a/spec/models/project_services/issue_tracker_data_spec.rb +++ b/spec/models/project_services/issue_tracker_data_spec.rb @@ -8,4 +8,28 @@ describe IssueTrackerData do describe 'Associations' do it { is_expected.to belong_to :service } end + + describe 'Validations' do + subject { described_class.new(service: service) } + + context 'url validations' do + context 'when service is inactive' do + it { is_expected.not_to validate_presence_of(:project_url) } + it { is_expected.not_to validate_presence_of(:issues_url) } + end + + context 'when service is active' do + before do + service.update(active: true) + end + + it_behaves_like 'issue tracker service URL attribute', :project_url + it_behaves_like 'issue tracker service URL attribute', :issues_url + it_behaves_like 'issue tracker service URL attribute', :new_issue_url + + it { is_expected.to validate_presence_of(:project_url) } + it { is_expected.to validate_presence_of(:issues_url) } + end + end + end end diff --git a/spec/models/project_services/issue_tracker_service_spec.rb b/spec/models/project_services/issue_tracker_service_spec.rb index f1cdee5c4a3..2fc4d69c2db 100644 --- a/spec/models/project_services/issue_tracker_service_spec.rb +++ b/spec/models/project_services/issue_tracker_service_spec.rb @@ -7,7 +7,7 @@ describe IssueTrackerService do let(:project) { create :project } describe 'only one issue tracker per project' do - let(:service) { RedmineService.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) } + let(:service) { RedmineService.new(project: project, active: true) } before do create(:custom_issue_tracker_service, project: project) diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index a976745023b..02060699e9a 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -6,18 +6,10 @@ describe JiraService do include Gitlab::Routing include AssetsHelpers - let(:title) { 'custom title' } - let(:description) { 'custom description' } - let(:url) { 'http://jira.example.com' } - let(:api_url) { 'http://api-jira.example.com' } - let(:username) { 'jira-username' } - let(:password) { 'jira-password' } - let(:transition_id) { 'test27' } - describe '#options' do let(:service) do - described_class.create( - project: create(:project), + described_class.new( + project: build_stubbed(:project), active: true, username: 'username', password: 'test', @@ -40,6 +32,78 @@ describe JiraService do describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } + it { is_expected.to allow_value(nil).for(:jira_issue_transition_id) } + it { is_expected.to allow_value('1,2,3').for(:jira_issue_transition_id) } + it { is_expected.to allow_value('1;2;3').for(:jira_issue_transition_id) } + it { is_expected.not_to allow_value('a,b,cd').for(:jira_issue_transition_id) } + end + + describe 'Validations' do + context 'when service is active' do + before do + subject.active = true + end + + it { is_expected.to validate_presence_of(:url) } + it_behaves_like 'issue tracker service URL attribute', :url + end + + context 'when service is inactive' do + before do + subject.active = false + end + + it { is_expected.not_to validate_presence_of(:url) } + it { is_expected.not_to validate_presence_of(:username) } + it { is_expected.not_to validate_presence_of(:password) } + end + + context 'validating urls' do + let(:service) do + described_class.new( + project: create(:project), + active: true, + username: 'username', + password: 'test', + jira_issue_transition_id: 24, + url: 'http://jira.test.com' + ) + end + + it 'is valid when all fields have required values' do + expect(service).to be_valid + end + + it 'is not valid when url is not a valid url' do + service.url = 'not valid' + + expect(service).not_to be_valid + end + + it 'is not valid when api url is not a valid url' do + service.api_url = 'not valid' + + expect(service).not_to be_valid + end + + it 'is not valid when username is missing' do + service.username = nil + + expect(service).not_to be_valid + end + + it 'is not valid when password is missing' do + service.password = nil + + expect(service).not_to be_valid + end + + it 'is valid when api url is a valid url' do + service.api_url = 'http://jira.test.com/api' + + expect(service).to be_valid + end + end end describe '.reference_pattern' do @@ -54,260 +118,55 @@ describe JiraService do describe '#create' do let(:params) do { - project: create(:project), - title: 'custom title', description: 'custom description', - url: url, api_url: api_url, - username: username, password: password, - jira_issue_transition_id: transition_id + project: create(:project), title: 'custom title', description: 'custom description' } end subject { described_class.create(params) } - it 'does not store data into properties' do - expect(subject.properties).to be_nil - end - - it 'sets title correctly' do - service = subject - - expect(service.title).to eq('custom title') + it 'does not store title & description into properties' do + expect(subject.properties.keys).not_to include('title', 'description') end - it 'sets service data correctly' do + it 'sets title & description correctly' do service = subject expect(service.title).to eq('custom title') expect(service.description).to eq('custom description') end - - it 'stores data in data_fields correcty' do - service = subject - - expect(service.jira_tracker_data.url).to eq(url) - expect(service.jira_tracker_data.api_url).to eq(api_url) - expect(service.jira_tracker_data.username).to eq(username) - expect(service.jira_tracker_data.password).to eq(password) - expect(service.jira_tracker_data.jira_issue_transition_id).to eq(transition_id) - end end - # we need to make sure we are able to read both from properties and jira_tracker_data table - # TODO: change this as part of #63084 context 'overriding properties' do + let(:url) { 'http://issue_tracker.example.com' } let(:access_params) do - { url: url, api_url: api_url, username: username, password: password, - jira_issue_transition_id: transition_id } - end - let(:data_params) do - { - url: url, api_url: api_url, - username: username, password: password, - jira_issue_transition_id: transition_id - } - end - - shared_examples 'handles jira fields' do - let(:data_params) do - { - url: url, api_url: api_url, - username: username, password: password, - jira_issue_transition_id: transition_id - } - end - - context 'reading data' do - it 'reads data correctly' do - expect(service.url).to eq(url) - expect(service.api_url).to eq(api_url) - expect(service.username).to eq(username) - expect(service.password).to eq(password) - expect(service.jira_issue_transition_id).to eq(transition_id) - end - end - - context '#update' do - context 'basic update' do - let(:new_username) { 'new_username' } - let(:new_url) { 'http://jira-new.example.com' } - - before do - service.update(username: new_username, url: new_url) - end - - it 'leaves properties field emtpy' do - # expect(service.reload.properties).to be_empty - end - - it 'stores updated data in jira_tracker_data table' do - data = service.jira_tracker_data.reload - - expect(data.url).to eq(new_url) - expect(data.api_url).to eq(api_url) - expect(data.username).to eq(new_username) - expect(data.password).to eq(password) - expect(data.jira_issue_transition_id).to eq(transition_id) - end - end - - context 'stored password invalidation' do - context 'when a password was previously set' do - context 'when only web url present' do - let(:data_params) do - { - url: url, api_url: nil, - username: username, password: password, - jira_issue_transition_id: transition_id - } - end - - it 'resets password if url changed' do - service - service.url = 'http://jira_edited.example.com' - service.save - - expect(service.reload.url).to eq('http://jira_edited.example.com') - expect(service.password).to be_nil - end - - it 'does not reset password if url "changed" to the same url as before' do - service.title = 'aaaaaa' - service.url = 'http://jira.example.com' - service.save - - expect(service.reload.url).to eq('http://jira.example.com') - expect(service.password).not_to be_nil - end - - it 'resets password if url not changed but api url added' do - service.api_url = 'http://jira_edited.example.com/rest/api/2' - service.save - - expect(service.reload.api_url).to eq('http://jira_edited.example.com/rest/api/2') - expect(service.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 - service.url = 'http://jira_edited.example.com' - service.password = password - service.save - - expect(service.password).to eq(password) - expect(service.url).to eq('http://jira_edited.example.com') - end - - it 'resets password if url changed, even if setter called multiple times' do - service.url = 'http://jira1.example.com/rest/api/2' - service.url = 'http://jira1.example.com/rest/api/2' - service.save - - expect(service.password).to be_nil - end - - it 'does not reset password if username changed' do - service.username = 'some_name' - service.save - - expect(service.reload.password).to eq(password) - end - - it 'does not reset password if password changed' do - service.url = 'http://jira_edited.example.com' - service.password = 'new_password' - service.save - - expect(service.reload.password).to eq('new_password') - end - - it 'does not reset password if the password is touched and same as before' do - service.url = 'http://jira_edited.example.com' - service.password = password - service.save - - expect(service.reload.password).to eq(password) - end - end - - context 'when both web and api url present' do - let(:data_params) do - { - url: url, api_url: 'http://jira.example.com/rest/api/2', - username: username, password: password, - jira_issue_transition_id: transition_id - } - end - - it 'resets password if api url changed' do - service.api_url = 'http://jira_edited.example.com/rest/api/2' - service.save - - expect(service.password).to be_nil - end - - it 'does not reset password if url changed' do - service.url = 'http://jira_edited.example.com' - service.save - - expect(service.password).to eq(password) - end - - it 'resets password if api url set to empty' do - service.update(api_url: '') - - expect(service.reload.password).to be_nil - end - end - end - - context 'when no password was previously set' do - let(:data_params) do - { - url: url, username: username - } - end - - it 'saves password if new url is set together with password' do - service.url = 'http://jira_edited.example.com/rest/api/2' - service.password = 'password' - service.save - expect(service.reload.password).to eq('password') - expect(service.reload.url).to eq('http://jira_edited.example.com/rest/api/2') - end - end - end - end + { url: url, username: 'username', password: 'password' } end # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 context 'when data are stored in properties' do - let(:properties) { data_params.merge(title: title, description: description) } - let!(:service) do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) do create(:jira_service, :without_properties_callback, properties: properties) end - it_behaves_like 'issue tracker fields' - it_behaves_like 'handles jira fields' + include_examples 'issue tracker fields' end context 'when data are stored in separated fields' do let(:service) do - create(:jira_service, data_params.merge(properties: {}, title: title, description: description)) + create(:jira_service, title: title, description: description, properties: access_params) end - it_behaves_like 'issue tracker fields' - it_behaves_like 'handles jira fields' + include_examples 'issue tracker fields' end context 'when data are stored in both properties and separated fields' do - let(:properties) { data_params.merge(title: title, description: description) } + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } let(:service) do - create(:jira_service, :without_properties_callback, active: false, properties: properties).tap do |service| - create(:jira_tracker_data, data_params.merge(service: service)) - end + create(:jira_service, :without_properties_callback, title: title, description: description, properties: properties) end - it_behaves_like 'issue tracker fields' - it_behaves_like 'handles jira fields' + include_examples 'issue tracker fields' end context 'when no title & description are set' do @@ -551,6 +410,111 @@ describe JiraService do end end + describe 'Stored password invalidation' do + let(:project) { create(:project) } + + context 'when a password was previously set' do + before do + @jira_service = described_class.create!( + project: project, + properties: { + url: 'http://jira.example.com/web', + username: 'mic', + password: 'password' + } + ) + end + + context 'when only web url present' do + it 'reset password if url changed' do + @jira_service.url = 'http://jira_edited.example.com/rest/api/2' + @jira_service.save + + expect(@jira_service.password).to be_nil + end + + it 'reset password if url not changed but api url added' do + @jira_service.api_url = 'http://jira_edited.example.com/rest/api/2' + @jira_service.save + + expect(@jira_service.password).to be_nil + end + end + + context 'when both web and api url present' do + before do + @jira_service.api_url = 'http://jira.example.com/rest/api/2' + @jira_service.password = 'password' + + @jira_service.save + end + it 'reset password if api url changed' do + @jira_service.api_url = 'http://jira_edited.example.com/rest/api/2' + @jira_service.save + + expect(@jira_service.password).to be_nil + end + + it 'does not reset password if url changed' do + @jira_service.url = 'http://jira_edited.example.com/rweb' + @jira_service.save + + expect(@jira_service.password).to eq('password') + end + + it 'reset password if api url set to empty' do + @jira_service.api_url = '' + @jira_service.save + + expect(@jira_service.password).to be_nil + end + end + + it 'does not reset password if username changed' do + @jira_service.username = 'some_name' + @jira_service.save + + expect(@jira_service.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 + @jira_service.url = 'http://jira_edited.example.com/rest/api/2' + @jira_service.password = 'password' + @jira_service.save + + expect(@jira_service.password).to eq('password') + expect(@jira_service.url).to eq('http://jira_edited.example.com/rest/api/2') + end + + it 'resets password if url changed, even if setter called multiple times' do + @jira_service.url = 'http://jira1.example.com/rest/api/2' + @jira_service.url = 'http://jira1.example.com/rest/api/2' + @jira_service.save + expect(@jira_service.password).to be_nil + end + end + + context 'when no password was previously set' do + before do + @jira_service = described_class.create( + project: project, + properties: { + url: 'http://jira.example.com/rest/api/2', + username: 'mic' + } + ) + end + + it 'saves password if new url is set together with password' do + @jira_service.url = 'http://jira_edited.example.com/rest/api/2' + @jira_service.password = 'password' + @jira_service.save + expect(@jira_service.password).to eq('password') + expect(@jira_service.url).to eq('http://jira_edited.example.com/rest/api/2') + end + end + end + describe 'description and title' do let(:title) { 'Jira One' } let(:description) { 'Jira One issue tracker' } @@ -575,7 +539,7 @@ describe JiraService do context 'when it is set in properties' do it 'values from properties are returned' do - service = create(:jira_service, :without_properties_callback, properties: properties) + service = create(:jira_service, properties: properties) expect(service.title).to eq(title) expect(service.description).to eq(description) @@ -638,8 +602,8 @@ describe JiraService do project = create(:project) service = project.create_jira_service(active: true) - expect(service.url).to eq('http://jira.sample/projects/project_a') - expect(service.api_url).to eq('http://jira.sample/api') + expect(service.properties['url']).to eq('http://jira.sample/projects/project_a') + expect(service.properties['api_url']).to eq('http://jira.sample/api') end end diff --git a/spec/models/project_services/jira_tracker_data_spec.rb b/spec/models/project_services/jira_tracker_data_spec.rb index 6cd3eb33d9b..1b6ece8531b 100644 --- a/spec/models/project_services/jira_tracker_data_spec.rb +++ b/spec/models/project_services/jira_tracker_data_spec.rb @@ -8,4 +8,35 @@ describe JiraTrackerData do describe 'Associations' do it { is_expected.to belong_to(:service) } end + + describe 'Validations' do + subject { described_class.new(service: service) } + + context 'jira_issue_transition_id' do + it { is_expected.to allow_value(nil).for(:jira_issue_transition_id) } + it { is_expected.to allow_value('1,2,3').for(:jira_issue_transition_id) } + it { is_expected.to allow_value('1;2;3').for(:jira_issue_transition_id) } + it { is_expected.not_to allow_value('a,b,cd').for(:jira_issue_transition_id) } + end + + context 'url validations' do + context 'when service is inactive' do + it { is_expected.not_to validate_presence_of(:url) } + it { is_expected.not_to validate_presence_of(:username) } + it { is_expected.not_to validate_presence_of(:password) } + end + + context 'when service is active' do + before do + service.update(active: true) + end + + it_behaves_like 'issue tracker service URL attribute', :url + + it { is_expected.to validate_presence_of(:url) } + it { is_expected.to validate_presence_of(:username) } + it { is_expected.to validate_presence_of(:password) } + end + end + end end diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb index 4ef4064d069..c1ee6546b12 100644 --- a/spec/models/project_services/redmine_service_spec.rb +++ b/spec/models/project_services/redmine_service_spec.rb @@ -9,15 +9,6 @@ describe RedmineService do end describe 'Validations' do - # if redmine is set in setting the urls are set to defaults - # therefore the validation passes as the values are not nil - before do - settings = { - 'redmine' => {} - } - allow(Gitlab.config).to receive(:issues_tracker).and_return(settings) - end - context 'when service is active' do before do subject.active = true @@ -26,7 +17,6 @@ describe RedmineService do it { is_expected.to validate_presence_of(:project_url) } it { is_expected.to validate_presence_of(:issues_url) } it { is_expected.to validate_presence_of(:new_issue_url) } - it_behaves_like 'issue tracker service URL attribute', :project_url it_behaves_like 'issue tracker service URL attribute', :issues_url it_behaves_like 'issue tracker service URL attribute', :new_issue_url @@ -64,7 +54,7 @@ describe RedmineService do create(:redmine_service, :without_properties_callback, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in separated fields' do @@ -72,7 +62,7 @@ describe RedmineService do create(:redmine_service, title: title, description: description, properties: access_params) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in both properties and separated fields' do @@ -81,7 +71,7 @@ describe RedmineService do create(:redmine_service, :without_properties_callback, title: title, description: description, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when no title & description are set' do diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/project_services/youtrack_service_spec.rb index eff9f451b1a..c48bf487af0 100644 --- a/spec/models/project_services/youtrack_service_spec.rb +++ b/spec/models/project_services/youtrack_service_spec.rb @@ -16,7 +16,6 @@ describe YoutrackService do it { is_expected.to validate_presence_of(:project_url) } it { is_expected.to validate_presence_of(:issues_url) } - it_behaves_like 'issue tracker service URL attribute', :project_url it_behaves_like 'issue tracker service URL attribute', :issues_url end @@ -52,7 +51,7 @@ describe YoutrackService do create(:youtrack_service, :without_properties_callback, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in separated fields' do @@ -60,7 +59,7 @@ describe YoutrackService do create(:youtrack_service, title: title, description: description, properties: access_params) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when data are stored in both properties and separated fields' do @@ -69,7 +68,7 @@ describe YoutrackService do create(:youtrack_service, :without_properties_callback, title: title, description: description, properties: properties) end - it_behaves_like 'issue tracker fields' + include_examples 'issue tracker fields' end context 'when no title & description are set' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 67f64822184..5b4b9c516a0 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2027,43 +2027,6 @@ describe Project do end end - describe '#latest_pipeline_for_ref' do - let(:project) { create(:project, :repository) } - let(:second_branch) { project.repository.branches[2] } - - let!(:pipeline_for_default_branch) do - create(:ci_empty_pipeline, project: project, sha: project.commit.id, - ref: project.default_branch) - end - let!(:pipeline_for_second_branch) do - create(:ci_empty_pipeline, project: project, sha: second_branch.target, - ref: second_branch.name) - end - - before do - create(:ci_empty_pipeline, project: project, sha: project.commit.parent.id, - ref: project.default_branch) - end - - context 'default repository branch' do - subject { project.latest_pipeline_for_ref(project.default_branch) } - - it { is_expected.to eq(pipeline_for_default_branch) } - end - - context 'provided ref' do - subject { project.latest_pipeline_for_ref(second_branch.name) } - - it { is_expected.to eq(pipeline_for_second_branch) } - end - - context 'bad ref' do - subject { project.latest_pipeline_for_ref(SecureRandom.uuid) } - - it { is_expected.to be_nil } - end - end - describe '#latest_successful_build_for_sha' do let(:project) { create(:project, :repository) } let(:pipeline) { create_pipeline(project) } diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 8714c67f29d..c690390e24d 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -13,8 +13,7 @@ RSpec.describe Release do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to have_many(:links).class_name('Releases::Link') } - it { is_expected.to have_many(:milestones) } - it { is_expected.to have_many(:milestone_releases) } + it { is_expected.to have_one(:milestone) } end describe 'validation' do @@ -39,15 +38,15 @@ RSpec.describe Release do context 'when a release is tied to a milestone for another project' do it 'creates a validation error' do - milestone = build(:milestone, project: create(:project)) - expect { release.milestones << milestone }.to raise_error + release.milestone = build(:milestone, project: create(:project)) + expect(release).not_to be_valid end end context 'when a release is tied to a milestone linked to the same project' do - it 'successfully links this release to this milestone' do - milestone = build(:milestone, project: project) - expect { release.milestones << milestone }.to change { MilestoneRelease.count }.by(1) + it 'is valid' do + release.milestone = build(:milestone, project: project) + expect(release).to be_valid end end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index d96e1398677..0797b9a9d83 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -257,8 +257,8 @@ describe Service do expect(service.title).to eq('random title') end - it 'sets data correctly' do - expect(service.data_fields.project_url).to eq('http://gitlab.example.com') + it 'creates the properties' do + expect(service.properties).to eq({ "project_url" => "http://gitlab.example.com" }) end end |