Merge branch '217809-fj-remove-snippet-multiple-files-ff' into 'master'

Enabling snippets with multiple files

See merge request gitlab-org/gitlab!43246

app/assets/javascripts/snippets/components/snippet_blob_actions_edit.vue

@@ -2,7 +2,6 @@
 import { GlButton } from '@gitlab/ui';
 import { cloneDeep } from 'lodash';
 import { s__, sprintf } from '~/locale';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import SnippetBlobEdit from './snippet_blob_edit.vue';
 import { SNIPPET_MAX_BLOBS } from '../constants';
 import { createBlob, decorateBlob, diffAll } from '../utils/blob';
@@ -12,7 +11,6 @@ export default {
     SnippetBlobEdit,
     GlButton,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     initBlobs: {
       type: Array,
@@ -52,12 +50,6 @@ export default {
     canAdd() {
       return this.count < SNIPPET_MAX_BLOBS;
     },
-    hasMultiFilesEnabled() {
-      return this.glFeatures.snippetMultipleFiles;
-    },
-    filesLabel() {
-      return this.hasMultiFilesEnabled ? s__('Snippets|Files') : s__('Snippets|File');
-    },
     firstInputId() {
       const blobId = this.blobIds[0];
 
@@ -132,19 +124,17 @@ export default {
 </script>
 <template>
   <div class="form-group file-editor">
-    <label :for="firstInputId">{{ filesLabel }}</label>
+    <label :for="firstInputId">{{ s__('Snippets|Files') }}</label>
     <snippet-blob-edit
       v-for="(blobId, index) in blobIds"
       :key="blobId"
       :class="{ 'gl-mt-3': index > 0 }"
       :blob="blobs[blobId]"
       :can-delete="canDelete"
-      :show-delete="hasMultiFilesEnabled"
       @blob-updated="updateBlob(blobId, $event)"
       @delete="deleteBlob(blobId)"
     />
     <gl-button
-      v-if="hasMultiFilesEnabled"
       :disabled="!canAdd"
       data-testid="add_button"
       class="gl-my-3"

app/assets/javascripts/snippets/components/snippet_blob_edit.vue

@@ -28,7 +28,7 @@ export default {
     showDelete: {
       type: Boolean,
       required: false,
-      default: false,
+      default: true,
     },
   },
   computed: {

app/controllers/projects/snippets_controller.rb

@@ -14,10 +14,6 @@ class Projects::SnippetsController < Projects::Snippets::ApplicationController
   before_action :authorize_update_snippet!, only: [:edit, :update]
   before_action :authorize_admin_snippet!, only: [:destroy]
 
-  before_action do
-    push_frontend_feature_flag(:snippet_multiple_files, current_user)
-  end
-
   def index
     @snippet_counts = ::Snippets::CountService
       .new(current_user, project: @project)

app/controllers/snippets_controller.rb

@@ -17,10 +17,6 @@ class SnippetsController < Snippets::ApplicationController
 
   layout 'snippets'
 
-  before_action do
-    push_frontend_feature_flag(:snippet_multiple_files, current_user)
-  end
-
   def index
     if params[:username].present?
       @user = UserFinder.new(params[:username]).find_by_username!

app/models/snippet.rb

@@ -19,7 +19,6 @@ class Snippet < ApplicationRecord
   extend ::Gitlab::Utils::Override
 
   MAX_FILE_COUNT = 10
-  MAX_SINGLE_FILE_COUNT = 1
 
   cache_markdown_field :title, pipeline: :single_line
   cache_markdown_field :description
@@ -175,8 +174,8 @@ class Snippet < ApplicationRecord
     Snippet.find_by(id: id, project: project)
   end
 
-  def self.max_file_limit(user)
-    Feature.enabled?(:snippet_multiple_files, user) ? MAX_FILE_COUNT : MAX_SINGLE_FILE_COUNT
+  def self.max_file_limit
+    MAX_FILE_COUNT
   end
 
   def initialize(attributes = {})

app/presenters/snippet_blob_presenter.rb

@@ -25,10 +25,6 @@ class SnippetBlobPresenter < BlobPresenter
 
   private
 
-  def snippet_multiple_files?
-    blob.container.repository_exists? && Feature.enabled?(:snippet_multiple_files, current_user)
-  end
-
   def snippet
     blob.container
   end
@@ -52,8 +48,6 @@ class SnippetBlobPresenter < BlobPresenter
   end
 
   def snippet_blob_raw_route(only_path: false)
-    return gitlab_raw_snippet_blob_url(snippet, blob.path, only_path: only_path) if snippet_multiple_files?
-
-    gitlab_raw_snippet_url(snippet, only_path: only_path)
+    gitlab_raw_snippet_blob_url(snippet, blob.path, only_path: only_path)
   end
 end

app/services/snippets/repository_validation_service.rb

@@ -52,7 +52,7 @@ module Snippets
 
     def check_file_count!
       file_count = repository.ls_files(snippet.default_branch).size
-      limit = Snippet.max_file_limit(current_user)
+      limit = Snippet.max_file_limit
 
       if file_count > limit
         raise RepositoryValidationError, _('Repository files count over the limit')

changelogs/unreleased/217809-fj-remove-snippet-multiple-files-ff.yml

+---
+title: Enable snippet multiple files
+merge_request: 43246
+author:
+type: added

config/feature_flags/development/snippet_multiple_files.yml

----
-name: snippet_multiple_files
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/32416
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/217809
-group: group::editor
-type: development
-default_enabled: false

lib/api/entities/snippet.rb

@@ -17,7 +17,7 @@ module API
       expose :file_name do |snippet|
         snippet.file_name_on_repo || snippet.file_name
       end
-      expose :files, if: ->(snippet, options) { snippet_multiple_files?(snippet, options[:current_user]) } do |snippet, options|
+      expose :files do |snippet, options|
         snippet.list_files.map do |file|
           {
             path: file,
@@ -25,10 +25,6 @@ module API
           }
         end
       end
-
-      def snippet_multiple_files?(snippet, current_user)
-        ::Feature.enabled?(:snippet_multiple_files, current_user) && snippet.repository_exists?
-      end
     end
   end
 end

lib/api/helpers/snippets_helpers.rb

@@ -93,7 +93,7 @@ module API
     def validate_params_for_multiple_files(snippet)
       return unless params[:content] || params[:file_name]
 
-      if Feature.enabled?(:snippet_multiple_files, current_user) && snippet.multiple_files?
+      if snippet.multiple_files?
         render_api_error!({ error: _('To update Snippets with multiple files, you must use the `files` parameter') }, 400)
       end
     end

lib/gitlab/git_access_snippet.rb

@@ -115,7 +115,7 @@ module Gitlab
     override :check_single_change_access
     def check_single_change_access(change, _skip_lfs_integrity_check: false)
       Checks::SnippetCheck.new(change, default_branch: snippet.default_branch, logger: logger).validate!
-      Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit(user), logger: logger).validate!
+      Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit, logger: logger).validate!
     rescue Checks::TimedLogger::TimeoutError
       raise TimeoutError, logger.full_message
     end

qa/qa/specs/features/browser_ui/3_create/snippet/create_personal_snippet_with_multiple_files_spec.rb

 # frozen_string_literal: true
 
 module QA
-  RSpec.describe 'Create', :requires_admin do
+  RSpec.describe 'Create' do
     describe 'Multiple file snippet' do
-      before do
-        Runtime::Feature.enable('snippet_multiple_files')
-      end
-
-      after do
-        Runtime::Feature.disable('snippet_multiple_files')
-      end
-
       it 'creates a personal snippet with multiple files', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/842' do
         Flow::Login.sign_in
 

spec/frontend/snippets/components/__snapshots__/snippet_blob_edit_spec.js.snap

@@ -9,6 +9,7 @@ exports[`Snippet Blob Edit component with loaded blob matches snapshot 1`] = `
     candelete="true"
     data-qa-selector="file_name_field"
     id="blob_local_7_file_path"
+    showdelete="true"
     value="foo/bar/test.md"
   />
    

spec/frontend/snippets/components/snippet_blob_actions_edit_spec.js

@@ -19,17 +19,12 @@ const TEST_BLOBS_UNLOADED = TEST_BLOBS.map(blob => ({ ...blob, content: '', isLo
 describe('snippets/components/snippet_blob_actions_edit', () => {
   let wrapper;
 
-  const createComponent = (props = {}, snippetMultipleFiles = true) => {
+  const createComponent = (props = {}) => {
     wrapper = shallowMount(SnippetBlobActionsEdit, {
       propsData: {
         initBlobs: TEST_BLOBS,
         ...props,
       },
-      provide: {
-        glFeatures: {
-          snippetMultipleFiles,
-        },
-      },
     });
   };
 
@@ -69,28 +64,24 @@ describe('snippets/components/snippet_blob_actions_edit', () => {
     wrapper = null;
   });
 
-  describe.each`
-    featureFlag | label      | showDelete | showAdd
-    ${true}     | ${'Files'} | ${true}    | ${true}
-    ${false}    | ${'File'}  | ${false}   | ${false}
-  `('with feature flag = $featureFlag', ({ featureFlag, label, showDelete, showAdd }) => {
+  describe('multi-file snippets rendering', () => {
     beforeEach(() => {
-      createComponent({}, featureFlag);
+      createComponent();
     });
 
     it('renders label', () => {
-      expect(findLabel().text()).toBe(label);
+      expect(findLabel().text()).toBe('Files');
     });
 
-    it(`renders delete button (show=${showDelete})`, () => {
+    it(`renders delete button (show=true)`, () => {
       expect(findFirstBlobEdit().props()).toMatchObject({
-        showDelete,
+        showDelete: true,
         canDelete: true,
       });
     });
 
-    it(`renders add button (show=${showAdd})`, () => {
-      expect(findAddButton().exists()).toBe(showAdd);
+    it(`renders add button (show=true)`, () => {
+      expect(findAddButton().exists()).toBe(true);
     });
   });
 

spec/frontend/snippets/components/snippet_blob_edit_spec.js

@@ -156,7 +156,7 @@ describe('Snippet Blob Edit component', () => {
     });
 
     it('shows blob header', () => {
-      const { canDelete = true, showDelete = false } = props;
+      const { canDelete = true, showDelete = true } = props;
 
       expect(findHeader().props()).toMatchObject({
         canDelete,

spec/lib/api/entities/snippet_spec.rb

@@ -21,16 +21,6 @@ RSpec.describe ::API::Entities::Snippet do
     it { expect(subject[:visibility]).to eq snippet.visibility }
     it { expect(subject).to include(:author) }
 
-    context 'with snippet_multiple_files feature disabled' do
-      before do
-        stub_feature_flags(snippet_multiple_files: false)
-      end
-
-      it 'does not return files' do
-        expect(subject).not_to include(:files)
-      end
-    end
-
     describe 'file_name' do
       it 'returns attribute from repository' do
         expect(subject[:file_name]).to eq snippet.blobs.first.path
@@ -77,14 +67,6 @@ RSpec.describe ::API::Entities::Snippet do
       let(:blob)   { snippet.blobs.first }
       let(:ref)    { blob.repository.root_ref }
 
-      context 'when repository does not exist' do
-        it 'does not include the files attribute' do
-          allow(snippet).to receive(:repository_exists?).and_return(false)
-
-          expect(subject).not_to include(:files)
-        end
-      end
-
       shared_examples 'snippet files' do
         let(:file) { subject[:files].first }
 
@@ -99,6 +81,14 @@ RSpec.describe ::API::Entities::Snippet do
         it 'has the raw url' do
           expect(file[:raw_url]).to match(raw_url)
         end
+
+        context 'when repository does not exist' do
+          it 'returns empty array' do
+            allow(snippet.repository).to receive(:empty?).and_return(true)
+
+            expect(subject[:files]).to be_empty
+          end
+        end
       end
 
       context 'with PersonalSnippet' do

spec/lib/gitlab/git_access_snippet_spec.rb

@@ -260,7 +260,7 @@ RSpec.describe Gitlab::GitAccessSnippet do
         service = double
 
         expect(service).to receive(:validate!).and_return(nil)
-        expect(Snippet).to receive(:max_file_limit).with(user).and_return(5)
+        expect(Snippet).to receive(:max_file_limit).and_return(5)
         expect(Gitlab::Checks::PushFileCountCheck).to receive(:new).with(anything, hash_including(limit: 5)).and_return(service)
 
         push_access_check

spec/models/snippet_spec.rb

@@ -717,19 +717,11 @@ RSpec.describe Snippet do
   end
 
   describe '.max_file_limit' do
-    subject { described_class.max_file_limit(nil) }
+    subject { described_class.max_file_limit }
 
     it "returns #{Snippet::MAX_FILE_COUNT}" do
       expect(subject).to eq Snippet::MAX_FILE_COUNT
     end
-
-    context 'when feature flag :snippet_multiple_files is disabled' do
-      it "returns #{described_class::MAX_SINGLE_FILE_COUNT}" do
-        stub_feature_flags(snippet_multiple_files: false)
-
-        expect(subject).to eq described_class::MAX_SINGLE_FILE_COUNT
-      end
-    end
   end
 
   describe '#list_files' do

spec/presenters/snippet_blob_presenter_spec.rb

@@ -133,28 +133,6 @@ RSpec.describe SnippetBlobPresenter do
       subject { described_class.new(snippet.blobs.first, current_user: user).raw_path }
 
       it_behaves_like 'snippet blob raw path'
-
-      context 'with snippet_multiple_files feature disabled' do
-        before do
-          stub_feature_flags(snippet_multiple_files: false)
-        end
-
-        context 'with ProjectSnippet' do
-          let(:snippet) { project_snippet }
-
-          it 'returns the raw path' do
-            expect(subject).to eq "/#{snippet.project.full_path}/-/snippets/#{snippet.id}/raw"
-          end
-        end
-
-        context 'with PersonalSnippet' do
-          let(:snippet) { personal_snippet }
-
-          it 'returns the raw path' do
-            expect(subject).to eq "/-/snippets/#{snippet.id}/raw"
-          end
-        end
-      end
     end
 
     describe '#raw_url' do
@@ -165,28 +143,6 @@ RSpec.describe SnippetBlobPresenter do
       end
 
       it_behaves_like 'snippet blob raw url'
-
-      context 'with snippet_multiple_files feature disabled' do
-        before do
-          stub_feature_flags(snippet_multiple_files: false)
-        end
-
-        context 'with ProjectSnippet' do
-          let(:snippet) { project_snippet }
-
-          it 'returns the raw project snippet url' do
-            expect(subject).to eq("http://test.host/#{project_snippet.project.full_path}/-/snippets/#{project_snippet.id}/raw")
-          end
-        end
-
-        context 'with PersonalSnippet' do
-          let(:snippet) { personal_snippet }
-
-          it 'returns the raw personal snippet url' do
-            expect(subject).to eq("http://test.host/-/snippets/#{personal_snippet.id}/raw")
-          end
-        end
-      end
     end
   end
 

spec/requests/api/project_snippets_spec.rb

@@ -116,10 +116,6 @@ RSpec.describe API::ProjectSnippets do
         let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123", user) }
       end
     end
-
-    it_behaves_like 'snippet_multiple_files feature disabled' do
-      subject { get api("/projects/#{project.id}/snippets/#{snippet.id}", user) }
-    end
   end
 
   describe 'POST /projects/:project_id/snippets/' do

spec/requests/api/snippets_spec.rb

@@ -167,8 +167,6 @@ RSpec.describe API::Snippets do
     it_behaves_like 'snippet access with different users' do
       let(:path) { "/snippets/#{snippet.id}" }
     end
-
-    it_behaves_like 'snippet_multiple_files feature disabled'
   end
 
   describe 'POST /snippets/' do
@@ -251,10 +249,6 @@ RSpec.describe API::Snippets do
 
     it_behaves_like 'snippet creation'
 
-    it_behaves_like 'snippet_multiple_files feature disabled' do
-      let(:snippet) { Snippet.find(json_response["id"]) }
-    end
-
     context 'with an external user' do
       let(:user) { create(:user, :external) }
 

spec/services/snippets/repository_validation_service_spec.rb

@@ -40,7 +40,7 @@ RSpec.describe Snippets::RepositoryValidationService do
     end
 
     it 'returns error when the repository has more file than the limit' do
-      limit = Snippet.max_file_limit(user) + 1
+      limit = Snippet.max_file_limit + 1
       files = Array.new(limit) { FFaker::Filesystem.file_name }
       allow(repository).to receive(:ls_files).and_return(files)
 

spec/support/shared_examples/requests/snippet_shared_examples.rb

@@ -99,18 +99,6 @@ RSpec.shared_examples 'snippet blob content' do
   end
 end
 
-RSpec.shared_examples 'snippet_multiple_files feature disabled' do
-  before do
-    stub_feature_flags(snippet_multiple_files: false)
-
-    subject
-  end
-
-  it 'does not return files attributes' do
-    expect(json_response).not_to have_key('files')
-  end
-end
-
 RSpec.shared_examples 'snippet creation with files parameter' do
   using RSpec::Parameterized::TableSyntax