Commit f3658bdb authored by Andrew Fontaine's avatar Andrew Fontaine

Backend For Referencing Feature Flags from Issues

This begins the implementation for the ability to reference feature
flags from issues.

Displaying current links is handled; however, the ability to create and
delete links is missing.
parent a2a6cbb5
......@@ -54,6 +54,14 @@ class Ability
end
end
def feature_flags_readable_by_user(feature_flags, user = nil, filters: {})
feature_flags = apply_filters_if_needed(feature_flags, user, filters)
DeclarativePolicy.user_scope do
feature_flags.select { |flag| allowed?(user, :read_feature_flag, flag) }
end
end
def allowed?(user, ability, subject = :global, opts = {})
if subject.is_a?(Hash)
opts = subject
......
# frozen_string_literal: true
module Projects
class IssueFeatureFlagsController < Projects::ApplicationController
include IssuableLinks
before_action :authorize_admin_feature_flags_issue_links!
feature_category :feature_flags
private
def list_service
::IssueFeatureFlags::ListService.new(issue, current_user)
end
def link
@link ||= ::FeatureFlagIssue.find(params[:id])
end
def issue
project.issues.find_by_iid(params[:issue_id])
end
end
end
......@@ -218,6 +218,20 @@ module EE
SQL
end
def related_feature_flags(current_user, preload: nil)
feature_flags = ::Operations::FeatureFlag
.select('operations_feature_flags.*, operations_feature_flags_issues.id AS link_id')
.joins(:feature_flag_issues)
.where(operations_feature_flags_issues: { issue_id: id })
.order('operations_feature_flags_issues.id ASC')
.includes(preload)
cross_project_filter = -> (feature_flags) { feature_flags.where(project: project) }
Ability.feature_flags_readable_by_user(feature_flags,
current_user,
filters: { read_cross_project: cross_project_filter })
end
override :relocation_target
def relocation_target
super || promoted_to_epic
......
# frozen_string_literal: true
module Issues
class LinkedIssueFeatureFlagEntity < Grape::Entity
include RequestAwareEntity
expose :id, :name, :iid
expose :active
expose :path do |link|
project_feature_flag_path(link.project, link.iid)
end
expose :reference do |link|
link.to_reference(issuable.project)
end
expose :link_type do |_issue|
'relates_to'
end
def issuable
request.issuable
end
end
end
# frozen_string_literal: true
module Issues
class LinkedIssueFeatureFlagSerializer < BaseSerializer
entity LinkedIssueFeatureFlagEntity
end
end
# frozen_string_literal: true
module IssueFeatureFlags
class ListService < IssuableLinks::ListService
extend ::Gitlab::Utils::Override
private
def child_issuables
issuable.related_feature_flags(current_user, preload: preload_for_collection)
end
override :serializer
def serializer
Issues::LinkedIssueFeatureFlagSerializer
end
override :preload_for_collection
def preload_for_collection
[{ project: :namespace }]
end
end
end
......@@ -5,4 +5,5 @@ resources :issues, only: [], constraints: { id: /\d+/ } do
get '/descriptions/:version_id/diff', action: :description_diff, as: :description_diff
delete '/descriptions/:version_id', action: :delete_description_version, as: :delete_description_version
end
resources :issue_feature_flags, only: [:index, :show], as: 'feature_flags', path: 'feature_flags'
end
......@@ -1039,6 +1039,44 @@ RSpec.describe Issue do
end
end
describe '#related_feature_flags' do
let_it_be(:user) { create(:user) }
let_it_be(:authorized_project) { create(:project) }
let_it_be(:authorized_project2) { create(:project) }
let_it_be(:unauthorized_project) { create(:project) }
let_it_be(:issue) { create(:issue, project: authorized_project) }
let_it_be(:authorized_feature_flag) { create(:operations_feature_flag, project: authorized_project) }
let_it_be(:authorized_feature_flag_b) { create(:operations_feature_flag, project: authorized_project2) }
let_it_be(:unauthorized_feature_flag) { create(:operations_feature_flag, project: unauthorized_project) }
let_it_be(:issue_link_a) { create(:feature_flag_issue, issue: issue, feature_flag: authorized_feature_flag) }
let_it_be(:issue_link_b) { create(:feature_flag_issue, issue: issue, feature_flag: unauthorized_feature_flag) }
let_it_be(:issue_link_c) { create(:feature_flag_issue, issue: issue, feature_flag: authorized_feature_flag_b) }
before_all do
authorized_project.add_developer(user)
authorized_project2.add_developer(user)
end
it 'returns only authorized related feature flags for a given user' do
expect(issue.related_feature_flags(user)).to contain_exactly(authorized_feature_flag, authorized_feature_flag_b)
end
describe 'when a user cannot read cross project' do
it 'only returns feature_flags within the same project' do
expect(Ability).to receive(:allowed?).with(user, :read_feature_flag, authorized_feature_flag).and_return(true)
expect(Ability).to receive(:allowed?).with(user, :read_cross_project).and_return(false)
expect(issue.related_feature_flags(user))
.to contain_exactly(authorized_feature_flag)
end
end
end
describe '.with_issue_type' do
let_it_be(:project) { create(:project) }
let_it_be(:issue) { create(:issue, project: project) }
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Projects::IssueFeatureFlagsController do
let_it_be(:project) { create(:project) }
let_it_be(:developer) { create(:user) }
let_it_be(:reporter) { create(:user) }
before_all do
project.add_developer(developer)
project.add_reporter(reporter)
end
before do
stub_licensed_features(feature_flags_related_issues: true)
end
describe 'GET #index' do
def setup
feature_flag = create(:operations_feature_flag, project: project)
issue = create(:issue, project: project)
link = create(:feature_flag_issue, feature_flag: feature_flag, issue: issue)
[feature_flag, issue, link]
end
def get_request(project, issue)
get project_issue_feature_flags_path(project, issue, format: :json)
end
it 'returns linked feature flags' do
feature_flag, issue = setup
sign_in(developer)
get_request(project, issue)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to match([a_hash_including({
'id' => feature_flag.id
})])
end
it 'does not return linked feature flags for a reporter' do
_, issue, _ = setup
sign_in(reporter)
get_request(project, issue)
expect(response).to have_gitlab_http_status(:not_found)
end
it 'orders by feature_flag_issue id' do
issue = create(:issue, project: project)
feature_flag_a = create(:operations_feature_flag, project: project)
feature_flag_b = create(:operations_feature_flag, project: project)
create(:feature_flag_issue, feature_flag: feature_flag_b, issue: issue)
create(:feature_flag_issue, feature_flag: feature_flag_a, issue: issue)
sign_in(developer)
get_request(project, issue)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response.map { |feature_flag| feature_flag['id'] }).to eq([feature_flag_b.id, feature_flag_a.id])
end
it 'does not make N+1 queries' do
feature_flag, _, _ = setup
sign_in(developer)
control_count = ActiveRecord::QueryRecorder.new { get_request(project, feature_flag) }.count
issue_b = create(:issue, project: project)
issue_c = create(:issue, project: project)
create(:feature_flag_issue, feature_flag: feature_flag, issue: issue_b)
create(:feature_flag_issue, feature_flag: feature_flag, issue: issue_c)
expect { get_request(project, feature_flag) }.not_to exceed_query_limit(control_count)
end
context 'when feature flag related issues feature is unlicensed' do
before do
stub_licensed_features(feature_flags_related_issues: false)
end
it 'does not return linked issues' do
feature_flag, _, _ = setup
sign_in(developer)
get_request(project, feature_flag)
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Issues::LinkedIssueFeatureFlagEntity do
let_it_be(:project) { create(:project) }
let_it_be(:developer) { create(:user) }
before_all do
project.add_developer(developer)
end
describe '#as_json' do
it 'returns json' do
issue = create(:issue, project: project)
feature_flag = create(:operations_feature_flag, project: project)
link = create(:feature_flag_issue, feature_flag: feature_flag, issue: issue)
allow(issue).to receive(:link_id).and_return(link.id)
request = double('request')
allow(request).to receive(:current_user).and_return(developer)
allow(request).to receive(:issuable).and_return(issue)
entity = described_class.new(feature_flag, request: request, current_user: developer)
expect(entity.as_json.slice(:link_type, :path, :reference)).to eq({
link_type: 'relates_to',
path: "/#{project.full_path}/-/feature_flags/#{feature_flag.iid}",
reference: "[feature_flag:#{feature_flag.iid}]"
})
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe IssueFeatureFlags::ListService do
let(:user) { create(:user) }
let(:project) { create(:project_empty_repo, :private) }
let(:issue) { create(:issue, project: project) }
let(:feature_flag) { create(:operations_feature_flag, project: project) }
describe '#execute' do
subject { described_class.new(issue, user).execute }
let(:feature_flag_b) { create(:operations_feature_flag, project: project) }
let(:feature_flag_c) { create(:operations_feature_flag, project: project) }
let(:feature_flag_d) { create(:operations_feature_flag, project: project) }
before do
create(:feature_flag_issue, feature_flag: feature_flag_b, issue: issue)
create(:feature_flag_issue, feature_flag: feature_flag_c, issue: issue)
create(:feature_flag_issue, feature_flag: feature_flag_d, issue: issue)
create(:feature_flag_issue, feature_flag: feature_flag, issue: issue)
end
context 'user can see feature flags' do
before do
project.add_developer(user)
end
it 'ensures no N+1 queries are made' do
control_count = ActiveRecord::QueryRecorder.new { described_class.new(issue, user).execute }.count
expect { described_class.new(issue, user).execute }.not_to exceed_query_limit(control_count)
end
it 'returns related issues' do
expect(subject.size).to eq(4)
expect(subject).to include(include(id: feature_flag.id,
name: feature_flag.name,
path: "/#{project.full_path}/-/feature_flags/#{feature_flag.iid}"))
expect(subject).to include(include(id: feature_flag_b.id,
name: feature_flag_b.name,
path: "/#{project.full_path}/-/feature_flags/#{feature_flag_b.iid}"))
expect(subject).to include(include(id: feature_flag_c.id,
name: feature_flag_c.name,
path: "/#{project.full_path}/-/feature_flags/#{feature_flag_c.iid}"))
expect(subject).to include(include(id: feature_flag_d.id,
name: feature_flag_d.name,
path: "/#{project.full_path}/-/feature_flags/#{feature_flag_d.iid}"))
end
end
context 'user can not see feature flags' do
it 'returns related issues' do
expect(subject.size).to eq(0)
end
end
end
end
......@@ -328,6 +328,69 @@ RSpec.describe Ability do
end
end
describe '.feature_flags_readable_by_user' do
context 'without a user' do
it 'returns no feature flags' do
feature_flag_1 = build(:operations_feature_flag)
feature_flag_2 = build(:operations_feature_flag, project: build(:project, :public))
feature_flags = described_class
.feature_flags_readable_by_user([feature_flag_1, feature_flag_2])
expect(feature_flags).to eq([])
end
end
context 'with a user' do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:feature_flag) { create(:operations_feature_flag, project: project) }
let(:cross_project) { create(:project) }
let(:cross_project_feature_flag) { create(:operations_feature_flag, project: cross_project) }
let(:other_feature_flag) { create(:operations_feature_flag) }
let(:all_feature_flags) do
[feature_flag, cross_project_feature_flag, other_feature_flag]
end
subject(:readable_feature_flags) do
described_class.feature_flags_readable_by_user(all_feature_flags, user)
end
before do
project.add_developer(user)
cross_project.add_developer(user)
end
it 'returns projects visible to the user' do
expect(readable_feature_flags).to contain_exactly(feature_flag, cross_project_feature_flag)
end
context 'when a user cannot read cross project and a filter is passed' do
before do
allow(described_class).to receive(:allowed?).and_call_original
expect(described_class).to receive(:allowed?).with(user, :read_cross_project) { false }
end
subject(:readable_feature_flags) do
read_cross_project_filter = -> (feature_flags) do
feature_flags.select { |flag| flag.project == project }
end
described_class.feature_flags_readable_by_user(
all_feature_flags, user,
filters: { read_cross_project: read_cross_project_filter }
)
end
it 'returns only feature flags of the specified project without checking access on others' do
expect(described_class).not_to receive(:allowed?).with(user, :read_feature_flag, cross_project_feature_flag)
expect(readable_feature_flags).to contain_exactly(feature_flag)
end
end
end
end
describe '.project_disabled_features_rules' do
let(:project) { create(:project, :wiki_disabled) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment