[Multiple issue assignee] Support for both schemas to able to live migrate themfix_assignee_migration

10 files changed, 53 insertions, 59 deletions

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 760ba246e3e..3b0c0c84e21 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -31,6 +31,16 @@ class Projects::IssuesController < Projects::ApplicationController
     @collection_type    = "Issue"
     @issues             = issues_collection
     @issues             = @issues.page(params[:page])
+
+    ### Live assignee migration
+    results = @issues.map(&:migrate_assignee)
+    if results.any?
+      # Reload the collection
+      @issues = issues_collection
+      @issues = @issues.page(params[:page])
+    end
+    ###
+
     @issuable_meta_data = issuable_meta_data(@issues, @collection_type)
 
     if @issues.out_of_range? && @issues.total_pages != 0
@@ -225,8 +235,13 @@ class Projects::IssuesController < Projects::ApplicationController
   protected
 
   def issue
-    # The Sortable default scope causes performance issues when used with find_by
-    @noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take!
+    unless @issue
+      # The Sortable default scope causes performance issues when used with find_by
+      @noteable = @issue = @project.issues.where(iid: params[:id]).reorder(nil).take!
+      @issue.migrate_assignee
+    end
+
+    @issue
   end
   alias_method :subscribable_resource, :issue
   alias_method :issuable, :issue
diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb
index b4c074bc69c..cd7099c6c56 100644
--- a/app/finders/issues_finder.rb
+++ b/app/finders/issues_finder.rb
@@ -48,6 +48,7 @@ class IssuesFinder < IssuableFinder
       OR (issues.confidential = TRUE
         AND (issues.author_id = :user_id
           OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id)
+          OR issues.assignee_id = :user_id
           OR issues.project_id IN(:project_ids)))',
       user_id: user.id,
       project_ids: user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
diff --git a/app/models/concerns/assignee_live_migration.rb b/app/models/concerns/assignee_live_migration.rb
new file mode 100644
index 00000000000..bf0fbc7a9d1
--- /dev/null
+++ b/app/models/concerns/assignee_live_migration.rb
@@ -0,0 +1,20 @@
+module AssigneeLiveMigration
+  # This method is a part of live migration concept. We need to migrate assignee_id
+  # to a separate table issue_assignees
+  def migrate_assignee
+    if assignee_needs_to_be_migrated?
+      Issue.transaction do
+        IssueAssignee.create(issue_id: id, user_id: assignee_id)
+        update(assignee_id: nil)
+      end
+
+      return true
+    end
+
+    false
+  end
+
+  def assignee_needs_to_be_migrated?
+    assignee_id
+  end
+end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index ecfc33ec1a1..89e269f7872 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -9,6 +9,7 @@ class Issue < ActiveRecord::Base
   include Spammable
   include FasterCacheKeys
   include RelativePositioning
+  include AssigneeLiveMigration
 
   DueDateStruct = Struct.new(:title, :name).freeze
   NoDueDate     = DueDateStruct.new('No Due Date', '0').freeze
@@ -31,9 +32,9 @@ class Issue < ActiveRecord::Base
 
   scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
 
-  scope :assigned, -> { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') }
-  scope :unassigned, -> { where('NOT EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') }
-  scope :assigned_to, ->(u) { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = ? AND issue_id = issues.id)', u.id)}
+  scope :assigned, -> { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id) OR issues.assignee_id IS NOT NULL') }
+  scope :unassigned, -> { where('NOT EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id) OR issues.assignee_id IS NULL') }
+  scope :assigned_to, ->(u) { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id) OR issues.assignee_id = :user_id', user_id: u.id)}
 
   scope :without_due_date, -> { where(due_date: nil) }
   scope :due_before, ->(date) { where('issues.due_date < ?', date) }
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index cd9f9a4a16e..1d558f5dd12 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -3,6 +3,7 @@ module Issues
     include SpamCheckService
 
     def execute(issue)
+      issue.migrate_assignee
       handle_move_between_iids(issue)
       filter_spam_check_params
       update(issue)
diff --git a/db/migrate/20170320173259_migrate_assignees.rb b/db/migrate/20170320173259_migrate_assignees.rb
deleted file mode 100644
index ba8edbd7d32..00000000000
--- a/db/migrate/20170320173259_migrate_assignees.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
-class MigrateAssignees < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  # Set this constant to true if this migration requires downtime.
-  DOWNTIME = false
-
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
-  # When using the methods "add_concurrent_index" or "add_column_with_default"
-  # you must disable the use of transactions as these methods can not run in an
-  # existing transaction. When using "add_concurrent_index" make sure that this
-  # method is the _only_ method called in the migration, any other changes
-  # should go in a separate migration. This ensures that upon failure _only_ the
-  # index creation fails and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  disable_ddl_transaction!
-
-  def up
-    # Optimisation: this accounts for most of the invalid assignee IDs on GitLab.com
-    update_column_in_batches(:issues, :assignee_id, nil) do |table, query|
-      query.where(table[:assignee_id].eq(0))
-    end
-
-    users = Arel::Table.new(:users)
-
-    update_column_in_batches(:issues, :assignee_id, nil) do |table, query|
-      query.where(table[:assignee_id].not_eq(nil)\
-        .and(
-          users.project("true").where(users[:id].eq(table[:assignee_id])).exists.not
-        ))
-    end
-
-    execute <<-EOF
-      INSERT INTO issue_assignees(issue_id, user_id)
-      SELECT id, assignee_id FROM issues WHERE assignee_id IS NOT NULL
-    EOF
-  end
-
-  def down
-    execute <<-EOF
-      DELETE FROM issue_assignees
-    EOF
-  end
-end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 226a7ddd50e..fb100f3f26c 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -83,7 +83,9 @@ module API
     end
 
     def find_project_issue(iid)
-      IssuesFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
+      issue = IssuesFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
+      issue.migrate_assignee
+      issue
     end
 
     def find_project_merge_request(iid)
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 78db960ae28..962609ed64b 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -14,6 +14,8 @@ module API
 
         issues = IssuesFinder.new(current_user, args).execute
 
+        issues.each(&:migrate_assignee)
+
         issues.reorder(args[:order_by] => args[:sort])
       end
 
diff --git a/lib/api/v3/helpers.rb b/lib/api/v3/helpers.rb
index 0f234d4cdad..2683e0b4c92 100644
--- a/lib/api/v3/helpers.rb
+++ b/lib/api/v3/helpers.rb
@@ -2,7 +2,9 @@ module API
   module V3
     module Helpers
       def find_project_issue(id)
-        IssuesFinder.new(current_user, project_id: user_project.id).find(id)
+        issue = IssuesFinder.new(current_user, project_id: user_project.id).find(id)
+        issue.migrate_assignee
+        issue
       end
 
       def find_project_merge_request(id)
diff --git a/lib/api/v3/issues.rb b/lib/api/v3/issues.rb
index cb371fdbab8..14e918502d3 100644
--- a/lib/api/v3/issues.rb
+++ b/lib/api/v3/issues.rb
@@ -22,6 +22,8 @@ module API
 
           issues = IssuesFinder.new(current_user, args).execute.inc_notes_with_associations
 
+          issues.each(&:migrate_assignee)
+
           if !match_all_labels && labels.present?
             issues = issues.includes(:labels).where('labels.title' => labels.split(','))
           end