Fix conflicting redirect search

This commit is contained in:
Michael Kozono 2017-08-07 16:41:39 -07:00
parent cfabe7bf38
commit fccd64bb6a
3 changed files with 62 additions and 28 deletions

View file

@ -8,5 +8,5 @@ class RedirectRoute < ActiveRecord::Base
presence: true,
uniqueness: { case_sensitive: false }
scope :matching_path_and_descendants, -> (path) { where('redirect_routes.path = ? OR redirect_routes.path LIKE ?', path, "#{sanitize_sql_like(path)}/%") }
scope :matching_path_and_descendants, -> (path) { where('LOWER(redirect_routes.path) = LOWER(?) OR LOWER(redirect_routes.path) LIKE LOWER(?)', path, "#{sanitize_sql_like(path)}/%") }
end

View file

@ -20,8 +20,16 @@ describe RedirectRoute do
let!(:redirect4) { group.redirect_routes.create(path: 'gitlabb/test/foo/bar') }
let!(:redirect5) { group.redirect_routes.create(path: 'gitlabb/test/baz') }
it 'returns correct routes' do
expect(described_class.matching_path_and_descendants('gitlabb/test')).to match_array([redirect2, redirect3, redirect4, redirect5])
context 'when the redirect route matches with same casing' do
it 'returns correct routes' do
expect(described_class.matching_path_and_descendants('gitlabb/test')).to match_array([redirect2, redirect3, redirect4, redirect5])
end
end
context 'when the redirect route matches with different casing' do
it 'returns correct routes' do
expect(described_class.matching_path_and_descendants('GitLABB/test')).to match_array([redirect2, redirect3, redirect4, redirect5])
end
end
end
end

View file

@ -145,45 +145,71 @@ describe Route do
describe '#delete_conflicting_redirects' do
context 'when a redirect route with the same path exists' do
let!(:redirect1) { route.create_redirect(route.path) }
context 'when the redirect route has matching case' do
let!(:redirect1) { route.create_redirect(route.path) }
it 'deletes the redirect' do
route.delete_conflicting_redirects
expect(route.conflicting_redirects).to be_empty
it 'deletes the redirect' do
expect do
route.delete_conflicting_redirects
end.to change{RedirectRoute.count}.by(-1)
end
context 'when redirect routes with paths descending from the route path exists' do
let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
let!(:other_redirect) { route.create_redirect("other") }
it 'deletes all redirects with paths that descend from the route path' do
expect do
route.delete_conflicting_redirects
end.to change{RedirectRoute.count}.by(-4)
end
end
end
context 'when redirect routes with paths descending from the route path exists' do
let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
let!(:other_redirect) { route.create_redirect("other") }
context 'when the redirect route is differently cased' do
let!(:redirect1) { route.create_redirect(route.path.upcase) }
it 'deletes all redirects with paths that descend from the route path' do
route.delete_conflicting_redirects
expect(route.conflicting_redirects).to be_empty
it 'deletes the redirect' do
expect do
route.delete_conflicting_redirects
end.to change{RedirectRoute.count}.by(-1)
end
end
end
end
describe '#conflicting_redirects' do
context 'when a redirect route with the same path exists' do
let!(:redirect1) { route.create_redirect(route.path) }
it 'returns an ActiveRecord::Relation' do
expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation)
end
it 'returns the redirect route' do
expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation)
expect(route.conflicting_redirects).to match_array([redirect1])
context 'when a redirect route with the same path exists' do
context 'when the redirect route has matching case' do
let!(:redirect1) { route.create_redirect(route.path) }
it 'returns the redirect route' do
expect(route.conflicting_redirects).to match_array([redirect1])
end
context 'when redirect routes with paths descending from the route path exists' do
let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
let!(:other_redirect) { route.create_redirect("other") }
it 'returns the redirect routes' do
expect(route.conflicting_redirects).to match_array([redirect1, redirect2, redirect3, redirect4])
end
end
end
context 'when redirect routes with paths descending from the route path exists' do
let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
let!(:other_redirect) { route.create_redirect("other") }
context 'when the redirect route is differently cased' do
let!(:redirect1) { route.create_redirect(route.path.upcase) }
it 'returns the redirect routes' do
expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation)
expect(route.conflicting_redirects).to match_array([redirect1, redirect2, redirect3, redirect4])
it 'returns the redirect route' do
expect(route.conflicting_redirects).to match_array([redirect1])
end
end
end