Merge branch '212223-snippet-creation-bug' into 'master'

Resolve "Bug when Snippet creation fails"

Closes #212223

See merge request gitlab-org/gitlab!27891

app/services/snippets/create_service.rb

@@ -9,72 +9,77 @@ module Snippets
     def execute
       filter_spam_check_params
 
-      snippet = if project
-                  project.snippets.build(params)
-                else
-                  PersonalSnippet.new(params)
-                end
+      @snippet = if project
+                   project.snippets.build(params)
+                 else
+                   PersonalSnippet.new(params)
+                 end
 
-      unless Gitlab::VisibilityLevel.allowed_for?(current_user, snippet.visibility_level)
-        deny_visibility_level(snippet)
+      unless Gitlab::VisibilityLevel.allowed_for?(current_user, @snippet.visibility_level)
+        deny_visibility_level(@snippet)
 
-        return snippet_error_response(snippet, 403)
+        return snippet_error_response(@snippet, 403)
       end
 
-      snippet.author = current_user
+      @snippet.author = current_user
 
-      spam_check(snippet, current_user)
+      spam_check(@snippet, current_user)
 
-      if save_and_commit(snippet)
-        UserAgentDetailService.new(snippet, @request).create
+      if save_and_commit
+        UserAgentDetailService.new(@snippet, @request).create
         Gitlab::UsageDataCounters::SnippetCounter.count(:create)
 
-        ServiceResponse.success(payload: { snippet: snippet } )
+        ServiceResponse.success(payload: { snippet: @snippet } )
       else
-        snippet_error_response(snippet, 400)
+        snippet_error_response(@snippet, 400)
       end
     end
 
     private
 
-    def save_and_commit(snippet)
-      snippet_saved = snippet.with_transaction_returning_status do
-        snippet.save && snippet.store_mentions!
+    def save_and_commit
+      snippet_saved = @snippet.with_transaction_returning_status do
+        @snippet.save && @snippet.store_mentions!
       end
 
       if snippet_saved && Feature.enabled?(:version_snippets, current_user)
-        create_repository_for(snippet)
-        create_commit(snippet)
+        create_repository
+        create_commit
       end
 
       snippet_saved
     rescue => e # Rescuing all because we can receive Creation exceptions, GRPC exceptions, Git exceptions, ...
-      snippet.errors.add(:base, e.message)
       log_error(e.message)
 
       # If the commit action failed we need to remove the repository if exists
-      snippet.repository.remove if snippet.repository_exists?
+      @snippet.repository.remove if @snippet.repository_exists?
 
       # If the snippet was created, we need to remove it as we
       # would do like if it had had any validation error
-      snippet.delete if snippet.persisted?
+      # and reassign a dupe so we don't return the deleted snippet
+      if @snippet.persisted?
+        @snippet.delete
+        @snippet = @snippet.dup
+      end
+
+      @snippet.errors.add(:base, e.message)
 
       false
     end
 
-    def create_repository_for(snippet)
-      snippet.create_repository
+    def create_repository
+      @snippet.create_repository
 
-      raise CreateRepositoryError, 'Repository could not be created' unless snippet.repository_exists?
+      raise CreateRepositoryError, 'Repository could not be created' unless @snippet.repository_exists?
     end
 
-    def create_commit(snippet)
+    def create_commit
       commit_attrs = {
         branch_name: 'master',
         message: 'Initial commit'
       }
 
-      snippet.snippet_repository.multi_files_action(current_user, snippet_files, commit_attrs)
+      @snippet.snippet_repository.multi_files_action(current_user, snippet_files, commit_attrs)
     end
 
     def snippet_files

changelogs/unreleased/212223-snippet-creation-bug.yml

+---
+title: Resolve Snippet creation failure bug
+merge_request: 27891
+author:
+type: fixed

spec/features/snippets/user_creates_snippet_spec.rb

@@ -99,6 +99,11 @@ shared_examples_for 'snippet editor' do
     it 'renders new page' do
       expect(page).to have_content('New Snippet')
     end
+
+    it 'has the correct action path' do
+      action = find('form.snippet-form')['action']
+      expect(action).to match(%r{/snippets\z})
+    end
   end
 
   it 'validation fails for the first time' do

spec/services/snippets/create_service_spec.rb

@@ -172,6 +172,10 @@ describe Snippets::CreateService do
         it 'returns the error' do
           expect(snippet.errors.full_messages).to include('Repository could not be created')
         end
+
+        it 'does not return a snippet with an id' do
+          expect(snippet.id).to be_nil
+        end
       end
 
       context 'when the commit action fails' do