Merge branch '199952-GraphQL-deprecated' into 'master'

Add `deprecated` property for GraphQL

See merge request gitlab-org/gitlab!27464

app/graphql/types/base_field.rb

@@ -12,6 +12,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)
 
       super(*args, **kwargs, &block)
     end
@@ -41,7 +42,7 @@ module Types
     attr_reader :feature_flag
 
     def feature_documentation_message(key, description)
-      "#{description}. Available only when feature flag `#{key}` is enabled."
+      "#{description}. Available only when feature flag `#{key}` is enabled"
     end
 
     def check_feature_flag(args)
@@ -51,6 +52,27 @@ 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
+
+      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/commit_type.rb

@@ -44,8 +44,8 @@ module Types
     field :latest_pipeline,
           type: Types::Ci::PipelineType,
           null: true,
-          description: "Latest pipeline of the commit",
-          deprecation_reason: 'Use pipelines',
+          deprecated: { reason: 'Use `pipelines`', milestone: 12.5 },
+          description: 'Latest pipeline of the commit',
           resolver: Resolvers::CommitPipelinesResolver.last
   end
 end

app/graphql/types/grafana_integration_type.rb

@@ -18,8 +18,8 @@ module Types
           description: 'Timestamp of the issue\'s last activity'
 
     field :token, GraphQL::STRING_TYPE, null: false,
-          deprecation_reason: 'Plain text token has been masked for security reasons',
-          description: 'API token for the Grafana integration. Field is permanently masked.'
+          deprecated: { reason: 'Plain text token has been masked for security reasons', milestone: 12.7 },
+          description: 'API token for the Grafana integration'
 
     def token
       object.masked_token

app/graphql/types/merge_request_type.rb

@@ -74,8 +74,9 @@ module Types
           description: 'Rebase commit SHA of the merge request'
     field :rebase_in_progress, GraphQL::BOOLEAN_TYPE, method: :rebase_in_progress?, null: false, calls_gitaly: true,
           description: 'Indicates if there is a rebase currently in progress for the merge request'
-    field :merge_commit_message, GraphQL::STRING_TYPE, method: :default_merge_commit_message, null: true, deprecation_reason: "Renamed to defaultMergeCommitMessage",
-          description: 'Deprecated - renamed to defaultMergeCommitMessage'
+    field :merge_commit_message, GraphQL::STRING_TYPE, method: :default_merge_commit_message, null: true,
+          deprecated: { reason: 'Use `defaultMergeCommitMessage`', milestone: 11.8 },
+          description: 'Default merge commit message of the merge request'
     field :default_merge_commit_message, GraphQL::STRING_TYPE, null: true,
           description: 'Default merge commit message of the merge request'
     field :merge_ongoing, GraphQL::BOOLEAN_TYPE, method: :merge_ongoing?, null: false,

doc/api/graphql/reference/gitlab_schema.graphql

@@ -306,7 +306,7 @@ type Commit {
   id: ID!
 
   """
-  Latest pipeline of the commit
+  Latest pipeline of the commit. Deprecated in 12.5: Use `pipelines`
   """
   latestPipeline(
     """
@@ -323,7 +323,7 @@ type Commit {
     Filter pipelines by their status
     """
     status: PipelineStatusEnum
-  ): Pipeline @deprecated(reason: "Use pipelines")
+  ): Pipeline @deprecated(reason: "Use `pipelines`. Deprecated in 12.5")
 
   """
   Raw commit message
@@ -2493,9 +2493,9 @@ type EpicIssue implements Noteable {
   designCollection: DesignCollection
 
   """
-  Deprecated. Use `designCollection`
+  The designs associated with this issue. Deprecated in 12.2: Use `designCollection`
   """
-  designs: DesignCollection @deprecated(reason: "Use designCollection")
+  designs: DesignCollection @deprecated(reason: "Use `designCollection`. Deprecated in 12.2")
 
   """
   Indicates discussion is locked on the issue
@@ -2548,7 +2548,7 @@ type EpicIssue implements Noteable {
   epicIssueId: ID!
 
   """
-  Current health status. Available only when feature flag `save_issuable_health_status` is enabled.
+  Current health status. Available only when feature flag `save_issuable_health_status` is enabled
   """
   healthStatus: HealthStatus
 
@@ -2984,9 +2984,9 @@ type GrafanaIntegration {
   id: ID!
 
   """
-  API token for the Grafana integration. Field is permanently masked.
+  API token for the Grafana integration. Deprecated in 12.7: Plain text token has been masked for security reasons
   """
-  token: String! @deprecated(reason: "Plain text token has been masked for security reasons")
+  token: String! @deprecated(reason: "Plain text token has been masked for security reasons. Deprecated in 12.7")
 
   """
   Timestamp of the issue's last activity
@@ -3489,9 +3489,9 @@ type Issue implements Noteable {
   designCollection: DesignCollection
 
   """
-  Deprecated. Use `designCollection`
+  The designs associated with this issue. Deprecated in 12.2: Use `designCollection`
   """
-  designs: DesignCollection @deprecated(reason: "Use designCollection")
+  designs: DesignCollection @deprecated(reason: "Use `designCollection`. Deprecated in 12.2")
 
   """
   Indicates discussion is locked on the issue
@@ -3539,7 +3539,7 @@ type Issue implements Noteable {
   epic: Epic
 
   """
-  Current health status. Available only when feature flag `save_issuable_health_status` is enabled.
+  Current health status. Available only when feature flag `save_issuable_health_status` is enabled
   """
   healthStatus: HealthStatus
 
@@ -4242,9 +4242,9 @@ type MergeRequest implements Noteable {
   ): LabelConnection
 
   """
-  Deprecated - renamed to defaultMergeCommitMessage
+  Default merge commit message of the merge request. Deprecated in 11.8: Use `defaultMergeCommitMessage`
   """
-  mergeCommitMessage: String @deprecated(reason: "Renamed to defaultMergeCommitMessage")
+  mergeCommitMessage: String @deprecated(reason: "Use `defaultMergeCommitMessage`. Deprecated in 11.8")
 
   """
   SHA of the merge request commit (set once merged)
@@ -6191,7 +6191,7 @@ type Project {
   visibility: String
 
   """
-  Vulnerabilities reported on the project. Available only when feature flag `first_class_vulnerabilities` is enabled.
+  Vulnerabilities reported on the project. Available only when feature flag `first_class_vulnerabilities` is enabled
   """
   vulnerabilities(
     """

doc/api/graphql/reference/gitlab_schema.json

@@ -968,7 +968,7 @@
             },
             {
               "name": "latestPipeline",
-              "description": "Latest pipeline of the commit",
+              "description": "Latest pipeline of the commit. Deprecated in 12.5: Use `pipelines`",
               "args": [
                 {
                   "name": "status",
@@ -1007,7 +1007,7 @@
                 "ofType": null
               },
               "isDeprecated": true,
-              "deprecationReason": "Use pipelines"
+              "deprecationReason": "Use `pipelines`. Deprecated in 12.5"
             },
             {
               "name": "message",
@@ -7244,7 +7244,7 @@
             },
             {
               "name": "designs",
-              "description": "Deprecated. Use `designCollection`",
+              "description": "The designs associated with this issue. Deprecated in 12.2: Use `designCollection`",
               "args": [
 
               ],
@@ -7254,7 +7254,7 @@
                 "ofType": null
               },
               "isDeprecated": true,
-              "deprecationReason": "Use designCollection"
+              "deprecationReason": "Use `designCollection`. Deprecated in 12.2"
             },
             {
               "name": "discussionLocked",
@@ -7397,7 +7397,7 @@
             },
             {
               "name": "healthStatus",
-              "description": "Current health status. Available only when feature flag `save_issuable_health_status` is enabled.",
+              "description": "Current health status. Available only when feature flag `save_issuable_health_status` is enabled",
               "args": [
 
               ],
@@ -8659,7 +8659,7 @@
             },
             {
               "name": "token",
-              "description": "API token for the Grafana integration. Field is permanently masked.",
+              "description": "API token for the Grafana integration. Deprecated in 12.7: Plain text token has been masked for security reasons",
               "args": [
 
               ],
@@ -8673,7 +8673,7 @@
                 }
               },
               "isDeprecated": true,
-              "deprecationReason": "Plain text token has been masked for security reasons"
+              "deprecationReason": "Plain text token has been masked for security reasons. Deprecated in 12.7"
             },
             {
               "name": "updatedAt",
@@ -9990,7 +9990,7 @@
             },
             {
               "name": "designs",
-              "description": "Deprecated. Use `designCollection`",
+              "description": "The designs associated with this issue. Deprecated in 12.2: Use `designCollection`",
               "args": [
 
               ],
@@ -10000,7 +10000,7 @@
                 "ofType": null
               },
               "isDeprecated": true,
-              "deprecationReason": "Use designCollection"
+              "deprecationReason": "Use `designCollection`. Deprecated in 12.2"
             },
             {
               "name": "discussionLocked",
@@ -10125,7 +10125,7 @@
             },
             {
               "name": "healthStatus",
-              "description": "Current health status. Available only when feature flag `save_issuable_health_status` is enabled.",
+              "description": "Current health status. Available only when feature flag `save_issuable_health_status` is enabled",
               "args": [
 
               ],
@@ -12105,7 +12105,7 @@
             },
             {
               "name": "mergeCommitMessage",
-              "description": "Deprecated - renamed to defaultMergeCommitMessage",
+              "description": "Default merge commit message of the merge request. Deprecated in 11.8: Use `defaultMergeCommitMessage`",
               "args": [
 
               ],
@@ -12115,7 +12115,7 @@
                 "ofType": null
               },
               "isDeprecated": true,
-              "deprecationReason": "Renamed to defaultMergeCommitMessage"
+              "deprecationReason": "Use `defaultMergeCommitMessage`. Deprecated in 11.8"
             },
             {
               "name": "mergeCommitSha",
@@ -18522,7 +18522,7 @@
             },
             {
               "name": "vulnerabilities",
-              "description": "Vulnerabilities reported on the project. Available only when feature flag `first_class_vulnerabilities` is enabled.",
+              "description": "Vulnerabilities reported on the project. Available only when feature flag `first_class_vulnerabilities` is enabled",
               "args": [
                 {
                   "name": "after",

doc/api/graphql/reference/index.md

@@ -82,7 +82,7 @@ Represents a project or group board
 | `authoredDate` | Time | Timestamp of when the commit was authored |
 | `description` | String | Description of the commit message |
 | `id` | ID! | ID (global ID) of the commit |
-| `latestPipeline` **{warning-solid}** | Pipeline | **Deprecated:** Use pipelines |
+| `latestPipeline` **{warning-solid}** | Pipeline | **Deprecated:** Use `pipelines`. Deprecated in 12.5 |
 | `message` | String | Raw commit message |
 | `sha` | String! | SHA1 ID of the commit |
 | `signatureHtml` | String | Rendered HTML of the commit signature |
@@ -413,13 +413,13 @@ Relationship between an epic and an issue
 | `description` | String | Description of the issue |
 | `descriptionHtml` | String | The GitLab Flavored Markdown rendering of `description` |
 | `designCollection` | DesignCollection | Collection of design images associated with this issue |
-| `designs` **{warning-solid}** | DesignCollection | **Deprecated:** Use designCollection |
+| `designs` **{warning-solid}** | DesignCollection | **Deprecated:** Use `designCollection`. Deprecated in 12.2 |
 | `discussionLocked` | Boolean! | Indicates discussion is locked on the issue |
 | `downvotes` | Int! | Number of downvotes the issue has received |
 | `dueDate` | Time | Due date of the issue |
 | `epic` | Epic | Epic to which this issue belongs |
 | `epicIssueId` | ID! | ID of the epic-issue relation |
-| `healthStatus` | HealthStatus | Current health status. Available only when feature flag `save_issuable_health_status` is enabled. |
+| `healthStatus` | HealthStatus | Current health status. Available only when feature flag `save_issuable_health_status` is enabled |
 | `id` | ID | Global ID of the epic-issue relation |
 | `iid` | ID! | Internal ID of the issue |
 | `milestone` | Milestone | Milestone of the issue |
@@ -483,7 +483,7 @@ Autogenerated return type of EpicTreeReorder
 | `enabled` | Boolean! | Indicates whether Grafana integration is enabled |
 | `grafanaUrl` | String! | Url for the Grafana host for the Grafana integration |
 | `id` | ID! | Internal ID of the Grafana integration |
-| `token` **{warning-solid}** | String! | **Deprecated:** Plain text token has been masked for security reasons |
+| `token` **{warning-solid}** | String! | **Deprecated:** Plain text token has been masked for security reasons. Deprecated in 12.7 |
 | `updatedAt` | Time! | Timestamp of the issue's last activity |
 
 ## Group
@@ -535,12 +535,12 @@ Autogenerated return type of EpicTreeReorder
 | `description` | String | Description of the issue |
 | `descriptionHtml` | String | The GitLab Flavored Markdown rendering of `description` |
 | `designCollection` | DesignCollection | Collection of design images associated with this issue |
-| `designs` **{warning-solid}** | DesignCollection | **Deprecated:** Use designCollection |
+| `designs` **{warning-solid}** | DesignCollection | **Deprecated:** Use `designCollection`. Deprecated in 12.2 |
 | `discussionLocked` | Boolean! | Indicates discussion is locked on the issue |
 | `downvotes` | Int! | Number of downvotes the issue has received |
 | `dueDate` | Time | Due date of the issue |
 | `epic` | Epic | Epic to which this issue belongs |
-| `healthStatus` | HealthStatus | Current health status. Available only when feature flag `save_issuable_health_status` is enabled. |
+| `healthStatus` | HealthStatus | Current health status. Available only when feature flag `save_issuable_health_status` is enabled |
 | `iid` | ID! | Internal ID of the issue |
 | `milestone` | Milestone | Milestone of the issue |
 | `reference` | String! | Internal reference of the issue. Returned in shortened format by default |
@@ -644,7 +644,7 @@ Autogenerated return type of MarkAsSpamSnippet
 | `id` | ID! | ID of the merge request |
 | `iid` | String! | Internal ID of the merge request |
 | `inProgressMergeCommitSha` | String | Commit SHA of the merge request if merge is in progress |
-| `mergeCommitMessage` **{warning-solid}** | String | **Deprecated:** Renamed to defaultMergeCommitMessage |
+| `mergeCommitMessage` **{warning-solid}** | String | **Deprecated:** Use `defaultMergeCommitMessage`. Deprecated in 11.8 |
 | `mergeCommitSha` | String | SHA of the merge request commit (set once merged) |
 | `mergeError` | String | Error message due to a merge error |
 | `mergeOngoing` | Boolean! | Indicates if a merge is currently occurring |

doc/development/api_graphql_styleguide.md

@@ -283,6 +283,51 @@ the `some_feature_flag` feature flag is enabled.
 
 If the feature flag is not enabled, an error will be returned saying the field does not exist.
 
+## Deprecating fields
+
+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).
+
+Fields 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.
+
+Example:
+
+```ruby
+field :token, GraphQL::STRING_TYPE, null: true,
+      deprecated: { reason: 'Login via token has been removed', milestone: 10.0 },
+      description: 'Token for login'
+```
+
+The original `description:` of the field should be maintained, and should
+_not_ be updated to mention the deprecation.
+
+### Deprecation reason styleguide
+
+Where the reason for deprecation is due to the field being replaced
+with another field, the `reason` must be:
+
+```plaintext
+Use `otherFieldName`
+```
+
+Example:
+
+```ruby
+field :designs, ::Types::DesignManagement::DesignCollectionType, null: true,
+      deprecated: { reason: 'Use `designCollection`', milestone: 10.0 },
+      description: 'The designs associated with this issue',
+```
+
+If the field is not being replaced by another field, a descriptive
+deprecation `reason` should be given.
+
 ## Enums
 
 GitLab GraphQL enums are defined in `app/graphql/types`. When defining new enums, the

ee/app/graphql/ee/types/issue_type.rb

@@ -14,9 +14,9 @@ module EE
               resolve: -> (obj, _args, _ctx) { obj.supports_weight? ? obj.weight : nil }
 
         field :designs, ::Types::DesignManagement::DesignCollectionType, null: true,
-              description: "Deprecated. Use `designCollection`",
               method: :design_collection,
-              deprecation_reason: 'Use designCollection'
+              deprecated: { reason: 'Use `designCollection`', milestone: 12.2 },
+              description: 'The designs associated with this issue'
 
         field :design_collection, ::Types::DesignManagement::DesignCollectionType, null: true,
               description: 'Collection of design images associated with this issue'

spec/graphql/types/base_field_spec.rb

@@ -175,7 +175,7 @@ describe Types::BaseField do
       let(:flag) { :test_flag }
 
       it 'prepends the description' do
-        expect(field.description). to eq 'Test description. Available only when feature flag `test_flag` is enabled.'
+        expect(field.description). to eq 'Test description. Available only when feature flag `test_flag` is enabled'
       end
 
       context 'falsey feature_flag values' do
@@ -196,4 +196,64 @@ describe Types::BaseField do
       end
     end
   end
+
+  describe '`deprecated` property' do
+    def test_field(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.0 }) }.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
+    end
+
+    it 'adds a formatted `deprecated_reason` to the field' do
+      field = test_field(deprecated: { milestone: 1.0, reason: 'Deprecation reason' })
+
+      expect(field.deprecation_reason).to eq('Deprecation reason. Deprecated in 1.0')
+    end
+
+    it 'appends to the description if given' do
+      field = test_field(
+        deprecated: { milestone: 1.0, reason: 'Deprecation reason' },
+        description: 'Field description'
+      )
+
+      expect(field.description).to eq('Field description. Deprecated in 1.0: Deprecation reason')
+    end
+
+    it 'does not append to the description if it is absent' do
+      field = test_field(deprecated: { milestone: 1.0, reason: 'Deprecation reason' })
+
+      expect(field.description).to be_nil
+    end
+
+    it 'interacts well with the `feature_flag` property' do
+      field = test_field(
+        deprecated: { milestone: 1.0, 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.0: Deprecation reason')
+    end
+  end
 end

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

@@ -6,7 +6,7 @@ describe Gitlab::Graphql::Docs::Renderer do
   describe '#contents' do
     # Returns a Schema that uses the given `type`
     def mock_schema(type)
-      query_type = Class.new(GraphQL::Schema::Object) do
+      query_type = Class.new(Types::BaseObject) do
         graphql_name 'QueryType'
 
         field :foo, type, null: true
@@ -27,7 +27,7 @@ describe Gitlab::Graphql::Docs::Renderer do
 
     context 'A type with a field with a [Array] return type' do
       let(:type) do
-        Class.new(GraphQL::Schema::Object) do
+        Class.new(Types::BaseObject) do
           graphql_name 'ArrayTest'
 
           field :foo, [GraphQL::STRING_TYPE], null: false, description: 'A description'
@@ -49,7 +49,7 @@ describe Gitlab::Graphql::Docs::Renderer do
 
     context 'A type with fields defined in reverse alphabetical order' do
       let(:type) do
-        Class.new(GraphQL::Schema::Object) do
+        Class.new(Types::BaseObject) do
           graphql_name 'OrderingTest'
 
           field :foo, GraphQL::STRING_TYPE, null: false, description: 'A description of foo field'
@@ -73,10 +73,10 @@ describe Gitlab::Graphql::Docs::Renderer do
 
     context 'A type with a deprecated field' do
       let(:type) do
-        Class.new(GraphQL::Schema::Object) do
+        Class.new(Types::BaseObject) do
           graphql_name 'DeprecatedTest'
 
-          field :foo, GraphQL::STRING_TYPE, null: false, deprecation_reason: 'This is deprecated', description: 'A description'
+          field :foo, GraphQL::STRING_TYPE, null: false, deprecated: { reason: 'This is deprecated', milestone: 1.0 }, description: 'A description'
         end
       end
 
@@ -86,7 +86,7 @@ describe Gitlab::Graphql::Docs::Renderer do
 
           | Name  | Type  | Description |
           | ---   |  ---- | ----------  |
-          | `foo` **{warning-solid}** | String! | **Deprecated:** This is deprecated |
+          | `foo` **{warning-solid}** | String! | **Deprecated:** This is deprecated. Deprecated in 1.0 |
         DOC
 
         is_expected.to include(expectation)