Commit 3b4398ba authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'graphql-expose-timelogs-against-issuables' into 'master'

Expose timelogs against issues and merge requests in graphql

See merge request gitlab-org/gitlab!57321
parents 0690b999 951ab3a6
......@@ -50,7 +50,8 @@ module ResolvesMergeRequests
approved_by: [:approved_by_users],
milestone: [:milestone],
security_auto_fix: [:author],
head_pipeline: [:merge_request_diff, { head_pipeline: [:merge_request] }]
head_pipeline: [:merge_request_diff, { head_pipeline: [:merge_request] }],
timelogs: [:timelogs]
}
end
end
......@@ -44,7 +44,8 @@ module Resolvers
{
alert_management_alert: [:alert_management_alert],
labels: [:labels],
assignees: [:assignees]
assignees: [:assignees],
timelogs: [:timelogs]
}
end
......
......@@ -124,6 +124,9 @@ module Types
field :create_note_email, GraphQL::STRING_TYPE, null: true,
description: 'User specific email address for the issue.'
field :timelogs, Types::TimelogType.connection_type, null: false,
description: 'Timelogs on the issue.'
def author
Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find
end
......
......@@ -186,6 +186,8 @@ module Types
description: 'Selected auto merge strategy.'
field :merge_user, Types::UserType, null: true,
description: 'User who merged this merge request.'
field :timelogs, Types::TimelogType.connection_type, null: false,
description: 'Timelogs on the merge request.'
def approved_by
object.approved_by_users
......
......@@ -2,9 +2,10 @@
module HasTimelogsReport
extend ActiveSupport::Concern
include Gitlab::Utils::StrongMemoize
def timelogs(start_time, end_time)
@timelogs ||= timelogs_for(start_time, end_time)
strong_memoize(:timelogs) { timelogs_for(start_time, end_time) }
end
def user_can_access_group_timelogs?(current_user)
......
---
title: Expose timelogs against issues and merge requests in GraphQL
merge_request: 57321
author: Lee Tickett @leetickett
type: added
......@@ -2905,6 +2905,7 @@ Relationship between an epic and an issue.
| `subscribed` | [`Boolean!`](#boolean) | Indicates the currently logged in user is subscribed to the issue. |
| `taskCompletionStatus` | [`TaskCompletionStatus!`](#taskcompletionstatus) | Task completion status of the issue. |
| `timeEstimate` | [`Int!`](#int) | Time estimate of the issue. |
| `timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the issue. |
| `title` | [`String!`](#string) | Title of the issue. |
| `titleHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `title`. |
| `totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the issue. |
......@@ -3468,6 +3469,7 @@ An edge in a connection.
| `subscribed` | [`Boolean!`](#boolean) | Indicates the currently logged in user is subscribed to the issue. |
| `taskCompletionStatus` | [`TaskCompletionStatus!`](#taskcompletionstatus) | Task completion status of the issue. |
| `timeEstimate` | [`Int!`](#int) | Time estimate of the issue. |
| `timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the issue. |
| `title` | [`String!`](#string) | Title of the issue. |
| `titleHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `title`. |
| `totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the issue. |
......@@ -3980,6 +3982,7 @@ An edge in a connection.
| `targetProjectId` | [`Int!`](#int) | ID of the merge request target project. |
| `taskCompletionStatus` | [`TaskCompletionStatus!`](#taskcompletionstatus) | Completion status of tasks. |
| `timeEstimate` | [`Int!`](#int) | Time estimate of the merge request. |
| `timelogs` | [`TimelogConnection!`](#timelogconnection) | Timelogs on the merge request. |
| `title` | [`String!`](#string) | Title of the merge request. |
| `titleHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `title`. |
| `totalTimeSpent` | [`Int!`](#int) | Total time reported as spent on the merge request. |
......
# frozen_string_literal: true
# Read about factories at https://github.com/thoughtbot/factory_bot
FactoryBot.define do
factory :timelog do
time_spent { 3600 }
issue
user { issue.project.creator }
for_issue
factory :issue_timelog, traits: [:for_issue]
factory :merge_request_timelog, traits: [:for_merge_request]
trait :for_issue do
issue
user { issue.author }
end
trait :for_merge_request do
merge_request
issue { nil }
user { merge_request.author }
end
end
end
......@@ -9,51 +9,57 @@ RSpec.describe Resolvers::TimelogResolver do
expect(described_class).to have_non_null_graphql_type(::Types::TimelogType.connection_type)
end
context "within a group" do
context "with a group" do
let_it_be(:current_user) { create(:user) }
let(:group) { create(:group) }
let(:project) { create(:project, :public, group: group) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, :public, group: group) }
before do
before_all do
group.add_developer(current_user)
project.add_developer(current_user)
end
before do
group.clear_memoization(:timelogs)
end
describe '#resolve' do
let(:issue) { create(:issue, project: project) }
let(:issue2) { create(:issue, project: project) }
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:issue2) { create(:issue, project: project) }
let_it_be(:timelog1) { create(:issue_timelog, issue: issue, spent_at: 2.days.ago.beginning_of_day) }
let_it_be(:timelog2) { create(:issue_timelog, issue: issue2, spent_at: 2.days.ago.end_of_day) }
let_it_be(:timelog3) { create(:issue_timelog, issue: issue2, spent_at: 10.days.ago) }
let(:args) { { start_time: 6.days.ago, end_time: 2.days.ago.noon } }
let!(:timelog1) { create(:timelog, issue: issue, spent_at: 2.days.ago.beginning_of_day) }
let!(:timelog2) { create(:timelog, issue: issue2, spent_at: 2.days.ago.end_of_day) }
let!(:timelog3) { create(:timelog, issue: issue2, spent_at: 10.days.ago) }
it 'finds all timelogs within given dates' do
timelogs = resolve_timelogs(args)
timelogs = resolve_timelogs(**args)
expect(timelogs).to contain_exactly(timelog1)
end
it 'return nothing when user has insufficient permissions' do
user = create(:user)
group.add_guest(current_user)
expect(resolve_timelogs(args)).to be_empty
expect(resolve_timelogs(user: user, **args)).to be_empty
end
context 'when start_time and end_date are present' do
let(:args) { { start_time: 6.days.ago, end_date: 2.days.ago } }
it 'finds timelogs until the end of day of end_date' do
timelogs = resolve_timelogs(args)
timelogs = resolve_timelogs(**args)
expect(timelogs).to contain_exactly(timelog1, timelog2)
end
end
context 'finds timelogs until the time specified on end_time' do
context 'when start_date and end_time are present' do
let(:args) { { start_date: 6.days.ago, end_time: 2.days.ago.noon } }
it 'finds all timelogs within start_date and end_time' do
timelogs = resolve_timelogs(args)
timelogs = resolve_timelogs(**args)
expect(timelogs).to contain_exactly(timelog1)
end
......@@ -66,7 +72,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { {} }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Start and End arguments must be present/)
end
end
......@@ -75,7 +81,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { start_time: 6.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Both Start and End arguments must be present/)
end
end
......@@ -84,7 +90,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { end_time: 2.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Both Start and End arguments must be present/)
end
end
......@@ -93,7 +99,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { start_date: 6.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Both Start and End arguments must be present/)
end
end
......@@ -102,7 +108,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { end_date: 2.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Both Start and End arguments must be present/)
end
end
......@@ -111,7 +117,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { start_time: 6.days.ago, start_date: 6.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Both Start and End arguments must be present/)
end
end
......@@ -120,7 +126,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { end_time: 2.days.ago, end_date: 2.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Both Start and End arguments must be present/)
end
end
......@@ -129,7 +135,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { start_date: 6.days.ago, end_date: 2.days.ago, end_time: 2.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Only Time or Date arguments must be present/)
end
end
......@@ -138,7 +144,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { start_time: 2.days.ago, end_time: 6.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /Start argument must be before End argument/)
end
end
......@@ -147,7 +153,7 @@ RSpec.describe Resolvers::TimelogResolver do
let(:args) { { start_time: 3.months.ago, end_time: 2.days.ago } }
it 'returns correct error' do
expect {resolve_timelogs(args)}
expect { resolve_timelogs(**args) }
.to raise_error(error_class, /The time range period cannot contain more than 60 days/)
end
end
......@@ -155,7 +161,8 @@ RSpec.describe Resolvers::TimelogResolver do
end
end
def resolve_timelogs(args = {}, context = { current_user: current_user })
def resolve_timelogs(user: current_user, **args)
context = { current_user: user }
resolve(described_class, obj: group, args: args, ctx: context)
end
end
......@@ -18,7 +18,7 @@ RSpec.describe GitlabSchema.types['Issue'] do
confidential discussion_locked upvotes downvotes user_notes_count user_discussions_count web_path web_url relative_position
emails_disabled subscribed time_estimate total_time_spent human_time_estimate human_total_time_spent closed_at created_at updated_at task_completion_status
design_collection alert_management_alert severity current_user_todos moved moved_to
create_note_email]
create_note_email timelogs]
fields.each do |field_name|
expect(described_class).to have_graphql_field(field_name)
......
......@@ -32,18 +32,16 @@ RSpec.describe HasTimelogsReport do
end
describe '#user_can_access_group_timelogs?' do
before do
it 'returns true if user can access group timelogs' do
group.add_developer(user)
end
it 'returns true if user can access group timelogs' do
expect(group.user_can_access_group_timelogs?(user)).to be_truthy
expect(group).to be_user_can_access_group_timelogs(user)
end
it 'returns false if user has insufficient permissions' do
group.add_guest(user)
expect(group.user_can_access_group_timelogs?(user)).to be_falsey
expect(group).not_to be_user_can_access_group_timelogs(user)
end
end
......
......@@ -56,9 +56,9 @@ RSpec.describe Timelog do
group = create(:group)
subgroup = create(:group, parent: group)
create(:timelog, issue: create(:issue, project: create(:project)))
timelog1 = create(:timelog, issue: create(:issue, project: create(:project, group: group)))
timelog2 = create(:timelog, issue: create(:issue, project: create(:project, group: subgroup)))
create(:issue_timelog)
timelog1 = create(:issue_timelog, issue: create(:issue, project: create(:project, group: group)))
timelog2 = create(:issue_timelog, issue: create(:issue, project: create(:project, group: subgroup)))
expect(described_class.for_issues_in_group(group)).to contain_exactly(timelog1, timelog2)
end
......@@ -66,9 +66,9 @@ RSpec.describe Timelog do
describe 'between_times' do
it 'returns collection of timelogs within given times' do
create(:timelog, spent_at: 65.days.ago)
timelog1 = create(:timelog, spent_at: 15.days.ago)
timelog2 = create(:timelog, spent_at: 5.days.ago)
create(:issue_timelog, spent_at: 65.days.ago)
timelog1 = create(:issue_timelog, spent_at: 15.days.ago)
timelog2 = create(:issue_timelog, spent_at: 5.days.ago)
timelogs = described_class.between_times(20.days.ago, 1.day.ago)
expect(timelogs).to contain_exactly(timelog1, timelog2)
......
......@@ -14,6 +14,7 @@ RSpec.describe 'Timelogs through GroupQuery' do
let_it_be(:timelog1) { create(:timelog, issue: issue, user: user, spent_at: '2019-08-13 14:00:00') }
let_it_be(:timelog2) { create(:timelog, issue: issue, user: user, spent_at: '2019-08-10 08:00:00') }
let_it_be(:params) { { startTime: '2019-08-10 12:00:00', endTime: '2019-08-21 12:00:00' } }
let(:timelogs_data) { graphql_data['group']['timelogs']['nodes'] }
before do
......@@ -34,11 +35,11 @@ RSpec.describe 'Timelogs through GroupQuery' do
end
it 'contains correct data', :aggregate_failures do
username = timelog_array.map {|data| data['user']['username'] }
username = timelog_array.map { |data| data['user']['username'] }
spent_at = timelog_array.map { |data| data['spentAt'].to_time }
time_spent = timelog_array.map { |data| data['timeSpent'] }
issue_title = timelog_array.map {|data| data['issue']['title'] }
milestone_title = timelog_array.map {|data| data['issue']['milestone']['title'] }
issue_title = timelog_array.map { |data| data['issue']['title'] }
milestone_title = timelog_array.map { |data| data['issue']['milestone']['title'] }
expect(username).to eq([user.username])
expect(spent_at.first).to be_like_time(timelog1.spent_at)
......@@ -50,7 +51,7 @@ RSpec.describe 'Timelogs through GroupQuery' do
context 'when arguments with no time are present' do
let!(:timelog3) { create(:timelog, issue: issue, user: user, spent_at: '2019-08-10 15:00:00') }
let!(:timelog4) { create(:timelog, issue: issue, user: user, spent_at: '2019-08-21 15:00:00') }
let(:params) { { startDate: '2019-08-10', endDate: '2019-08-21' }}
let(:params) { { startDate: '2019-08-10', endDate: '2019-08-21' } }
it 'sets times as start of day and end of day' do
expect(response).to have_gitlab_http_status(:ok)
......@@ -111,12 +112,10 @@ RSpec.describe 'Timelogs through GroupQuery' do
}
NODE
graphql_query_for("group", { "fullPath" => group.full_path },
[query_graphql_field(
"timelogs",
timelog_params,
timelog_nodes
)]
graphql_query_for(
:group,
{ full_path: group.full_path },
query_graphql_field(:timelogs, timelog_params, timelog_nodes)
)
end
end
......@@ -5,14 +5,14 @@ require 'spec_helper'
RSpec.describe 'getting an issue list for a project' do
include GraphqlHelpers
let(:issues_data) { graphql_data['project']['issues']['edges'] }
let_it_be(:project) { create(:project, :repository, :public) }
let_it_be(:current_user) { create(:user) }
let_it_be(:issue_a, reload: true) { create(:issue, project: project, discussion_locked: true) }
let_it_be(:issue_b, reload: true) { create(:issue, :with_alert, project: project) }
let_it_be(:issues, reload: true) { [issue_a, issue_b] }
let(:issues_data) { graphql_data['project']['issues']['edges'] }
let(:fields) do
<<~QUERY
edges {
......@@ -76,7 +76,7 @@ RSpec.describe 'getting an issue list for a project' do
end
end
context 'no limit is provided' do
context 'when no limit is provided' do
let(:issue_limit) { nil }
it 'returns all issues' do
......@@ -143,13 +143,15 @@ RSpec.describe 'getting an issue list for a project' do
let_it_be(:data_path) { [:project, :issues] }
def pagination_query(params)
graphql_query_for(:project, { full_path: sort_project.full_path },
graphql_query_for(
:project,
{ full_path: sort_project.full_path },
query_graphql_field(:issues, params, "#{page_info} nodes { iid }")
)
end
def pagination_results_data(data)
data.map { |issue| issue.dig('iid').to_i }
data.map { |issue| issue['iid'].to_i }
end
context 'when sorting by due date' do
......@@ -189,27 +191,38 @@ RSpec.describe 'getting an issue list for a project' do
it_behaves_like 'sorted paginated query' do
let(:sort_param) { :RELATIVE_POSITION_ASC }
let(:first_param) { 2 }
let(:expected_results) { [relative_issue5.iid, relative_issue3.iid, relative_issue1.iid, relative_issue4.iid, relative_issue2.iid] }
let(:expected_results) do
[
relative_issue5.iid, relative_issue3.iid, relative_issue1.iid,
relative_issue4.iid, relative_issue2.iid
]
end
end
end
end
context 'when sorting by priority' do
let_it_be(:sort_project) { create(:project, :public) }
let_it_be(:early_milestone) { create(:milestone, project: sort_project, due_date: 10.days.from_now) }
let_it_be(:late_milestone) { create(:milestone, project: sort_project, due_date: 30.days.from_now) }
let_it_be(:priority_label1) { create(:label, project: sort_project, priority: 1) }
let_it_be(:priority_label2) { create(:label, project: sort_project, priority: 5) }
let_it_be(:priority_issue1) { create(:issue, project: sort_project, labels: [priority_label1], milestone: late_milestone) }
let_it_be(:priority_issue2) { create(:issue, project: sort_project, labels: [priority_label2]) }
let_it_be(:priority_issue3) { create(:issue, project: sort_project, milestone: early_milestone) }
let_it_be(:priority_issue4) { create(:issue, project: sort_project) }
let_it_be(:on_project) { { project: sort_project } }
let_it_be(:early_milestone) { create(:milestone, **on_project, due_date: 10.days.from_now) }
let_it_be(:late_milestone) { create(:milestone, **on_project, due_date: 30.days.from_now) }
let_it_be(:priority_1) { create(:label, **on_project, priority: 1) }
let_it_be(:priority_2) { create(:label, **on_project, priority: 5) }
let_it_be(:priority_issue1) { create(:issue, **on_project, labels: [priority_1], milestone: late_milestone) }
let_it_be(:priority_issue2) { create(:issue, **on_project, labels: [priority_2]) }
let_it_be(:priority_issue3) { create(:issue, **on_project, milestone: early_milestone) }
let_it_be(:priority_issue4) { create(:issue, **on_project) }
context 'when ascending' do
it_behaves_like 'sorted paginated query' do
let(:sort_param) { :PRIORITY_ASC }
let(:first_param) { 2 }
let(:expected_results) { [priority_issue3.iid, priority_issue1.iid, priority_issue2.iid, priority_issue4.iid] }
let(:expected_results) do
[
priority_issue3.iid, priority_issue1.iid,
priority_issue2.iid, priority_issue4.iid
]
end
end
end
......@@ -217,7 +230,9 @@ RSpec.describe 'getting an issue list for a project' do
it_behaves_like 'sorted paginated query' do
let(:sort_param) { :PRIORITY_DESC }
let(:first_param) { 2 }
let(:expected_results) { [priority_issue1.iid, priority_issue3.iid, priority_issue2.iid, priority_issue4.iid] }
let(:expected_results) do
[priority_issue1.iid, priority_issue3.iid, priority_issue2.iid, priority_issue4.iid]
end
end
end
end
......@@ -275,7 +290,7 @@ RSpec.describe 'getting an issue list for a project' do
end
end
context 'fetching alert management alert' do
context 'when fetching alert management alert' do
let(:fields) do
<<~QUERY
edges {
......@@ -297,7 +312,7 @@ RSpec.describe 'getting an issue list for a project' do
it 'avoids N+1 queries' do
control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) }
create(:alert_management_alert, :with_issue, project: project )
create(:alert_management_alert, :with_issue, project: project)
expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control)
end
......@@ -312,7 +327,7 @@ RSpec.describe 'getting an issue list for a project' do
end
end
context 'fetching labels' do
context 'when fetching labels' do
let(:fields) do
<<~QUERY
edges {
......@@ -362,7 +377,7 @@ RSpec.describe 'getting an issue list for a project' do
end
end
context 'fetching assignees' do
context 'when fetching assignees' do
let(:fields) do
<<~QUERY
edges {
......@@ -420,9 +435,10 @@ RSpec.describe 'getting an issue list for a project' do
query = graphql_query_for(
:project,
{ full_path: project.full_path },
query_graphql_field(:issues, search_params, [
query_graphql_field(
:issues, search_params,
query_graphql_field(:nodes, nil, requested_fields)
])
)
)
post_graphql(query, current_user: current_user)
end
......@@ -448,5 +464,16 @@ RSpec.describe 'getting an issue list for a project' do
include_examples 'N+1 query check'
end
context 'when requesting `timelogs`' do
let(:requested_fields) { 'timelogs { nodes { timeSpent } }' }
before do
create_list(:issue_timelog, 2, issue: issue_a)
create(:issue_timelog, issue: issue_b)
end
include_examples 'N+1 query check'
end
end
end
......@@ -299,6 +299,7 @@ RSpec.describe 'getting merge request listings nested in a project' do
reviewers { nodes { username } }
participants { nodes { username } }
headPipeline { status }
timelogs { nodes { timeSpent } }
SELECT
end
......@@ -307,7 +308,7 @@ RSpec.describe 'getting merge request listings nested in a project' do
query($first: Int) {
project(fullPath: "#{project.full_path}") {
mergeRequests(first: $first) {
nodes { #{mr_fields} }
nodes { iid #{mr_fields} }
}
}
}
......@@ -324,6 +325,7 @@ RSpec.describe 'getting merge request listings nested in a project' do
mr.assignees << current_user
mr.reviewers << create(:user)
mr.reviewers << current_user
mr.timelogs << create(:merge_request_timelog, merge_request: mr)
end
end
......@@ -345,7 +347,7 @@ RSpec.describe 'getting merge request listings nested in a project' do
end
def user_collection
{ 'nodes' => all(match(a_hash_including('username' => be_present))) }
{ 'nodes' => be_present.and(all(match(a_hash_including('username' => be_present)))) }
end
it 'returns appropriate results' do
......@@ -358,7 +360,8 @@ RSpec.describe 'getting merge request listings nested in a project' do
'assignees' => user_collection,
'reviewers' => user_collection,
'participants' => user_collection,
'headPipeline' => { 'status' => be_present }
'headPipeline' => { 'status' => be_present },
'timelogs' => { 'nodes' => be_one }
)))
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