タグ循環の禁止 (#332) #382
@@ -6,6 +6,7 @@ class TagImplication < ApplicationRecord
|
|||||||
validates :parent_tag_id, presence: true
|
validates :parent_tag_id, presence: true
|
||||||
|
|
||||||
validate :parent_tag_mustnt_be_itself
|
validate :parent_tag_mustnt_be_itself
|
||||||
|
validate :parent_tag_mustnt_create_cycle
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
@@ -14,4 +15,27 @@ class TagImplication < ApplicationRecord
|
|||||||
errors.add :parent_tag_id, '親タグは子タグと同一であってはなりません.'
|
errors.add :parent_tag_id, '親タグは子タグと同一であってはなりません.'
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def parent_tag_mustnt_create_cycle
|
||||||
|
return if tag_id.blank? || parent_tag_id.blank?
|
||||||
|
return if errors[:parent_tag_id].present?
|
||||||
|
|
||||||
|
seen = { }
|
||||||
|
stack = [parent_tag_id]
|
||||||
|
|
||||||
|
until stack.empty?
|
||||||
|
current_id = stack.pop
|
||||||
|
next if seen[current_id]
|
||||||
|
|
||||||
|
seen[current_id] = true
|
||||||
|
|
||||||
|
if current_id == tag_id
|
||||||
|
errors.add :parent_tag_id, '親タグに子孫タグを指定すると循環します.'
|
||||||
|
errors.add :base, 'タグの親子関係が循環します.'
|
||||||
|
return
|
||||||
|
end
|
||||||
|
|
||||||
|
stack.concat(TagImplication.where(tag_id: current_id).pluck(:parent_tag_id))
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -0,0 +1,36 @@
|
|||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
RSpec.describe TagImplication, type: :model do
|
||||||
|
it 'rejects a parent tag that would create a cycle' do
|
||||||
|
child = create(:tag, name: 'tag_implication_cycle_child')
|
||||||
|
parent = create(:tag, name: 'tag_implication_cycle_parent')
|
||||||
|
|
||||||
|
described_class.create!(tag: child, parent_tag: parent)
|
||||||
|
|
||||||
|
implication = described_class.new(tag: parent, parent_tag: child)
|
||||||
|
|
||||||
|
expect(implication).not_to be_valid
|
||||||
|
expect(implication.errors[:parent_tag_id]).to include(
|
||||||
|
'親タグに子孫タグを指定すると循環します.'
|
||||||
|
)
|
||||||
|
expect(implication.errors[:base]).to be_present
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'terminates even when existing data already contains a cycle' do
|
||||||
|
child = create(:tag, name: 'tag_implication_existing_cycle_child')
|
||||||
|
parent = create(:tag, name: 'tag_implication_existing_cycle_parent')
|
||||||
|
ancestor = create(:tag, name: 'tag_implication_existing_cycle_ancestor')
|
||||||
|
|
||||||
|
described_class.create!(tag: parent, parent_tag: ancestor)
|
||||||
|
described_class.insert_all!(
|
||||||
|
[
|
||||||
|
{ tag_id: ancestor.id, parent_tag_id: parent.id,
|
||||||
|
created_at: Time.current, updated_at: Time.current }
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
implication = described_class.new(tag: child, parent_tag: parent)
|
||||||
|
|
||||||
|
expect(implication).to be_valid
|
||||||
|
end
|
||||||
|
end
|
||||||
@@ -54,7 +54,17 @@ RSpec.describe Tag, type: :model do
|
|||||||
first = create(:tag, name: 'expand_cycle_first')
|
first = create(:tag, name: 'expand_cycle_first')
|
||||||
second = create(:tag, name: 'expand_cycle_second')
|
second = create(:tag, name: 'expand_cycle_second')
|
||||||
TagImplication.create!(tag: first, parent_tag: second)
|
TagImplication.create!(tag: first, parent_tag: second)
|
||||||
TagImplication.create!(tag: second, parent_tag: first)
|
now = Time.current
|
||||||
|
TagImplication.insert_all!(
|
||||||
|
[
|
||||||
|
{
|
||||||
|
tag_id: second.id,
|
||||||
|
parent_tag_id: first.id,
|
||||||
|
created_at: now,
|
||||||
|
updated_at: now
|
||||||
|
}
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
expect(described_class.expand_parent_tags([first])).to contain_exactly(first, second)
|
expect(described_class.expand_parent_tags([first])).to contain_exactly(first, second)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -56,6 +56,17 @@ RSpec.describe "TagChildren", type: :request do
|
|||||||
|
|
||||||
expect(response).to have_http_status(:no_content)
|
expect(response).to have_http_status(:no_content)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'returns 422 and does not create relation when the new link makes a cycle' do
|
||||||
|
TagImplication.create!(tag: parent, parent_tag: child)
|
||||||
|
|
||||||
|
expect {
|
||||||
|
do_request
|
||||||
|
}.not_to change(TagImplication, :count)
|
||||||
|
|
||||||
|
expect(response).to have_http_status(:unprocessable_entity)
|
||||||
|
expect(TagImplication.where(tag: child, parent_tag: parent)).not_to exist
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when Tag.find raises (invalid ids)" do
|
context "when Tag.find raises (invalid ids)" do
|
||||||
|
|||||||
@@ -797,7 +797,17 @@ RSpec.describe 'Tags API', type: :request do
|
|||||||
)
|
)
|
||||||
TagImplication.create!(tag: first, parent_tag: root_material)
|
TagImplication.create!(tag: first, parent_tag: root_material)
|
||||||
TagImplication.create!(tag: second, parent_tag: first)
|
TagImplication.create!(tag: second, parent_tag: first)
|
||||||
TagImplication.create!(tag: first, parent_tag: second)
|
now = Time.current
|
||||||
|
TagImplication.insert_all!(
|
||||||
|
[
|
||||||
|
{
|
||||||
|
tag_id: first.id,
|
||||||
|
parent_tag_id: second.id,
|
||||||
|
created_at: now,
|
||||||
|
updated_at: now
|
||||||
|
}
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
get '/tags/with-depth', params: { parent: root_material.id }
|
get '/tags/with-depth', params: { parent: root_material.id }
|
||||||
|
|
||||||
@@ -1364,8 +1374,6 @@ RSpec.describe 'Tags API', type: :request do
|
|||||||
end
|
end
|
||||||
|
|
||||||
it 'parent_tags に指定すると循環する tag は 422 にする' do
|
it 'parent_tags に指定すると循環する tag は 422 にする' do
|
||||||
pending '#332 で対応予定'
|
|
||||||
|
|
||||||
child = Tag.create!(
|
child = Tag.create!(
|
||||||
tag_name: TagName.create!(name: 'put_cycle_child'),
|
tag_name: TagName.create!(name: 'put_cycle_child'),
|
||||||
category: :general
|
category: :general
|
||||||
|
|||||||
新しい課題から参照
ユーザをブロックする