Merge branch '217802-fj-remove-file_name-and-content-snippet-mutations' into 'master'

Remove file_name and content from snippet mutations

See merge request gitlab-org/gitlab!40727

app/graphql/mutations/snippets/create.rb

@@ -16,14 +16,6 @@ module Mutations
                required: true,
                description: 'Title of the snippet'
 
-      argument :file_name, GraphQL::STRING_TYPE,
-               required: false,
-               description: 'File name of the snippet'
-
-      argument :content, GraphQL::STRING_TYPE,
-               required: false,
-               description: 'Content of the snippet'
-
       argument :description, GraphQL::STRING_TYPE,
                required: false,
                description: 'Description of the snippet'

app/graphql/mutations/snippets/update.rb

@@ -14,14 +14,6 @@ module Mutations
                required: false,
                description: 'Title of the snippet'
 
-      argument :file_name, GraphQL::STRING_TYPE,
-               required: false,
-               description: 'File name of the snippet'
-
-      argument :content, GraphQL::STRING_TYPE,
-               required: false,
-               description: 'Content of the snippet'
-
       argument :description, GraphQL::STRING_TYPE,
                required: false,
                description: 'Description of the snippet'

changelogs/unreleased/217802-fj-remove-file_name-and-content-snippet-mutations.yml

+---
+title: Remove file_name and content in snippet mutations
+merge_request: 40727
+author:
+type: changed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -2746,21 +2746,11 @@ input CreateSnippetInput {
   """
   clientMutationId: String
 
-  """
-  Content of the snippet
-  """
-  content: String
-
   """
   Description of the snippet
   """
   description: String
 
-  """
-  File name of the snippet
-  """
-  fileName: String
-
   """
   The project full path the snippet is associated with
   """
@@ -16627,21 +16617,11 @@ input UpdateSnippetInput {
   """
   clientMutationId: String
 
-  """
-  Content of the snippet
-  """
-  content: String
-
   """
   Description of the snippet
   """
   description: String
 
-  """
-  File name of the snippet
-  """
-  fileName: String
-
   """
   The global id of the snippet to update
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -7390,26 +7390,6 @@
               },
               "defaultValue": null
             },
-            {
-              "name": "fileName",
-              "description": "File name of the snippet",
-              "type": {
-                "kind": "SCALAR",
-                "name": "String",
-                "ofType": null
-              },
-              "defaultValue": null
-            },
-            {
-              "name": "content",
-              "description": "Content of the snippet",
-              "type": {
-                "kind": "SCALAR",
-                "name": "String",
-                "ofType": null
-              },
-              "defaultValue": null
-            },
             {
               "name": "description",
               "description": "Description of the snippet",
@@ -48948,26 +48928,6 @@
               },
               "defaultValue": null
             },
-            {
-              "name": "fileName",
-              "description": "File name of the snippet",
-              "type": {
-                "kind": "SCALAR",
-                "name": "String",
-                "ofType": null
-              },
-              "defaultValue": null
-            },
-            {
-              "name": "content",
-              "description": "Content of the snippet",
-              "type": {
-                "kind": "SCALAR",
-                "name": "String",
-                "ofType": null
-              },
-              "defaultValue": null
-            },
             {
               "name": "description",
               "description": "Description of the snippet",

spec/requests/api/graphql/mutations/snippets/create_spec.rb

@@ -7,22 +7,24 @@ RSpec.describe 'Creating a Snippet' do
 
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project) }
-  let(:content) { 'Initial content' }
+
   let(:description) { 'Initial description' }
   let(:title) { 'Initial title' }
-  let(:file_name) { 'Initial file_name' }
   let(:visibility_level) { 'public' }
+  let(:action) { :create }
+  let(:file_1) { { filePath: 'example_file1', content: 'This is the example file 1' }}
+  let(:file_2) { { filePath: 'example_file2', content: 'This is the example file 2' }}
+  let(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] }
   let(:project_path) { nil }
   let(:uploaded_files) { nil }
   let(:mutation_vars) do
     {
-      content: content,
       description: description,
       visibility_level: visibility_level,
-      file_name: file_name,
       title: title,
       project_path: project_path,
-      uploaded_files: uploaded_files
+      uploaded_files: uploaded_files,
+      blob_actions: actions
     }
   end
 
@@ -62,26 +64,47 @@ RSpec.describe 'Creating a Snippet' do
   context 'when the user has permission' do
     let(:current_user) { user }
 
-    context 'with PersonalSnippet' do
-      it 'creates the Snippet' do
+    shared_examples 'does not create snippet' do
+      it 'does not create the Snippet' do
         expect do
           subject
-        end.to change { Snippet.count }.by(1)
+        end.not_to change { Snippet.count }
       end
 
+      it 'does not return Snippet' do
+        subject
+
+        expect(mutation_response['snippet']).to be_nil
+      end
+    end
+
+    shared_examples 'creates snippet' do
       it 'returns the created Snippet' do
+        expect do
           subject
+        end.to change { Snippet.count }.by(1)
 
-        expect(mutation_response['snippet']['blob']['richData']).to be_nil
-        expect(mutation_response['snippet']['blob']['plainData']).to match(content)
         expect(mutation_response['snippet']['title']).to eq(title)
         expect(mutation_response['snippet']['description']).to eq(description)
-        expect(mutation_response['snippet']['fileName']).to eq(file_name)
         expect(mutation_response['snippet']['visibilityLevel']).to eq(visibility_level)
-        expect(mutation_response['snippet']['project']).to be_nil
+        expect(mutation_response['snippet']['blobs'][0]['plainData']).to match(file_1[:content])
+        expect(mutation_response['snippet']['blobs'][0]['fileName']).to match(file_1[:file_path])
+        expect(mutation_response['snippet']['blobs'][1]['plainData']).to match(file_2[:content])
+        expect(mutation_response['snippet']['blobs'][1]['fileName']).to match(file_2[:file_path])
+      end
+
+      context 'when action is invalid' do
+        let(:file_1) { { filePath: 'example_file1' }}
+
+        it_behaves_like 'a mutation that returns errors in the response', errors: ['Snippet actions have invalid data']
+        it_behaves_like 'does not create snippet'
       end
     end
 
+    context 'with PersonalSnippet' do
+      it_behaves_like 'creates snippet'
+    end
+
     context 'with ProjectSnippet' do
       let(:project_path) { project.full_path }
 
@@ -89,23 +112,7 @@ RSpec.describe 'Creating a Snippet' do
         project.add_developer(current_user)
       end
 
-      it 'creates the Snippet' do
-        expect do
-          subject
-        end.to change { Snippet.count }.by(1)
-      end
-
-      it 'returns the created Snippet' do
-        subject
-
-        expect(mutation_response['snippet']['blob']['richData']).to be_nil
-        expect(mutation_response['snippet']['blob']['plainData']).to match(content)
-        expect(mutation_response['snippet']['title']).to eq(title)
-        expect(mutation_response['snippet']['description']).to eq(description)
-        expect(mutation_response['snippet']['fileName']).to eq(file_name)
-        expect(mutation_response['snippet']['visibilityLevel']).to eq(visibility_level)
-        expect(mutation_response['snippet']['project']['fullPath']).to eq(project_path)
-      end
+      it_behaves_like 'creates snippet'
 
       context 'when the project path is invalid' do
         let(:project_path) { 'foobar' }
@@ -124,61 +131,6 @@ RSpec.describe 'Creating a Snippet' do
       end
     end
 
-    shared_examples 'does not create snippet' do
-      it 'does not create the Snippet' do
-        expect do
-          subject
-        end.not_to change { Snippet.count }
-      end
-
-      it 'does not return Snippet' do
-        subject
-
-        expect(mutation_response['snippet']).to be_nil
-      end
-    end
-
-    context 'when snippet is created using the files param' do
-      let(:action) { :create }
-      let(:file_1) { { filePath: 'example_file1', content: 'This is the example file 1' }}
-      let(:file_2) { { filePath: 'example_file2', content: 'This is the example file 2' }}
-      let(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] }
-      let(:mutation_vars) do
-        {
-          description: description,
-          visibility_level: visibility_level,
-          project_path: project_path,
-          title: title,
-          blob_actions: actions
-        }
-      end
-
-      it 'creates the Snippet' do
-        expect do
-          subject
-        end.to change { Snippet.count }.by(1)
-      end
-
-      it 'returns the created Snippet' do
-        subject
-
-        expect(mutation_response['snippet']['title']).to eq(title)
-        expect(mutation_response['snippet']['description']).to eq(description)
-        expect(mutation_response['snippet']['visibilityLevel']).to eq(visibility_level)
-        expect(mutation_response['snippet']['blobs'][0]['plainData']).to match(file_1[:content])
-        expect(mutation_response['snippet']['blobs'][0]['fileName']).to match(file_1[:file_path])
-        expect(mutation_response['snippet']['blobs'][1]['plainData']).to match(file_2[:content])
-        expect(mutation_response['snippet']['blobs'][1]['fileName']).to match(file_2[:file_path])
-      end
-
-      context 'when action is invalid' do
-        let(:file_1) { { filePath: 'example_file1' }}
-
-        it_behaves_like 'a mutation that returns errors in the response', errors: ['Snippet actions have invalid data']
-        it_behaves_like 'does not create snippet'
-      end
-    end
-
     context 'when there are ActiveRecord validation errors' do
       let(:title) { '' }
 
@@ -187,7 +139,7 @@ RSpec.describe 'Creating a Snippet' do
     end
 
     context 'when there non ActiveRecord errors' do
-      let(:file_name) { 'invalid://file/path' }
+      let(:file_1) { { filePath: 'invalid://file/path', content: 'foobar' }}
 
       it_behaves_like 'a mutation that returns errors in the response', errors: ['Repository Error creating the snippet - Invalid file name']
       it_behaves_like 'does not create snippet'

spec/requests/api/graphql/mutations/snippets/update_spec.rb

@@ -12,18 +12,20 @@ RSpec.describe 'Updating a Snippet' do
   let(:updated_content) { 'Updated content' }
   let(:updated_description) { 'Updated description' }
   let(:updated_title) { 'Updated_title' }
-  let(:updated_file_name) { 'Updated file_name' }
   let(:current_user) { snippet.author }
-
+  let(:updated_file) { 'CHANGELOG' }
+  let(:deleted_file) { 'README' }
   let(:snippet_gid) { GitlabSchema.id_from_object(snippet).to_s }
   let(:mutation_vars) do
     {
       id: snippet_gid,
-      content: updated_content,
       description: updated_description,
       visibility_level: 'public',
-      file_name: updated_file_name,
-      title: updated_title
+      title: updated_title,
+      blob_actions: [
+        { action: :update, filePath: updated_file, content: updated_content },
+        { action: :delete, filePath: deleted_file }
+      ]
     }
   end
 
@@ -50,22 +52,33 @@ RSpec.describe 'Updating a Snippet' do
     end
 
     context 'when the user has permission' do
-      it 'updates the Snippet' do
+      it 'updates the snippet record' do
         post_graphql_mutation(mutation, current_user: current_user)
 
         expect(snippet.reload.title).to eq(updated_title)
       end
 
-      it 'returns the updated Snippet' do
+      it 'updates the Snippet' do
+        blob_to_update = blob_at(updated_file)
+        blob_to_delete = blob_at(deleted_file)
+
+        expect(blob_to_update.data).not_to eq updated_content
+        expect(blob_to_delete).to be_present
+
         post_graphql_mutation(mutation, current_user: current_user)
 
-        expect(mutation_response['snippet']['blob']['richData']).to be_nil
-        expect(mutation_response['snippet']['blob']['plainData']).to match(updated_content)
+        blob_to_update = blob_at(updated_file)
+        blob_to_delete = blob_at(deleted_file)
+
+        aggregate_failures do
+          expect(blob_to_update.data).to eq updated_content
+          expect(blob_to_delete).to be_nil
+          expect(blob_in_mutation_response(updated_file)['plainData']).to match(updated_content)
           expect(mutation_response['snippet']['title']).to eq(updated_title)
           expect(mutation_response['snippet']['description']).to eq(updated_description)
-        expect(mutation_response['snippet']['fileName']).to eq(updated_file_name)
           expect(mutation_response['snippet']['visibilityLevel']).to eq('public')
         end
+      end
 
       context 'when there are ActiveRecord validation errors' do
         let(:updated_title) { '' }
@@ -79,23 +92,37 @@ RSpec.describe 'Updating a Snippet' do
         end
 
         it 'returns the Snippet with its original values' do
+          blob_to_update = blob_at(updated_file)
+          blob_to_delete = blob_at(deleted_file)
+
           post_graphql_mutation(mutation, current_user: current_user)
 
-          expect(mutation_response['snippet']['blob']['richData']).to be_nil
-          expect(mutation_response['snippet']['blob']['plainData']).to match(original_content)
+          aggregate_failures do
+            expect(blob_at(updated_file).data).to eq blob_to_update.data
+            expect(blob_at(deleted_file).data).to eq blob_to_delete.data
+            expect(blob_in_mutation_response(deleted_file)['plainData']).not_to be_nil
             expect(mutation_response['snippet']['title']).to eq(original_title)
             expect(mutation_response['snippet']['description']).to eq(original_description)
-          expect(mutation_response['snippet']['fileName']).to eq(original_file_name)
             expect(mutation_response['snippet']['visibilityLevel']).to eq('private')
           end
         end
       end
+
+      def blob_in_mutation_response(filename)
+        mutation_response['snippet']['blobs'].select { |blob| blob['name'] == filename }[0]
+      end
+
+      def blob_at(filename)
+        snippet.repository.blob_at('HEAD', filename)
+      end
+    end
   end
 
   describe 'PersonalSnippet' do
     let(:snippet) do
       create(:personal_snippet,
              :private,
+             :repository,
              file_name: original_file_name,
              title: original_title,
              content: original_content,
@@ -111,6 +138,7 @@ RSpec.describe 'Updating a Snippet' do
     let(:snippet) do
       create(:project_snippet,
              :private,
+             :repository,
              project: project,
              author: create(:user),
              file_name: original_file_name,
@@ -149,40 +177,4 @@ RSpec.describe 'Updating a Snippet' do
 
     it_behaves_like 'when the snippet is not found'
   end
-
-  context 'when using the files params' do
-    let!(:snippet) { create(:personal_snippet, :private, :repository) }
-    let(:updated_content) { 'updated_content' }
-    let(:updated_file) { 'CHANGELOG' }
-    let(:deleted_file) { 'README' }
-    let(:mutation_vars) do
-      {
-        id: snippet_gid,
-        blob_actions: [
-          { action: :update, filePath: updated_file, content: updated_content },
-          { action: :delete, filePath: deleted_file }
-        ]
-      }
-    end
-
-    it 'updates the Snippet' do
-      blob_to_update = blob_at(updated_file)
-      expect(blob_to_update.data).not_to eq updated_content
-
-      blob_to_delete = blob_at(deleted_file)
-      expect(blob_to_delete).to be_present
-
-      post_graphql_mutation(mutation, current_user: current_user)
-
-      blob_to_update = blob_at(updated_file)
-      expect(blob_to_update.data).to eq updated_content
-
-      blob_to_delete = blob_at(deleted_file)
-      expect(blob_to_delete).to be_nil
-    end
-
-    def blob_at(filename)
-      snippet.repository.blob_at('HEAD', filename)
-    end
-  end
 end