diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index dbb22dcf97b3f0f019c7623978aeadbdccffdcc4..bf2692e5987412b18112410f8a44f0eef0ee2c5d 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -48633,10 +48633,9 @@ definitions: description: Only allow to merge if all threads are resolved enabled_foundational_flows: type: array - description: IDs of enabled foundational flows + description: References of enabled foundational flows items: - type: integer - format: int32 + type: string description: Update a group. Available only for users who can administrate groups. API_Entities_GroupDetail: type: object diff --git a/ee/app/assets/javascripts/ai/settings/components/ai_common_settings_form.vue b/ee/app/assets/javascripts/ai/settings/components/ai_common_settings_form.vue index f28ae7b1edebd98576a7092e82cdf523e650be93..9a024f2793760becefc01c8d3d69dbafb4a4b108 100644 --- a/ee/app/assets/javascripts/ai/settings/components/ai_common_settings_form.vue +++ b/ee/app/assets/javascripts/ai/settings/components/ai_common_settings_form.vue @@ -172,8 +172,8 @@ export default { return this.availability === AVAILABILITY_OPTIONS.NEVER_ON; }, hasSelectedFlowIdsChanged() { - const current = (this.localSelectedFlowIds || []).slice().sort((a, b) => a - b); - const initial = (this.selectedFoundationalFlowIds || []).slice().sort((a, b) => a - b); + const current = (this.localSelectedFlowIds || []).slice().sort(); + const initial = (this.selectedFoundationalFlowIds || []).slice().sort(); return JSON.stringify(current) !== JSON.stringify(initial); }, diff --git a/ee/app/assets/javascripts/ai/settings/components/foundational_flow_selector.vue b/ee/app/assets/javascripts/ai/settings/components/foundational_flow_selector.vue index 81457ebd9793eb06562b87970191583905ec3bc8..2c840e20fa98914848e08bdb1e3f4fe0016effb2 100644 --- a/ee/app/assets/javascripts/ai/settings/components/foundational_flow_selector.vue +++ b/ee/app/assets/javascripts/ai/settings/components/foundational_flow_selector.vue @@ -20,13 +20,13 @@ export default { }, emits: ['input'], methods: { - isFlowSelected(catalogItemId) { - return this.value.includes(catalogItemId); + isFlowSelected(reference) { + return this.value.includes(reference); }, - toggleFlow(catalogItemId, checked) { + toggleFlow(reference, checked) { const newSelection = checked - ? [...this.value, catalogItemId] - : this.value.filter((id) => id !== catalogItemId); + ? [...this.value, reference] + : this.value.filter((ref) => ref !== reference); this.$emit('input', newSelection); }, @@ -38,11 +38,11 @@ export default {
{{ flow.name }} diff --git a/ee/app/assets/javascripts/ai/settings/index.js b/ee/app/assets/javascripts/ai/settings/index.js index 8b05c5e7ba024c9e447757cf938ce07399c50c5a..1ef93bb76aaa1063c9cae8b9bd5080cffe5ae609 100644 --- a/ee/app/assets/javascripts/ai/settings/index.js +++ b/ee/app/assets/javascripts/ai/settings/index.js @@ -58,7 +58,7 @@ export const initAiSettings = (id, component, options = {}) => { showFoundationalAgentsPerAgentAvailability, foundationalAgentsStatuses, availableFoundationalFlows, - selectedFoundationalFlows, + selectedFoundationalFlowReferences, duoAgentPlatformEnabled, } = el.dataset; @@ -161,7 +161,9 @@ export const initAiSettings = (id, component, options = {}) => { return flows; })(), initialSelectedFoundationalFlowIds: (() => { - const selected = selectedFoundationalFlows ? JSON.parse(selectedFoundationalFlows) : []; + const selected = selectedFoundationalFlowReferences + ? JSON.parse(selectedFoundationalFlowReferences) + : []; return selected; })(), }, diff --git a/ee/app/helpers/ee/groups/settings_helper.rb b/ee/app/helpers/ee/groups/settings_helper.rb index dd04113b7ee9acd6396288e2c2129f1f3e764674..d841a6b194b737ada134cbc41b39d0b96580fa0f 100644 --- a/ee/app/helpers/ee/groups/settings_helper.rb +++ b/ee/app/helpers/ee/groups/settings_helper.rb @@ -121,29 +121,28 @@ def foundational_flows_settings_data duo_foundational_flows_availability: @group.namespace_settings.duo_foundational_flows_availability.to_s, duo_sast_fp_detection_availability: @group.namespace_settings.duo_sast_fp_detection_availability.to_s, available_foundational_flows: available_foundational_flows_json, - selected_foundational_flows: selected_foundational_flows_json + selected_foundational_flow_references: selected_foundational_flows_json } end def available_foundational_flows_json return [].to_json unless @group.root? - ::Ai::Catalog::Item.foundational_flows - .select(:id, :name, :description, :foundational_flow_reference) - .map do |item| - { - catalog_item_id: item.id, - name: item.name, - description: item.description, - reference: item.foundational_flow_reference - } - end.to_json + ::Ai::DuoWorkflows::WorkflowDefinition::ITEMS + .select { |item| item[:foundational_flow_reference].present? } + .map do |item| + { + name: item[:name], + description: item[:description], + reference: item[:foundational_flow_reference] + } + end.to_json end def selected_foundational_flows_json return [].to_json unless @group.root? - @group.selected_foundational_flow_ids.to_json + @group.selected_foundational_flow_references.to_json end def cascading_tooltip_data(setting_key) diff --git a/ee/app/models/ai/catalog/item.rb b/ee/app/models/ai/catalog/item.rb index a380da17b0590fd106032d88089ddca7d0a99b1a..ab43dbb296ee3cf2c3f77741918926947f5ca732 100644 --- a/ee/app/models/ai/catalog/item.rb +++ b/ee/app/models/ai/catalog/item.rb @@ -104,6 +104,16 @@ def public_or_visible_to_user(current_user) def foundational_flow_ids foundational_flows.order(:id).limit(FOUNDATIONAL_FLOWS_LIMIT).pluck_primary_key end + + def foundational_flow_ids_for_references(references) + return {} if references.blank? + + foundational_flows + .where(foundational_flow_reference: references) + .limit(FOUNDATIONAL_FLOWS_LIMIT) + .pluck(:foundational_flow_reference, :id) + .to_h + end end def deleted? diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index 14efb9757a01302f3a6ac6ac940109668e6f9ce6..b063239de37cad33b528d2c5bd13908895f97f54 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -733,6 +733,8 @@ def enabled_flow_catalog_item_ids end def sync_enabled_foundational_flows!(target_ids) + target_ids = target_ids.uniq + if target_ids.empty? enabled_foundational_flow_records .for_namespace(id) @@ -743,21 +745,30 @@ def sync_enabled_foundational_flows!(target_ids) .where.not(catalog_item_id: target_ids) .delete_all - current_time = Time.current - records = target_ids.map do |catalog_item_id| - Ai::Catalog::EnabledFoundationalFlow.new( - namespace_id: id, - project_id: nil, - catalog_item_id: catalog_item_id, - created_at: current_time, - updated_at: current_time + existing_catalog_item_ids = enabled_foundational_flow_records + .for_namespace(id) + .limit(::Ai::Catalog::Item::FOUNDATIONAL_FLOWS_LIMIT) + .pluck(:catalog_item_id) + + new_catalog_item_ids = target_ids - existing_catalog_item_ids + + if new_catalog_item_ids.any? + current_time = Time.current + records = new_catalog_item_ids.map do |catalog_item_id| + Ai::Catalog::EnabledFoundationalFlow.new( + namespace_id: id, + project_id: nil, + catalog_item_id: catalog_item_id, + created_at: current_time, + updated_at: current_time + ) + end + + Ai::Catalog::EnabledFoundationalFlow.bulk_insert!( + records, + unique_by: [:namespace_id, :catalog_item_id] ) end - - Ai::Catalog::EnabledFoundationalFlow.bulk_insert!( - records, - unique_by: [:namespace_id, :catalog_item_id] - ) end rescue ActiveRecord::RecordInvalid => e ::Gitlab::ErrorTracking.track_exception( @@ -776,8 +787,11 @@ def remove_foundational_flow_consumers(catalog_item_ids) .delete_all end - def selected_foundational_flow_ids - enabled_foundational_flow_records.limit(100).pluck(:catalog_item_id) + def selected_foundational_flow_references + enabled_foundational_flow_records + .for_namespace(id) + .includes(:catalog_item) + .filter_map { |record| record.catalog_item.foundational_flow_reference } end def can_assign_custom_roles_to_group_links? diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb index ea18f6cc544b1547ee78c44f810e29b3bea7f0c6..eb9151e397af380a9451ecc7991d877fe949ab92 100644 --- a/ee/app/models/ee/namespace_setting.rb +++ b/ee/app/models/ee/namespace_setting.rb @@ -23,7 +23,7 @@ module NamespaceSetting order("last_dormant_member_review_at ASC NULLS FIRST") end - attribute :enabled_foundational_flows, :integer, array: true, default: nil + attribute :enabled_foundational_flows, :string, array: true, default: nil belongs_to :default_compliance_framework, optional: true, class_name: "ComplianceManagement::Framework" @@ -316,6 +316,7 @@ def experiment_features_allowed lock_web_based_commit_signing_enabled duo_foundational_flows_enabled lock_duo_foundational_flows_enabled + enabled_foundational_flows ].freeze override :allowed_namespace_settings_params diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index e4d6dddcf1e1335ad8dad668a9994bcc290b2df9..8dabb4afddc7df480921279db28b9e18f1c0b09d 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -843,6 +843,8 @@ def remove_foundational_flow_consumers(catalog_item_ids) end def sync_enabled_foundational_flows!(target_ids) + target_ids = target_ids.uniq + if target_ids.empty? enabled_foundational_flow_records .for_project(id) @@ -853,21 +855,30 @@ def sync_enabled_foundational_flows!(target_ids) .where.not(catalog_item_id: target_ids) .delete_all - current_time = Time.current - records = target_ids.map do |catalog_item_id| - Ai::Catalog::EnabledFoundationalFlow.new( - project_id: id, - namespace_id: nil, - catalog_item_id: catalog_item_id, - created_at: current_time, - updated_at: current_time + existing_catalog_item_ids = enabled_foundational_flow_records + .for_project(id) + .limit(::Ai::Catalog::Item::FOUNDATIONAL_FLOWS_LIMIT) + .pluck(:catalog_item_id) + + new_catalog_item_ids = target_ids - existing_catalog_item_ids + + if new_catalog_item_ids.any? + current_time = Time.current + records = new_catalog_item_ids.map do |catalog_item_id| + Ai::Catalog::EnabledFoundationalFlow.new( + project_id: id, + namespace_id: nil, + catalog_item_id: catalog_item_id, + created_at: current_time, + updated_at: current_time + ) + end + + Ai::Catalog::EnabledFoundationalFlow.bulk_insert!( + records, + unique_by: [:project_id, :catalog_item_id] ) end - - Ai::Catalog::EnabledFoundationalFlow.bulk_insert!( - records, - unique_by: [:project_id, :catalog_item_id] - ) end rescue ActiveRecord::RecordInvalid => e ::Gitlab::ErrorTracking.track_exception( diff --git a/ee/app/services/ai/cascade_duo_settings_service.rb b/ee/app/services/ai/cascade_duo_settings_service.rb index 107f0a11ff28c18fb1f1f9ad6f3f4b37d8e27057..9357231fa4c40bec469ff2168203642681f1099d 100644 --- a/ee/app/services/ai/cascade_duo_settings_service.rb +++ b/ee/app/services/ai/cascade_duo_settings_service.rb @@ -68,20 +68,17 @@ def update_projects(group) end def cascade_flow_selection(group) - target_ids = @flow_ids || [] + target_references = @flow_ids || [] + target_ids = convert_flow_references_to_ids(target_references) group.sync_enabled_foundational_flows!(target_ids) - group.descendants.each_batch do |batch| - batch.each do |descendant_group| - descendant_group.sync_enabled_foundational_flows!(target_ids) - end + group.descendants.each do |descendant_group| + descendant_group.sync_enabled_foundational_flows!(target_ids) end - group.all_projects.each_batch do |batch| - batch.each do |project| - project.sync_enabled_foundational_flows!(target_ids) - end + group.all_projects.each do |project| + project.sync_enabled_foundational_flows!(target_ids) end end @@ -92,8 +89,16 @@ def foundational_flows_changed? def schedule_foundational_flows_sync(group) ::Ai::Catalog::Flows::CascadeSyncFoundationalFlowsWorker.perform_async( group.id, - @current_user&.id + @current_user&.id, + @flow_ids ) end + + def convert_flow_references_to_ids(flow_references) + return [] if flow_references.blank? + + reference_to_id_map = ::Ai::Catalog::Item.foundational_flow_ids_for_references(flow_references) + flow_references.filter_map { |ref| reference_to_id_map[ref] } + end end end diff --git a/ee/app/services/ee/groups/update_service.rb b/ee/app/services/ee/groups/update_service.rb index 61e896120347ff238989300218db77cf755076f9..83e206dd23e502aaf1dfa9ad2ebf30f04d5a6244 100644 --- a/ee/app/services/ee/groups/update_service.rb +++ b/ee/app/services/ee/groups/update_service.rb @@ -10,7 +10,6 @@ module UpdateService :remove_dormant_members_period, :allow_enterprise_bypass_placeholder_confirmation, :enterprise_bypass_expires_at, - :enabled_foundational_flows, :display_gitlab_credits_user_data ].freeze @@ -167,8 +166,9 @@ def log_audit_events def update_cascading_settings previous_changes = group.namespace_settings.previous_changes + submitted_flow_refs = group.namespace_settings.enabled_foundational_flows - return unless previous_changes.present? + return unless previous_changes.present? || submitted_flow_refs cascading_ai_settings = [:duo_features_enabled, :duo_remote_flows_enabled, :auto_duo_code_review_enabled, :duo_foundational_flows_enabled, :duo_sast_fp_detection_enabled] @@ -179,12 +179,9 @@ def update_cascading_settings end end.to_h - submitted_flow_ids = group.namespace_settings.enabled_foundational_flows - old_flow_ids = group.selected_foundational_flow_ids - - if !submitted_flow_ids.nil? && old_flow_ids.sort != submitted_flow_ids.sort - group.sync_enabled_foundational_flows!(submitted_flow_ids) - changed_ai_settings[:enabled_foundational_flows] = submitted_flow_ids + if submitted_flow_refs + sync_parent_group_flows(submitted_flow_refs) + changed_ai_settings[:enabled_foundational_flows] = submitted_flow_refs end if changed_ai_settings.any? @@ -200,6 +197,35 @@ def update_cascading_settings end end + def sync_parent_group_flows(flow_references) + ::Ai::Catalog::Flows::SeedFoundationalFlowsService.new( + current_user: current_user, + organization: group.organization + ).execute + + catalog_item_ids = convert_flow_references_to_ids(flow_references) + + group.sync_enabled_foundational_flows!(catalog_item_ids) + + ::Ai::Catalog::Flows::SyncFoundationalFlowsService.new( + group, + current_user: current_user + ).execute + rescue StandardError => e + ::Gitlab::ErrorTracking.track_exception( + e, + group_id: group.id, + flow_references: flow_references + ) + end + + def convert_flow_references_to_ids(flow_references) + return [] if flow_references.blank? + + reference_to_id_map = ::Ai::Catalog::Item.foundational_flow_ids_for_references(flow_references) + flow_references.filter_map { |ref| reference_to_id_map[ref] } + end + def handle_pending_members settings = group.namespace_settings diff --git a/ee/app/workers/ai/catalog/flows/cascade_sync_foundational_flows_worker.rb b/ee/app/workers/ai/catalog/flows/cascade_sync_foundational_flows_worker.rb index 4b076d906d386f134ae3230bd2900ece54e76eae..f6fed329f216bf5de90c1222c629c746c174ac13 100644 --- a/ee/app/workers/ai/catalog/flows/cascade_sync_foundational_flows_worker.rb +++ b/ee/app/workers/ai/catalog/flows/cascade_sync_foundational_flows_worker.rb @@ -13,7 +13,7 @@ class CascadeSyncFoundationalFlowsWorker idempotent! defer_on_database_health_signal :gitlab_main, [:namespace_settings, :project_settings], 1.minute - def perform(group_id, user_id = nil) + def perform(group_id, user_id = nil, flow_references = nil) group = Group.find_by_id(group_id) return unless group @@ -21,7 +21,9 @@ def perform(group_id, user_id = nil) seed_foundational_flows(group, user) - sync_groups(group, user) + convert_references_to_ids(flow_references) if flow_references + + sync_groups(group, user, skip_parent: true) sync_projects(group, user) end @@ -29,20 +31,36 @@ def perform(group_id, user_id = nil) private def seed_foundational_flows(group, user) - organization = group.organization - return unless organization - ::Ai::Catalog::Flows::SeedFoundationalFlowsService.new( current_user: user, - organization: organization + organization: group.organization ).execute end - def sync_groups(group, user) - ::Ai::Catalog::Flows::SyncFoundationalFlowsService.new( - group, - current_user: user - ).execute + def convert_references_to_ids(flow_references) + references = flow_references || [] + return [] if references.empty? + + reference_to_id = ::Ai::Catalog::Item.foundational_flow_ids_for_references(references) + references.filter_map { |ref| reference_to_id[ref] } + end + + def sync_groups(group, user, skip_parent: false) + unless skip_parent + ::Ai::Catalog::Flows::SyncFoundationalFlowsService.new( + group, + current_user: user + ).execute + end + + group.descendants.each_batch do |batch| + batch.each do |descendant_group| + ::Ai::Catalog::Flows::SyncFoundationalFlowsService.new( + descendant_group, + current_user: user + ).execute + end + end end def sync_projects(group, user) diff --git a/ee/lib/ee/api/helpers/groups_helpers.rb b/ee/lib/ee/api/helpers/groups_helpers.rb index b6cc393bb641dfe6f34f5528c3ea5f7067eebba7..70f6d6bbc422aa0147627fa10f2f24e008f57fe2 100644 --- a/ee/lib/ee/api/helpers/groups_helpers.rb +++ b/ee/lib/ee/api/helpers/groups_helpers.rb @@ -108,7 +108,7 @@ module GroupsHelpers type: ::Grape::API::Boolean, desc: 'Only allow to merge if all threads are resolved' optional :enabled_foundational_flows, - type: Array[Integer], desc: 'IDs of enabled foundational flows' + type: Array[String], desc: 'References of enabled foundational flows' end params :optional_projects_params_ee do diff --git a/ee/spec/frontend/ai/settings/components/ai_common_settings_form_spec.js b/ee/spec/frontend/ai/settings/components/ai_common_settings_form_spec.js index d85c526680b260f383c2f685cf44bb92a86f3391..2a16221d0fde0300a4421448e82ad98ab0a093ea 100644 --- a/ee/spec/frontend/ai/settings/components/ai_common_settings_form_spec.js +++ b/ee/spec/frontend/ai/settings/components/ai_common_settings_form_spec.js @@ -375,21 +375,30 @@ describe('AiCommonSettingsForm', () => { describe('foundational flow selection integration', () => { it('emits change-selected-flow-ids event when DuoFlowSettings emits it', async () => { - await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [1, 2, 3]); + await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [ + 'code_review/v1', + 'bug_triage/v1', + 'documentation/v1', + ]); - expect(wrapper.emitted('change-selected-flow-ids')[0]).toEqual([[1, 2, 3]]); + expect(wrapper.emitted('change-selected-flow-ids')[0]).toEqual([ + ['code_review/v1', 'bug_triage/v1', 'documentation/v1'], + ]); }); it('updates internal localSelectedFlowIds data when change-selected-flow-ids event is received', async () => { - createComponent({ props: { selectedFoundationalFlowIds: [1] } }); + createComponent({ props: { selectedFoundationalFlowIds: ['code_review/v1'] } }); expect(findSaveButton().props('disabled')).toBe(true); - await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [1, 2]); + await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [ + 'code_review/v1', + 'bug_triage/v1', + ]); expect(findSaveButton().props('disabled')).toBe(false); - await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [1]); + await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', ['code_review/v1']); expect(findSaveButton().props('disabled')).toBe(true); }); @@ -397,25 +406,41 @@ describe('AiCommonSettingsForm', () => { it('enables save button when flow IDs change from empty to non-empty', async () => { expect(findSaveButton().props('disabled')).toBe(true); - await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [5, 6]); + await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [ + 'sast_fp_detection/v1', + 'resolve_sast_vulnerability/v1', + ]); expect(findSaveButton().props('disabled')).toBe(false); }); it('enables save button when flow IDs order changes', async () => { - createComponent({ props: { selectedFoundationalFlowIds: [1, 2, 3] } }); + createComponent({ + props: { + selectedFoundationalFlowIds: ['code_review/v1', 'bug_triage/v1', 'documentation/v1'], + }, + }); expect(findSaveButton().props('disabled')).toBe(true); - await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [3, 2, 1]); + await findDuoFlowSettings().vm.$emit('change-selected-flow-ids', [ + 'documentation/v1', + 'bug_triage/v1', + 'code_review/v1', + ]); expect(findSaveButton().props('disabled')).toBe(true); }); it('passes selectedFoundationalFlowIds prop to DuoFlowSettings', () => { - createComponent({ props: { selectedFoundationalFlowIds: [10, 20] } }); + createComponent({ + props: { selectedFoundationalFlowIds: ['code_review/v1', 'bug_triage/v1'] }, + }); - expect(findDuoFlowSettings().props('selectedFoundationalFlowIds')).toEqual([10, 20]); + expect(findDuoFlowSettings().props('selectedFoundationalFlowIds')).toEqual([ + 'code_review/v1', + 'bug_triage/v1', + ]); }); }); diff --git a/ee/spec/frontend/ai/settings/components/duo_flow_settings_spec.js b/ee/spec/frontend/ai/settings/components/duo_flow_settings_spec.js index 0f2e2ff1ed7d45cdcd87e645ddb2f1ae2e333cc6..5d35f065209c3ee8065e4aa1f684eaa537e62e77 100644 --- a/ee/spec/frontend/ai/settings/components/duo_flow_settings_spec.js +++ b/ee/spec/frontend/ai/settings/components/duo_flow_settings_spec.js @@ -366,7 +366,7 @@ describe('DuoFlowSettings', () => { wrapper = createWrapper({ duoRemoteFlowsAvailability: true, duoFoundationalFlowsAvailability: true, - selectedFoundationalFlowIds: [1, 2], + selectedFoundationalFlowIds: ['code_review/v1', 'bug_triage/v1'], }); }); @@ -375,7 +375,10 @@ describe('DuoFlowSettings', () => { }); it('passes the selected flow ids to the selector', () => { - expect(findFoundationalFlowSelector().props('value')).toEqual([1, 2]); + expect(findFoundationalFlowSelector().props('value')).toEqual([ + 'code_review/v1', + 'bug_triage/v1', + ]); }); it('passes disabled state to the selector', () => { @@ -383,10 +386,16 @@ describe('DuoFlowSettings', () => { }); it('emits change-selected-flow-ids when selector value changes', async () => { - await findFoundationalFlowSelector().vm.$emit('input', [3, 4, 5]); + await findFoundationalFlowSelector().vm.$emit('input', [ + 'documentation/v1', + 'sast_fp_detection/v1', + 'resolve_sast_vulnerability/v1', + ]); expect(wrapper.emitted('change-selected-flow-ids')).toHaveLength(1); - expect(wrapper.emitted('change-selected-flow-ids')[0]).toEqual([[3, 4, 5]]); + expect(wrapper.emitted('change-selected-flow-ids')[0]).toEqual([ + ['documentation/v1', 'sast_fp_detection/v1', 'resolve_sast_vulnerability/v1'], + ]); }); }); @@ -409,7 +418,7 @@ describe('DuoFlowSettings', () => { wrapper = createWrapper({ duoRemoteFlowsAvailability: true, duoFoundationalFlowsAvailability: true, - selectedFoundationalFlowIds: [1, 2], + selectedFoundationalFlowIds: ['code_review/v1', 'bug_triage/v1'], }); }); diff --git a/ee/spec/frontend/ai/settings/components/foundational_flow_selector_spec.js b/ee/spec/frontend/ai/settings/components/foundational_flow_selector_spec.js index cbf3354b7863e5e791d50d7f4e688b9238f71a4f..9c6605b4417f609912cbad984d4d54f2820200c0 100644 --- a/ee/spec/frontend/ai/settings/components/foundational_flow_selector_spec.js +++ b/ee/spec/frontend/ai/settings/components/foundational_flow_selector_spec.js @@ -7,17 +7,17 @@ describe('FoundationalFlowSelector', () => { const mockFlows = [ { - catalog_item_id: 1, + reference: 'code_review/v1', name: 'Code Review Flow', description: 'Automated code review assistant', }, { - catalog_item_id: 2, + reference: 'bug_triage/v1', name: 'Bug Triage Flow', description: 'Automatically triages and categorizes bugs', }, { - catalog_item_id: 3, + reference: 'documentation/v1', name: 'Documentation Flow', description: 'Generates documentation from code', }, @@ -94,7 +94,7 @@ describe('FoundationalFlowSelector', () => { }); it('shows selected flows as checked', () => { - wrapper = createWrapper({ value: [1, 3] }); + wrapper = createWrapper({ value: ['code_review/v1', 'documentation/v1'] }); expect(findCheckboxAt(0).props('checked')).toBe(true); expect(findCheckboxAt(1).props('checked')).toBe(false); @@ -102,7 +102,7 @@ describe('FoundationalFlowSelector', () => { }); it('shows all checkboxes as checked when all flows are selected', () => { - wrapper = createWrapper({ value: [1, 2, 3] }); + wrapper = createWrapper({ value: ['code_review/v1', 'bug_triage/v1', 'documentation/v1'] }); findAllCheckboxes().wrappers.forEach((checkbox) => { expect(checkbox.props('checked')).toBe(true); @@ -145,96 +145,96 @@ describe('FoundationalFlowSelector', () => { await findCheckboxAt(0).vm.$emit('input', true); expect(wrapper.emitted('input')).toHaveLength(1); - expect(wrapper.emitted('input')[0]).toEqual([[1]]); + expect(wrapper.emitted('input')[0]).toEqual([['code_review/v1']]); }); it('emits input event with multiple flow ids when multiple checkboxes are checked', async () => { await findCheckboxAt(0).vm.$emit('input', true); - wrapper = createWrapper({ value: [1] }); + wrapper = createWrapper({ value: ['code_review/v1'] }); await findCheckboxAt(2).vm.$emit('input', true); expect(wrapper.emitted('input')).toHaveLength(1); - expect(wrapper.emitted('input')[0]).toEqual([[1, 3]]); + expect(wrapper.emitted('input')[0]).toEqual([['code_review/v1', 'documentation/v1']]); }); it('emits input event with removed flow id when checkbox is unchecked', async () => { - wrapper = createWrapper({ value: [1, 2, 3] }); + wrapper = createWrapper({ value: ['code_review/v1', 'bug_triage/v1', 'documentation/v1'] }); await findCheckboxAt(1).vm.$emit('input', false); expect(wrapper.emitted('input')).toHaveLength(1); - expect(wrapper.emitted('input')[0]).toEqual([[1, 3]]); + expect(wrapper.emitted('input')[0]).toEqual([['code_review/v1', 'documentation/v1']]); }); it('preserves existing selections when adding new flow', async () => { - wrapper = createWrapper({ value: [1] }); + wrapper = createWrapper({ value: ['code_review/v1'] }); await findCheckboxAt(1).vm.$emit('input', true); - expect(wrapper.emitted('input')[0]).toEqual([[1, 2]]); + expect(wrapper.emitted('input')[0]).toEqual([['code_review/v1', 'bug_triage/v1']]); }); it('preserves existing selections when removing a flow', async () => { - wrapper = createWrapper({ value: [1, 2, 3] }); + wrapper = createWrapper({ value: ['code_review/v1', 'bug_triage/v1', 'documentation/v1'] }); await findCheckboxAt(1).vm.$emit('input', false); - expect(wrapper.emitted('input')[0]).toEqual([[1, 3]]); + expect(wrapper.emitted('input')[0]).toEqual([['code_review/v1', 'documentation/v1']]); }); }); describe('isFlowSelected method', () => { it('returns true for selected flow ids', () => { - wrapper = createWrapper({ value: [1, 3] }); + wrapper = createWrapper({ value: ['code_review/v1', 'documentation/v1'] }); - expect(wrapper.vm.isFlowSelected(1)).toBe(true); - expect(wrapper.vm.isFlowSelected(3)).toBe(true); + expect(wrapper.vm.isFlowSelected('code_review/v1')).toBe(true); + expect(wrapper.vm.isFlowSelected('documentation/v1')).toBe(true); }); it('returns false for unselected flow ids', () => { - wrapper = createWrapper({ value: [1, 3] }); + wrapper = createWrapper({ value: ['code_review/v1', 'documentation/v1'] }); - expect(wrapper.vm.isFlowSelected(2)).toBe(false); + expect(wrapper.vm.isFlowSelected('bug_triage/v1')).toBe(false); }); it('returns false when value is empty', () => { wrapper = createWrapper({ value: [] }); - expect(wrapper.vm.isFlowSelected(1)).toBe(false); - expect(wrapper.vm.isFlowSelected(2)).toBe(false); + expect(wrapper.vm.isFlowSelected('code_review/v1')).toBe(false); + expect(wrapper.vm.isFlowSelected('bug_triage/v1')).toBe(false); }); }); describe('toggleFlow method', () => { beforeEach(() => { - wrapper = createWrapper({ value: [1] }); + wrapper = createWrapper({ value: ['code_review/v1'] }); }); it('adds flow id when checked is true', () => { - wrapper.vm.toggleFlow(2, true); + wrapper.vm.toggleFlow('bug_triage/v1', true); - expect(wrapper.emitted('input')[0]).toEqual([[1, 2]]); + expect(wrapper.emitted('input')[0]).toEqual([['code_review/v1', 'bug_triage/v1']]); }); it('removes flow id when checked is false', () => { - wrapper.vm.toggleFlow(1, false); + wrapper.vm.toggleFlow('code_review/v1', false); expect(wrapper.emitted('input')[0]).toEqual([[]]); }); it('adds flow id even if already in the array', () => { - wrapper.vm.toggleFlow(1, true); + wrapper.vm.toggleFlow('code_review/v1', true); // The component doesn't prevent duplicates - expect(wrapper.emitted('input')[0]).toEqual([[1, 1]]); + expect(wrapper.emitted('input')[0]).toEqual([['code_review/v1', 'code_review/v1']]); }); it('handles removing non-existent flow id gracefully', () => { - wrapper.vm.toggleFlow(99, false); + wrapper.vm.toggleFlow('non_existent/v1', false); - expect(wrapper.emitted('input')[0]).toEqual([[1]]); + expect(wrapper.emitted('input')[0]).toEqual([['code_review/v1']]); }); }); @@ -242,7 +242,7 @@ describe('FoundationalFlowSelector', () => { it('handles flow with missing description', () => { const flowsWithoutDescription = [ { - catalog_item_id: 1, + reference: 'flow_without_desc/v1', name: 'Flow Without Description', description: undefined, }, @@ -256,7 +256,7 @@ describe('FoundationalFlowSelector', () => { it('handles flow with empty description', () => { const flowsWithEmptyDescription = [ { - catalog_item_id: 1, + reference: 'flow_with_empty_desc/v1', name: 'Flow With Empty Description', description: '', }, @@ -275,7 +275,7 @@ describe('FoundationalFlowSelector', () => { await findCheckboxAt(0).vm.$emit('input', true); expect(wrapper.emitted('input')).toHaveLength(3); - expect(wrapper.emitted('input')[2]).toEqual([[1]]); + expect(wrapper.emitted('input')[2]).toEqual([['code_review/v1']]); }); }); }); diff --git a/ee/spec/helpers/ee/groups/settings_helper_spec.rb b/ee/spec/helpers/ee/groups/settings_helper_spec.rb index 2d165688a4fba93aabaa4f0b0ef83e9a594f1dde..9ef3ee98eba07c0233297bec872bf2438829af92 100644 --- a/ee/spec/helpers/ee/groups/settings_helper_spec.rb +++ b/ee/spec/helpers/ee/groups/settings_helper_spec.rb @@ -90,12 +90,19 @@ let(:add_on_purchase) { nil } let(:root_ancestor) { group } - let!(:foundational_flow) { create(:ai_catalog_item, :with_foundational_flow_reference, public: true) } # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Needs to be loaded from DB + let(:test_workflows) do + [{ + foundational_flow_reference: 'test_flow/v1', + name: 'Test Flow', + description: 'Test Description' + }] + end before do allow(helper).to receive(:show_early_access_program_banner?).and_return(true) allow(current_user).to receive(:can?).with(:admin_duo_workflow, group).and_return(true) stub_saas_features(gitlab_com_subscriptions: true) + stub_const('::Ai::DuoWorkflows::WorkflowDefinition::ITEMS', test_workflows) end it 'returns the expected data' do @@ -136,12 +143,11 @@ ai_settings_minimum_access_level_manage: group.ai_minimum_access_level_manage, ai_settings_minimum_access_level_enable_on_projects: group.ai_minimum_access_level_enable_on_projects, available_foundational_flows: Gitlab::Json.generate([{ - catalog_item_id: foundational_flow.id, - name: foundational_flow.name, - description: foundational_flow.description, - reference: foundational_flow.foundational_flow_reference + name: 'Test Flow', + description: 'Test Description', + reference: 'test_flow/v1' }]), - selected_foundational_flows: '[]' + selected_foundational_flow_references: '[]' } ) end @@ -193,7 +199,7 @@ duo_workflow_available: "false", duo_workflow_mcp_enabled: "false", available_foundational_flows: '[]', - selected_foundational_flows: '[]' + selected_foundational_flow_references: '[]' } ) end diff --git a/ee/spec/models/ai/catalog/item_spec.rb b/ee/spec/models/ai/catalog/item_spec.rb index 14b90300ccc1e82834a38e6294c07bf85ae0f298..c17ac5c9af72704a871d110b1c507d6c060c4e92 100644 --- a/ee/spec/models/ai/catalog/item_spec.rb +++ b/ee/spec/models/ai/catalog/item_spec.rb @@ -302,6 +302,48 @@ end end + describe '.foundational_flow_ids_for_references' do + let_it_be(:code_review_flow) do + create(:ai_catalog_item, :with_foundational_flow_reference, + foundational_flow_reference: 'code_review/v1') + end + + let_it_be(:sast_flow) do + create(:ai_catalog_item, :with_foundational_flow_reference, + foundational_flow_reference: 'sast_fp_detection/v1') + end + + let_it_be(:item_without_reference) { create(:ai_catalog_item) } + + it 'returns a hash mapping references to IDs' do + result = described_class.foundational_flow_ids_for_references(['code_review/v1', 'sast_fp_detection/v1']) + + expect(result).to eq({ + 'code_review/v1' => code_review_flow.id, + 'sast_fp_detection/v1' => sast_flow.id + }) + end + + it 'returns empty hash for blank references' do + expect(described_class.foundational_flow_ids_for_references([])).to eq({}) + expect(described_class.foundational_flow_ids_for_references(nil)).to eq({}) + end + + it 'only returns matching references' do + result = described_class.foundational_flow_ids_for_references(['code_review/v1', 'nonexistent/v1']) + + expect(result).to eq({ 'code_review/v1' => code_review_flow.id }) + end + + it 'respects FOUNDATIONAL_FLOWS_LIMIT' do + stub_const("#{described_class}::FOUNDATIONAL_FLOWS_LIMIT", 1) + + result = described_class.foundational_flow_ids_for_references(['code_review/v1', 'sast_fp_detection/v1']) + + expect(result.size).to eq(1) + end + end + describe '.order_by_id_desc' do subject { described_class.order_by_id_desc } diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index 78e9ac95929c3a5485f633a2bd96d08243534725..03bcf4dd8866d1f4b1e207d5dccd85eeed11cd14 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -3386,24 +3386,23 @@ def yielded_projects(group) end end - describe '#selected_foundational_flow_ids' do + describe '#selected_foundational_flow_references' do let_it_be(:namespace) { create(:group) } let_it_be(:flow1) { create(:ai_catalog_item, :with_foundational_flow_reference, public: true) } let_it_be(:flow2) { create(:ai_catalog_item, :with_foundational_flow_reference, public: true) } - it 'returns selected flow IDs' do + it 'returns selected flow references' do create(:ai_catalog_enabled_foundational_flow, :for_namespace, namespace: namespace, catalog_item: flow1) create(:ai_catalog_enabled_foundational_flow, :for_namespace, namespace: namespace, catalog_item: flow2) - expect(namespace.selected_foundational_flow_ids).to match_array([flow1.id, flow2.id]) + expect(namespace.selected_foundational_flow_references).to match_array([ + flow1.foundational_flow_reference, + flow2.foundational_flow_reference + ]) end - it 'limits to 100 records' do - allow(namespace.enabled_foundational_flow_records).to receive(:limit).with(100).and_call_original - - namespace.selected_foundational_flow_ids - - expect(namespace.enabled_foundational_flow_records).to have_received(:limit).with(100) + it 'returns empty array when no flows are enabled' do + expect(namespace.selected_foundational_flow_references).to eq([]) end end diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index 04ba22c432d3c6320c1b2667edcf803a2dc9ae3f..938cb442bc5d21029f2ddbc7fff106fbe9271746 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -958,10 +958,10 @@ describe 'attributes' do describe '#enabled_foundational_flows' do - it 'accepts an array of integers' do - setting.enabled_foundational_flows = [1, 2, 3] + it 'accepts an array of strings' do + setting.enabled_foundational_flows = ['code_review/v1', 'bug_triage/v1', 'documentation/v1'] - expect(setting.enabled_foundational_flows).to match_array([1, 2, 3]) + expect(setting.enabled_foundational_flows).to match_array(['code_review/v1', 'bug_triage/v1', 'documentation/v1']) end it 'defaults to nil' do diff --git a/ee/spec/services/ai/cascade_duo_settings_service_spec.rb b/ee/spec/services/ai/cascade_duo_settings_service_spec.rb index d7b7dcc251b9e868935577bf5b6aa42e437dbcfa..dad679c34776e0881efc3dd641c4c03c7a712e14 100644 --- a/ee/spec/services/ai/cascade_duo_settings_service_spec.rb +++ b/ee/spec/services/ai/cascade_duo_settings_service_spec.rb @@ -98,7 +98,7 @@ it 'schedules worker to sync flows' do expect(Ai::Catalog::Flows::CascadeSyncFoundationalFlowsWorker) - .to receive(:perform_async).with(group.id, user.id) + .to receive(:perform_async).with(group.id, user.id, nil) service.cascade_for_group(group) end @@ -108,7 +108,7 @@ it 'schedules worker to sync flows' do expect(Ai::Catalog::Flows::CascadeSyncFoundationalFlowsWorker) - .to receive(:perform_async).with(group.id, nil) + .to receive(:perform_async).with(group.id, nil, nil) service.cascade_for_group(group) end @@ -142,7 +142,7 @@ it 'schedules worker to sync flows' do expect(Ai::Catalog::Flows::CascadeSyncFoundationalFlowsWorker) - .to receive(:perform_async).with(group.id, user.id) + .to receive(:perform_async).with(group.id, user.id, nil) service.cascade_for_group(group) end @@ -151,16 +151,17 @@ context 'when enabled_foundational_flows is provided' do let_it_be(:flow1) do create(:ai_catalog_item, :with_foundational_flow_reference, public: true, - organization: group.organization) + organization: group.organization, foundational_flow_reference: 'code_review/v1') end let_it_be(:flow2) do create(:ai_catalog_item, :with_foundational_flow_reference, public: true, - organization: group.organization) + organization: group.organization, foundational_flow_reference: 'sast_fp_detection/v1') end let(:flow_ids) { [flow1.id, flow2.id] } - let(:setting_attributes) { { 'enabled_foundational_flows' => flow_ids } } + let(:flow_references) { [flow1.foundational_flow_reference, flow2.foundational_flow_reference] } + let(:setting_attributes) { { 'enabled_foundational_flows' => flow_references } } subject(:service) { described_class.new(setting_attributes, current_user: user) } @@ -174,7 +175,7 @@ it 'schedules worker to sync flows' do expect(Ai::Catalog::Flows::CascadeSyncFoundationalFlowsWorker) - .to receive(:perform_async).with(group.id, user.id) + .to receive(:perform_async).with(group.id, user.id, flow_references) service.cascade_for_group(group) end diff --git a/ee/spec/services/groups/update_service_spec.rb b/ee/spec/services/groups/update_service_spec.rb index 78ee7bb03b9ee3f8a636a2d123ac402d1e6aa600..0845b1b3f65d4b68cb2c5e7073f521942bb85fa4 100644 --- a/ee/spec/services/groups/update_service_spec.rb +++ b/ee/spec/services/groups/update_service_spec.rb @@ -380,22 +380,20 @@ def update_file_template_project_id(id) create(:ai_catalog_item, :with_foundational_flow_reference, public: true, organization: group.organization) end - let(:params) { { enabled_foundational_flows: [flow1.id, flow2.id] } } + let(:params) do + { enabled_foundational_flows: [flow1.foundational_flow_reference, flow2.foundational_flow_reference] } + end before_all do group.add_owner(user) end - it 'syncs enabled foundational flows' do - update_group(group, user, params) - - expect(group.selected_foundational_flow_ids).to match_array([flow1.id, flow2.id]) - end - it 'schedules cascade worker with user_id', :sidekiq_inline do expect(Namespaces::CascadeDuoSettingsWorker).to receive(:perform_async).with( group.id, - hash_including(enabled_foundational_flows: [flow1.id, flow2.id]), # Use symbol key + hash_including(enabled_foundational_flows: [ + flow1.foundational_flow_reference, flow2.foundational_flow_reference + ]), user.id ) @@ -406,14 +404,33 @@ def update_file_template_project_id(id) let(:params) { { enabled_foundational_flows: [] } } before do - group.namespace_settings.update!(enabled_foundational_flows: [flow1.id]) + group.namespace_settings.update!(enabled_foundational_flows: [flow1.foundational_flow_reference]) group.sync_enabled_foundational_flows!([flow1.id]) end it 'removes all enabled flows' do update_group(group, user, params) - expect(group.selected_foundational_flow_ids).to be_empty + expect(group.selected_foundational_flow_references).to be_empty + end + end + + context 'when sync fails' do + let(:params) do + { enabled_foundational_flows: [flow1.foundational_flow_reference] } + end + + it 'tracks the exception and continues' do + allow_next_instance_of(Ai::Catalog::Flows::SyncFoundationalFlowsService) do |service| + allow(service).to receive(:execute).and_raise(StandardError, 'sync failed') + end + + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + instance_of(StandardError), + hash_including(group_id: group.id, flow_references: [flow1.foundational_flow_reference]) + ) + + update_group(group, user, params) end end end diff --git a/ee/spec/workers/ai/catalog/flows/cascade_sync_foundational_flows_worker_spec.rb b/ee/spec/workers/ai/catalog/flows/cascade_sync_foundational_flows_worker_spec.rb index bd28e01804a12a34e290164de514f52219bae0c1..913155a1a935d50147a6239af5a71795b01204ca 100644 --- a/ee/spec/workers/ai/catalog/flows/cascade_sync_foundational_flows_worker_spec.rb +++ b/ee/spec/workers/ai/catalog/flows/cascade_sync_foundational_flows_worker_spec.rb @@ -29,16 +29,15 @@ .and_return(seed_service) expect(seed_service).to receive(:execute) - worker.perform(group.id, user.id) + worker.perform(group.id, user.id, nil) end - it 'calls SyncFoundationalFlowsService for the group' do + it 'calls SyncFoundationalFlowsService for all descendant groups' do expect(Ai::Catalog::Flows::SyncFoundationalFlowsService) - .to receive(:new).with(group, current_user: user) + .to receive(:new).with(subgroup, current_user: user) .and_return(sync_service) - expect(sync_service).to receive(:execute) - worker.perform(group.id, user.id) + worker.perform(group.id, user.id, nil) end it 'calls SyncFoundationalFlowsService for all projects in the group hierarchy' do @@ -49,7 +48,40 @@ .to receive(:new).with(subgroup_project, current_user: user) .and_return(sync_service) - worker.perform(group.id, user.id) + worker.perform(group.id, user.id, nil) + end + + context 'when flow_references are provided' do + let_it_be(:catalog_item) { create(:ai_catalog_item, foundational_flow_reference: 'test_flow') } + + it 'converts flow references to catalog item IDs' do + expect(worker).to receive(:convert_references_to_ids) + .with(['test_flow']) + .and_call_original + + worker.perform(group.id, user.id, ['test_flow']) + end + + it 'returns empty array when references are empty' do + result = worker.send(:convert_references_to_ids, []) + expect(result).to eq([]) + end + + it 'returns empty array when references are nil' do + result = worker.send(:convert_references_to_ids, nil) + expect(result).to eq([]) + end + end + + context 'when skip_parent is false' do + it 'syncs the parent group' do + expect(Ai::Catalog::Flows::SyncFoundationalFlowsService) + .to receive(:new).with(group, current_user: user) + .and_return(sync_service) + expect(sync_service).to receive(:execute) + + worker.send(:sync_groups, group, user, skip_parent: false) + end end context 'when user_id is not provided' do @@ -59,10 +91,10 @@ .and_return(seed_service) expect(Ai::Catalog::Flows::SyncFoundationalFlowsService) - .to receive(:new).with(group, current_user: nil) + .to receive(:new).with(subgroup, current_user: nil) .and_return(sync_service) - worker.perform(group.id, nil) + worker.perform(group.id, nil, nil) end end @@ -73,10 +105,10 @@ .and_return(seed_service) expect(Ai::Catalog::Flows::SyncFoundationalFlowsService) - .to receive(:new).with(group, current_user: nil) + .to receive(:new).with(subgroup, current_user: nil) .and_return(sync_service) - worker.perform(group.id, non_existing_record_id) + worker.perform(group.id, non_existing_record_id, nil) end end @@ -86,23 +118,7 @@ expect(Ai::Catalog::Flows::SyncFoundationalFlowsService).not_to receive(:new) worker.perform(non_existing_record_id, user.id) - end - end - - context 'when group has no organization' do - let(:group_without_org) { create(:group) } - - before do - allow(group_without_org).to receive(:organization).and_return(nil) - allow(Group).to receive(:find_by_id).with(group_without_org.id).and_return(group_without_org) - end - - it 'does not call SeedFoundationalFlowsService but continues with sync operations' do - expect(Ai::Catalog::Flows::SeedFoundationalFlowsService).not_to receive(:new) - expect(Ai::Catalog::Flows::SyncFoundationalFlowsService).to receive(:new).at_least(:once) - .and_return(sync_service) - - worker.perform(group_without_org.id, user.id) + worker.perform(non_existing_record_id, user.id, nil) end end end