From f0a2c4116c862fdfa26015aa1a964714d41855e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 17 Jan 2019 18:49:07 +0100 Subject: [PATCH 1/2] Allow IssuableFinder to filter by closed_at MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/finders/issuable_finder.rb | 8 ++++++ app/models/concerns/closed_at_filterable.rb | 14 ++++++++++ app/models/concerns/issuable.rb | 1 + spec/finders/issues_finder_spec.rb | 30 +++++++++++++++++++++ 4 files changed, 53 insertions(+) create mode 100644 app/models/concerns/closed_at_filterable.rb diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 23af2e0521c..5870f158690 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -94,6 +94,7 @@ class IssuableFinder items = by_scope(items) items = by_created_at(items) items = by_updated_at(items) + items = by_closed_at(items) items = by_state(items) items = by_group(items) items = by_assignee(items) @@ -353,6 +354,13 @@ class IssuableFinder items end + def by_closed_at(items) + items = items.closed_after(params[:closed_after]) if params[:closed_after].present? + items = items.closed_before(params[:closed_before]) if params[:closed_before].present? + + items + end + # rubocop: disable CodeReuse/ActiveRecord def by_state(items) case params[:state].to_s diff --git a/app/models/concerns/closed_at_filterable.rb b/app/models/concerns/closed_at_filterable.rb new file mode 100644 index 00000000000..239c2e47611 --- /dev/null +++ b/app/models/concerns/closed_at_filterable.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module ClosedAtFilterable + extend ActiveSupport::Concern + + included do + scope :closed_before, ->(date) { where(scoped_table[:closed_at].lteq(date)) } + scope :closed_after, ->(date) { where(scoped_table[:closed_at].gteq(date)) } + + def self.scoped_table + arel_table.alias(table_name) + end + end +end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 0a77fbeba08..429a63f83cc 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -23,6 +23,7 @@ module Issuable include Sortable include CreatedAtFilterable include UpdatedAtFilterable + include ClosedAtFilterable # This object is used to gather issuable meta data for displaying # upvotes, downvotes, notes and closing merge requests count for issues and merge requests diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 34cb09942be..fe8000e419b 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -416,6 +416,36 @@ describe IssuesFinder do end end + context 'filtering by closed_at' do + let!(:closed_issue1) { create(:issue, project: project1, state: :closed, closed_at: 1.week.ago) } + let!(:closed_issue2) { create(:issue, project: project2, state: :closed, closed_at: 1.week.from_now) } + let!(:closed_issue3) { create(:issue, project: project2, state: :closed, closed_at: 2.weeks.from_now) } + + context 'through closed_after' do + let(:params) { { state: :closed, closed_after: closed_issue3.closed_at } } + + it 'returns issues closed on or after the given date' do + expect(issues).to contain_exactly(closed_issue3) + end + end + + context 'through closed_before' do + let(:params) { { state: :closed, closed_before: closed_issue1.closed_at } } + + it 'returns issues closed on or before the given date' do + expect(issues).to contain_exactly(closed_issue1) + end + end + + context 'through closed_after and closed_before' do + let(:params) { { state: :closed, closed_after: closed_issue2.closed_at, closed_before: closed_issue3.closed_at } } + + it 'returns issues closed between the given dates' do + expect(issues).to contain_exactly(closed_issue2, closed_issue3) + end + end + end + context 'filtering by reaction name' do context 'user searches by no reaction' do let(:params) { { my_reaction_emoji: 'None' } } From 87dfe5a27a51c722bbeaa4582168f1277a0df034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 17 Jan 2019 18:49:24 +0100 Subject: [PATCH 2/2] Add GraphQL filters for issuables (state, labels, time fields) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/graphql/resolvers/issues_resolver.rb | 25 ++++++- app/graphql/types/issuable_state_enum.rb | 12 ++++ app/graphql/types/issue_state_enum.rb | 8 +++ app/graphql/types/issue_type.rb | 2 +- app/graphql/types/merge_request_state_enum.rb | 10 +++ app/graphql/types/merge_request_type.rb | 2 +- ...ed_after-for-issuesresolver-in-graphql.yml | 5 ++ doc/api/README.md | 2 +- .../graphql/resolvers/issues_resolver_spec.rb | 69 ++++++++++++++++--- .../graphql/types/issuable_state_enum_spec.rb | 9 +++ spec/graphql/types/issue_state_enum_spec.rb | 9 +++ .../types/merge_request_state_enum_spec.rb | 13 ++++ .../graphql/issuable_state_shared_examples.rb | 5 ++ 13 files changed, 156 insertions(+), 15 deletions(-) create mode 100644 app/graphql/types/issuable_state_enum.rb create mode 100644 app/graphql/types/issue_state_enum.rb create mode 100644 app/graphql/types/merge_request_state_enum.rb create mode 100644 changelogs/unreleased/56492-implement-new-arguments-state-closed_before-and-closed_after-for-issuesresolver-in-graphql.yml create mode 100644 spec/graphql/types/issuable_state_enum_spec.rb create mode 100644 spec/graphql/types/issue_state_enum_spec.rb create mode 100644 spec/graphql/types/merge_request_state_enum_spec.rb create mode 100644 spec/support/shared_examples/graphql/issuable_state_shared_examples.rb diff --git a/app/graphql/resolvers/issues_resolver.rb b/app/graphql/resolvers/issues_resolver.rb index fd1b46ba860..b98d8bd1fff 100644 --- a/app/graphql/resolvers/issues_resolver.rb +++ b/app/graphql/resolvers/issues_resolver.rb @@ -9,7 +9,30 @@ module Resolvers argument :iids, [GraphQL::ID_TYPE], required: false, description: 'The list of IIDs of issues, e.g., [1, 2]' - + argument :state, Types::IssuableStateEnum, + required: false, + description: "Current state of Issue" + argument :label_name, GraphQL::STRING_TYPE.to_list_type, + required: false, + description: "Labels applied to the Issue" + argument :created_before, Types::TimeType, + required: false, + description: "Issues created before this date" + argument :created_after, Types::TimeType, + required: false, + description: "Issues created after this date" + argument :updated_before, Types::TimeType, + required: false, + description: "Issues updated before this date" + argument :updated_after, Types::TimeType, + required: false, + description: "Issues updated after this date" + argument :closed_before, Types::TimeType, + required: false, + description: "Issues closed before this date" + argument :closed_after, Types::TimeType, + required: false, + description: "Issues closed after this date" argument :search, GraphQL::STRING_TYPE, required: false argument :sort, Types::Sort, diff --git a/app/graphql/types/issuable_state_enum.rb b/app/graphql/types/issuable_state_enum.rb new file mode 100644 index 00000000000..f2f6d6c6cab --- /dev/null +++ b/app/graphql/types/issuable_state_enum.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Types + class IssuableStateEnum < BaseEnum + graphql_name 'IssuableState' + description 'State of a GitLab issue or merge request' + + value 'opened' + value 'closed' + value 'locked' + end +end diff --git a/app/graphql/types/issue_state_enum.rb b/app/graphql/types/issue_state_enum.rb new file mode 100644 index 00000000000..6521407fc9d --- /dev/null +++ b/app/graphql/types/issue_state_enum.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Types + class IssueStateEnum < IssuableStateEnum + graphql_name 'IssueState' + description 'State of a GitLab issue' + end +end diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb index a8f2f7914a8..87f6b1f8278 100644 --- a/app/graphql/types/issue_type.rb +++ b/app/graphql/types/issue_type.rb @@ -11,7 +11,7 @@ module Types field :iid, GraphQL::ID_TYPE, null: false field :title, GraphQL::STRING_TYPE, null: false field :description, GraphQL::STRING_TYPE, null: true - field :state, GraphQL::STRING_TYPE, null: false + field :state, IssueStateEnum, null: false field :author, Types::UserType, null: false, diff --git a/app/graphql/types/merge_request_state_enum.rb b/app/graphql/types/merge_request_state_enum.rb new file mode 100644 index 00000000000..92f52726ab3 --- /dev/null +++ b/app/graphql/types/merge_request_state_enum.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Types + class MergeRequestStateEnum < IssuableStateEnum + graphql_name 'MergeRequestState' + description 'State of a GitLab merge request' + + value 'merged' + end +end diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index 7e63d4022b1..7827b6e3717 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -12,7 +12,7 @@ module Types field :iid, GraphQL::ID_TYPE, null: false field :title, GraphQL::STRING_TYPE, null: false field :description, GraphQL::STRING_TYPE, null: true - field :state, GraphQL::STRING_TYPE, null: true + field :state, MergeRequestStateEnum, null: false field :created_at, Types::TimeType, null: false field :updated_at, Types::TimeType, null: false field :source_project, Types::ProjectType, null: true diff --git a/changelogs/unreleased/56492-implement-new-arguments-state-closed_before-and-closed_after-for-issuesresolver-in-graphql.yml b/changelogs/unreleased/56492-implement-new-arguments-state-closed_before-and-closed_after-for-issuesresolver-in-graphql.yml new file mode 100644 index 00000000000..9b7aed82d49 --- /dev/null +++ b/changelogs/unreleased/56492-implement-new-arguments-state-closed_before-and-closed_after-for-issuesresolver-in-graphql.yml @@ -0,0 +1,5 @@ +--- +title: "Implement new arguments `state`, `closed_before` and `closed_after` for `IssuesResolver` in GraphQL" +merge_request: 24910 +author: +type: changed diff --git a/doc/api/README.md b/doc/api/README.md index 89069fe60e1..48d9ad8f30d 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -134,7 +134,7 @@ Endpoints are available for: ## Road to GraphQL Going forward, we will start on moving to -[GraphQL](http://graphql.org/learn/best-practices/) and deprecate the use of +[GraphQL](graphql/index.md) and deprecate the use of controller-specific endpoints. GraphQL has a number of benefits: 1. We avoid having to maintain two different APIs. diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb index 66de372e9fe..5f9c180cbb7 100644 --- a/spec/graphql/resolvers/issues_resolver_spec.rb +++ b/spec/graphql/resolvers/issues_resolver_spec.rb @@ -5,16 +5,63 @@ describe Resolvers::IssuesResolver do let(:current_user) { create(:user) } set(:project) { create(:project) } - set(:issue) { create(:issue, project: project) } - set(:issue2) { create(:issue, project: project, title: 'foo') } + set(:issue1) { create(:issue, project: project, state: :opened, created_at: 3.hours.ago, updated_at: 3.hours.ago) } + set(:issue2) { create(:issue, project: project, state: :closed, title: 'foo', created_at: 1.hour.ago, updated_at: 1.hour.ago, closed_at: 1.hour.ago) } + set(:label1) { create(:label, project: project) } + set(:label2) { create(:label, project: project) } before do project.add_developer(current_user) + create(:label_link, label: label1, target: issue1) + create(:label_link, label: label1, target: issue2) + create(:label_link, label: label2, target: issue2) end describe '#resolve' do it 'finds all issues' do - expect(resolve_issues).to contain_exactly(issue, issue2) + expect(resolve_issues).to contain_exactly(issue1, issue2) + end + + it 'filters by state' do + expect(resolve_issues(state: 'opened')).to contain_exactly(issue1) + expect(resolve_issues(state: 'closed')).to contain_exactly(issue2) + end + + it 'filters by labels' do + expect(resolve_issues(label_name: [label1.title])).to contain_exactly(issue1, issue2) + expect(resolve_issues(label_name: [label1.title, label2.title])).to contain_exactly(issue2) + end + + describe 'filters by created_at' do + it 'filters by created_before' do + expect(resolve_issues(created_before: 2.hours.ago)).to contain_exactly(issue1) + end + + it 'filters by created_after' do + expect(resolve_issues(created_after: 2.hours.ago)).to contain_exactly(issue2) + end + end + + describe 'filters by updated_at' do + it 'filters by updated_before' do + expect(resolve_issues(updated_before: 2.hours.ago)).to contain_exactly(issue1) + end + + it 'filters by updated_after' do + expect(resolve_issues(updated_after: 2.hours.ago)).to contain_exactly(issue2) + end + end + + describe 'filters by closed_at' do + let!(:issue3) { create(:issue, project: project, state: :closed, closed_at: 3.hours.ago) } + + it 'filters by closed_before' do + expect(resolve_issues(closed_before: 2.hours.ago)).to contain_exactly(issue3) + end + + it 'filters by closed_after' do + expect(resolve_issues(closed_after: 2.hours.ago)).to contain_exactly(issue2) + end end it 'searches issues' do @@ -22,7 +69,7 @@ describe Resolvers::IssuesResolver do end it 'sort issues' do - expect(resolve_issues(sort: 'created_desc')).to eq [issue2, issue] + expect(resolve_issues(sort: 'created_desc')).to eq [issue2, issue1] end it 'returns issues user can see' do @@ -30,31 +77,31 @@ describe Resolvers::IssuesResolver do create(:issue, confidential: true) - expect(resolve_issues).to contain_exactly(issue, issue2) + expect(resolve_issues).to contain_exactly(issue1, issue2) end it 'finds a specific issue with iid' do - expect(resolve_issues(iid: issue.iid)).to contain_exactly(issue) + expect(resolve_issues(iid: issue1.iid)).to contain_exactly(issue1) end it 'finds a specific issue with iids' do - expect(resolve_issues(iids: issue.iid)).to contain_exactly(issue) + expect(resolve_issues(iids: issue1.iid)).to contain_exactly(issue1) end it 'finds multiple issues with iids' do - expect(resolve_issues(iids: [issue.iid, issue2.iid])) - .to contain_exactly(issue, issue2) + expect(resolve_issues(iids: [issue1.iid, issue2.iid])) + .to contain_exactly(issue1, issue2) end it 'finds only the issues within the project we are looking at' do another_project = create(:project) - iids = [issue, issue2].map(&:iid) + iids = [issue1, issue2].map(&:iid) iids.each do |iid| create(:issue, project: another_project, iid: iid) end - expect(resolve_issues(iids: iids)).to contain_exactly(issue, issue2) + expect(resolve_issues(iids: iids)).to contain_exactly(issue1, issue2) end end diff --git a/spec/graphql/types/issuable_state_enum_spec.rb b/spec/graphql/types/issuable_state_enum_spec.rb new file mode 100644 index 00000000000..65a80fa4176 --- /dev/null +++ b/spec/graphql/types/issuable_state_enum_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['IssuableState'] do + it { expect(described_class.graphql_name).to eq('IssuableState') } + + it_behaves_like 'issuable state' +end diff --git a/spec/graphql/types/issue_state_enum_spec.rb b/spec/graphql/types/issue_state_enum_spec.rb new file mode 100644 index 00000000000..de19e6fc505 --- /dev/null +++ b/spec/graphql/types/issue_state_enum_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['IssueState'] do + it { expect(described_class.graphql_name).to eq('IssueState') } + + it_behaves_like 'issuable state' +end diff --git a/spec/graphql/types/merge_request_state_enum_spec.rb b/spec/graphql/types/merge_request_state_enum_spec.rb new file mode 100644 index 00000000000..626e33b18d3 --- /dev/null +++ b/spec/graphql/types/merge_request_state_enum_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['MergeRequestState'] do + it { expect(described_class.graphql_name).to eq('MergeRequestState') } + + it_behaves_like 'issuable state' + + it 'exposes all the existing merge request states' do + expect(described_class.values.keys).to include('merged') + end +end diff --git a/spec/support/shared_examples/graphql/issuable_state_shared_examples.rb b/spec/support/shared_examples/graphql/issuable_state_shared_examples.rb new file mode 100644 index 00000000000..713f0a879c1 --- /dev/null +++ b/spec/support/shared_examples/graphql/issuable_state_shared_examples.rb @@ -0,0 +1,5 @@ +RSpec.shared_examples 'issuable state' do + it 'exposes all the existing issuable states' do + expect(described_class.values.keys).to include(*%w[opened closed locked]) + end +end