From a1c77c597a8badc37b5ab30fa4193492de4d54c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= <fcaplette@gitlab.com> Date: Wed, 24 Jul 2024 19:25:30 +0000 Subject: [PATCH] Fix AI comments summary streaming Use the new sendDuoChatCommand util in ai summary feature to fix loading and streaming. This is behind a FF `summarize_notes_with_duo` --- .../note_actions/ai_summarize_notes.vue | 63 ++++++++----------- .../note_actions/ai_summarize_notes_spec.js | 57 ++--------------- 2 files changed, 33 insertions(+), 87 deletions(-) diff --git a/ee/app/assets/javascripts/notes/components/note_actions/ai_summarize_notes.vue b/ee/app/assets/javascripts/notes/components/note_actions/ai_summarize_notes.vue index 0c1f6f3c8f46..ec62cb344771 100644 --- a/ee/app/assets/javascripts/notes/components/note_actions/ai_summarize_notes.vue +++ b/ee/app/assets/javascripts/notes/components/note_actions/ai_summarize_notes.vue @@ -3,11 +3,10 @@ import { GlButton, GlTooltipDirective } from '@gitlab/ui'; import { s__, __ } from '~/locale'; import { createAlert } from '~/alert'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import chatMutation from 'ee/ai/graphql/chat.mutation.graphql'; +import { sendDuoChatCommand } from 'ee/ai/utils'; import aiActionMutation from 'ee/graphql_shared/mutations/ai_action.mutation.graphql'; import { MAX_REQUEST_TIMEOUT } from 'ee/notes/constants'; import { BV_HIDE_TOOLTIP } from '~/lib/utils/constants'; -import { duoChatGlobalState } from '~/super_sidebar/constants'; export default { components: { @@ -48,50 +47,42 @@ export default { return; } - let mutation; - let variables; - if (this.glFeatures.summarizeNotesWithDuo) { - mutation = chatMutation; - variables = { + sendDuoChatCommand({ question: '/summarize_comments', resourceId: this.resourceGlobalId, - }; - duoChatGlobalState.isShown = true; + }); } else { - mutation = aiActionMutation; - variables = { - input: { - summarizeComments: { resourceId: this.resourceGlobalId }, - clientSubscriptionId: this.summarizeClientSubscriptionId, - }, - }; this.$parent.$emit('set-ai-loading', true); - } - this.errorAlert?.dismiss(); + this.errorAlert?.dismiss(); + this.timeout = window.setTimeout(this.handleError, MAX_REQUEST_TIMEOUT); - this.timeout = window.setTimeout(this.handleError, MAX_REQUEST_TIMEOUT); + this.$apollo + .mutate({ + mutation: aiActionMutation, + variables: { + input: { + summarizeComments: { resourceId: this.resourceGlobalId }, + clientSubscriptionId: this.summarizeClientSubscriptionId, + }, + }, + }) + .then(({ data: { aiAction } }) => { + const errors = aiAction?.errors || []; - this.$apollo - .mutate({ - mutation, - variables, - }) - .then(({ data: { aiAction } }) => { - const errors = aiAction?.errors || []; + // in some cases the tooltip can get stuck + // open when clicking the button, so hide again just in case + this.hideTooltips(); - // in some cases the tooltip can get stuck - // open when clicking the button, so hide again just in case - this.hideTooltips(); + if (errors[0]) { + this.handleError(new Error(errors[0])); + } - if (errors[0]) { - this.handleError(new Error(errors[0])); - } - - clearTimeout(this.timeout); - }) - .catch(this.handleError); + clearTimeout(this.timeout); + }) + .catch(this.handleError); + } }, hideTooltips() { this.$nextTick(() => { diff --git a/ee/spec/frontend/notes/components/note_actions/ai_summarize_notes_spec.js b/ee/spec/frontend/notes/components/note_actions/ai_summarize_notes_spec.js index 3b8c51e9b181..a69668e0691a 100644 --- a/ee/spec/frontend/notes/components/note_actions/ai_summarize_notes_spec.js +++ b/ee/spec/frontend/notes/components/note_actions/ai_summarize_notes_spec.js @@ -4,21 +4,20 @@ import { GlButton } from '@gitlab/ui'; import { BV_HIDE_TOOLTIP } from '~/lib/utils/constants'; import createMockApollo from 'helpers/mock_apollo_helper'; import { mountExtended } from 'helpers/vue_test_utils_helper'; +import * as aiUtils from 'ee/ai/utils'; import AiSummaryNotes from 'ee/notes/components/note_actions/ai_summarize_notes.vue'; -import chatMutation from 'ee/ai/graphql/chat.mutation.graphql'; import aiActionMutation from 'ee/graphql_shared/mutations/ai_action.mutation.graphql'; import waitForPromises from 'helpers/wait_for_promises'; import { createAlert } from '~/alert'; -import { duoChatGlobalState } from '~/super_sidebar/constants'; -import { MOCK_TANUKI_BOT_MUTATATION_RES } from 'ee_jest/ai/tanuki_bot/mock_data'; Vue.use(VueApollo); jest.mock('~/alert'); +jest.mock('ee/ai/utils'); +jest.spyOn(aiUtils, 'sendDuoChatCommand'); describe('AiSummarizeNotes component', () => { let wrapper; let aiActionMutationHandler; - let chatMutationHandler; const resourceGlobalId = 'gid://gitlab/Issue/1'; const clientSubscriptionId = 'someId'; const LONGER_THAN_MAX_REQUEST_TIMEOUT = 1000 * 20; // 20 seconds @@ -26,13 +25,9 @@ describe('AiSummarizeNotes component', () => { const findButton = () => wrapper.findComponent(GlButton); const createWrapper = (featureFlagEnabled = false) => { - chatMutationHandler = jest.fn().mockResolvedValue(MOCK_TANUKI_BOT_MUTATATION_RES); aiActionMutationHandler = jest.fn(); - const mockApollo = createMockApollo([ - [chatMutation, chatMutationHandler], - [aiActionMutation, aiActionMutationHandler], - ]); + const mockApollo = createMockApollo([[aiActionMutation, aiActionMutationHandler]]); wrapper = mountExtended(AiSummaryNotes, { apolloProvider: mockApollo, @@ -53,22 +48,13 @@ describe('AiSummarizeNotes component', () => { describe('with summarizeNotesWithDuo feature flag enabled', () => { describe('on click', () => { - it('opens duochat', async () => { + it('calls sendDuoChatCommand with correct variables', async () => { createWrapper(true); await findButton().trigger('click'); await nextTick(); - expect(duoChatGlobalState.isShown).toBe(true); - }); - - it('calls mutation', async () => { - createWrapper(true); - - await findButton().trigger('click'); - await nextTick(); - - expect(chatMutationHandler).toHaveBeenCalledWith({ + expect(aiUtils.sendDuoChatCommand).toHaveBeenCalledWith({ question: '/summarize_comments', resourceId: resourceGlobalId, }); @@ -85,37 +71,6 @@ describe('AiSummarizeNotes component', () => { expect(bsTooltipHide).toHaveBeenCalled(); }); - - describe('unsuccessful mutation', () => { - beforeEach(() => { - createWrapper(true); - chatMutationHandler.mockResolvedValue({ - data: { aiAction: { errors: ['GraphQL Error'], requestId: '123' } }, - }); - findButton().trigger('click'); - }); - - it('shows error if no response within timeout limit', async () => { - jest.advanceTimersByTime(LONGER_THAN_MAX_REQUEST_TIMEOUT); - await nextTick(); - - expect(createAlert).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'Something went wrong', - }), - ); - }); - - it('shows error on error response', async () => { - await waitForPromises(); - - expect(createAlert).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'GraphQL Error', - }), - ); - }); - }); }); }); -- GitLab