We use it in different components related to merge requests and work items, so it's difficult to find a good place for it. In the end, I decided to add to this shared module given that we have other constants in the same situation there already, so it's the next best choice.

Wanderson Policarpo (3f53d04f) at 12 Mar 00:36

What does this MR do and why?

Enable Duo Code Review by default for new customers, shows a banner on the "Get started" page of their new group that explains that Code Review is enabled. Controlled with the :dap_code_review_default_on_new_signups feature flag.

Changelog: changed EE: true

Screenshots

References

How to set up and validate locally

1. Turn on feature flags

# Enable required feature flags
Feature.enable(:dap_code_review_default_on_new_signups)
Feature.enable(:ultimate_trial_with_dap)
Feature.enable(:dap_free_tier_gitlab_credits)
Feature.enable(:service_accounts_available_on_free_or_unlicensed)

# Disable email confirmation so you can sign in immediately
Gitlab::CurrentSettings.update!(email_confirmation_setting: 'off')

2. Since GDK typically does not have CDot fully setup and does not run low priority Sidekiq queues by default, you'll have to simulate the trial completing. In rails console:

org = Organizations::Organization.find(1)
user = User.new(name: 'Test', username: 'testuser8', email: '[email protected]', password: 'gizmogizmo', confirmed_at: Time.now)
user.assign_personal_namespace(org)
user.save!
Organizations::OrganizationUser.create!(organization: org, user: user, access_level: :owner)
group = Groups::CreateService.new(user, name: 'testgroup5', path: 'testgroup5', visibility_level: 20, organization_id: org.id).execute.payload[:group]
project = Projects::CreateService.new(user, name: 'My Project', namespace_id: group.id, organization_id: org.id).execute
Onboarding::Progress.onboard(group.reset)
group.gitlab_subscription.update!(trial: true, trial_starts_on: Date.current, trial_ends_on: 1.year.from_now)
GitlabSubscriptions::AddOnPurchase.create!(add_on: GitlabSubscriptions::AddOn.find_or_create_by!(name: :gitlab_credits, description: 'Add-on for GitLab Credits.'), namespace: group, organization: org, quantity: 1, started_at: Time.current, expires_on: 1.year.from_now, purchase_xid: 'fake-purchase5')
GitlabSubscriptions::Trials::ApplyTrialService.new(uid: user.id, trial_user_information: { namespace_id: group.id }).send(:after_success_hook)
Namespaces::CascadeDuoSettingsWorker.new.perform(group.id, { 'duo_foundational_flows_enabled' => true, 'auto_duo_code_review_enabled' => true, 'enabled_foundational_flows' => ['code_review/v1'] }, user.id)
Ai::Catalog::Flows::CascadeSyncFoundationalFlowsWorker.new.perform(group.id, user.id)

3. Navigate to http://gdk.test:3000/testgroup/my-project/-/get_started to see the "Automated code review is enabled" banner
4. Click "To opt out, change your settings." or navigate to http://gdk.test:3000/groups/testgroup/-/settings/merge_requests — the "Enable automatic reviews by GitLab Duo" toggle should be checked
5. Start an MR in the project, GitlabDuo should be assigned as a review and start a review (if you have Duo Workflow setup)

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #592822

@squadri I left a couple of comments for you, let me know if you have any questions 🏓

suggestion(non-blocking): We may want to use a instance_double instead for clarity.

suggestion: For here and all the upper calls, current_user could be renamed to composite_identity or service_account_user to avoid confusion with the human current_user, what do you think?

question: When exactly current_user can be nil?

question: Same here, what is the benefit of find_noteable over the original implementation. They look the same to me.

question: Do you know when workflow.service_account can be nil? I could use that instead of @params[:service_account] for clarity. I know they will be the same, but makes it clearer that the service account here is the same that started the workflow.

I'm not sure in which situation the fallback to @current_user will be used, could you please clarify?

question: Do we have plans to support other noteable entities than an Issue? This change seems redundant to me, what is the benefit of find_noteable over the original workflow.issue?

suggestion: Calling present? seems redundant here.

        workflow_params[:issue_id] = @resource.iid if @resource.is_a?(Issue)

We have an issue to remove instance-wide service account usage already: #592437

@ck3g I addressed all your suggestions in 14be2ed9, please let me know what you think.

Validations added.

Fair enough. I removed that index and left only the composite one.

No plans to filter by check_id for now., so I dropped that index.

I added a spec for that enum.

The associations are marked as required already, but I added validations to all fields anyway.

@kivikakk I addressed all your suggestions in 14be2ed9, please let me know what you think.