Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
Diffstat (limited to 'spec')
-rw-r--r--spec/features/dashboard/groups_list_spec.rb22
-rw-r--r--spec/features/groups/members/manage_access_requests_spec.rb47
-rw-r--r--spec/features/groups/members/master_manages_access_requests_spec.rb8
-rw-r--r--spec/features/projects/issues/user_toggles_subscription_spec.rb8
-rw-r--r--spec/features/projects/members/master_manages_access_requests_spec.rb45
-rw-r--r--spec/finders/groups_finder_spec.rb84
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/notes.json5
-rw-r--r--spec/helpers/auth_helper_spec.rb24
-rw-r--r--spec/javascripts/sidebar/mock_data.js2
-rw-r--r--spec/javascripts/sidebar/sidebar_move_issue_spec.js9
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb87
-rw-r--r--spec/lib/omni_auth/strategies/jwt_spec.rb87
-rw-r--r--spec/models/members/group_member_spec.rb48
-rw-r--r--spec/models/members/project_member_spec.rb12
-rw-r--r--spec/requests/api/discussions_spec.rb33
-rw-r--r--spec/requests/openid_connect_spec.rb9
-rw-r--r--spec/services/notes/resolve_service_spec.rb23
-rw-r--r--spec/services/notification_service_spec.rb42
-rw-r--r--spec/services/projects/update_pages_service_spec.rb51
-rw-r--r--spec/services/repository_archive_clean_up_service_spec.rb68
-rw-r--r--spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb52
-rw-r--r--spec/support/shared_examples/models/members_notifications_shared_example.rb63
-rw-r--r--spec/support/shared_examples/requests/api/diff_discussions.rb57
-rw-r--r--spec/support/shared_examples/requests/api/resolvable_discussions.rb87
-rw-r--r--spec/workers/mail_scheduler/issue_due_worker_spec.rb4
-rw-r--r--spec/workers/mail_scheduler/notification_service_worker_spec.rb44
-rw-r--r--spec/workers/namespaceless_project_destroy_worker_spec.rb15
27 files changed, 738 insertions, 298 deletions
diff --git a/spec/features/dashboard/groups_list_spec.rb b/spec/features/dashboard/groups_list_spec.rb
index a71020002dc..ed47f7ed390 100644
--- a/spec/features/dashboard/groups_list_spec.rb
+++ b/spec/features/dashboard/groups_list_spec.rb
@@ -40,7 +40,7 @@ feature 'Dashboard Groups page', :js do
expect(page).to have_content(nested_group.name)
end
- describe 'when filtering groups', :nested_groups do
+ context 'when filtering groups', :nested_groups do
before do
group.add_owner(user)
nested_group.add_owner(user)
@@ -75,7 +75,7 @@ feature 'Dashboard Groups page', :js do
end
end
- describe 'group with subgroups', :nested_groups do
+ context 'with subgroups', :nested_groups do
let!(:subgroup) { create(:group, :public, parent: group) }
before do
@@ -106,7 +106,7 @@ feature 'Dashboard Groups page', :js do
end
end
- describe 'when using pagination' do
+ context 'when using pagination' do
let(:group) { create(:group, created_at: 5.days.ago) }
let(:group2) { create(:group, created_at: 2.days.ago) }
@@ -141,4 +141,20 @@ feature 'Dashboard Groups page', :js do
expect(page).not_to have_selector("#group-#{group2.id}")
end
end
+
+ context 'when signed in as admin' do
+ let(:admin) { create(:admin) }
+
+ it 'shows only groups admin is member of' do
+ group.add_owner(admin)
+ expect(another_group).to be_persisted
+
+ sign_in(admin)
+ visit dashboard_groups_path
+ wait_for_requests
+
+ expect(page).to have_content(group.name)
+ expect(page).not_to have_content(another_group.name)
+ end
+ end
end
diff --git a/spec/features/groups/members/manage_access_requests_spec.rb b/spec/features/groups/members/manage_access_requests_spec.rb
deleted file mode 100644
index b83cd657ef7..00000000000
--- a/spec/features/groups/members/manage_access_requests_spec.rb
+++ /dev/null
@@ -1,47 +0,0 @@
-require 'spec_helper'
-
-feature 'Groups > Members > Manage access requests' do
- let(:user) { create(:user) }
- let(:owner) { create(:user) }
- let(:group) { create(:group, :public, :access_requestable) }
-
- background do
- group.request_access(user)
- group.add_owner(owner)
- sign_in(owner)
- end
-
- scenario 'owner can see access requests' do
- visit group_group_members_path(group)
-
- expect_visible_access_request(group, user)
- end
-
- scenario 'owner can grant access' do
- visit group_group_members_path(group)
-
- expect_visible_access_request(group, user)
-
- perform_enqueued_jobs { click_on 'Grant access' }
-
- expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
- expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was granted"
- end
-
- scenario 'owner can deny access' do
- visit group_group_members_path(group)
-
- expect_visible_access_request(group, user)
-
- perform_enqueued_jobs { click_on 'Deny access' }
-
- expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
- expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was denied"
- end
-
- def expect_visible_access_request(group, user)
- expect(group.requesters.exists?(user_id: user)).to be_truthy
- expect(page).to have_content "Users requesting access to #{group.name} 1"
- expect(page).to have_content user.name
- end
-end
diff --git a/spec/features/groups/members/master_manages_access_requests_spec.rb b/spec/features/groups/members/master_manages_access_requests_spec.rb
new file mode 100644
index 00000000000..2fd6d1ec599
--- /dev/null
+++ b/spec/features/groups/members/master_manages_access_requests_spec.rb
@@ -0,0 +1,8 @@
+require 'spec_helper'
+
+feature 'Groups > Members > Master manages access requests' do
+ it_behaves_like 'Master manages access requests' do
+ let(:entity) { create(:group, :public, :access_requestable) }
+ let(:members_page_path) { group_group_members_path(entity) }
+ end
+end
diff --git a/spec/features/projects/issues/user_toggles_subscription_spec.rb b/spec/features/projects/issues/user_toggles_subscription_spec.rb
index 117a614b980..c2b2a193682 100644
--- a/spec/features/projects/issues/user_toggles_subscription_spec.rb
+++ b/spec/features/projects/issues/user_toggles_subscription_spec.rb
@@ -1,9 +1,9 @@
require "spec_helper"
describe "User toggles subscription", :js do
- set(:project) { create(:project_empty_repo, :public) }
- set(:user) { create(:user) }
- set(:issue) { create(:issue, project: project, author: user) }
+ let(:project) { create(:project_empty_repo, :public) }
+ let(:user) { create(:user) }
+ let(:issue) { create(:issue, project: project, author: user) }
before do
project.add_developer(user)
@@ -12,7 +12,7 @@ describe "User toggles subscription", :js do
visit(project_issue_path(project, issue))
end
- it "unsibscribes from issue" do
+ it "unsubscribes from issue" do
subscription_button = find(".js-issuable-subscribe-button")
# Check we're subscribed.
diff --git a/spec/features/projects/members/master_manages_access_requests_spec.rb b/spec/features/projects/members/master_manages_access_requests_spec.rb
index 1f4eec0a317..3ac6ca4fc86 100644
--- a/spec/features/projects/members/master_manages_access_requests_spec.rb
+++ b/spec/features/projects/members/master_manages_access_requests_spec.rb
@@ -1,47 +1,8 @@
require 'spec_helper'
feature 'Projects > Members > Master manages access requests' do
- let(:user) { create(:user) }
- let(:master) { create(:user) }
- let(:project) { create(:project, :public, :access_requestable) }
-
- background do
- project.request_access(user)
- project.add_master(master)
- sign_in(master)
- end
-
- scenario 'master can see access requests' do
- visit project_project_members_path(project)
-
- expect_visible_access_request(project, user)
- end
-
- scenario 'master can grant access' do
- visit project_project_members_path(project)
-
- expect_visible_access_request(project, user)
-
- perform_enqueued_jobs { click_on 'Grant access' }
-
- expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
- expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.full_name} project was granted"
- end
-
- scenario 'master can deny access' do
- visit project_project_members_path(project)
-
- expect_visible_access_request(project, user)
-
- perform_enqueued_jobs { click_on 'Deny access' }
-
- expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
- expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.full_name} project was denied"
- end
-
- def expect_visible_access_request(project, user)
- expect(project.requesters.exists?(user_id: user)).to be_truthy
- expect(page).to have_content "Users requesting access to #{project.name} 1"
- expect(page).to have_content user.name
+ it_behaves_like 'Master manages access requests' do
+ let(:entity) { create(:project, :public, :access_requestable) }
+ let(:members_page_path) { project_project_members_path(entity) }
end
end
diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb
index abc470788e1..16c0d418d98 100644
--- a/spec/finders/groups_finder_spec.rb
+++ b/spec/finders/groups_finder_spec.rb
@@ -2,43 +2,71 @@ require 'spec_helper'
describe GroupsFinder do
describe '#execute' do
- let(:user) { create(:user) }
-
- context 'root level groups' do
- let!(:private_group) { create(:group, :private) }
- let!(:internal_group) { create(:group, :internal) }
- let!(:public_group) { create(:group, :public) }
-
- context 'without a user' do
- subject { described_class.new.execute }
-
- it { is_expected.to eq([public_group]) }
+ let(:user) { create(:user) }
+
+ describe 'root level groups' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:user_type, :params, :results) do
+ nil | { all_available: true } | %i(public_group user_public_group)
+ nil | { all_available: false } | %i(public_group user_public_group)
+ nil | {} | %i(public_group user_public_group)
+
+ :regular | { all_available: true } | %i(public_group internal_group user_public_group user_internal_group
+ user_private_group)
+ :regular | { all_available: false } | %i(user_public_group user_internal_group user_private_group)
+ :regular | {} | %i(public_group internal_group user_public_group user_internal_group user_private_group)
+
+ :external | { all_available: true } | %i(public_group user_public_group user_internal_group user_private_group)
+ :external | { all_available: false } | %i(user_public_group user_internal_group user_private_group)
+ :external | {} | %i(public_group user_public_group user_internal_group user_private_group)
+
+ :admin | { all_available: true } | %i(public_group internal_group private_group user_public_group
+ user_internal_group user_private_group)
+ :admin | { all_available: false } | %i(user_public_group user_internal_group user_private_group)
+ :admin | {} | %i(public_group internal_group private_group user_public_group user_internal_group
+ user_private_group)
end
- context 'with a user' do
- subject { described_class.new(user).execute }
-
- context 'normal user' do
- it { is_expected.to contain_exactly(public_group, internal_group) }
- end
-
- context 'external user' do
- let(:user) { create(:user, external: true) }
-
- it { is_expected.to contain_exactly(public_group) }
+ with_them do
+ before do
+ # Fixme: Because of an issue: https://github.com/tomykaira/rspec-parameterized/issues/8#issuecomment-381888428
+ # The groups need to be created here, not with let syntax, and also compared by name and not ids
+
+ @groups = {
+ private_group: create(:group, :private, name: 'private_group'),
+ internal_group: create(:group, :internal, name: 'internal_group'),
+ public_group: create(:group, :public, name: 'public_group'),
+
+ user_private_group: create(:group, :private, name: 'user_private_group'),
+ user_internal_group: create(:group, :internal, name: 'user_internal_group'),
+ user_public_group: create(:group, :public, name: 'user_public_group')
+ }
+
+ if user_type
+ user =
+ case user_type
+ when :regular
+ create(:user)
+ when :external
+ create(:user, external: true)
+ when :admin
+ create(:user, :admin)
+ end
+ @groups.values_at(:user_private_group, :user_internal_group, :user_public_group).each do |group|
+ group.add_developer(user)
+ end
+ end
end
- context 'user is member of the private group' do
- before do
- private_group.add_guest(user)
- end
+ subject { described_class.new(User.last, params).execute.to_a }
- it { is_expected.to contain_exactly(public_group, internal_group, private_group) }
- end
+ it { is_expected.to match_array(@groups.values_at(*results)) }
end
end
context 'subgroups', :nested_groups do
+ let(:user) { create(:user) }
let!(:parent_group) { create(:group, :public) }
let!(:public_subgroup) { create(:group, :public, parent: parent_group) }
let!(:internal_subgroup) { create(:group, :internal, parent: parent_group) }
diff --git a/spec/fixtures/api/schemas/public_api/v4/notes.json b/spec/fixtures/api/schemas/public_api/v4/notes.json
index 4c4ca3b582f..0f9c221fd6d 100644
--- a/spec/fixtures/api/schemas/public_api/v4/notes.json
+++ b/spec/fixtures/api/schemas/public_api/v4/notes.json
@@ -24,7 +24,10 @@
"system": { "type": "boolean" },
"noteable_id": { "type": "integer" },
"noteable_iid": { "type": "integer" },
- "noteable_type": { "type": "string" }
+ "noteable_type": { "type": "string" },
+ "resolved": { "type": "boolean" },
+ "resolvable": { "type": "boolean" },
+ "resolved_by": { "type": ["string", "null"] }
},
"required": [
"id", "body", "attachment", "author", "created_at", "updated_at",
diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb
index c94fedd615b..120b23e66ac 100644
--- a/spec/helpers/auth_helper_spec.rb
+++ b/spec/helpers/auth_helper_spec.rb
@@ -18,6 +18,30 @@ describe AuthHelper do
end
end
+ describe "providers_for_base_controller" do
+ it 'returns all enabled providers from devise' do
+ allow(helper).to receive(:auth_providers) { [:twitter, :github] }
+ expect(helper.providers_for_base_controller).to include(*[:twitter, :github])
+ end
+
+ it 'excludes ldap providers' do
+ allow(helper).to receive(:auth_providers) { [:twitter, :ldapmain] }
+ expect(helper.providers_for_base_controller).not_to include(:ldapmain)
+ end
+ end
+
+ describe "form_based_providers" do
+ it 'includes LDAP providers' do
+ allow(helper).to receive(:auth_providers) { [:twitter, :ldapmain] }
+ expect(helper.form_based_providers).to eq %i(ldapmain)
+ end
+
+ it 'includes crowd provider' do
+ allow(helper).to receive(:auth_providers) { [:twitter, :crowd] }
+ expect(helper.form_based_providers).to eq %i(crowd)
+ end
+ end
+
describe 'enabled_button_based_providers' do
before do
allow(helper).to receive(:auth_providers) { [:twitter, :github] }
diff --git a/spec/javascripts/sidebar/mock_data.js b/spec/javascripts/sidebar/mock_data.js
index 8b6e8b24f00..fcd7bea3f6d 100644
--- a/spec/javascripts/sidebar/mock_data.js
+++ b/spec/javascripts/sidebar/mock_data.js
@@ -138,7 +138,7 @@ const RESPONSE_MAP = {
},
{
id: 20,
- name_with_namespace: 'foo / bar',
+ name_with_namespace: '<img src=x onerror=alert(document.domain)> foo / bar',
},
],
},
diff --git a/spec/javascripts/sidebar/sidebar_move_issue_spec.js b/spec/javascripts/sidebar/sidebar_move_issue_spec.js
index a3fb965fbab..00847df4b60 100644
--- a/spec/javascripts/sidebar/sidebar_move_issue_spec.js
+++ b/spec/javascripts/sidebar/sidebar_move_issue_spec.js
@@ -69,6 +69,15 @@ describe('SidebarMoveIssue', function () {
expect($.fn.glDropdown).toHaveBeenCalled();
});
+
+ it('escapes html from project name', (done) => {
+ this.$toggleButton.dropdown('toggle');
+
+ setTimeout(() => {
+ expect(this.$content.find('.js-move-issue-dropdown-item')[1].innerHTML.trim()).toEqual('&lt;img src=x onerror=alert(document.domain)&gt; foo / bar');
+ done();
+ });
+ });
});
describe('onConfirmClicked', () => {
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index da1a6229ccf..9924641f829 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -234,59 +234,72 @@ describe Gitlab::Git::Repository, seed_helper: true do
it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :tag_names
end
- shared_examples 'archive check' do |extenstion|
- it { expect(metadata['ArchivePath']).to match(%r{tmp/gitlab-git-test.git/gitlab-git-test-master-#{SeedRepo::LastCommit::ID}}) }
- it { expect(metadata['ArchivePath']).to end_with extenstion }
- end
+ describe '#archive_metadata' do
+ let(:storage_path) { '/tmp' }
+ let(:cache_key) { File.join(repository.gl_repository, SeedRepo::LastCommit::ID) }
- describe '#archive_prefix' do
- let(:project_name) { 'project-name'}
+ let(:append_sha) { true }
+ let(:ref) { 'master' }
+ let(:format) { nil }
- before do
- expect(repository).to receive(:name).once.and_return(project_name)
- end
+ let(:expected_extension) { 'tar.gz' }
+ let(:expected_filename) { "#{expected_prefix}.#{expected_extension}" }
+ let(:expected_path) { File.join(storage_path, cache_key, expected_filename) }
+ let(:expected_prefix) { "gitlab-git-test-#{ref}-#{SeedRepo::LastCommit::ID}" }
- it 'returns parameterised string for a ref containing slashes' do
- prefix = repository.archive_prefix('test/branch', 'SHA', append_sha: nil)
+ subject(:metadata) { repository.archive_metadata(ref, storage_path, format, append_sha: append_sha) }
- expect(prefix).to eq("#{project_name}-test-branch-SHA")
+ it 'sets RepoPath to the repository path' do
+ expect(metadata['RepoPath']).to eq(repository.path)
end
- it 'returns correct string for a ref containing dots' do
- prefix = repository.archive_prefix('test.branch', 'SHA', append_sha: nil)
-
- expect(prefix).to eq("#{project_name}-test.branch-SHA")
+ it 'sets CommitId to the commit SHA' do
+ expect(metadata['CommitId']).to eq(SeedRepo::LastCommit::ID)
end
- it 'returns string with sha when append_sha is false' do
- prefix = repository.archive_prefix('test.branch', 'SHA', append_sha: false)
-
- expect(prefix).to eq("#{project_name}-test.branch")
+ it 'sets ArchivePrefix to the expected prefix' do
+ expect(metadata['ArchivePrefix']).to eq(expected_prefix)
end
- end
- describe '#archive' do
- let(:metadata) { repository.archive_metadata('master', '/tmp', append_sha: true) }
+ it 'sets ArchivePath to the expected globally-unique path' do
+ # This is really important from a security perspective. Think carefully
+ # before changing it: https://gitlab.com/gitlab-org/gitlab-ce/issues/45689
+ expect(expected_path).to include(File.join(repository.gl_repository, SeedRepo::LastCommit::ID))
- it_should_behave_like 'archive check', '.tar.gz'
- end
-
- describe '#archive_zip' do
- let(:metadata) { repository.archive_metadata('master', '/tmp', 'zip', append_sha: true) }
+ expect(metadata['ArchivePath']).to eq(expected_path)
+ end
- it_should_behave_like 'archive check', '.zip'
- end
+ context 'append_sha varies archive path and filename' do
+ where(:append_sha, :ref, :expected_prefix) do
+ sha = SeedRepo::LastCommit::ID
- describe '#archive_bz2' do
- let(:metadata) { repository.archive_metadata('master', '/tmp', 'tbz2', append_sha: true) }
+ true | 'master' | "gitlab-git-test-master-#{sha}"
+ true | sha | "gitlab-git-test-#{sha}-#{sha}"
+ false | 'master' | "gitlab-git-test-master"
+ false | sha | "gitlab-git-test-#{sha}"
+ nil | 'master' | "gitlab-git-test-master-#{sha}"
+ nil | sha | "gitlab-git-test-#{sha}"
+ end
- it_should_behave_like 'archive check', '.tar.bz2'
- end
+ with_them do
+ it { expect(metadata['ArchivePrefix']).to eq(expected_prefix) }
+ it { expect(metadata['ArchivePath']).to eq(expected_path) }
+ end
+ end
- describe '#archive_fallback' do
- let(:metadata) { repository.archive_metadata('master', '/tmp', 'madeup', append_sha: true) }
+ context 'format varies archive path and filename' do
+ where(:format, :expected_extension) do
+ nil | 'tar.gz'
+ 'madeup' | 'tar.gz'
+ 'tbz2' | 'tar.bz2'
+ 'zip' | 'zip'
+ end
- it_should_behave_like 'archive check', '.tar.gz'
+ with_them do
+ it { expect(metadata['ArchivePrefix']).to eq(expected_prefix) }
+ it { expect(metadata['ArchivePath']).to eq(expected_path) }
+ end
+ end
end
describe '#size' do
diff --git a/spec/lib/omni_auth/strategies/jwt_spec.rb b/spec/lib/omni_auth/strategies/jwt_spec.rb
new file mode 100644
index 00000000000..23485fbcb18
--- /dev/null
+++ b/spec/lib/omni_auth/strategies/jwt_spec.rb
@@ -0,0 +1,87 @@
+require 'spec_helper'
+
+describe OmniAuth::Strategies::Jwt do
+ include Rack::Test::Methods
+ include DeviseHelpers
+
+ context '.decoded' do
+ let(:strategy) { described_class.new({}) }
+ let(:timestamp) { Time.now.to_i }
+ let(:jwt_config) { Devise.omniauth_configs[:jwt] }
+ let(:key) { JWT.encode(claims, jwt_config.strategy.secret) }
+
+ let(:claims) do
+ {
+ id: 123,
+ name: "user_example",
+ email: "user@example.com",
+ iat: timestamp
+ }
+ end
+
+ before do
+ allow_any_instance_of(OmniAuth::Strategy).to receive(:options).and_return(jwt_config.strategy)
+ allow_any_instance_of(Rack::Request).to receive(:params).and_return({ 'jwt' => key })
+ end
+
+ it 'decodes the user information' do
+ result = strategy.decoded
+
+ expect(result["id"]).to eq(123)
+ expect(result["name"]).to eq("user_example")
+ expect(result["email"]).to eq("user@example.com")
+ expect(result["iat"]).to eq(timestamp)
+ end
+
+ context 'required claims is missing' do
+ let(:claims) do
+ {
+ id: 123,
+ email: "user@example.com",
+ iat: timestamp
+ }
+ end
+
+ it 'raises error' do
+ expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid)
+ end
+ end
+
+ context 'when valid_within is specified but iat attribute is missing in response' do
+ let(:claims) do
+ {
+ id: 123,
+ name: "user_example",
+ email: "user@example.com"
+ }
+ end
+
+ before do
+ jwt_config.strategy.valid_within = Time.now.to_i
+ end
+
+ it 'raises error' do
+ expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid)
+ end
+ end
+
+ context 'when timestamp claim is too skewed from present' do
+ let(:claims) do
+ {
+ id: 123,
+ name: "user_example",
+ email: "user@example.com",
+ iat: timestamp - 10.minutes.to_i
+ }
+ end
+
+ before do
+ jwt_config.strategy.valid_within = 2.seconds
+ end
+
+ it 'raises error' do
+ expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid)
+ end
+ end
+ end
+end
diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb
index 5a3b5b1f517..ffc78015f94 100644
--- a/spec/models/members/group_member_spec.rb
+++ b/spec/models/members/group_member_spec.rb
@@ -28,52 +28,12 @@ describe GroupMember do
end
end
- describe 'notifications' do
- describe "#after_create" do
- it "sends email to user" do
- membership = build(:group_member)
+ it_behaves_like 'members notifications', :group
- allow(membership).to receive(:notification_service)
- .and_return(double('NotificationService').as_null_object)
- expect(membership).to receive(:notification_service)
+ describe '#real_source_type' do
+ subject { create(:group_member).real_source_type }
- membership.save
- end
- end
-
- describe "#after_update" do
- before do
- @group_member = create :group_member
- allow(@group_member).to receive(:notification_service)
- .and_return(double('NotificationService').as_null_object)
- end
-
- it "sends email to user" do
- expect(@group_member).to receive(:notification_service)
- @group_member.update_attribute(:access_level, GroupMember::MASTER)
- end
-
- it "does not send an email when the access level has not changed" do
- expect(@group_member).not_to receive(:notification_service)
- @group_member.update_attribute(:access_level, GroupMember::OWNER)
- end
- end
-
- describe '#after_accept_request' do
- it 'calls NotificationService.accept_group_access_request' do
- member = create(:group_member, user: build(:user), requested_at: Time.now)
-
- expect_any_instance_of(NotificationService).to receive(:new_group_member)
-
- member.__send__(:after_accept_request)
- end
- end
-
- describe '#real_source_type' do
- subject { create(:group_member).real_source_type }
-
- it { is_expected.to eq 'Group' }
- end
+ it { is_expected.to eq 'Group' }
end
describe '#update_two_factor_requirement' do
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index b8b0e63f92e..574eb468e4c 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -123,15 +123,5 @@ describe ProjectMember do
it { expect(@project_2.users).to be_empty }
end
- describe 'notifications' do
- describe '#after_accept_request' do
- it 'calls NotificationService.new_project_member' do
- member = create(:project_member, user: create(:user), requested_at: Time.now)
-
- expect_any_instance_of(NotificationService).to receive(:new_project_member)
-
- member.__send__(:after_accept_request)
- end
- end
- end
+ it_behaves_like 'members notifications', :project
end
diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb
index 4a44b219a67..ef34192f888 100644
--- a/spec/requests/api/discussions_spec.rb
+++ b/spec/requests/api/discussions_spec.rb
@@ -2,32 +2,53 @@ require 'spec_helper'
describe API::Discussions do
let(:user) { create(:user) }
- let!(:project) { create(:project, :public, namespace: user.namespace) }
+ let!(:project) { create(:project, :public, :repository, namespace: user.namespace) }
let(:private_user) { create(:user) }
before do
- project.add_reporter(user)
+ project.add_developer(user)
end
- context "when noteable is an Issue" do
+ context 'when noteable is an Issue' do
let!(:issue) { create(:issue, project: project, author: user) }
let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) }
- it_behaves_like "discussions API", 'projects', 'issues', 'iid' do
+ it_behaves_like 'discussions API', 'projects', 'issues', 'iid' do
let(:parent) { project }
let(:noteable) { issue }
let(:note) { issue_note }
end
end
- context "when noteable is a Snippet" do
+ context 'when noteable is a Snippet' do
let!(:snippet) { create(:project_snippet, project: project, author: user) }
let!(:snippet_note) { create(:discussion_note_on_snippet, noteable: snippet, project: project, author: user) }
- it_behaves_like "discussions API", 'projects', 'snippets', 'id' do
+ it_behaves_like 'discussions API', 'projects', 'snippets', 'id' do
let(:parent) { project }
let(:noteable) { snippet }
let(:note) { snippet_note }
end
end
+
+ context 'when noteable is a Merge Request' do
+ let!(:noteable) { create(:merge_request_with_diffs, source_project: project, target_project: project, author: user) }
+ let!(:note) { create(:discussion_note_on_merge_request, noteable: noteable, project: project, author: user) }
+ let!(:diff_note) { create(:diff_note_on_merge_request, noteable: noteable, project: project, author: user) }
+ let(:parent) { project }
+
+ it_behaves_like 'discussions API', 'projects', 'merge_requests', 'iid'
+ it_behaves_like 'diff discussions API', 'projects', 'merge_requests', 'iid'
+ it_behaves_like 'resolvable discussions API', 'projects', 'merge_requests', 'iid'
+ end
+
+ context 'when noteable is a Commit' do
+ let!(:noteable) { create(:commit, project: project, author: user) }
+ let!(:note) { create(:discussion_note_on_commit, commit_id: noteable.id, project: project, author: user) }
+ let!(:diff_note) { create(:diff_note_on_commit, commit_id: noteable.id, project: project, author: user) }
+ let(:parent) { project }
+
+ it_behaves_like 'discussions API', 'projects', 'repository/commits', 'id'
+ it_behaves_like 'diff discussions API', 'projects', 'repository/commits', 'id'
+ end
end
diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb
index 6bed8e812c0..cd1a6cfc427 100644
--- a/spec/requests/openid_connect_spec.rb
+++ b/spec/requests/openid_connect_spec.rb
@@ -153,4 +153,13 @@ describe 'OpenID Connect requests' do
end
end
end
+
+ context 'OpenID configuration information' do
+ it 'correctly returns the configuration' do
+ get '/.well-known/openid-configuration'
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response).to have_key('issuer')
+ end
+ end
end
diff --git a/spec/services/notes/resolve_service_spec.rb b/spec/services/notes/resolve_service_spec.rb
new file mode 100644
index 00000000000..b54d40a7a5c
--- /dev/null
+++ b/spec/services/notes/resolve_service_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Notes::ResolveService do
+ let(:merge_request) { create(:merge_request) }
+ let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) }
+ let(:user) { merge_request.author }
+
+ describe '#execute' do
+ it "resolves the note" do
+ described_class.new(merge_request.project, user).execute(note)
+ note.reload
+
+ expect(note.resolved?).to be true
+ expect(note.resolved_by).to eq(user)
+ end
+
+ it "sends notifications if all discussions are resolved" do
+ expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(merge_request)
+
+ described_class.new(merge_request.project, user).execute(note)
+ end
+ end
+end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 55bbe954491..48ef5f3c115 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -96,6 +96,37 @@ describe NotificationService, :mailer do
it_should_behave_like 'participating by assignee notification'
end
+ describe '#async' do
+ let(:async) { notification.async }
+ set(:key) { create(:personal_key) }
+
+ it 'returns an Async object with the correct parent' do
+ expect(async).to be_a(described_class::Async)
+ expect(async.parent).to eq(notification)
+ end
+
+ context 'when receiving a public method' do
+ it 'schedules a MailScheduler::NotificationServiceWorker' do
+ expect(MailScheduler::NotificationServiceWorker)
+ .to receive(:perform_async).with('new_key', key)
+
+ async.new_key(key)
+ end
+ end
+
+ context 'when receiving a private method' do
+ it 'raises NoMethodError' do
+ expect { async.notifiable?(key) }.to raise_error(NoMethodError)
+ end
+ end
+
+ context 'when recieving a non-existent method' do
+ it 'raises NoMethodError' do
+ expect { async.foo(key) }.to raise_error(NoMethodError)
+ end
+ end
+ end
+
describe 'Keys' do
describe '#new_key' do
let(:key_options) { {} }
@@ -982,6 +1013,8 @@ describe NotificationService, :mailer do
let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' }
before do
+ project.add_master(merge_request.author)
+ project.add_master(merge_request.assignee)
build_team(merge_request.target_project)
add_users_with_subscription(merge_request.target_project, merge_request)
update_custom_notification(:new_merge_request, @u_guest_custom, resource: project)
@@ -1093,15 +1126,18 @@ describe NotificationService, :mailer do
end
describe '#reassigned_merge_request' do
+ let(:current_user) { create(:user) }
+
before do
update_custom_notification(:reassign_merge_request, @u_guest_custom, resource: project)
update_custom_notification(:reassign_merge_request, @u_custom_global)
end
it do
- notification.reassigned_merge_request(merge_request, merge_request.author)
+ notification.reassigned_merge_request(merge_request, current_user, merge_request.author)
should_email(merge_request.assignee)
+ should_email(merge_request.author)
should_email(@u_watcher)
should_email(@u_participant_mentioned)
should_email(@subscriber)
@@ -1116,7 +1152,7 @@ describe NotificationService, :mailer do
end
it 'adds "assigned" reason for new assignee' do
- notification.reassigned_merge_request(merge_request, merge_request.author)
+ notification.reassigned_merge_request(merge_request, current_user, merge_request.author)
email = find_email_for(merge_request.assignee)
@@ -1126,7 +1162,7 @@ describe NotificationService, :mailer do
it_behaves_like 'participating notifications' do
let(:participant) { create(:user, username: 'user-participant') }
let(:issuable) { merge_request }
- let(:notification_trigger) { notification.reassigned_merge_request(merge_request, @u_disabled) }
+ let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, merge_request.author) }
end
end
diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb
index 1b6caeab15d..a418808fd26 100644
--- a/spec/services/projects/update_pages_service_spec.rb
+++ b/spec/services/projects/update_pages_service_spec.rb
@@ -29,25 +29,10 @@ describe Projects::UpdatePagesService do
end
describe 'pages artifacts' do
- context 'with expiry date' do
- before do
- build.artifacts_expire_in = "2 days"
- build.save!
- end
-
- it "doesn't delete artifacts" do
- expect(execute).to eq(:success)
-
- expect(build.reload.artifacts?).to eq(true)
- end
- end
-
- context 'without expiry date' do
- it "does delete artifacts" do
- expect(execute).to eq(:success)
+ it "doesn't delete artifacts after deploying" do
+ expect(execute).to eq(:success)
- expect(build.reload.artifacts?).to eq(false)
- end
+ expect(build.reload.artifacts?).to eq(true)
end
end
@@ -100,25 +85,10 @@ describe Projects::UpdatePagesService do
end
describe 'pages artifacts' do
- context 'with expiry date' do
- before do
- build.artifacts_expire_in = "2 days"
- build.save!
- end
-
- it "doesn't delete artifacts" do
- expect(execute).to eq(:success)
-
- expect(build.artifacts?).to eq(true)
- end
- end
-
- context 'without expiry date' do
- it "does delete artifacts" do
- expect(execute).to eq(:success)
+ it "doesn't delete artifacts after deploying" do
+ expect(execute).to eq(:success)
- expect(build.reload.artifacts?).to eq(false)
- end
+ expect(build.artifacts?).to eq(true)
end
end
@@ -171,13 +141,12 @@ describe Projects::UpdatePagesService do
build.reload
expect(deploy_status).to be_failed
- expect(build.artifacts?).to be_truthy
end
end
context 'when failed to extract zip artifacts' do
before do
- allow_any_instance_of(described_class)
+ expect_any_instance_of(described_class)
.to receive(:extract_zip_archive!)
.and_raise(Projects::UpdatePagesService::FailedToExtractError)
end
@@ -188,21 +157,19 @@ describe Projects::UpdatePagesService do
build.reload
expect(deploy_status).to be_failed
- expect(build.artifacts?).to be_truthy
end
end
context 'when missing artifacts metadata' do
before do
- allow(build).to receive(:artifacts_metadata?).and_return(false)
+ expect(build).to receive(:artifacts_metadata?).and_return(false)
end
- it 'does not raise an error and remove artifacts as failed job' do
+ it 'does not raise an error as failed job' do
execute
build.reload
expect(deploy_status).to be_failed
- expect(build.artifacts?).to be_falsey
end
end
end
diff --git a/spec/services/repository_archive_clean_up_service_spec.rb b/spec/services/repository_archive_clean_up_service_spec.rb
index 2d7fa3f80f7..ab1c638fc39 100644
--- a/spec/services/repository_archive_clean_up_service_spec.rb
+++ b/spec/services/repository_archive_clean_up_service_spec.rb
@@ -1,15 +1,47 @@
require 'spec_helper'
describe RepositoryArchiveCleanUpService do
- describe '#execute' do
- subject(:service) { described_class.new }
+ subject(:service) { described_class.new }
+ describe '#execute (new archive locations)' do
+ let(:sha) { "0" * 40 }
+
+ it 'removes outdated archives and directories in a new-style path' do
+ in_directory_with_files("project-999/#{sha}", %w[tar tar.bz2 tar.gz zip], 3.hours) do |dirname, files|
+ service.execute
+
+ files.each { |filename| expect(File.exist?(filename)).to be_falsy }
+ expect(File.directory?(dirname)).to be_falsy
+ expect(File.directory?(File.dirname(dirname))).to be_falsy
+ end
+ end
+
+ it 'does not remove directories when they contain outdated non-archives' do
+ in_directory_with_files("project-999/#{sha}", %w[tar conf rb], 2.hours) do |dirname, files|
+ service.execute
+
+ expect(File.directory?(dirname)).to be_truthy
+ end
+ end
+
+ it 'does not remove in-date archives in a new-style path' do
+ in_directory_with_files("project-999/#{sha}", %w[tar tar.bz2 tar.gz zip], 1.hour) do |dirname, files|
+ service.execute
+
+ files.each { |filename| expect(File.exist?(filename)).to be_truthy }
+ end
+ end
+ end
+
+ describe '#execute (legacy archive locations)' do
context 'when the downloads directory does not exist' do
it 'does not remove any archives' do
path = '/invalid/path/'
stub_repository_downloads_path(path)
+ allow(File).to receive(:directory?).and_call_original
expect(File).to receive(:directory?).with(path).and_return(false)
+
expect(service).not_to receive(:clean_up_old_archives)
expect(service).not_to receive(:clean_up_empty_directories)
@@ -19,7 +51,7 @@ describe RepositoryArchiveCleanUpService do
context 'when the downloads directory exists' do
shared_examples 'invalid archive files' do |dirname, extensions, mtime|
- it 'does not remove files and directoy' do
+ it 'does not remove files and directory' do
in_directory_with_files(dirname, extensions, mtime) do |dir, files|
service.execute
@@ -43,7 +75,7 @@ describe RepositoryArchiveCleanUpService do
end
context 'with files older than 2 hours inside invalid directories' do
- it_behaves_like 'invalid archive files', 'john_doe/sample.git', %w[conf rb tar tar.gz], 2.hours
+ it_behaves_like 'invalid archive files', 'john/doe/sample.git', %w[conf rb tar tar.gz], 2.hours
end
context 'with files newer than 2 hours that matches valid archive extensions' do
@@ -58,24 +90,24 @@ describe RepositoryArchiveCleanUpService do
it_behaves_like 'invalid archive files', 'sample.git', %w[conf rb tar tar.gz], 1.hour
end
end
+ end
- def in_directory_with_files(dirname, extensions, mtime)
- Dir.mktmpdir do |tmpdir|
- stub_repository_downloads_path(tmpdir)
- dir = File.join(tmpdir, dirname)
- files = create_temporary_files(dir, extensions, mtime)
+ def in_directory_with_files(dirname, extensions, mtime)
+ Dir.mktmpdir do |tmpdir|
+ stub_repository_downloads_path(tmpdir)
+ dir = File.join(tmpdir, dirname)
+ files = create_temporary_files(dir, extensions, mtime)
- yield(dir, files)
- end
+ yield(dir, files)
end
+ end
- def stub_repository_downloads_path(path)
- allow(Gitlab.config.gitlab).to receive(:repository_downloads_path).and_return(path)
- end
+ def stub_repository_downloads_path(path)
+ allow(Gitlab.config.gitlab).to receive(:repository_downloads_path).and_return(path)
+ end
- def create_temporary_files(dir, extensions, mtime)
- FileUtils.mkdir_p(dir)
- FileUtils.touch(extensions.map { |ext| File.join(dir, "sample.#{ext}") }, mtime: Time.now - mtime)
- end
+ def create_temporary_files(dir, extensions, mtime)
+ FileUtils.mkdir_p(dir)
+ FileUtils.touch(extensions.map { |ext| File.join(dir, "sample.#{ext}") }, mtime: Time.now - mtime)
end
end
diff --git a/spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb b/spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb
new file mode 100644
index 00000000000..b29bb3c2fc0
--- /dev/null
+++ b/spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb
@@ -0,0 +1,52 @@
+RSpec.shared_examples 'Master manages access requests' do
+ let(:user) { create(:user) }
+ let(:master) { create(:user) }
+
+ before do
+ entity.request_access(user)
+ entity.respond_to?(:add_owner) ? entity.add_owner(master) : entity.add_master(master)
+ sign_in(master)
+ end
+
+ it 'master can see access requests' do
+ visit members_page_path
+
+ expect_visible_access_request(entity, user)
+ end
+
+ it 'master can grant access', :js do
+ visit members_page_path
+
+ expect_visible_access_request(entity, user)
+
+ accept_confirm { click_on 'Grant access' }
+
+ expect_no_visible_access_request(entity, user)
+
+ page.within('.members-list') do
+ expect(page).to have_content user.name
+ end
+ end
+
+ it 'master can deny access', :js do
+ visit members_page_path
+
+ expect_visible_access_request(entity, user)
+
+ accept_confirm { click_on 'Deny access' }
+
+ expect_no_visible_access_request(entity, user)
+ expect(page).not_to have_content user.name
+ end
+
+ def expect_visible_access_request(entity, user)
+ expect(entity.requesters.exists?(user_id: user)).to be_truthy
+ expect(page).to have_content "Users requesting access to #{entity.name} 1"
+ expect(page).to have_content user.name
+ end
+
+ def expect_no_visible_access_request(entity, user)
+ expect(entity.requesters.exists?(user_id: user)).to be_falsy
+ expect(page).not_to have_content "Users requesting access to #{entity.name}"
+ end
+end
diff --git a/spec/support/shared_examples/models/members_notifications_shared_example.rb b/spec/support/shared_examples/models/members_notifications_shared_example.rb
new file mode 100644
index 00000000000..76611e54306
--- /dev/null
+++ b/spec/support/shared_examples/models/members_notifications_shared_example.rb
@@ -0,0 +1,63 @@
+RSpec.shared_examples 'members notifications' do |entity_type|
+ let(:notification_service) { double('NotificationService').as_null_object }
+
+ before do
+ allow(member).to receive(:notification_service).and_return(notification_service)
+ end
+
+ describe "#after_create" do
+ let(:member) { build(:"#{entity_type}_member") }
+
+ it "sends email to user" do
+ expect(notification_service).to receive(:"new_#{entity_type}_member").with(member)
+
+ member.save
+ end
+ end
+
+ describe "#after_update" do
+ let(:member) { create(:"#{entity_type}_member", :developer) }
+
+ it "calls NotificationService.update_#{entity_type}_member" do
+ expect(notification_service).to receive(:"update_#{entity_type}_member").with(member)
+
+ member.update_attribute(:access_level, Member::MASTER)
+ end
+
+ it "does not send an email when the access level has not changed" do
+ expect(notification_service).not_to receive(:"update_#{entity_type}_member")
+
+ member.touch
+ end
+ end
+
+ describe '#accept_request' do
+ let(:member) { create(:"#{entity_type}_member", :access_request) }
+
+ it "calls NotificationService.new_#{entity_type}_member" do
+ expect(notification_service).to receive(:"new_#{entity_type}_member").with(member)
+
+ member.accept_request
+ end
+ end
+
+ describe "#accept_invite!" do
+ let(:member) { create(:"#{entity_type}_member", :invited) }
+
+ it "calls NotificationService.accept_#{entity_type}_invite" do
+ expect(notification_service).to receive(:"accept_#{entity_type}_invite").with(member)
+
+ member.accept_invite!(build(:user))
+ end
+ end
+
+ describe "#decline_invite!" do
+ let(:member) { create(:"#{entity_type}_member", :invited) }
+
+ it "calls NotificationService.decline_#{entity_type}_invite" do
+ expect(notification_service).to receive(:"decline_#{entity_type}_invite").with(member)
+
+ member.decline_invite!
+ end
+ end
+end
diff --git a/spec/support/shared_examples/requests/api/diff_discussions.rb b/spec/support/shared_examples/requests/api/diff_discussions.rb
new file mode 100644
index 00000000000..85a4bd8ca27
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/diff_discussions.rb
@@ -0,0 +1,57 @@
+shared_examples 'diff discussions API' do |parent_type, noteable_type, id_name|
+ describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
+ it "includes diff discussions" do
+ get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user)
+
+ discussion = json_response.find { |record| record['id'] == diff_note.discussion_id }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(discussion).not_to be_nil
+ expect(discussion['individual_note']).to eq(false)
+ expect(discussion['notes'].first['body']).to eq(diff_note.note)
+ end
+ end
+
+ describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do
+ it "returns a discussion by id" do
+ get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{diff_note.discussion_id}", user)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['id']).to eq(diff_note.discussion_id)
+ expect(json_response['notes'].first['body']).to eq(diff_note.note)
+ expect(json_response['notes'].first['position']).to eq(diff_note.position.to_h.stringify_keys)
+ end
+ end
+
+ describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
+ it "creates a new diff note" do
+ position = diff_note.position.to_h
+
+ post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!', position: position
+
+ expect(response).to have_gitlab_http_status(201)
+ expect(json_response['notes'].first['body']).to eq('hi!')
+ expect(json_response['notes'].first['type']).to eq('DiffNote')
+ expect(json_response['notes'].first['position']).to eq(position.stringify_keys)
+ end
+
+ it "returns a 400 bad request error when position is invalid" do
+ position = diff_note.position.to_h.merge(new_line: '100000')
+
+ post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!', position: position
+
+ expect(response).to have_gitlab_http_status(400)
+ end
+ end
+
+ describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do
+ it 'adds a new note to the diff discussion' do
+ post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{diff_note.discussion_id}/notes", user), body: 'hi!'
+
+ expect(response).to have_gitlab_http_status(201)
+ expect(json_response['body']).to eq('hi!')
+ expect(json_response['type']).to eq('DiffNote')
+ end
+ end
+end
diff --git a/spec/support/shared_examples/requests/api/resolvable_discussions.rb b/spec/support/shared_examples/requests/api/resolvable_discussions.rb
new file mode 100644
index 00000000000..408ad08cc48
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/resolvable_discussions.rb
@@ -0,0 +1,87 @@
+shared_examples 'resolvable discussions API' do |parent_type, noteable_type, id_name|
+ describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do
+ it "resolves discussion if resolved is true" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", user), resolved: true
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['notes'].size).to eq(1)
+ expect(json_response['notes'][0]['resolved']).to eq(true)
+ end
+
+ it "unresolves discussion if resolved is false" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", user), resolved: false
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['notes'].size).to eq(1)
+ expect(json_response['notes'][0]['resolved']).to eq(false)
+ end
+
+ it "returns a 400 bad request error if resolved parameter is not passed" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", user)
+
+ expect(response).to have_gitlab_http_status(400)
+ end
+
+ it "returns a 401 unauthorized error if user is not authenticated" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}"), resolved: true
+
+ expect(response).to have_gitlab_http_status(401)
+ end
+
+ it "returns a 403 error if user resolves discussion of someone else" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", private_user), resolved: true
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+
+ context 'when user does not have access to read the discussion' do
+ before do
+ parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+ end
+
+ it 'responds with 404' do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", private_user), resolved: true
+
+ expect(response).to have_gitlab_http_status(404)
+ end
+ end
+ end
+
+ describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
+ it 'returns resolved note when resolved parameter is true' do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}/notes/#{note.id}", user), resolved: true
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['resolved']).to eq(true)
+ end
+
+ it 'returns a 404 error when note id not found' do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}/notes/12345", user),
+ body: 'Hello!'
+
+ expect(response).to have_gitlab_http_status(404)
+ end
+
+ it 'returns a 400 bad request error if neither body nor resolved parameter is given' do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}/notes/#{note.id}", user)
+
+ expect(response).to have_gitlab_http_status(400)
+ end
+
+ it "returns a 403 error if user resolves note of someone else" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}/notes/#{note.id}", private_user), resolved: true
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+end
diff --git a/spec/workers/mail_scheduler/issue_due_worker_spec.rb b/spec/workers/mail_scheduler/issue_due_worker_spec.rb
index 48ac1b8a1a4..1026ae5b4bf 100644
--- a/spec/workers/mail_scheduler/issue_due_worker_spec.rb
+++ b/spec/workers/mail_scheduler/issue_due_worker_spec.rb
@@ -12,8 +12,8 @@ describe MailScheduler::IssueDueWorker do
create(:issue, :opened, project: project, due_date: 2.days.from_now) # due on another day
create(:issue, :opened, due_date: Date.tomorrow) # different project
- expect_any_instance_of(NotificationService).to receive(:issue_due).with(issue1)
- expect_any_instance_of(NotificationService).to receive(:issue_due).with(issue2)
+ expect(worker.notification_service).to receive(:issue_due).with(issue1)
+ expect(worker.notification_service).to receive(:issue_due).with(issue2)
worker.perform(project.id)
end
diff --git a/spec/workers/mail_scheduler/notification_service_worker_spec.rb b/spec/workers/mail_scheduler/notification_service_worker_spec.rb
new file mode 100644
index 00000000000..f725c8763a0
--- /dev/null
+++ b/spec/workers/mail_scheduler/notification_service_worker_spec.rb
@@ -0,0 +1,44 @@
+require 'spec_helper'
+
+describe MailScheduler::NotificationServiceWorker do
+ let(:worker) { described_class.new }
+ let(:method) { 'new_key' }
+ set(:key) { create(:personal_key) }
+
+ def serialize(*args)
+ ActiveJob::Arguments.serialize(args)
+ end
+
+ describe '#perform' do
+ it 'deserializes arguments from global IDs' do
+ expect(worker.notification_service).to receive(method).with(key)
+
+ worker.perform(method, *serialize(key))
+ end
+
+ context 'when the arguments cannot be deserialized' do
+ it 'does nothing' do
+ expect(worker.notification_service).not_to receive(method)
+
+ worker.perform(method, key.to_global_id.to_s.succ)
+ end
+ end
+
+ context 'when the method is not a public method' do
+ it 'raises NoMethodError' do
+ expect { worker.perform('notifiable?', *serialize(key)) }.to raise_error(NoMethodError)
+ end
+ end
+ end
+
+ describe '.perform_async' do
+ it 'serializes arguments as global IDs when scheduling' do
+ Sidekiq::Testing.fake! do
+ described_class.perform_async(method, key)
+
+ expect(described_class.jobs.count).to eq(1)
+ expect(described_class.jobs.first).to include('args' => [method, *serialize(key)])
+ end
+ end
+ end
+end
diff --git a/spec/workers/namespaceless_project_destroy_worker_spec.rb b/spec/workers/namespaceless_project_destroy_worker_spec.rb
index 479d9396eca..eec110dfbfb 100644
--- a/spec/workers/namespaceless_project_destroy_worker_spec.rb
+++ b/spec/workers/namespaceless_project_destroy_worker_spec.rb
@@ -22,13 +22,11 @@ describe NamespacelessProjectDestroyWorker do
end
end
- # Only possible with schema 20180222043024 and lower.
- # Project#namespace_id has not null constraint since then
- context 'project has no namespace', :migration, schema: 20180222043024 do
- let!(:project) do
- project = build(:project, namespace_id: nil)
- project.save(validate: false)
- project
+ context 'project has no namespace' do
+ let!(:project) { create(:project) }
+
+ before do
+ allow_any_instance_of(Project).to receive(:namespace).and_return(nil)
end
context 'project not a fork of another project' do
@@ -61,8 +59,7 @@ describe NamespacelessProjectDestroyWorker do
let!(:parent_project) { create(:project) }
let(:project) do
namespaceless_project = fork_project(parent_project)
- namespaceless_project.namespace_id = nil
- namespaceless_project.save(validate: false)
+ namespaceless_project.save
namespaceless_project
end