Commit 26265cf1 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'issue_198425' into 'master'

Allow to query group milestones with GraphQL

See merge request gitlab-org/gitlab!23635
parents 4976a9b3 22b56d5f
# frozen_string_literal: true
module TimeFrameFilter
def by_timeframe(items)
return items unless params[:start_date] && params[:start_date]
start_date = params[:start_date].to_date
end_date = params[:end_date].to_date
items.within_timeframe(start_date, end_date)
rescue ArgumentError
items
end
end
......@@ -11,6 +11,7 @@
class MilestonesFinder
include FinderMethods
include TimeFrameFilter
attr_reader :params
......@@ -24,6 +25,7 @@ class MilestonesFinder
items = by_title(items)
items = by_search_title(items)
items = by_state(items)
items = by_timeframe(items)
order(items)
end
......
# frozen_string_literal: true
module TimeFrameArguments
extend ActiveSupport::Concern
included do
argument :start_date, Types::TimeType,
required: false,
description: 'List items within a time frame where items.start_date is between startDate and endDate parameters (endDate parameter must be present)'
argument :end_date, Types::TimeType,
required: false,
description: 'List items within a time frame where items.end_date is between startDate and endDate parameters (startDate parameter must be present)'
end
def validate_timeframe_params!(args)
return unless args[:start_date].present? || args[:end_date].present?
error_message =
if args[:start_date].nil? || args[:end_date].nil?
"Both startDate and endDate must be present."
elsif args[:start_date] > args[:end_date]
"startDate is after endDate"
end
if error_message
raise Gitlab::Graphql::Errors::ArgumentError, error_message
end
end
end
# frozen_string_literal: true
module Resolvers
class MilestoneResolver < BaseResolver
include Gitlab::Graphql::Authorize::AuthorizeResource
include TimeFrameArguments
argument :state, Types::MilestoneStateEnum,
required: false,
description: 'Filter milestones by state'
type Types::MilestoneType, null: true
def resolve(**args)
validate_timeframe_params!(args)
authorize!
MilestonesFinder.new(milestones_finder_params(args)).execute
end
private
def milestones_finder_params(args)
{
state: args[:state] || 'all',
start_date: args[:start_date],
end_date: args[:end_date]
}.merge(parent_id_parameter)
end
def parent
@parent ||= object.respond_to?(:sync) ? object.sync : object
end
def parent_id_parameter
if parent.is_a?(Group)
{ group_ids: parent.id }
elsif parent.is_a?(Project)
{ project_ids: parent.id }
end
end
# MilestonesFinder does not check for current_user permissions,
# so for now we need to keep it here.
def authorize!
Ability.allowed?(context[:current_user], :read_milestone, parent) || raise_resource_not_available_error!
end
end
end
......@@ -42,6 +42,10 @@ module Types
field :parent, GroupType, null: true,
description: 'Parent group',
resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Group, obj.parent_id).find }
field :milestones, Types::MilestoneType.connection_type, null: true,
description: 'Find milestones',
resolver: Resolvers::MilestoneResolver
end
end
......
# frozen_string_literal: true
module Types
class MilestoneStateEnum < BaseEnum
value 'active'
value 'closed'
end
end
......@@ -3,25 +3,36 @@
module Types
class MilestoneType < BaseObject
graphql_name 'Milestone'
description 'Represents a milestone.'
present_using MilestonePresenter
authorize :read_milestone
field :id, GraphQL::ID_TYPE, null: false,
description: 'ID of the milestone'
field :description, GraphQL::STRING_TYPE, null: true,
description: 'Description of the milestone'
field :title, GraphQL::STRING_TYPE, null: false,
description: 'Title of the milestone'
field :state, GraphQL::STRING_TYPE, null: false,
field :description, GraphQL::STRING_TYPE, null: true,
description: 'Description of the milestone'
field :state, Types::MilestoneStateEnum, null: false,
description: 'State of the milestone'
field :web_path, GraphQL::STRING_TYPE, null: false, method: :milestone_path,
description: 'Web path of the milestone'
field :due_date, Types::TimeType, null: true,
description: 'Timestamp of the milestone due date'
field :start_date, Types::TimeType, null: true,
description: 'Timestamp of the milestone start date'
field :created_at, Types::TimeType, null: false,
description: 'Timestamp of milestone creation'
field :updated_at, Types::TimeType, null: false,
description: 'Timestamp of last milestone update'
end
......
......@@ -59,6 +59,12 @@ class Milestone < ApplicationRecord
where(project_id: projects).or(where(group_id: groups))
end
scope :within_timeframe, -> (start_date, end_date) do
where('start_date is not NULL or due_date is not NULL')
.where('start_date is NULL or start_date <= ?', end_date)
.where('due_date is NULL or due_date >= ?', start_date)
end
scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) }
scope :reorder_by_due_date_asc, -> { reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) }
......
# frozen_string_literal: true
class MilestonePresenter < Gitlab::View::Presenter::Delegated
presents :milestone
def milestone_path
url_builder.milestone_path(milestone)
end
private
def url_builder
@url_builder ||= Gitlab::UrlBuilder.new(milestone)
end
end
---
title: Expose group milestones on GraphQL
merge_request: 23635
author:
type: added
......@@ -1735,8 +1735,8 @@ type Epic implements Noteable {
before: String
"""
List epics within a time frame where epics.end_date is between start_date
and end_date parameters (start_date parameter must be present)
List items within a time frame where items.end_date is between startDate and
endDate parameters (startDate parameter must be present)
"""
endDate: Time
......@@ -1776,8 +1776,8 @@ type Epic implements Noteable {
sort: EpicSort
"""
List epics within a time frame where epics.start_date is between start_date
and end_date parameters (end_date parameter must be present)
List items within a time frame where items.start_date is between startDate
and endDate parameters (endDate parameter must be present)
"""
startDate: Time
......@@ -2704,8 +2704,8 @@ type Group {
authorUsername: String
"""
List epics within a time frame where epics.end_date is between start_date
and end_date parameters (start_date parameter must be present)
List items within a time frame where items.end_date is between startDate and
endDate parameters (startDate parameter must be present)
"""
endDate: Time
......@@ -2735,8 +2735,8 @@ type Group {
sort: EpicSort
"""
List epics within a time frame where epics.start_date is between start_date
and end_date parameters (end_date parameter must be present)
List items within a time frame where items.start_date is between startDate
and endDate parameters (endDate parameter must be present)
"""
startDate: Time
......@@ -2766,8 +2766,8 @@ type Group {
before: String
"""
List epics within a time frame where epics.end_date is between start_date
and end_date parameters (start_date parameter must be present)
List items within a time frame where items.end_date is between startDate and
endDate parameters (startDate parameter must be present)
"""
endDate: Time
......@@ -2807,8 +2807,8 @@ type Group {
sort: EpicSort
"""
List epics within a time frame where epics.start_date is between start_date
and end_date parameters (end_date parameter must be present)
List items within a time frame where items.start_date is between startDate
and endDate parameters (endDate parameter must be present)
"""
startDate: Time
......@@ -2853,6 +2853,48 @@ type Group {
"""
mentionsDisabled: Boolean
"""
Find milestones
"""
milestones(
"""
Returns the elements in the list that come after the specified cursor.
"""
after: String
"""
Returns the elements in the list that come before the specified cursor.
"""
before: String
"""
List items within a time frame where items.end_date is between startDate and
endDate parameters (startDate parameter must be present)
"""
endDate: Time
"""
Returns the first _n_ elements from the list.
"""
first: Int
"""
Returns the last _n_ elements from the list.
"""
last: Int
"""
List items within a time frame where items.start_date is between startDate
and endDate parameters (endDate parameter must be present)
"""
startDate: Time
"""
Filter milestones by state
"""
state: MilestoneStateEnum
): MilestoneConnection
"""
Name of the namespace
"""
......@@ -4457,6 +4499,9 @@ type Metadata {
version: String!
}
"""
Represents a milestone.
"""
type Milestone {
"""
Timestamp of milestone creation
......@@ -4486,7 +4531,7 @@ type Milestone {
"""
State of the milestone
"""
state: String!
state: MilestoneStateEnum!
"""
Title of the milestone
......@@ -4497,6 +4542,51 @@ type Milestone {
Timestamp of last milestone update
"""
updatedAt: Time!
"""
Web path of the milestone
"""
webPath: String!
}
"""
The connection type for Milestone.
"""
type MilestoneConnection {
"""
A list of edges.
"""
edges: [MilestoneEdge]
"""
A list of nodes.
"""
nodes: [Milestone]
"""
Information to aid in pagination.
"""
pageInfo: PageInfo!
}
"""
An edge in a connection.
"""
type MilestoneEdge {
"""
A cursor for use in pagination.
"""
cursor: String!
"""
The item at the end of the edge.
"""
node: Milestone
}
enum MilestoneStateEnum {
active
closed
}
"""
......
......@@ -683,6 +683,8 @@ Autogenerated return type of MergeRequestSetWip
## Milestone
Represents a milestone.
| Name | Type | Description |
| --- | ---- | ---------- |
| `createdAt` | Time! | Timestamp of milestone creation |
......@@ -690,9 +692,10 @@ Autogenerated return type of MergeRequestSetWip
| `dueDate` | Time | Timestamp of the milestone due date |
| `id` | ID! | ID of the milestone |
| `startDate` | Time | Timestamp of the milestone start date |
| `state` | String! | State of the milestone |
| `state` | MilestoneStateEnum! | State of the milestone |
| `title` | String! | Title of the milestone |
| `updatedAt` | Time! | Timestamp of last milestone update |
| `webPath` | String! | Web path of the milestone |
## Namespace
......
......@@ -20,6 +20,8 @@
# include_descendant_groups: boolean
class EpicsFinder < IssuableFinder
include TimeFrameFilter
def self.scalar_params
@scalar_params ||= %i[
parent_id
......@@ -114,22 +116,6 @@ class EpicsFinder < IssuableFinder
end
end
# rubocop: disable CodeReuse/ActiveRecord
def by_timeframe(items)
return items unless params[:start_date] && params[:end_date]
end_date = params[:end_date].to_date
start_date = params[:start_date].to_date
items
.where('epics.start_date is not NULL or epics.end_date is not NULL')
.where('epics.start_date is NULL or epics.start_date <= ?', end_date)
.where('epics.end_date is NULL or epics.end_date >= ?', start_date)
rescue ArgumentError
items
end
# rubocop: enable CodeReuse/ActiveRecord
def parent_id?
params[:parent_id].present?
end
......
......@@ -2,6 +2,8 @@
module Resolvers
class EpicResolver < BaseResolver
include TimeFrameArguments
argument :iid, GraphQL::ID_TYPE,
required: false,
description: 'IID of the epic, e.g., "1"'
......@@ -30,14 +32,6 @@ module Resolvers
required: false,
description: 'Filter epics by labels'
argument :start_date, Types::TimeType,
required: false,
description: 'List epics within a time frame where epics.start_date is between start_date and end_date parameters (end_date parameter must be present)'
argument :end_date, Types::TimeType,
required: false,
description: 'List epics within a time frame where epics.end_date is between start_date and end_date parameters (start_date parameter must be present)'
type Types::EpicType, null: true
def resolve(**args)
......@@ -46,7 +40,7 @@ module Resolvers
return [] unless resolver_object.present?
return [] unless epic_feature_enabled?
validate_date_params!(args)
validate_timeframe_params!(args)
find_epics(transform_args(args))
end
......@@ -63,16 +57,6 @@ module Resolvers
group.feature_available?(:epics)
end
def validate_date_params!(args)
return unless args[:start_date].present? || args[:end_date].present?
date_params_complete = args[:start_date] && args[:end_date]
unless date_params_complete
raise Gitlab::Graphql::Errors::ArgumentError, "Both start_date and end_date must be present."
end
end
def transform_args(args)
transformed = args.dup
transformed[:group_id] = group.id
......
......@@ -67,6 +67,12 @@ module EE
scope :in_issues, -> (issues) { joins(:epic_issues).where(epic_issues: { issue_id: issues }).distinct }
scope :has_parent, -> { where.not(parent_id: nil) }
scope :within_timeframe, -> (start_date, end_date) do
where('start_date is not NULL or end_date is not NULL')
.where('start_date is NULL or start_date <= ?', end_date)
.where('end_date is NULL or end_date >= ?', start_date)
end
scope :order_start_or_end_date_asc, -> do
reorder(Arel.sql("COALESCE(start_date, end_date) ASC NULLS FIRST"))
end
......
......@@ -73,13 +73,13 @@ describe Resolvers::EpicResolver do
end
context 'when only start_date is present' do
it 'returns epics within timeframe' do
it 'raises error' do
expect { resolve_epics(start_date: '2019-08-13') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
end
end
context 'when only end_date is present' do
it 'returns epics within timeframe' do
it 'raises error' do
expect { resolve_epics(end_date: '2019-08-13') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
end
end
......
......@@ -284,6 +284,15 @@ describe Epic do
end
end
it_behaves_like 'within_timeframe scope' do
let_it_be(:now) { Time.now }
let_it_be(:group) { create(:group) }
let_it_be(:resource_1) { create(:epic, group: group, start_date: now - 1.day, end_date: now + 1.day) }
let_it_be(:resource_2) { create(:epic, group: group, start_date: now + 2.days, end_date: now + 3.days) }
let_it_be(:resource_3) { create(:epic, group: group, end_date: now) }
let_it_be(:resource_4) { create(:epic, group: group, start_date: now) }
end
describe '#start_date_from_milestones' do
context 'fixed date' do
it 'returns start date from start date sourcing milestone' do
......
......@@ -3,13 +3,14 @@
require 'spec_helper'
describe MilestonesFinder do
let(:now) { Time.now }
let(:group) { create(:group) }
let(:project_1) { create(:project, namespace: group) }
let(:project_2) { create(:project, namespace: group) }
let!(:milestone_1) { create(:milestone, group: group, title: 'one test', due_date: Date.today) }
let!(:milestone_2) { create(:milestone, group: group) }
let!(:milestone_3) { create(:milestone, project: project_1, state: 'active', due_date: Date.tomorrow) }
let!(:milestone_4) { create(:milestone, project: project_2, state: 'active') }
let!(:milestone_1) { create(:milestone, group: group, title: 'one test', start_date: now - 1.day, due_date: now) }
let!(:milestone_2) { create(:milestone, group: group, start_date: now + 1.day, due_date: now + 2.days) }
let!(:milestone_3) { create(:milestone, project: project_1, state: 'active', start_date: now + 2.days, due_date: now + 3.days) }
let!(:milestone_4) { create(:milestone, project: project_2, state: 'active', start_date: now + 4.days, due_date: now + 5.days) }
it 'returns milestones for projects' do
result = described_class.new(project_ids: [project_1.id, project_2.id], state: 'all').execute
......@@ -33,8 +34,11 @@ describe MilestonesFinder do
end
it 'orders milestones by due date' do
expect(result.first).to eq(milestone_1)
expect(result.second).to eq(milestone_3)
milestone = create(:milestone, group: group, due_date: now - 2.days)
expect(result.first).to eq(milestone)
expect(result.second).to eq(milestone_1)
expect(result.third).to eq(milestone_2)
end
end
......@@ -77,6 +81,34 @@ describe MilestonesFinder do
expect(result.to_a).to contain_exactly(milestone_1)
end
context 'by timeframe' do
it 'returns milestones with start_date and due_date between timeframe' do
params.merge!(start_date: now - 1.day, end_date: now + 3.days)
milestones = described_class.new(params).execute
expect(milestones).to match_array([milestone_1, milestone_2, milestone_3])
end
it 'returns milestones which starts before the timeframe' do
milestone = create(:milestone, project: project_2, start_date: now - 5.days)
params.merge!(start_date: now - 3.days, end_date: now - 2.days)
milestones = described_class.new(params).execute
expect(milestones).to match_array([milestone])
end
it 'returns milestones which ends after the timeframe' do
milestone = create(:milestone, project: project_2, due_date: now + 6.days)
params.merge!(start_date: now + 6.days, end_date: now + 7.days)
milestones = described_class.new(params).execute
expect(milestones).to match_array([milestone])
end
end
end
describe '#find_by' do
......
# frozen_string_literal: true
require 'spec_helper'
describe Resolvers::MilestoneResolver do
include GraphqlHelpers
describe '#resolve' do
let_it_be(:current_user) { create(:user) }
context 'for group milestones' do
let_it_be(:now) { Time.now }
let_it_be(:group) { create(:group, :private) }
def resolve_group_milestones(args = {}, context = { current_user: current_user })
resolve(described_class, obj: group, args: args, ctx: context)
end
before do
group.add_developer(current_user)
end
it 'calls MilestonesFinder#execute' do
expect_next_instance_of(MilestonesFinder) do |finder|
expect(finder).to receive(:execute)
end
resolve_group_milestones
end
context 'without parameters' do
it 'calls MilestonesFinder to retrieve all milestones' do
expect(MilestonesFinder).to receive(:new)
.with(group_ids: group.id, state: 'all', start_date: nil, end_date: nil)
.and_call_original
resolve_group_milestones
end
end
context 'with parameters' do
it 'calls MilestonesFinder with correct parameters' do
start_date = now
end_date = start_date + 1.hour
expect(MilestonesFinder).to receive(:new)
.with(group_ids: group.id, state: 'closed', start_date: start_date, end_date: end_date)
.and_call_original
resolve_group_milestones(start_date: start_date, end_date: end_date, state: 'closed')
end
end
context 'by timeframe' do
context 'when start_date and end_date are present' do
context 'when start date is after end_date' do
it 'raises error' do
expect do
resolve_group_milestones(start_date: now, end_date: now - 2.days)
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError, "startDate is after endDate")
end
end
end
context 'when only start_date is present' do
it 'raises error' do
expect do
resolve_group_milestones(start_date: now)
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError, /Both startDate and endDate/)
end
end
context 'when only end_date is present' do
it 'raises error' do
expect do
resolve_group_milestones(end_date: now)
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError, /Both startDate and endDate/)
end
end
end
context 'when user cannot read milestones' do
it 'raises error' do
unauthorized_user = create(:user)
expect do
resolve_group_milestones({}, { current_user: unauthorized_user })
end.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
end
end
end
......@@ -197,6 +197,15 @@ describe Milestone do
end
end
it_behaves_like 'within_timeframe scope' do
let_it_be(:now) { Time.now }
let_it_be(:project) { create(:project, :empty_repo) }
let_it_be(:resource_1) { create(:milestone, project: project, start_date: now - 1.day, due_date: now + 1.day) }
let_it_be(:resource_2) { create(:milestone, project: project, start_date: now + 2.days, due_date: now + 3.days) }
let_it_be(:resource_3) { create(:milestone, project: project, due_date: now) }
let_it_be(:resource_4) { create(:milestone, project: project, start_date: now) }
end
describe "#percent_complete" do
it "does not count open issues" do
milestone.issues << issue
......
# frozen_string_literal: true
require 'spec_helper'
describe MilestonePresenter do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:milestone) { create(:milestone, group: group) }
let_it_be(:presenter) { described_class.new(milestone, current_user: user) }
before do
group.add_developer(user)
end
describe '#milestone_path' do
it 'returns correct path' do
expect(presenter.milestone_path).to eq("/groups/#{group.full_path}/-/milestones/#{milestone.iid}")
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Milestones through GroupQuery' do
include GraphqlHelpers
let_it_be(:user) { create(:user) }
let_it_be(:now) { Time.now }
let_it_be(:group) { create(:group, :private) }
let_it_be(:milestone_1) { create(:milestone, group: group) }
let_it_be(:milestone_2) { create(:milestone, group: group, state: :closed, start_date: now, due_date: now + 1.day) }
let_it_be(:milestone_3) { create(:milestone, group: group, start_date: now, due_date: now + 2.days) }
let_it_be(:milestone_4) { create(:milestone, group: group, state: :closed, start_date: now - 2.days, due_date: now - 1.day) }
let_it_be(:milestone_from_other_group) { create(:milestone, group: create(:group)) }
let(:milestone_data) { graphql_data['group']['milestones']['edges'] }
describe 'Get list of milestones from a group' do
before do
group.add_developer(user)
end
context 'when the request is correct' do
before do
fetch_milestones(user)
end
it_behaves_like 'a working graphql query'
it 'returns milestones successfully' do
expect(response).to have_gitlab_http_status(200)
expect(graphql_errors).to be_nil
expect_array_response(milestone_1.to_global_id.to_s, milestone_2.to_global_id.to_s, milestone_3.to_global_id.to_s, milestone_4.to_global_id.to_s)
end
end
context 'when filtering by timeframe' do
it 'fetches milestones between start_date and due_date' do
fetch_milestones(user, { start_date: now.to_s, end_date: (now + 2.days).to_s })
expect_array_response(milestone_2.to_global_id.to_s, milestone_3.to_global_id.to_s)
end
end
context 'when filtering by state' do
it 'returns milestones with given state' do
fetch_milestones(user, { state: :active })
expect_array_response(milestone_1.to_global_id.to_s, milestone_3.to_global_id.to_s)
end
end
def fetch_milestones(user = nil, args = {})
post_graphql(milestones_query(args), current_user: user)
end
def milestones_query(args = {})
milestone_node = <<~NODE
edges {
node {
id
title
state
}
}
NODE
graphql_query_for("group",
{ full_path: group.full_path },
[query_graphql_field("milestones", args, milestone_node)]
)
end
def expect_array_response(*items)
expect(response).to have_gitlab_http_status(:success)
expect(milestone_data).to be_an Array
expect(milestone_node_array('id')).to match_array(items)
end
def milestone_node_array(extract_attribute = nil)
node_array(milestone_data, extract_attribute)
end
end
end
......@@ -185,12 +185,13 @@ module GraphqlHelpers
end
# Fairly dumb Ruby => GraphQL rendering function. Only suitable for testing.
# Missing support for Enums (feel free to add if you need it).
# Use symbol for Enum values
def as_graphql_literal(value)
case value
when Array then "[#{value.map { |v| as_graphql_literal(v) }.join(',')}]"
when Integer, Float then value.to_s
when String then "\"#{value.gsub(/"/, '\\"')}\""
when Symbol then value
when nil then 'null'
when true then 'true'
when false then 'false'
......
# frozen_string_literal: true
RSpec.shared_examples 'within_timeframe scope' do
describe '.within_timeframe' do
it 'returns resources with start_date and/or end_date between timeframe' do
resources = described_class.within_timeframe(now + 2.days, now + 3.days)
expect(resources).to match_array([resource_2, resource_4])
end
it 'returns resources which starts before the timeframe' do
resources = described_class.within_timeframe(now, now + 1.day)
expect(resources).to match_array([resource_1, resource_3, resource_4])
end
it 'returns resources which ends after the timeframe' do
resources = described_class.within_timeframe(now + 3.days, now + 5.days)
expect(resources).to match_array([resource_2, resource_4])
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