From 4673f31c75b3231815025cfb930cfb5cbfd16fc8 Mon Sep 17 00:00:00 2001
From: romanrizzi <rizziromanalejandro@gmail.com>
Date: Thu, 27 Feb 2020 11:07:46 -0300
Subject: [PATCH] FIX: Bulk badge awards should work even if the CSV has nil
 values

---
 app/controllers/admin/badges_controller.rb     |  8 ++++++--
 .../fixtures/csv/usernames_with_nil_values.csv |  4 ++++
 spec/requests/admin/badges_controller_spec.rb  | 18 ++++++++++++++----
 3 files changed, 24 insertions(+), 6 deletions(-)
 create mode 100644 spec/fixtures/csv/usernames_with_nil_values.csv

diff --git a/app/controllers/admin/badges_controller.rb b/app/controllers/admin/badges_controller.rb
index 2c1846d3dae..e3aca683d64 100644
--- a/app/controllers/admin/badges_controller.rb
+++ b/app/controllers/admin/badges_controller.rb
@@ -55,8 +55,12 @@ class Admin::BadgesController < Admin::AdminController
       csv.rewind
 
       csv.each_line do |email_line|
-        batch.concat CSV.parse_line(email_line)
-        line_number += 1
+        line = CSV.parse_line(email_line).first
+
+        if line.present?
+          batch << line
+          line_number += 1
+        end
 
         # Split the emails in batches of 200 elements.
         full_batch = csv.lineno % (BadgeGranter::MAX_ITEMS_FOR_DELTA * batch_number) == 0
diff --git a/spec/fixtures/csv/usernames_with_nil_values.csv b/spec/fixtures/csv/usernames_with_nil_values.csv
new file mode 100644
index 00000000000..b35b163ca50
--- /dev/null
+++ b/spec/fixtures/csv/usernames_with_nil_values.csv
@@ -0,0 +1,4 @@
+username1,
+username2,
+username3,
+,,
diff --git a/spec/requests/admin/badges_controller_spec.rb b/spec/requests/admin/badges_controller_spec.rb
index 06d294bcc0d..f54875a21aa 100644
--- a/spec/requests/admin/badges_controller_spec.rb
+++ b/spec/requests/admin/badges_controller_spec.rb
@@ -179,6 +179,8 @@ describe Admin::BadgesController do
     end
 
     describe '#mass_award' do
+      before { @user = Fabricate(:user, email: 'user1@test.com', username: 'username1') }
+
       it 'does nothing when there is no file' do
         post "/admin/badges/award/#{badge.id}.json", params: { file: '' }
 
@@ -202,23 +204,31 @@ describe Admin::BadgesController do
       it 'awards the badge using a list of user emails' do
         Jobs.run_immediately!
 
-        user = Fabricate(:user, email: 'user1@test.com')
         file = file_from_fixtures('user_emails.csv', 'csv')
 
         post "/admin/badges/award/#{badge.id}.json", params: { file: fixture_file_upload(file) }
 
-        expect(UserBadge.exists?(user: user, badge: badge)).to eq(true)
+        expect(UserBadge.exists?(user: @user, badge: badge)).to eq(true)
       end
 
       it 'awards the badge using a list of usernames' do
         Jobs.run_immediately!
 
-        user = Fabricate(:user, username: 'username1')
         file = file_from_fixtures('usernames.csv', 'csv')
 
         post "/admin/badges/award/#{badge.id}.json", params: { file: fixture_file_upload(file) }
 
-        expect(UserBadge.exists?(user: user, badge: badge)).to eq(true)
+        expect(UserBadge.exists?(user: @user, badge: badge)).to eq(true)
+      end
+
+      it 'works with a CSV containing nil values' do
+        Jobs.run_immediately!
+
+        file = file_from_fixtures('usernames_with_nil_values.csv', 'csv')
+
+        post "/admin/badges/award/#{badge.id}.json", params: { file: fixture_file_upload(file) }
+
+        expect(UserBadge.exists?(user: @user, badge: badge)).to eq(true)
       end
     end
   end