Merge branch 'move-graphql-timelogs-to-ce' into 'master'

Move graphql timelogs to ce

See merge request gitlab-org/gitlab!56633

.rubocop_manual_todo.yml

@@ -1663,6 +1663,7 @@ Gitlab/NamespacedClass:
     - 'app/policies/service_policy.rb'
     - 'app/policies/suggestion_policy.rb'
     - 'app/policies/timebox_policy.rb'
+    - 'app/policies/timelog_policy.rb'
     - 'app/policies/todo_policy.rb'
     - 'app/policies/user_policy.rb'
     - 'app/policies/wiki_page_policy.rb'
@@ -2265,7 +2266,6 @@ Gitlab/NamespacedClass:
     - 'ee/app/policies/iteration_policy.rb'
     - 'ee/app/policies/push_rule_policy.rb'
     - 'ee/app/policies/saml_provider_policy.rb'
-    - 'ee/app/policies/timelog_policy.rb'
     - 'ee/app/policies/vulnerability_policy.rb'
     - 'ee/app/presenters/approval_rule_presenter.rb'
     - 'ee/app/presenters/audit_event_presenter.rb'

ee/app/graphql/resolvers/timelog_resolver.rb → app/graphql/resolvers/timelog_resolver.rb

@@ -45,7 +45,6 @@ module Resolvers
     end
 
     def timelogs_available_for_user?
-      group&.feature_available?(:group_timelogs) &&
       group&.user_can_access_group_timelogs?(context[:current_user])
     end
 

app/graphql/types/group_type.rb

@@ -114,6 +114,12 @@ module Types
           description: 'Labels available on this group.',
           resolver: Resolvers::GroupLabelsResolver
 
+    field :timelogs, ::Types::TimelogType.connection_type, null: false,
+          description: 'Time logged on issues in the group and its subgroups.',
+          extras: [:lookahead],
+          complexity: 5,
+          resolver: ::Resolvers::TimelogResolver
+
     def avatar_url
       object.avatar_url(only_path: false)
     end

ee/app/models/concerns/has_timelogs_report.rb → app/models/concerns/has_timelogs_report.rb

@@ -8,8 +8,6 @@ module HasTimelogsReport
   end
 
   def user_can_access_group_timelogs?(current_user)
-    return unless feature_available?(:group_timelogs)
-
     Ability.allowed?(current_user, :read_group_timelogs, self)
   end
 

app/models/group.rb

@@ -16,6 +16,7 @@ class Group < Namespace
   include Gitlab::Utils::StrongMemoize
   include GroupAPICompatibility
   include EachBatch
+  include HasTimelogsReport
 
   ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT = 10
 

app/policies/group_policy.rb

@@ -130,6 +130,7 @@ class GroupPolicy < BasePolicy
     enable :read_prometheus
     enable :read_package
     enable :read_package_settings
+    enable :read_group_timelogs
   end
 
   rule { maintainer }.policy do

app/policies/project_policy.rb

@@ -259,6 +259,7 @@ class ProjectPolicy < BasePolicy
     enable :read_confidential_issues
     enable :read_package
     enable :read_product_analytics
+    enable :read_group_timelogs
   end
 
   # We define `:public_user_access` separately because there are cases in gitlab-ee

changelogs/unreleased/move-graphql-timelogs-to-ce.yml

+---
+title: Move graphql timelogs to CE
+merge_request: 56633
+author: Lee Tickett @leetickett
+type: fixed

doc/api/graphql/reference/index.md

@@ -3065,7 +3065,6 @@ Autogenerated return type of GitlabSubscriptionActivate.
 | `fullName` | [`String!`](#string) | Full name of the namespace. |
 | `fullPath` | [`ID!`](#id) | Full path of the namespace. |
 | `groupMembers` | [`GroupMemberConnection`](#groupmemberconnection) | A membership of a user within this group. |
-| `groupTimelogsEnabled` | [`Boolean`](#boolean) | Indicates if Group timelogs are enabled for namespace |
 | `id` | [`ID!`](#id) | ID of the namespace. |
 | `isTemporaryStorageIncreaseEnabled` | [`Boolean!`](#boolean) | Status of the temporary storage increase. |
 | `issues` | [`IssueConnection`](#issueconnection) | Issues for projects in this group. |
@@ -3093,7 +3092,7 @@ Autogenerated return type of GitlabSubscriptionActivate.
 | `storageSizeLimit` | [`Float`](#float) | Total storage limit of the root namespace in bytes. |
 | `subgroupCreationLevel` | [`String`](#string) | The permission level required to create subgroups within the group. |
 | `temporaryStorageIncreaseEndsOn` | [`Time`](#time) | Date until the temporary storage increase is active. |
-| `timelogs` | [`TimelogConnection!`](#timelogconnection) | Time logged in issues by group members. |
+| `timelogs` | [`TimelogConnection!`](#timelogconnection) | Time logged on issues in the group and its subgroups. |
 | `totalRepositorySize` | [`Float`](#float) | Total repository size of all projects in the root namespace in bytes. |
 | `totalRepositorySizeExcess` | [`Float`](#float) | Total excess repository size of all projects in the root namespace in bytes. |
 | `twoFactorGracePeriod` | [`Int`](#int) | Time before two-factor authentication is enforced. |

ee/app/graphql/ee/types/group_type.rb

@@ -6,7 +6,7 @@ module EE
       extend ActiveSupport::Concern
 
       prepended do
-        %i[epics group_timelogs].each do |feature|
+        %i[epics].each do |feature|
           field "#{feature}_enabled", GraphQL::BOOLEAN_TYPE, null: true,
                 description: "Indicates if #{feature.to_s.humanize} are enabled for namespace"
 
@@ -43,12 +43,6 @@ module EE
               description: 'Find iteration cadences.',
               resolver: ::Resolvers::Iterations::CadencesResolver
 
-        field :timelogs, ::Types::TimelogType.connection_type, null: false,
-              description: 'Time logged in issues by group members.',
-              extras: [:lookahead],
-              complexity: 5,
-              resolver: ::Resolvers::TimelogResolver
-
         field :vulnerabilities,
               ::Types::VulnerabilityType.connection_type,
               null: true,

ee/app/models/ee/group.rb

@@ -12,7 +12,6 @@ module EE
     prepended do
       include TokenAuthenticatable
       include InsightsFeature
-      include HasTimelogsReport
       include HasWiki
       include CanMoveRepositoryStorage
 

ee/app/models/license.rb

@@ -127,7 +127,6 @@ class License < ApplicationRecord
     scoped_labels
     smartcard_auth
     swimlanes
-    group_timelogs
     type_of_work_analytics
     minimal_access_role
     unprotection_restrictions

ee/app/policies/ee/group_policy.rb

@@ -94,10 +94,6 @@ module EE
         @subject.saml_group_sync_available?
       end
 
-      condition(:group_timelogs_available) do
-        @subject.feature_available?(:group_timelogs)
-      end
-
       with_scope :global
       condition(:commit_committer_check_disabled_globally) do
         !PushRule.global&.commit_committer_check
@@ -152,7 +148,6 @@ module EE
         enable :admin_issue_board_list
         enable :view_productivity_analytics
         enable :view_type_of_work_charts
-        enable :read_group_timelogs
         enable :download_wiki_code
       end
 
@@ -311,8 +306,6 @@ module EE
         enable :read_group_saml_identity
       end
 
-      rule { ~group_timelogs_available }.prevent :read_group_timelogs
-
       rule { ~(admin | allow_to_manage_default_branch_protection) }.policy do
         prevent :update_default_branch_protection
       end

ee/app/policies/ee/project_policy.rb

@@ -164,10 +164,6 @@ module EE
         @subject.feature_available?(:status_page, @user)
       end
 
-      condition(:group_timelogs_available) do
-        @subject.feature_available?(:group_timelogs)
-      end
-
       condition(:over_storage_limit, scope: :subject) do
         @subject.root_namespace.over_storage_limit?
       end
@@ -204,14 +200,11 @@ module EE
         prevent :admin_feature_flags_issue_links
       end
 
-      rule { ~group_timelogs_available }.prevent :read_group_timelogs
-
       rule { can?(:guest_access) & iterations_available }.enable :read_iteration
 
       rule { can?(:reporter_access) }.policy do
         enable :admin_issue_board
         enable :admin_epic_issue
-        enable :read_group_timelogs
       end
 
       rule { oncall_schedules_available & can?(:reporter_access) }.enable :read_incident_management_oncall_schedule

ee/spec/graphql/ee/types/group_type_spec.rb

@@ -13,7 +13,6 @@ RSpec.describe GitlabSchema.types['Group'] do
 
   it { expect(described_class).to have_graphql_field(:iterations) }
   it { expect(described_class).to have_graphql_field(:iteration_cadences) }
-  it { expect(described_class).to have_graphql_field(:groupTimelogsEnabled) }
   it { expect(described_class).to have_graphql_field(:timelogs, complexity: 5) }
   it { expect(described_class).to have_graphql_field(:vulnerabilities) }
   it { expect(described_class).to have_graphql_field(:vulnerability_scanners) }

ee/spec/policies/group_policy_spec.rb

@@ -290,72 +290,6 @@ RSpec.describe GroupPolicy do
     it { is_expected.not_to be_allowed(:read_group_repository_analytics) }
   end
 
-  context 'when timelogs report feature is enabled' do
-    before do
-      stub_licensed_features(group_timelogs: true)
-    end
-
-    context 'admin' do
-      let(:current_user) { admin }
-
-      context 'when admin mode is enabled', :enable_admin_mode do
-        it { is_expected.to be_allowed(:read_group_timelogs) }
-      end
-
-      context 'when admin mode is disabled' do
-        it { is_expected.to be_disallowed(:read_group_timelogs) }
-      end
-    end
-
-    context 'with owner' do
-      let(:current_user) { owner }
-
-      it { is_expected.to be_allowed(:read_group_timelogs) }
-    end
-
-    context 'with maintainer' do
-      let(:current_user) { maintainer }
-
-      it { is_expected.to be_allowed(:read_group_timelogs) }
-    end
-
-    context 'with reporter' do
-      let(:current_user) { reporter }
-
-      it { is_expected.to be_allowed(:read_group_timelogs) }
-    end
-
-    context 'with guest' do
-      let(:current_user) { guest }
-
-      it { is_expected.to be_disallowed(:read_group_timelogs) }
-    end
-
-    context 'with non member' do
-      let(:current_user) { create(:user) }
-
-      it { is_expected.to be_disallowed(:read_group_timelogs) }
-    end
-
-    context 'with anonymous' do
-      let(:current_user) { nil }
-
-      it { is_expected.to be_disallowed(:read_group_timelogs) }
-    end
-  end
-
-  context 'when timelogs report feature is disabled' do
-    let(:current_user) { admin }
-
-    before do
-      stub_licensed_features(group_timelogs: false)
-    end
-
-    it 'is disallowed even with admin mode', :enable_admin_mode do
-      is_expected.to be_disallowed(:read_group_timelogs)
-    end
-  end
-
   describe 'per group SAML' do
     def stub_group_saml_config(enabled)
       allow(::Gitlab::Auth::GroupSaml::Config).to receive_messages(enabled?: enabled)

ee/spec/policies/project_policy_spec.rb

@@ -1251,70 +1251,6 @@ RSpec.describe ProjectPolicy do
     end
   end
 
-  context 'when timelogs report feature is enabled' do
-    before do
-      stub_licensed_features(group_timelogs: true)
-    end
-
-    context 'admin' do
-      let(:current_user) { admin }
-
-      context 'when admin mode enabled', :enable_admin_mode do
-        it { is_expected.to be_allowed(:read_group_timelogs) }
-      end
-
-      context 'when admin mode disabled' do
-        it { is_expected.to be_disallowed(:read_group_timelogs) }
-      end
-    end
-
-    context 'with owner' do
-      let(:current_user) { owner }
-
-      it { is_expected.to be_allowed(:read_group_timelogs) }
-    end
-
-    context 'with maintainer' do
-      let(:current_user) { maintainer }
-
-      it { is_expected.to be_allowed(:read_group_timelogs) }
-    end
-
-    context 'with reporter' do
-      let(:current_user) { reporter }
-
-      it { is_expected.to be_allowed(:read_group_timelogs) }
-    end
-
-    context 'with guest' do
-      let(:current_user) { guest }
-
-      it { is_expected.to be_disallowed(:read_group_timelogs) }
-    end
-
-    context 'with non member' do
-      let(:current_user) { non_member }
-
-      it { is_expected.to be_disallowed(:read_group_timelogs) }
-    end
-
-    context 'with anonymous' do
-      let(:current_user) { anonymous }
-
-      it { is_expected.to be_disallowed(:read_group_timelogs) }
-    end
-  end
-
-  context 'when timelogs report feature is disabled' do
-    let(:current_user) { admin }
-
-    before do
-      stub_licensed_features(group_timelogs: false)
-    end
-
-    it { is_expected.to be_disallowed(:read_group_timelogs) }
-  end
-
   context 'when dora4 analytics is available' do
     let(:current_user) { developer }
 

ee/spec/graphql/resolvers/timelog_resolver_spec.rb → spec/graphql/resolvers/timelog_resolver_spec.rb

@@ -17,7 +17,6 @@ RSpec.describe Resolvers::TimelogResolver do
     before do
       group.add_developer(current_user)
       project.add_developer(current_user)
-      stub_licensed_features(group_timelogs: true)
     end
 
     describe '#resolve' do
@@ -40,12 +39,6 @@ RSpec.describe Resolvers::TimelogResolver do
         expect(resolve_timelogs(args)).to be_empty
       end
 
-      it 'returns nothing when feature is disabled' do
-        stub_licensed_features(group_timelogs: false)
-
-        expect(resolve_timelogs(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 } }
 

ee/spec/models/concerns/has_timelogs_report_spec.rb → spec/models/concerns/has_timelogs_report_spec.rb

@@ -34,19 +34,12 @@ RSpec.describe HasTimelogsReport do
   describe '#user_can_access_group_timelogs?' do
     before do
       group.add_developer(user)
-      stub_licensed_features(group_timelogs: true)
     end
 
     it 'returns true if user can access group timelogs' do
       expect(group.user_can_access_group_timelogs?(user)).to be_truthy
     end
 
-    it 'returns false if feature group_timelogs is disabled' do
-      stub_licensed_features(group_timelogs: false)
-
-      expect(group.user_can_access_group_timelogs?(user)).to be_falsey
-    end
-
     it 'returns false if user has insufficient permissions' do
       group.add_guest(user)
 

spec/policies/group_policy_spec.rb

@@ -922,4 +922,54 @@ RSpec.describe GroupPolicy do
       it { expect(described_class.new(current_user, subgroup)).to be_allowed(:read_label) }
     end
   end
+
+  context 'timelogs' do
+    context 'with admin' do
+      let(:current_user) { admin }
+
+      context 'when admin mode is enabled', :enable_admin_mode do
+        it { is_expected.to be_allowed(:read_group_timelogs) }
+      end
+
+      context 'when admin mode is disabled' do
+        it { is_expected.to be_disallowed(:read_group_timelogs) }
+      end
+    end
+
+    context 'with owner' do
+      let(:current_user) { owner }
+
+      it { is_expected.to be_allowed(:read_group_timelogs) }
+    end
+
+    context 'with maintainer' do
+      let(:current_user) { maintainer }
+
+      it { is_expected.to be_allowed(:read_group_timelogs) }
+    end
+
+    context 'with reporter' do
+      let(:current_user) { reporter }
+
+      it { is_expected.to be_allowed(:read_group_timelogs) }
+    end
+
+    context 'with guest' do
+      let(:current_user) { guest }
+
+      it { is_expected.to be_disallowed(:read_group_timelogs) }
+    end
+
+    context 'with non member' do
+      let(:current_user) { create(:user) }
+
+      it { is_expected.to be_disallowed(:read_group_timelogs) }
+    end
+
+    context 'with anonymous' do
+      let(:current_user) { nil }
+
+      it { is_expected.to be_disallowed(:read_group_timelogs) }
+    end
+  end
 end

spec/policies/project_policy_spec.rb

@@ -1353,4 +1353,54 @@ RSpec.describe ProjectPolicy do
       end
     end
   end
+
+  context 'timelogs' do
+    context 'with admin' do
+      let(:current_user) { admin }
+
+      context 'when admin mode enabled', :enable_admin_mode do
+        it { is_expected.to be_allowed(:read_group_timelogs) }
+      end
+
+      context 'when admin mode disabled' do
+        it { is_expected.to be_disallowed(:read_group_timelogs) }
+      end
+    end
+
+    context 'with owner' do
+      let(:current_user) { owner }
+
+      it { is_expected.to be_allowed(:read_group_timelogs) }
+    end
+
+    context 'with maintainer' do
+      let(:current_user) { maintainer }
+
+      it { is_expected.to be_allowed(:read_group_timelogs) }
+    end
+
+    context 'with reporter' do
+      let(:current_user) { reporter }
+
+      it { is_expected.to be_allowed(:read_group_timelogs) }
+    end
+
+    context 'with guest' do
+      let(:current_user) { guest }
+
+      it { is_expected.to be_disallowed(:read_group_timelogs) }
+    end
+
+    context 'with non member' do
+      let(:current_user) { non_member }
+
+      it { is_expected.to be_disallowed(:read_group_timelogs) }
+    end
+
+    context 'with anonymous' do
+      let(:current_user) { anonymous }
+
+      it { is_expected.to be_disallowed(:read_group_timelogs) }
+    end
+  end
 end

spec/requests/api/graphql/custom_emoji_query_spec.rb

@@ -16,7 +16,15 @@ RSpec.describe 'getting custom emoji within namespace' do
 
   describe "Query CustomEmoji on Group" do
     def custom_emoji_query(group)
-      graphql_query_for('group', 'fullPath' => group.full_path)
+      fields = all_graphql_fields_for('Group')
+      # TODO: Set required timelogs args elsewhere https://gitlab.com/gitlab-org/gitlab/-/issues/325499
+      fields.selection['timelogs(startDate: "2021-03-01" endDate: "2021-03-30")'] = fields.selection.delete('timelogs')
+
+      graphql_query_for(
+        'group',
+        { fullPath: group.full_path },
+        fields
+      )
     end
 
     it 'returns emojis when authorised' do

ee/spec/requests/api/graphql/group/timelogs_spec.rb → spec/requests/api/graphql/group/timelogs_spec.rb

@@ -10,8 +10,7 @@ RSpec.describe 'Timelogs through GroupQuery' do
     let_it_be(:group)     { create(:group) }
     let_it_be(:project)   { create(:project, :public, group: group) }
     let_it_be(:milestone) { create(:milestone, group: group) }
-    let_it_be(:epic)      { create(:epic, group: group) }
-    let_it_be(:issue)     { create(:issue, project: project, milestone: milestone, epic: epic) }
+    let_it_be(:issue)     { create(:issue, project: project, milestone: milestone) }
     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' } }
@@ -19,7 +18,6 @@ RSpec.describe 'Timelogs through GroupQuery' do
 
     before do
       group.add_developer(user)
-      stub_licensed_features(group_timelogs: true, epics: true)
     end
 
     context 'when the request is correct' do
@@ -33,7 +31,6 @@ RSpec.describe 'Timelogs through GroupQuery' do
         expect(response).to have_gitlab_http_status(:ok)
         expect(graphql_errors).to be_nil
         expect(timelog_array.size).to eq 1
-        expect(graphql_data['group']['groupTimelogsEnabled']).to be_truthy
       end
 
       it 'contains correct data', :aggregate_failures do
@@ -42,14 +39,12 @@ RSpec.describe 'Timelogs through GroupQuery' do
         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'] }
-        epic_title = timelog_array.map {|data| data['issue']['epic']['title'] }
 
         expect(username).to eq([user.username])
         expect(spent_at.first).to be_like_time(timelog1.spent_at)
         expect(time_spent).to eq([timelog1.time_spent])
         expect(issue_title).to eq([issue.title])
         expect(milestone_title).to eq([milestone.title])
-        expect(epic_title).to eq([epic.title])
       end
 
       context 'when arguments with no time are present' do
@@ -65,21 +60,6 @@ RSpec.describe 'Timelogs through GroupQuery' do
     end
 
     context 'when requests has errors' do
-      context 'when group_timelogs feature is disabled' do
-        before do
-          stub_licensed_features(group_timelogs: false)
-        end
-
-        it 'returns empty' do
-          post_graphql(query, current_user: user)
-
-          expect(response).to have_gitlab_http_status(:success)
-          expect(graphql_errors).to be_nil
-          expect(timelogs_data).to be_empty
-          expect(graphql_data['group']['groupTimelogsEnabled']).to be_falsey
-        end
-      end
-
       context 'when there are no timelogs present' do
         before do
           Timelog.delete_all
@@ -91,7 +71,6 @@ RSpec.describe 'Timelogs through GroupQuery' do
           expect(response).to have_gitlab_http_status(:success)
           expect(graphql_errors).to be_nil
           expect(timelogs_data).to be_empty
-          expect(graphql_data['group']['groupTimelogsEnabled']).to be_truthy
         end
       end
 
@@ -104,7 +83,6 @@ RSpec.describe 'Timelogs through GroupQuery' do
           expect(response).to have_gitlab_http_status(:success)
           expect(graphql_errors).to be_nil
           expect(timelogs_data).to be_empty
-          expect(graphql_data['group']['groupTimelogsEnabled']).to be_truthy
         end
       end
     end
@@ -129,15 +107,12 @@ RSpec.describe 'Timelogs through GroupQuery' do
           milestone {
             title
           }
-          epic {
-            title
-          }
         }
       }
     NODE
 
     graphql_query_for("group", { "fullPath" => group.full_path },
-      ['groupTimelogsEnabled', query_graphql_field(
+      [query_graphql_field(
         "timelogs",
         timelog_params,
         timelog_nodes

spec/requests/api/graphql/group_query_spec.rb

@@ -17,7 +17,15 @@ RSpec.describe 'getting group information' do
   # similar to the API "GET /groups/:id"
   describe "Query group(fullPath)" do
     def group_query(group)
-      graphql_query_for('group', 'fullPath' => group.full_path)
+      fields = all_graphql_fields_for('Group')
+      # TODO: Set required timelogs args elsewhere https://gitlab.com/gitlab-org/gitlab/-/issues/325499
+      fields.selection['timelogs(startDate: "2021-03-01" endDate: "2021-03-30")'] = fields.selection.delete('timelogs')
+
+      graphql_query_for(
+        'group',
+        { fullPath: group.full_path },
+        fields
+      )
     end
 
     it_behaves_like 'a working graphql query' do