Replace too long list of arguments with keywords

Rubocop was complaining about too many arguments. But
`ApplicationJob#perform` needs all arguments handled in one call. While
we could allow the `perform` method generally to have more arguments,
there could be other methods called `perform` which should still be
scrutinised. Instead, it seems acceptable to me to have more arguments
as long as they are clearly named as keyword arguments. Rails uses this
a lot to document all options including their default values, for
example in Active Storage. It's better then bundling several arguments
in an undocumented hash just to reduce the number of given arguments.

And once we upgraded to Ruby 3.1, we can clean the method calls up as
well. `call(user: user)` becomes `call(user:)` without repetition.
This commit is contained in:
Maikel Linke
2023-05-24 04:54:13 +01:00
parent e56c06571c
commit 828b2f6f44
4 changed files with 23 additions and 8 deletions

View File

@@ -44,6 +44,9 @@ Metrics/BlockLength:
"xdescribe",
]
Metrics/ParameterLists:
CountKeywordArgs: false
Rails/ApplicationRecord:
Exclude:
# Migrations should not contain application code:

View File

@@ -65,7 +65,8 @@ module Admin
blob = ReportBlob.create_for_upload_later!(report_filename)
ReportJob.perform_later(
report_class, spree_current_user, params, format, blob, ScopedChannel.for_id(params[:uuid])
report_class: report_class, user: spree_current_user, params: params,
format: format, blob: blob, channel: ScopedChannel.for_id(params[:uuid]),
)
render cable_ready: cable_car.

View File

@@ -9,7 +9,7 @@ class ReportJob < ApplicationJob
NOTIFICATION_TIME = 5.seconds
def perform(report_class, user, params, format, blob, channel = nil)
def perform(report_class:, user:, params:, format:, blob:, channel: nil)
start_time = Time.zone.now
report = report_class.new(user, params, render: true)

View File

@@ -3,7 +3,10 @@
require 'spec_helper'
describe ReportJob do
let(:report_args) { [report_class, user, params, format, blob] }
let(:report_args) {
{ report_class: report_class, user: user, params: params, format: format,
blob: blob }
}
let(:report_class) { Reporting::Reports::UsersAndEnterprises::Base }
let(:user) { enterprise.owner }
let(:enterprise) { create(:enterprise) }
@@ -13,13 +16,13 @@ describe ReportJob do
it "generates a report" do
job = perform_enqueued_jobs(only: ReportJob) do
ReportJob.perform_later(*report_args)
ReportJob.perform_later(**report_args)
end
expect_csv_report
end
it "enqueues a job for async processing" do
job = ReportJob.perform_later(*report_args)
job = ReportJob.perform_later(**report_args)
expect(blob.content_stored?).to eq false
perform_enqueued_jobs(only: ReportJob)
@@ -29,12 +32,16 @@ describe ReportJob do
end
it "triggers an email when the report is done" do
# Setup test data which also triggers emails:
report_args
# Send emails for quick jobs as well:
stub_const("ReportJob::NOTIFICATION_TIME", 0)
ReportJob.perform_later(*report_args)
expect {
# We need to create this job within the block because of a bug in
# rspec-rails: https://github.com/rspec/rspec-rails/issues/2668
ReportJob.perform_later(**report_args)
perform_enqueued_jobs(only: ReportJob)
}.to enqueue_mail(ReportMailer, :report_ready).with(
params: {
@@ -46,9 +53,13 @@ describe ReportJob do
end
it "triggers no email when the report is done quickly" do
ReportJob.perform_later(*report_args)
# Setup test data which also triggers emails:
report_args
expect {
# We need to create this job within the block because of a bug in
# rspec-rails: https://github.com/rspec/rspec-rails/issues/2668
ReportJob.perform_later(**report_args)
perform_enqueued_jobs(only: ReportJob)
}.to_not enqueue_mail
end