Merge branch 'ajk-graphql-fancy-deprecations' into 'master'

First Class Deprecations for GraphQL

See merge request gitlab-org/gitlab!56698

app/graphql/mutations/base_mutation.rb

@@ -9,6 +9,7 @@ module Mutations
     ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'
 
     field_class ::Types::BaseField
+    argument_class ::Types::BaseArgument
 
     field :errors, [GraphQL::STRING_TYPE],
           null: false,

app/graphql/types/base_argument.rb

@@ -4,8 +4,10 @@ module Types
   class BaseArgument < GraphQL::Schema::Argument
     include GitlabStyleDeprecations
 
+    attr_reader :deprecation
+
     def initialize(*args, **kwargs, &block)
-      kwargs = gitlab_deprecation(kwargs)
+      @deprecation = gitlab_deprecation(kwargs)
 
       super(*args, **kwargs, &block)
     end

app/graphql/types/base_enum.rb

@@ -26,7 +26,7 @@ module Types
 
       def value(*args, **kwargs, &block)
         enum[args[0].downcase] = kwargs[:value] || args[0]
-        kwargs = gitlab_deprecation(kwargs)
+        gitlab_deprecation(kwargs)
 
         super(*args, **kwargs, &block)
       end

app/graphql/types/base_field.rb

@@ -8,6 +8,8 @@ module Types
 
     DEFAULT_COMPLEXITY = 1
 
+    attr_reader :deprecation
+
     def initialize(**kwargs, &block)
       @calls_gitaly = !!kwargs.delete(:calls_gitaly)
       @constant_complexity = kwargs[:complexity].is_a?(Integer) && kwargs[:complexity] > 0
@@ -16,7 +18,7 @@ module Types
       kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity])
       @feature_flag = kwargs[:feature_flag]
       kwargs = check_feature_flag(kwargs)
-      kwargs = gitlab_deprecation(kwargs)
+      @deprecation = gitlab_deprecation(kwargs)
 
       super(**kwargs, &block)
 

app/graphql/types/concerns/gitlab_style_deprecations.rb

@@ -7,25 +7,21 @@ module GitlabStyleDeprecations
 
   private
 
+  # Mutate the arguments, returns the deprecation
   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-arguments-and-enum-values'
     end
 
-    deprecation = kwargs.delete(:deprecated)
-    return kwargs unless deprecation
+    deprecation = ::Gitlab::Graphql::Deprecation.parse(kwargs.delete(:deprecated))
+    return unless deprecation
 
-    milestone, reason = deprecation.values_at(:milestone, :reason).map(&:presence)
+    raise ArgumentError, "Bad deprecation. #{deprecation.errors.full_messages.to_sentence}" unless deprecation.valid?
 
-    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)
+    kwargs[:deprecation_reason] = deprecation.deprecation_reason
+    kwargs[:description] = deprecation.edit_description(kwargs[:description])
 
-    deprecated_in = "Deprecated in #{milestone}"
-    kwargs[:deprecation_reason] = "#{reason}. #{deprecated_in}."
-    kwargs[:description] += " #{deprecated_in}: #{reason}." if kwargs[:description]
-
-    kwargs
+    deprecation
   end
 end

lib/gitlab/graphql/deprecation.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    class Deprecation
+      REASONS = {
+        renamed: 'This was renamed.',
+        discouraged: 'Use of this is not recommended.'
+      }.freeze
+
+      include ActiveModel::Validations
+
+      validates :milestone, presence: true, format: { with: /\A\d+\.\d+\z/, message: 'must be milestone-ish' }
+      validates :reason, presence: true
+      validates :reason,
+                format: { with: /.*[^.]\z/, message: 'must not end with a period' },
+                if: :reason_is_string?
+      validate :milestone_is_string
+      validate :reason_known_or_string
+
+      def self.parse(options)
+        new(**options) if options
+      end
+
+      def initialize(reason: nil, milestone: nil, replacement: nil)
+        @reason = reason.presence
+        @milestone = milestone.presence
+        @replacement = replacement.presence
+      end
+
+      def ==(other)
+        return false unless other.is_a?(self.class)
+
+        [reason_text, milestone, replacement] == [:reason_text, :milestone, :replacement].map do |attr|
+          other.send(attr) # rubocop: disable GitlabSecurity/PublicSend
+        end
+      end
+      alias_method :eql, :==
+
+      def markdown(context: :inline)
+        parts = [
+          "#{deprecated_in(format: :markdown)}.",
+          reason_text,
+          replacement.then { |r| "Use: `#{r}`." if r }
+        ].compact
+
+        case context
+        when :block
+          ['WARNING:', *parts].join("\n")
+        when :inline
+          parts.join(' ')
+        end
+      end
+
+      def edit_description(original_description)
+        @original_description = original_description
+        return unless original_description
+
+        original_description + description_suffix
+      end
+
+      def original_description
+        return unless @original_description
+        return @original_description if @original_description.ends_with?('.')
+
+        "#{@original_description}."
+      end
+
+      def deprecation_reason
+        [
+          reason_text,
+          replacement && "Please use `#{replacement}`.",
+          "#{deprecated_in}."
+        ].compact.join(' ')
+      end
+
+      private
+
+      attr_reader :reason, :milestone, :replacement
+
+      def milestone_is_string
+        return if milestone.is_a?(String)
+
+        errors.add(:milestone, 'must be a string')
+      end
+
+      def reason_known_or_string
+        return if REASONS.key?(reason)
+        return if reason_is_string?
+
+        errors.add(:reason, 'must be a known reason or a string')
+      end
+
+      def reason_is_string?
+        reason.is_a?(String)
+      end
+
+      def reason_text
+        @reason_text ||= REASONS[reason] || "#{reason.to_s.strip}."
+      end
+
+      def description_suffix
+        " #{deprecated_in}: #{reason_text}"
+      end
+
+      def deprecated_in(format: :plain)
+        case format
+        when :plain
+          "Deprecated in #{milestone}"
+        when :markdown
+          "**Deprecated** in #{milestone}"
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/graphql/deprecation_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'active_model'
+
+RSpec.describe ::Gitlab::Graphql::Deprecation do
+  let(:options) { {} }
+
+  subject(:deprecation) { described_class.parse(options) }
+
+  describe '.parse' do
+    context 'with nil' do
+      let(:options) { nil }
+
+      it 'parses to nil' do
+        expect(deprecation).to be_nil
+      end
+    end
+
+    context 'with empty options' do
+      let(:options) { {} }
+
+      it 'parses to an empty deprecation' do
+        expect(deprecation).to eq(described_class.new)
+      end
+    end
+
+    context 'with defined options' do
+      let(:options) { { reason: :renamed, milestone: '10.10' } }
+
+      it 'assigns the properties' do
+        expect(deprecation).to eq(described_class.new(reason: 'This was renamed', milestone: '10.10'))
+      end
+    end
+  end
+
+  describe 'validations' do
+    let(:options) { { reason: :renamed, milestone: '10.10' } }
+
+    it { is_expected.to be_valid }
+
+    context 'when the milestone is absent' do
+      before do
+        options.delete(:milestone)
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when the milestone is not milestone-ish' do
+      before do
+        options[:milestone] = 'next year'
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when the milestone is not a string' do
+      before do
+        options[:milestone] = 10.01
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when the reason is absent' do
+      before do
+        options.delete(:reason)
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when the reason is not a known reason' do
+      before do
+        options[:reason] = :not_stylish_enough
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when the reason is a string' do
+      before do
+        options[:reason] = 'not stylish enough'
+      end
+
+      it { is_expected.to be_valid }
+    end
+
+    context 'when the reason is a string ending with a period' do
+      before do
+        options[:reason] = 'not stylish enough.'
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+  end
+
+  describe '#deprecation_reason' do
+    context 'when there is a replacement' do
+      let(:options) { { reason: :renamed, milestone: '10.10', replacement: 'X.y' } }
+
+      it 'renders as reason-replacement-milestone' do
+        expect(deprecation.deprecation_reason).to eq('This was renamed. Please use `X.y`. Deprecated in 10.10.')
+      end
+    end
+
+    context 'when there is no replacement' do
+      let(:options) { { reason: :renamed, milestone: '10.10' } }
+
+      it 'renders as reason-milestone' do
+        expect(deprecation.deprecation_reason).to eq('This was renamed. Deprecated in 10.10.')
+      end
+    end
+
+    describe 'processing of reason' do
+      described_class::REASONS.each_key do |known_reason|
+        context "when the reason is a known reason such as #{known_reason.inspect}" do
+          let(:options) { { reason: known_reason } }
+
+          it 'renders the reason_text correctly' do
+            expect(deprecation.deprecation_reason).to start_with(described_class::REASONS[known_reason])
+          end
+        end
+      end
+
+      context 'when the reason is any other string' do
+        let(:options) { { reason: 'unhelpful' } }
+
+        it 'appends a period' do
+          expect(deprecation.deprecation_reason).to start_with('unhelpful.')
+        end
+      end
+    end
+  end
+
+  describe '#edit_description' do
+    let(:options) { { reason: :renamed, milestone: '10.10' } }
+
+    it 'appends milestone:reason with a leading space if there is a description' do
+      desc = deprecation.edit_description('Some description.')
+
+      expect(desc).to eq('Some description. Deprecated in 10.10: This was renamed.')
+    end
+
+    it 'returns nil if there is no description' do
+      desc = deprecation.edit_description(nil)
+
+      expect(desc).to be_nil
+    end
+  end
+
+  describe '#original_description' do
+    it 'records the description passed to it' do
+      deprecation.edit_description('Some description.')
+
+      expect(deprecation.original_description).to eq('Some description.')
+    end
+  end
+
+  describe '#markdown' do
+    context 'when there is a replacement' do
+      let(:options) { { reason: :renamed, milestone: '10.10', replacement: 'X.y' } }
+
+      context 'when the context is :inline' do
+        it 'renders on one line' do
+          expectation = '**Deprecated** in 10.10. This was renamed. Use: `X.y`.'
+
+          expect(deprecation.markdown).to eq(expectation)
+          expect(deprecation.markdown(context: :inline)).to eq(expectation)
+        end
+      end
+
+      context 'when the context is :block' do
+        it 'renders a warning note' do
+          expectation = <<~MD.chomp
+            WARNING:
+            **Deprecated** in 10.10.
+            This was renamed.
+            Use: `X.y`.
+          MD
+
+          expect(deprecation.markdown(context: :block)).to eq(expectation)
+        end
+      end
+    end
+
+    context 'when there is no replacement' do
+      let(:options) { { reason: 'Removed', milestone: '10.10' } }
+
+      context 'when the context is :inline' do
+        it 'renders on one line' do
+          expectation = '**Deprecated** in 10.10. Removed.'
+
+          expect(deprecation.markdown).to eq(expectation)
+          expect(deprecation.markdown(context: :inline)).to eq(expectation)
+        end
+      end
+
+      context 'when the context is :block' do
+        it 'renders a warning note' do
+          expectation = <<~MD.chomp
+            WARNING:
+            **Deprecated** in 10.10.
+            Removed.
+          MD
+
+          expect(deprecation.markdown(context: :block)).to eq(expectation)
+        end
+      end
+    end
+  end
+end

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

@@ -13,18 +13,18 @@ RSpec.shared_examples 'Gitlab-style deprecations' do
     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`'
+        include("Reason can't be blank")
       )
       expect { subject(deprecated: { reason: 'Deprecation reason' }) }.to raise_error(
         ArgumentError,
-        'Please provide a `milestone` within `deprecated`'
+        include("Milestone can't be blank")
       )
     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`'
+        include("Milestone must be a string")
       )
     end
   end
@@ -49,4 +49,22 @@ RSpec.shared_examples 'Gitlab-style deprecations' do
 
     expect(deprecable.description).to be_nil
   end
+
+  it 'adds information about the replacement if provided' do
+    deprecable = subject(deprecated: { milestone: '1.10', reason: :renamed, replacement: 'Foo.bar' })
+
+    expect(deprecable.deprecation_reason).to include 'Please use `Foo.bar`'
+  end
+
+  it 'supports named reasons: renamed' do
+    deprecable = subject(deprecated: { milestone: '1.10', reason: :renamed })
+
+    expect(deprecable.deprecation_reason).to include 'This was renamed.'
+  end
+
+  it 'supports named reasons: discouraged' do
+    deprecable = subject(deprecated: { milestone: '1.10', reason: :discouraged })
+
+    expect(deprecable.deprecation_reason).to include 'Use of this is not recommended.'
+  end
 end