Skip to content

Commit d16cf30

Browse files
committed
Rename the connection args -> grav args, and ensures the city shows, and show nearbyShows connections give the right pagination data
1 parent 261c88b commit d16cf30

21 files changed

+125
-71
lines changed

_schema.graphql

-2
Original file line numberDiff line numberDiff line change
@@ -1971,7 +1971,6 @@ type City {
19711971
name: String
19721972
coordinates: LatLng
19731973
shows(
1974-
size: Int
19751974
sort: PartnerShowSorts
19761975
status: EventStatus
19771976
after: String
@@ -6900,7 +6899,6 @@ type Show implements Node {
69006899

69016900
# Shows that are near (~75km) from this show
69026901
nearbyShows(
6903-
size: Int
69046902
sort: PartnerShowSorts
69056903
status: EventStatus
69066904
after: String

docs/dataloaders.md

+13-13
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,24 @@ artwork's artist - using the dataloader pattern that call would only happen once
3737

3838
Our usage has a few moving parts:
3939

40-
* The `api` object: for example [`/lib/apis/gravity.js`][api_grav] - it is a wrapper around [fetch][fetch] that
40+
- The `api` object: for example [`/lib/apis/gravity.js`][api_grav] - it is a wrapper around [fetch][fetch] that
4141
customizes the request to the service. The params tend to differ depending on the authentication method for that
4242
server.
4343

44-
* The `loader factory` - there are three loader factories. They all end up exposing the same API, so your tests should
44+
- The `loader factory` - there are three loader factories. They all end up exposing the same API, so your tests should
4545
be the same shape regardless of the servers or types of calls you need to make.
4646

4747
When an unauthenticated API call is made we take the result and store it in memcache, then the next time (potentially
4848
on another user's request) the cached result is passed back and we update memcache for the next request.
4949

50-
* [`loader_without_authentication_factory`][no_auth_loader] for API requests which can call the `api` directly and be
50+
- [`loader_without_authentication_factory`][no_auth_loader] for API requests which can call the `api` directly and be
5151
safely cached in memcache.
52-
* [`loader_with_authentication_factory`][auth_loader] for API requests that _could_ require a call for an
52+
- [`loader_with_authentication_factory`][auth_loader] for API requests that _could_ require a call for an
5353
authentication token (like the examples in [Adding a New Microservice][new_micro].)
54-
* [`loader_one_off_factory`][one_off_loader] for API requests which have custom auth schemes, or need to ignore cache
54+
- [`loader_one_off_factory`][one_off_loader] for API requests which have custom auth schemes, or need to ignore cache
5555
completely.
5656

57-
* The `loader` themselves. These are a set of functions which [are exposed][loaders] as properties on the root object
57+
- The `loader` themselves. These are a set of functions which [are exposed][loaders] as properties on the root object
5858
during the resolving stages of our graphQL implementation.
5959

6060
```js
@@ -92,17 +92,17 @@ Our usage has a few moving parts:
9292
},
9393
}),
9494

95-
resolve: (artist, options, request, { rootValue: { artistArtworksLoader } }) => {
95+
resolve: async (artist, options, request, { rootValue: { artistArtworksLoader } }) => {
9696
const { limit: size, offset } = getPagingParameters(options)
9797
const { sort, filter, published } = options
9898

9999
const gravityArgs = { size, offset, sort, filter, published }
100-
return artistArtworksLoader(artist.id, gravityArgs).then(artworks =>
101-
connectionFromArraySlice(artworks, options, {
102-
arrayLength: artistArtworkArrayLength(artist, filter),
103-
sliceStart: offset,
104-
})
105-
)
100+
const artworks = await artistArtworksLoader(artist.id, gravityArgs)
101+
102+
return connectionFromArraySlice(artworks, options, {
103+
arrayLength: artistArtworkArrayLength(artist, filter),
104+
sliceStart: offset,
105+
})
106106
},
107107
},
108108
// [...]

src/lib/helpers.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,14 @@ export const queriedForFieldsOtherThanBlacklisted = (
9090
export const queryContainsField = (fieldASTs, soughtField) => {
9191
return parseFieldASTsIntoArray(fieldASTs).includes(soughtField)
9292
}
93-
export const parseRelayOptions = options => {
93+
94+
export const convertConnectionArgsToGravityArgs = options => {
9495
const { limit: size, offset } = getPagingParameters(options)
9596
const page = Math.round((size + offset) / size)
9697
const gravityArgs = omit(options, ["first", "after", "last", "before"])
9798
return Object.assign({}, { page, size, offset }, gravityArgs)
9899
}
100+
99101
export const removeNulls = object => {
100102
Object.keys(object).forEach(key => object[key] == null && delete object[key]) // eslint-disable-line eqeqeq, no-param-reassign, max-len
101103
}

src/lib/loaders/loaders_without_authentication/gravity.ts

+1
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export default opts => {
101101
setsLoader: gravityLoader("sets"),
102102
showLoader: gravityLoader(id => `show/${id}`),
103103
showsLoader: gravityLoader("shows"),
104+
showsWithHeadersLoader: gravityLoader("shows", {}, { headers: true }),
104105
similarArtworksLoader: gravityLoader("related/artworks"),
105106
similarGenesLoader: gravityLoader(
106107
id => `gene/${id}/similar`,

src/schema/__tests__/city.test.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ describe("City", () => {
4949
{
5050
city(slug: "sacramende-ca-usa") {
5151
name
52-
shows {
52+
shows(first: 1) {
5353
edges {
5454
node {
5555
id
@@ -61,9 +61,11 @@ describe("City", () => {
6161
`
6262

6363
const mockShows = [{ id: "first-show" }]
64-
const mockShowsLoader = jest.fn(() => Promise.resolve(mockShows))
64+
const mockShowsLoader = jest.fn(() =>
65+
Promise.resolve({ body: mockShows, headers: { "x-total-count": 1 } })
66+
)
6567
const rootValue = {
66-
showsLoader: mockShowsLoader,
68+
showsWithHeadersLoader: mockShowsLoader,
6769
accessToken: null,
6870
userID: null,
6971
}

src/schema/__tests__/show.test.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ describe("Show type", () => {
3232

3333
rootValue = {
3434
showLoader: sinon.stub().returns(Promise.resolve(showData)),
35-
showsLoader: sinon.stub().returns(Promise.resolve([showData])),
35+
showsWithHeadersLoader: sinon
36+
.stub()
37+
.returns(
38+
Promise.resolve({ body: [showData], headers: { "x-total-count": 1 } })
39+
),
3640
galaxyGalleriesLoader: sinon.stub().returns(Promise.resolve(galaxyData)),
3741
partnerShowLoader: sinon.stub().returns(Promise.resolve(showData)),
3842
}
@@ -617,7 +621,7 @@ describe("Show type", () => {
617621
const query = gql`
618622
{
619623
show(id: "new-museum-1-2015-triennial-surround-audience") {
620-
nearbyShows {
624+
nearbyShows(first: 1) {
621625
edges {
622626
node {
623627
id

src/schema/artist/index.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ import {
4949
GraphQLInt,
5050
} from "graphql"
5151
import { connectionFromArraySlice } from "graphql-relay"
52-
import { parseRelayOptions } from "lib/helpers"
52+
import { convertConnectionArgsToGravityArgs } from "lib/helpers"
5353
import { totalViaLoader } from "lib/total"
5454

5555
// Manually curated list of artist id's who has verified auction lots that can be
@@ -94,7 +94,7 @@ export const ArtistType = new GraphQLObjectType({
9494
_request,
9595
{ rootValue: { articlesLoader } }
9696
) => {
97-
const pageOptions = parseRelayOptions(args)
97+
const pageOptions = convertConnectionArgsToGravityArgs(args)
9898
const { page, size, offset } = pageOptions
9999

100100
const gravityArgs = omit(args, ["first", "after", "last", "before"])
@@ -242,7 +242,9 @@ export const ArtistType = new GraphQLObjectType({
242242
}
243243

244244
// Convert `after` cursors to page params
245-
const { page, size, offset } = parseRelayOptions(options)
245+
const { page, size, offset } = convertConnectionArgsToGravityArgs(
246+
options
247+
)
246248
const diffusionArgs = {
247249
page,
248250
size,

src/schema/artist/related.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { pageable, getPagingParameters } from "relay-cursor-paging"
33
import { SuggestedArtistsArgs } from "schema/me/suggested_artists_args"
44
import { artistConnection } from "schema/artist"
55
import { geneConnection } from "schema/gene"
6-
import { parseRelayOptions } from "lib/helpers"
6+
import { convertConnectionArgsToGravityArgs } from "lib/helpers"
77
import { createPageCursors } from "schema/fields/pagination"
88
import { assign } from "lodash"
99
import { connectionFromArraySlice, connectionFromArray } from "graphql-relay"
@@ -60,7 +60,9 @@ export const Related = {
6060
},
6161
}
6262
) => {
63-
const { page, size, offset } = parseRelayOptions(args)
63+
const { page, size, offset } = convertConnectionArgsToGravityArgs(
64+
args
65+
)
6466
const { kind, exclude_artists_without_artworks } = args
6567
const gravityArgs = {
6668
page,

src/schema/artist/shows.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { GraphQLInt, GraphQLString, GraphQLBoolean } from "graphql"
33
import PartnerShowSorts from "schema/sorts/partner_show_sorts"
44
import { merge, defaults, reject, includes, omit } from "lodash"
55
import { createPageCursors } from "schema/fields/pagination"
6-
import { parseRelayOptions } from "lib/helpers"
6+
import { convertConnectionArgsToGravityArgs } from "lib/helpers"
77
import { connectionFromArraySlice } from "graphql-relay"
88

99
// TODO: Fix upstream, for now we remove shows from certain Partner types
@@ -75,7 +75,7 @@ export const ShowField = {
7575
export const ShowsConnectionField = {
7676
args: pageable(ShowArgs),
7777
resolve: ({ id }, args, _request, { rootValue: { relatedShowsLoader } }) => {
78-
const pageOptions = parseRelayOptions(args)
78+
const pageOptions = convertConnectionArgsToGravityArgs(args)
7979
const { page, size, offset } = pageOptions
8080
const gravityArgs = omit(args, ["first", "after", "last", "before"])
8181
return relatedShowsLoader(

src/schema/city/index.ts

+22-8
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ import EventStatus from "schema/input_fields/event_status"
1515

1616
import cityData from "./city_data.json"
1717
import { pageable } from "relay-cursor-paging"
18-
import { connectionFromArray } from "graphql-relay"
18+
import { connectionFromArraySlice } from "graphql-relay"
1919
import { LOCAL_DISCOVERY_RADIUS_KM } from "./constants"
20+
import { convertConnectionArgsToGravityArgs } from "lib/helpers"
2021

2122
const CityType = new GraphQLObjectType({
2223
name: "City",
@@ -33,23 +34,36 @@ const CityType = new GraphQLObjectType({
3334
shows: {
3435
type: showConnection,
3536
args: pageable({
36-
size: {
37-
type: GraphQLInt,
38-
},
3937
sort: PartnerShowSorts,
4038
status: EventStatus,
4139
}),
42-
resolve: (city, args, _context, { rootValue: { showsLoader } }) => {
40+
resolve: async (
41+
city,
42+
args,
43+
_c,
44+
{ rootValue: { showsWithHeadersLoader } }
45+
) => {
4346
const gravityOptions = {
44-
...args,
47+
...convertConnectionArgsToGravityArgs(args),
4548
displayable: true,
4649
near: `${city.coordinates.lat},${city.coordinates.lng}`,
4750
max_distance: LOCAL_DISCOVERY_RADIUS_KM,
51+
total_count: true,
4852
}
53+
delete gravityOptions.page
54+
55+
const response = await showsWithHeadersLoader(gravityOptions)
56+
const { headers, body: cities } = response
4957

50-
return showsLoader(gravityOptions).then(response => {
51-
return connectionFromArray(response, args)
58+
const results = connectionFromArraySlice(cities, args, {
59+
arrayLength: headers["x-total-count"],
60+
sliceStart: gravityOptions.offset,
5261
})
62+
63+
// This is in our schema, so might as well fill it
64+
// @ts-ignore
65+
results.totalCount = headers["x-total-count"]
66+
return results
5367
},
5468
},
5569
fairs: {

src/schema/collection.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import CollectionSorts from "./sorts/collection_sorts"
77
import { artworkConnection } from "./artwork"
88
import {
99
queriedForFieldsOtherThanBlacklisted,
10-
parseRelayOptions,
10+
convertConnectionArgsToGravityArgs,
1111
} from "lib/helpers"
1212
import { GravityIDFields, NodeInterface } from "./object_identification"
1313
import {
@@ -47,7 +47,7 @@ export const CollectionType = new GraphQLObjectType({
4747
) => {
4848
const gravityOptions = Object.assign(
4949
{ total_count: true },
50-
parseRelayOptions(options)
50+
convertConnectionArgsToGravityArgs(options)
5151
)
5252
// Adds a default case for the sort
5353
gravityOptions.sort = gravityOptions.sort || "-position"

src/schema/fair.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { omit } from "lodash"
22
import { pageable } from "relay-cursor-paging"
33
import { connectionFromArraySlice } from "graphql-relay"
4-
import { parseRelayOptions } from "lib/helpers"
4+
import { convertConnectionArgsToGravityArgs } from "lib/helpers"
55
import moment from "moment"
66
import cached from "./fields/cached"
77
import date from "./fields/date"
@@ -143,7 +143,10 @@ const FairType = new GraphQLObjectType({
143143
_request,
144144
{ rootValue: { fairBoothsLoader } }
145145
) => {
146-
const gravityOptions = omit(parseRelayOptions(options), ["page"])
146+
const gravityOptions = omit(
147+
convertConnectionArgsToGravityArgs(options),
148+
["page"]
149+
)
147150
gravityOptions.sort = gravityOptions.sort || "-featured"
148151

149152
return Promise.all([

src/schema/filter_artworks.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { computeTotalPages, createPageCursors } from "./fields/pagination"
88
import { artworkConnection } from "./artwork"
99
import { pageable } from "relay-cursor-paging"
1010
import {
11-
parseRelayOptions,
11+
convertConnectionArgsToGravityArgs,
1212
queriedForFieldsOtherThanBlacklisted,
1313
removeNulls,
1414
} from "lib/helpers"
@@ -99,7 +99,7 @@ export const FilterArtworksType = new GraphQLObjectType({
9999
request,
100100
{ rootValue: { filterArtworksLoader } }
101101
) => {
102-
const relayOptions = parseRelayOptions(args)
102+
const relayOptions = convertConnectionArgsToGravityArgs(args)
103103

104104
return filterArtworksLoader(
105105
assign(gravityOptions, relayOptions, {})

src/schema/gene.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import filterArtworks, {
1212
} from "./filter_artworks"
1313
import {
1414
queriedForFieldsOtherThanBlacklisted,
15-
parseRelayOptions,
15+
convertConnectionArgsToGravityArgs,
1616
} from "lib/helpers"
1717
import { GravityIDFields, NodeInterface } from "./object_identification"
1818
import {
@@ -65,7 +65,10 @@ export const GeneType = new GraphQLObjectType({
6565
request,
6666
{ rootValue: { geneArtistsLoader } }
6767
) => {
68-
const parsedOptions = _.omit(parseRelayOptions(options), "page")
68+
const parsedOptions = _.omit(
69+
convertConnectionArgsToGravityArgs(options),
70+
"page"
71+
)
6972
const gravityOptions = _.extend(parsedOptions, {
7073
exclude_artists_without_artworks: true,
7174
})
@@ -93,7 +96,7 @@ export const GeneType = new GraphQLObjectType({
9396
request,
9497
{ rootValue: { filterArtworksLoader } }
9598
) => {
96-
const gravityOptions = parseRelayOptions(options)
99+
const gravityOptions = convertConnectionArgsToGravityArgs(options)
97100
// Do some massaging of the options for ElasticSearch
98101
gravityOptions.aggregations = options.aggregations || []
99102
gravityOptions.aggregations.push("total")

src/schema/gene_families.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
connectionDefinitions,
55
connectionFromPromisedArray,
66
} from "graphql-relay"
7-
import { parseRelayOptions } from "lib/helpers"
7+
import { convertConnectionArgsToGravityArgs } from "lib/helpers"
88

99
const { connectionType: GeneFamilyConnection } = connectionDefinitions({
1010
nodeType: GeneFamily.type,
@@ -15,9 +15,13 @@ const GeneFamilies = {
1515
description: "A list of Gene Families",
1616
args: pageable(),
1717
resolve: (_root, options, _request, { rootValue }) => {
18-
const gravityOptions = Object.assign({}, parseRelayOptions(options), {
19-
sort: "position",
20-
})
18+
const gravityOptions = Object.assign(
19+
{},
20+
convertConnectionArgsToGravityArgs(options),
21+
{
22+
sort: "position",
23+
}
24+
)
2125
return connectionFromPromisedArray(
2226
rootValue.geneFamiliesLoader(gravityOptions),
2327
gravityOptions

src/schema/me/conversation/index.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
} from "graphql"
1414
import { pageable } from "relay-cursor-paging"
1515
import { connectionFromArraySlice, connectionDefinitions } from "graphql-relay"
16-
import { parseRelayOptions } from "lib/helpers"
16+
import { convertConnectionArgsToGravityArgs } from "lib/helpers"
1717
import { ArtworkType } from "schema/artwork"
1818
import { ShowType } from "schema/show"
1919
import { GlobalIDField, NodeInterface } from "schema/object_identification"
@@ -350,7 +350,7 @@ export const ConversationFields = {
350350
req,
351351
{ rootValue: { conversationMessagesLoader } }
352352
) => {
353-
const { page, size, offset } = parseRelayOptions(options)
353+
const { page, size, offset } = convertConnectionArgsToGravityArgs(options)
354354
return conversationMessagesLoader({
355355
page,
356356
size,

0 commit comments

Comments
 (0)