Skip to content

Commit

Permalink
DEV: Unify params access in services
Browse files Browse the repository at this point in the history
Currently, there are two ways (kind of) for accessing `params` inside a
service:
- when there is no contract or it hasn’t been reached yet, `params` is
  just the hash that was provided to the service. To access a key, you
  have to use the bracket notation `params[:my_key]`.
- when there is a contract and it has been executed successfully,
  `params` now references the contract and the attributes are accessible
  using methods (`params.my_key`).

This patch unifies how `params` exposes its attributes. Now, even if
there is no contract at all in a service, `params` will expose its
attributes through methods, that way things are more consistent.

This patch also makes sure there is always a `params` object available
even when no `params` key is provided to the service (this allows a
contract to fail because its attributes are blank instead of having the
service raising an error because it doesn’t find `params` in its context).
  • Loading branch information
Flink committed Dec 13, 2024
1 parent cbc0ece commit 9e9abe0
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 3 deletions.
22 changes: 21 additions & 1 deletion lib/service/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,12 @@ def steps
def initialize(initial_context = {})
@context =
Context.build(
initial_context.merge(__steps__: self.class.steps, __service_class__: self.class),
initial_context
.compact
.reverse_merge(params: {})
.merge(__steps__: self.class.steps, __service_class__: self.class),
)
initialize_params
end

# @!visibility private
Expand All @@ -463,5 +467,21 @@ def fail!(message)
context["result.step.#{step_name}"].fail(error: message)
context.fail!
end

private

def initialize_params
klass =
Data.define(*context[:params].keys) do
alias to_hash to_h

delegate :slice, :merge, to: :to_h

def method_missing(*)
nil
end
end
context[:params] = klass.new(*context[:params].values)
end
end
end
2 changes: 1 addition & 1 deletion plugins/chat/app/services/chat/create_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class CreateMessage
private

def accept_blocks(guardian:, params:)
params[:blocks] ? guardian.user.bot? : true
params.blocks ? guardian.user.bot? : true
end

def no_silenced_user(guardian:)
Expand Down
2 changes: 1 addition & 1 deletion plugins/chat/app/services/chat/update_channel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class UpdateChannel
private

def fetch_channel(params:)
Chat::Channel.find_by(id: params[:channel_id])
Chat::Channel.find_by(id: params.channel_id)
end

def check_channel_permission(guardian:, channel:)
Expand Down
34 changes: 34 additions & 0 deletions spec/lib/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,38 @@ def generic_step(default_arg: 2)
end
end
end

describe "Parameters handling" do
subject(:result) { service_class.call(**args) }

context "when calling the service without any params" do
let(:args) { {} }

it "instantiate a default params object" do
expect(result[:params]).not_to be_nil
end
end

context "when calling the service with params" do
let(:args) { { params: { param1: "one" } } }

context "when there is no `params` step defined" do
it "allows accessing `params` through methods" do
expect(result[:params].param1).to eq("one")
end

it "returns nothing for a non-existent key" do
expect(result[:params].non_existent_key).to be_nil
end
end

context "when there is a `params` step defined" do
before { service_class.class_eval { params { attribute :param1 } } }

it "returns the contract as the params object" do
expect(result[:params]).to be_a(Service::ContractBase)
end
end
end
end
end

0 comments on commit 9e9abe0

Please sign in to comment.