diff options
Diffstat (limited to 'spec')
4 files changed, 228 insertions, 162 deletions
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index d00cd8f1d6b..36e44a9b941 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe GroupsHelper do include ApplicationHelper + include AvatarsHelper describe '#group_icon_url' do it 'returns an url for the avatar' do @@ -135,6 +136,37 @@ RSpec.describe GroupsHelper do end end + describe '#group_title_link' do + let_it_be(:group) { create(:group, :with_avatar) } + + let(:raw_link) { group_title_link(group, show_avatar: true) } + let(:document) { Nokogiri::HTML.parse(raw_link) } + + describe 'link' do + subject(:link) { document.css('.group-path').first } + + it 'uses the group name as innerText' do + expect(link.inner_text).to eq(group.name) + end + + it 'links to the group path' do + expect(link.attr('href')).to eq(group_path(group)) + end + end + + describe 'icon' do + subject(:icon) { document.css('.avatar-tile').first } + + it 'specifies the group name as the alt text' do + expect(icon.attr('alt')).to eq(group.name) + end + + it 'uses the group\'s avatar_url' do + expect(icon.attr('src')).to eq(group.avatar_url) + end + end + end + describe '#share_with_group_lock_help_text' do context 'traversal queries' do let_it_be_with_reload(:root_group) { create(:group) } diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 1e869df0988..c2d64aa2fb3 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -87,7 +87,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end context 'when IP is already banned' do - subject { gl_auth.find_for_git_client('username', 'password', project: nil, ip: 'ip') } + subject { gl_auth.find_for_git_client('username-does-not-matter', 'password-does-not-matter', project: nil, ip: 'ip') } before do expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter| @@ -219,16 +219,16 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end it 'recognizes master passwords' do - user = create(:user, password: 'password') + user = create(:user) - expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) + expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) end include_examples 'user login operation with unique ip limit' do - let(:user) { create(:user, password: 'password') } + let(:user) { create(:user) } def operation - expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) + expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) end end @@ -502,8 +502,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do user = create( :user, :blocked, - username: 'normal_user', - password: 'my-secret' + username: 'normal_user' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -512,7 +511,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when 2fa is enabled globally' do let_it_be(:user) do - create(:user, username: 'normal_user', password: 'my-secret', otp_grace_period_started_at: 1.day.ago) + create(:user, username: 'normal_user', otp_grace_period_started_at: 1.day.ago) end before do @@ -536,7 +535,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when 2fa is enabled personally' do let(:user) do - create(:user, :two_factor, username: 'normal_user', password: 'my-secret', otp_grace_period_started_at: 1.day.ago) + create(:user, :two_factor, username: 'normal_user', otp_grace_period_started_at: 1.day.ago) end it 'fails' do @@ -548,8 +547,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'goes through lfs authentication' do user = create( :user, - username: 'normal_user', - password: 'my-secret' + username: 'normal_user' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -559,8 +557,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'goes through oauth authentication when the username is oauth2' do user = create( :user, - username: 'oauth2', - password: 'my-secret' + username: 'oauth2' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -635,7 +632,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when deploy token and user have the same username' do let(:username) { 'normal_user' } - let(:user) { create(:user, username: username, password: 'my-secret') } + let(:user) { create(:user, username: username) } let(:deploy_token) { create(:deploy_token, username: username, read_registry: false, projects: [project]) } it 'succeeds for the token' do @@ -648,7 +645,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'succeeds for the user' do auth_success = { actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities } - expect(gl_auth.find_for_git_client(username, 'my-secret', project: project, ip: 'ip')) + expect(gl_auth.find_for_git_client(username, user.password, project: project, ip: 'ip')) .to have_attributes(auth_success) end end @@ -834,72 +831,64 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end describe 'find_with_user_password' do - let!(:user) do - create(:user, - username: username, - password: password, - password_confirmation: password) - end - + let!(:user) { create(:user, username: username) } let(:username) { 'John' } # username isn't lowercase, test this - let(:password) { 'my-secret' } it "finds user by valid login/password" do - expect(gl_auth.find_with_user_password(username, password)).to eql user + expect(gl_auth.find_with_user_password(username, user.password)).to eql user end it 'finds user by valid email/password with case-insensitive email' do - expect(gl_auth.find_with_user_password(user.email.upcase, password)).to eql user + expect(gl_auth.find_with_user_password(user.email.upcase, user.password)).to eql user end it 'finds user by valid username/password with case-insensitive username' do - expect(gl_auth.find_with_user_password(username.upcase, password)).to eql user + expect(gl_auth.find_with_user_password(username.upcase, user.password)).to eql user end it "does not find user with invalid password" do - password = 'wrong' - expect(gl_auth.find_with_user_password(username, password)).not_to eql user + expect(gl_auth.find_with_user_password(username, 'incorrect_password')).not_to eql user end it "does not find user with invalid login" do - user = 'wrong' - expect(gl_auth.find_with_user_password(username, password)).not_to eql user + username = 'wrong' + expect(gl_auth.find_with_user_password(username, user.password)).not_to eql user end include_examples 'user login operation with unique ip limit' do def operation - expect(gl_auth.find_with_user_password(username, password)).to eq(user) + expect(gl_auth.find_with_user_password(username, user.password)).to eq(user) end end it 'finds the user in deactivated state' do user.deactivate! - expect(gl_auth.find_with_user_password(username, password)).to eql user + expect(gl_auth.find_with_user_password(username, user.password)).to eql user end it "does not find user in blocked state" do user.block - expect(gl_auth.find_with_user_password(username, password)).not_to eql user + expect(gl_auth.find_with_user_password(username, user.password)).not_to eql user end it 'does not find user in locked state' do user.lock_access! - expect(gl_auth.find_with_user_password(username, password)).not_to eql user + expect(gl_auth.find_with_user_password(username, user.password)).not_to eql user end it "does not find user in ldap_blocked state" do user.ldap_block - expect(gl_auth.find_with_user_password(username, password)).not_to eql user + expect(gl_auth.find_with_user_password(username, user.password)).not_to eql user end it 'does not find user in blocked_pending_approval state' do user.block_pending_approval - expect(gl_auth.find_with_user_password(username, password)).not_to eql user + expect(gl_auth.find_with_user_password(username, user.password)).not_to eql user end context 'with increment_failed_attempts' do @@ -917,7 +906,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do user.save! expect do - gl_auth.find_with_user_password(username, password, increment_failed_attempts: true) + gl_auth.find_with_user_password(username, user.password, increment_failed_attempts: true) user.reload end.to change(user, :failed_attempts).from(2).to(0) end @@ -946,7 +935,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do user.save! expect do - gl_auth.find_with_user_password(username, password, increment_failed_attempts: true) + gl_auth.find_with_user_password(username, user.password, increment_failed_attempts: true) user.reload end.not_to change(user, :failed_attempts) end @@ -961,7 +950,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it "tries to autheticate with db before ldap" do expect(Gitlab::Auth::Ldap::Authentication).not_to receive(:login) - expect(gl_auth.find_with_user_password(username, password)).to eq(user) + expect(gl_auth.find_with_user_password(username, user.password)).to eq(user) end it "does not find user by using ldap as fallback to for authentication" do @@ -983,7 +972,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end it "does not find user by valid login/password" do - expect(gl_auth.find_with_user_password(username, password)).to be_nil + expect(gl_auth.find_with_user_password(username, user.password)).to be_nil end context "with ldap enabled" do @@ -992,7 +981,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end it "does not find non-ldap user by valid login/password" do - expect(gl_auth.find_with_user_password(username, password)).to be_nil + expect(gl_auth.find_with_user_password(username, user.password)).to be_nil end end end diff --git a/spec/requests/api/graphql/project/error_tracking/sentry_detailed_error_request_spec.rb b/spec/requests/api/graphql/project/error_tracking/sentry_detailed_error_request_spec.rb index 2b85704f479..2fe5fb593fe 100644 --- a/spec/requests/api/graphql/project/error_tracking/sentry_detailed_error_request_spec.rb +++ b/spec/requests/api/graphql/project/error_tracking/sentry_detailed_error_request_spec.rb @@ -34,6 +34,8 @@ RSpec.describe 'getting a detailed sentry error' do context 'when data is loading via reactive cache' do before do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + post_graphql(query, current_user: current_user) end @@ -48,6 +50,10 @@ RSpec.describe 'getting a detailed sentry error' do .to receive(:issue_details) .and_return({ issue: sentry_detailed_error }) + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with('error_tracking_view_details', values: current_user.id) + post_graphql(query, current_user: current_user) end diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb index c761af86c16..439d3d8c64e 100644 --- a/spec/requests/projects/merge_requests_discussions_spec.rb +++ b/spec/requests/projects/merge_requests_discussions_spec.rb @@ -16,9 +16,16 @@ RSpec.describe 'merge requests discussions' do login_as(user) end + # rubocop:disable RSpec/InstanceVariable def send_request - get discussions_namespace_project_merge_request_path(namespace_id: project.namespace, project_id: project, id: merge_request.iid) + get( + discussions_namespace_project_merge_request_path(namespace_id: project.namespace, project_id: project, id: merge_request.iid), + headers: { 'If-None-Match' => @etag } + ) + + @etag = response.etag end + # rubocop:enable RSpec/InstanceVariable it 'returns 200' do send_request @@ -63,11 +70,6 @@ RSpec.describe 'merge requests discussions' do let!(:award_emoji) { create(:award_emoji, awardable: first_note) } let!(:author_membership) { project.add_maintainer(author) } - before do - # Make a request to cache the discussions - send_request - end - shared_examples 'cache miss' do it 'does not hit a warm cache' do expect_next_instance_of(DiscussionSerializer) do |serializer| @@ -80,176 +82,213 @@ RSpec.describe 'merge requests discussions' do end end - it 'gets cached on subsequent requests' do - expect_next_instance_of(DiscussionSerializer) do |serializer| - expect(serializer).not_to receive(:represent) - end + shared_examples 'cache hit' do + it 'gets cached on subsequent requests' do + expect_next_instance_of(DiscussionSerializer) do |serializer| + expect(serializer).not_to receive(:represent) + end - send_request + send_request + end end - context 'when a note in a discussion got updated' do + context 'when mr_discussions_http_cache and disabled_mr_discussions_redis_cache are enabled' do before do - first_note.update!(updated_at: 1.minute.from_now) + send_request end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } - end - end + it_behaves_like 'cache hit' - context 'when a note in a discussion got its reference state updated' do - before do - reference.close! - end + context 'when a note in a discussion got updated' do + before do + first_note.update!(updated_at: 1.minute.from_now) + end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - end - context 'when a note in a discussion got resolved' do - before do - travel_to(1.minute.from_now) do - first_note.resolve!(user) + context 'when a note in a discussion got its reference state updated' do + before do + reference.close! end - end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - end - context 'when a note is added to a discussion' do - let!(:third_note) { create(:diff_note_on_merge_request, in_reply_to: first_note, noteable: merge_request, project: project) } + context 'when a note in a discussion got resolved' do + before do + travel_to(1.minute.from_now) do + first_note.resolve!(user) + end + end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note, third_note] } + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - end - context 'when a note is removed from a discussion' do - before do - second_note.destroy! - end + context 'when a note is added to a discussion' do + let!(:third_note) { create(:diff_note_on_merge_request, in_reply_to: first_note, noteable: merge_request, project: project) } - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note] } + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note, third_note] } + end end - end - context 'when an emoji is awarded to a note in discussion' do - before do - travel_to(1.minute.from_now) do - create(:award_emoji, awardable: first_note) + context 'when a note is removed from a discussion' do + before do + second_note.destroy! end - end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note] } + end end - end - context 'when an award emoji is removed from a note in discussion' do - before do - travel_to(1.minute.from_now) do - award_emoji.destroy! + context 'when an emoji is awarded to a note in discussion' do + before do + travel_to(1.minute.from_now) do + create(:award_emoji, awardable: first_note) + end + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } end end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } + context 'when an award emoji is removed from a note in discussion' do + before do + travel_to(1.minute.from_now) do + award_emoji.destroy! + end + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - end - context 'when the diff note position changes' do - before do - # This replicates a position change wherein timestamps aren't updated - # which is why `Gitlab::Timeless.timeless` is utilized. This is the - # same approach being used in Discussions::UpdateDiffPositionService - # which is responsible for updating the positions of diff discussions - # when MR updates. - first_note.position = Gitlab::Diff::Position.new( - old_path: first_note.position.old_path, - new_path: first_note.position.new_path, - old_line: first_note.position.old_line, - new_line: first_note.position.new_line + 1, - diff_refs: first_note.position.diff_refs - ) - - Gitlab::Timeless.timeless(first_note, &:save) + context 'when the diff note position changes' do + before do + # This replicates a position change wherein timestamps aren't updated + # which is why `Gitlab::Timeless.timeless` is utilized. This is the + # same approach being used in Discussions::UpdateDiffPositionService + # which is responsible for updating the positions of diff discussions + # when MR updates. + first_note.position = Gitlab::Diff::Position.new( + old_path: first_note.position.old_path, + new_path: first_note.position.new_path, + old_line: first_note.position.old_line, + new_line: first_note.position.new_line + 1, + diff_refs: first_note.position.diff_refs + ) + + Gitlab::Timeless.timeless(first_note, &:save) + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } + context 'when the HEAD diff note position changes' do + before do + # This replicates a DiffNotePosition change. This is the same approach + # being used in Discussions::CaptureDiffNotePositionService which is + # responsible for updating/creating DiffNotePosition of a diff discussions + # in relation to HEAD diff. + new_position = Gitlab::Diff::Position.new( + old_path: first_note.position.old_path, + new_path: first_note.position.new_path, + old_line: first_note.position.old_line, + new_line: first_note.position.new_line + 1, + diff_refs: first_note.position.diff_refs + ) + + DiffNotePosition.create_or_update_for( + first_note, + diff_type: :head, + position: new_position, + line_code: 'bd4b7bfff3a247ccf6e3371c41ec018a55230bcc_534_521' + ) + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - end - context 'when the HEAD diff note position changes' do - before do - # This replicates a DiffNotePosition change. This is the same approach - # being used in Discussions::CaptureDiffNotePositionService which is - # responsible for updating/creating DiffNotePosition of a diff discussions - # in relation to HEAD diff. - new_position = Gitlab::Diff::Position.new( - old_path: first_note.position.old_path, - new_path: first_note.position.new_path, - old_line: first_note.position.old_line, - new_line: first_note.position.new_line + 1, - diff_refs: first_note.position.diff_refs - ) - - DiffNotePosition.create_or_update_for( - first_note, - diff_type: :head, - position: new_position, - line_code: 'bd4b7bfff3a247ccf6e3371c41ec018a55230bcc_534_521' - ) + context 'when author detail changes' do + before do + author.update!(name: "#{author.name} (Updated)") + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } + context 'when author status changes' do + before do + Users::SetStatusService.new(author, message: "updated status").execute + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - end - context 'when author detail changes' do - before do - author.update!(name: "#{author.name} (Updated)") + context 'when author role changes' do + before do + Members::UpdateService.new(owner, access_level: Gitlab::Access::GUEST).execute(author_membership) + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } + context 'when current_user role changes' do + before do + Members::UpdateService.new(owner, access_level: Gitlab::Access::GUEST).execute(project.member(user)) + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end end end - context 'when author status changes' do + context 'when mr_discussions_http_cache is disabled' do before do - Users::SetStatusService.new(author, message: "updated status").execute + stub_feature_flags(mr_discussions_http_cache: false) + send_request end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } - end + it_behaves_like 'cache hit' end - context 'when author role changes' do + context 'when disabled_mr_discussions_redis_cache is disabled' do before do - Members::UpdateService.new(owner, access_level: Gitlab::Access::GUEST).execute(author_membership) + stub_feature_flags(disabled_mr_discussions_redis_cache: false) + send_request end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } - end + it_behaves_like 'cache hit' end - context 'when current_user role changes' do + context 'when mr_discussions_http_cache and disabled_mr_discussions_redis_cache are disabled' do before do - Members::UpdateService.new(owner, access_level: Gitlab::Access::GUEST).execute(project.member(user)) + stub_feature_flags(mr_discussions_http_cache: false, disabled_mr_discussions_redis_cache: false) + send_request end - it_behaves_like 'cache miss' do - let(:changed_notes) { [first_note, second_note] } - end + it_behaves_like 'cache hit' end end end |