Commit 957fb5ec authored by Jarka Košanová's avatar Jarka Košanová

Refactor issuables link to be more general

- rename issues to issuables
- create concern for shared functionality (EpicLinks, EpicIssues)
parent da1fccf1
...@@ -114,7 +114,7 @@ export default { ...@@ -114,7 +114,7 @@ export default {
RelatedIssuesService.remove(issueToRemove.relation_path) RelatedIssuesService.remove(issueToRemove.relation_path)
.then(res => res.json()) .then(res => res.json())
.then(data => { .then(data => {
this.store.setRelatedIssues(data.issues); this.store.setRelatedIssues(data.issuables);
}) })
.catch(res => { .catch(res => {
if (res && res.status !== 404) { if (res && res.status !== 404) {
...@@ -142,7 +142,7 @@ export default { ...@@ -142,7 +142,7 @@ export default {
.then(data => { .then(data => {
// We could potentially lose some pending issues in the interim here // We could potentially lose some pending issues in the interim here
this.store.setPendingReferences([]); this.store.setPendingReferences([]);
this.store.setRelatedIssues(data.issues); this.store.setRelatedIssues(data.issuables);
this.isSubmitting = false; this.isSubmitting = false;
// Close the form on submission // Close the form on submission
......
...@@ -16,7 +16,7 @@ class RelatedIssuesService { ...@@ -16,7 +16,7 @@ class RelatedIssuesService {
return this.relatedIssuesResource.save( return this.relatedIssuesResource.save(
{}, {},
{ {
issue_references: newIssueReferences, issuable_references: newIssueReferences,
}, },
); );
} }
......
# frozen_string_literal: true
module EpicRelations
extend ActiveSupport::Concern
include IssuableLinks
included do
skip_before_action :authorize_destroy_issuable!
skip_before_action :authorize_create_epic!
skip_before_action :authorize_update_issuable!
before_action :authorize_admin_epic!, only: [:create, :destroy, :update]
end
def authorize_admin_epic!
render_403 unless can?(current_user, :admin_epic, epic)
end
end
...@@ -2,25 +2,33 @@ ...@@ -2,25 +2,33 @@
module IssuableLinks module IssuableLinks
def index def index
render json: issues render json: issuables
end end
def create def create
result = create_service.execute result = create_service.execute
render json: { message: result[:message], issues: issues }, status: result[:http_status] render json: { message: result[:message], issuables: issuables }, status: result[:http_status]
end end
def destroy def destroy
result = destroy_service.execute result = destroy_service.execute
render json: { issues: issues }, status: result[:http_status] render json: { issuables: issuables }, status: result[:http_status]
end end
private private
def issuables
list_service.execute
end
def list_service
raise NotImplementedError
end
def create_params def create_params
params.slice(:issue_references) params.slice(:issuable_references)
end end
def create_service def create_service
......
# frozen_string_literal: true # frozen_string_literal: true
class Groups::EpicIssuesController < Groups::EpicsController class Groups::EpicIssuesController < Groups::EpicsController
include IssuableLinks include EpicRelations
skip_before_action :authorize_destroy_issuable!
skip_before_action :authorize_create_epic!
skip_before_action :authorize_update_issuable!
before_action :authorize_admin_epic!, only: [:create, :destroy, :update]
before_action :authorize_issue_link_association!, only: [:destroy, :update] before_action :authorize_issue_link_association!, only: [:destroy, :update]
def update def update
...@@ -26,12 +21,8 @@ class Groups::EpicIssuesController < Groups::EpicsController ...@@ -26,12 +21,8 @@ class Groups::EpicIssuesController < Groups::EpicsController
EpicIssues::DestroyService.new(link, current_user) EpicIssues::DestroyService.new(link, current_user)
end end
def issues def list_service
EpicIssues::ListService.new(epic, current_user).execute EpicIssues::ListService.new(epic, current_user)
end
def authorize_admin_epic!
render_403 unless can?(current_user, :admin_epic, epic)
end end
def authorize_issue_link_association! def authorize_issue_link_association!
......
...@@ -9,10 +9,6 @@ module Projects ...@@ -9,10 +9,6 @@ module Projects
private private
def issues
IssueLinks::ListService.new(issue, current_user).execute
end
def authorize_admin_issue_link! def authorize_admin_issue_link!
render_403 unless can?(current_user, :admin_issue_link, @project) render_403 unless can?(current_user, :admin_issue_link, @project)
end end
...@@ -29,6 +25,10 @@ module Projects ...@@ -29,6 +25,10 @@ module Projects
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def list_service
IssueLinks::ListService.new(issue, current_user)
end
def create_service def create_service
IssueLinks::CreateService.new(issue, current_user, create_params) IssueLinks::CreateService.new(issue, current_user, create_params)
end end
......
...@@ -25,7 +25,7 @@ module EE ...@@ -25,7 +25,7 @@ module EE
epic_param = params.delete(:epic) epic_param = params.delete(:epic)
if epic_param if epic_param
EpicIssues::CreateService.new(epic_param, current_user, { target_issue: issue }).execute EpicIssues::CreateService.new(epic_param, current_user, { target_issuable: issue }).execute
else else
link = EpicIssue.find_by(issue_id: issue.id) # rubocop: disable CodeReuse/ActiveRecord link = EpicIssue.find_by(issue_id: issue.id) # rubocop: disable CodeReuse/ActiveRecord
......
...@@ -3,7 +3,7 @@ module EpicIssues ...@@ -3,7 +3,7 @@ module EpicIssues
private private
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def relate_issues(referenced_issue) def relate_issuables(referenced_issue)
link = EpicIssue.find_or_initialize_by(issue: referenced_issue) link = EpicIssue.find_or_initialize_by(issue: referenced_issue)
affected_epics = [issuable] affected_epics = [issuable]
...@@ -41,17 +41,17 @@ module EpicIssues ...@@ -41,17 +41,17 @@ module EpicIssues
{ group: issuable.group } { group: issuable.group }
end end
def linkable_issues(issues) def linkable_issuables(issues)
@linkable_issues ||= begin @linkable_issues ||= begin
return [] unless can?(current_user, :admin_epic, issuable.group) return [] unless can?(current_user, :admin_epic, issuable.group)
issues.select do |issue| issues.select do |issue|
issuable_group_descendants.include?(issue.project.group) && !previous_related_issues.include?(issue) issuable_group_descendants.include?(issue.project.group) && !previous_related_issuables.include?(issue)
end end
end end
end end
def previous_related_issues def previous_related_issuables
@related_issues ||= issuable.issues.to_a @related_issues ||= issuable.issues.to_a
end end
......
...@@ -2,7 +2,7 @@ module EpicIssues ...@@ -2,7 +2,7 @@ module EpicIssues
class ListService < IssuableLinks::ListService class ListService < IssuableLinks::ListService
private private
def issues def child_issuables
return [] unless issuable&.group&.feature_available?(:epics) return [] unless issuable&.group&.feature_available?(:epics)
issuable.issues_readable_by(current_user) issuable.issues_readable_by(current_user)
......
...@@ -11,58 +11,62 @@ module IssuableLinks ...@@ -11,58 +11,62 @@ module IssuableLinks
# otherwise create issue links for the issues which # otherwise create issue links for the issues which
# are still not assigned and return success message. # are still not assigned and return success message.
if render_conflict_error? if render_conflict_error?
return error('Issue(s) already assigned', 409) return error(issuables_assigned_message, 409)
end end
if render_not_found_error? if render_not_found_error?
return error('No Issue found for given params', 404) return error(issuables_not_found_message, 404)
end end
create_issue_links create_links
success success
end end
private private
def render_conflict_error? def render_conflict_error?
referenced_issues.present? && (referenced_issues - previous_related_issues).empty? referenced_issuables.present? && (referenced_issuables - previous_related_issuables).empty?
end end
def render_not_found_error? def render_not_found_error?
linkable_issues(referenced_issues).empty? linkable_issuables(referenced_issuables).empty?
end end
def create_issue_links def create_links
issues = linkable_issues(referenced_issues) objects = linkable_issuables(referenced_issuables)
issues.each do |referenced_issue| objects.each do |referenced_object|
relate_issues(referenced_issue) do |params| relate_issuables(referenced_object) do |params|
create_notes(referenced_issue, params) create_notes(referenced_object, params)
end end
end end
end end
def referenced_issues def referenced_issuables
@referenced_issues ||= begin @referenced_issuables ||= begin
target_issue = params[:target_issue] target_issuable = params[:target_issuable]
if params[:issue_references].present? if params[:issuable_references].present?
extract_issues_from_references extract_references
elsif target_issue elsif target_issuable
[target_issue] [target_issuable]
else else
[] []
end end
end end
end end
def extract_issues_from_references def extract_references
issue_references = params[:issue_references] issuable_references = params[:issuable_references]
text = issue_references.join(' ') text = issuable_references.join(' ')
extractor = Gitlab::ReferenceExtractor.new(issuable.project, @current_user) extractor = Gitlab::ReferenceExtractor.new(issuable.project, current_user)
extractor.analyze(text, extractor_context) extractor.analyze(text, extractor_context)
references(extractor)
end
def references(extractor)
extractor.issues extractor.issues
end end
...@@ -70,16 +74,24 @@ module IssuableLinks ...@@ -70,16 +74,24 @@ module IssuableLinks
{} {}
end end
def linkable_issues(issues) def linkable_issuables(objects)
raise NotImplementedError raise NotImplementedError
end end
def previous_related_issues def previous_related_issuables
raise NotImplementedError raise NotImplementedError
end end
def relate_issues(referenced_issue) def relate_issuables(referenced_object)
raise NotImplementedError raise NotImplementedError
end end
def issuables_assigned_message
'Issue(s) already assigned'
end
def issuables_not_found_message
'No Issue found for given params'
end
end end
end end
...@@ -9,29 +9,33 @@ module IssuableLinks ...@@ -9,29 +9,33 @@ module IssuableLinks
end end
def execute def execute
issues.map do |referenced_issue| child_issuables.map do |referenced_object|
to_hash(referenced_issue) to_hash(referenced_object)
end end
end end
private private
def relation_path(issue) def relation_path(object)
raise NotImplementedError raise NotImplementedError
end end
def reference(issue) def reference(object)
issue.to_reference(issuable.project) object.to_reference(issuable.project)
end end
def to_hash(issue) def issuable_path(object)
project_issue_path(object.project, object.iid)
end
def to_hash(object)
{ {
id: issue.id, id: object.id,
title: issue.title, title: object.title,
state: issue.state, state: object.state,
reference: reference(issue), reference: reference(object),
path: project_issue_path(issue.project, issue.iid), path: issuable_path(object),
relation_path: relation_path(issue) relation_path: relation_path(object)
} }
end end
end end
......
module IssueLinks module IssueLinks
class CreateService < IssuableLinks::CreateService class CreateService < IssuableLinks::CreateService
def relate_issues(referenced_issue) def relate_issuables(referenced_issue)
link = IssueLink.create(source: issuable, target: referenced_issue) link = IssueLink.create(source: issuable, target: referenced_issue)
yield if link.persisted? yield if link.persisted?
end end
def linkable_issues(issues) def linkable_issuables(issues)
@linkable_issues ||= begin @linkable_issuables ||= begin
issues.select { |issue| can?(current_user, :admin_issue_link, issue) } issues.select { |issue| can?(current_user, :admin_issue_link, issue) }
end end
end end
...@@ -17,7 +17,7 @@ module IssueLinks ...@@ -17,7 +17,7 @@ module IssueLinks
SystemNoteService.relate_issue(referenced_issue, issuable, current_user) SystemNoteService.relate_issue(referenced_issue, issuable, current_user)
end end
def previous_related_issues def previous_related_issuables
@related_issues ||= issuable.related_issues(current_user).to_a @related_issues ||= issuable.related_issues(current_user).to_a
end end
end end
......
...@@ -4,7 +4,7 @@ module IssueLinks ...@@ -4,7 +4,7 @@ module IssueLinks
private private
def issues def child_issuables
issuable.related_issues(current_user, preload: { project: :namespace }) issuable.related_issues(current_user, preload: { project: :namespace })
end end
......
...@@ -89,7 +89,7 @@ module API ...@@ -89,7 +89,7 @@ module API
issue = Issue.find(params[:issue_id]) issue = Issue.find(params[:issue_id])
create_params = { target_issue: issue } create_params = { target_issuable: issue }
result = ::EpicIssues::CreateService.new(epic, current_user, create_params).execute result = ::EpicIssues::CreateService.new(epic, current_user, create_params).execute
......
...@@ -35,7 +35,7 @@ module API ...@@ -35,7 +35,7 @@ module API
target_issue = find_project_issue(declared_params[:target_issue_iid], target_issue = find_project_issue(declared_params[:target_issue_iid],
declared_params[:target_project_id]) declared_params[:target_project_id])
create_params = { target_issue: target_issue } create_params = { target_issuable: target_issue }
result = ::IssueLinks::CreateService result = ::IssueLinks::CreateService
.new(source_issue, current_user, create_params) .new(source_issue, current_user, create_params)
......
...@@ -65,7 +65,7 @@ describe Groups::EpicIssuesController do ...@@ -65,7 +65,7 @@ describe Groups::EpicIssuesController do
subject do subject do
reference = [issue.to_reference(full: true)] reference = [issue.to_reference(full: true)]
post :create, group_id: group, epic_id: epic.to_param, issue_references: reference post :create, group_id: group, epic_id: epic.to_param, issuable_references: reference
end end
it_behaves_like 'unlicensed epics action' it_behaves_like 'unlicensed epics action'
...@@ -81,7 +81,7 @@ describe Groups::EpicIssuesController do ...@@ -81,7 +81,7 @@ describe Groups::EpicIssuesController do
list_service_response = EpicIssues::ListService.new(epic, user).execute list_service_response = EpicIssues::ListService.new(epic, user).execute
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response).to eq('message' => nil, 'issues' => list_service_response.as_json) expect(json_response).to eq('message' => nil, 'issuables' => list_service_response.as_json)
end end
it 'creates a new EpicIssue record' do it 'creates a new EpicIssue record' do
......
...@@ -163,7 +163,7 @@ describe('RelatedIssuesRoot', () => { ...@@ -163,7 +163,7 @@ describe('RelatedIssuesRoot', () => {
next( next(
request.respondWith( request.respondWith(
JSON.stringify({ JSON.stringify({
issues: [issuable1], issuables: [issuable1],
result: { result: {
message: 'something was successfully related', message: 'something was successfully related',
status: 'success', status: 'success',
...@@ -196,7 +196,7 @@ describe('RelatedIssuesRoot', () => { ...@@ -196,7 +196,7 @@ describe('RelatedIssuesRoot', () => {
next( next(
request.respondWith( request.respondWith(
JSON.stringify({ JSON.stringify({
issues: [issuable1, issuable2], issuables: [issuable1, issuable2],
result: { result: {
message: 'something was successfully related', message: 'something was successfully related',
status: 'success', status: 'success',
......
...@@ -38,26 +38,26 @@ describe Projects::IssueLinksController do ...@@ -38,26 +38,26 @@ describe Projects::IssueLinksController do
context 'with success' do context 'with success' do
let(:user_role) { :developer } let(:user_role) { :developer }
let(:issue_references) { [issue_b.to_reference] } let(:issuable_references) { [issue_b.to_reference] }
it 'returns success JSON' do it 'returns success JSON' do
post namespace_project_issue_links_path(issue_links_params(issue_references: issue_references)) post namespace_project_issue_links_path(issue_links_params(issuable_references: issuable_references))
list_service_response = IssueLinks::ListService.new(issue, user).execute list_service_response = IssueLinks::ListService.new(issue, user).execute
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response).to eq('message' => nil, expect(json_response).to eq('message' => nil,
'issues' => list_service_response.as_json) 'issuables' => list_service_response.as_json)
end end
end end
context 'with failure' do context 'with failure' do
context 'when unauthorized' do context 'when unauthorized' do
let(:user_role) { :guest } let(:user_role) { :guest }
let(:issue_references) { [issue_b.to_reference] } let(:issuable_references) { [issue_b.to_reference] }
it 'returns 403' do it 'returns 403' do
post namespace_project_issue_links_path(issue_links_params(issue_references: issue_references)) post namespace_project_issue_links_path(issue_links_params(issuable_references: issuable_references))
expect(response).to have_gitlab_http_status(403) expect(response).to have_gitlab_http_status(403)
end end
...@@ -65,15 +65,15 @@ describe Projects::IssueLinksController do ...@@ -65,15 +65,15 @@ describe Projects::IssueLinksController do
context 'when failing service result' do context 'when failing service result' do
let(:user_role) { :developer } let(:user_role) { :developer }
let(:issue_references) { ['#999'] } let(:issuable_references) { ['#999'] }
it 'returns failure JSON' do it 'returns failure JSON' do
post namespace_project_issue_links_path(issue_links_params(issue_references: issue_references)) post namespace_project_issue_links_path(issue_links_params(issuable_references: issuable_references))
list_service_response = IssueLinks::ListService.new(issue, user).execute list_service_response = IssueLinks::ListService.new(issue, user).execute
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
expect(json_response).to eq('message' => 'No Issue found for given params', 'issues' => list_service_response.as_json) expect(json_response).to eq('message' => 'No Issue found for given params', 'issuables' => list_service_response.as_json)
end end
end end
end end
...@@ -121,7 +121,7 @@ describe Projects::IssueLinksController do ...@@ -121,7 +121,7 @@ describe Projects::IssueLinksController do
list_service_response = IssueLinks::ListService.new(issue, user).execute list_service_response = IssueLinks::ListService.new(issue, user).execute
expect(json_response).to eq('issues' => list_service_response.as_json) expect(json_response).to eq('issuables' => list_service_response.as_json)
end end
end end
......
...@@ -14,7 +14,7 @@ describe EpicIssues::CreateService do ...@@ -14,7 +14,7 @@ describe EpicIssues::CreateService do
let!(:existing_link) { create(:epic_issue, epic: epic, issue: issue3) } let!(:existing_link) { create(:epic_issue, epic: epic, issue: issue3) }
def assign_issue(references) def assign_issue(references)
params = { issue_references: references } params = { issuable_references: references }
described_class.new(epic, user, params).execute described_class.new(epic, user, params).execute
end end
...@@ -93,9 +93,9 @@ describe EpicIssues::CreateService do ...@@ -93,9 +93,9 @@ describe EpicIssues::CreateService do
end end
context 'when the reference list is empty' do context 'when the reference list is empty' do
it 'returns an error' do subject { assign_issue([]) }
expect(assign_issue([])).to eq(message: 'No Issue found for given params', status: :error, http_status: 404)
end include_examples 'returns an error'
it 'does not create a system note' do it 'does not create a system note' do
expect { assign_issue([]) }.not_to change { Note.count } expect { assign_issue([]) }.not_to change { Note.count }
...@@ -124,7 +124,7 @@ describe EpicIssues::CreateService do ...@@ -124,7 +124,7 @@ describe EpicIssues::CreateService do
allow(extractor).to receive(:analyze) allow(extractor).to receive(:analyze)
allow(extractor).to receive(:issues).and_return([issue]) allow(extractor).to receive(:issues).and_return([issue])
params = { issue_references: [valid_reference] } params = { issuable_references: [valid_reference] }
control_count = ActiveRecord::QueryRecorder.new { described_class.new(epic, user, params).execute }.count control_count = ActiveRecord::QueryRecorder.new { described_class.new(epic, user, params).execute }.count
user = create(:user) user = create(:user)
...@@ -135,7 +135,7 @@ describe EpicIssues::CreateService do ...@@ -135,7 +135,7 @@ describe EpicIssues::CreateService do
group.add_developer(user) group.add_developer(user)
allow(extractor).to receive(:issues).and_return(issues) allow(extractor).to receive(:issues).and_return(issues)
params = { issue_references: issues.map { |i| i.to_reference(full: true) } } params = { issuable_references: issues.map { |i| i.to_reference(full: true) } }
# threshold 24 because 6 queries are generated for each insert # threshold 24 because 6 queries are generated for each insert
# (savepoint, find, exists, relative_position get, insert, release savepoint) # (savepoint, find, exists, relative_position get, insert, release savepoint)
...@@ -238,7 +238,7 @@ describe EpicIssues::CreateService do ...@@ -238,7 +238,7 @@ describe EpicIssues::CreateService do
let(:another_epic) { create(:epic, group: group) } let(:another_epic) { create(:epic, group: group) }
subject do subject do
params = { issue_references: [valid_reference] } params = { issuable_references: [valid_reference] }
described_class.new(another_epic, user, params).execute described_class.new(another_epic, user, params).execute
end end
......
...@@ -20,7 +20,7 @@ describe IssueLinks::CreateService do ...@@ -20,7 +20,7 @@ describe IssueLinks::CreateService do
context 'when the reference list is empty' do context 'when the reference list is empty' do
let(:params) do let(:params) do
{ issue_references: [] } { issuable_references: [] }
end end
it 'returns error' do it 'returns error' do
...@@ -30,7 +30,7 @@ describe IssueLinks::CreateService do ...@@ -30,7 +30,7 @@ describe IssueLinks::CreateService do
context 'when Issue not found' do context 'when Issue not found' do
let(:params) do let(:params) do
{ issue_references: ['#999'] } { issuable_references: ['#999'] }
end end
it 'returns error' do it 'returns error' do
...@@ -43,14 +43,14 @@ describe IssueLinks::CreateService do ...@@ -43,14 +43,14 @@ describe IssueLinks::CreateService do
end end
context 'when user has no permission to target project Issue' do context 'when user has no permission to target project Issue' do
let(:target_issue) { create :issue } let(:target_issuable) { create :issue }
let(:params) do let(:params) do
{ issue_references: [target_issue.to_reference(project)] } { issuable_references: [target_issuable.to_reference(project)] }
end end
it 'returns error' do it 'returns error' do
target_issue.project.add_guest(user) target_issuable.project.add_guest(user)
is_expected.to eq(message: 'No Issue found for given params', status: :error, http_status: 404) is_expected.to eq(message: 'No Issue found for given params', status: :error, http_status: 404)
end end
...@@ -62,7 +62,7 @@ describe IssueLinks::CreateService do ...@@ -62,7 +62,7 @@ describe IssueLinks::CreateService do
context 'source and target are the same issue' do context 'source and target are the same issue' do
let(:params) do let(:params) do
{ issue_references: [issue.to_reference] } { issuable_references: [issue.to_reference] }
end end
it 'does not create notes' do it 'does not create notes' do
...@@ -85,7 +85,7 @@ describe IssueLinks::CreateService do ...@@ -85,7 +85,7 @@ describe IssueLinks::CreateService do
let(:another_project_issue_ref) { another_project_issue.to_reference(project) } let(:another_project_issue_ref) { another_project_issue.to_reference(project) }
let(:params) do let(:params) do
{ issue_references: [issue_a_ref, another_project_issue_ref] } { issuable_references: [issue_a_ref, another_project_issue_ref] }
end end
before do before do
...@@ -129,7 +129,7 @@ describe IssueLinks::CreateService do ...@@ -129,7 +129,7 @@ describe IssueLinks::CreateService do
end end
let(:params) do let(:params) do
{ issue_references: [issue_b.to_reference, issue_a.to_reference] } { issuable_references: [issue_b.to_reference, issue_a.to_reference] }
end end
it 'returns success status' do it 'returns success status' do
......
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