Merge branch 'ld-support-deprecations-for-enums' into 'master'

Add Enum support for `deprecated` argument

See merge request gitlab-org/gitlab!41630

app/graphql/types/base_enum.rb

@@ -2,9 +2,12 @@
 
 module Types
   class BaseEnum < GraphQL::Schema::Enum
+    extend GitlabStyleDeprecations
+
     class << self
       def value(*args, **kwargs, &block)
         enum[args[0].downcase] = kwargs[:value] || args[0]
+        kwargs = gitlab_deprecation(kwargs)
 
         super(*args, **kwargs, &block)
       end

app/graphql/types/base_field.rb

@@ -3,6 +3,7 @@
 module Types
   class BaseField < GraphQL::Schema::Field
     prepend Gitlab::Graphql::Authorize
+    include GitlabStyleDeprecations
 
     DEFAULT_COMPLEXITY = 1
 
@@ -12,7 +13,7 @@ module Types
       kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity])
       @feature_flag = kwargs[:feature_flag]
       kwargs = check_feature_flag(kwargs)
-      kwargs = handle_deprecated(kwargs)
+      kwargs = gitlab_deprecation(kwargs)
 
       super(*args, **kwargs, &block)
     end
@@ -52,28 +53,6 @@ module Types
       args
     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)
       return current if current.present? && current > 0
 

app/graphql/types/concerns/gitlab_style_deprecations.rb

+# 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

config/application.rb

@@ -49,7 +49,8 @@ module Gitlab
                                      #{config.root}/app/models/members
                                      #{config.root}/app/models/project_services
                                      #{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")
 

doc/development/api_graphql_styleguide.md

@@ -366,16 +366,16 @@ def foo
 end
 ```
 
-## Deprecating fields
+## Deprecating fields and enum values
 
 GitLab's GraphQL API is versionless, which means we maintain backwards
 compatibility with older versions of the API with every change. Rather
-than removing a field, we need to _deprecate_ the field instead. In
-future, GitLab
-[may remove deprecated fields](https://gitlab.com/gitlab-org/gitlab/-/issues/32292).
+than removing a field or [enum value](#enums), we need to _deprecate_ it instead.
+In future, GitLab
+[may remove deprecated parts of the schema](https://gitlab.com/gitlab-org/gitlab/-/issues/32292).
 
-Fields are deprecated using the `deprecated` property. The value
-of the property is a `Hash` of:
+Fields and enum values are deprecated using the `deprecated` property.
+The value of the property is a `Hash` of:
 
 - `reason` - Reason for the deprecation.
 - `milestone` - Milestone that the field was deprecated.
@@ -388,13 +388,14 @@ field :token, GraphQL::STRING_TYPE, null: true,
       description: 'Token for login'
 ```
 
-The original `description:` of the field should be maintained, and should
-_not_ be updated to mention the deprecation.
+The original `description` of the things being deprecated should be maintained,
+and should _not_ be updated to mention the deprecation. Instead, the `reason` will
+be appended to the `description`.
 
 ### Deprecation reason style guide
 
-Where the reason for deprecation is due to the field being replaced
-with another field, the `reason` must be:
+Where the reason for deprecation is due to the field or enum value being
+replaced, the `reason` must be:
 
 ```plaintext
 Use `otherFieldName`
@@ -408,9 +409,22 @@ field :designs, ::Types::DesignManagement::DesignCollectionType, null: true,
       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
 deprecation `reason` should be given.
 
+See also [Aliasing and deprecating mutations](#aliasing-and-deprecating-mutations).
+
 ## Enums
 
 GitLab GraphQL enums are defined in `app/graphql/types`. When defining new enums, the
@@ -455,6 +469,9 @@ module Types
 end
 ```
 
+Enum values can be deprecated using the
+[`deprecated` keyword](#deprecating-fields-and-enum-values).
+
 ## JSON
 
 When data to be returned by GraphQL is stored as
@@ -1155,7 +1172,8 @@ mount_aliased_mutation 'BarMutation', Mutations::FooMutation
 ```
 
 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:
 

spec/graphql/types/base_enum_spec.rb

@@ -21,4 +21,16 @@ RSpec.describe Types::BaseEnum do
       expect(enum.enum).to be_a(HashWithIndifferentAccess)
     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

spec/graphql/types/base_field_spec.rb

@@ -167,70 +167,23 @@ RSpec.describe Types::BaseField do
     end
   end
 
-  describe '`deprecated` property' do
-    def test_field(args = {})
+  include_examples 'Gitlab-style deprecations' do
+    def subject(args = {})
       base_args = { name: 'test', type: GraphQL::STRING_TYPE, null: true }
 
       described_class.new(**base_args.merge(args))
     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
-      field = test_field(
+      field = subject(
         deprecated: { milestone: '1.10', reason: 'Deprecation reason' },
         description: 'Field description',
         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

spec/lib/gitlab/graphql/docs/renderer_spec.rb

@@ -99,7 +99,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
           graphql_name 'MyEnum'
 
           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
 
         Class.new(Types::BaseObject) do
@@ -115,7 +115,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
 
           | 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 |
         DOC
 

spec/support/shared_examples/graphql/types/gitlab_style_deprecations_shared_examples.rb

+# 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