Skip to content

G
gitlab-ce
Commit 91a735d4 authored by Oswaldo Ferreira's avatar Oswaldo Ferreira
Browse files
Options

Rename models, namespaces, routes to new IssueLink pattern

parent 7afdb317
Show whitespace changes
Inline Side-by-side
Showing with 204 additions and 114 deletions
app/controllers/projects/related_issues_controller.rb → app/controllers/projects/issue_links_controller.rb
View file @ 91a735d4
module Projects
class RelatedIssuesController < ApplicationController
before_action :authorize_read_related_issue!
before_action :authorize_admin_related_issue!, only: [:create, :destroy]
class IssueLinksController < ApplicationController
before_action :authorize_read_issue_link!
before_action :authorize_admin_issue_link!, only: [:create, :destroy]
def index
render json: issues
......@@ -9,7 +9,7 @@ module Projects
def create
opts = params.slice(:issue_references)
result = RelatedIssues::CreateService.new(issue, current_user, opts).execute
result = IssueLinks::CreateService.new(issue, current_user, opts).execute
if result[:status] == :success
render json: { result: result, issues: issues }, status: result[:http_status]
......@@ -19,13 +19,13 @@ module Projects
end
def destroy
related_issue = RelatedIssue.find(params[:id])
issue_link = IssueLink.find(params[:id])
# In order to remove a given relation, one must be allowed to admin_related_issue both the current
# In order to remove a given relation, one must be allowed to admin_issue_link both the current
# project and on the related issue project.
return render_404 unless can?(current_user, :admin_related_issue, related_issue.related_issue.project)
return render_404 unless can?(current_user, :admin_issue_link, issue_link.target.project)
result = RelatedIssues::DestroyService.new(related_issue, current_user).execute
result = IssueLinks::DestroyService.new(issue_link, current_user).execute
render json: { result: result, issues: issues }
end
......@@ -33,15 +33,15 @@ module Projects
private
def issues
RelatedIssues::ListService.new(issue, current_user).execute
IssueLinks::ListService.new(issue, current_user).execute
end
def authorize_admin_related_issue!
render_404 unless can?(current_user, :admin_related_issue, @project)
def authorize_admin_issue_link!
render_404 unless can?(current_user, :admin_issue_link, @project)
end
def authorize_read_related_issue!
render_404 unless can?(current_user, :read_related_issue, @project)
def authorize_read_issue_link!
render_404 unless can?(current_user, :read_issue_link, @project)
end
def issue
......
app/models/issue_link.rb 0 → 100644
View file @ 91a735d4
class IssueLink < ActiveRecord::Base
belongs_to :source, class_name: 'Issue'
belongs_to :target, class_name: 'Issue'
validates :source, presence: true
validates :target, presence: true
validates :source, uniqueness: { scope: :target_id, message: 'is already related' }
validate :check_self_relation
private
def check_self_relation
return unless source || target
if source == target
errors.add(:source, 'cannot be related to itself')
end
end
end
app/models/related_issue.rb deleted 100644 → 0
View file @ 7afdb317
class RelatedIssue < ActiveRecord::Base
belongs_to :issue
belongs_to :related_issue, class_name: 'Issue'
validates :issue, presence: true
validates :related_issue, presence: true
validates :issue, uniqueness: { scope: :related_issue_id, message: 'is already related' }
validate :check_self_relation
private
def check_self_relation
return unless issue || related_issue
if issue == related_issue
errors.add(:issue, 'cannot be related to itself')
end
end
end
app/policies/project_policy.rb
View file @ 91a735d4
......@@ -57,7 +57,7 @@ class ProjectPolicy < BasePolicy
end
# EE-only
can! :read_related_issue
can! :read_issue_link
end
def reporter_access!
......@@ -84,7 +84,7 @@ class ProjectPolicy < BasePolicy
end
# EE-only
can! :admin_related_issue
can! :admin_issue_link
end
# Permissions given when an user is team member of a project
......@@ -329,6 +329,6 @@ class ProjectPolicy < BasePolicy
can! :read_issue
# EE-only
can! :read_related_issue
can! :read_issue_link
end
end
app/services/related_issues/create_service.rb → app/services/issue_links/create_service.rb
View file @ 91a735d4
module RelatedIssues
module IssueLinks
class CreateService < BaseService
def initialize(issue, user, params)
@issue, @current_user, @params = issue, user, params.dup
......@@ -9,7 +9,7 @@ module RelatedIssues
return error('No Issue found for given reference', 401)
end
create_related_issues
create_issue_link
success_message
rescue => exception
......@@ -18,8 +18,8 @@ module RelatedIssues
private
def create_related_issues
RelatedIssue.transaction do
def create_issue_link
IssueLink.transaction do
referenced_issues.each do |referenced_issue|
relate_issues(referenced_issue)
create_notes(referenced_issue)
......@@ -28,7 +28,7 @@ module RelatedIssues
end
def relate_issues(referenced_issue)
RelatedIssue.create!(issue: @issue, related_issue: referenced_issue)
IssueLink.create!(source: @issue, target: referenced_issue)
end
def create_notes(referenced_issue)
......
app/services/related_issues/destroy_service.rb → app/services/issue_links/destroy_service.rb
View file @ 91a735d4
module RelatedIssues
module IssueLinks
class DestroyService < BaseService
def initialize(related_issue, user)
@related_issue = related_issue
def initialize(issue_link, user)
@issue_link = issue_link
@current_user = user
@issue = related_issue.issue
@referenced_issue = related_issue.related_issue
@issue = issue_link.source
@referenced_issue = issue_link.target
end
def execute
......@@ -17,7 +17,7 @@ module RelatedIssues
private
def remove_relation!
@related_issue.destroy!
@issue_link.destroy!
end
def create_notes!
......
app/services/related_issues/list_service.rb → app/services/issue_links/list_service.rb
View file @ 91a735d4
module RelatedIssues
module IssueLinks
class ListService
include Gitlab::Routing
......@@ -29,14 +29,14 @@ module RelatedIssues
# TODO: Simplify query using AR
@issues = Issue.find_by_sql(
<<-SQL.strip_heredoc
SELECT issues.*, related_issues.id as related_issues_id FROM issues
INNER JOIN related_issues ON related_issues.related_issue_id = issues.id
WHERE related_issues.issue_id = #{@issue.id} AND issues.deleted_at IS NULL
SELECT issues.*, issue_links.id as issue_links_id FROM issues
INNER JOIN issue_links ON issue_links.target_id = issues.id
WHERE issue_links.source_id = #{@issue.id} AND issues.deleted_at IS NULL
UNION ALL
SELECT issues.*, related_issues.id as related_issues_id FROM issues
INNER JOIN related_issues ON related_issues.issue_id = issues.id
WHERE related_issues.related_issue_id = #{@issue.id} AND issues.deleted_at IS NULL
ORDER BY related_issues_id
SELECT issues.*, issue_links.id as issue_links_id FROM issues
INNER JOIN issue_links ON issue_links.source_id = issues.id
WHERE issue_links.target_id = #{@issue.id} AND issues.deleted_at IS NULL
ORDER BY issue_links_id
SQL
)
......@@ -45,12 +45,12 @@ module RelatedIssues
end
def destroy_relation_path(issue)
return unless Ability.allowed?(@current_user, :admin_related_issue, issue.project)
return unless Ability.allowed?(@current_user, :admin_issue_link, issue.project)
namespace_project_issue_related_issue_path(issue.project.namespace,
namespace_project_issue_link_path(issue.project.namespace,
issue.project,
issue.iid,
issue.related_issues_id)
issue.issue_links_id)
end
end
end
config/locales/en.yml
View file @ 91a735d4
......@@ -3,6 +3,11 @@
en:
hello: "Hello world"
activerecord:
attributes:
issue_link:
source: Source issue
target: Target issue
errors:
messages:
label_already_exists_at_group_level: "already exists at group level for %{group}. Please choose another one."
......
config/routes/project.rb
View file @ 91a735d4
......@@ -312,7 +312,7 @@ constraints(ProjectUrlConstrainer.new) do
post :export_csv
end
resources :related_issues, only: [:index, :create, :destroy]
resources :issue_links, only: [:index, :create, :destroy], as: 'links', path: 'links'
end
resources :project_members, except: [:show, :new, :edit], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ }, concerns: :access_requestable do
......
spec/controllers/projects/related_issues_controller_spec.rb → spec/controllers/projects/issue_links_controller_spec.rb
View file @ 91a735d4
require 'rails_helper'
describe Projects::RelatedIssuesController, type: :controller do
describe Projects::IssueLinksController, type: :controller do
let(:user) { create :user }
let(:project) { create(:project_empty_repo) }
let(:issue) { create :issue, project: project }
describe 'GET #index' do
let(:service) { double(RelatedIssues::ListService, execute: service_response) }
let(:service) { double(IssueLinks::ListService, execute: service_response) }
let(:service_response) { [{ 'foo' => 'bar' }] }
before do
project.team << [user, :guest]
sign_in user
allow(RelatedIssues::ListService).to receive(:new)
allow(IssueLinks::ListService).to receive(:new)
.with(issue, user)
.and_return(service)
end
......@@ -32,8 +32,8 @@ describe Projects::RelatedIssuesController, type: :controller do
end
describe 'POST #create' do
let(:service) { double(RelatedIssues::CreateService, execute: service_response) }
let(:list_service) { double(RelatedIssues::ListService, execute: list_service_response) }
let(:service) { double(IssueLinks::CreateService, execute: service_response) }
let(:list_service) { double(IssueLinks::ListService, execute: list_service_response) }
let(:service_response) { { message: 'yay', status: :success } }
let(:list_service_response) { [{ 'foo' => 'bar' }] }
let(:issue_references) { double }
......@@ -43,11 +43,11 @@ describe Projects::RelatedIssuesController, type: :controller do
project.team << [user, user_role]
sign_in user
allow(RelatedIssues::ListService).to receive(:new)
allow(IssueLinks::ListService).to receive(:new)
.with(issue, user)
.and_return(list_service)
allow(RelatedIssues::CreateService).to receive(:new)
allow(IssueLinks::CreateService).to receive(:new)
.with(issue, user, { issue_references: issue_references })
.and_return(service)
end
......@@ -90,9 +90,9 @@ describe Projects::RelatedIssuesController, type: :controller do
describe 'DELETE #destroy' do
let(:referenced_issue) { create :issue, project: project }
let(:related_issue) { create :related_issue, related_issue: referenced_issue }
let(:service) { double(RelatedIssues::DestroyService, execute: service_response) }
let(:list_service) { double(RelatedIssues::ListService, execute: list_service_response) }
let(:issue_link) { create :issue_link, target: referenced_issue }
let(:service) { double(IssueLinks::DestroyService, execute: service_response) }
let(:list_service) { double(IssueLinks::ListService, execute: list_service_response) }
let(:service_response) { { 'message' => 'yay' } }
let(:list_service_response) { [{ 'foo' => 'bar' }] }
let(:current_project_user_role) { :developer }
......@@ -101,7 +101,7 @@ describe Projects::RelatedIssuesController, type: :controller do
delete :destroy, namespace_id: issue.project.namespace,
project_id: issue.project,
issue_id: issue,
id: related_issue,
id: issue_link,
format: :json
end
......@@ -109,12 +109,12 @@ describe Projects::RelatedIssuesController, type: :controller do
project.team << [user, current_project_user_role]
sign_in user
allow(RelatedIssues::ListService).to receive(:new)
allow(IssueLinks::ListService).to receive(:new)
.with(issue, user)
.and_return(list_service)
allow(RelatedIssues::DestroyService).to receive(:new)
.with(related_issue, user)
allow(IssueLinks::DestroyService).to receive(:new)
.with(issue_link, user)
.and_return(service)
end
......
spec/factories/issue_links.rb 0 → 100644
View file @ 91a735d4
FactoryGirl.define do
factory :issue_link do
source factory: :issue
target factory: :issue
end
end
spec/factories/related_issues.rb deleted 100644 → 0
View file @ 7afdb317
FactoryGirl.define do
factory :related_issue do
issue
related_issue factory: :issue
end
end
spec/models/related_issue_spec.rb → spec/models/issue_link_spec.rb
View file @ 91a735d4
require 'spec_helper'
describe RelatedIssue do
describe IssueLink do
describe 'Associations' do
it { is_expected.to belong_to(:issue) }
it { is_expected.to belong_to(:related_issue).class_name('Issue') }
it { is_expected.to belong_to(:source).class_name('Issue') }
it { is_expected.to belong_to(:target).class_name('Issue') }
end
describe 'Validation' do
subject { create :related_issue }
subject { create :issue_link }
it { is_expected.to validate_presence_of(:issue) }
it { is_expected.to validate_presence_of(:related_issue) }
it { is_expected.to validate_presence_of(:source) }
it { is_expected.to validate_presence_of(:target) }
it do
is_expected.to validate_uniqueness_of(:issue)
.scoped_to(:related_issue_id)
is_expected.to validate_uniqueness_of(:source)
.scoped_to(:target_id)
.with_message(/already related/)
end
context 'self relation' do
it 'invalidates object' do
issue = create :issue
related_issue = build :related_issue, issue: issue, related_issue: issue
issue_link = build :issue_link, source: issue, target: issue
expect(related_issue).to be_invalid
expect(related_issue.errors[:issue]).to include("cannot be related to itself")
expect(issue_link).to be_invalid
expect(issue_link.errors[:source]).to include("cannot be related to itself")
end
end
end
......
spec/policies/project_policy_spec.rb
View file @ 91a735d4
......@@ -13,7 +13,7 @@ describe ProjectPolicy, models: true do
let(:guest_permissions) do
%i[
read_project read_board read_list read_wiki read_issue read_label
read_related_issue read_milestone read_project_snippet read_project_member
read_issue_link read_milestone read_project_snippet read_project_member
read_note create_project create_issue create_note
upload_file
]
......@@ -22,7 +22,7 @@ describe ProjectPolicy, models: true do
let(:reporter_permissions) do
%i[
download_code fork_project create_project_snippet update_issue
admin_issue admin_label admin_related_issue admin_list read_commit_status read_build
admin_issue admin_label admin_issue_link admin_list read_commit_status read_build
read_container_image read_pipeline read_environment read_deployment
read_merge_request download_wiki_code
]
......@@ -71,7 +71,7 @@ describe ProjectPolicy, models: true do
let(:auditor_permissions) do
%i[
download_code download_wiki_code read_project read_board read_list
read_wiki read_issue read_label read_related_issue read_milestone read_project_snippet
read_wiki read_issue read_label read_issue_link read_milestone read_project_snippet
read_project_member read_note read_cycle_analytics read_pipeline
read_build read_commit_status read_container_image read_environment
read_deployment read_merge_request read_pages
......
spec/services/related_issues/create_service_spec.rb → spec/services/issue_links/create_service_spec.rb
View file @ 91a735d4
require 'spec_helper'
describe RelatedIssues::CreateService, service: true do
describe IssueLinks::CreateService, service: true do
describe '#execute' do
let(:namespace) { create :namespace }
let(:project) { create :empty_project, namespace: namespace }
......@@ -36,7 +36,7 @@ describe RelatedIssues::CreateService, service: true do
end
it 'no relationship is created' do
expect { subject }.not_to change(RelatedIssue, :count)
expect { subject }.not_to change(IssueLink, :count)
end
end
......@@ -52,7 +52,7 @@ describe RelatedIssues::CreateService, service: true do
end
it 'no relationship is created' do
expect { subject }.not_to change(RelatedIssue, :count)
expect { subject }.not_to change(IssueLink, :count)
end
end
......@@ -73,10 +73,10 @@ describe RelatedIssues::CreateService, service: true do
end
it 'creates relationships' do
expect { subject }.to change(RelatedIssue, :count).from(0).to(2)
expect { subject }.to change(IssueLink, :count).from(0).to(2)
expect(RelatedIssue.first).to have_attributes(issue: issue, related_issue: issue_a)
expect(RelatedIssue.last).to have_attributes(issue: issue, related_issue: another_project_issue)
expect(IssueLink.first).to have_attributes(source: issue, target: issue_a)
expect(IssueLink.last).to have_attributes(source: issue, target: another_project_issue)
end
it 'returns success message with Issue reference' do
......@@ -138,7 +138,7 @@ describe RelatedIssues::CreateService, service: true do
let(:issue_b) { create :issue, project: project }
before do
create :related_issue, issue: issue, related_issue: issue_a
create :issue_link, source: issue, target: issue_a
end
let(:params) do
......@@ -146,11 +146,11 @@ describe RelatedIssues::CreateService, service: true do
end
it 'returns error' do
is_expected.to eq(message: "Validation failed: Issue is already related", status: :error, http_status: 401)
is_expected.to eq(message: "Validation failed: Source issue is already related", status: :error, http_status: 401)
end
it 'no relation is created' do
expect { subject }.not_to change(RelatedIssue, :count)
expect { subject }.not_to change(IssueLink, :count)
end
end
end
......
spec/services/related_issues/destroy_service_spec.rb → spec/services/issue_links/destroy_service_spec.rb
View file @ 91a735d4
require 'spec_helper'
describe RelatedIssues::DestroyService, service: true do
describe IssueLinks::DestroyService, service: true do
describe '#execute' do
let(:user) { create :user }
let!(:related_issue) { create :related_issue }
let!(:issue_link) { create :issue_link }
subject { described_class.new(related_issue, user).execute }
subject { described_class.new(issue_link, user).execute }
it 'removes related issue' do
expect { subject }.to change(RelatedIssue, :count).from(1).to(0)
expect { subject }.to change(IssueLink, :count).from(1).to(0)
end
it 'creates notes' do
# Two-way notes creation
expect(SystemNoteService).to receive(:unrelate_issue)
.with(related_issue.issue, related_issue.related_issue, user)
.with(issue_link.source, issue_link.target, user)
expect(SystemNoteService).to receive(:unrelate_issue)
.with(related_issue.related_issue, related_issue.issue, user)
.with(issue_link.target, issue_link.source, user)
subject
end
......
spec/services/related_issues/list_service_spec.rb → spec/services/issue_links/list_service_spec.rb
View file @ 91a735d4
require 'spec_helper'
describe RelatedIssues::ListService, service: true do
describe IssueLinks::ListService, service: true do
let(:user) { create :user }
let(:project) { create(:project_empty_repo) }
let(:issue) { create :issue, project: project }
......@@ -17,22 +17,22 @@ describe RelatedIssues::ListService, service: true do
let(:issue_c) { create :issue, project: project }
let(:issue_d) { create :issue, project: project }
let!(:related_issue_c) do
create(:related_issue, id: 999,
issue: issue_d,
related_issue: issue)
let!(:issue_link_c) do
create(:issue_link, id: 999,
source: issue_d,
target: issue)
end
let!(:related_issue_b) do
create(:related_issue, id: 998,
issue: issue,
related_issue: issue_c)
let!(:issue_link_b) do
create(:issue_link, id: 998,
source: issue,
target: issue_c)
end
let!(:related_issue_a) do
create(:related_issue, id: 997,
issue: issue,
related_issue: issue_b)
let!(:issue_link_a) do
create(:issue_link, id: 997,
source: issue,
target: issue_b)
end
it 'verifies number of queries' do
......@@ -52,7 +52,7 @@ describe RelatedIssues::ListService, service: true do
path: "/#{project.full_path}/issues/#{issue_b.iid}",
project_path: issue_b.project.path,
namespace_full_path: issue_b.project.namespace.full_path,
destroy_relation_path: "/#{project.full_path}/issues/#{issue_b.iid}/related_issues/#{related_issue_a.id}"
destroy_relation_path: "/#{project.full_path}/issues/#{issue_b.iid}/links/#{issue_link_a.id}"
}
)
......@@ -65,7 +65,7 @@ describe RelatedIssues::ListService, service: true do
path: "/#{project.full_path}/issues/#{issue_c.iid}",
project_path: issue_c.project.path,
namespace_full_path: issue_c.project.namespace.full_path,
destroy_relation_path: "/#{project.full_path}/issues/#{issue_c.iid}/related_issues/#{related_issue_b.id}"
destroy_relation_path: "/#{project.full_path}/issues/#{issue_c.iid}/links/#{issue_link_b.id}"
}
)
......@@ -78,7 +78,7 @@ describe RelatedIssues::ListService, service: true do
path: "/#{project.full_path}/issues/#{issue_d.iid}",
project_path: issue_d.project.path,
namespace_full_path: issue_d.project.namespace.full_path,
destroy_relation_path: "/#{project.full_path}/issues/#{issue_d.iid}/related_issues/#{related_issue_c.id}"
destroy_relation_path: "/#{project.full_path}/issues/#{issue_d.iid}/links/#{issue_link_c.id}"
}
)
end
......@@ -87,8 +87,8 @@ describe RelatedIssues::ListService, service: true do
context 'referencing issue with removed relationships' do
context 'when referenced a deleted issue' do
let(:issue_b) { create :issue, project: project }
let!(:related_issue) do
create(:related_issue, issue: issue, related_issue: issue_b)
let!(:issue_link) do
create(:issue_link, source: issue, target: issue_b)
end
it 'ignores issue' do
......@@ -100,8 +100,8 @@ describe RelatedIssues::ListService, service: true do
context 'when referenced an issue with deleted project' do
let(:issue_b) { create :issue, project: project }
let!(:related_issue) do
create(:related_issue, issue: issue, related_issue: issue_b)
let!(:issue_link) do
create(:issue_link, source: issue, target: issue_b)
end
it 'ignores issue' do
......@@ -113,8 +113,8 @@ describe RelatedIssues::ListService, service: true do
context 'when referenced an issue with deleted namespace' do
let(:issue_b) { create :issue, project: project }
let!(:related_issue) do
create(:related_issue, issue: issue, related_issue: issue_b)
let!(:issue_link) do
create(:issue_link, source: issue, target: issue_b)
end
it 'ignores issue' do
......@@ -127,8 +127,8 @@ describe RelatedIssues::ListService, service: true do
context 'user cannot see relations' do
context 'when user cannot see the referenced issue' do
let!(:related_issue) do
create(:related_issue, issue: issue)
let!(:issue_link) do
create(:issue_link, source: issue)
end
it 'returns an empty list' do
......@@ -137,8 +137,8 @@ describe RelatedIssues::ListService, service: true do
end
context 'when user cannot see the issue that referenced' do
let!(:related_issue) do
create(:related_issue, related_issue: issue)
let!(:issue_link) do
create(:issue_link, target: issue)
end
it 'returns an empty list' do
......@@ -148,8 +148,8 @@ describe RelatedIssues::ListService, service: true do
end
context 'remove relations' do
let!(:related_issue) do
create(:related_issue, issue: issue, related_issue: referenced_issue)
let!(:issue_link) do
create(:issue_link, source: issue, target: referenced_issue)
end
context 'when user can admin related issues on one project' do
......@@ -171,7 +171,7 @@ describe RelatedIssues::ListService, service: true do
it 'returns related issue destroy relation path' do
expect(subject.first[:destroy_relation_path])
.to eq("/#{project.full_path}/issues/#{referenced_issue.iid}/related_issues/#{related_issue.id}")
.to eq("/#{project.full_path}/issues/#{referenced_issue.iid}/links/#{issue_link.id}")
end
end
end
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7