From c5d7660000be72dd03ac52641debbd2bcf6fbc4d Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Fri, 27 Mar 2015 12:58:23 +0100
Subject: [PATCH] Fix errors.
---
app/mailers/emails/groups.rb | 1 +
app/mailers/emails/profile.rb | 6 +-
app/mailers/emails/projects.rb | 5 +-
app/mailers/notify.rb | 12 +-
app/models/concerns/mentionable.rb | 2 +-
app/services/projects/participants_service.rb | 19 +--
lib/gitlab/markdown.rb | 4 +-
lib/gitlab/reference_extractor.rb | 24 +--
spec/helpers/gitlab_markdown_helper_spec.rb | 8 +
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
--
GitLab