From 8a7fe3b2769ca5b6f026d5c2d73c30bbbefdaefe Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Tue, 22 Dec 2020 14:28:07 -0300 Subject: [PATCH] FIX: Don't enqueue imported users when there're multiple custom fields. (#11559) My initial implementation didn't consider this case. We should skip imported users if the "imported_id" field is present, even if there're other custom fields. --- app/jobs/scheduled/enqueue_suspect_users.rb | 13 ++++++++++--- spec/jobs/enqueue_suspect_users_spec.rb | 8 ++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/app/jobs/scheduled/enqueue_suspect_users.rb b/app/jobs/scheduled/enqueue_suspect_users.rb index b88118a3a31..e056020f3c4 100644 --- a/app/jobs/scheduled/enqueue_suspect_users.rb +++ b/app/jobs/scheduled/enqueue_suspect_users.rb @@ -19,9 +19,16 @@ module Jobs .where("user_stats.posts_read_count <= 1 AND user_stats.topics_entered <= 1") .joins("LEFT OUTER JOIN reviewables r ON r.target_id = users.id AND r.target_type = 'User'") .where('r.id IS NULL') - .joins('LEFT OUTER JOIN user_custom_fields ucf ON users.id = ucf.user_id') - .group('users.id, ucf.id') - .having('ucf.id IS NULL OR NOT bool_or(ucf.name = ?)', 'import_id') + .joins( + <<~SQL + LEFT OUTER JOIN ( + SELECT user_id + FROM user_custom_fields + WHERE user_custom_fields.name = 'import_id' + ) AS ucf ON ucf.user_id = users.id + SQL + ) + .where('ucf.user_id IS NULL') .limit(10) users.each do |user| diff --git a/spec/jobs/enqueue_suspect_users_spec.rb b/spec/jobs/enqueue_suspect_users_spec.rb index b1b30e77952..d3dbc79b589 100644 --- a/spec/jobs/enqueue_suspect_users_spec.rb +++ b/spec/jobs/enqueue_suspect_users_spec.rb @@ -80,5 +80,13 @@ describe Jobs::EnqueueSuspectUsers do expect(ReviewableUser.where(target: suspect_user).exists?).to eq(true) end + + it 'ignores imported users even if they have multiple custom fields' do + suspect_user.upsert_custom_fields({ field_a: 'value', field_b: 'value', import_id: 'fake_id' }) + + subject.execute({}) + + expect(ReviewableUser.where(target: suspect_user).exists?).to eq(false) + end end end