From c61dd3eab4dbdb0ae634e0d1fa44b672eb2129c3 Mon Sep 17 00:00:00 2001
From: Ryan Lue <hello@ryanlue.com>
Date: Thu, 19 Jul 2018 18:25:20 +0800
Subject: [PATCH] Specify updated_by_id in LDAP deactivation sequencer unit
 (fixes #2111)

---
 .../unit/import/ldap/users/lost/deactivate.rb | 15 ++++------
 .../import/ldap/users/lost/deactivate_spec.rb | 29 +++++++++++++++++++
 2 files changed, 34 insertions(+), 10 deletions(-)
 create mode 100644 spec/lib/sequencer/unit/import/ldap/users/lost/deactivate_spec.rb

diff --git a/lib/sequencer/unit/import/ldap/users/lost/deactivate.rb b/lib/sequencer/unit/import/ldap/users/lost/deactivate.rb
index 29d3a97b7..cfe39998d 100644
--- a/lib/sequencer/unit/import/ldap/users/lost/deactivate.rb
+++ b/lib/sequencer/unit/import/ldap/users/lost/deactivate.rb
@@ -10,16 +10,11 @@ class Sequencer
               def process
                 return if dry_run
 
-                # we need to update in slices since some DBs
-                # have a limit for IN length
-                lost_ids.each_slice(5000) do |slice|
-
-                  # we need to instanciate every entry and set
-                  # the active state this way to send notifications
-                  # to the client
-                  ::User.where(id: slice).each do |user|
-                    user.update!(active: false)
-                  end
+                # Why not use `#update_all`?
+                # It bypasses validations/callbacks
+                # (which are used to send notifications to the client)
+                ::User.where(id: lost_ids).find_each do |user|
+                  user.update!(active: false, updated_by_id: 1)
                 end
               end
             end
diff --git a/spec/lib/sequencer/unit/import/ldap/users/lost/deactivate_spec.rb b/spec/lib/sequencer/unit/import/ldap/users/lost/deactivate_spec.rb
new file mode 100644
index 000000000..979e66131
--- /dev/null
+++ b/spec/lib/sequencer/unit/import/ldap/users/lost/deactivate_spec.rb
@@ -0,0 +1,29 @@
+require 'rails_helper'
+
+RSpec.describe Sequencer::Unit::Import::Ldap::Users::Lost::Deactivate, sequencer: :unit do
+  let!(:lost_users) { create_list(:user, sample_length, attributes) }
+  let(:sample_length) { 2 }
+
+  context 'when provided ids of active users' do
+    let(:attributes) { { active: true } }
+
+    it 'deactivates them' do
+      expect { process(lost_ids: lost_users.pluck(:id), dry_run: false) }
+        .to change { lost_users.each(&:reload).pluck(:active) }.to(Array.new(sample_length, false))
+    end
+  end
+
+  context 'when provided ids of users with any `updated_by_id`' do
+    # ordinarily, a History log's created_by_id is based on this value (or UserInfo.current_user_id),
+    # but this Sequencer unit is expected to override it
+    let(:attributes) { { updated_by_id: 2 } }
+
+    it 'enforces created_by_id => 1 in newly created History logs' do
+      expect { process(lost_ids: lost_users.pluck(:id), dry_run: false) }
+        .to change { History.count }.by(sample_length)
+
+      expect(History.last(sample_length).pluck(:created_by_id))
+        .to eq(Array.new(sample_length, 1))
+    end
+  end
+end