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
from
319d62e
to
a8bcdc1
Compare
nickskalkin
force-pushed
the
nickskalkin/viewing-room-fixes
branch
from
a8bcdc1
to
900e4cd
Compare
|
| 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
| 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.
| @@ -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!
| @@ -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
| viewingRoomsConnection( | ||
| after: String | ||
| before: String |
There was a problem hiding this comment.
same as above - hopefully not a breaking change
| partnerID: ID | ||
| statuses: [ViewingRoomStatusEnum!] | ||
| statuses: [ViewingRoomStatusEnum!] = [live] |
There was a problem hiding this comment.
need to fix this
| 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
| # 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)
|
|
||
| # 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!
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.