Commit 2e488d5a authored by Amy Qualls's avatar Amy Qualls Committed by Luke Duncalfe

Revisions for tone and style

Wording and phrasing tweaks in the new portion of the page. Change
gerund to present tense. Fix some Vale-identified issues over the
entire page, while I'm here, since it's an unowned page.
parent f8b94b64
...@@ -22,7 +22,7 @@ which is exposed as an API endpoint at `/api/graphql`. ...@@ -22,7 +22,7 @@ which is exposed as an API endpoint at `/api/graphql`.
## Deep Dive ## Deep Dive
In March 2019, Nick Thomas hosted a Deep Dive (GitLab team members only: `https://gitlab.com/gitlab-org/create-stage/issues/1`) In March 2019, Nick Thomas hosted a Deep Dive (GitLab team members only: `https://gitlab.com/gitlab-org/create-stage/issues/1`)
on the GitLab [GraphQL API](../api/graphql/index.md) to share his domain specific knowledge on the GitLab [GraphQL API](../api/graphql/index.md) to share domain-specific knowledge
with anyone who may work in this part of the codebase in the future. You can find the with anyone who may work in this part of the codebase in the future. You can find the
<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> <i class="fa fa-youtube-play youtube" aria-hidden="true"></i>
[recording on YouTube](https://www.youtube.com/watch?v=-9L_1MWrjkg), and the slides on [recording on YouTube](https://www.youtube.com/watch?v=-9L_1MWrjkg), and the slides on
...@@ -83,7 +83,7 @@ developers must familiarize themselves with our [Deprecation and Removal process ...@@ -83,7 +83,7 @@ developers must familiarize themselves with our [Deprecation and Removal process
Breaking changes are: Breaking changes are:
- Removing or renaming a field, argument, enum value or mutation. - Removing or renaming a field, argument, enum value, or mutation.
- Changing the type of a field, argument or enum value. - Changing the type of a field, argument or enum value.
- Raising the [complexity](#max-complexity) of a field or complexity multipliers in a resolver. - Raising the [complexity](#max-complexity) of a field or complexity multipliers in a resolver.
- Changing a field from being _not_ nullable (`null: false`) to nullable (`null: true`), as - Changing a field from being _not_ nullable (`null: false`) to nullable (`null: true`), as
...@@ -110,7 +110,7 @@ See also: ...@@ -110,7 +110,7 @@ See also:
- [Exposing Global IDs](#exposing-global-ids). - [Exposing Global IDs](#exposing-global-ids).
- [Mutation arguments](#object-identifier-arguments). - [Mutation arguments](#object-identifier-arguments).
- [Deprecating Global IDs](#deprecating-global-ids). - [Deprecating Global IDs](#deprecate-global-ids).
We have a custom scalar type (`Types::GlobalIDType`) which should be used as the We have a custom scalar type (`Types::GlobalIDType`) which should be used as the
type of input and output arguments when the value is a `GlobalID`. The benefits type of input and output arguments when the value is a `GlobalID`. The benefits
...@@ -118,12 +118,12 @@ of using this type instead of `ID` are: ...@@ -118,12 +118,12 @@ of using this type instead of `ID` are:
- it validates that the value is a `GlobalID` - it validates that the value is a `GlobalID`
- it parses it into a `GlobalID` before passing it to user code - it parses it into a `GlobalID` before passing it to user code
- it can be parameterized on the type of the object (e.g. - it can be parameterized on the type of the object (for example,
`GlobalIDType[Project]`) which offers even better validation and security. `GlobalIDType[Project]`) which offers even better validation and security.
Consider using this type for all new arguments and result types. Remember that Consider using this type for all new arguments and result types. Remember that
it is perfectly possible to parameterize this type with a concern or a it is perfectly possible to parameterize this type with a concern or a
supertype, if you want to accept a wider range of objects (e.g. supertype, if you want to accept a wider range of objects (such as
`GlobalIDType[Issuable]` vs `GlobalIDType[Issue]`). `GlobalIDType[Issuable]` vs `GlobalIDType[Issue]`).
## Types ## Types
...@@ -342,7 +342,7 @@ For example, instead of `latest_pipeline`, use `pipelines(last: 1)`. ...@@ -342,7 +342,7 @@ For example, instead of `latest_pipeline`, use `pipelines(last: 1)`.
By default, the API returns at most a maximum number of records defined in By default, the API returns at most a maximum number of records defined in
[`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb) [`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb)
per page within a connection and this will also be the default number of records per page in a connection and this is also the default number of records
returned per page if no limiting arguments (`first:` or `last:`) are provided by a client. returned per page if no limiting arguments (`first:` or `last:`) are provided by a client.
The `max_page_size` argument can be used to specify a different page size limit The `max_page_size` argument can be used to specify a different page size limit
...@@ -370,7 +370,7 @@ Complexity is described in [our client documentation](../api/graphql/index.md#ma ...@@ -370,7 +370,7 @@ Complexity is described in [our client documentation](../api/graphql/index.md#ma
Complexity limits are defined in [`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb). Complexity limits are defined in [`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb).
By default, fields will add `1` to a query's complexity score. This can be overridden by By default, fields add `1` to a query's complexity score. This can be overridden by
[providing a custom `complexity`](https://graphql-ruby.org/queries/complexity_and_depth.html) value for a field. [providing a custom `complexity`](https://graphql-ruby.org/queries/complexity_and_depth.html) value for a field.
Developers should specify higher complexity for fields that cause more _work_ to be performed Developers should specify higher complexity for fields that cause more _work_ to be performed
...@@ -391,7 +391,7 @@ field :blob, type: Types::Snippets::BlobType, ...@@ -391,7 +391,7 @@ field :blob, type: Types::Snippets::BlobType,
calls_gitaly: true calls_gitaly: true
``` ```
This will increment the [`complexity` score](#field-complexity) of the field by `1`. This increments the [`complexity` score](#field-complexity) of the field by `1`.
If a resolver calls Gitaly, it can be annotated with If a resolver calls Gitaly, it can be annotated with
`BaseResolver.calls_gitaly!`. This passes `calls_gitaly: true` to any `BaseResolver.calls_gitaly!`. This passes `calls_gitaly: true` to any
...@@ -481,7 +481,7 @@ You can refer to these guidelines to decide which approach to use: ...@@ -481,7 +481,7 @@ You can refer to these guidelines to decide which approach to use:
The `feature_flag` property allows you to toggle the field's The `feature_flag` property allows you to toggle the field's
[visibility](https://graphql-ruby.org/authorization/visibility.html) [visibility](https://graphql-ruby.org/authorization/visibility.html)
within the GraphQL schema. This removes the field from the schema in the GraphQL schema. This removes the field from the schema
when the flag is disabled. when the flag is disabled.
A description is [appended](https://gitlab.com/gitlab-org/gitlab/-/blob/497b556/app/graphql/types/base_field.rb#L44-53) A description is [appended](https://gitlab.com/gitlab-org/gitlab/-/blob/497b556/app/graphql/types/base_field.rb#L44-53)
...@@ -596,22 +596,22 @@ end ...@@ -596,22 +596,22 @@ end
If the field, argument, or enum value being deprecated is not being replaced, If the field, argument, or enum value being deprecated is not being replaced,
a descriptive deprecation `reason` should be given. a descriptive deprecation `reason` should be given.
### Deprecating Global IDs ### Deprecate Global IDs
We use the [rails/globalid](https://github.com/rails/globalid) gem to generate and parse We use the [`rails/globalid`](https://github.com/rails/globalid) gem to generate and parse
Global IDs, so as such they are coupled to model names. This means that when we rename a Global IDs, so as such they are coupled to model names. When we rename a
model it will change its Global ID. model, its Global ID changes.
If the Global ID is used as an _argument_ type anywhere in the schema then the Global ID If the Global ID is used as an _argument_ type anywhere in the schema, then the Global ID
change would normally constitute a breaking change. change would normally constitute a breaking change.
In order to continue to support clients using the old Global ID argument, we add a deprecation To continue to support clients using the old Global ID argument, we add a deprecation
to `Gitlab::GlobalId::Deprecations`. to `Gitlab::GlobalId::Deprecations`.
NOTE: NOTE:
If the Global ID is _only_ [exposed as a field](#exposing-global-ids) then we do not need to If the Global ID is _only_ [exposed as a field](#exposing-global-ids) then we do not need to
deprecate it. We consider the change to the way a Global ID is expressed in a field to be deprecate it. We consider the change to the way a Global ID is expressed in a field to be
backwards-compatible as we expect that clients will not parse these values: they are meant to backwards-compatible. We expect that clients don't parse these values: they are meant to
be treated as opaque tokens, and any structure in them is incidental and not to be relied on. be treated as opaque tokens, and any structure in them is incidental and not to be relied on.
**Example scenario:** **Example scenario:**
...@@ -629,7 +629,7 @@ argument :id, Types::GlobalIDType[::PrometheusService], ...@@ -629,7 +629,7 @@ argument :id, Types::GlobalIDType[::PrometheusService],
description: "The ID of the integration to mutate." description: "The ID of the integration to mutate."
``` ```
Currently clients call the mutation by passing a Global ID string that looks like Clients call the mutation by passing a Global ID string that looks like
`"gid://gitlab/PrometheusService/1"`, named as `PrometheusServiceID`, as the `input.id` argument: `"gid://gitlab/PrometheusService/1"`, named as `PrometheusServiceID`, as the `input.id` argument:
```graphql ```graphql
...@@ -654,7 +654,7 @@ argument :id, Types::GlobalIDType[::Integrations::Prometheus], ...@@ -654,7 +654,7 @@ argument :id, Types::GlobalIDType[::Integrations::Prometheus],
description: "The ID of the integration to mutate." description: "The ID of the integration to mutate."
``` ```
This would cause a breaking change to the mutation, as the API will now reject clients who This would cause a breaking change to the mutation, as the API now rejects clients who
pass an `id` argument as `"gid://gitlab/PrometheusService/1"`, or that specify the argument pass an `id` argument as `"gid://gitlab/PrometheusService/1"`, or that specify the argument
type as `PrometheusServiceID` in the query signature. type as `PrometheusServiceID` in the query signature.
...@@ -674,14 +674,14 @@ support for the former argument style, remove the `Deprecation`: ...@@ -674,14 +674,14 @@ support for the former argument style, remove the `Deprecation`:
DEPRECATIONS = [].freeze DEPRECATIONS = [].freeze
``` ```
During the deprecation period the API will accept these values for the argument: During the deprecation period the API accepts these values for the argument:
Formatted as either: Formatted as either:
- `"gid://gitlab/PrometheusService/1"` - `"gid://gitlab/PrometheusService/1"`
- `"gid://gitlab/Integrations::Prometheus/1"` - `"gid://gitlab/Integrations::Prometheus/1"`
And query signatures will recognize the type under the old and new type names, accepting either: And query signatures recognizes the type under the old and new type names, accepting either:
- `PrometheusServiceID` - `PrometheusServiceID`
- `IntegrationsPrometheusID` - `IntegrationsPrometheusID`
...@@ -693,7 +693,7 @@ This is because we are deprecating using a bespoke method outside of the ...@@ -693,7 +693,7 @@ This is because we are deprecating using a bespoke method outside of the
[`@deprecated` directive](https://spec.graphql.org/June2018/#sec--deprecated), so validators are not [`@deprecated` directive](https://spec.graphql.org/June2018/#sec--deprecated), so validators are not
aware of the support. aware of the support.
The documentation will mention that the old Global ID style is now deprecated. The documentation mentions that the old Global ID style is now deprecated.
See also [Aliasing and deprecating mutations](#aliasing-and-deprecating-mutations). See also [Aliasing and deprecating mutations](#aliasing-and-deprecating-mutations).
...@@ -884,7 +884,7 @@ field :genus, ...@@ -884,7 +884,7 @@ field :genus,
see: { 'Wikipedia page on genera' => 'https://wikipedia.org/wiki/Genus' } see: { 'Wikipedia page on genera' => 'https://wikipedia.org/wiki/Genus' }
``` ```
This will render in our documentation as: This renders in our documentation as:
```markdown ```markdown
A taxonomic genus. See: [Wikipedia page on genera](https://wikipedia.org/wiki/Genus) A taxonomic genus. See: [Wikipedia page on genera](https://wikipedia.org/wiki/Genus)
...@@ -959,11 +959,11 @@ overhead. If you are writing: ...@@ -959,11 +959,11 @@ overhead. If you are writing:
### Error handling ### Error handling
Resolvers may raise errors, which will be converted to top-level errors as Resolvers may raise errors, which are converted to top-level errors as
appropriate. All anticipated errors should be caught and transformed to an appropriate. All anticipated errors should be caught and transformed to an
appropriate GraphQL error (see appropriate GraphQL error (see
[`Gitlab::Graphql::Errors`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/graphql/errors.rb)). [`Gitlab::Graphql::Errors`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/graphql/errors.rb)).
Any uncaught errors will be suppressed and the client will receive the message Any uncaught errors are suppressed and the client receives the message
`Internal service error`. `Internal service error`.
The one special case is permission errors. In the REST API we return The one special case is permission errors. In the REST API we return
...@@ -974,7 +974,7 @@ Query resolvers **should not raise errors for unauthorized resources**. ...@@ -974,7 +974,7 @@ Query resolvers **should not raise errors for unauthorized resources**.
The rationale for this is that clients must not be able to distinguish between The rationale for this is that clients must not be able to distinguish between
the absence of a record and the presence of one they do not have access to. To the absence of a record and the presence of one they do not have access to. To
do so is a security vulnerability, since it leaks information we want to keep do so is a security vulnerability, because it leaks information we want to keep
hidden. hidden.
In most cases you don't need to worry about this - this is handled correctly by In most cases you don't need to worry about this - this is handled correctly by
...@@ -1137,7 +1137,7 @@ class MyThingResolver < BaseResolver ...@@ -1137,7 +1137,7 @@ class MyThingResolver < BaseResolver
end end
``` ```
By default, fields defined in `#preloads` will be preloaded if that field By default, fields defined in `#preloads` are preloaded if that field
is selected in the query. Occasionally, finer control may be is selected in the query. Occasionally, finer control may be
needed to avoid preloading too much or incorrect content. needed to avoid preloading too much or incorrect content.
...@@ -1221,7 +1221,7 @@ available in the `Resolver` class as `parent`. ...@@ -1221,7 +1221,7 @@ available in the `Resolver` class as `parent`.
To find the parent object in your `Presenter` class: To find the parent object in your `Presenter` class:
1. Add the parent object to the GraphQL `context` from within your resolver's `resolve` method: 1. Add the parent object to the GraphQL `context` from your resolver's `resolve` method:
```ruby ```ruby
def resolve(**args) def resolve(**args)
...@@ -1416,7 +1416,7 @@ Where an object has an `iid`, prefer to use the `full_path` or `group_path` ...@@ -1416,7 +1416,7 @@ Where an object has an `iid`, prefer to use the `full_path` or `group_path`
of its parent in combination with its `iid` as arguments to identify an of its parent in combination with its `iid` as arguments to identify an
object rather than its `id`. object rather than its `id`.
See also [Deprecating Global IDs](#deprecating-global-ids). See also [Deprecate Global IDs](#deprecate-global-ids).
### Fields ### Fields
...@@ -1429,7 +1429,7 @@ In the most common situations, a mutation would return 2 fields: ...@@ -1429,7 +1429,7 @@ In the most common situations, a mutation would return 2 fields:
By inheriting any new mutations from `Mutations::BaseMutation` the By inheriting any new mutations from `Mutations::BaseMutation` the
`errors` field is automatically added. A `clientMutationId` field is `errors` field is automatically added. A `clientMutationId` field is
also added, this can be used by the client to identify the result of a also added, this can be used by the client to identify the result of a
single mutation when multiple are performed within a single request. single mutation when multiple are performed in a single request.
### The `resolve` method ### The `resolve` method
...@@ -1549,7 +1549,7 @@ There are three states a mutation response can be in: ...@@ -1549,7 +1549,7 @@ There are three states a mutation response can be in:
#### Success #### Success
In the happy path, errors *may* be returned, along with the anticipated payload, but In the happy path, errors *may* be returned, along with the anticipated payload, but
if everything was successful, then `errors` should be an empty array, since if everything was successful, then `errors` should be an empty array, because
there are no problems we need to inform the user of. there are no problems we need to inform the user of.
```javascript ```javascript
...@@ -1626,7 +1626,7 @@ of errors should be treated as internal, and not shown to the user in specific ...@@ -1626,7 +1626,7 @@ of errors should be treated as internal, and not shown to the user in specific
detail. detail.
We need to inform the user when the mutation fails, but we do not need to We need to inform the user when the mutation fails, but we do not need to
tell them why, since they cannot have caused it, and nothing they can do tell them why, because they cannot have caused it, and nothing they can do
fixes it, although we may offer to retry the mutation. fixes it, although we may offer to retry the mutation.
#### Categorizing errors #### Categorizing errors
...@@ -1646,7 +1646,7 @@ See also the [frontend GraphQL guide](../development/fe_guide/graphql.md#handlin ...@@ -1646,7 +1646,7 @@ See also the [frontend GraphQL guide](../development/fe_guide/graphql.md#handlin
### Aliasing and deprecating mutations ### Aliasing and deprecating mutations
The `#mount_aliased_mutation` helper allows us to alias a mutation as The `#mount_aliased_mutation` helper allows us to alias a mutation as
another name within `MutationType`. another name in `MutationType`.
For example, to alias a mutation called `FooMutation` as `BarMutation`: For example, to alias a mutation called `FooMutation` as `BarMutation`:
...@@ -1667,7 +1667,7 @@ mount_aliased_mutation 'UpdateFoo', ...@@ -1667,7 +1667,7 @@ mount_aliased_mutation 'UpdateFoo',
``` ```
Deprecated mutations should be added to `Types::DeprecatedMutations` and Deprecated mutations should be added to `Types::DeprecatedMutations` and
tested for within the unit test of `Types::MutationType`. The merge request tested for in the unit test of `Types::MutationType`. The merge request
[!34798](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34798) [!34798](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34798)
can be referred to as an example of this, including the method of testing can be referred to as an example of this, including the method of testing
deprecated aliased mutations. deprecated aliased mutations.
...@@ -1693,7 +1693,7 @@ We cannot test subscriptions using GraphiQL, because they require an Action Cabl ...@@ -1693,7 +1693,7 @@ We cannot test subscriptions using GraphiQL, because they require an Action Cabl
All fields under `Types::SubscriptionType` are subscriptions that clients can subscribe to. These fields require a subscription class, All fields under `Types::SubscriptionType` are subscriptions that clients can subscribe to. These fields require a subscription class,
which is a descendant of `Subscriptions::BaseSubscription` and is stored under `app/graphql/subscriptions`. which is a descendant of `Subscriptions::BaseSubscription` and is stored under `app/graphql/subscriptions`.
The arguments required to subscribe and the fields that are returned are defined within the subscription class. Multiple fields can share The arguments required to subscribe and the fields that are returned are defined in the subscription class. Multiple fields can share
the same subscription class if they have the same arguments and return the same fields. the same subscription class if they have the same arguments and return the same fields.
This class runs during the initial subscription request and subsequent updates. You can read more about this in the This class runs during the initial subscription request and subsequent updates. You can read more about this in the
...@@ -1725,8 +1725,8 @@ as normal. ...@@ -1725,8 +1725,8 @@ as normal.
Sometimes a mutation or resolver may accept a number of optional Sometimes a mutation or resolver may accept a number of optional
arguments, but we still want to validate that at least one of the optional arguments, but we still want to validate that at least one of the optional
arguments is provided. In this situation, consider using the `#ready?` arguments is provided. In this situation, consider using the `#ready?`
method within your mutation or resolver to provide the validation. The method in your mutation or resolver to provide the validation. The
`#ready?` method is called before any work is done within the `#ready?` method is called before any work is done in the
`#resolve` method. `#resolve` method.
Example: Example:
...@@ -1800,7 +1800,7 @@ For speed, you should test most logic in unit tests instead of integration tests ...@@ -1800,7 +1800,7 @@ For speed, you should test most logic in unit tests instead of integration tests
However, integration tests that check if data is returned verify the following However, integration tests that check if data is returned verify the following
additional items: additional items:
- The mutation is actually queryable within the schema (was mounted in `MutationType`). - The mutation is actually queryable in the schema (was mounted in `MutationType`).
- The data returned by a resolver or mutation correctly matches the - The data returned by a resolver or mutation correctly matches the
[return types](https://graphql-ruby.org/fields/introduction.html#field-return-type) of [return types](https://graphql-ruby.org/fields/introduction.html#field-return-type) of
the fields and resolves without errors. the fields and resolves without errors.
...@@ -1948,7 +1948,7 @@ to protect server resources from overly ambitious or malicious queries. ...@@ -1948,7 +1948,7 @@ to protect server resources from overly ambitious or malicious queries.
These values can be set as defaults and overridden in specific queries as needed. These values can be set as defaults and overridden in specific queries as needed.
The complexity values can be set per object as well, and the final query complexity is The complexity values can be set per object as well, and the final query complexity is
evaluated based on how many objects are being returned. This is useful evaluated based on how many objects are being returned. This is useful
for objects that are expensive (e.g. requiring Gitaly calls). for objects that are expensive (such as requiring Gitaly calls).
For example, a conditional complexity method in a resolver: For example, a conditional complexity method in a resolver:
......
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