Merge branch 'fj-228825-remove-file-path-validation-in-snippet-create-action' into 'master'

Remove file_path validation in snippet create action

See merge request gitlab-org/gitlab!36809

app/models/snippet_input_action.rb

@@ -15,7 +15,7 @@ class SnippetInputAction
 
   validates :action, inclusion: { in: ACTIONS, message: "%{value} is not a valid action" }
   validates :previous_path, presence: true, if: :move_action?
-  validates :file_path, presence: true
+  validates :file_path, presence: true, unless: :create_action?
   validates :content, presence: true, if: -> (action) { action.create_action? || action.update_action? }
   validate :ensure_same_file_path_and_previous_path, if: :update_action?
   validate :ensure_different_file_path_and_previous_path, if: :move_action?

changelogs/unreleased/fj-228825-remove-file-path-validation-in-snippet-create-action.yml

+---
+title: Remove file_path validation in snippet create action
+merge_request: 36809
+author:
+type: changed

spec/models/snippet_input_action_spec.rb

@@ -29,8 +29,8 @@ RSpec.describe SnippetInputAction do
       :move    | 'foobar'  | ''       | 'foo1'   | nil                | true  | nil
       :create  | 'foobar'  | nil      | 'foobar' | nil                | false | :content
       :create  | 'foobar'  | ''       | 'foobar' | nil                | false | :content
-      :create  | nil       | 'foobar' | 'foobar' | nil                | false | :file_path
-      :create  | ''        | 'foobar' | 'foobar' | nil                | false | :file_path
+      :create  | nil       | 'foobar' | 'foobar' | nil                | true  | nil
+      :create  | ''        | 'foobar' | 'foobar' | nil                | true  | nil
       :update  | 'foobar'  | nil      | 'foobar' | nil                | false | :content
       :update  | 'foobar'  | ''       | 'foobar' | nil                | false | :content
       :update  | 'other'   | 'foobar' | 'foobar' | nil                | false | :file_path

spec/services/snippets/create_service_spec.rb

@@ -177,10 +177,8 @@ RSpec.describe Snippets::CreateService do
         end
 
         it 'returns a generic error' do
-          response = subject
-
-          expect(response).to be_error
-          expect(response.payload[:snippet].errors[:repository]).to eq ['Error creating the snippet']
+          expect(subject).to be_error
+          expect(snippet.errors[:repository]).to eq ['Error creating the snippet']
         end
       end
 
@@ -250,7 +248,7 @@ RSpec.describe Snippets::CreateService do
       end
 
       it 'commit the files to the repository' do
-        subject
+        expect(subject).to be_success
 
         blob = snippet.repository.blob_at('master', file_path)
 
@@ -261,10 +259,7 @@ RSpec.describe Snippets::CreateService do
         let(:extra_opts) { { content: 'foo', file_name: 'path' } }
 
         it 'a validation error is raised' do
-          response = subject
-          snippet = response.payload[:snippet]
-
-          expect(response).to be_error
+          expect(subject).to be_error
           expect(snippet.errors.full_messages_for(:content)).to eq ['Content and snippet files cannot be used together']
           expect(snippet.errors.full_messages_for(:file_name)).to eq ['File name and snippet files cannot be used together']
           expect(snippet.repository.exists?).to be_falsey
@@ -275,10 +270,7 @@ RSpec.describe Snippets::CreateService do
         let(:snippet_files) { [{ action: 'invalid_action', file_path: 'snippet_file_path.rb', content: 'snippet_content' }] }
 
         it 'a validation error is raised' do
-          response = subject
-          snippet = response.payload[:snippet]
-
-          expect(response).to be_error
+          expect(subject).to be_error
           expect(snippet.errors.full_messages_for(:snippet_files)).to eq ['Snippet files have invalid data']
           expect(snippet.repository.exists?).to be_falsey
         end
@@ -288,14 +280,21 @@ RSpec.describe Snippets::CreateService do
         let(:snippet_files) { [{ action: 'delete', file_path: 'snippet_file_path.rb' }] }
 
         it 'a validation error is raised' do
-          response = subject
-          snippet = response.payload[:snippet]
-
-          expect(response).to be_error
+          expect(subject).to be_error
           expect(snippet.errors.full_messages_for(:snippet_files)).to eq ['Snippet files have invalid data']
           expect(snippet.repository.exists?).to be_falsey
         end
       end
+
+      context 'when "create" operation does not have file_path or is empty' do
+        let(:snippet_files) { [{ action: 'create', content: content }, { action: 'create', content: content, file_path: '' }] }
+
+        it 'generates the file path for the files' do
+          expect(subject).to be_success
+          expect(snippet.repository.blob_at('master', 'snippetfile1.txt').data).to eq content
+          expect(snippet.repository.blob_at('master', 'snippetfile2.txt').data).to eq content
+        end
+      end
     end
 
     context 'when ProjectSnippet' do

spec/services/snippets/update_service_spec.rb

@@ -537,10 +537,18 @@ RSpec.describe Snippets::UpdateService do
             it_behaves_like 'returns an error', 'Snippet files have invalid data'
           end
 
-          context 'when file_path is not present' do
-            let(:snippet_files) { [{ action: :create, content: content }] }
+          context 'when file_path is not present or empty' do
+            let(:snippet_files) { [{ action: :create, content: content }, { action: :create, file_path: '', content: content }] }
 
-            it_behaves_like 'returns an error', 'Snippet files have invalid data'
+            it 'generates the file path for the files' do
+              expect(blob('snippetfile1.txt')).to be_nil
+              expect(blob('snippetfile2.txt')).to be_nil
+
+              expect(subject).to be_success
+
+              expect(blob('snippetfile1.txt').data).to eq content
+              expect(blob('snippetfile2.txt').data).to eq content
+            end
           end
 
           context 'when file_path already exists in the repository' do