Commit f62a0a7b authored by Alex Kalderimis's avatar Alex Kalderimis

Add and update tests

Add specs to check for designs fields
---------------------------------------

This also includes changes derived from review feedback:

* Move parameter validation to separate method
* hide private implementation
* use named parameters

Add requests to test full GraphQL lifecycle
---------------------------------------------

This adds new GraphQL feature tests to verify the ability to query for
designs and designs-at-versions throughout the graph

Add tests for pagination
----------------------------

This adds tests for pagination over designs, versions and
designs-at-versions

It also changes graphql_helpers so that attribute serialization is now
sensitive to the type of the argument (so that pagination arguments can
be passed easily), which required changes elsewhere
in the codebase.
parent 3801ff88
......@@ -8,6 +8,7 @@ module DesignManagement
# ids: integer[]
# filenames: string[]
# visible_at_version: ?version
# filenames: String[]
def initialize(issue, current_user, params = {})
@issue = issue
@current_user = current_user
......
......@@ -12,15 +12,7 @@ module Resolvers
description: 'Find a design by its filename'
def resolve(filename: nil, id: nil)
params = if !filename.present? && !id.present?
error('one of id or filename must be passed')
elsif filename.present? && id.present?
error('only one of id or filename may be passed')
elsif filename.present?
{ filenames: [filename] }
else
{ ids: [GitlabSchema.parse_gid(id, expected_type: ::DesignManagement::Design).model_id] }
end
params = parse_args(filename, id)
build_finder(params).execute.first
end
......@@ -35,17 +27,31 @@ module Resolvers
object.issue
end
def user
context[:current_user]
end
def build_finder(params)
::DesignManagement::DesignsFinder.new(issue, user, params)
::DesignManagement::DesignsFinder.new(issue, current_user, params)
end
def error(msg)
raise ::Gitlab::Graphql::Errors::ArgumentError, msg
end
def parse_args(filename, id)
provided = [filename, id].map(&:present?)
if provided.none?
error('one of id or filename must be passed')
elsif provided.all?
error('only one of id or filename may be passed')
elsif filename.present?
{ filenames: [filename] }
else
{ ids: [parse_gid(id)] }
end
end
def parse_gid(gid)
GitlabSchema.parse_gid(gid, expected_type: ::DesignManagement::Design).model_id
end
end
end
end
......@@ -21,33 +21,30 @@ module Resolvers
::Resolvers::DesignManagement::DesignResolver
end
def resolve(**args)
find_designs(args)
def resolve(ids: nil, filenames: nil, at_version: nil)
::DesignManagement::DesignsFinder.new(
issue,
current_user,
ids: design_ids(ids),
filenames: filenames,
visible_at_version: version(at_version),
order: :id
).execute
end
private
def version(args)
args[:at_version] ? GitlabSchema.object_from_id(args[:at_version])&.sync : nil
def version(at_version)
GitlabSchema.object_from_id(at_version)&.sync if at_version
end
def design_ids(args)
args[:ids] ? args[:ids].map { |id| GlobalID.parse(id).model_id } : nil
def design_ids(ids)
ids&.map { |id| GlobalID.parse(id).model_id }
end
def issue
object.issue
end
def find_designs(args)
::DesignManagement::DesignsFinder.new(
issue,
context[:current_user],
ids: design_ids(args),
filenames: args[:filenames],
visible_at_version: version(args)
).execute
end
end
end
end
......@@ -11,6 +11,7 @@ module Resolvers
as: :sha,
required: false,
description: 'The SHA256 of the most recent acceptable version'
argument :earlier_or_equal_to_id, GraphQL::ID_TYPE,
as: :id,
required: false,
......@@ -25,14 +26,17 @@ module Resolvers
version = cutoff(parent, id, sha)
raise ::Gitlab::Graphql::Errors::ResourceNotAvailable, 'cutoff not found' unless version.present?
return find if version == :unconstrained
if version == :unconstrained
find
else
find(earlier_or_equal_to: version)
end
end
private
# Find the version most recent version that the client will accept
# Find the most recent version that the client will accept
def cutoff(parent, id, sha)
if sha.present? || id.present?
specific_version(id, sha)
......
# frozen_string_literal: true
require "spec_helper"
require 'spec_helper'
describe Resolvers::DesignManagement::DesignResolver do
include GraphqlHelpers
......@@ -10,7 +10,7 @@ describe Resolvers::DesignManagement::DesignResolver do
enable_design_management
end
describe "#resolve" do
describe '#resolve' do
let_it_be(:issue) { create(:issue) }
let_it_be(:project) { issue.project }
let_it_be(:first_version) { create(:design_version) }
......@@ -27,55 +27,55 @@ describe Resolvers::DesignManagement::DesignResolver do
project.add_developer(current_user)
end
context "when the user cannot see designs" do
context 'when the user cannot see designs' do
let(:gql_context) { { current_user: create(:user) } }
it "returns nothing" do
it 'returns nothing' do
expect(resolve_design).to be_nil
end
end
context "when no argument has been passed" do
context 'when no argument has been passed' do
let(:args) { {} }
it 'raises an error' do
expect { resolve_design }.to raise_error(::Gitlab::Graphql::Errors::ArgumentError)
expect { resolve_design }.to raise_error(::Gitlab::Graphql::Errors::ArgumentError, /must/)
end
end
context "when both arguments have been passed" do
context 'when both arguments have been passed' do
let(:args) { { filename: first_design.filename, id: GitlabSchema.id_from_object(first_design).to_s } }
it 'raises an error' do
expect { resolve_design }.to raise_error(::Gitlab::Graphql::Errors::ArgumentError)
expect { resolve_design }.to raise_error(::Gitlab::Graphql::Errors::ArgumentError, /may/)
end
end
context "by ID" do
it "returns the specified design" do
context 'by ID' do
it 'returns the specified design' do
expect(resolve_design).to eq(first_design)
end
context 'the ID belongs to a design on another issue' do
let(:args) { { id: GitlabSchema.id_from_object(design_on_other_issue).to_s } }
it "returns nothing" do
it 'returns nothing' do
expect(resolve_design).to be_nil
end
end
end
context "by filename" do
context 'by filename' do
let(:args) { { filename: first_design.filename } }
it "returns the specified design" do
it 'returns the specified design' do
expect(resolve_design).to eq(first_design)
end
context 'the filename belongs to a design on another issue' do
let(:args) { { filename: design_on_other_issue.filename } }
it "returns nothing" do
it 'returns nothing' do
expect(resolve_design).to be_nil
end
end
......
......@@ -8,4 +8,6 @@ describe GitlabSchema.types['Issue'] do
it { expect(described_class).to have_graphql_field(:weight) }
it { expect(described_class).to have_graphql_field(:designs) }
it { expect(described_class).to have_graphql_field(:design_collection) }
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Query.project(fullPath).issue(iid).designCollection.version(sha)' do
include GraphqlHelpers
include DesignManagementTestHelpers
let_it_be(:project) { create(:project) }
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:developer) { create(:user) }
let_it_be(:stranger) { create(:user) }
let_it_be(:old_version) do
create(:design_version, issue: issue,
created_designs: create_list(:design, 3, issue: issue))
end
let_it_be(:version) do
create(:design_version, issue: issue,
modified_designs: old_version.designs,
created_designs: create_list(:design, 2, issue: issue))
end
let(:current_user) { developer }
def query(vq = version_fields)
graphql_query_for(:project, { fullPath: project.full_path },
query_graphql_field(:issue, { iid: issue.iid.to_s },
query_graphql_field(:design_collection, nil,
query_graphql_field(:version, { sha: version.sha }, vq))))
end
let(:post_query) { post_graphql(query, current_user: current_user) }
let(:path_prefix) { %w[project issue designCollection version] }
let(:data) { graphql_data.dig(*path) }
before do
enable_design_management
project.add_developer(developer)
end
describe 'scalar fields' do
let(:path) { path_prefix }
let(:version_fields) { query_graphql_field(:sha) }
before do
post_query
end
{ id: ->(x) { x.to_global_id.to_s }, sha: ->(x) { x.sha } }.each do |field, value|
describe ".#{field}" do
let(:version_fields) { query_graphql_field(field) }
it "retrieves the #{field}" do
expect(data).to match(a_hash_including(field.to_s => value[version]))
end
end
end
end
describe 'design_at_version' do
let(:path) { path_prefix + %w[designAtVersion] }
let(:design) { issue.designs.visible_at_version(version).to_a.sample }
let(:design_at_version) { build(:design_at_version, design: design, version: version) }
let(:version_fields) do
query_graphql_field(:design_at_version, dav_params, 'id filename')
end
shared_examples :finds_dav do
it 'finds all the designs as of the given version' do
post_query
expect(data).to match(
a_hash_including(
'id' => global_id_of(design_at_version),
'filename' => design.filename
))
end
context 'when the current_user is not authorized' do
let(:current_user) { stranger }
it 'returns nil' do
post_query
expect(data).to be_nil
end
end
end
context 'by ID' do
let(:dav_params) { { id: global_id_of(design_at_version) } }
include_examples :finds_dav
end
context 'by filename' do
let(:dav_params) { { filename: design.filename } }
include_examples :finds_dav
end
context 'by design_id' do
let(:dav_params) { { design_id: global_id_of(design) } }
include_examples :finds_dav
end
end
describe 'designs_at_version' do
let(:path) { path_prefix + %w[designsAtVersion edges] }
let(:version_fields) do
query_graphql_field(:designs_at_version, dav_params, 'edges { node { id filename } }')
end
let(:dav_params) { nil }
let(:results) do
issue.designs.visible_at_version(version).map do |d|
dav = build(:design_at_version, design: d, version: version)
{ 'id' => global_id_of(dav), 'filename' => d.filename }
end
end
it 'finds all the designs as of the given version' do
post_query
expect(data.pluck('node')).to match_array(results)
end
describe 'filtering' do
let(:designs) { issue.designs.sample(3) }
let(:filenames) { designs.map(&:filename) }
let(:ids) do
designs.map { |d| global_id_of(build(:design_at_version, design: d, version: version)) }
end
before do
post_query
end
describe 'by filename' do
let(:dav_params) { { filenames: filenames } }
it 'finds the designs by filename' do
expect(data.map { |e| e.dig('node', 'id') }).to match_array(ids)
end
end
describe 'by design-id' do
let(:dav_params) { { ids: designs.map { |d| global_id_of(d) } } }
it 'finds the designs by id' do
expect(data.map { |e| e.dig('node', 'filename') }).to match_array(filenames)
end
end
end
describe 'pagination' do
let(:end_cursor) { graphql_data_at(*path_prefix, :designs_at_version, :page_info, :end_cursor) }
let(:ids) do
::DesignManagement::Design.visible_at_version(version).order(:id).map do |d|
global_id_of(build(:design_at_version, design: d, version: version))
end
end
let(:version_fields) do
query_graphql_field(:designs_at_version, { first: 2 }, fields)
end
let(:cursored_query) do
frag = query_graphql_field(:designs_at_version, { after: end_cursor }, fields)
query(frag)
end
let(:fields) { ['pageInfo { endCursor }', 'edges { node { id } }'] }
def response_values(data = graphql_data)
data.dig(*path).map { |e| e.dig('node', 'id') }
end
it 'sorts designs for reliable pagination' do
post_graphql(query, current_user: current_user)
expect(response_values).to match_array(ids.take(2))
post_graphql(cursored_query, current_user: current_user)
new_data = JSON.parse(response.body).fetch('data')
expect(response_values(new_data)).to match_array(ids.drop(2))
end
end
end
describe 'designs' do
let(:path) { path_prefix + %w[designs edges] }
let(:version_fields) do
query_graphql_field(:designs, nil, 'edges { node { id filename } }')
end
let(:results) do
version.designs.map do |design|
{ 'id' => global_id_of(design), 'filename' => design.filename }
end
end
it 'finds all the designs as of the given version' do
post_query
expect(data.pluck('node')).to match_array(results)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Getting versions related to an issue' do
include GraphqlHelpers
include DesignManagementTestHelpers
let_it_be(:issue) { create(:issue) }
let_it_be(:version_a) do
create(:design_version, issue: issue)
end
let_it_be(:version_b) do
create(:design_version, issue: issue)
end
let_it_be(:version_c) do
create(:design_version, issue: issue)
end
let_it_be(:version_d) do
create(:design_version, issue: issue)
end
let_it_be(:owner) { issue.project.owner }
def version_query(params = version_params)
query_graphql_field(:versions, params, version_query_fields)
end
let(:version_params) { nil }
let(:version_query_fields) { ['edges { node { sha } }'] }
let(:project) { issue.project }
let(:current_user) { owner }
let(:query) { make_query }
def make_query(vq = version_query)
graphql_query_for(:project, { fullPath: project.full_path },
query_graphql_field(:issue, { iid: issue.iid.to_s },
query_graphql_field(:design_collection, {}, vq)))
end
let(:design_collection) do
graphql_data_at(:project, :issue, :design_collection)
end
def response_values(data = graphql_data, key = 'sha')
path = %w[project issue designCollection versions edges]
data.dig(*path).map { |e| e.dig('node', key) }
end
before do
enable_design_management
end
it 'returns the design filename' do
post_graphql(query, current_user: current_user)
expect(response_values).to match_array([version_a, version_b, version_c, version_d].map(&:sha))
end
describe 'filter by sha' do
let(:sha) { version_b.sha }
let(:version_params) { { earlier_or_equal_to_sha: sha } }
it 'finds only those versions at or before the given cut-off' do
post_graphql(query, current_user: current_user)
expect(response_values).to contain_exactly(version_a.sha, version_b.sha)
end
end
describe 'filter by id' do
let(:id) { global_id_of(version_c) }
let(:version_params) { { earlier_or_equal_to_id: id } }
it 'finds only those versions at or before the given cut-off' do
post_graphql(query, current_user: current_user)
expect(response_values).to contain_exactly(version_a.sha, version_b.sha, version_c.sha)
end
end
describe 'pagination' do
let(:end_cursor) { design_collection.dig('versions', 'pageInfo', 'endCursor') }
let(:ids) { issue.design_collection.versions.ordered.map(&:sha) }
let(:query) { make_query(version_query(first: 2)) }
let(:cursored_query) do
make_query(version_query(after: end_cursor))
end
let(:version_query_fields) { ['pageInfo { endCursor }', 'edges { node { sha } }'] }
it 'sorts designs for reliable pagination' do
post_graphql(query, current_user: current_user)
expect(response_values).to match_array(ids.take(2))
post_graphql(cursored_query, current_user: current_user)
new_data = JSON.parse(response.body).fetch('data')
expect(response_values(new_data)).to match_array(ids.drop(2))
end
end
end
......@@ -60,7 +60,7 @@ describe 'Getting designs related to an issue' do
{ 'fullPath' => design.project.full_path },
query_graphql_field(
'issue',
{ iid: design.issue.iid },
{ iid: design.issue.iid.to_s },
query_graphql_field(
'designs', {}, design_node
)
......
# frozen_string_literal: true
require 'spec_helper'
describe 'Query.project(fullPath).issue(iid)' do
include GraphqlHelpers
let_it_be(:project) { create(:project) }
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:issue_b) { create(:issue, project: project) }
let_it_be(:developer) { create(:user) }
let(:current_user) { developer }
let_it_be(:project_params) { { 'fullPath' => project.full_path } }
let_it_be(:issue_params) { { 'iid' => issue.iid.to_s } }
let_it_be(:issue_fields) { 'title' }
let(:query) do
graphql_query_for('project', project_params, project_fields)
end
let(:project_fields) do
query_graphql_field(:issue, issue_params, issue_fields)
end
shared_examples 'being able to fetch a design-like object by ID' do
let(:design) { design_a }
let(:path) { %w[project issue designCollection] + [GraphqlHelpers.fieldnamerize(object_field_name)] }
let(:design_fields) do
[
query_graphql_field(:filename),
query_graphql_field(:project, nil, query_graphql_field(:id))
]
end
let(:design_collection_fields) do
query_graphql_field(object_field_name, object_params, object_fields)
end
let(:object_fields) { design_fields }
context 'the ID is passed' do
let(:object_params) { { id: global_id_of(object) } }
let(:result_fields) { {} }
let(:expected_fields) do
result_fields.merge({ 'filename' => design.filename, 'project' => id_hash(project) })
end
it 'retrieves the object' do
post_query
data = graphql_data.dig(*path)
expect(data).to match(a_hash_including(expected_fields))
end
context 'the user is unauthorized' do
let(:current_user) { create(:user) }
it_behaves_like 'a failure to find anything'
end
end
context 'without parameters' do
let(:object_params) { nil }
it 'raises an error' do
post_query
expect(graphql_errors).to include(no_argument_error)
end
end
context 'attempting to retrieve an object from a different issue' do
let(:object_params) { { id: global_id_of(object_on_other_issue) } }
it_behaves_like 'a failure to find anything'
end
end
before do
project.add_developer(developer)
end
let(:post_query) { post_graphql(query, current_user: current_user) }
describe '.designCollection' do
include DesignManagementTestHelpers
let_it_be(:design_a) { create(:design, issue: issue) }
let_it_be(:version_a) { create(:design_version, issue: issue, created_designs: [design_a]) }
let(:issue_fields) do
query_graphql_field(:design_collection, dc_params, design_collection_fields)
end
let(:dc_params) { nil }
let(:design_collection_fields) { nil }
before do
enable_design_management
end
describe '.design' do
let(:object) { design }
let(:object_field_name) { :design }
let(:no_argument_error) do
custom_graphql_error(path, a_string_matching(%r/id or filename/))
end
let_it_be(:object_on_other_issue) { create(:design, issue: issue_b) }
it_behaves_like 'being able to fetch a design-like object by ID'
it_behaves_like 'being able to fetch a design-like object by ID' do
let(:object_params) { { filename: design.filename } }
end
end
describe '.version' do
let(:version) { version_a }
let(:path) { %w[project issue designCollection version] }
let(:design_collection_fields) do
query_graphql_field(:version, version_params, 'id sha')
end
context 'no parameters' do
let(:version_params) { nil }
it 'raises an error' do
post_query
expect(graphql_errors).to include(custom_graphql_error(path, a_string_matching(%r/id or sha/)))
end
end
shared_examples 'a successful query for a version' do
it 'finds the version' do
post_query
data = graphql_data.dig(*path)
expect(data).to match(
a_hash_including('id' => global_id_of(version),
'sha' => version.sha)
)
end
end
context '(sha: STRING_TYPE)' do
let(:version_params) { { sha: version.sha } }
it_behaves_like 'a successful query for a version'
end
context '(id: ID_TYPE)' do
let(:version_params) { { id: global_id_of(version) } }
it_behaves_like 'a successful query for a version'
end
end
describe '.designAtVersion' do
it_behaves_like 'being able to fetch a design-like object by ID' do
let(:object) { build(:design_at_version, design: design, version: version) }
let(:object_field_name) { :design_at_version }
let(:version) { version_a }
let(:result_fields) { { 'version' => id_hash(version) } }
let(:object_fields) do
design_fields + [query_graphql_field(:version, nil, query_graphql_field(:id))]
end
let(:no_argument_error) { missing_required_argument(path, :id) }
let(:object_on_other_issue) { build(:design_at_version, issue: issue_b) }
end
end
end
def id_hash(object)
a_hash_including('id' => global_id_of(object))
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Query' do
include GraphqlHelpers
let_it_be(:project) { create(:project) }
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:developer) { create(:user) }
let(:current_user) { developer }
describe '.designManagement' do
include DesignManagementTestHelpers
let_it_be(:version) { create(:design_version, issue: issue) }
let_it_be(:design) { version.designs.first }
let(:query_result) { graphql_data.dig(*path) }
let(:query) { graphql_query_for(:design_management, nil, dm_fields) }
before do
enable_design_management
project.add_developer(developer)
post_graphql(query, current_user: current_user)
end
shared_examples 'a query that needs authorization' do
context 'the current user is not able to read designs' do
let(:current_user) { create(:user) }
it 'does not retrieve the record' do
expect(query_result).to be_nil
end
it 'raises an error' do
expect(graphql_errors).to include(
a_hash_including('message' => a_string_matching(%r{you don't have permission}))
)
end
end
end
describe '.version' do
let(:path) { %w[designManagement version] }
let(:dm_fields) do
query_graphql_field(:version, { 'id' => global_id_of(version) }, 'id sha')
end
it_behaves_like 'a working graphql query'
it_behaves_like 'a query that needs authorization'
context 'the current user is able to read designs' do
it 'fetches the expected data' do
expect(query_result).to eq('id' => global_id_of(version), 'sha' => version.sha)
end
end
end
describe '.designAtVersion' do
let_it_be(:design_at_version) do
::DesignManagement::DesignAtVersion.new(design: design, version: version)
end
let(:path) { %w[designManagement designAtVersion] }
let(:dm_fields) do
query_graphql_field(:design_at_version, { 'id' => global_id_of(design_at_version) }, <<~FIELDS)
id
filename
version { id sha }
design { id }
issue { title iid }
project { id fullPath }
FIELDS
end
it_behaves_like 'a working graphql query'
it_behaves_like 'a query that needs authorization'
context 'the current user is able to read designs' do
it 'fetches the expected data, including the correct associations' do
expect(query_result).to eq(
'id' => global_id_of(design_at_version),
'filename' => design_at_version.design.filename,
'version' => { 'id' => global_id_of(version), 'sha' => version.sha },
'design' => { 'id' => global_id_of(design) },
'issue' => { 'title' => issue.title, 'iid' => issue.iid.to_s },
'project' => { 'id' => global_id_of(project), 'fullPath' => project.full_path }
)
end
end
end
end
end
......@@ -14,7 +14,7 @@ describe 'getting merge request information nested in a project' do
graphql_query_for(
'project',
{ 'fullPath' => project.full_path },
query_graphql_field('mergeRequest', iid: merge_request.iid)
query_graphql_field('mergeRequest', iid: merge_request.iid.to_s)
)
end
......
......@@ -25,7 +25,7 @@ describe 'getting task completion status information' do
graphql_query_for(
'project',
{ 'fullPath' => project.full_path },
query_graphql_field(type, { iid: iid }, fields)
query_graphql_field(type, { iid: iid.to_s }, fields)
)
end
......
......@@ -177,16 +177,26 @@ module GraphqlHelpers
def attributes_to_graphql(attributes)
attributes.map do |name, value|
value_str = if value.is_a?(Array)
'["' + value.join('","') + '"]'
else
"\"#{value}\""
end
value_str = as_graphql_literal(value)
"#{GraphqlHelpers.fieldnamerize(name.to_s)}: #{value_str}"
end.join(", ")
end
# Fairly dumb Ruby => GraphQL rendering function. Only suitable for testing.
# Missing support for Enums (feel free to add if you need it).
def as_graphql_literal(value)
case value
when Array then "[#{value.map { |v| as_graphql_literal(v) }.join(',')}]"
when Integer, Float then value.to_s
when String then "\"#{value.gsub(/"/, '\\"')}\""
when nil then 'null'
when true then 'true'
when false then 'false'
else raise ArgumentError, "Cannot represent #{value} as GraphQL literal"
end
end
def post_multiplex(queries, current_user: nil, headers: {})
post api('/', current_user, version: 'graphql'), params: { _json: queries }, headers: headers
end
......@@ -229,6 +239,11 @@ module GraphqlHelpers
json_response['data'] || (raise NoData, graphql_errors)
end
def graphql_data_at(*path)
keys = path.map { |segment| GraphqlHelpers.fieldnamerize(segment) }
graphql_data.dig(*keys)
end
def graphql_errors
case json_response
when Hash # regular query
......
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