Merge branch 'optinally-expose-description-html-in-release-api' into 'master'

Add an option to expose `description_html` in Release API [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!62802

config/feature_flags/development/remove_description_html_in_release_api.yml

----
-name: remove_description_html_in_release_api
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60380
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329188
-milestone: '13.12'
-type: development
-group: group::release
-default_enabled: true

config/feature_flags/development/remove_description_html_in_release_api_override.yml

----
-name: remove_description_html_in_release_api_override
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60380
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329188
-milestone: '13.12'
-type: development
-group: group::release
-default_enabled: false

doc/api/releases/index.md

@@ -10,7 +10,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 > - Using this API you can manipulate GitLab [Release](../../user/project/releases/index.md) entries.
 > - For manipulating links as a release asset, see [Release Links API](links.md).
 > - Release Evidences were [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/26019) in GitLab 12.5.
-> - `description_html` field was [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/299447) in GitLab 13.12.
+> - `description_html` became an opt-in field [with GitLab 13.12 for performance reasons](https://gitlab.com/gitlab-org/gitlab/-/issues/299447).
+    Please pass the `include_html_description` query string parameter if you need it.
 
 ## List Releases
 
@@ -25,6 +26,7 @@ GET /projects/:id/releases
 | `id`          | integer/string | yes      | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). |
 | `order_by`    | string         | no       | The field to use as order. Either `released_at` (default) or `created_at`. |
 | `sort`        | string         | no       | The direction of the order. Either `desc` (default) for descending order or `asc` for ascending order. |
+| `include_html_description` | boolean        | no       | If `true`, a response includes HTML rendered Markdown of the release description.   |
 
 Example request:
 
@@ -228,6 +230,7 @@ GET /projects/:id/releases/:tag_name
 | ------------- | -------------- | -------- | ----------------------------------------------------------------------------------- |
 | `id`          | integer/string | yes      | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). |
 | `tag_name`    | string         | yes      | The Git tag the release is associated with.                                         |
+| `include_html_description` | boolean        | no       | If `true`, a response includes HTML rendered Markdown of the release description.   |
 
 Example request:
 

lib/api/entities/release.rb

@@ -8,7 +8,7 @@ module API
       expose :name
       expose :tag, as: :tag_name, if: ->(_, _) { can_download_code? }
       expose :description
-      expose :description_html, unless: ->(_, _) { remove_description_html? } do |entity|
+      expose :description_html, if: -> (_, options) { options[:include_html_description] } do |entity|
         MarkupHelper.markdown_field(entity, :description, current_user: options[:current_user])
       end
       expose :created_at
@@ -45,11 +45,6 @@ module API
       def can_read_milestone?
         Ability.allowed?(options[:current_user], :read_milestone, object.project)
       end
-
-      def remove_description_html?
-        ::Feature.enabled?(:remove_description_html_in_release_api, object.project, default_enabled: :yaml) &&
-          ::Feature.disabled?(:remove_description_html_in_release_api_override, object.project)
-      end
     end
   end
 end

lib/api/releases.rb

@@ -29,6 +29,8 @@ module API
                             desc: 'Return releases ordered by `released_at` or `created_at`.'
         optional :sort, type: String, values: %w[asc desc], default: 'desc',
                         desc: 'Return releases sorted in `asc` or `desc` order.'
+        optional :include_html_description, type: Boolean,
+                               desc: 'If `true`, a response includes HTML rendered markdown of the release description.'
       end
       get ':id/releases' do
         releases = ::ReleasesFinder.new(user_project, current_user, declared_params.slice(:order_by, :sort)).execute
@@ -43,7 +45,8 @@ module API
                         # context is unnecessary here.
                         cache_context: -> (_) { "user:{#{current_user&.id}}" },
                         expires_in: 5.minutes,
-                        current_user: current_user
+                        current_user: current_user,
+                        include_html_description: params[:include_html_description]
       end
 
       desc 'Get a single project release' do
@@ -53,11 +56,13 @@ module API
       end
       params do
         requires :tag_name, type: String, desc: 'The name of the tag', as: :tag
+        optional :include_html_description, type: Boolean,
+                               desc: 'If `true`, a response includes HTML rendered markdown of the release description.'
       end
       get ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMENTS do
         authorize_download_code!
 
-        present release, with: Entities::Release, current_user: current_user
+        present release, with: Entities::Release, current_user: current_user, include_html_description: params[:include_html_description]
       end
 
       desc 'Create a new release' do

spec/lib/api/entities/release_spec.rb

@@ -8,7 +8,8 @@ RSpec.describe API::Entities::Release do
   let(:release) { create(:release, project: project) }
   let(:evidence) { release.evidences.first }
   let(:user) { create(:user) }
-  let(:entity) { described_class.new(release, current_user: user).as_json }
+  let(:entity) { described_class.new(release, current_user: user, include_html_description: include_html_description).as_json }
+  let(:include_html_description) { false }
 
   before do
     ::Releases::CreateEvidenceService.new(release).execute
@@ -58,10 +59,8 @@ RSpec.describe API::Entities::Release do
       expect(description_html).to be_nil
     end
 
-    context 'when remove_description_html_in_release_api feature flag is disabled' do
-      before do
-        stub_feature_flags(remove_description_html_in_release_api: false)
-      end
+    context 'when include_html_description option is true' do
+      let(:include_html_description) { true }
 
       it 'renders special references if current user has access' do
         project.add_reporter(user)
@@ -77,18 +76,5 @@ RSpec.describe API::Entities::Release do
         expect(description_html).not_to include(issue_title)
       end
     end
-
-    context 'when remove_description_html_in_release_api_override feature flag is enabled' do
-      before do
-        stub_feature_flags(remove_description_html_in_release_api_override: project)
-      end
-
-      it 'renders special references if current user has access' do
-        project.add_reporter(user)
-
-        expect(description_html).to include(issue_path)
-        expect(description_html).to include(issue_title)
-      end
-    end
   end
 end

spec/requests/api/releases_spec.rb

@@ -50,6 +50,12 @@ RSpec.describe API::Releases do
         expect(json_response.second['tag_name']).to eq(release_1.tag)
       end
 
+      it 'does not include description_html' do
+        get api("/projects/#{project.id}/releases", maintainer)
+
+        expect(json_response.map { |h| h['description_html'] }).to contain_exactly(nil, nil)
+      end
+
       RSpec.shared_examples 'release sorting' do |order_by|
         subject { get api(url, access_level), params: { sort: sort, order_by: order_by } }
 
@@ -107,6 +113,15 @@ RSpec.describe API::Releases do
         expect(json_response.second['commit_path']).to eq("/#{release_1.project.full_path}/-/commit/#{release_1.commit.id}")
         expect(json_response.second['tag_path']).to eq("/#{release_1.project.full_path}/-/tags/#{release_1.tag}")
       end
+
+      context 'when include_html_description option is true' do
+        it 'includes description_html field' do
+          get api("/projects/#{project.id}/releases", maintainer), params: { include_html_description: true }
+
+          expect(json_response.map { |h| h['description_html'] })
+            .to contain_exactly(instance_of(String), instance_of(String))
+        end
+      end
     end
 
     it 'returns an upcoming_release status for a future release' do
@@ -328,6 +343,12 @@ RSpec.describe API::Releases do
           .to match_array(release.sources.map(&:url))
       end
 
+      it 'does not include description_html' do
+        get api("/projects/#{project.id}/releases/v0.1", maintainer)
+
+        expect(json_response['description_html']).to eq(nil)
+      end
+
       context 'with evidence' do
         let!(:evidence) { create(:evidence, release: release) }
 
@@ -403,6 +424,14 @@ RSpec.describe API::Releases do
         end
       end
 
+      context 'when include_html_description option is true' do
+        it 'includes description_html field' do
+          get api("/projects/#{project.id}/releases/v0.1", maintainer), params: { include_html_description: true }
+
+          expect(json_response['description_html']).to be_instance_of(String)
+        end
+      end
+
       context 'when user is a guest' do
         it 'responds 403 Forbidden' do
           get api("/projects/#{project.id}/releases/v0.1", guest)

spec/spec_helper.rb

@@ -286,9 +286,6 @@ RSpec.configure do |config|
       # As we're ready to change `master` usages to `main`, let's enable it
       stub_feature_flags(main_branch_over_master: false)
 
-      # Selectively disable by actor https://docs.gitlab.com/ee/development/feature_flags/#selectively-disable-by-actor
-      stub_feature_flags(remove_description_html_in_release_api_override: false)
-
       # Disable issue respositioning to avoid heavy load on database when importing big projects.
       # This is only turned on when app is handling heavy project imports.
       # Can be removed when we find a better way to deal with the problem.