Commit 60d61b68 authored by Luke Duncalfe's avatar Luke Duncalfe Committed by Fabio Pitino

Add Enum support for `deprecated` argument

`ruby-graphql` supports

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27464 added support
for a custom `deprecated` keyword for `field` to allow us to specify the
milestone that a field was deprecated in a structured way.

Enum values are also able to be deprecated. This change adds support for
enum values.

Also see: https://gitlab.com/gitlab-org/gitlab/-/issues/199952
parent dfc26be5
...@@ -2,9 +2,12 @@ ...@@ -2,9 +2,12 @@
module Types module Types
class BaseEnum < GraphQL::Schema::Enum class BaseEnum < GraphQL::Schema::Enum
extend GitlabStyleDeprecations
class << self class << self
def value(*args, **kwargs, &block) def value(*args, **kwargs, &block)
enum[args[0].downcase] = kwargs[:value] || args[0] enum[args[0].downcase] = kwargs[:value] || args[0]
kwargs = gitlab_deprecation(kwargs)
super(*args, **kwargs, &block) super(*args, **kwargs, &block)
end end
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
module Types module Types
class BaseField < GraphQL::Schema::Field class BaseField < GraphQL::Schema::Field
prepend Gitlab::Graphql::Authorize prepend Gitlab::Graphql::Authorize
include GitlabStyleDeprecations
DEFAULT_COMPLEXITY = 1 DEFAULT_COMPLEXITY = 1
...@@ -12,7 +13,7 @@ module Types ...@@ -12,7 +13,7 @@ module Types
kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity]) kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity])
@feature_flag = kwargs[:feature_flag] @feature_flag = kwargs[:feature_flag]
kwargs = check_feature_flag(kwargs) kwargs = check_feature_flag(kwargs)
kwargs = handle_deprecated(kwargs) kwargs = gitlab_deprecation(kwargs)
super(*args, **kwargs, &block) super(*args, **kwargs, &block)
end end
...@@ -52,28 +53,6 @@ module Types ...@@ -52,28 +53,6 @@ module Types
args args
end end
def handle_deprecated(kwargs)
if kwargs[:deprecation_reason].present?
raise ArgumentError, 'Use `deprecated` property instead of `deprecation_reason`. ' \
'See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields'
end
deprecation = kwargs.delete(:deprecated)
return kwargs unless deprecation
milestone, reason = deprecation.values_at(:milestone, :reason).map(&:presence)
raise ArgumentError, 'Please provide a `milestone` within `deprecated`' unless milestone
raise ArgumentError, 'Please provide a `reason` within `deprecated`' unless reason
raise ArgumentError, '`milestone` must be a `String`' unless milestone.is_a?(String)
deprecated_in = "Deprecated in #{milestone}"
kwargs[:deprecation_reason] = "#{reason}. #{deprecated_in}"
kwargs[:description] += ". #{deprecated_in}: #{reason}" if kwargs[:description]
kwargs
end
def field_complexity(resolver_class, current) def field_complexity(resolver_class, current)
return current if current.present? && current > 0 return current if current.present? && current > 0
......
# frozen_string_literal: true
# Concern for handling deprecation arguments.
# https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields-and-enum-values
module GitlabStyleDeprecations
extend ActiveSupport::Concern
private
def gitlab_deprecation(kwargs)
if kwargs[:deprecation_reason].present?
raise ArgumentError, 'Use `deprecated` property instead of `deprecation_reason`. ' \
'See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields-and-enum-values'
end
deprecation = kwargs.delete(:deprecated)
return kwargs unless deprecation
milestone, reason = deprecation.values_at(:milestone, :reason).map(&:presence)
raise ArgumentError, 'Please provide a `milestone` within `deprecated`' unless milestone
raise ArgumentError, 'Please provide a `reason` within `deprecated`' unless reason
raise ArgumentError, '`milestone` must be a `String`' unless milestone.is_a?(String)
deprecated_in = "Deprecated in #{milestone}"
kwargs[:deprecation_reason] = "#{reason}. #{deprecated_in}"
kwargs[:description] += ". #{deprecated_in}: #{reason}" if kwargs[:description]
kwargs
end
end
...@@ -49,7 +49,8 @@ module Gitlab ...@@ -49,7 +49,8 @@ module Gitlab
#{config.root}/app/models/members #{config.root}/app/models/members
#{config.root}/app/models/project_services #{config.root}/app/models/project_services
#{config.root}/app/graphql/resolvers/concerns #{config.root}/app/graphql/resolvers/concerns
#{config.root}/app/graphql/mutations/concerns]) #{config.root}/app/graphql/mutations/concerns
#{config.root}/app/graphql/types/concerns])
config.generators.templates.push("#{config.root}/generator_templates") config.generators.templates.push("#{config.root}/generator_templates")
......
...@@ -366,16 +366,16 @@ def foo ...@@ -366,16 +366,16 @@ def foo
end end
``` ```
## Deprecating fields ## Deprecating fields and enum values
GitLab's GraphQL API is versionless, which means we maintain backwards GitLab's GraphQL API is versionless, which means we maintain backwards
compatibility with older versions of the API with every change. Rather compatibility with older versions of the API with every change. Rather
than removing a field, we need to _deprecate_ the field instead. In than removing a field or [enum value](#enums), we need to _deprecate_ it instead.
future, GitLab In future, GitLab
[may remove deprecated fields](https://gitlab.com/gitlab-org/gitlab/-/issues/32292). [may remove deprecated parts of the schema](https://gitlab.com/gitlab-org/gitlab/-/issues/32292).
Fields are deprecated using the `deprecated` property. The value Fields and enum values are deprecated using the `deprecated` property.
of the property is a `Hash` of: The value of the property is a `Hash` of:
- `reason` - Reason for the deprecation. - `reason` - Reason for the deprecation.
- `milestone` - Milestone that the field was deprecated. - `milestone` - Milestone that the field was deprecated.
...@@ -388,13 +388,14 @@ field :token, GraphQL::STRING_TYPE, null: true, ...@@ -388,13 +388,14 @@ field :token, GraphQL::STRING_TYPE, null: true,
description: 'Token for login' description: 'Token for login'
``` ```
The original `description:` of the field should be maintained, and should The original `description` of the things being deprecated should be maintained,
_not_ be updated to mention the deprecation. and should _not_ be updated to mention the deprecation. Instead, the `reason` will
be appended to the `description`.
### Deprecation reason style guide ### Deprecation reason style guide
Where the reason for deprecation is due to the field being replaced Where the reason for deprecation is due to the field or enum value being
with another field, the `reason` must be: replaced, the `reason` must be:
```plaintext ```plaintext
Use `otherFieldName` Use `otherFieldName`
...@@ -408,9 +409,22 @@ field :designs, ::Types::DesignManagement::DesignCollectionType, null: true, ...@@ -408,9 +409,22 @@ field :designs, ::Types::DesignManagement::DesignCollectionType, null: true,
description: 'The designs associated with this issue', description: 'The designs associated with this issue',
``` ```
```ruby
module Types
class TodoStateEnum < BaseEnum
value 'pending', deprecated: { reason: 'Use PENDING', milestone: '10.0' }
value 'done', deprecated: { reason: 'Use DONE', milestone: '10.0' }
value 'PENDING', value: 'pending'
value 'DONE', value: 'done'
end
end
```
If the field is not being replaced by another field, a descriptive If the field is not being replaced by another field, a descriptive
deprecation `reason` should be given. deprecation `reason` should be given.
See also [Aliasing and deprecating mutations](#aliasing-and-deprecating-mutations).
## Enums ## Enums
GitLab GraphQL enums are defined in `app/graphql/types`. When defining new enums, the GitLab GraphQL enums are defined in `app/graphql/types`. When defining new enums, the
...@@ -455,6 +469,9 @@ module Types ...@@ -455,6 +469,9 @@ module Types
end end
``` ```
Enum values can be deprecated using the
[`deprecated` keyword](#deprecating-fields-and-enum-values).
## JSON ## JSON
When data to be returned by GraphQL is stored as When data to be returned by GraphQL is stored as
...@@ -1155,7 +1172,8 @@ mount_aliased_mutation 'BarMutation', Mutations::FooMutation ...@@ -1155,7 +1172,8 @@ mount_aliased_mutation 'BarMutation', Mutations::FooMutation
``` ```
This allows us to rename a mutation and continue to support the old name, This allows us to rename a mutation and continue to support the old name,
when coupled with the [`deprecated`](#deprecating-fields) argument. when coupled with the [`deprecated`](#deprecating-fields-and-enum-values)
argument.
Example: Example:
......
...@@ -21,4 +21,16 @@ RSpec.describe Types::BaseEnum do ...@@ -21,4 +21,16 @@ RSpec.describe Types::BaseEnum do
expect(enum.enum).to be_a(HashWithIndifferentAccess) expect(enum.enum).to be_a(HashWithIndifferentAccess)
end end
end end
include_examples 'Gitlab-style deprecations' do
def subject(args = {})
enum = Class.new(described_class) do
graphql_name 'TestEnum'
value 'TEST_VALUE', **args
end
enum.to_graphql.values['TEST_VALUE']
end
end
end end
...@@ -167,70 +167,23 @@ RSpec.describe Types::BaseField do ...@@ -167,70 +167,23 @@ RSpec.describe Types::BaseField do
end end
end end
describe '`deprecated` property' do include_examples 'Gitlab-style deprecations' do
def test_field(args = {}) def subject(args = {})
base_args = { name: 'test', type: GraphQL::STRING_TYPE, null: true } base_args = { name: 'test', type: GraphQL::STRING_TYPE, null: true }
described_class.new(**base_args.merge(args)) described_class.new(**base_args.merge(args))
end end
describe 'validations' do
it 'raises an informative error if `deprecation_reason` is used' do
expect { test_field(deprecation_reason: 'foo') }.to raise_error(
ArgumentError,
'Use `deprecated` property instead of `deprecation_reason`. ' \
'See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields'
)
end
it 'raises an error if a required property is missing', :aggregate_failures do
expect { test_field(deprecated: { milestone: '1.10' }) }.to raise_error(
ArgumentError,
'Please provide a `reason` within `deprecated`'
)
expect { test_field(deprecated: { reason: 'Deprecation reason' }) }.to raise_error(
ArgumentError,
'Please provide a `milestone` within `deprecated`'
)
end
it 'raises an error if milestone is not a String', :aggregate_failures do
expect { test_field(deprecated: { milestone: 1.10, reason: 'Deprecation reason' }) }.to raise_error(
ArgumentError,
'`milestone` must be a `String`'
)
end
end
it 'adds a formatted `deprecated_reason` to the field' do
field = test_field(deprecated: { milestone: '1.10', reason: 'Deprecation reason' })
expect(field.deprecation_reason).to eq('Deprecation reason. Deprecated in 1.10')
end
it 'appends to the description if given' do
field = test_field(
deprecated: { milestone: '1.10', reason: 'Deprecation reason' },
description: 'Field description'
)
expect(field.description).to eq('Field description. Deprecated in 1.10: Deprecation reason')
end
it 'does not append to the description if it is absent' do
field = test_field(deprecated: { milestone: '1.10', reason: 'Deprecation reason' })
expect(field.description).to be_nil
end
it 'interacts well with the `feature_flag` property' do it 'interacts well with the `feature_flag` property' do
field = test_field( field = subject(
deprecated: { milestone: '1.10', reason: 'Deprecation reason' }, deprecated: { milestone: '1.10', reason: 'Deprecation reason' },
description: 'Field description', description: 'Field description',
feature_flag: 'foo_flag' feature_flag: 'foo_flag'
) )
expect(field.description).to eq('Field description. Available only when feature flag `foo_flag` is enabled. Deprecated in 1.10: Deprecation reason') expectation = 'Field description. Available only when feature flag `foo_flag` is enabled. Deprecated in 1.10: Deprecation reason'
expect(field.description).to eq(expectation)
end end
end end
end end
...@@ -99,7 +99,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do ...@@ -99,7 +99,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
graphql_name 'MyEnum' graphql_name 'MyEnum'
value 'BAZ', description: 'A description of BAZ' value 'BAZ', description: 'A description of BAZ'
value 'BAR', description: 'A description of BAR', deprecation_reason: 'This is deprecated' value 'BAR', description: 'A description of BAR', deprecated: { reason: 'This is deprecated', milestone: '1.10' }
end end
Class.new(Types::BaseObject) do Class.new(Types::BaseObject) do
...@@ -115,7 +115,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do ...@@ -115,7 +115,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
| Value | Description | | Value | Description |
| ----- | ----------- | | ----- | ----------- |
| `BAR` **{warning-solid}** | **Deprecated:** This is deprecated | | `BAR` **{warning-solid}** | **Deprecated:** This is deprecated. Deprecated in 1.10 |
| `BAZ` | A description of BAZ | | `BAZ` | A description of BAZ |
DOC DOC
......
# frozen_string_literal: true
RSpec.shared_examples 'Gitlab-style deprecations' do
describe 'validations' do
it 'raises an informative error if `deprecation_reason` is used' do
expect { subject(deprecation_reason: 'foo') }.to raise_error(
ArgumentError,
'Use `deprecated` property instead of `deprecation_reason`. ' \
'See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields-and-enum-values'
)
end
it 'raises an error if a required property is missing', :aggregate_failures do
expect { subject(deprecated: { milestone: '1.10' }) }.to raise_error(
ArgumentError,
'Please provide a `reason` within `deprecated`'
)
expect { subject(deprecated: { reason: 'Deprecation reason' }) }.to raise_error(
ArgumentError,
'Please provide a `milestone` within `deprecated`'
)
end
it 'raises an error if milestone is not a String', :aggregate_failures do
expect { subject(deprecated: { milestone: 1.10, reason: 'Deprecation reason' }) }.to raise_error(
ArgumentError,
'`milestone` must be a `String`'
)
end
end
it 'adds a formatted `deprecated_reason` to the subject' do
deprecable = subject(deprecated: { milestone: '1.10', reason: 'Deprecation reason' })
expect(deprecable.deprecation_reason).to eq('Deprecation reason. Deprecated in 1.10')
end
it 'appends to the description if given' do
deprecable = subject(
deprecated: { milestone: '1.10', reason: 'Deprecation reason' },
description: 'Deprecable description'
)
expect(deprecable.description).to eq('Deprecable description. Deprecated in 1.10: Deprecation reason')
end
it 'does not append to the description if it is absent' do
deprecable = subject(deprecated: { milestone: '1.10', reason: 'Deprecation reason' })
expect(deprecable.description).to be_nil
end
end
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