diff --git a/app/assets/javascripts/vue_shared/components/markdown/header.vue b/app/assets/javascripts/vue_shared/components/markdown/header.vue index 4e827d71126f80bd00592025352e5f279e976482..3486f231b398ffe2d200413a7a15b7f0c81f261b 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/header.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/header.vue @@ -122,12 +122,7 @@ export default { return [`<details><summary>${expandText}</summary>`, `{text}`, '</details>'].join('\n'); }, showAiActions() { - return ( - this.resourceGlobalId && - this.glFeatures.openaiExperimentation && - this.glFeatures.summarizeNotes && - this.glFeatures.summarizeComments - ); + return this.resourceGlobalId && this.glFeatures.summarizeComments; }, }, watch: { diff --git a/ee/app/assets/javascripts/vue_shared/components/markdown/ai_actions_dropdown.vue b/ee/app/assets/javascripts/vue_shared/components/markdown/ai_actions_dropdown.vue index 67c3ed6ea399da478e9ce1467484d3b29ce7ebae..5455048572fe8b6abebd7fb6a64026eb331a533e 100644 --- a/ee/app/assets/javascripts/vue_shared/components/markdown/ai_actions_dropdown.vue +++ b/ee/app/assets/javascripts/vue_shared/components/markdown/ai_actions_dropdown.vue @@ -111,7 +111,7 @@ export default { .mutate({ mutation: aiActionMutation, variables: { input } }) .then(({ data: { aiAction } }) => { if (aiAction.errors.length > 0) { - this.handleError(); + this.handleError(new Error(aiAction.errors)); return; } this.$apollo.subscriptions.aiCompletionResponse.start(); @@ -130,8 +130,12 @@ export default { }, handleError(error) { const alertOptions = error ? { captureError: true, error } : {}; - this.errorAlert = createAlert({ message: __('Something went wrong'), ...alertOptions }); + this.errorAlert = createAlert({ + message: error ? error.message : __('Something went wrong'), + ...alertOptions, + }); this.loading = false; + clearTimeout(this.timeout); }, }, availableActions: [ diff --git a/ee/app/controllers/ee/projects/issues_controller.rb b/ee/app/controllers/ee/projects/issues_controller.rb index 35a61b573cc8f11df6935412e383c75ac471e90b..a9d920685b9fe16419a7875e386d1f0ae6a2420d 100644 --- a/ee/app/controllers/ee/projects/issues_controller.rb +++ b/ee/app/controllers/ee/projects/issues_controller.rb @@ -19,12 +19,7 @@ module IssuesController before_action only: :show do push_licensed_feature(:escalation_policies, project) push_frontend_feature_flag(:real_time_issue_weight, project) - - if project.licensed_feature_available?(:summarize_notes) && project.member?(current_user) - push_licensed_feature(:summarize_notes, project) - push_frontend_feature_flag(:summarize_comments, project.group) - push_frontend_feature_flag(:openai_experimentation) - end + push_force_frontend_feature_flag(:summarize_comments, can?(current_user, :summarize_notes, issue)) end before_action :redirect_if_test_case, only: [:show] diff --git a/ee/app/controllers/groups/epics_controller.rb b/ee/app/controllers/groups/epics_controller.rb index a355a7050581bb3299951fb5775b26e4cb4538d6..16551bcd7e959cd08e4e41b1ba3cc91baa1b9c0e 100644 --- a/ee/app/controllers/groups/epics_controller.rb +++ b/ee/app/controllers/groups/epics_controller.rb @@ -14,6 +14,7 @@ class Groups::EpicsController < Groups::ApplicationController before_action :authorize_update_issuable!, only: :update before_action :authorize_create_epic!, only: [:create, :new] before_action :verify_group_bulk_edit_enabled!, only: [:bulk_update] + before_action :set_summarize_notes_feature_flag, only: :show after_action :log_epic_show, only: :show before_action do @@ -22,9 +23,6 @@ class Groups::EpicsController < Groups::ApplicationController push_frontend_feature_flag(:content_editor_on_issues, @group) push_frontend_feature_flag(:or_issuable_queries, @group) push_frontend_feature_flag(:saved_replies, current_user) - push_frontend_feature_flag(:summarize_comments, current_user) - push_licensed_feature(:summarize_notes, group) if group.licensed_feature_available?(:summarize_notes) - push_frontend_feature_flag(:openai_experimentation, current_user) end feature_category :portfolio_management @@ -119,4 +117,8 @@ def authorize_create_epic! def verify_group_bulk_edit_enabled! render_404 unless group.licensed_feature_available?(:group_bulk_edit) end + + def set_summarize_notes_feature_flag + push_force_frontend_feature_flag(:summarize_comments, can?(current_user, :summarize_notes, epic)) + end end diff --git a/ee/app/policies/ee/issue_policy.rb b/ee/app/policies/ee/issue_policy.rb index 765c5b33aeea032611f66308838f8ca7a2f1b6fd..16b96d83c8ea0625f169ad7299d02045c974ab35 100644 --- a/ee/app/policies/ee/issue_policy.rb +++ b/ee/app/policies/ee/issue_policy.rb @@ -5,9 +5,24 @@ module IssuePolicy extend ActiveSupport::Concern prepended do + with_scope :subject + condition(:ai_available) do + ::Feature.enabled?(:openai_experimentation) && @subject.send_to_ai? + end + + with_scope :subject + condition(:summarize_notes_enabled) do + ::Feature.enabled?(:summarize_comments, subject_container) && + subject_container.licensed_feature_available?(:summarize_notes) + end + rule { can_be_promoted_to_epic }.policy do enable :promote_to_epic end + + rule do + ai_available & summarize_notes_enabled & is_project_member & can?(:read_issue) & ~confidential + end.enable :summarize_notes end end end diff --git a/ee/app/policies/epic_policy.rb b/ee/app/policies/epic_policy.rb index a39017ce126522097d07487007a9086553bdf6bc..89d5e9bf0a67bb5593aab1a8e3af5040ad8b41fd 100644 --- a/ee/app/policies/epic_policy.rb +++ b/ee/app/policies/epic_policy.rb @@ -22,6 +22,19 @@ class EpicPolicy < BasePolicy @subject.group.licensed_feature_available?(:subepics) end + condition(:is_member) do + @user && @subject.group.member?(@user) + end + + condition(:ai_available, scope: :subject) do + ::Feature.enabled?(:openai_experimentation) && @subject.send_to_ai? + end + + condition(:summarize_notes_enabled, scope: :subject) do + ::Feature.enabled?(:summarize_comments, @subject.group) && + @subject.group.licensed_feature_available?(:summarize_notes) + end + rule { can?(:read_epic) }.policy do enable :read_epic_iid enable :read_note @@ -91,4 +104,8 @@ class EpicPolicy < BasePolicy rule { can?(:reporter_access) }.policy do enable :mark_note_as_internal end + + rule { ai_available & summarize_notes_enabled & is_member & ~confidential }.policy do + enable :summarize_notes + end end diff --git a/ee/app/services/llm/generate_summary_service.rb b/ee/app/services/llm/generate_summary_service.rb index d2bb3d1515172ab5a0598021d9a85f44fb726f26..9e76175b5d09c16688e5d2f7e1f37d2841a3e5b6 100644 --- a/ee/app/services/llm/generate_summary_service.rb +++ b/ee/app/services/llm/generate_summary_service.rb @@ -2,7 +2,7 @@ module Llm class GenerateSummaryService < BaseService - SUPPORTED_ISSUABLE_TYPES = %w[issue work_item merge_request epic].freeze + SUPPORTED_ISSUABLE_TYPES = %w[issue epic].freeze private @@ -14,8 +14,7 @@ def perform def valid? super && SUPPORTED_ISSUABLE_TYPES.include?(resource.to_ability_name) && - Feature.enabled?(:summarize_comments, resource.resource_parent) && - resource.resource_parent.root_ancestor.licensed_feature_available?(:summarize_notes) && + Ability.allowed?(user, :summarize_notes, resource) && !resource.notes.for_summarize_by_ai.empty? end end diff --git a/ee/spec/frontend/vue_shared/components/markdown/ai_actions_dropdown_spec.js b/ee/spec/frontend/vue_shared/components/markdown/ai_actions_dropdown_spec.js index 38533f30c9a25940bfca96d5ab2f461852e191af..b69069ced4a8c7cb1bd42498ca88f2c540b44749 100644 --- a/ee/spec/frontend/vue_shared/components/markdown/ai_actions_dropdown_spec.js +++ b/ee/spec/frontend/vue_shared/components/markdown/ai_actions_dropdown_spec.js @@ -142,8 +142,14 @@ describe('AI actions dropdown component', () => { }); it('shows an error when the AI action mutation response contains errors', async () => { + const errors = ['oh no', 'it didnt do the thing', 'zzzeezoo']; + aiActionMutationHandler.mockResolvedValue({ - errors: ['oh no', 'it didnt do the thing', 'zzzeezoo'], + data: { + aiAction: { + errors, + }, + }, }); await selectSummariseComments(); @@ -151,7 +157,9 @@ describe('AI actions dropdown component', () => { expect(createAlert).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Something went wrong', + message: errors.join(','), + captureError: true, + error: expect.any(Error), }), ); }); @@ -165,7 +173,7 @@ describe('AI actions dropdown component', () => { expect(createAlert).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Something went wrong', + message: 'ding', captureError: true, error: mockError, }), @@ -179,7 +187,7 @@ describe('AI actions dropdown component', () => { expect(createAlert).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Something went wrong', + message: 'ding', captureError: true, error: mockError, }), diff --git a/ee/spec/policies/epic_policy_spec.rb b/ee/spec/policies/epic_policy_spec.rb index d080e6a4c21ffd8f840123cc41f1c53a904029c2..1602148903c5a80d8e5919de6b0d6ea031712cf3 100644 --- a/ee/spec/policies/epic_policy_spec.rb +++ b/ee/spec/policies/epic_policy_spec.rb @@ -369,4 +369,79 @@ end end end + + describe 'summarize_notes' do + let_it_be(:group) { create(:group, :private) } + + before do + stub_licensed_features(summarize_notes: true) + stub_feature_flags(summarize_comments: group) + end + + context 'when a member' do + before do + group.add_guest(user) + end + + context 'on a public group' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it { is_expected.to be_allowed(:summarize_notes) } + + context 'when license is not set' do + before do + stub_licensed_features(summarize_notes: false) + end + + it { is_expected.to be_disallowed(:summarize_notes) } + end + + context 'when feature flag is not set' do + before do + stub_feature_flags(summarize_comments: false) + end + + it { is_expected.to be_disallowed(:summarize_notes) } + end + + context 'on confidential epic' do + let_it_be(:epic) { create(:epic, :confidential, group: group) } + + it { is_expected.to be_disallowed(:summarize_notes) } + end + end + + context 'on a private group' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it { is_expected.to be_disallowed(:summarize_notes) } + end + + context 'on confidential epic' do + let_it_be(:epic) { create(:epic, :confidential, group: group) } + + it { is_expected.to be_disallowed(:summarize_notes) } + end + end + + context 'when not a member' do + let_it_be(:user) { create(:user) } + + context 'on a public group' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it { is_expected.to be_disallowed(:summarize_notes) } + end + + context 'on a private group' do + it { is_expected.to be_disallowed(:summarize_notes) } + end + end + end end diff --git a/ee/spec/policies/issue_policy_spec.rb b/ee/spec/policies/issue_policy_spec.rb index c10547cfc01e6d42128657d9e4b7d85ff0116db1..cbb0b620c23c8b37a8342b2d742aab243b82a50f 100644 --- a/ee/spec/policies/issue_policy_spec.rb +++ b/ee/spec/policies/issue_policy_spec.rb @@ -7,8 +7,9 @@ let_it_be(:namespace) { create(:group) } let_it_be(:project) { create(:project, group: namespace) } let_it_be(:issue) { create(:issue, project: project) } + let(:user) { owner } - subject { described_class.new(owner, issue) } + subject { described_class.new(user, issue) } before do namespace.add_owner(owner) @@ -19,4 +20,69 @@ end it { is_expected.to be_allowed(:create_issue, :update_issue, :read_issue_iid, :reopen_issue, :create_design, :create_note) } + + describe 'summarize_notes' do + before do + stub_licensed_features(summarize_notes: true) + stub_feature_flags(summarize_comments: project) + end + + context 'when a member' do + context 'on a public project' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it { is_expected.to be_allowed(:summarize_notes) } + + context 'when license is not set' do + before do + stub_licensed_features(summarize_notes: false) + end + + it { is_expected.to be_disallowed(:summarize_notes) } + end + + context 'when feature flag is not set' do + before do + stub_feature_flags(summarize_comments: false) + end + + it { is_expected.to be_disallowed(:summarize_notes) } + end + + context 'on confidential issue' do + let_it_be(:issue) { create(:issue, :confidential, project: project) } + + it { is_expected.to be_disallowed(:summarize_notes) } + end + end + + context 'on a private project' do + let_it_be(:project) { create(:project, :private) } + + it { is_expected.to be_disallowed(:summarize_notes) } + end + + context 'on confidential issue' do + let_it_be(:issue) { create(:issue, :confidential, project: project) } + + it { is_expected.to be_disallowed(:summarize_notes) } + end + end + + context 'when not a member' do + let_it_be(:user) { create(:user) } + + context 'on a public project' do + let_it_be(:project) { create(:project, :public) } + + it { is_expected.to be_disallowed(:summarize_notes) } + end + + context 'on a private project' do + it { is_expected.to be_disallowed(:summarize_notes) } + end + end + end end diff --git a/ee/spec/requests/groups/epics_controller_spec.rb b/ee/spec/requests/groups/epics_controller_spec.rb index 3065dcf144b0ad9844706c5e42e2fcc1d0347070..3f5931691d7d12bf201ad48fc4d510514af5e936 100644 --- a/ee/spec/requests/groups/epics_controller_spec.rb +++ b/ee/spec/requests/groups/epics_controller_spec.rb @@ -7,13 +7,13 @@ let(:epic) { create(:epic, group: group) } let(:user) { create(:user) } - before do - stub_licensed_features(epics: true) - sign_in(user) - group.add_developer(user) - end - describe 'GET #new' do + before do + stub_licensed_features(epics: true) + sign_in(user) + group.add_developer(user) + end + it_behaves_like "observability csp policy", described_class do let(:tested_path) do new_group_epic_path(group) @@ -22,10 +22,60 @@ end describe 'GET #show' do + before do + sign_in(user) + end + it_behaves_like "observability csp policy", described_class do + before do + group.add_developer(user) + stub_licensed_features(epics: true) + end + let(:tested_path) do group_epic_path(group, epic) end end + + context 'for summarize notes feature' do + let(:summarize_notes_enabled) { true } + let(:group) { create(:group, :public) } + + before do + stub_licensed_features(epics: true, summarize_notes: summarize_notes_enabled) + end + + context 'when user is a member' do + before do + group.add_developer(user) + end + + context 'when license is set' do + it 'exposes the required feature flags' do + get group_epic_path(group, epic) + + expect(response.body).to have_pushed_frontend_feature_flags(summarizeComments: true) + end + end + + context 'when license is not set' do + let(:summarize_notes_enabled) { false } + + it 'does not expose the feature flags' do + get group_epic_path(group, epic) + + expect(response.body).not_to have_pushed_frontend_feature_flags(summarizeComments: true) + end + end + end + + context 'when user is not a member' do + it 'does not expose the feature flags' do + get group_epic_path(group, epic) + + expect(response.body).not_to have_pushed_frontend_feature_flags(summarizeComments: true) + end + end + end end end diff --git a/ee/spec/requests/projects/issues_controller_spec.rb b/ee/spec/requests/projects/issues_controller_spec.rb index 68d859ed596910b2fa4ed9e980e0578a046385a6..5278af3faf59ea274f44f404c68453eb9f74f3e2 100644 --- a/ee/spec/requests/projects/issues_controller_spec.rb +++ b/ee/spec/requests/projects/issues_controller_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } + let_it_be(:project) { create(:project, :public, group: group) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:user) { issue.author } let_it_be(:blocking_issue) { create(:issue, project: project) } @@ -85,8 +85,6 @@ def get_show get_show expect(response.body).to have_pushed_frontend_feature_flags(summarizeComments: true) - expect(response.body).to have_pushed_frontend_feature_flags(summarizeNotes: true) - expect(response.body).to have_pushed_frontend_feature_flags(openaiExperimentation: true) end end @@ -95,8 +93,6 @@ def get_show get_show expect(response.body).not_to have_pushed_frontend_feature_flags(summarizeComments: true) - expect(response.body).not_to have_pushed_frontend_feature_flags(summarizeNotes: true) - expect(response.body).not_to have_pushed_frontend_feature_flags(openaiExperimentation: true) end end end @@ -110,8 +106,6 @@ def get_show get_show expect(response.body).not_to have_pushed_frontend_feature_flags(summarizeComments: true) - expect(response.body).not_to have_pushed_frontend_feature_flags(summarizeNotes: true) - expect(response.body).not_to have_pushed_frontend_feature_flags(openaiExperimentation: true) end end end diff --git a/ee/spec/services/llm/generate_summary_service_spec.rb b/ee/spec/services/llm/generate_summary_service_spec.rb index ae1efb7ea217765b0eaa1dbe7465099887bac14f..8308a6f444ae55028afc58f8c24bff0dd0892707 100644 --- a/ee/spec/services/llm/generate_summary_service_spec.rb +++ b/ee/spec/services/llm/generate_summary_service_spec.rb @@ -65,22 +65,6 @@ end end - context 'for a merge request' do - let_it_be(:resource) { create(:merge_request, source_project: project) } - - it_behaves_like "issuable without notes" - - context 'with notes' do - before do - create_pair(:note_on_merge_request, project: resource.project, noteable: resource) - end - - it_behaves_like "issuable with notes" - it_behaves_like "ensures feature flags and license" - it_behaves_like "ensures user membership" - end - end - context 'for an issue' do let_it_be(:resource) { create(:issue, project: project) }