mirror of
https://github.com/discourse/discourse.git
synced 2024-11-25 09:42:07 +08:00
DEV: Disallow default params in service steps
With the current implementation, a service step can be written as: ```ruby def my_step(a_default_value: 2) … end ``` That’s a pattern we want to avoid as default values (if needed) should be probably defined in a contract. This patch makes a service raise an exception if a default value is encountered.
This commit is contained in:
parent
fe1098ebac
commit
afdb1ac0a0
|
@ -112,6 +112,9 @@ module Service
|
|||
def call(instance, context)
|
||||
object = class_name&.new(context)
|
||||
method = object&.method(:call) || instance.method(method_name)
|
||||
if method.parameters.any? { _1[0] != :keyreq }
|
||||
raise "In #{type} '#{name}': default values in step implementations are not allowed. Maybe they could be defined in a contract?"
|
||||
end
|
||||
args = context.to_h.slice(*method.parameters.select { _1[0] == :keyreq }.map(&:last))
|
||||
context[result_key] = Context.build(object: object)
|
||||
instance.instance_exec(**args, &method)
|
||||
|
|
|
@ -62,7 +62,7 @@ module Chat
|
|||
search_category_channels(context, guardian)
|
||||
end
|
||||
|
||||
def fetch_direct_message_channels(guardian:, contract:, users:, **args)
|
||||
def fetch_direct_message_channels(guardian:, contract:, users:)
|
||||
return unless contract.include_direct_message_channels
|
||||
return unless guardian.can_create_direct_message?
|
||||
search_direct_message_channels(context, guardian, contract, users)
|
||||
|
|
61
spec/lib/service_spec.rb
Normal file
61
spec/lib/service_spec.rb
Normal file
|
@ -0,0 +1,61 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
RSpec.describe Service do
|
||||
let(:service_class) { Class.new { include Service::Base } }
|
||||
|
||||
describe "Steps" do
|
||||
describe "Model step" do
|
||||
context "when providing default values to step implementation" do
|
||||
before do
|
||||
service_class.class_eval do
|
||||
model :my_model
|
||||
|
||||
def fetch_my_model(default_arg: 2)
|
||||
true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "raises an error" do
|
||||
expect { service_class.call }.to raise_error(/In model 'my_model': default values/)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "Policy step" do
|
||||
context "when providing default values to step implementation" do
|
||||
before do
|
||||
service_class.class_eval do
|
||||
policy :my_policy
|
||||
|
||||
def my_policy(default_arg: 2)
|
||||
true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "raises an error" do
|
||||
expect { service_class.call }.to raise_error(/In policy 'my_policy': default values/)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "Generic step" do
|
||||
context "when providing default values to step implementation" do
|
||||
before do
|
||||
service_class.class_eval do
|
||||
step :generic_step
|
||||
|
||||
def generic_step(default_arg: 2)
|
||||
true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "raises an error" do
|
||||
expect { service_class.call }.to raise_error(/In step 'generic_step': default values/)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue
Block a user