Merge branch 'zillemarco-timelogs-sorting' into 'master'

Added sorting option to timelogs GraphQL resolver

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106364



Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: Ashraf Khamis <akhamis@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Reviewed-by: Marco Zille <marco.zille@gmail.com>
Reviewed-by: Lee Tickett <ltickett@gitlab.com>
Co-authored-by: Marco Zille <marco.zille@gmail.com>

app/graphql/resolvers/timelog_resolver.rb

@@ -34,19 +34,23 @@ class TimelogResolver < BaseResolver
              required: false,
              description: 'List timelogs for a user.'
 
+    argument :sort, Types::TimeTracking::TimelogSortEnum,
+             description: 'List timelogs in a particular order.',
+             required: false,
+             default_value: { field: 'spent_at', direction: :asc }
+
     def resolve_with_lookahead(**args)
       validate_args!(object, args)
 
-      timelogs = object&.timelogs || Timelog.limit(GitlabSchema.default_max_page_size)
+      timelogs = object&.timelogs || Timelog.all
 
-      if args.any?
-        args = parse_datetime_args(args)
+      args = parse_datetime_args(args)
 
-        timelogs = apply_user_filter(timelogs, args)
-        timelogs = apply_project_filter(timelogs, args)
-        timelogs = apply_time_filter(timelogs, args)
-        timelogs = apply_group_filter(timelogs, args)
-      end
+      timelogs = apply_user_filter(timelogs, args)
+      timelogs = apply_project_filter(timelogs, args)
+      timelogs = apply_time_filter(timelogs, args)
+      timelogs = apply_group_filter(timelogs, args)
+      timelogs = apply_sorting(timelogs, args)
 
       apply_lookahead(timelogs)
     end
@@ -60,7 +64,12 @@ def preloads
     end
 
     def validate_args!(object, args)
-      if args.empty? && object.nil?
+      # sort is always provided because of its default value so we
+      # should check the remaining args to make sure at least one filter
+      # argument was provided
+      cleaned_args = args.except(:sort)
+
+      if cleaned_args.empty? && object.nil?
         raise_argument_error('Provide at least one argument')
       elsif args[:start_time] && args[:start_date]
         raise_argument_error('Provide either a start date or time, but not both')
@@ -132,6 +141,15 @@ def apply_time_filter(timelogs, args)
       timelogs
     end
 
+    def apply_sorting(timelogs, args)
+      return timelogs unless args[:sort]
+
+      field = args[:sort][:field]
+      direction = args[:sort][:direction]
+
+      timelogs.sort_by_field(field, direction)
+    end
+
     def raise_argument_error(message)
       raise Gitlab::Graphql::Errors::ArgumentError, message
     end

app/graphql/types/time_tracking/timelog_sort_enum.rb

+# frozen_string_literal: true
+
+module Types
+  module TimeTracking
+    class TimelogSortEnum < SortEnum
+      graphql_name 'TimelogSort'
+      description 'Values for sorting timelogs'
+
+      sortable_fields = ['Spent at', 'Time spent']
+
+      sortable_fields.each do |field|
+        value "#{field.upcase.tr(' ', '_')}_ASC",
+          value: { field: field.downcase.tr(' ', '_'), direction: :asc },
+          description: "#{field} by ascending order."
+        value "#{field.upcase.tr(' ', '_')}_DESC",
+          value: { field: field.downcase.tr(' ', '_'), direction: :desc },
+          description: "#{field} by descending order."
+      end
+    end
+  end
+end

app/models/timelog.rb

@@ -35,10 +35,21 @@ class Timelog < ApplicationRecord
     where('spent_at <= ?', end_time)
   end
 
+  scope :order_scope_asc, ->(field) { order(arel_table[field].asc.nulls_last) }
+  scope :order_scope_desc, ->(field) { order(arel_table[field].desc.nulls_last) }
+
   def issuable
     issue || merge_request
   end
 
+  def self.sort_by_field(field, direction)
+    if direction == :asc
+      order_scope_asc(field)
+    else
+      order_scope_desc(field)
+    end
+  end
+
   private
 
   def issuable_id_is_present

doc/api/graphql/reference/index.md

@@ -505,6 +505,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="querytimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="querytimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="querytimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="querytimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="querytimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="querytimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="querytimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |
@@ -14171,6 +14172,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="grouptimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="grouptimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="grouptimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="grouptimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="grouptimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="grouptimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="grouptimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |
@@ -15344,6 +15346,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="mergerequestassigneetimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="mergerequestassigneetimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="mergerequestassigneetimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="mergerequestassigneetimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="mergerequestassigneetimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="mergerequestassigneetimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="mergerequestassigneetimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |
@@ -15574,6 +15577,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="mergerequestauthortimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="mergerequestauthortimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="mergerequestauthortimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="mergerequestauthortimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="mergerequestauthortimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="mergerequestauthortimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="mergerequestauthortimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |
@@ -15823,6 +15827,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="mergerequestparticipanttimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="mergerequestparticipanttimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="mergerequestparticipanttimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="mergerequestparticipanttimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="mergerequestparticipanttimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="mergerequestparticipanttimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="mergerequestparticipanttimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |
@@ -16071,6 +16076,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="mergerequestreviewertimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="mergerequestreviewertimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="mergerequestreviewertimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="mergerequestreviewertimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="mergerequestreviewertimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="mergerequestreviewertimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="mergerequestreviewertimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |
@@ -18325,6 +18331,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="projecttimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="projecttimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="projecttimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="projecttimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="projecttimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="projecttimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="projecttimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |
@@ -20178,6 +20185,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="usercoretimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="usercoretimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="usercoretimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="usercoretimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="usercoretimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="usercoretimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="usercoretimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |
@@ -22850,6 +22858,25 @@ Category of error.
 | <a id="timeboxreporterrorreasonupdated_asc"></a>`updated_asc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `UPDATED_ASC`. |
 | <a id="timeboxreporterrorreasonupdated_desc"></a>`updated_desc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `UPDATED_DESC`. |
+### `TimelogSort`
+
+Values for sorting timelogs.
+
+| Value | Description |
+| ----- | ----------- |
+| <a id="timelogsortcreated_asc"></a>`CREATED_ASC` | Created at ascending order. |
+| <a id="timelogsortcreated_desc"></a>`CREATED_DESC` | Created at descending order. |
+| <a id="timelogsortspent_at_asc"></a>`SPENT_AT_ASC` | Spent at by ascending order. |
+| <a id="timelogsortspent_at_desc"></a>`SPENT_AT_DESC` | Spent at by descending order. |
+| <a id="timelogsorttime_spent_asc"></a>`TIME_SPENT_ASC` | Time spent by ascending order. |
+| <a id="timelogsorttime_spent_desc"></a>`TIME_SPENT_DESC` | Time spent by descending order. |
+| <a id="timelogsortupdated_asc"></a>`UPDATED_ASC` | Updated at ascending order. |
+| <a id="timelogsortupdated_desc"></a>`UPDATED_DESC` | Updated at descending order. |
+| <a id="timelogsortcreated_asc"></a>`created_asc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `CREATED_ASC`. |
+| <a id="timelogsortcreated_desc"></a>`created_desc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `CREATED_DESC`. |
+| <a id="timelogsortupdated_asc"></a>`updated_asc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `UPDATED_ASC`. |
+| <a id="timelogsortupdated_desc"></a>`updated_desc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `UPDATED_DESC`. |
+
 ### `TodoActionEnum`
 | Value | Description |
@@ -24464,6 +24491,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="usertimelogsendtime"></a>`endTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or before endTime. |
 | <a id="usertimelogsgroupid"></a>`groupId` | [`GroupID`](#groupid) | List timelogs for a group. |
 | <a id="usertimelogsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | List timelogs for a project. |
+| <a id="usertimelogssort"></a>`sort` | [`TimelogSort`](#timelogsort) | List timelogs in a particular order. |
 | <a id="usertimelogsstartdate"></a>`startDate` | [`Time`](#time) | List timelogs within a date range where the logged date is equal to or after startDate. |
 | <a id="usertimelogsstarttime"></a>`startTime` | [`Time`](#time) | List timelogs within a time range where the logged time is equal to or after startTime. |
 | <a id="usertimelogsusername"></a>`username` | [`String`](#string) | List timelogs for a user. |

spec/graphql/resolvers/timelog_resolver_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Resolvers::TimelogResolver do
+RSpec.describe Resolvers::TimelogResolver, feature_category: :team_planning do
   include GraphqlHelpers
 
   let_it_be(:current_user) { create(:user) }
@@ -262,18 +262,6 @@
     it_behaves_like 'with a user'
   end
 
-  context 'when > `default_max_page_size` records' do
-    let(:object) { nil }
-    let!(:timelog_list) { create_list(:timelog, 101, issue: issue) }
-    let(:args) { { project_id: global_id_of(project) } }
-    let(:extra_args) { {} }
-
-    it 'pagination returns `default_max_page_size` and sets `has_next_page` true' do
-      expect(timelogs.items.count).to be(100)
-      expect(timelogs.has_next_page).to be(true)
-    end
-  end
-
   context 'when no object or arguments provided' do
     let(:object) { nil }
     let(:args) { {} }
@@ -286,6 +274,21 @@
     end
   end
 
+  context 'when the sort argument is provided' do
+    let_it_be(:timelog_a) { create(:issue_timelog, time_spent: 7200, spent_at: 1.hour.ago, user: current_user) }
+    let_it_be(:timelog_b) { create(:issue_timelog, time_spent: 5400, spent_at: 2.hours.ago, user: current_user) }
+    let_it_be(:timelog_c) { create(:issue_timelog, time_spent: 1800, spent_at: 30.minutes.ago, user: current_user) }
+    let_it_be(:timelog_d) { create(:issue_timelog, time_spent: 3600, spent_at: 1.day.ago, user: current_user) }
+
+    let(:object) { current_user }
+    let(:args) { { sort: 'TIME_SPENT_ASC' } }
+    let(:extra_args) { {} }
+
+    it 'returns all the timelogs in the correct order' do
+      expect(timelogs.items).to eq([timelog_c, timelog_d, timelog_b, timelog_a])
+    end
+  end
+
   def resolve_timelogs(user: current_user, obj: object, **args)
     context = { current_user: user }
     resolve(described_class, obj: obj, args: args.merge(extra_args), ctx: context)

spec/graphql/types/query_type_spec.rb

@@ -135,7 +135,7 @@
     subject { described_class.fields['timelogs'] }
 
     it 'returns timelogs' do
-      is_expected.to have_graphql_arguments(:startDate, :endDate, :startTime, :endTime, :username, :projectId, :groupId, :after, :before, :first, :last)
+      is_expected.to have_graphql_arguments(:startDate, :endDate, :startTime, :endTime, :username, :projectId, :groupId, :after, :before, :first, :last, :sort)
       is_expected.to have_graphql_type(Types::TimelogType.connection_type)
       is_expected.to have_graphql_resolver(Resolvers::TimelogResolver)
     end

spec/graphql/types/time_tracking/timelog_sort_enum_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabSchema.types['TimelogSort'], feature_category: :team_planning do
+  specify { expect(described_class.graphql_name).to eq('TimelogSort') }
+
+  it_behaves_like 'common sort values'
+
+  it 'exposes all the contact sort values' do
+    expect(described_class.values.keys).to include(
+      *%w[
+        SPENT_AT_ASC
+        SPENT_AT_DESC
+        TIME_SPENT_ASC
+        TIME_SPENT_DESC
+      ]
+    )
+  end
+end

spec/models/timelog_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Timelog do
+RSpec.describe Timelog, feature_category: :team_planning do
   subject { create(:timelog) }
 
   let_it_be(:issue) { create(:issue) }
@@ -149,4 +149,30 @@ def just_after(time)
       end
     end
   end
+
+  describe 'sorting' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:timelog_a) { create(:issue_timelog, time_spent: 7200, spent_at: 1.hour.ago, user: user) }
+    let_it_be(:timelog_b) { create(:issue_timelog, time_spent: 5400, spent_at: 2.hours.ago, user: user) }
+    let_it_be(:timelog_c) { create(:issue_timelog, time_spent: 1800, spent_at: 30.minutes.ago, user: user) }
+    let_it_be(:timelog_d) { create(:issue_timelog, time_spent: 3600, spent_at: 1.day.ago, user: user) }
+
+    describe '.sort_by_field' do
+      it 'sorts timelogs by time spent in ascending order' do
+        expect(user.timelogs.sort_by_field('time_spent', :asc)).to eq([timelog_c, timelog_d, timelog_b, timelog_a])
+      end
+
+      it 'sorts timelogs by time spent in descending order' do
+        expect(user.timelogs.sort_by_field('time_spent', :desc)).to eq([timelog_a, timelog_b, timelog_d, timelog_c])
+      end
+
+      it 'sorts timelogs by spent at in ascending order' do
+        expect(user.timelogs.sort_by_field('spent_at', :asc)).to eq([timelog_d, timelog_b, timelog_a, timelog_c])
+      end
+
+      it 'sorts timelogs by spent at in descending order' do
+        expect(user.timelogs.sort_by_field('spent_at', :desc)).to eq([timelog_c, timelog_a, timelog_b, timelog_d])
+      end
+    end
+  end
 end