Merge pull request #4587 from techAPJ/invite-upload

FIX: simplify CSV file upload

app/assets/javascripts/discourse/components/csv-uploader.js.es6

@@ -0,0 +1,17 @@
+import computed from "ember-addons/ember-computed-decorators";
+import UploadMixin from "discourse/mixins/upload";
+
+export default Em.Component.extend(UploadMixin, {
+  type: "csv",
+  tagName: "span",
+  uploadUrl: "/invites/upload_csv",
+
+  @computed("uploading")
+  uploadButtonText(uploading) {
+    return uploading ? I18n.t("uploading") : I18n.t("user.invited.bulk_invite.text");
+  },
+
+  uploadDone() {
+    bootbox.alert(I18n.t("user.invited.bulk_invite.success"));
+  }
+});

app/assets/javascripts/discourse/controllers/user-invited-show.js.es6

@@ -18,8 +18,6 @@ export default Ember.Controller.extend({
     this.set('searchTerm', '');
   },
 
-  uploadText: function() { return I18n.t("user.invited.bulk_invite.text"); }.property(),
-
   /**
     Observe the search term box with a debouncer and change the results.
 

app/assets/javascripts/discourse/routes/user-invited-show.js.es6

@@ -33,14 +33,6 @@ export default Discourse.Route.extend({
     showInvite() {
       showModal("invite", { model: this.currentUser });
       this.controllerFor("invite").reset();
-    },
-
-    uploadSuccess(filename) {
-      bootbox.alert(I18n.t("user.invited.bulk_invite.success", { filename: filename }));
-    },
-
-    uploadError(filename, message) {
-      bootbox.alert(I18n.t("user.invited.bulk_invite.error", { filename: filename, message: message }));
     }
   }
 });

app/assets/javascripts/discourse/templates/components/csv-uploader.hbs

@@ -0,0 +1,7 @@
+<label class="btn" disabled={{uploading}}>
+  {{fa-icon "upload"}}&nbsp;{{uploadButtonText}}
+  <input disabled={{uploading}} type="file" accept=".csv" style="visibility: hidden; position: absolute;" />
+</label>
+{{#if uploading}}
+  <span>{{i18n 'upload_selector.uploading'}} {{uploadProgress}}%</span>
+{{/if}}

app/assets/javascripts/discourse/templates/user-invited-show.hbs

@@ -16,7 +16,7 @@
             <div class="pull-right">
               {{d-button icon="plus" action="showInvite" label="user.invited.create" class="btn"}}
               {{#if canBulkInvite}}
-                {{resumable-upload target="/invites/upload" success="uploadSuccess" error="uploadError" uploadText=uploadText}}
+                {{csv-uploader uploading=uploading}}
               {{/if}}
               {{#if showReinviteAllButton}}
                 {{#if reinvitedAll}}

app/controllers/invites_controller.rb

@@ -6,7 +6,7 @@ class InvitesController < ApplicationController
   skip_before_filter :check_xhr, :preload_json
   skip_before_filter :redirect_to_login_if_required
 
-  before_filter :ensure_logged_in, only: [:destroy, :create, :create_invite_link, :resend_invite, :resend_all_invites, :check_csv_chunk, :upload_csv_chunk]
+  before_filter :ensure_logged_in, only: [:destroy, :create, :create_invite_link, :resend_invite, :resend_all_invites, :upload_csv]
   before_filter :ensure_new_registrations_allowed, only: [:show, :redeem_disposable_invite]
   before_filter :ensure_not_logged_in, only: [:show, :redeem_disposable_invite]
 
@@ -147,48 +147,29 @@ class InvitesController < ApplicationController
     render nothing: true
   end
 
-  def check_csv_chunk
+  def upload_csv
     guardian.ensure_can_bulk_invite_to_forum!(current_user)
 
-    filename           = params.fetch(:resumableFilename)
-    identifier         = params.fetch(:resumableIdentifier)
-    chunk_number       = params.fetch(:resumableChunkNumber)
-    current_chunk_size = params.fetch(:resumableCurrentChunkSize).to_i
+    file = params[:file] || params[:files].first
+    name = params[:name] || File.basename(file.original_filename, ".*")
+    extension = File.extname(file.original_filename)
 
-    # path to chunk file
-    chunk = Invite.chunk_path(identifier, filename, chunk_number)
-    # check chunk upload status
-    status = HandleChunkUpload.check_chunk(chunk, current_chunk_size: current_chunk_size)
-
-    render nothing: true, status: status
-  end
-
-  def upload_csv_chunk
-    guardian.ensure_can_bulk_invite_to_forum!(current_user)
-
-    filename = params.fetch(:resumableFilename)
-    return render status: 415, text: I18n.t("bulk_invite.file_should_be_csv") unless (filename.to_s.end_with?(".csv") || filename.to_s.end_with?(".txt"))
-
-    file               = params.fetch(:file)
-    identifier         = params.fetch(:resumableIdentifier)
-    chunk_number       = params.fetch(:resumableChunkNumber).to_i
-    chunk_size         = params.fetch(:resumableChunkSize).to_i
-    total_size         = params.fetch(:resumableTotalSize).to_i
-    current_chunk_size = params.fetch(:resumableCurrentChunkSize).to_i
-
-    # path to chunk file
-    chunk = Invite.chunk_path(identifier, filename, chunk_number)
-    # upload chunk
-    HandleChunkUpload.upload_chunk(chunk, file: file)
-
-    uploaded_file_size = chunk_number * chunk_size
-    # when all chunks are uploaded
-    if uploaded_file_size + current_chunk_size >= total_size
-      # handle bulk_invite processing in a background thread
-      Jobs.enqueue(:bulk_invite, filename: filename, identifier: identifier, chunks: chunk_number, current_user_id: current_user.id)
+    Scheduler::Defer.later("Upload CSV") do
+      begin
+        data = if extension == ".csv"
+          path = Invite.create_csv(file, name)
+          Jobs.enqueue(:bulk_invite, filename: "#{name}.csv", current_user_id: current_user.id)
+          {url: path}
+        else
+          failed_json.merge(errors: [I18n.t("bulk_invite.file_should_be_csv")])
+        end
+      rescue
+        failed_json.merge(errors: [I18n.t("bulk_invite.error")])
+      end
+      MessageBus.publish("/uploads/csv", data.as_json, user_ids: [current_user.id])
     end
 
-    render nothing: true
+    render json: success_json
   end
 
   def fetch_username

app/jobs/regular/bulk_invite.rb

@@ -14,21 +14,12 @@ module Jobs
     end
 
     def execute(args)
-      filename     = args[:filename]
-      identifier   = args[:identifier]
-      chunks       = args[:chunks].to_i
+      filename = args[:filename]
       @current_user = User.find_by(id: args[:current_user_id])
-
-      raise Discourse::InvalidParameters.new(:filename)   if filename.blank?
-      raise Discourse::InvalidParameters.new(:identifier) if identifier.blank?
-      raise Discourse::InvalidParameters.new(:chunks)     if chunks <= 0
-
-      # merge chunks, and get csv path
-      csv_path = get_csv_path(filename, identifier, chunks)
+      raise Discourse::InvalidParameters.new(:filename) if filename.blank?
 
       # read csv file, and send out invitations
-      read_csv_file(csv_path)
-
+      read_csv_file("#{Invite.base_directory}/#{filename}")
     ensure
       # send notification to user regarding progress
       notify_user
@@ -37,17 +28,6 @@ module Jobs
       FileUtils.rm_rf(csv_path) rescue nil
     end
 
-    def get_csv_path(filename, identifier, chunks)
-      csv_path = "#{Invite.base_directory}/#{filename}"
-      tmp_csv_path = "#{csv_path}.tmp"
-      # path to tmp directory
-      tmp_directory = File.dirname(Invite.chunk_path(identifier, filename, 0))
-      # merge all chunks
-      HandleChunkUpload.merge_chunks(chunks, upload_path: csv_path, tmp_upload_path: tmp_csv_path, model: Invite, identifier: identifier, filename: filename, tmp_directory: tmp_directory)
-
-      return csv_path
-    end
-
     def read_csv_file(csv_path)
       CSV.foreach(csv_path, encoding: "iso-8859-1:UTF-8") do |csv_info|
         if csv_info[0]

app/models/invite.rb

@@ -265,8 +265,12 @@ class Invite < ActiveRecord::Base
     File.join(Rails.root, "public", "uploads", "csv", RailsMultisite::ConnectionManagement.current_db)
   end
 
-  def self.chunk_path(identifier, filename, chunk_number)
-    File.join(Invite.base_directory, "tmp", identifier, "#{filename}.part#{chunk_number}")
+  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
 

config/locales/client.en.yml

@@ -778,11 +778,9 @@ en:
         link_generated: "Invite link generated successfully!"
         valid_for: "Invite link is only valid for this email address: %{email}"
         bulk_invite:
-          none: "You haven't invited anyone here yet. You can send individual invites, or invite a bunch of people at once by <a href='https://meta.discourse.org/t/send-bulk-invites/16468'>uploading a bulk invite file</a>."
+          none: "You haven't invited anyone here yet. You can send individual invites, or invite a bunch of people at once by <a href='https://meta.discourse.org/t/send-bulk-invites/16468'>uploading a CSV file</a>."
           text: "Bulk Invite from File"
-          uploading: "Uploading..."
           success: "File uploaded successfully, you will be notified via message when the process is complete."
-          error: "There was an error uploading '{{filename}}': {{message}}"
 
       password:
         title: "Password"

config/locales/server.en.yml

@@ -134,7 +134,8 @@ en:
       <<: *errors
 
   bulk_invite:
-    file_should_be_csv: "The uploaded file should be of csv or txt format."
+    file_should_be_csv: "The uploaded file should be of csv format."
+    error: "There was an error uploading that file. Please try again later."
 
   backup:
     operation_already_running: "An operation is currently running. Can't start a new job right now."

config/routes.rb

@@ -617,12 +617,8 @@ Discourse::Application.routes.draw do
   resources :queued_posts, constraints: StaffConstraint.new
   get 'queued-posts' => 'queued_posts#index'
 
-  resources :invites do
-    collection do
-      get "upload" => "invites#check_csv_chunk"
-      post "upload" => "invites#upload_csv_chunk"
-    end
-  end
+  resources :invites
+  post "invites/upload_csv" => "invites#upload_csv"
   post "invites/reinvite" => "invites#resend_invite"
   post "invites/reinvite-all" => "invites#resend_all_invites"
   post "invites/link" => "invites#create_invite_link"

config/site_settings.yml

@@ -686,7 +686,7 @@ files:
     default: 3072
   authorized_extensions:
     client: true
-    default: 'jpg|jpeg|png|gif'
+    default: 'jpg|jpeg|png|gif|csv'
     refresh: true
     type: list
   crawl_images:

spec/controllers/invites_controller_spec.rb

@@ -367,33 +367,10 @@ describe InvitesController do
 
   end
 
-  context '.check_csv_chunk' do
+  context '.upload_csv' do
     it 'requires you to be logged in' do
       expect {
-        post :check_csv_chunk
-      }.to raise_error(Discourse::NotLoggedIn)
-    end
-
-    context 'while logged in' do
-      let(:resumableChunkNumber) { 1 }
-      let(:resumableCurrentChunkSize) { 46 }
-      let(:resumableIdentifier) { '46-discoursecsv' }
-      let(:resumableFilename) { 'discourse.csv' }
-
-      it "fails if you can't bulk invite to the forum" do
-        log_in
-        post :check_csv_chunk, resumableChunkNumber: resumableChunkNumber, resumableCurrentChunkSize: resumableCurrentChunkSize.to_i, resumableIdentifier: resumableIdentifier, resumableFilename: resumableFilename
-        expect(response).not_to be_success
-      end
-
-    end
-
-  end
-
-  context '.upload_csv_chunk' do
-    it 'requires you to be logged in' do
-      expect {
-        post :upload_csv_chunk
+        xhr :post, :upload_csv
       }.to raise_error(Discourse::NotLoggedIn)
     end
 
@@ -402,27 +379,19 @@ describe InvitesController do
       let(:file) do
         ActionDispatch::Http::UploadedFile.new({ filename: 'discourse.csv', tempfile: csv_file })
       end
-      let(:resumableChunkNumber) { 1 }
-      let(:resumableChunkSize) { 1048576 }
-      let(:resumableCurrentChunkSize) { 46 }
-      let(:resumableTotalSize) { 46 }
-      let(:resumableType) { 'text/csv' }
-      let(:resumableIdentifier) { '46-discoursecsv' }
-      let(:resumableFilename) { 'discourse.csv' }
-      let(:resumableRelativePath) { 'discourse.csv' }
+      let(:filename) { 'discourse.csv' }
 
       it "fails if you can't bulk invite to the forum" do
         log_in
-        post :upload_csv_chunk, file: file, resumableChunkNumber: resumableChunkNumber.to_i, resumableChunkSize: resumableChunkSize.to_i, resumableCurrentChunkSize: resumableCurrentChunkSize.to_i, resumableTotalSize: resumableTotalSize.to_i, resumableType: resumableType, resumableIdentifier: resumableIdentifier, resumableFilename: resumableFilename
+        xhr :post, :upload_csv, file: file, name: filename
         expect(response).not_to be_success
       end
 
-      it "allows admins to bulk invite" do
+      it "allows admin to bulk invite" do
         log_in(:admin)
-        post :upload_csv_chunk, file: file, resumableChunkNumber: resumableChunkNumber.to_i, resumableChunkSize: resumableChunkSize.to_i, resumableCurrentChunkSize: resumableCurrentChunkSize.to_i, resumableTotalSize: resumableTotalSize.to_i, resumableType: resumableType, resumableIdentifier: resumableIdentifier, resumableFilename: resumableFilename
+        xhr :post, :upload_csv, file: file, name: filename
         expect(response).to be_success
       end
-
     end
 
   end

spec/jobs/bulk_invite_spec.rb

@@ -5,15 +5,8 @@ describe Jobs::BulkInvite do
   context '.execute' do
 
     it 'raises an error when the filename is missing' do
-      expect { Jobs::BulkInvite.new.execute(identifier: '46-discoursecsv', chunks: '1') }.to raise_error(Discourse::InvalidParameters)
-    end
-
-    it 'raises an error when the identifier is missing' do
-      expect { Jobs::BulkInvite.new.execute(filename: 'discourse.csv', chunks: '1') }.to raise_error(Discourse::InvalidParameters)
-    end
-
-    it 'raises an error when the chunks is missing' do
-      expect { Jobs::BulkInvite.new.execute(filename: 'discourse.csv', identifier: '46-discoursecsv') }.to raise_error(Discourse::InvalidParameters)
+      user = Fabricate(:user)
+      expect { Jobs::BulkInvite.new.execute(current_user_id: user.id) }.to raise_error(Discourse::InvalidParameters)
     end
 
     context '.read_csv_file' do