Commit 4e501c94 authored by Alex Kalderimis's avatar Alex Kalderimis

Changes in response to reviewer comments

This updates some module doc comments, changes some variable names, uses
`global_id_of` in a few more places, and adds a new use of `before_all`.
parent cc54581b
...@@ -8,21 +8,30 @@ module Gitlab ...@@ -8,21 +8,30 @@ module Gitlab
# example: # example:
# #
# class MyAwesomeClass # class MyAwesomeClass
# def sum_frobbocities(ids) # include ::Gitlab::Graphql::Laziness
# ids.map { |n| get_the_thing(n) }.map(&method(:force).sum #
# # takes a list of id and list of factors, and computes
# # sum of [SomeObject[i]#value * factor[i]]
# def resolve(ids:, factors:)
# ids.zip(factors)
# .map { |id, factor| promise_an_int(id, factor) }
# .map(&method(:force))
# .sum
# end # end
# #
# def get_the_thing(id) # # returns a promise for an Integer
# thunk = SomeBatchLoader.load(id) # def (id, factor)
# defer { force(thunk).frobbocity * 2 } # thunk = SomeObject.lazy_find(id)
# defer { force(thunk).value * factor }
# end # end
# end # end
# #
# In the example above, we use defer to delay forcing the batch-loaded # In the example above, we use defer to delay forcing the batch-loaded
# item until we need it, and then we use `force` to consume the lazy values # item until we need it, and then we use `force` to consume the lazy values
# #
# If `SomeBatchLoader.load(id)` batches correctly, calling # If `SomeObject.lazy_find(id)` batches correctly, calling
# `sum_frobbocities` will only perform one batched load. # `resolve` will only perform one batched load for all objects, rather than
# loading them individually before combining the results.
# #
module Laziness module Laziness
def defer(&block) def defer(&block)
......
...@@ -4,6 +4,7 @@ require 'spec_helper' ...@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe ::CachingArrayResolver do RSpec.describe ::CachingArrayResolver do
include GraphqlHelpers include GraphqlHelpers
include Gitlab::Graphql::Laziness
let_it_be(:admins) { create_list(:user, 4, admin: true) } let_it_be(:admins) { create_list(:user, 4, admin: true) }
let(:query_context) { { current_user: admins.first } } let(:query_context) { { current_user: admins.first } }
...@@ -211,8 +212,4 @@ RSpec.describe ::CachingArrayResolver do ...@@ -211,8 +212,4 @@ RSpec.describe ::CachingArrayResolver do
allow(resolver).to receive(:field_options).and_return(opts.merge(max_page_size: max_page_size)) allow(resolver).to receive(:field_options).and_return(opts.merge(max_page_size: max_page_size))
resolve(resolver, args: args, ctx: query_context, schema: schema, field: field) resolve(resolver, args: args, ctx: query_context, schema: schema, field: field)
end end
def force(lazy)
::Gitlab::Graphql::Lazy.force(lazy)
end
end end
...@@ -163,9 +163,9 @@ RSpec.describe 'Creating a Snippet' do ...@@ -163,9 +163,9 @@ RSpec.describe 'Creating a Snippet' do
context 'when there are uploaded files' do context 'when there are uploaded files' do
shared_examples 'expected files argument' do |file_value, expected_value| shared_examples 'expected files argument' do |file_value, expected_value|
let(:uploaded_files) { file_value } let(:uploaded_files) { file_value }
let(:ham) { build(:snippet) } let(:snippet) { build(:snippet) }
let(:creation_response) do let(:creation_response) do
::ServiceResponse.error(message: 'urk', payload: { snippet: ham }) ::ServiceResponse.error(message: 'urk', payload: { snippet: snippet })
end end
it do it do
......
...@@ -71,12 +71,12 @@ RSpec.describe 'query terraform states' do ...@@ -71,12 +71,12 @@ RSpec.describe 'query terraform states' do
'updatedAt' => terraform_state.updated_at.iso8601, 'updatedAt' => terraform_state.updated_at.iso8601,
'lockedByUser' => { 'id' => global_id_of(terraform_state.locked_by_user) }, 'lockedByUser' => { 'id' => global_id_of(terraform_state.locked_by_user) },
'latestVersion' => { 'latestVersion' => {
'id' => eq(latest_version.to_global_id.to_s), 'id' => eq(global_id_of(latest_version)),
'serial' => eq(latest_version.version), 'serial' => eq(latest_version.version),
'downloadPath' => eq(download_path), 'downloadPath' => eq(download_path),
'createdAt' => eq(latest_version.created_at.iso8601), 'createdAt' => eq(latest_version.created_at.iso8601),
'updatedAt' => eq(latest_version.updated_at.iso8601), 'updatedAt' => eq(latest_version.updated_at.iso8601),
'createdByUser' => { 'id' => eq(latest_version.created_by_user.to_global_id.to_s) }, 'createdByUser' => { 'id' => eq(global_id_of(latest_version.created_by_user)) },
'job' => { 'name' => eq(latest_version.build.name) } 'job' => { 'name' => eq(latest_version.build.name) }
} }
}) })
......
...@@ -32,7 +32,7 @@ RSpec.describe 'getting project information' do ...@@ -32,7 +32,7 @@ RSpec.describe 'getting project information' do
end end
context 'when the user has access to the project', :use_clean_rails_memory_store_caching, :request_store do context 'when the user has access to the project', :use_clean_rails_memory_store_caching, :request_store do
before do before_all do
project.add_developer(current_user) project.add_developer(current_user)
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