FIX: use hijack for processing bulk invites (#7679)

FIX: do not store bulk invite CSV file on server
This commit is contained in:
Arpit Jalan 2019-06-04 20:19:46 +05:30 committed by GitHub
parent ecc9c76692
commit e7fe7010b8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 52 additions and 75 deletions

View File

@ -172,24 +172,31 @@ class InvitesController < ApplicationController
def upload_csv def upload_csv
guardian.ensure_can_bulk_invite_to_forum!(current_user) guardian.ensure_can_bulk_invite_to_forum!(current_user)
file = params[:file] || params[:files].first hijack do
name = params[:name] || File.basename(file.original_filename, ".*") begin
extension = File.extname(file.original_filename) file = params[:file] || params[:files].first
begin if File.read(file.tempfile).scan(/\n/).count.to_i > 50000
data = if extension.downcase == ".csv" return render json: failed_json.merge(errors: [I18n.t("bulk_invite.max_rows")]), status: 422
path = Invite.create_csv(file, name) end
Jobs.enqueue(:bulk_invite, filename: "#{name}#{extension}", current_user_id: current_user.id)
{ url: path } invites = []
else CSV.foreach(file.tempfile) do |row|
failed_json.merge(errors: [I18n.t("bulk_invite.file_should_be_csv")]) invite_hash = { email: row[0], groups: row[1], topic_id: row[2] }
if invite_hash[:email].present?
invites.push(invite_hash)
end
end
if invites.present?
Jobs.enqueue(:bulk_invite, invites: invites, current_user_id: current_user.id)
render json: success_json
else
render json: failed_json.merge(errors: [I18n.t("bulk_invite.error")]), status: 422
end
rescue
render json: failed_json.merge(errors: [I18n.t("bulk_invite.error")]), status: 422
end end
rescue
failed_json.merge(errors: [I18n.t("bulk_invite.error")])
end end
MessageBus.publish("/uploads/csv", data.as_json, user_ids: [current_user.id])
render json: success_json
end end
def fetch_username def fetch_username

View File

@ -1,6 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
require 'csv'
require_dependency 'system_message' require_dependency 'system_message'
module Jobs module Jobs
@ -18,48 +17,38 @@ module Jobs
end end
def execute(args) def execute(args)
filename = args[:filename] invites = args[:invites]
raise Discourse::InvalidParameters.new(:filename) if filename.blank? raise Discourse::InvalidParameters.new(:invites) if invites.blank?
@current_user = User.find_by(id: args[:current_user_id]) @current_user = User.find_by(id: args[:current_user_id])
raise Discourse::InvalidParameters.new(:current_user_id) unless @current_user raise Discourse::InvalidParameters.new(:current_user_id) unless @current_user
@guardian = Guardian.new(@current_user) @guardian = Guardian.new(@current_user)
csv_path = "#{Invite.base_directory}/#{filename}" process_invites(invites)
# read csv file, and send out invitations
read_csv_file(csv_path)
ensure ensure
# send notification to user regarding progress
notify_user notify_user
File.delete(csv_path) if csv_path
end end
private private
def read_csv_file(csv_path) def process_invites(invites)
file = File.open(csv_path, encoding: 'bom|utf-8') invites.each do |invite|
CSV.new(file).each do |csv_info| if (EmailValidator.email_regex =~ invite[:email])
if csv_info[0] # email is valid
if (EmailValidator.email_regex =~ csv_info[0]) send_invite(invite)
# email is valid @sent += 1
send_invite(csv_info, $INPUT_LINE_NUMBER) else
@sent += 1 # invalid email
else save_log "Invalid Email '#{invite[:email]}"
# invalid email @failed += 1
save_log "Invalid Email '#{csv_info[0]}' at line number '#{$INPUT_LINE_NUMBER}'"
@failed += 1
end
end end
end end
rescue Exception => e rescue Exception => e
save_log "Bulk Invite Process Failed -- '#{e.message}'" save_log "Bulk Invite Process Failed -- '#{e.message}'"
@failed += 1 @failed += 1
ensure
file&.close
end end
def get_groups(group_names, csv_line_number) def get_groups(group_names)
groups = [] groups = []
if group_names if group_names
@ -73,7 +62,7 @@ module Jobs
groups.push(group) groups.push(group)
else else
# invalid group # invalid group
save_log "Invalid Group '#{group_name}' at line number '#{csv_line_number}'" save_log "Invalid Group '#{group_name}'"
@failed += 1 @failed += 1
end end
} }
@ -82,13 +71,13 @@ module Jobs
groups groups
end end
def get_topic(topic_id, csv_line_number) def get_topic(topic_id)
topic = nil topic = nil
if topic_id if topic_id
topic = Topic.find_by_id(topic_id) topic = Topic.find_by_id(topic_id)
if topic.nil? if topic.nil?
save_log "Invalid Topic ID '#{topic_id}' at line number '#{csv_line_number}'" save_log "Invalid Topic ID '#{topic_id}'"
@failed += 1 @failed += 1
end end
end end
@ -96,10 +85,10 @@ module Jobs
return topic return topic
end end
def send_invite(csv_info, csv_line_number) def send_invite(invite)
email = csv_info[0] email = invite[:email]
groups = get_groups(csv_info[1], csv_line_number) groups = get_groups(invite[:groups])
topic = get_topic(csv_info[2], csv_line_number) topic = get_topic(invite[:topic_id])
begin begin
if user = User.find_by_email(email) if user = User.find_by_email(email)
@ -171,7 +160,5 @@ module Jobs
result result
end end
end end
end end

View File

@ -242,14 +242,6 @@ class Invite < ActiveRecord::Base
def self.base_directory def self.base_directory
File.join(Rails.root, "public", "uploads", "csv", RailsMultisite::ConnectionManagement.current_db) File.join(Rails.root, "public", "uploads", "csv", RailsMultisite::ConnectionManagement.current_db)
end end
def self.create_csv(file, name)
extension = File.extname(file.original_filename)
path = "#{Invite.base_directory}/#{name}#{extension}"
FileUtils.mkdir_p(Pathname.new(path).dirname)
File.open(path, "wb") { |f| f << file.tempfile.read }
path
end
end end
# == Schema Information # == Schema Information

View File

@ -218,6 +218,7 @@ en:
bulk_invite: bulk_invite:
file_should_be_csv: "The uploaded file should be of csv format." file_should_be_csv: "The uploaded file should be of csv format."
max_rows: "Maximum of 50,000 invites can be sent at a time. Try splitting the file in smaller parts."
error: "There was an error uploading that file. Please try again later." error: "There was an error uploading that file. Please try again later."
topic_invite: topic_invite:

View File

@ -1,2 +0,0 @@
test2@discourse.org
test@discourse.org,GROUP1;group2,999
1 test2 discourse.org
2 test discourse.org,GROUP1;group2,999

View File

@ -10,30 +10,22 @@ describe Jobs::BulkInvite do
fab!(:group2) { Fabricate(:group, name: 'group2') } fab!(:group2) { Fabricate(:group, name: 'group2') }
fab!(:topic) { Fabricate(:topic, id: 999) } fab!(:topic) { Fabricate(:topic, id: 999) }
let(:email) { "test@discourse.org" } let(:email) { "test@discourse.org" }
let(:basename) { "bulk_invite.csv" } let(:invites) { [{ email: 'test2@discourse.org' }, { email: 'test@discourse.org', groups: 'GROUP1;group2', topic_id: 999 }] }
let(:filename) { "#{Invite.base_directory}/#{basename}" }
before do it 'raises an error when the invites array is missing' do
Invite.create_csv(
fixture_file_upload("#{Rails.root}/spec/fixtures/csv/#{basename}"),
"bulk_invite"
)
end
it 'raises an error when the filename is missing' do
expect { Jobs::BulkInvite.new.execute(current_user_id: user.id) } expect { Jobs::BulkInvite.new.execute(current_user_id: user.id) }
.to raise_error(Discourse::InvalidParameters, /filename/) .to raise_error(Discourse::InvalidParameters, /invites/)
end end
it 'raises an error when current_user_id is not valid' do it 'raises an error when current_user_id is not valid' do
expect { Jobs::BulkInvite.new.execute(filename: filename) } expect { Jobs::BulkInvite.new.execute(invites: invites) }
.to raise_error(Discourse::InvalidParameters, /current_user_id/) .to raise_error(Discourse::InvalidParameters, /current_user_id/)
end end
it 'creates the right invites' do it 'creates the right invites' do
described_class.new.execute( described_class.new.execute(
current_user_id: admin.id, current_user_id: admin.id,
filename: basename, invites: invites
) )
invite = Invite.last invite = Invite.last
@ -53,7 +45,7 @@ describe Jobs::BulkInvite do
described_class.new.execute( described_class.new.execute(
current_user_id: admin.id, current_user_id: admin.id,
filename: basename, invites: invites
) )
invite = Invite.last invite = Invite.last
@ -70,7 +62,7 @@ describe Jobs::BulkInvite do
described_class.new.execute( described_class.new.execute(
current_user_id: user.id, current_user_id: user.id,
filename: basename invites: invites
) )
invite = Invite.last invite = Invite.last
@ -90,7 +82,7 @@ describe Jobs::BulkInvite do
expect do expect do
described_class.new.execute( described_class.new.execute(
current_user_id: admin.id, current_user_id: admin.id,
filename: basename invites: invites
) )
end.to change { Invite.count }.by(1) end.to change { Invite.count }.by(1)