Commit 733e1012 authored by Stan Hu's avatar Stan Hu

Merge branch 'use-prepare-for-epic-starts-with-iid-verification' into 'master'

Use `ready?` for EpicsResolver parameter validation

See merge request gitlab-org/gitlab!27337
parents 6cc70ca6 119f3770
...@@ -38,15 +38,19 @@ module Resolvers ...@@ -38,15 +38,19 @@ module Resolvers
type Types::EpicType, null: true type Types::EpicType, null: true
def ready?(**args)
validate_timeframe_params!(args)
validate_starts_with_iid!(args)
super(args)
end
def resolve(**args) def resolve(**args)
@resolver_object = object.respond_to?(:sync) ? object.sync : object @resolver_object = object.respond_to?(:sync) ? object.sync : object
return [] unless resolver_object.present? return [] unless resolver_object.present?
return [] unless epic_feature_enabled? return [] unless epic_feature_enabled?
validate_timeframe_params!(args)
validate_iid_starts_with_query!(args)
find_epics(transform_args(args)) find_epics(transform_args(args))
end end
...@@ -86,16 +90,6 @@ module Resolvers ...@@ -86,16 +90,6 @@ module Resolvers
parent.group parent.group
end end
def validate_iid_starts_with_query!(args)
return unless args[:iid_starts_with].present?
iid_starts_with_query = args[:iid_starts_with]
unless EpicsFinder.valid_iid_query?(iid_starts_with_query)
raise Gitlab::Graphql::Errors::ArgumentError, "#{iid_starts_with_query} is not a valid iid search term"
end
end
# If we're querying for multiple iids and selecting issues, then ideally # If we're querying for multiple iids and selecting issues, then ideally
# we want to batch the epic and issue queries into one to reduce N+1 and memory. # we want to batch the epic and issue queries into one to reduce N+1 and memory.
# https://gitlab.com/gitlab-org/gitlab/issues/11841 # https://gitlab.com/gitlab-org/gitlab/issues/11841
...@@ -112,5 +106,13 @@ module Resolvers ...@@ -112,5 +106,13 @@ module Resolvers
def self.complexity_multiplier(args) def self.complexity_multiplier(args)
0.001 0.001
end end
def validate_starts_with_iid!(args)
return unless args[:iid_starts_with].present?
unless EpicsFinder.valid_iid_query?(args[:iid_starts_with])
raise Gitlab::Graphql::Errors::ArgumentError, 'Invalid `iidStartsWith` query'
end
end
end end
end end
...@@ -71,18 +71,6 @@ describe Resolvers::EpicsResolver do ...@@ -71,18 +71,6 @@ describe Resolvers::EpicsResolver do
expect(epics).to match_array([epic1, epic2]) expect(epics).to match_array([epic1, epic2])
end end
end end
context 'when only start_date is present' do
it 'raises error' do
expect { resolve_epics(start_date: '2019-08-13') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
end
end
context 'when only end_date is present' do
it 'raises error' do
expect { resolve_epics(end_date: '2019-08-13') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
end
end
end end
context 'with state' do context 'with state' do
...@@ -226,18 +214,6 @@ describe Resolvers::EpicsResolver do ...@@ -226,18 +214,6 @@ describe Resolvers::EpicsResolver do
expect(epics).to contain_exactly(epic5) expect(epics).to contain_exactly(epic5)
end end
it 'fails if iid_starts_with contains a non-numeric string' do
expect { resolve_epics(iid_starts_with: 'foo') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, 'foo is not a valid iid search term')
end
it 'fails if iid_starts_with contains a non-numeric string with line breaks' do
expect { resolve_epics(iid_starts_with: "foo\n1") }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, "foo\n1 is not a valid iid search term")
end
it 'fails if iid_starts_with contains a string which contains a negative number' do
expect { resolve_epics(iid_starts_with: '-1') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, '-1 is not a valid iid search term')
end
end end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe 'getting epics information' do
include GraphqlHelpers
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
before do
group.add_maintainer(user)
stub_licensed_features(epics: true)
end
describe 'query for epics which start with an iid' do
let_it_be(:epic1) { create(:epic, group: group, iid: 11) }
let_it_be(:epic2) { create(:epic, group: group, iid: 22) }
let_it_be(:epic3) { create(:epic, group: group, iid: 2223) }
let_it_be(:epic4) { create(:epic, group: group, iid: 29) }
context 'when a valid iidStartsWith query is provided' do
it 'returns the expected epics' do
query_epics_which_start_with_iid('22')
expect_epics_response(epic2, epic3)
end
end
context 'when invalid iidStartsWith query is provided' do
it 'fails with negative number' do
query_epics_which_start_with_iid('-2')
expect(graphql_errors).to include(a_hash_including('message' => 'Invalid `iidStartsWith` query'))
end
it 'fails with string' do
query_epics_which_start_with_iid('foo')
expect(graphql_errors).to include(a_hash_including('message' => 'Invalid `iidStartsWith` query'))
end
it 'fails if query contains line breaks' do
query_epics_which_start_with_iid('2\nfoo')
expect(graphql_errors).to include(a_hash_including('message' => 'Invalid `iidStartsWith` query'))
end
end
def query_epics_which_start_with_iid(iid)
post_graphql(epics_query(group, 'iidStartsWith', iid), current_user: user)
end
end
describe 'query for epics by time frame' do
let_it_be(:epic1) { create(:epic, group: group, state: :opened, start_date: "2019-08-13", end_date: "2019-08-20") }
let_it_be(:epic2) { create(:epic, group: group, state: :closed, start_date: "2019-08-13", end_date: "2019-08-21") }
let_it_be(:epic3) { create(:epic, group: group, state: :closed, start_date: "2019-08-22", end_date: "2019-08-26") }
let_it_be(:epic4) { create(:epic, group: group, state: :closed, start_date: "2019-08-10", end_date: "2019-08-12") }
context 'when start_date and end_date are present' do
it 'returns epics within timeframe' do
post_graphql(epics_query_by_hash(group, 'startDate' => '2019-08-13', 'endDate' => '2019-08-21'), current_user: user)
expect_epics_response(epic1, epic2)
end
end
context 'when only start_date is present' do
it 'raises error' do
post_graphql(epics_query(group, 'startDate', '2019-08-13'), current_user: user)
expect(graphql_errors).to include(a_hash_including('message' => 'Both startDate and endDate must be present.'))
end
end
context 'when only end_date is present' do
it 'raises error' do
post_graphql(epics_query(group, 'endDate', '2019-08-13'), current_user: user)
expect(graphql_errors).to include(a_hash_including('message' => 'Both startDate and endDate must be present.'))
end
end
end
def epics_query(group, field, value)
epics_query_by_hash(group, field => value)
end
def epics_query_by_hash(group, args)
field_queries = args.map { |key, value| "#{key}:\"#{value}\"" }.join(',')
<<~QUERY
query {
group(fullPath:"#{group.full_path}") {
id,
epics(#{field_queries}) {
nodes {
id
}
}
}
}
QUERY
end
def expect_epics_response(*epics)
actual_epics = graphql_data['group']['epics']['nodes'].map { |epic| epic['id'] }
expected_epics = epics.map { |epic| epic.to_global_id.to_s }
expect(actual_epics).to contain_exactly(*expected_epics)
expect(graphql_errors).to be_nil
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