Skip to content
Snippets Groups Projects
Commit 1238183c authored by Vasilii Iakliushin's avatar Vasilii Iakliushin :two:
Browse files

BranchRuleDelete mutation: fix 500 error when rule is missing

Contributes to #508213

**Problem**

When the branch rule is missing, the application raises an unhandled
exception that leads to InternalServer error.

**Solution**

Handle the exception in `GitlabSchema` class for non-active record
models and return a `nil` for missing objects.

Changelog: fixed
parent 3b60c652
No related branches found
No related tags found
1 merge request!175275BranchRuleDelete mutation: fix 500 error when rule is missing
......@@ -97,7 +97,12 @@ def find_by_gid(gid)
elsif gid.model_class.respond_to?(:lazy_find)
gid.model_class.lazy_find(gid.model_id)
else
gid.find
begin
gid.find
# other if conditions return nil when the record is not found
rescue ActiveRecord::RecordNotFound
nil
end
end
end
......
......@@ -143,6 +143,16 @@
described_class.object_from_id(user2.to_global_id)].map(&:sync)
end.not_to exceed_query_limit(1)
end
context 'when record is not found' do
let(:user) { build(:user, id: non_existing_record_id) }
it 'returns nil' do
result = described_class.object_from_id(user.to_global_id.to_s)
expect(result.sync).to be_nil
end
end
end
context 'with classes that are not ActiveRecord subclasses and have implemented .lazy_find' do
......@@ -163,6 +173,16 @@
described_class.object_from_id(note2.to_global_id)].map(&:sync)
end.not_to exceed_query_limit(1)
end
context 'when record is not found' do
let(:note) { build(:discussion_note_on_merge_request, id: non_existing_record_id) }
it 'returns nil' do
result = described_class.object_from_id(note.to_global_id.to_s)
expect(result.sync).to be_nil
end
end
end
context 'with other classes' do
......@@ -188,6 +208,18 @@ def initialize(id)
expect(described_class.object_from_id(result.to_global_id)).to eq(result)
end
context 'when class raises an ActiveRecord::RecordNotFound' do
before do
allow(TestGlobalId).to receive(:find).with("123").and_raise(ActiveRecord::RecordNotFound)
end
it 'returns nil' do
result = TestGlobalId.new(123)
expect(described_class.object_from_id(result.to_global_id)).to be_nil
end
end
end
it 'raises the correct error on invalid input' do
......
......@@ -56,5 +56,16 @@
expect { mutation_request }.not_to change { ProtectedBranch.count }
end
end
context 'when a branch rule is missing' do
let(:protected_branch) { build(:protected_branch, id: non_existing_record_id) }
it_behaves_like 'a mutation that returns top-level errors',
errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
it 'does not destroy the BranchRule' do
expect { mutation_request }.not_to change { ProtectedBranch.count }
end
end
end
end
......@@ -87,7 +87,7 @@
end
end
context 'when branch rule cannot be found' do
context 'when an invalid global id is given' do
let(:global_id) { project.to_gid.to_s }
let(:error_message) { %("#{global_id}" does not represent an instance of Projects::BranchRule) }
let(:global_id_error) { a_hash_including('message' => a_string_including(error_message)) }
......@@ -98,5 +98,12 @@
expect(graphql_errors).to include(global_id_error)
end
end
context 'when a branch rule is missing' do
let(:protected_branch) { build(:protected_branch, id: non_existing_record_id) }
it_behaves_like 'a mutation that returns top-level errors',
errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment