diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index cc2073081b54a0f27be61ee32e7f0e7d731fa4e2..6297b2db369080c9e2fc0dddf367ba717164236e 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -61,31 +61,26 @@ class IssuableFinder
def project
return @project if defined?(@project)
- if project?
- @project = Project.find(params[:project_id])
+ project = Project.find(params[:project_id])
+ project = nil unless Ability.allowed?(current_user, :"read_#{klass.to_ability_name}", project)
- unless Ability.allowed?(current_user, :read_project, @project)
- @project = nil
- end
- else
- @project = nil
- end
-
- @project
+ @project = project
end
def projects
return @projects if defined?(@projects)
+ return @projects = project if project?
- if project?
- @projects = project
- elsif current_user && params[:authorized_only].presence && !current_user_related?
- @projects = current_user.authorized_projects.reorder(nil)
- elsif group
- @projects = GroupProjectsFinder.new(group).execute(current_user).reorder(nil)
- else
- @projects = ProjectsFinder.new.execute(current_user).reorder(nil)
- end
+ projects =
+ if current_user && params[:authorized_only].presence && !current_user_related?
+ current_user.authorized_projects
+ elsif group
+ GroupProjectsFinder.new(group).execute(current_user)
+ else
+ ProjectsFinder.new.execute(current_user)
+ end
+
+ @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil)
end
def search
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 93a6b3122e07d4d7b614a57b1010bdd1cd7a8cf3..664bb594aa9f1a96c6ab32517a671894a4ebbd67 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -183,6 +183,10 @@ module Issuable
grouping_columns
end
+
+ def to_ability_name
+ model_name.singular
+ end
end
def today?
@@ -244,7 +248,7 @@ module Issuable
# issuable.class # => MergeRequest
# issuable.to_ability_name # => "merge_request"
def to_ability_name
- self.class.to_s.underscore
+ self.class.to_ability_name
end
# Returns a Hash of attributes to be used for Twitter card metadata
diff --git a/app/models/project.rb b/app/models/project.rb
index 4c9c7c001dd4a44f32d6608de2746637befa0da1..bbe590b5a8a0d122813bcebcb56c6150096c1a5c 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -207,8 +207,38 @@ class Project < ActiveRecord::Base
scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct }
scope :with_push, -> { joins(:events).where('events.action = ?', Event::PUSHED) }
- scope :with_builds_enabled, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id').where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0') }
- scope :with_issues_enabled, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id').where('project_features.issues_access_level IS NULL or project_features.issues_access_level > 0') }
+ scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') }
+
+ # "enabled" here means "not disabled". It includes private features!
+ scope :with_feature_enabled, ->(feature) {
+ access_level_attribute = ProjectFeature.access_level_attribute(feature)
+ with_project_feature.where(project_features: { access_level_attribute => [nil, ProjectFeature::PRIVATE, ProjectFeature::ENABLED] })
+ }
+
+ # Picks a feature where the level is exactly that given.
+ scope :with_feature_access_level, ->(feature, level) {
+ access_level_attribute = ProjectFeature.access_level_attribute(feature)
+ with_project_feature.where(project_features: { access_level_attribute => level })
+ }
+
+ scope :with_builds_enabled, -> { with_feature_enabled(:builds) }
+ scope :with_issues_enabled, -> { with_feature_enabled(:issues) }
+
+ # project features may be "disabled", "internal" or "enabled". If "internal",
+ # they are only available to team members. This scope returns projects where
+ # the feature is either enabled, or internal with permission for the user.
+ def self.with_feature_available_for_user(feature, user)
+ return with_feature_enabled(feature) if user.try(:admin?)
+
+ unconditional = with_feature_access_level(feature, [nil, ProjectFeature::ENABLED])
+ return unconditional if user.nil?
+
+ conditional = with_feature_access_level(feature, ProjectFeature::PRIVATE)
+ authorized = user.authorized_projects.merge(conditional.reorder(nil))
+
+ union = Gitlab::SQL::Union.new([unconditional.select(:id), authorized.select(:id)])
+ where(arel_table[:id].in(Arel::Nodes::SqlLiteral.new(union.to_sql)))
+ end
scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') }
scope :abandoned, -> { where('projects.last_activity_at < ?', 6.months.ago) }
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index b37ce1d3cf6d60e9b868a47e5c604889d4f661ab..34fd5a57b5e9de0a09764b04493cbd0f42731879 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -20,6 +20,15 @@ class ProjectFeature < ActiveRecord::Base
FEATURES = %i(issues merge_requests wiki snippets builds repository)
+ class << self
+ def access_level_attribute(feature)
+ feature = feature.model_name.plural.to_sym if feature.respond_to?(:model_name)
+ raise ArgumentError, "invalid project feature: #{feature}" unless FEATURES.include?(feature)
+
+ "#{feature}_access_level".to_sym
+ end
+ end
+
# Default scopes force us to unscope here since a service may need to check
# permissions for a project in pending_delete
# http://stackoverflow.com/questions/1540645/how-to-disable-default-scope-for-a-belongs-to
@@ -35,9 +44,8 @@ class ProjectFeature < ActiveRecord::Base
default_value_for :repository_access_level, value: ENABLED, allows_nil: false
def feature_available?(feature, user)
- raise ArgumentError, 'invalid project feature' unless FEATURES.include?(feature)
-
- get_permission(user, public_send("#{feature}_access_level"))
+ access_level = public_send(ProjectFeature.access_level_attribute(feature))
+ get_permission(user, access_level)
end
def builds_enabled?
diff --git a/spec/features/groups/issues_spec.rb b/spec/features/groups/issues_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..476eca17a9d8e36c3ee7bad6679ebf98febc434f
--- /dev/null
+++ b/spec/features/groups/issues_spec.rb
@@ -0,0 +1,8 @@
+require 'spec_helper'
+
+feature 'Group issues page', feature: true do
+ let(:path) { issues_group_path(group) }
+ let(:issuable) { create(:issue, project: project, title: "this is my created issuable")}
+
+ include_examples 'project features apply to issuables', Issue
+end
diff --git a/spec/features/groups/merge_requests_spec.rb b/spec/features/groups/merge_requests_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..a2791b5754407a3cfee9deec2c54b0c80cb58cd7
--- /dev/null
+++ b/spec/features/groups/merge_requests_spec.rb
@@ -0,0 +1,8 @@
+require 'spec_helper'
+
+feature 'Group merge requests page', feature: true do
+ let(:path) { merge_requests_group_path(group) }
+ let(:issuable) { create(:merge_request, source_project: project, target_project: project, title: "this is my created issuable")}
+
+ include_examples 'project features apply to issuables', MergeRequest
+end
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index a9603074c3240f90561f92b14c678eb1c4777eb5..6e987967ca5331243dd5bde1a31991370536f90e 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -97,6 +97,11 @@ describe Issue, "Issuable" do
end
end
+ describe '.to_ability_name' do
+ it { expect(Issue.to_ability_name).to eq("issue") }
+ it { expect(MergeRequest.to_ability_name).to eq("merge_request") }
+ end
+
describe "#today?" do
it "returns true when created today" do
# Avoid timezone differences and just return exactly what we want
diff --git a/spec/support/project_features_apply_to_issuables_shared_examples.rb b/spec/support/project_features_apply_to_issuables_shared_examples.rb
new file mode 100644
index 0000000000000000000000000000000000000000..4621d17549b9f3badc42e06235644f6628c046fd
--- /dev/null
+++ b/spec/support/project_features_apply_to_issuables_shared_examples.rb
@@ -0,0 +1,56 @@
+shared_examples 'project features apply to issuables' do |klass|
+ let(:described_class) { klass }
+
+ let(:group) { create(:group) }
+ let(:user_in_group) { create(:group_member, :developer, user: create(:user), group: group ).user }
+ let(:user_outside_group) { create(:user) }
+
+ let(:project) { create(:empty_project, :public, project_args) }
+
+ def project_args
+ feature = "#{described_class.model_name.plural}_access_level".to_sym
+
+ args = { group: group }
+ args[feature] = access_level
+
+ args
+ end
+
+ before do
+ _ = issuable
+ login_as(user)
+ visit path
+ end
+
+ context 'public access level' do
+ let(:access_level) { ProjectFeature::ENABLED }
+
+ context 'group member' do
+ let(:user) { user_in_group }
+
+ it { expect(page).to have_content(issuable.title) }
+ end
+
+ context 'non-member' do
+ let(:user) { user_outside_group }
+
+ it { expect(page).to have_content(issuable.title) }
+ end
+ end
+
+ context 'private access level' do
+ let(:access_level) { ProjectFeature::PRIVATE }
+
+ context 'group member' do
+ let(:user) { user_in_group }
+
+ it { expect(page).to have_content(issuable.title) }
+ end
+
+ context 'non-member' do
+ let(:user) { user_outside_group }
+
+ it { expect(page).not_to have_content(issuable.title) }
+ end
+ end
+end