Commit 26edc5c2 authored by Francisco Javier López's avatar Francisco Javier López Committed by James Lopez

Add spam flag to snippet mutations

This commit introduces a concern to return spam
attributes returned in mutations. It has been applied
first to Snippets.
parent daf1d01a
......@@ -81,7 +81,7 @@ class GraphqlController < ApplicationController
end
def context
@context ||= { current_user: current_user, is_sessionless_user: !!sessionless_user? }
@context ||= { current_user: current_user, is_sessionless_user: !!sessionless_user?, request: request }
end
def build_variables(variable_info)
......
# frozen_string_literal: true
module Mutations
module SpammableMutationFields
extend ActiveSupport::Concern
included do
field :spam,
GraphQL::BOOLEAN_TYPE,
null: true,
description: 'Indicates whether the operation returns a record detected as spam'
end
def with_spam_params(&block)
request = Feature.enabled?(:snippet_spam) ? context[:request] : nil
yield.merge({ api: true, request: request })
end
def with_spam_fields(spammable, &block)
{ spam: spammable.spam? }.merge!(yield)
end
end
end
......@@ -3,6 +3,7 @@
module Mutations
module Snippets
class Create < BaseMutation
include SpammableMutationFields
include ResolvesProject
graphql_name 'CreateSnippet'
......@@ -56,10 +57,12 @@ module Mutations
::Gitlab::UsageDataCounters::EditorUniqueCounter.track_snippet_editor_edit_action(author: current_user)
end
{
snippet: service_response.success? ? snippet : nil,
errors: errors_on_object(snippet)
}
with_spam_fields(snippet) do
{
snippet: service_response.success? ? snippet : nil,
errors: errors_on_object(snippet)
}
end
end
private
......@@ -81,14 +84,16 @@ module Mutations
end
def create_params(args)
args.tap do |create_args|
# We need to rename `blob_actions` into `snippet_actions` because
# it's the expected key param
create_args[:snippet_actions] = create_args.delete(:blob_actions)&.map(&:to_h)
# We need to rename `uploaded_files` into `files` because
# it's the expected key param
create_args[:files] = create_args.delete(:uploaded_files)
with_spam_params do
args.tap do |create_args|
# We need to rename `blob_actions` into `snippet_actions` because
# it's the expected key param
create_args[:snippet_actions] = create_args.delete(:blob_actions)&.map(&:to_h)
# We need to rename `uploaded_files` into `files` because
# it's the expected key param
create_args[:files] = create_args.delete(:uploaded_files)
end
end
end
end
......
......@@ -3,6 +3,8 @@
module Mutations
module Snippets
class Update < Base
include SpammableMutationFields
graphql_name 'UpdateSnippet'
argument :id,
......@@ -39,10 +41,12 @@ module Mutations
::Gitlab::UsageDataCounters::EditorUniqueCounter.track_snippet_editor_edit_action(author: current_user)
end
{
snippet: result.success? ? snippet : snippet.reset,
errors: errors_on_object(snippet)
}
with_spam_fields(snippet) do
{
snippet: result.success? ? snippet : snippet.reset,
errors: errors_on_object(snippet)
}
end
end
private
......@@ -52,10 +56,12 @@ module Mutations
end
def update_params(args)
args.tap do |update_args|
# We need to rename `blob_actions` into `snippet_actions` because
# it's the expected key param
update_args[:snippet_actions] = update_args.delete(:blob_actions)&.map(&:to_h)
with_spam_params do
args.tap do |update_args|
# We need to rename `blob_actions` into `snippet_actions` because
# it's the expected key param
update_args[:snippet_actions] = update_args.delete(:blob_actions)&.map(&:to_h)
end
end
end
end
......
---
title: Add spam flag to snippet create/update mutations
merge_request: 44010
author:
type: changed
---
name: snippet_spam
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44010
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/262013
type: development
group: group::editor
default_enabled: false
......@@ -3504,6 +3504,11 @@ type CreateSnippetPayload {
The snippet after mutation
"""
snippet: Snippet
"""
Indicates whether the operation returns a record detected as spam
"""
spam: Boolean
}
"""
......@@ -19256,6 +19261,11 @@ type UpdateSnippetPayload {
The snippet after mutation
"""
snippet: Snippet
"""
Indicates whether the operation returns a record detected as spam
"""
spam: Boolean
}
scalar Upload
......
......@@ -9438,6 +9438,20 @@
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "spam",
"description": "Indicates whether the operation returns a record detected as spam",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "Boolean",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
......@@ -56073,6 +56087,20 @@
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "spam",
"description": "Indicates whether the operation returns a record detected as spam",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "Boolean",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
......@@ -561,6 +561,7 @@ Autogenerated return type of CreateSnippet.
| `clientMutationId` | String | A unique identifier for the client performing the mutation. |
| `errors` | String! => Array | Errors encountered during execution of the mutation. |
| `snippet` | Snippet | The snippet after mutation |
| `spam` | Boolean | Indicates whether the operation returns a record detected as spam |
### CreateTestCasePayload
......@@ -2732,6 +2733,7 @@ Autogenerated return type of UpdateSnippet.
| `clientMutationId` | String | A unique identifier for the client performing the mutation. |
| `errors` | String! => Array | Errors encountered during execution of the mutation. |
| `snippet` | Snippet | The snippet after mutation |
| `spam` | Boolean | Indicates whether the operation returns a record detected as spam |
### User
......
......@@ -98,6 +98,12 @@ RSpec.describe GraphqlController do
expect(assigns(:context)[:is_sessionless_user]).to be false
end
end
it 'includes request object in context' do
post :execute
expect(assigns(:context)[:request]).to eq request
end
end
describe 'Admin Mode' do
......
......@@ -76,6 +76,8 @@ RSpec.describe 'Creating a Snippet' do
expect(mutation_response['snippet']).to be_nil
end
it_behaves_like 'spam flag is present'
end
shared_examples 'creates snippet' do
......@@ -103,6 +105,10 @@ RSpec.describe 'Creating a Snippet' do
end
it_behaves_like 'snippet edit usage data counters'
it_behaves_like 'spam flag is present'
it_behaves_like 'can raise spam flag' do
let(:service) { Snippets::CreateService }
end
end
context 'with PersonalSnippet' do
......@@ -142,6 +148,9 @@ RSpec.describe 'Creating a Snippet' do
it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"]
it_behaves_like 'does not create snippet'
it_behaves_like 'can raise spam flag' do
let(:service) { Snippets::CreateService }
end
end
context 'when there non ActiveRecord errors' do
......
......@@ -37,6 +37,8 @@ RSpec.describe 'Updating a Snippet' do
graphql_mutation_response(:update_snippet)
end
subject { post_graphql_mutation(mutation, current_user: current_user) }
shared_examples 'graphql update actions' do
context 'when the user does not have permission' do
let(:current_user) { create(:user) }
......@@ -46,14 +48,14 @@ RSpec.describe 'Updating a Snippet' do
it 'does not update the Snippet' do
expect do
post_graphql_mutation(mutation, current_user: current_user)
subject
end.not_to change { snippet.reload }
end
end
context 'when the user has permission' do
it 'updates the snippet record' do
post_graphql_mutation(mutation, current_user: current_user)
subject
expect(snippet.reload.title).to eq(updated_title)
end
......@@ -65,7 +67,7 @@ RSpec.describe 'Updating a Snippet' do
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)
subject
blob_to_update = blob_at(updated_file)
blob_to_delete = blob_at(deleted_file)
......@@ -79,13 +81,19 @@ RSpec.describe 'Updating a Snippet' do
end
end
it_behaves_like 'can raise spam flag' do
let(:service) { Snippets::UpdateService }
end
it_behaves_like 'spam flag is present'
context 'when there are ActiveRecord validation errors' do
let(:updated_title) { '' }
it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"]
it 'does not update the Snippet' do
post_graphql_mutation(mutation, current_user: current_user)
subject
expect(snippet.reload.title).to eq(original_title)
end
......@@ -94,7 +102,7 @@ RSpec.describe 'Updating a Snippet' do
blob_to_update = blob_at(updated_file)
blob_to_delete = blob_at(deleted_file)
post_graphql_mutation(mutation, current_user: current_user)
subject
aggregate_failures do
expect(blob_at(updated_file).data).to eq blob_to_update.data
......@@ -104,6 +112,11 @@ RSpec.describe 'Updating a Snippet' do
expect(mutation_response['snippet']['visibilityLevel']).to eq('private')
end
end
it_behaves_like 'spam flag is present'
it_behaves_like 'can raise spam flag' do
let(:service) { Snippets::UpdateService }
end
end
def blob_at(filename)
......@@ -144,7 +157,7 @@ RSpec.describe 'Updating a Snippet' do
context 'when the author is not a member of the project' do
it 'returns an an error' do
post_graphql_mutation(mutation, current_user: current_user)
subject
errors = json_response['errors']
expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR)
......@@ -162,7 +175,7 @@ RSpec.describe 'Updating a Snippet' do
it 'returns an an error' do
project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::DISABLED)
post_graphql_mutation(mutation, current_user: current_user)
subject
errors = json_response['errors']
expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR)
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.shared_examples 'spam flag is present' do
specify :aggregate_failures do
subject
expect(mutation_response).to have_key('spam')
expect(mutation_response['spam']).to be_falsey
end
end
RSpec.shared_examples 'can raise spam flag' do
it 'spam parameters are passed to the service' do
expect(service).to receive(:new).with(anything, anything, hash_including(api: true, request: instance_of(ActionDispatch::Request)))
subject
end
context 'when the snippet is detected as spam' do
it 'raises spam flag' do
allow_next_instance_of(service) do |instance|
allow(instance).to receive(:spam_check) do |snippet, user, _|
snippet.spam!
end
end
subject
expect(mutation_response['spam']).to be true
expect(mutation_response['errors']).to include("Your snippet has been recognized as spam and has been discarded.")
end
end
context 'when :snippet_spam flag is disabled' do
before do
stub_feature_flags(snippet_spam: false)
end
it 'request parameter is not passed to the service' do
expect(service).to receive(:new).with(anything, anything, hash_not_including(request: instance_of(ActionDispatch::Request)))
subject
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment