Reviewer feedback implemented

Frontend:
- extract 'minimum_access_level' into constant
- prefix testid's with 'invite-members-modal'
- Use inject/provide instead of props for newProjectPath

Database:
- move 'tasks_to_be_done' to a separate table
- add a unique index for [member_id, project_id]
- search all member_tasks for a user when redirecting

Backend:
- removed unneeded 'with_indifferent_access'
- added model validations
- added idempotency spec
- added test for missing 'tasks_to_be_done'
- search by label instead of title when creating issues

Technical writer/UX:
- textual changes

Dangerbot:
- use 'match_array' instead of 'eq'

.rubocop_manual_todo.yml

@@ -1165,6 +1165,7 @@ Gitlab/NamespacedClass:
     - 'app/models/members/group_member.rb'
     - 'app/models/members/last_group_owner_assigner.rb'
     - 'app/models/members/project_member.rb'
+    - 'app/models/members/member_task.rb'
     - 'app/models/members_preloader.rb'
     - 'app/models/merge_request.rb'
     - 'app/models/merge_request_assignee.rb'

app/assets/javascripts/invite_members/components/invite_members_modal.vue

@@ -51,6 +51,7 @@ export default {
     MembersTokenSelect,
     GroupSelect,
   },
+  inject: ['newProjectPath'],
   props: {
     id: {
       type: String,
@@ -108,10 +109,6 @@ export default {
       type: Array,
       required: true,
     },
-    newProjectPath: {
-      type: String,
-      required: true,
-    },
     projects: {
       type: Array,
       required: true,
@@ -191,10 +188,16 @@ export default {
       return this.$options.labels[this.inviteeType].placeHolder;
     },
     tasksToBeDoneEnabled() {
-      return getParameterValues('open_modal')[0] === 'invite_members_for_task';
+      return (
+        getParameterValues('open_modal')[0] === 'invite_members_for_task' &&
+        this.tasksToBeDoneOptions.length
+      );
     },
     showTasksToBeDone() {
-      return this.tasksToBeDoneEnabled && this.selectedAccessLevel >= 30;
+      return (
+        this.tasksToBeDoneEnabled &&
+        this.selectedAccessLevel >= INVITE_MEMBERS_FOR_TASK.minimum_access_level
+      );
     },
     showTaskProjects() {
       return !this.isProject && this.selectedTasksToBeDone.length;
@@ -395,10 +398,10 @@ export default {
       },
       tasksToBeDone: {
         title: s__(
-          'InviteMembersModal|Create an issue for your new team member to work on (optional)',
+          'InviteMembersModal|Create issues for your new team member to work on (optional)',
         ),
         noProjects: s__(
-          'InviteMembersModal|To assign an issue to a new team member, you need a project for the issue. %{linkStart}Create a project to get started.%{linkEnd}',
+          'InviteMembersModal|To assign issues to a new team member, you need a project for the issues. %{linkStart}Create a project to get started.%{linkEnd}',
         ),
       },
       tasksProject: {
@@ -543,7 +546,7 @@ export default {
           data-testid="area-of-focus-checks"
         />
       </div>
-      <div v-if="showTasksToBeDone" data-testid="tasks-to-be-done">
+      <div v-if="showTasksToBeDone" data-testid="invite-members-modal-tasks-to-be-done">
         <label class="gl-mt-5">
           {{ $options.labels.members.tasksToBeDone.title }}
         </label>
@@ -551,7 +554,7 @@ export default {
           <gl-form-checkbox-group
             v-model="selectedTasksToBeDone"
             :options="tasksToBeDoneOptions"
-            data-testid="tasks"
+            data-testid="invite-members-modal-tasks"
           />
           <template v-if="showTaskProjects">
             <label class="gl-mt-5 gl-display-block">
@@ -560,7 +563,7 @@ export default {
             <gl-dropdown
               class="gl-w-half gl-xs-w-full"
               :text="selectedTaskProject.title"
-              data-testid="project-select"
+              data-testid="invite-members-modal-project-select"
             >
               <template v-for="project in projects">
                 <gl-dropdown-item
@@ -576,7 +579,12 @@ export default {
             </gl-dropdown>
           </template>
         </template>
-        <gl-alert v-else variant="tip" :dismissible="false" data-testid="no-projects-alert">
+        <gl-alert
+          v-else-if="tasksToBeDoneEnabled"
+          variant="tip"
+          :dismissible="false"
+          data-testid="invite-members-modal-no-projects-alert"
+        >
           <gl-sprintf :message="$options.labels.members.tasksToBeDone.noProjects">
             <template #link="{ content }">
               <gl-link :href="newProjectPath" target="_blank" class="gl-label-link">

app/assets/javascripts/invite_members/constants.js

@@ -9,6 +9,7 @@ export const MEMBER_AREAS_OF_FOCUS = {
   submit: 'submit',
 };
 export const INVITE_MEMBERS_FOR_TASK = {
+  minimum_access_level: 30,
   name: 'invite_members_for_task',
   view: 'modal_opened_from_email',
   submit: 'submit',

app/assets/javascripts/invite_members/init_invite_members_modal.js

@@ -14,6 +14,9 @@ export default function initInviteMembersModal() {
 
   return new Vue({
     el,
+    provide: {
+      newProjectPath: el.dataset.newProjectPath,
+    },
     render: (createElement) =>
       createElement(InviteMembersModal, {
         props: {
@@ -24,9 +27,8 @@ export default function initInviteMembersModal() {
           groupSelectFilter: el.dataset.groupsFilter,
           groupSelectParentId: parseInt(el.dataset.parentId, 10),
           areasOfFocusOptions: JSON.parse(el.dataset.areasOfFocusOptions),
-          tasksToBeDoneOptions: JSON.parse(el.dataset.tasksToBeDoneOptions),
-          projects: JSON.parse(el.dataset.projects),
-          newProjectPath: el.dataset.newProjectPath,
+          tasksToBeDoneOptions: JSON.parse(el.dataset.tasksToBeDoneOptions || '[]'),
+          projects: JSON.parse(el.dataset.projects || '[]'),
           noSelectionAreasOfFocus: JSON.parse(el.dataset.noSelectionAreasOfFocus),
           usersFilter: el.dataset.usersFilter,
           filterId: parseInt(el.dataset.filterId, 10),

app/controllers/registrations/welcome_controller.rb

@@ -71,7 +71,9 @@ def show_signup_onboarding?
     end
 
     def show_tasks_to_be_done?
-      current_user.members.last&.tasks_to_be_done.present?
+      return unless experiment(:invite_members_for_task).enabled?
+
+      MemberTask.for_members(current_user.members).exists?
     end
 
     def trial_params

app/helpers/invite_members_helper.rb

@@ -32,10 +32,7 @@ def common_invite_modal_dataset(source)
     dataset = {
       id: source.id,
       name: source.name,
-      default_access_level: Gitlab::Access::GUEST,
-      tasks_to_be_done_options: tasks_to_be_done_options.to_json,
-      projects: projects_for_source(source).to_json,
-      new_project_path: source.is_a?(Group) ? new_project_path(namespace_id: source.id) : ''
+      default_access_level: Gitlab::Access::GUEST
     }
 
     experiment(:member_areas_of_focus, user: current_user) do |e|
@@ -45,6 +42,14 @@ def common_invite_modal_dataset(source)
       e.candidate { dataset.merge!(areas_of_focus_options: member_areas_of_focus_options.to_json, no_selection_areas_of_focus: ['no_selection']) }
     end
 
+    if show_invite_members_for_task?
+      dataset.merge!(
+        tasks_to_be_done_options: tasks_to_be_done_options.to_json,
+        projects: projects_for_source(source).to_json,
+        new_project_path: source.is_a?(Group) ? new_project_path(namespace_id: source.id) : ''
+      )
+    end
+
     dataset
   end
 
@@ -75,8 +80,14 @@ def users_filter_data(group)
     {}
   end
 
+  def show_invite_members_for_task?
+    return unless current_user && experiment(:invite_members_for_task).enabled?
+
+    params[:open_modal] == 'invite_members_for_task'
+  end
+
   def tasks_to_be_done_options
-    ::Member::TASKS_TO_BE_DONE.keys.map { |task| { value: task, text: localized_tasks_to_be_done_choices[task] } }
+    ::MemberTask::TASKS.keys.map { |task| { value: task, text: localized_tasks_to_be_done_choices[task] } }
   end
 
   def projects_for_source(source)

app/helpers/members_helper.rb

@@ -61,7 +61,7 @@ def localized_tasks_to_be_done_choices
       code: s_('TasksToBeDone|Create/import code into a project (repository)'),
       ci: s_('TasksToBeDone|Set up CI/CD pipelines to build, test, deploy, and monitor code'),
       issues: s_('TasksToBeDone|Create/import issues (tickets) to collaborate on ideas and plan work')
-    }.with_indifferent_access.freeze
+    }.freeze
   end
 
   private

app/models/member.rb

@@ -13,23 +13,20 @@ class Member < ApplicationRecord
   include FromUnion
   include UpdateHighestRole
   include RestrictedSignup
+  include Gitlab::Experiment::Dsl
 
   AVATAR_SIZE = 40
   ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT = 10
-  TASKS_TO_BE_DONE = {
-    code: 0,
-    ci: 1,
-    issues: 2
-  }.freeze
 
   attr_accessor :raw_invite_token
 
   belongs_to :created_by, class_name: "User"
   belongs_to :user
   belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
-  belongs_to :tasks_project, class_name: 'Project'
+  has_one :member_task
 
   delegate :name, :username, :email, to: :user, prefix: true
+  delegate :tasks_to_be_done, to: :member_task, allow_nil: true
 
   validates :expires_at, allow_blank: true, future_date: true
   validates :user, presence: true, unless: :invite?
@@ -383,16 +380,6 @@ def created_by_name
     created_by&.name
   end
 
-  def tasks_to_be_done
-    Array(self[:tasks_to_be_done]).map { |task| TASKS_TO_BE_DONE.key(task) }
-  end
-
-  def tasks_to_be_done=(tasks)
-    self[:tasks_to_be_done] = Array(tasks).map do |task|
-      TASKS_TO_BE_DONE[task.to_sym] || raise(ArgumentError, "#{task} is not a valid value for tasks_to_be_done")
-    end.uniq
-  end
-
   private
 
   def send_invite
@@ -430,8 +417,12 @@ def refresh_member_authorized_projects(blocking:)
   def after_accept_invite
     post_create_hook
 
-    run_after_commit_or_now do
-      TasksToBeDone::CreateWorker.perform_async(tasks_project_id, created_by_id, [user_id.to_i], tasks_to_be_done)
+    if experiment(:invite_members_for_task).enabled?
+      run_after_commit_or_now do
+        if member_task
+          TasksToBeDone::CreateWorker.perform_async(member_task.id, created_by_id, [user_id.to_i])
+        end
+      end
     end
   end
 

app/models/members/member_task.rb

+# frozen_string_literal: true
+
+class MemberTask < ApplicationRecord
+  TASKS = {
+    code: 0,
+    ci: 1,
+    issues: 2
+  }.freeze
+
+  belongs_to :member
+  belongs_to :project
+
+  validates :member, :project, presence: true
+  validates :tasks, inclusion: { in: TASKS.values }
+  validate :tasks_uniqueness
+  validate :project_in_member_source
+
+  scope :for_members, -> (members) { joins(:member).where(member: members) }
+
+  def tasks_to_be_done
+    Array(self[:tasks]).map { |task| TASKS.key(task) }
+  end
+
+  def tasks_to_be_done=(tasks)
+    self[:tasks] = Array(tasks).map do |task|
+      TASKS[task.to_sym]
+    end.uniq
+  end
+
+  private
+
+  def tasks_uniqueness
+    errors.add(:tasks, 'are not unique') unless Array(tasks).length == Array(tasks).uniq.length
+  end
+
+  def project_in_member_source
+    if member.is_a?(GroupMember)
+      errors.add(:project, _('is not in the member group')) unless project.namespace == member.source
+    elsif member.is_a?(ProjectMember)
+      errors.add(:project, _('is not the member project')) unless project == member.source
+    end
+  end
+end

app/services/members/create_service.rb

@@ -117,11 +117,16 @@ def track_areas_of_focus(member)
     end
 
     def create_tasks_to_be_done
-      # Only create task issues for existing users. Tasks for new users are created when they signup.
-      return if self.instance_of?(Members::InviteService)
+      return unless experiment(:invite_members_for_task).enabled?
       return if params[:tasks_to_be_done].blank? || params[:tasks_project_id].blank?
 
-      TasksToBeDone::CreateWorker.perform_async(params[:tasks_project_id], current_user.id, invites.map(&:to_i), params[:tasks_to_be_done])
+      valid_members = members.select { |member| member.valid? && member.member_task.valid? }
+      return unless valid_members.present?
+
+      # We can take the first `member_task` here, since all tasks will have the same attributes needed
+      # for the `TasksToBeDone::CreateWorker`, ie. `project` and `tasks_to_be_done`.
+      member_task = valid_members[0].member_task
+      TasksToBeDone::CreateWorker.perform_async(member_task.id, current_user.id, valid_members.map(&:user_id))
     end
 
     def areas_of_focus

app/services/members/creator_service.rb

@@ -4,6 +4,8 @@ module Members
   # This class serves as more of an app-wide way we add/create members
   # All roads to add members should take this path.
   class CreatorService
+    include Gitlab::Experiment::Dsl
+
     class << self
       def parsed_access_level(access_level)
         access_levels.fetch(access_level) { access_level.to_i }
@@ -24,6 +26,7 @@ def initialize(source, user, access_level, **args)
     def execute
       find_or_build_member
       update_member
+      create_member_task
 
       member
     end
@@ -57,9 +60,22 @@ def member_attributes
       {
         created_by: member.created_by || current_user,
         access_level: access_level,
-        expires_at: args[:expires_at],
+        expires_at: args[:expires_at]
+      }
+    end
+
+    def create_member_task
+      return unless experiment(:invite_members_for_task).enabled?
+      return unless member.persisted?
+      return if member_task_attributes.value?(nil)
+
+      member.create_member_task(member_task_attributes)
+    end
+
+    def member_task_attributes
+      {
         tasks_to_be_done: args[:tasks_to_be_done],
-        tasks_project_id: args[:tasks_project_id]
+        project_id: args[:tasks_project_id]
       }
     end
 

app/services/members/invite_service.rb

@@ -39,6 +39,11 @@ def add_error_for_member(member)
       errors[invite_email(member)] = member.errors.full_messages.to_sentence
     end
 
+    override :create_tasks_to_be_done
+    def create_tasks_to_be_done
+      # Only create task issues for existing users. Tasks for new users are created when they signup.
+    end
+
     def invite_email(member)
       member.invite_email || member.user.email
     end

app/services/tasks_to_be_done/base_service.rb

@@ -2,11 +2,14 @@
 
 module TasksToBeDone
   class BaseService < ::IssuableBaseService
-    def initialize(project:, current_user:, assignee_ids:)
+    LABEL_PREFIX = 'tasks to be done'
+
+    def initialize(project:, current_user:, assignee_ids: [])
       params = {
         assignee_ids: assignee_ids,
         title: title,
-        description: description
+        description: description,
+        add_labels: label_name
       }
       super(project: project, current_user: current_user, params: params)
     end
@@ -24,7 +27,29 @@ def execute
     private
 
     def existing_task_issue
-      project.issues.opened.where(title: params[:title]).last # rubocop: disable CodeReuse/ActiveRecord
+      IssuesFinder.new(
+        current_user,
+        project_id: project.id,
+        state: 'opened',
+        non_archived: true,
+        label_name: label_name
+      ).execute.last
+    end
+
+    def title
+      raise NotImplementedError
+    end
+
+    def description
+      raise NotImplementedError
+    end
+
+    def label_suffix
+      raise NotImplementedError
+    end
+
+    def label_name
+      "#{LABEL_PREFIX}:#{label_suffix}"
     end
   end
 end

app/services/tasks_to_be_done/create_ci_task_service.rb

@@ -36,5 +36,9 @@ def description
         * [ ] Select **CI / CD** in the left navigation to start setting up CI / CD in your project.
       DESCRIPTION
     end
+
+    def label_suffix
+      'ci'
+    end
   end
 end

app/services/tasks_to_be_done/create_code_task_service.rb

@@ -14,6 +14,7 @@ def description
 
         **With GitLab Groups, you can:**
 
+        * Create one or multiple Projects for hosting your codebase (repositories).
         * Assemble related projects together.
         * Grant members access to several projects at once.
 
@@ -23,7 +24,6 @@ def description
 
         **Within GitLab Projects, you can**
 
-        * Create one or multiple Projects for hosting your codebase (repositories).
         * Use it as an issue tracker.
         * Collaborate on code.
         * Continuously build, test, and deploy your app with built-in GitLab CI/CD.
@@ -44,5 +44,9 @@ def description
         :tada: All done, you can close this issue!
       DESCRIPTION
     end
+
+    def label_suffix
+      'code'
+    end
   end
 end

app/services/tasks_to_be_done/create_issues_task_service.rb

@@ -35,5 +35,9 @@ def description
         That's it! You can close this issue.
       DESCRIPTION
     end
+
+    def label_suffix
+      'issues'
+    end
   end
 end

app/workers/tasks_to_be_done/create_worker.rb

@@ -5,19 +5,17 @@ class CreateWorker
     include ApplicationWorker
 
     data_consistency :always
-
-    sidekiq_options retry: 3
-
     idempotent!
     feature_category :onboarding
     urgency :low
     worker_resource_boundary :cpu
 
-    def perform(project_id, current_user_id, assignee_ids, tasks_to_be_done)
-      project = Project.find(project_id)
+    def perform(member_task_id, current_user_id, assignee_ids = [])
+      member_task = MemberTask.find(member_task_id)
       current_user = User.find(current_user_id)
+      project = member_task.project
 
-      tasks_to_be_done.each do |task|
+      member_task.tasks_to_be_done.each do |task|
         service_class(task)
           .new(project: project, current_user: current_user, assignee_ids: assignee_ids)
           .execute

config/feature_flags/experiment/invite_members_for_task.yml

@@ -2,7 +2,7 @@
 name: invite_members_for_task
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69299
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339747
-milestone: '14.4'
+milestone: '14.5'
 type: experiment
 group: group::activation
 default_enabled: false

db/migrate/20210921120535_add_tasks_to_be_done_to_members.rb

-# frozen_string_literal: true
-
-class AddTasksToBeDoneToMembers < Gitlab::Database::Migration[1.0]
-  def change
-    add_column :members, :tasks_to_be_done, :integer, array: true, null: true
-    add_column :members, :tasks_project_id, :bigint, null: true
-  end
-end

db/migrate/20210921155501_add_task_project_foreign_key_to_members.rb

-# frozen_string_literal: true
-
-class AddTaskProjectForeignKeyToMembers < Gitlab::Database::Migration[1.0]
-  disable_ddl_transaction!
-
-  INDEX_NAME = 'index_members_on_tasks_project_id'
-
-  def up
-    add_concurrent_index :members, :tasks_project_id, name: INDEX_NAME
-    add_concurrent_foreign_key :members, :projects, column: :tasks_project_id, on_delete: :nullify
-  end
-
-  def down
-    with_lock_retries do
-      remove_foreign_key_if_exists :members, column: :tasks_project_id
-    end
-
-    remove_concurrent_index_by_name :members, name: INDEX_NAME
-  end
-end