diff options
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/import/github_controller_spec.rb | 7 | ||||
-rw-r--r-- | spec/controllers/uploads_controller_spec.rb | 296 | ||||
-rw-r--r-- | spec/features/security/dashboard_access_spec.rb | 12 | ||||
-rw-r--r-- | spec/features/security/profile_access_spec.rb | 107 | ||||
-rw-r--r-- | spec/helpers/application_helper_spec.rb | 18 | ||||
-rw-r--r-- | spec/helpers/events_helper_spec.rb | 12 | ||||
-rw-r--r-- | spec/helpers/gitlab_markdown_helper_spec.rb | 54 | ||||
-rw-r--r-- | spec/helpers/groups_helper.rb | 21 | ||||
-rw-r--r-- | spec/lib/gitlab/ldap/access_spec.rb | 18 | ||||
-rw-r--r-- | spec/lib/gitlab/reference_extractor_spec.rb | 19 | ||||
-rw-r--r-- | spec/mailers/notify_spec.rb | 144 | ||||
-rw-r--r-- | spec/models/project_services/hipchat_service_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/project_services/slack_service/push_message_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 23 | ||||
-rw-r--r-- | spec/services/git_push_service_spec.rb | 9 | ||||
-rw-r--r-- | spec/support/repo_helpers.rb | 19 |
16 files changed, 661 insertions, 106 deletions
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index b8820413406..5b967bfcc0c 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -27,17 +27,20 @@ describe Import::GithubController do describe "GET status" do before do @repo = OpenStruct.new(login: 'vim', full_name: 'asd/vim') + @org = OpenStruct.new(login: 'company') + @org_repo = OpenStruct.new(login: 'company', full_name: 'company/repo') end it "assigns variables" do @project = create(:project, import_type: 'github', creator_id: user.id) controller.stub_chain(:client, :repos).and_return([@repo]) - controller.stub_chain(:client, :orgs).and_return([]) + controller.stub_chain(:client, :orgs).and_return([@org]) + controller.stub_chain(:client, :org_repos).with(@org.login).and_return([@org_repo]) get :status expect(assigns(:already_added_projects)).to eq([@project]) - expect(assigns(:repos)).to eq([@repo]) + expect(assigns(:repos)).to eq([@repo, @org_repo]) end it "does not show already added project" do diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb new file mode 100644 index 00000000000..0f9780356b1 --- /dev/null +++ b/spec/controllers/uploads_controller_spec.rb @@ -0,0 +1,296 @@ +require 'spec_helper' + +describe UploadsController do + let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } + + describe "GET show" do + context "when viewing a user avatar" do + context "when signed in" do + before do + sign_in(user) + end + + context "when the user is blocked" do + before do + user.block + end + + it "redirects to the sign in page" do + get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "image.png" + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when the user isn't blocked" do + it "responds with status 200" do + get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + end + + context "when not signed in" do + it "responds with status 200" do + get :show, model: "user", mounted_as: "avatar", id: user.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + end + + context "when viewing a project avatar" do + let!(:project) { create(:project, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } + + context "when the project is public" do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + context "when not signed in" do + it "responds with status 200" do + get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + it "responds with status 200" do + get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + end + + context "when the project is private" do + before do + project.update_attribute(:visibility_level, Project::PRIVATE) + end + + context "when not signed in" do + it "redirects to the sign in page" do + get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png" + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + context "when the user has access to the project" do + before do + project.team << [user, :master] + end + + context "when the user is blocked" do + before do + user.block + project.team << [user, :master] + end + + it "redirects to the sign in page" do + get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png" + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when the user isn't blocked" do + it "responds with status 200" do + get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + end + + context "when the user doesn't have access to the project" do + it "responds with status 404" do + get :show, model: "project", mounted_as: "avatar", id: project.id, filename: "image.png" + + expect(response.status).to eq(404) + end + end + end + end + end + + context "when viewing a group avatar" do + let!(:group) { create(:group, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } + let!(:project) { create(:project, namespace: group) } + + context "when the group has public projects" do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + context "when not signed in" do + it "responds with status 200" do + get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + it "responds with status 200" do + get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + end + + context "when the project doesn't have public projects" do + context "when not signed in" do + it "redirects to the sign in page" do + get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png" + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + context "when the user has access to the project" do + before do + project.team << [user, :master] + end + + context "when the user is blocked" do + before do + user.block + project.team << [user, :master] + end + + it "redirects to the sign in page" do + get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png" + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when the user isn't blocked" do + it "responds with status 200" do + get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + end + + context "when the user doesn't have access to the project" do + it "responds with status 404" do + get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png" + + expect(response.status).to eq(404) + end + end + end + end + end + + context "when viewing a note attachment" do + let!(:note) { create(:note, :with_attachment) } + let(:project) { note.project } + + context "when the project is public" do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + context "when not signed in" do + it "responds with status 200" do + get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + it "responds with status 200" do + get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + end + + context "when the project is private" do + before do + project.update_attribute(:visibility_level, Project::PRIVATE) + end + + context "when not signed in" do + it "redirects to the sign in page" do + get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png" + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when signed in" do + before do + sign_in(user) + end + + context "when the user has access to the project" do + before do + project.team << [user, :master] + end + + context "when the user is blocked" do + before do + user.block + project.team << [user, :master] + end + + it "redirects to the sign in page" do + get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png" + + expect(response).to redirect_to(new_user_session_path) + end + end + + context "when the user isn't blocked" do + it "responds with status 200" do + get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png" + + expect(response.status).to eq(200) + end + end + end + + context "when the user doesn't have access to the project" do + it "responds with status 404" do + get :show, model: "note", mounted_as: "attachment", id: note.id, filename: "image.png" + + expect(response.status).to eq(404) + end + end + end + end + end + end +end diff --git a/spec/features/security/dashboard_access_spec.rb b/spec/features/security/dashboard_access_spec.rb index d1f00a3dd82..67238e3ab76 100644 --- a/spec/features/security/dashboard_access_spec.rb +++ b/spec/features/security/dashboard_access_spec.rb @@ -25,8 +25,8 @@ describe "Dashboard access", feature: true do it { is_expected.to be_denied_for :visitor } end - describe "GET /dashboard/projects" do - subject { projects_dashboard_path } + describe "GET /dashboard/projects/starred" do + subject { starred_dashboard_projects_path } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } @@ -52,4 +52,12 @@ describe "Dashboard access", feature: true do it { expect(new_group_path).to be_allowed_for :user } it { expect(new_group_path).to be_denied_for :visitor } end + + describe "GET /profile/groups" do + subject { dashboard_groups_path } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :visitor } + end end diff --git a/spec/features/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb index 5f254c42e58..2512a9c0e3d 100644 --- a/spec/features/security/profile_access_spec.rb +++ b/spec/features/security/profile_access_spec.rb @@ -1,76 +1,65 @@ require 'spec_helper' -describe "Users Security", feature: true do - describe "Project" do - before do - @u1 = create(:user) - end - - describe "GET /login" do - it { expect(new_user_session_path).not_to be_404_for :visitor } - end - - describe "GET /profile/keys" do - subject { profile_keys_path } +describe "Profile access", feature: true do + before do + @u1 = create(:user) + end - it { is_expected.to be_allowed_for @u1 } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } - end + describe "GET /login" do + it { expect(new_user_session_path).not_to be_404_for :visitor } + end - describe "GET /profile" do - subject { profile_path } + describe "GET /profile/keys" do + subject { profile_keys_path } - it { is_expected.to be_allowed_for @u1 } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } - end + it { is_expected.to be_allowed_for @u1 } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :visitor } + end - describe "GET /profile/account" do - subject { profile_account_path } + describe "GET /profile" do + subject { profile_path } - it { is_expected.to be_allowed_for @u1 } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } - end + it { is_expected.to be_allowed_for @u1 } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :visitor } + end - describe "GET /profile/design" do - subject { design_profile_path } + describe "GET /profile/account" do + subject { profile_account_path } - it { is_expected.to be_allowed_for @u1 } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } - end + it { is_expected.to be_allowed_for @u1 } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :visitor } + end - describe "GET /profile/history" do - subject { history_profile_path } + describe "GET /profile/design" do + subject { design_profile_path } - it { is_expected.to be_allowed_for @u1 } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } - end + it { is_expected.to be_allowed_for @u1 } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :visitor } + end - describe "GET /profile/notifications" do - subject { profile_notifications_path } + describe "GET /profile/history" do + subject { history_profile_path } - it { is_expected.to be_allowed_for @u1 } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } - end + it { is_expected.to be_allowed_for @u1 } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :visitor } + end - describe "GET /profile/groups" do - subject { profile_groups_path } + describe "GET /profile/notifications" do + subject { profile_notifications_path } - it { is_expected.to be_allowed_for @u1 } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } - end + it { is_expected.to be_allowed_for @u1 } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :visitor } end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 99ff8a32ea5..4c11709ed6e 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -39,24 +39,6 @@ describe ApplicationHelper do end end - describe 'group_icon' do - avatar_file_path = File.join(Rails.root, 'public', 'gitlab_logo.png') - - it 'should return an url for the avatar' do - group = create(:group) - group.avatar = File.open(avatar_file_path) - group.save! - expect(group_icon(group.path).to_s). - to match("/uploads/group/avatar/#{ group.id }/gitlab_logo.png") - end - - it 'should give default avatar_icon when no avatar is present' do - group = create(:group) - group.save! - expect(group_icon(group.path)).to match('group_avatar.png') - end - end - describe 'project_icon' do avatar_file_path = File.join(Rails.root, 'public', 'gitlab_logo.png') diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index c4a192ac1aa..b392371deb4 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -4,6 +4,8 @@ describe EventsHelper do include ApplicationHelper include GitlabMarkdownHelper + let(:current_user) { create(:user, email: "current@email.com") } + it 'should display one line of plain text without alteration' do input = 'A short, plain note' expect(event_note(input)).to match(input) @@ -50,4 +52,14 @@ describe EventsHelper do expect(event_note(input)).to match(link_url) expect(event_note(input)).to match(expected_link_text) end + + it 'should preserve code color scheme' do + input = "```ruby\ndef test\n 'hello world'\nend\n```" + expected = '<pre class="code highlight white ruby">' \ + "<code><span class=\"k\">def</span> <span class=\"nf\">test</span>\n" \ + " <span class=\"s1\">\'hello world\'</span>\n" \ + "<span class=\"k\">end</span>\n" \ + '</code></pre>' + expect(event_note(input)).to eq(expected) + end end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 76fcf888a6a..fd80c615221 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -9,6 +9,7 @@ describe GitlabMarkdownHelper do let(:user) { create(:user, username: 'gfm') } let(:commit) { project.repository.commit } + let(:earlier_commit){ project.repository.commit("HEAD~2") } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:snippet) { create(:project_snippet, project: project) } @@ -53,6 +54,53 @@ describe GitlabMarkdownHelper do to have_selector('a.gfm.foo') end + describe "referencing a commit range" do + let(:expected) { namespace_project_compare_path(project.namespace, project, from: earlier_commit.id, to: commit.id) } + + it "should link using a full id" do + actual = "What happened in #{earlier_commit.id}...#{commit.id}" + expect(gfm(actual)).to match(expected) + end + + it "should link using a short id" do + actual = "What happened in #{earlier_commit.short_id}...#{commit.short_id}" + expected = namespace_project_compare_path(project.namespace, project, from: earlier_commit.short_id, to: commit.short_id) + expect(gfm(actual)).to match(expected) + end + + it "should link inclusively" do + actual = "What happened in #{earlier_commit.id}..#{commit.id}" + expected = namespace_project_compare_path(project.namespace, project, from: "#{earlier_commit.id}^", to: commit.id) + expect(gfm(actual)).to match(expected) + end + + it "should link with adjacent text" do + actual = "(see #{earlier_commit.id}...#{commit.id})" + expect(gfm(actual)).to match(expected) + end + + it "should keep whitespace intact" do + actual = "Changes #{earlier_commit.id}...#{commit.id} dramatically" + expected = /Changes <a.+>#{earlier_commit.id}...#{commit.id}<\/a> dramatically/ + expect(gfm(actual)).to match(expected) + end + + it "should not link with an invalid id" do + actual = expected = "What happened in #{earlier_commit.id.reverse}...#{commit.id.reverse}" + expect(gfm(actual)).to eq(expected) + end + + it "should include a title attribute" do + actual = "What happened in #{earlier_commit.id}...#{commit.id}" + expect(gfm(actual)).to match(/title="Commits #{earlier_commit.id} through #{commit.id}"/) + end + + it "should include standard gfm classes" do + actual = "What happened in #{earlier_commit.id}...#{commit.id}" + expect(gfm(actual)).to match(/class="\s?gfm gfm-commit_range\s?"/) + end + end + describe "referencing a commit" do let(:expected) { namespace_project_commit_path(project.namespace, project, commit) } @@ -616,19 +664,19 @@ describe GitlabMarkdownHelper do it "should generate absolute urls for emoji" do expect(markdown(':smile:')).to( - include(%(src="#{Gitlab.config.gitlab.url}/assets/emoji/smile.png)) + include(%(src="#{Gitlab.config.gitlab.url}/assets/emoji/#{Emoji.emoji_filename('smile')}.png)) ) end it "should generate absolute urls for emoji if relative url is present" do allow(Gitlab.config.gitlab).to receive(:url).and_return('http://localhost/gitlab/root') - expect(markdown(":smile:")).to include("src=\"http://localhost/gitlab/root/assets/emoji/smile.png") + expect(markdown(":smile:")).to include("src=\"http://localhost/gitlab/root/assets/emoji/#{Emoji.emoji_filename('smile')}.png") end it "should generate absolute urls for emoji if asset_host is present" do allow(Gitlab::Application.config).to receive(:asset_host).and_return("https://cdn.example.com") ActionView::Base.any_instance.stub_chain(:config, :asset_host).and_return("https://cdn.example.com") - expect(markdown(":smile:")).to include("src=\"https://cdn.example.com/assets/emoji/smile.png") + expect(markdown(":smile:")).to include("src=\"https://cdn.example.com/assets/emoji/#{Emoji.emoji_filename('smile')}.png") end diff --git a/spec/helpers/groups_helper.rb b/spec/helpers/groups_helper.rb new file mode 100644 index 00000000000..3e99ab84ec9 --- /dev/null +++ b/spec/helpers/groups_helper.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe GroupsHelper do + describe 'group_icon' do + avatar_file_path = File.join(Rails.root, 'public', 'gitlab_logo.png') + + it 'should return an url for the avatar' do + group = create(:group) + group.avatar = File.open(avatar_file_path) + group.save! + expect(group_icon(group.path).to_s). + to match("/uploads/group/avatar/#{ group.id }/gitlab_logo.png") + end + + it 'should give default avatar_icon when no avatar is present' do + group = create(:group) + group.save! + expect(group_icon(group.path)).to match('group_avatar.png') + end + end +end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index a2b05249147..707a0521ab3 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -20,12 +20,26 @@ describe Gitlab::LDAP::Access do before { Gitlab::LDAP::Person.stub(disabled_via_active_directory?: true) } it { is_expected.to be_falsey } + + it "should block user in GitLab" do + access.allowed? + user.should be_blocked + end end context 'and has no disabled flag in active diretory' do - before { Gitlab::LDAP::Person.stub(disabled_via_active_directory?: false) } + before do + user.block + + Gitlab::LDAP::Person.stub(disabled_via_active_directory?: false) + end it { is_expected.to be_truthy } + + it "should unblock user in GitLab" do + access.allowed? + user.should_not be_blocked + end end context 'without ActiveDirectory enabled' do @@ -38,4 +52,4 @@ describe Gitlab::LDAP::Access do end end end -end
\ No newline at end of file +end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 0847c31258c..034f8ee7c45 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -31,6 +31,11 @@ describe Gitlab::ReferenceExtractor do expect(subject.commits).to eq([{ project: nil, id: '98cf0ae3' }]) end + it 'extracts commit ranges' do + subject.analyze('here you go, a commit range: 98cf0ae3...98cf0ae4', nil) + expect(subject.commit_ranges).to eq([{ project: nil, id: '98cf0ae3...98cf0ae4' }]) + end + it 'extracts multiple references and preserves their order' do subject.analyze('@me and @you both care about this', nil) expect(subject.users).to eq([ @@ -100,5 +105,19 @@ describe Gitlab::ReferenceExtractor do expect(extracted[0].sha).to eq(commit.sha) expect(extracted[0].message).to eq(commit.message) end + + it 'accesses valid commit ranges' do + commit = project.repository.commit('master') + earlier_commit = project.repository.commit('master~2') + + subject.analyze("this references commits #{earlier_commit.sha[0..6]}...#{commit.sha[0..6]}", + project) + extracted = subject.commit_ranges_for(project) + expect(extracted.size).to eq(1) + expect(extracted[0][0].sha).to eq(earlier_commit.sha) + expect(extracted[0][0].message).to eq(earlier_commit.message) + expect(extracted[0][1].sha).to eq(commit.sha) + expect(extracted[0][1].message).to eq(commit.message) + end end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 3b09c618f2a..e3a3b542358 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -5,6 +5,7 @@ describe Notify do include EmailSpec::Matchers include RepoHelpers + let(:gitlab_sender_display_name) { Gitlab.config.gitlab.email_display_name } let(:gitlab_sender) { Gitlab.config.gitlab.email_from } let(:recipient) { create(:user, email: 'recipient@example.com') } let(:project) { create(:project) } @@ -23,7 +24,7 @@ describe Notify do shared_examples 'an email sent from GitLab' do it 'is sent from GitLab' do sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq('GitLab') + expect(sender.display_name).to eq(gitlab_sender_display_name) expect(sender.address).to eq(gitlab_sender) end end @@ -182,6 +183,13 @@ describe Notify do context 'for issues' do let(:issue) { create(:issue, author: current_user, assignee: assignee, project: project) } let(:issue_with_description) { create(:issue, author: current_user, assignee: assignee, project: project, description: Faker::Lorem.sentence) } + let(:issue_with_image) do + create(:issue, + author: current_user, + assignee: assignee, + project: project, + description: "![test](#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/12345/test.jpg)") + end describe 'that are new' do subject { Notify.new_issue_email(issue.assignee_id, issue.id) } @@ -206,6 +214,22 @@ describe Notify do end end + describe 'that contain images' do + let(:png) { File.read("#{Rails.root}/spec/fixtures/dk.png") } + let(:png_encoded) { Base64::encode64(png) } + + before :each do + file_path = File.join(Rails.root, 'public', 'uploads', issue_with_image.project.path_with_namespace, '12345/test.jpg') + allow(File).to receive(:file?).with(file_path).and_return(true) + allow(File).to receive(:read).with(file_path).and_return(png) + end + + subject { Notify.new_issue_email(issue_with_image.assignee_id, issue_with_image.id) } + it 'replaces attached images with inline images' do + is_expected.to have_body_text URI.encode(png_encoded) + end + end + describe 'that have been reassigned' do subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user) } @@ -270,6 +294,14 @@ describe Notify do let(:merge_author) { create(:user) } let(:merge_request) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project) } let(:merge_request_with_description) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project, description: Faker::Lorem.sentence) } + let(:merge_request_with_image) do + create(:merge_request, + author: current_user, + assignee: assignee, + source_project: project, + target_project: project, + description: "![test](#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/12345/test.jpg)") + end describe 'that are new' do subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) } @@ -306,6 +338,22 @@ describe Notify do end end + describe 'that are new and contain contain images in the description' do + let(:png) {File.read("#{Rails.root}/spec/fixtures/dk.png")} + let(:png_encoded) { Base64::encode64(png) } + + before :each do + file_path = File.join(Rails.root, 'public', 'uploads', merge_request_with_image.project.path_with_namespace, '/12345/test.jpg') + allow(File).to receive(:file?).with(file_path).and_return(true) + allow(File).to receive(:read).with(file_path).and_return(png) + end + + subject { Notify.new_merge_request_email(merge_request_with_image.assignee_id, merge_request_with_image.id) } + it 'replaces attached images with inline images' do + is_expected.to have_body_text URI.encode(png_encoded) + end + end + describe 'that are reassigned' do subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } @@ -414,9 +462,12 @@ describe Notify do describe 'project access changed' do let(:project) { create(:project) } let(:user) { create(:user) } - let(:project_member) { create(:project_member, - project: project, - user: user) } + let(:project_member) do + create(:project_member, + project: project, + user: user) + end + subject { Notify.project_access_granted_email(project_member.id) } it_behaves_like 'an email sent from GitLab' @@ -456,6 +507,32 @@ describe Notify do end end + describe 'on a commit that contains an image' do + let(:commit) { project.repository.commit } + let(:note_with_image) do + create(:note, + project: project, + author: note_author, + note: "![test](#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/12345/test.jpg)") + end + + let(:png) {File.read("#{Rails.root}/spec/fixtures/dk.png")} + let(:png_encoded) { Base64::encode64(png) } + + before :each do + file_path = File.join(Rails.root, 'public', 'uploads', note_with_image.project.path_with_namespace, '12345/test.jpg') + allow(File).to receive(:file?).with(file_path).and_return(true) + allow(File).to receive(:read).with(file_path).and_return(png) + allow(Note).to receive(:find).with(note_with_image.id).and_return(note_with_image) + allow(note_with_image).to receive(:noteable).and_return(commit) + end + + subject { Notify.note_commit_email(recipient.id, note_with_image.id) } + it 'replaces attached images with inline images' do + is_expected.to have_body_text URI.encode(png_encoded) + end + end + describe 'on a commit' do let(:commit) { project.repository.commit } @@ -568,9 +645,10 @@ describe Notify do let(:user) { create(:user) } let(:compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, sample_image_commit.id, sample_commit.id) } let(:commits) { Commit.decorate(compare.commits) } - let(:diff_path) { namespace_project_compare_path(project.namespace, project, from: commits.first, to: commits.last) } + let(:diff_path) { namespace_project_compare_path(project.namespace, project, from: Commit.new(compare.base), to: Commit.new(compare.head)) } + let(:send_from_committer_email) { false } - subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare) } + subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare, false, send_from_committer_email) } it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -583,7 +661,7 @@ describe Notify do end it 'has the correct subject' do - is_expected.to have_subject /#{commits.length} new commits pushed to repository/ + is_expected.to have_subject /\[#{project.path_with_namespace}\]\[master\] #{commits.length} commits:/ end it 'includes commits list' do @@ -597,6 +675,58 @@ describe Notify do it 'contains a link to the diff' do is_expected.to have_body_text /#{diff_path}/ end + + it 'doesn not contain the misleading footer' do + is_expected.not_to have_body_text /you are a member of/ + end + + context "when set to send from committer email if domain matches" do + + let(:send_from_committer_email) { true } + + before do + allow(Gitlab.config.gitlab).to receive(:host).and_return("gitlab.corp.company.com") + end + + context "when the committer email domain is within the GitLab domain" do + + before do + user.update_attribute(:email, "user@company.com") + user.confirm! + end + + it "is sent from the committer email" do + sender = subject.header[:from].addrs[0] + expect(sender.address).to eq(user.email) + end + end + + context "when the committer email domain is not completely within the GitLab domain" do + + before do + user.update_attribute(:email, "user@something.company.com") + user.confirm! + end + + it "is sent from the default email" do + sender = subject.header[:from].addrs[0] + expect(sender.address).to eq(gitlab_sender) + end + end + + context "when the committer email domain is outside the GitLab domain" do + + before do + user.update_attribute(:email, "user@mpany.com") + user.confirm! + end + + it "is sent from the default email" do + sender = subject.header[:from].addrs[0] + expect(sender.address).to eq(gitlab_sender) + end + end + end end describe 'email on push with a single commit' do diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index b9f2bee148d..8ab847e6432 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -63,7 +63,7 @@ describe HipchatService do end context 'tag_push events' do - let(:push_sample_data) { Gitlab::PushDataBuilder.build(project, user, '000000', '111111', 'refs/tags/test', []) } + let(:push_sample_data) { Gitlab::PushDataBuilder.build(project, user, Gitlab::Git::BLANK_SHA, '1' * 40, 'refs/tags/test', []) } it "should call Hipchat API for tag push events" do hipchat.execute(push_sample_data) diff --git a/spec/models/project_services/slack_service/push_message_spec.rb b/spec/models/project_services/slack_service/push_message_spec.rb index 3ef065459d8..10963481a12 100644 --- a/spec/models/project_services/slack_service/push_message_spec.rb +++ b/spec/models/project_services/slack_service/push_message_spec.rb @@ -43,7 +43,7 @@ describe SlackService::PushMessage do let(:args) { { after: 'after', - before: '000000', + before: Gitlab::Git::BLANK_SHA, project_name: 'project_name', ref: 'refs/tags/new_tag', user_name: 'user_name', @@ -61,7 +61,7 @@ describe SlackService::PushMessage do context 'new branch' do before do - args[:before] = '000000' + args[:before] = Gitlab::Git::BLANK_SHA end it 'returns a message regarding a new branch' do @@ -75,7 +75,7 @@ describe SlackService::PushMessage do context 'removed branch' do before do - args[:after] = '000000' + args[:after] = Gitlab::Git::BLANK_SHA end it 'returns a message regarding a removed branch' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index eeb0f3d9ee0..b3a38f6c5b9 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -18,4 +18,27 @@ describe Repository do it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') } end + + context :timestamps_by_user_log do + before do + Date.stub(:today).and_return(Date.new(2015, 03, 01)) + end + + describe 'single e-mail for user' do + let(:user) { create(:user, email: sample_commit.author_email) } + + subject { repository.timestamps_by_user_log(user) } + + it { is_expected.to eq(["2014-08-06", "2014-07-31", "2014-07-31"]) } + end + + describe 'multiple emails for user' do + let(:email_alias) { create(:email, email: another_sample_commit.author_email) } + let(:user) { create(:user, email: sample_commit.author_email, emails: [email_alias]) } + + subject { repository.timestamps_by_user_log(user) } + + it { is_expected.to eq(["2015-01-10", "2014-08-06", "2014-07-31", "2014-07-31"]) } + end + end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index e264072b573..1b1e3ca5f8b 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -197,15 +197,6 @@ describe GitPushService do service.execute(project, user, @blankrev, @newrev, 'refs/heads/other') end - - it "finds references in the first push to a default branch" do - allow(project.repository).to receive(:commits_between).with(@blankrev, @newrev).and_return([]) - allow(project.repository).to receive(:commits).with(@newrev).and_return([commit]) - - expect(Note).to receive(:create_cross_reference_note).with(issue, commit, commit_author, project) - - service.execute(project, user, @blankrev, @newrev, 'refs/heads/master') - end end describe "closing issues from pushed commits" do diff --git a/spec/support/repo_helpers.rb b/spec/support/repo_helpers.rb index 4c4775da692..aadf791bf3f 100644 --- a/spec/support/repo_helpers.rb +++ b/spec/support/repo_helpers.rb @@ -43,6 +43,25 @@ eos ) end + def another_sample_commit + OpenStruct.new( + id: "e56497bb5f03a90a51293fc6d516788730953899", + parent_id: '4cd80ccab63c82b4bad16faa5193fbd2aa06df40', + author_full_name: "Sytse Sijbrandij", + author_email: "sytse@gitlab.com", + files_changed_count: 1, + message: <<eos +Add directory structure for tree_helper spec + +This directory structure is needed for a testing the method flatten_tree(tree) in the TreeHelper module + +See [merge request #275](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/275#note_732774) + +See merge request !2 +eos + ) + end + def sample_big_commit OpenStruct.new( id: "913c66a37b4a45b9769037c55c2d238bd0942d2e", |