From e0a8496a094971dae746dae511b6a41d0cdc92c7 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 1 Jun 2020 00:08:25 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../mutations/discussions/toggle_resolve.rb | 73 ++++ app/graphql/types/mutation_type.rb | 1 + app/graphql/types/notes/discussion_type.rb | 2 + app/graphql/types/notes/note_type.rb | 12 +- app/graphql/types/resolvable_interface.rb | 28 ++ app/models/discussion.rb | 1 + .../13049-graphql-resolve-discussion.yml | 5 + doc/administration/gitaly/index.md | 4 +- doc/administration/gitaly/praefect.md | 9 - .../graphql/reference/gitlab_schema.graphql | 102 +++++- doc/api/graphql/reference/gitlab_schema.json | 329 +++++++++++++++++- doc/api/graphql/reference/index.md | 21 +- spec/factories/notes.rb | 15 +- .../discussions/toggle_resolve_spec.rb | 155 +++++++++ .../types/notes/diff_position_type_spec.rb | 17 +- .../types/notes/discussion_type_spec.rb | 16 +- spec/graphql/types/notes/note_type_spec.rb | 22 +- .../graphql/types/notes/noteable_type_spec.rb | 10 +- .../types/resolvable_interface_spec.rb | 16 + .../discussions/toggle_resolve_spec.rb | 49 +++ 20 files changed, 835 insertions(+), 52 deletions(-) create mode 100644 app/graphql/mutations/discussions/toggle_resolve.rb create mode 100644 app/graphql/types/resolvable_interface.rb create mode 100644 changelogs/unreleased/13049-graphql-resolve-discussion.yml create mode 100644 spec/graphql/mutations/discussions/toggle_resolve_spec.rb create mode 100644 spec/graphql/types/resolvable_interface_spec.rb create mode 100644 spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb diff --git a/app/graphql/mutations/discussions/toggle_resolve.rb b/app/graphql/mutations/discussions/toggle_resolve.rb new file mode 100644 index 00000000000..41fd22c6b55 --- /dev/null +++ b/app/graphql/mutations/discussions/toggle_resolve.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module Mutations + module Discussions + class ToggleResolve < BaseMutation + graphql_name 'DiscussionToggleResolve' + + description 'Toggles the resolved state of a discussion' + + argument :id, + GraphQL::ID_TYPE, + required: true, + description: 'The global id of the discussion' + + argument :resolve, + GraphQL::BOOLEAN_TYPE, + required: true, + description: 'Will resolve the discussion when true, and unresolve the discussion when false' + + field :discussion, + Types::Notes::DiscussionType, + null: true, + description: 'The discussion after mutation' + + def resolve(id:, resolve:) + discussion = authorized_find_discussion!(id: id) + errors = [] + + begin + if resolve + resolve!(discussion) + else + unresolve!(discussion) + end + rescue ActiveRecord::RecordNotSaved + errors << "Discussion failed to be #{'un' unless resolve}resolved" + end + + { + discussion: discussion, + errors: errors + } + end + + private + + # `Discussion` permissions are checked through `Discussion#can_resolve?`, + # so we use this method of checking permissions rather than by defining + # an `authorize` permission and calling `authorized_find!`. + def authorized_find_discussion!(id:) + find_object(id: id).tap do |discussion| + raise_resource_not_available_error! unless discussion&.can_resolve?(current_user) + end + end + + def find_object(id:) + GitlabSchema.object_from_id(id, expected_type: ::Discussion) + end + + def resolve!(discussion) + ::Discussions::ResolveService.new( + discussion.project, + current_user, + one_or_more_discussions: discussion + ).execute + end + + def unresolve!(discussion) + discussion.unresolve! + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index a1edfe626d8..590ed7e960a 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -14,6 +14,7 @@ module Types mount_mutation Mutations::AwardEmojis::Toggle mount_mutation Mutations::Branches::Create, calls_gitaly: true mount_mutation Mutations::Commits::Create, calls_gitaly: true + mount_mutation Mutations::Discussions::ToggleResolve mount_mutation Mutations::Issues::SetConfidential mount_mutation Mutations::Issues::SetDueDate mount_mutation Mutations::Issues::Update diff --git a/app/graphql/types/notes/discussion_type.rb b/app/graphql/types/notes/discussion_type.rb index 74a233e9d26..a51d253097d 100644 --- a/app/graphql/types/notes/discussion_type.rb +++ b/app/graphql/types/notes/discussion_type.rb @@ -7,6 +7,8 @@ module Types authorize :read_note + implements(Types::ResolvableInterface) + field :id, GraphQL::ID_TYPE, null: false, description: "ID of this discussion" field :reply_id, GraphQL::ID_TYPE, null: false, diff --git a/app/graphql/types/notes/note_type.rb b/app/graphql/types/notes/note_type.rb index d48cc868434..8755b4ccad5 100644 --- a/app/graphql/types/notes/note_type.rb +++ b/app/graphql/types/notes/note_type.rb @@ -9,6 +9,8 @@ module Types expose_permissions Types::PermissionTypes::Note + implements(Types::ResolvableInterface) + field :id, GraphQL::ID_TYPE, null: false, description: 'ID of the note' @@ -22,11 +24,6 @@ module Types description: 'User who wrote this note', resolve: -> (note, args, context) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, note.author_id).find } - field :resolved_by, Types::UserType, - null: true, - description: 'User that resolved the discussion', - resolve: -> (note, _args, _context) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, note.resolved_by_id).find } - field :system, GraphQL::BOOLEAN_TYPE, null: false, description: 'Indicates whether this note was created by the system or by a user' @@ -44,11 +41,6 @@ module Types description: "Timestamp of the note's last activity" field :discussion, Types::Notes::DiscussionType, null: true, description: 'The discussion this note is a part of' - field :resolvable, GraphQL::BOOLEAN_TYPE, null: false, - description: 'Indicates if this note can be resolved. That is, if it is a resolvable discussion or simply a standalone note', - method: :resolvable? - field :resolved_at, Types::TimeType, null: true, - description: "Timestamp of the note's resolution" field :position, Types::Notes::DiffPositionType, null: true, description: 'The position of this note on a diff' field :confidential, GraphQL::BOOLEAN_TYPE, null: true, diff --git a/app/graphql/types/resolvable_interface.rb b/app/graphql/types/resolvable_interface.rb new file mode 100644 index 00000000000..a39092c70ca --- /dev/null +++ b/app/graphql/types/resolvable_interface.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Types + # This Interface contains fields that are shared between objects that include either + # the `ResolvableNote` or `ResolvableDiscussion` modules. + module ResolvableInterface + include Types::BaseInterface + + field :resolved_by, Types::UserType, + null: true, + description: 'User who resolved the object' + + def resolved_by + return unless object.resolved_by_id + + Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.resolved_by_id).find + end + + field :resolved, GraphQL::BOOLEAN_TYPE, null: false, + description: 'Indicates if the object is resolved', + method: :resolved? + field :resolvable, GraphQL::BOOLEAN_TYPE, null: false, + description: 'Indicates if the object can be resolved', + method: :resolvable? + field :resolved_at, Types::TimeType, null: true, + description: 'Timestamp of when the object was resolved' + end +end diff --git a/app/models/discussion.rb b/app/models/discussion.rb index c07078c03dd..e928bb0959a 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -20,6 +20,7 @@ class Discussion :noteable_ability_name, :to_ability_name, :editable?, + :resolved_by_id, :system_note_with_references_visible_for?, :resource_parent, diff --git a/changelogs/unreleased/13049-graphql-resolve-discussion.yml b/changelogs/unreleased/13049-graphql-resolve-discussion.yml new file mode 100644 index 00000000000..850d5b5aac4 --- /dev/null +++ b/changelogs/unreleased/13049-graphql-resolve-discussion.yml @@ -0,0 +1,5 @@ +--- +title: Add a GraphQL mutation for toggling the resolved state of a Discussion +merge_request: 32934 +author: +type: added diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index ed71e8fe440..808fa315636 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -1116,5 +1116,5 @@ server to keep them synchronized if possible. ### Praefect -Praefect is an experimental daemon that allows for replication of the Git data. -It can be setup with omnibus, [as explained here](./praefect.md). +Praefect is a router and transaction manager for Gitaly, and a required +component for running a Gitaly Cluster. For more information see [Gitaly Cluster](praefect.md). diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md index e5044d8874f..3dbb56bde32 100644 --- a/doc/administration/gitaly/praefect.md +++ b/doc/administration/gitaly/praefect.md @@ -319,15 +319,6 @@ application server, or a Gitaly node. } ``` -1. Enable the database replication queue: - - ```ruby - praefect['postgres_queue_enabled'] = true - ``` - - In the next release, database replication queue will be enabled by default. - See [issue #2615](https://gitlab.com/gitlab-org/gitaly/-/issues/2615). - 1. Enable automatic failover by editing `/etc/gitlab/gitlab.rb`: ```ruby diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 3fd8160aa82..b847e927846 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -2757,7 +2757,7 @@ type DiffRefs { startSha: String! } -type Discussion { +type Discussion implements ResolvableInterface { """ Timestamp of the discussion's creation """ @@ -2797,6 +2797,26 @@ type Discussion { ID used to reply to this discussion """ replyId: ID! + + """ + Indicates if the object can be resolved + """ + resolvable: Boolean! + + """ + Indicates if the object is resolved + """ + resolved: Boolean! + + """ + Timestamp of when the object was resolved + """ + resolvedAt: Time + + """ + User who resolved the object + """ + resolvedBy: User } """ @@ -2834,6 +2854,46 @@ type DiscussionEdge { node: Discussion } +""" +Autogenerated input type of DiscussionToggleResolve +""" +input DiscussionToggleResolveInput { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + + """ + The global id of the discussion + """ + id: ID! + + """ + Will resolve the discussion when true, and unresolve the discussion when false + """ + resolve: Boolean! +} + +""" +Autogenerated return type of DiscussionToggleResolve +""" +type DiscussionToggleResolvePayload { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + + """ + The discussion after mutation + """ + discussion: Discussion + + """ + Errors encountered during execution of the mutation. + """ + errors: [String!]! +} + """ Autogenerated input type of DismissVulnerability """ @@ -7114,6 +7174,11 @@ type Mutation { designManagementUpload(input: DesignManagementUploadInput!): DesignManagementUploadPayload destroyNote(input: DestroyNoteInput!): DestroyNotePayload destroySnippet(input: DestroySnippetInput!): DestroySnippetPayload + + """ + Toggles the resolved state of a discussion + """ + discussionToggleResolve(input: DiscussionToggleResolveInput!): DiscussionToggleResolvePayload dismissVulnerability(input: DismissVulnerabilityInput!): DismissVulnerabilityPayload epicAddIssue(input: EpicAddIssueInput!): EpicAddIssuePayload epicSetSubscription(input: EpicSetSubscriptionInput!): EpicSetSubscriptionPayload @@ -7305,7 +7370,7 @@ type NamespaceEdge { node: Namespace } -type Note { +type Note implements ResolvableInterface { """ User who wrote this note """ @@ -7352,17 +7417,22 @@ type Note { project: Project """ - Indicates if this note can be resolved. That is, if it is a resolvable discussion or simply a standalone note + Indicates if the object can be resolved """ resolvable: Boolean! """ - Timestamp of the note's resolution + Indicates if the object is resolved + """ + resolved: Boolean! + + """ + Timestamp of when the object was resolved """ resolvedAt: Time """ - User that resolved the discussion + User who resolved the object """ resolvedBy: User @@ -9946,6 +10016,28 @@ type RequirementStatesCount { opened: Int } +interface ResolvableInterface { + """ + Indicates if the object can be resolved + """ + resolvable: Boolean! + + """ + Indicates if the object is resolved + """ + resolved: Boolean! + + """ + Timestamp of when the object was resolved + """ + resolvedAt: Time + + """ + User who resolved the object + """ + resolvedBy: User +} + type RootStorageStatistics { """ The CI artifacts size in bytes diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 0aa6df23262..b980075b560 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -7739,11 +7739,79 @@ }, "isDeprecated": false, "deprecationReason": null + }, + { + "name": "resolvable", + "description": "Indicates if the object can be resolved", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "resolved", + "description": "Indicates if the object is resolved", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "resolvedAt", + "description": "Timestamp of when the object was resolved", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "resolvedBy", + "description": "User who resolved the object", + "args": [ + + ], + "type": { + "kind": "OBJECT", + "name": "User", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null } ], "inputFields": null, "interfaces": [ - + { + "kind": "INTERFACE", + "name": "ResolvableInterface", + "ofType": null + } ], "enumValues": null, "possibleTypes": null @@ -7860,6 +7928,122 @@ "enumValues": null, "possibleTypes": null }, + { + "kind": "INPUT_OBJECT", + "name": "DiscussionToggleResolveInput", + "description": "Autogenerated input type of DiscussionToggleResolve", + "fields": null, + "inputFields": [ + { + "name": "id", + "description": "The global id of the discussion", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "resolve", + "description": "Will resolve the discussion when true, and unresolve the discussion when false", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + } + ], + "interfaces": null, + "enumValues": null, + "possibleTypes": null + }, + { + "kind": "OBJECT", + "name": "DiscussionToggleResolvePayload", + "description": "Autogenerated return type of DiscussionToggleResolve", + "fields": [ + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "discussion", + "description": "The discussion after mutation", + "args": [ + + ], + "type": { + "kind": "OBJECT", + "name": "Discussion", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "errors", + "description": "Errors encountered during execution of the mutation.", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + } + } + }, + "isDeprecated": false, + "deprecationReason": null + } + ], + "inputFields": null, + "interfaces": [ + + ], + "enumValues": null, + "possibleTypes": null + }, { "kind": "INPUT_OBJECT", "name": "DismissVulnerabilityInput", @@ -20355,6 +20539,33 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "discussionToggleResolve", + "description": "Toggles the resolved state of a discussion", + "args": [ + { + "name": "input", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "INPUT_OBJECT", + "name": "DiscussionToggleResolveInput", + "ofType": null + } + }, + "defaultValue": null + } + ], + "type": { + "kind": "OBJECT", + "name": "DiscussionToggleResolvePayload", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "dismissVulnerability", "description": null, @@ -21754,7 +21965,25 @@ }, { "name": "resolvable", - "description": "Indicates if this note can be resolved. That is, if it is a resolvable discussion or simply a standalone note", + "description": "Indicates if the object can be resolved", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "resolved", + "description": "Indicates if the object is resolved", "args": [ ], @@ -21772,7 +22001,7 @@ }, { "name": "resolvedAt", - "description": "Timestamp of the note's resolution", + "description": "Timestamp of when the object was resolved", "args": [ ], @@ -21786,7 +22015,7 @@ }, { "name": "resolvedBy", - "description": "User that resolved the discussion", + "description": "User who resolved the object", "args": [ ], @@ -21855,7 +22084,11 @@ ], "inputFields": null, "interfaces": [ - + { + "kind": "INTERFACE", + "name": "ResolvableInterface", + "ofType": null + } ], "enumValues": null, "possibleTypes": null @@ -29085,6 +29318,92 @@ "enumValues": null, "possibleTypes": null }, + { + "kind": "INTERFACE", + "name": "ResolvableInterface", + "description": null, + "fields": [ + { + "name": "resolvable", + "description": "Indicates if the object can be resolved", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "resolved", + "description": "Indicates if the object is resolved", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "resolvedAt", + "description": "Timestamp of when the object was resolved", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "resolvedBy", + "description": "User who resolved the object", + "args": [ + + ], + "type": { + "kind": "OBJECT", + "name": "User", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + } + ], + "inputFields": null, + "interfaces": null, + "enumValues": null, + "possibleTypes": [ + { + "kind": "OBJECT", + "name": "Discussion", + "ofType": null + }, + { + "kind": "OBJECT", + "name": "Note", + "ofType": null + } + ] + }, { "kind": "OBJECT", "name": "RootStorageStatistics", diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 1407868d252..2ed8dfd0dd3 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -465,6 +465,20 @@ Autogenerated return type of DestroySnippet | `createdAt` | Time! | Timestamp of the discussion's creation | | `id` | ID! | ID of this discussion | | `replyId` | ID! | ID used to reply to this discussion | +| `resolvable` | Boolean! | Indicates if the object can be resolved | +| `resolved` | Boolean! | Indicates if the object is resolved | +| `resolvedAt` | Time | Timestamp of when the object was resolved | +| `resolvedBy` | User | User who resolved the object | + +## DiscussionToggleResolvePayload + +Autogenerated return type of DiscussionToggleResolve + +| Name | Type | Description | +| --- | ---- | ---------- | +| `clientMutationId` | String | A unique identifier for the client performing the mutation. | +| `discussion` | Discussion | The discussion after mutation | +| `errors` | String! => Array | Errors encountered during execution of the mutation. | ## DismissVulnerabilityPayload @@ -1095,9 +1109,10 @@ Represents a milestone. | `id` | ID! | ID of the note | | `position` | DiffPosition | The position of this note on a diff | | `project` | Project | Project associated with the note | -| `resolvable` | Boolean! | Indicates if this note can be resolved. That is, if it is a resolvable discussion or simply a standalone note | -| `resolvedAt` | Time | Timestamp of the note's resolution | -| `resolvedBy` | User | User that resolved the discussion | +| `resolvable` | Boolean! | Indicates if the object can be resolved | +| `resolved` | Boolean! | Indicates if the object is resolved | +| `resolvedAt` | Time | Timestamp of when the object was resolved | +| `resolvedBy` | User | User who resolved the object | | `system` | Boolean! | Indicates whether this note was created by the system or by a user | | `updatedAt` | Time! | Timestamp of the note's last activity | | `userPermissions` | NotePermissions! | Permissions for the current user on the resource | diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index a7d4b4eb57a..3db4723a1e9 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -23,11 +23,6 @@ FactoryBot.define do factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: 'DiscussionNote' do association :project, :repository - - trait :resolved do - resolved_at { Time.now } - resolved_by { create(:user) } - end end factory :track_mr_picking_note, traits: [:on_merge_request, :system] do @@ -76,11 +71,6 @@ FactoryBot.define do end end - trait :resolved do - resolved_at { Time.now } - resolved_by { create(:user) } - end - factory :image_diff_note_on_merge_request do position do build(:image_diff_position, @@ -155,6 +145,11 @@ FactoryBot.define do end end + trait :resolved do + resolved_at { Time.now } + resolved_by { association(:user) } + end + trait :system do system { true } end diff --git a/spec/graphql/mutations/discussions/toggle_resolve_spec.rb b/spec/graphql/mutations/discussions/toggle_resolve_spec.rb new file mode 100644 index 00000000000..73ed94abab9 --- /dev/null +++ b/spec/graphql/mutations/discussions/toggle_resolve_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Mutations::Discussions::ToggleResolve do + subject(:mutation) do + described_class.new(object: nil, context: { current_user: user }, field: nil) + end + + let_it_be(:project) { create(:project, :repository) } + + describe '#resolve' do + subject do + mutation.resolve({ id: id_arg, resolve: resolve_arg }) + end + + let(:id_arg) { discussion.to_global_id.to_s } + let(:resolve_arg) { true } + let(:mutated_discussion) { subject[:discussion] } + let(:errors) { subject[:errors] } + + shared_examples 'a working resolve method' do + context 'when the user does not have permission' do + let_it_be(:user) { create(:user) } + + it 'raises an error if the resource is not accessible to the user' do + expect { subject }.to raise_error( + Gitlab::Graphql::Errors::ResourceNotAvailable, + "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + ) + end + end + + context 'when the user has permission' do + let_it_be(:user) { create(:user, developer_projects: [project]) } + + context 'when discussion cannot be found' do + let(:id_arg) { "#{discussion.to_global_id}foo" } + + it 'raises an error' do + expect { subject }.to raise_error( + Gitlab::Graphql::Errors::ResourceNotAvailable, + "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + ) + end + end + + context 'when discussion is not a Discussion' do + let(:discussion) { create(:note, noteable: noteable, project: project) } + + it 'raises an error' do + expect { subject }.to raise_error( + Gitlab::Graphql::Errors::ArgumentError, + "#{discussion.to_global_id} is not a valid id for Discussion." + ) + end + end + + shared_examples 'returns a resolved discussion without errors' do + it 'returns a resolved discussion' do + expect(mutated_discussion).to be_resolved + end + + it 'returns empty errors' do + expect(errors).to be_empty + end + end + + shared_examples 'returns an unresolved discussion without errors' do + it 'returns an unresolved discussion' do + expect(mutated_discussion).not_to be_resolved + end + + it 'returns empty errors' do + expect(errors).to be_empty + end + end + + context 'when the `resolve` argument is true' do + include_examples 'returns a resolved discussion without errors' + + context 'when the discussion is already resolved' do + before do + discussion.resolve!(user) + end + + include_examples 'returns a resolved discussion without errors' + end + + context 'when the service raises an `ActiveRecord::RecordNotSaved` error' do + before do + allow_next_instance_of(::Discussions::ResolveService) do |service| + allow(service).to receive(:execute).and_raise(ActiveRecord::RecordNotSaved) + end + end + + it 'does not resolve the discussion' do + expect(mutated_discussion).not_to be_resolved + end + + it 'returns errors' do + expect(errors).to contain_exactly('Discussion failed to be resolved') + end + end + end + + context 'when the `resolve` argument is false' do + let(:resolve_arg) { false } + + context 'when the discussion is resolved' do + before do + discussion.resolve!(user) + end + + include_examples 'returns an unresolved discussion without errors' + + context 'when the service raises an `ActiveRecord::RecordNotSaved` error' do + before do + allow_next_instance_of(discussion.class) do |instance| + allow(instance).to receive(:unresolve!).and_raise(ActiveRecord::RecordNotSaved) + end + end + + it 'does not unresolve the discussion' do + expect(mutated_discussion).to be_resolved + end + + it 'returns errors' do + expect(errors).to contain_exactly('Discussion failed to be unresolved') + end + end + end + + context 'when the discussion is already unresolved' do + include_examples 'returns an unresolved discussion without errors' + end + end + end + end + + context 'when discussion is on a merge request' do + let_it_be(:noteable) { create(:merge_request, source_project: project) } + let(:discussion) { create(:diff_note_on_merge_request, noteable: noteable, project: project).to_discussion } + + it_behaves_like 'a working resolve method' + end + + context 'when discussion is on a design' do + let_it_be(:noteable) { create(:design, :with_file, issue: create(:issue, project: project)) } + let(:discussion) { create(:diff_note_on_design, noteable: noteable, project: project).to_discussion } + + it_behaves_like 'a working resolve method' + end + end +end diff --git a/spec/graphql/types/notes/diff_position_type_spec.rb b/spec/graphql/types/notes/diff_position_type_spec.rb index 01f355cb278..87f3810d55c 100644 --- a/spec/graphql/types/notes/diff_position_type_spec.rb +++ b/spec/graphql/types/notes/diff_position_type_spec.rb @@ -1,11 +1,22 @@ # frozen_string_literal: true + require 'spec_helper' describe GitlabSchema.types['DiffPosition'] do it 'exposes the expected fields' do - expected_fields = [:diff_refs, :file_path, :old_path, - :new_path, :position_type, :old_line, :new_line, :x, :y, - :width, :height] + expected_fields = %i[ + diff_refs + file_path + height + new_line + new_path + old_line + old_path + position_type + width + x + y + ] expect(described_class).to have_graphql_fields(*expected_fields) end diff --git a/spec/graphql/types/notes/discussion_type_spec.rb b/spec/graphql/types/notes/discussion_type_spec.rb index 44774594d17..177000b01b2 100644 --- a/spec/graphql/types/notes/discussion_type_spec.rb +++ b/spec/graphql/types/notes/discussion_type_spec.rb @@ -1,8 +1,22 @@ # frozen_string_literal: true + require 'spec_helper' describe GitlabSchema.types['Discussion'] do - specify { expect(described_class).to have_graphql_fields(:id, :created_at, :notes, :reply_id) } + it 'exposes the expected fields' do + expected_fields = %i[ + created_at + id + notes + reply_id + resolvable + resolved + resolved_at + resolved_by + ] + + expect(described_class).to have_graphql_fields(*expected_fields) + end specify { expect(described_class).to require_graphql_authorizations(:read_note) } end diff --git a/spec/graphql/types/notes/note_type_spec.rb b/spec/graphql/types/notes/note_type_spec.rb index 019f742ee77..d6cd0800234 100644 --- a/spec/graphql/types/notes/note_type_spec.rb +++ b/spec/graphql/types/notes/note_type_spec.rb @@ -1,11 +1,27 @@ # frozen_string_literal: true + require 'spec_helper' describe GitlabSchema.types['Note'] do it 'exposes the expected fields' do - expected_fields = [:id, :project, :author, :body, :created_at, - :updated_at, :discussion, :resolvable, :position, :user_permissions, - :resolved_by, :resolved_at, :system, :body_html, :confidential] + expected_fields = %i[ + author + body + body_html + confidential + created_at + discussion + id + position + project + resolvable + resolved + resolved_at + resolved_by + system + updated_at + user_permissions + ] expect(described_class).to have_graphql_fields(*expected_fields) end diff --git a/spec/graphql/types/notes/noteable_type_spec.rb b/spec/graphql/types/notes/noteable_type_spec.rb index 4a81f45bd4e..e673076d8a7 100644 --- a/spec/graphql/types/notes/noteable_type_spec.rb +++ b/spec/graphql/types/notes/noteable_type_spec.rb @@ -1,8 +1,16 @@ # frozen_string_literal: true + require 'spec_helper' describe Types::Notes::NoteableType do - specify { expect(described_class).to have_graphql_fields(:notes, :discussions) } + it 'exposes the expected fields' do + expected_fields = %i[ + discussions + notes + ] + + expect(described_class).to have_graphql_fields(*expected_fields) + end describe ".resolve_type" do it 'knows the correct type for objects' do diff --git a/spec/graphql/types/resolvable_interface_spec.rb b/spec/graphql/types/resolvable_interface_spec.rb new file mode 100644 index 00000000000..231287f9969 --- /dev/null +++ b/spec/graphql/types/resolvable_interface_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Types::ResolvableInterface do + it 'exposes the expected fields' do + expected_fields = %i[ + resolvable + resolved + resolved_at + resolved_by + ] + + expect(described_class).to have_graphql_fields(*expected_fields) + end +end diff --git a/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb b/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb new file mode 100644 index 00000000000..95e967c039d --- /dev/null +++ b/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Toggling the resolve status of a discussion' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:noteable) { create(:merge_request, source_project: project) } + let(:discussion) do + create(:diff_note_on_merge_request, noteable: noteable, project: project).to_discussion + end + let(:mutation) do + graphql_mutation(:discussion_toggle_resolve, { id: discussion.to_global_id.to_s, resolve: true }) + end + let(:mutation_response) { graphql_mutation_response(:discussion_toggle_resolve) } + + context 'when the user does not have permission' do + let_it_be(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns top-level errors', + errors: ["The resource that you are attempting to access does not exist or you don't have permission to perform this action"] + end + + context 'when user has permission' do + let_it_be(:current_user) { create(:user, developer_projects: [project]) } + + it 'returns the discussion without errors', :aggregate_failures do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response).to include( + 'discussion' => be_present, + 'errors' => be_empty + ) + end + + context 'when an error is encountered' do + before do + allow_next_instance_of(::Discussions::ResolveService) do |service| + allow(service).to receive(:execute).and_raise(ActiveRecord::RecordNotSaved) + end + end + + it_behaves_like 'a mutation that returns errors in the response', + errors: ['Discussion failed to be resolved'] + end + end +end