Skip to content

use conventional opaque (base64-encoded) GraphQL IDs for Discussion{Thread,Comment}

Warren Gifford requested to merge gql-discussions-standard-id into master

Created by: sqs

Low priority

Previously, the GraphQL ID values were strings of the form "123", not opaque identifiers. This goes against Facebook's recommendations (https://facebook.github.io/relay/docs/en/graphql-server-specification.html):

The IDs we got back were base64 strings. IDs are designed to be opaque (the only thing that should be passed to the id argument on node is the unaltered result of querying id on some object in the system), and base64ing a string is a useful convention in GraphQL to remind viewers that the string is an opaque identifier.

and is different from how all other IDs work in our own GraphQL API. It also means that DiscussionThread and DiscussionComment don't implement the GraphQL Node type, so you can't look up threads and comments solely given their ID in the Query.node resolver.

(See related discussion at https://sourcegraph.slack.com/archives/C07KZF47K/p1556217341197600.)

This commit makes DiscussionThread and DiscussionComment use standard GraphQL IDs that are opaque and that are globally addressable. It preserves backcompat for old GraphQL API clients (such as the code discussions Sourcegraph extension). It also preserves existing URLs (i.e., the URLs still just use the same numeric ID, not some base64 monstrosity).

Merge request reports

Loading