diff options
| -rw-r--r-- | app/mailers/emails/groups.rb | 1 | ||||
| -rw-r--r-- | app/mailers/emails/profile.rb | 6 | ||||
| -rw-r--r-- | app/mailers/emails/projects.rb | 5 | ||||
| -rw-r--r-- | app/mailers/notify.rb | 12 | ||||
| -rw-r--r-- | app/models/concerns/mentionable.rb | 2 | ||||
| -rw-r--r-- | app/services/projects/participants_service.rb | 19 | ||||
| -rw-r--r-- | lib/gitlab/markdown.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/reference_extractor.rb | 24 | ||||
| -rw-r--r-- | spec/helpers/gitlab_markdown_helper_spec.rb | 8 | ||||
| -rw-r--r-- | spec/lib/gitlab/reference_extractor_spec.rb | 153 |
10 files changed, 114 insertions, 120 deletions
diff --git a/app/mailers/emails/groups.rb b/app/mailers/emails/groups.rb index 26f43bf955e..626eb593d51 100644 --- a/app/mailers/emails/groups.rb +++ b/app/mailers/emails/groups.rb @@ -4,6 +4,7 @@ module Emails @group_member = GroupMember.find(group_member_id) @group = @group_member.group @target_url = group_url(@group) + @current_user = @group_member.user mail(to: @group_member.user.email, subject: subject("Access to group was granted")) end diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index ab5b0765352..3a83b083109 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -1,7 +1,7 @@ module Emails module Profile def new_user_email(user_id, token = nil) - @user = User.find(user_id) + @current_user = @user = User.find(user_id) @target_url = user_url(@user) @token = token mail(to: @user.notification_email, subject: subject("Account was created for you")) @@ -9,13 +9,13 @@ module Emails def new_email_email(email_id) @email = Email.find(email_id) - @user = @email.user + @current_user = @user = @email.user mail(to: @user.notification_email, subject: subject("Email was added to your account")) end def new_ssh_key_email(key_id) @key = Key.find(key_id) - @user = @key.user + @current_user = @user = @key.user @target_url = user_url(@user) mail(to: @user.notification_email, subject: subject("SSH key was added to your account")) end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 3cd812825e2..20a863c3742 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -4,12 +4,13 @@ module Emails @project_member = ProjectMember.find user_project_id @project = @project_member.project @target_url = namespace_project_url(@project.namespace, @project) + @current_user = @project_member.user mail(to: @project_member.user.email, subject: subject("Access to project was granted")) end def project_was_moved_email(project_id, user_id) - @user = User.find user_id + @current_user = @user = User.find user_id @project = Project.find project_id @target_url = namespace_project_url(@project.namespace, @project) mail(to: @user.notification_email, @@ -28,7 +29,7 @@ module Emails end @project = Project.find(project_id) - @author = User.find(author_id) + @current_user = @author = User.find(author_id) @reverse_compare = reverse_compare @compare = compare @ref_name = Gitlab::Git.ref_name(ref) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 8fcdd3bc853..5dbbfac9c8b 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -13,6 +13,9 @@ class Notify < ActionMailer::Base add_template_helper MergeRequestsHelper add_template_helper EmailsHelper + attr_accessor :current_user + helper_method :current_user, :can? + default_url_options[:host] = Gitlab.config.gitlab.host default_url_options[:protocol] = Gitlab.config.gitlab.protocol default_url_options[:port] = Gitlab.config.gitlab.port unless Gitlab.config.gitlab_on_standard_port? @@ -79,9 +82,8 @@ class Notify < ActionMailer::Base # # Returns a String containing the User's email address. def recipient(recipient_id) - if recipient = User.find(recipient_id) - recipient.notification_email - end + @current_user = User.find(recipient_id) + @current_user.notification_email end # Set the References header field @@ -154,4 +156,8 @@ class Notify < ActionMailer::Base mail(headers, &block) end + + def can? + Ability.abilities.allowed?(user, action, subject) + end end diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index bf0465728e7..b7882a2bb16 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -46,7 +46,7 @@ module Mentionable return [] if mentionable_text.blank? ext = Gitlab::ReferenceExtractor.new(self.project, current_user) - ext.analyze(text) + ext.analyze(mentionable_text) ext.users.uniq end diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index bcdde0950c5..ae6260bcdab 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -1,10 +1,5 @@ module Projects class ParticipantsService < BaseService - def initialize(project, user) - @project = project - @user = user - end - def execute(note_type, note_id) participating = if note_type && note_id @@ -12,7 +7,7 @@ module Projects else [] end - project_members = sorted(@project.team.members) + project_members = sorted(project.team.members) participants = all_members + groups + project_members + participating participants.uniq end @@ -20,11 +15,11 @@ module Projects def participants_in(type, id) users = case type when "Issue" - issue = @project.issues.find_by_iid(id) - issue ? issue.participants(user) : [] + issue = project.issues.find_by_iid(id) + issue ? issue.participants(current_user) : [] when "MergeRequest" - merge_request = @project.merge_requests.find_by_iid(id) - merge_request ? merge_request.participants(user) : [] + merge_request = project.merge_requests.find_by_iid(id) + merge_request ? merge_request.participants(current_user) : [] when "Commit" author_ids = Note.for_commit_id(id).pluck(:author_id).uniq User.where(id: author_ids) @@ -41,14 +36,14 @@ module Projects end def groups - @user.authorized_groups.sort_by(&:path).map do |group| + current_user.authorized_groups.sort_by(&:path).map do |group| count = group.users.count { username: group.path, name: "#{group.name} (#{count})" } end end def all_members - count = @project.team.members.flatten.count + count = project.team.members.flatten.count [{ username: "all", name: "All Project and Group Members (#{count})" }] end end diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 7a3c8823e4d..e7c261b7453 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -192,7 +192,7 @@ module Gitlab project_path = $LAST_MATCH_INFO[:project] if project_path actual_project = ::Project.find_with_namespace(project_path) - actual_project ||= nil unless can?(user, :read_project, actual_project) + actual_project = nil unless can?(user, :read_project, actual_project) project_prefix = project_path end @@ -235,7 +235,7 @@ module Gitlab # # Returns string rendered by the processing method def reference_link(type, identifier, project = @project, user = current_user, prefix_text = nil) - send("reference_#{type}", identifier, project, prefix_text) + send("reference_#{type}", identifier, project, user, prefix_text) end def reference_user(identifier, project = @project, user = current_user, _ = nil) diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 719274394f0..2f38c1dcc89 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -7,7 +7,7 @@ module Gitlab def initialize(project, current_user = nil) @project = project - @current_user = user + @current_user = current_user @references = Hash.new { [] } end @@ -51,7 +51,7 @@ module Gitlab def issues references[:issues].map do |entry| - if should_lookup?(entry[:project]) + if entry[:project].default_issues_tracker? entry[:project].issues.where(iid: entry[:id]).first end end.compact @@ -59,9 +59,7 @@ module Gitlab def merge_requests references[:merge_requests].map do |entry| - if should_lookup?(entry[:project]) - entry[:project].merge_requests.where(iid: entry[:id]).first - end + entry[:project].merge_requests.where(iid: entry[:id]).first end.compact end @@ -73,17 +71,15 @@ module Gitlab def commits references[:commits].map do |entry| - repo = entry[:project].repository if entry[:project] - if should_lookup?(entry[:project]) - repo.commit(entry[:id]) if repo - end + repo = entry[:project].repository + repo.commit(entry[:id]) if repo end.compact end def commit_ranges references[:commit_ranges].map do |entry| repo = entry[:project].repository if entry[:project] - if repo && should_lookup?(entry[:project]) + if repo from_id, to_id = entry[:id].split(/\.{2,3}/, 2) [repo.commit(from_id), repo.commit(to_id)] end @@ -95,13 +91,5 @@ module Gitlab def reference_link(type, identifier, project, user, _) references[type] << { project: project, id: identifier } end - - def should_lookup?(entry_project) - if entry_project.nil? - false - else - project.nil? || entry_project.default_issues_tracker? - end - end end end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 0d06c6ffb82..944e743675c 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -4,6 +4,11 @@ describe GitlabMarkdownHelper do include ApplicationHelper include IssuesHelper + # TODO: Properly test this + def can?(*) + true + end + let!(:project) { create(:project) } let(:empty_project) { create(:empty_project) } @@ -15,6 +20,9 @@ describe GitlabMarkdownHelper do let(:snippet) { create(:project_snippet, project: project) } let(:member) { project.project_members.where(user_id: user).first } + # Helper expects a current_user method. + let(:current_user) { user } + def url_helper(image_name) File.join(root_url, 'assets', image_name) end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index b3f4bb5aeda..8ed4d5bc68f 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -1,73 +1,76 @@ require 'spec_helper' describe Gitlab::ReferenceExtractor do + let(:project) { create(:project) } + subject { Gitlab::ReferenceExtractor.new(project) } + it 'extracts username references' do - subject.analyze('this contains a @user reference', nil) - expect(subject.users).to eq([{ project: nil, id: 'user' }]) + subject.analyze('this contains a @user reference') + expect(subject.users).to eq([{ project: project, id: 'user' }]) end it 'extracts issue references' do - subject.analyze('this one talks about issue #1234', nil) - expect(subject.issues).to eq([{ project: nil, id: '1234' }]) + subject.analyze('this one talks about issue #1234') + expect(subject.issues).to eq([{ project: project, id: '1234' }]) end it 'extracts JIRA issue references' do - subject.analyze('this one talks about issue JIRA-1234', nil) - expect(subject.issues).to eq([{ project: nil, id: 'JIRA-1234' }]) + subject.analyze('this one talks about issue JIRA-1234') + expect(subject.issues).to eq([{ project: project, id: 'JIRA-1234' }]) end it 'extracts merge request references' do - subject.analyze("and here's !43, a merge request", nil) - expect(subject.merge_requests).to eq([{ project: nil, id: '43' }]) + subject.analyze("and here's !43, a merge request") + expect(subject.merge_requests).to eq([{ project: project, id: '43' }]) end it 'extracts snippet ids' do - subject.analyze('snippets like $12 get extracted as well', nil) - expect(subject.snippets).to eq([{ project: nil, id: '12' }]) + subject.analyze('snippets like $12 get extracted as well') + expect(subject.snippets).to eq([{ project: project, id: '12' }]) end it 'extracts commit shas' do - subject.analyze('commit shas 98cf0ae3 are pulled out as Strings', nil) - expect(subject.commits).to eq([{ project: nil, id: '98cf0ae3' }]) + subject.analyze('commit shas 98cf0ae3 are pulled out as Strings') + expect(subject.commits).to eq([{ project: project, 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' }]) + subject.analyze('here you go, a commit range: 98cf0ae3...98cf0ae4') + expect(subject.commit_ranges).to eq([{ project: project, id: '98cf0ae3...98cf0ae4' }]) end it 'extracts multiple references and preserves their order' do - subject.analyze('@me and @you both care about this', nil) + subject.analyze('@me and @you both care about this') expect(subject.users).to eq([ - { project: nil, id: 'me' }, - { project: nil, id: 'you' } + { project: project, id: 'me' }, + { project: project, id: 'you' } ]) end it 'leaves the original note unmodified' do text = 'issue #123 is just the worst, @user' - subject.analyze(text, nil) + subject.analyze(text) expect(text).to eq('issue #123 is just the worst, @user') end it 'extracts no references for <pre>..</pre> blocks' do - subject.analyze("<pre>def puts '#1 issue'\nend\n</pre>```", nil) + subject.analyze("<pre>def puts '#1 issue'\nend\n</pre>```") expect(subject.issues).to be_blank end it 'extracts no references for <code>..</code> blocks' do - subject.analyze("<code>def puts '!1 request'\nend\n</code>```", nil) + subject.analyze("<code>def puts '!1 request'\nend\n</code>```") expect(subject.merge_requests).to be_blank end it 'extracts no references for code blocks with language' do - subject.analyze("this code:\n```ruby\ndef puts '#1 issue'\nend\n```", nil) + subject.analyze("this code:\n```ruby\ndef puts '#1 issue'\nend\n```") expect(subject.issues).to be_blank end it 'extracts issue references for invalid code blocks' do - subject.analyze('test: ```this one talks about issue #1234```', nil) - expect(subject.issues).to eq([{ project: nil, id: '1234' }]) + subject.analyze('test: ```this one talks about issue #1234```') + expect(subject.issues).to eq([{ project: project, id: '1234' }]) end it 'handles all possible kinds of references' do @@ -75,70 +78,64 @@ describe Gitlab::ReferenceExtractor do expect(subject).to respond_to(*accessors) end - context 'with a project' do - let(:project) { create(:project) } - - it 'accesses valid user objects on the project team' do - @u_foo = create(:user, username: 'foo') - @u_bar = create(:user, username: 'bar') - create(:user, username: 'offteam') + it 'accesses valid user objects on the project team' do + @u_foo = create(:user, username: 'foo') + @u_bar = create(:user, username: 'bar') + create(:user, username: 'offteam') - project.team << [@u_foo, :reporter] - project.team << [@u_bar, :guest] + project.team << [@u_foo, :reporter] + project.team << [@u_bar, :guest] - subject.analyze('@foo, @baduser, @bar, and @offteam', project) - expect(subject.users_for(project)).to eq([@u_foo, @u_bar]) - end + subject.analyze('@foo, @baduser, @bar, and @offteam') + expect(subject.users).to eq([@u_foo, @u_bar]) + end - it 'accesses valid issue objects' do - @i0 = create(:issue, project: project) - @i1 = create(:issue, project: project) + it 'accesses valid issue objects' do + @i0 = create(:issue, project: project) + @i1 = create(:issue, project: project) - subject.analyze("##{@i0.iid}, ##{@i1.iid}, and #999.", project) - expect(subject.issues_for(project)).to eq([@i0, @i1]) - end + subject.analyze("##{@i0.iid}, ##{@i1.iid}, and #999.") + expect(subject.issues).to eq([@i0, @i1]) + end - it 'accesses valid merge requests' do - @m0 = create(:merge_request, source_project: project, target_project: project, source_branch: 'aaa') - @m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'bbb') + it 'accesses valid merge requests' do + @m0 = create(:merge_request, source_project: project, target_project: project, source_branch: 'aaa') + @m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'bbb') - subject.analyze("!999, !#{@m1.iid}, and !#{@m0.iid}.", project) - expect(subject.merge_requests_for(project)).to eq([@m1, @m0]) - end + subject.analyze("!999, !#{@m1.iid}, and !#{@m0.iid}.") + expect(subject.merge_requests).to eq([@m1, @m0]) + end - it 'accesses valid snippets' do - @s0 = create(:project_snippet, project: project) - @s1 = create(:project_snippet, project: project) - @s2 = create(:project_snippet) + it 'accesses valid snippets' do + @s0 = create(:project_snippet, project: project) + @s1 = create(:project_snippet, project: project) + @s2 = create(:project_snippet) - subject.analyze("$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}", project) - expect(subject.snippets_for(project)).to eq([@s0, @s1]) - end + subject.analyze("$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}") + expect(subject.snippets).to eq([@s0, @s1]) + end - it 'accesses valid commits' do - commit = project.repository.commit('master') + it 'accesses valid commits' do + commit = project.repository.commit('master') - subject.analyze("this references commits #{commit.sha[0..6]} and 012345", - project) - extracted = subject.commits_for(project) - expect(extracted.size).to eq(1) - expect(extracted[0].sha).to eq(commit.sha) - expect(extracted[0].message).to eq(commit.message) - end + subject.analyze("this references commits #{commit.sha[0..6]} and 012345") + extracted = subject.commits + expect(extracted.size).to eq(1) + 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') + 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 + subject.analyze("this references commits #{earlier_commit.sha[0..6]}...#{commit.sha[0..6]}") + extracted = subject.commit_ranges + 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 context 'with a project with an underscore' do @@ -146,12 +143,10 @@ describe Gitlab::ReferenceExtractor do let(:issue) { create(:issue, project: project) } it 'handles project issue references' do - subject.analyze("this refers issue #{project.path_with_namespace}##{issue.iid}", - project) - extracted = subject.issues_for(project) + subject.analyze("this refers issue #{project.path_with_namespace}##{issue.iid}") + extracted = subject.issues expect(extracted.size).to eq(1) expect(extracted).to eq([issue]) end - end end |