refactor: fixes some misalignments between stitched / unstitched viewing rooms #6278
Conversation
| @@ -12,7 +12,7 @@ import config from "config" | |||
|
|
|||
| const rootFieldsAllowList = ["agreement"].concat( | |||
| config.USE_UNSTITCHED_VIEWING_ROOM_SCHEMA | |||
| ? [] | |||
| ? ["viewingRooms"] | |||
There was a problem hiding this comment.
I wanted to remove this field completely (gravity PR), but seems like it is still used by some clients. Don't know by whom yet. Do I understand it right that it shouldn't be a problem to temporarily keep it here?
There was a problem hiding this comment.
Sure. Seems like an easy enough field to add directly to MP w/o stitching, esp considering everything else already added! and then you can remove it from here too.
nickskalkin
force-pushed
the
nickskalkin/viewing-room-fixes
branch
2 times, most recently
from
a8bcdc1 to
900e4cd
Compare
_schemaV2.graphql
Outdated
| input CreateViewingRoomInput { | ||
| attributes: ViewingRoomAttributes | ||
|
|
||
| # Main text | ||
| body: String | ||
|
|
||
| # A unique identifier for the client performing the mutation. |
There was a problem hiding this comment.
a bunch of changes like that - I've decided to not add "obvious" rails-generated descriptions, but I can do it if you think I should
_schemaV2.graphql
Outdated
| clientMutationId: String | ||
|
|
||
| # End datetime | ||
| endAt: ISO8601DateTime |
There was a problem hiding this comment.
Should I add GravityISO8601DateTime type? Similar to GravityARImage below
There was a problem hiding this comment.
nah, clients probably are fine either way since it's just a string. These fields may not even be used directly by clients vs. the other time fields like distanceToClose.
_schemaV2.graphql
Outdated
| @@ -15204,7 +15191,9 @@ type Partner implements Node { | |||
| vatNumber: String | |||
| viewingRoomsConnection( | |||
| after: String | |||
| before: String | |||
There was a problem hiding this comment.
new "default" connection params - it won't be a breaking change, right?
There was a problem hiding this comment.
Adding stuff not a breaking change!
_schemaV2.graphql
Outdated
| @@ -22198,15 +22153,21 @@ type Viewer { | |||
| # Recaptcha token. | |||
| recaptchaToken: String! | |||
| ): VerifyUser | |||
|
|
|||
| # Find a viewing room by ID | |||
| viewingRoom(id: ID!): ViewingRoom | |||
There was a problem hiding this comment.
maybe I should remove this field since it was not there before
_schemaV2.graphql
Outdated
| viewingRoomsConnection( | ||
| after: String | ||
| before: String |
There was a problem hiding this comment.
same as above - hopefully not a breaking change
_schemaV2.graphql
Outdated
| partnerID: ID | ||
| statuses: [ViewingRoomStatusEnum!] | ||
| statuses: [ViewingRoomStatusEnum!] = [live] |
There was a problem hiding this comment.
need to fix this
_schemaV2.graphql
Outdated
| type ViewingRoomsConnection { | ||
| # A list of edges. | ||
| edges: [ViewingRoomsEdge] | ||
|
|
||
| # A list of nodes. | ||
| nodes: [ViewingRoom] |
There was a problem hiding this comment.
argh, really don't want to add noted right to ViewingRoomsConnection (without edges in the middle). Is there a guaranteed way to tell wether there are any clients which do viewingRoomsConnection.nodes?
There was a problem hiding this comment.
Just via yarn relay on this new schema AFAIK
_schemaV2.graphql
Outdated
| # A list of nodes. | ||
| nodes: [ViewingRoom] | ||
| pageCursors: PageCursors | ||
| pageCursors: PageCursors! |
There was a problem hiding this comment.
there was no ! before - breaking change?
There was a problem hiding this comment.
i dont think so! as long as the MP pagination resolvers dont generate any nulls (they shouldn't)
_schemaV2.graphql
Outdated
|
|
||
| # Information to aid in pagination. | ||
| pageInfo: PageInfo! | ||
| totalCount: Int | ||
| totalPages: Int |
There was a problem hiding this comment.
couldn't find totalPages being used in Force / Eigen, hopefully we can delete this field?
There was a problem hiding this comment.
yarn relay should reveal it
| @@ -11,7 +11,7 @@ export const createViewingRoomMutation = mutationWithClientMutationId< | |||
| any, | |||
| ResolverContext | |||
| >({ | |||
| name: "createViewingRoom", | |||
| name: "CreateViewingRoom", | |||
There was a problem hiding this comment.
Nice, yup this is important!
There was a problem hiding this comment.
Nice! The schema changes look like they don't have any breaking changes to clients, so you can even commit the schema changes anytime (or it's fine to wait on that until after the flag is being flipped).
This is an excellent piece of work, overall. Unstitching viewing rooms (and some weirdness w/ the images, all the mutations, 'subsections', and everything else) - as a no-op to clients - really really good!
nickskalkin
force-pushed
the
nickskalkin/viewing-room-fixes
branch
from
900e4cd to
13165f1
Compare
nickskalkin
force-pushed
the
nickskalkin/viewing-room-fixes
branch
from
13165f1 to
1985a05
Compare
Warning
I've included _schemaV2.graphql to this PR only to discuss the rest of differences between stitched / unstitched schemas
Made some fixes in order to make stitched / unstitched viewing room schemas look more alike.