moving destroy of problems into background jobs (errbit#1151)

scicco · stevecrozz · commit 3e1f0d89f7dc · 2016-12-22T10:40:29.000-08:00
diff --git a/Gemfile b/Gemfile
@@ -112,5 +112,7 @@ gem 'jquery-rails', '~> 2.1.4'
 gem 'pjax_rails'
 gem 'underscore-rails'
 
+gem 'sucker_punch'
+
 ENV['USER_GEMFILE'] ||= './UserGemfile'
 eval_gemfile ENV['USER_GEMFILE'] if File.exist?(ENV['USER_GEMFILE'])
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -388,6 +388,8 @@ GEM
       colorize (>= 0.7.0)
       net-scp (>= 1.1.2)
       net-ssh (>= 2.8.0)
+    sucker_punch (2.0.2)
+      concurrent-ruby (~> 1.0.0)
     term-ansicolor (1.3.2)
       tins (~> 1.0)
     therubyracer (0.12.2)
@@ -487,6 +489,7 @@ DEPENDENCIES
   rubocop
   rushover
   sass-rails
+  sucker_punch
   therubyracer
   timecop
   uglifier
diff --git a/app/controllers/problems_controller.rb b/app/controllers/problems_controller.rb
@@ -114,14 +114,14 @@ def unmerge_several
   end
 
   def destroy_several
-    nb_problem_destroy = ProblemDestroy.execute(selected_problems)
-    flash[:notice] = "#{I18n.t(:n_errs_have, count: nb_problem_destroy)} #{I18n.t('n_errs_have.been_deleted')}."
+    DestroyProblemsByIdJob.perform_later(selected_problems_ids)
+    flash[:notice] = "#{I18n.t(:n_errs, count: selected_problems.size)} #{I18n.t('n_errs.will_be_deleted')}."
     redirect_to :back
   end
 
   def destroy_all
-    nb_problem_destroy = ProblemDestroy.execute(app.problems)
-    flash[:success] = "#{I18n.t(:n_errs_have, count: nb_problem_destroy)} #{I18n.t('n_errs_have.been_deleted')}."
+    DestroyProblemsByAppJob.perform_later(app.id)
+    flash[:success] = "#{I18n.t(:n_errs, count: app.problems.count)} #{I18n.t('n_errs.will_be_deleted')}."
     redirect_to :back
   rescue ActionController::RedirectBackError
     redirect_to app_path(app)
diff --git a/app/controllers/problems_searcher.rb b/app/controllers/problems_searcher.rb
@@ -25,6 +25,10 @@ module ProblemsSearcher
       Array(Problem.find(err_ids))
     end
 
+    expose(:selected_problems_ids) do
+      selected_problems.map(&:id).map(&:to_s)
+    end
+
     expose(:err_ids) do
       (params[:problems] || []).compact
     end
diff --git a/app/jobs/destroy_problems_by_app_job.rb b/app/jobs/destroy_problems_by_app_job.rb
@@ -0,0 +1,8 @@
+class DestroyProblemsByAppJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(app_id)
+    app = App.find_by(id: app_id)
+    ::ProblemDestroy.execute(app.problems)
+  end
+end
diff --git a/app/jobs/destroy_problems_by_id_job.rb b/app/jobs/destroy_problems_by_id_job.rb
@@ -0,0 +1,12 @@
+class DestroyProblemsByIdJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(problem_ids)
+    bson_problem_ids = []
+    problem_ids.each do |id|
+      bson_problem_ids << BSON::ObjectId.from_string(id)
+    end
+    problems = Problem.find(bson_problem_ids).to_a
+    ::ProblemDestroy.execute(problems)
+  end
+end
diff --git a/config/application.rb b/config/application.rb
@@ -48,5 +48,7 @@ class Application < Rails::Application
 
     # Configure Devise mailer to use our mailer layout.
     config.to_prepare { Devise::Mailer.layout 'mailer' }
+
+    config.active_job.queue_adapter = :sucker_punch
   end
 end
diff --git a/config/initializers/sucker_punch.rb b/config/initializers/sucker_punch.rb
@@ -0,0 +1 @@
+require 'sucker_punch/async_syntax'
diff --git a/config/locales/en.yml b/config/locales/en.yml
@@ -10,7 +10,10 @@ en:
         success: "Good news everyone! '%{app_name}' was successfully updated."
       destroy:
         success: "'%{app_name}' was successfully destroyed."
-
+  n_errs:
+    one:   "%{count} error"
+    other: "%{count} errors"
+    will_be_deleted: will be deleted
   n_errs_have:
     one:   "%{count} error has"
     other: "%{count} errors have"
diff --git a/config/locales/pt-BR.yml b/config/locales/pt-BR.yml
@@ -11,6 +11,10 @@ pt-BR:
       destroy:
         success: "'%{app_name}' foi destruído com sucesso."
 
+  n_errs:
+    one:   "%{count} erro"
+    other: "%{count} erros"
+    will_be_deleted: serà excluído(s)
   n_errs_have:
     one:   "%{count} erro foi"
     other: "%{count} erros foram"
diff --git a/spec/controllers/problems_controller_spec.rb b/spec/controllers/problems_controller_spec.rb
@@ -434,7 +434,7 @@
 
       it "should display a message" do
         put :destroy_all, app_id: @app.id
-        expect(request.flash[:success]).to match(/been deleted/)
+        expect(request.flash[:success]).to match(/be deleted/)
       end
 
       it "should redirect back to the app page" do
diff --git a/spec/jobs/destroy_problems_by_app_job_spec.rb b/spec/jobs/destroy_problems_by_app_job_spec.rb
@@ -0,0 +1,33 @@
+RSpec.describe DestroyProblemsByAppJob, type: :job do
+  before do
+    @app      = Fabricate(:app)
+    @problem1 = Fabricate(:problem, app: @app)
+    @problem2 = Fabricate(:problem, app: @app)
+  end
+
+  it "destroys all problems" do
+    expect do
+      DestroyProblemsByAppJob.perform_later(@app.id)
+    end.to change(Problem, :count).by(-2)
+    expect(@app.problems.count).to eq 0
+  end
+
+  it "destroys all problems, even with a large amount of them" do
+    app2 = Fabricate(:app)
+    500.times do
+      Fabricate(:problem, app: app2)
+    end
+    expect do
+      DestroyProblemsByAppJob.perform_later(app2.id)
+    end.to change(Problem, :count).by(-500)
+    expect(app2.problems.count).to eq 0
+  end
+
+  it "should work with a fresh new application" do
+    app2 = Fabricate(:app)
+    expect do
+      DestroyProblemsByAppJob.perform_later(app2.id)
+    end.to change(Problem, :count).by(0)
+    expect(app2.problems.count).to eq 0
+  end
+end
diff --git a/spec/jobs/destroy_problems_by_id_job_spec.rb b/spec/jobs/destroy_problems_by_id_job_spec.rb
@@ -0,0 +1,37 @@
+RSpec.describe DestroyProblemsByIdJob, type: :job do
+  before do
+    @app      = Fabricate(:app)
+    @problem1 = Fabricate(:problem, app: @app)
+    @problem2 = Fabricate(:problem, app: @app)
+    @problem3 = Fabricate(:problem, app: @app)
+    @problem4 = Fabricate(:problem, app: @app)
+    @problem5 = Fabricate(:problem, app: @app)
+    @problem6 = Fabricate(:problem, app: @app)
+  end
+
+  it "destroys all selected problems" do
+    expect do
+      DestroyProblemsByIdJob.perform_later([@problem2.id.to_s, @problem5.id.to_s, @problem1.id.to_s])
+    end.to change(Problem, :count).by(-3)
+    expect(@app.problems.count).to eq 3
+  end
+
+  it "destroys all selected problems, even with a large amount of them" do
+    app2 = Fabricate(:app)
+    500.times do
+      Fabricate(:problem, app: app2)
+    end
+    expect do
+      DestroyProblemsByIdJob.perform_later(app2.problems.to_a.sample(100).map(&:id).map(&:to_s))
+    end.to change(Problem, :count).by(-100)
+    expect(app2.problems.count).to eq 400
+  end
+
+  it "should work with a fresh new application" do
+    app2 = Fabricate(:app)
+    expect do
+      DestroyProblemsByIdJob.perform_later([])
+    end.to change(Problem, :count).by(0)
+    expect(app2.problems.count).to eq 0
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -27,6 +27,7 @@
 require 'xmpp4r/muc'
 require 'mongoid-rspec'
 require 'fabrication'
+require 'sucker_punch/testing/inline'
 require 'errbit_plugin/mock_issue_tracker'
 
 # Requires supporting files with custom matchers and macros, etc,
