mirror of
https://github.com/discourse/discourse.git
synced 2025-05-23 14:11:16 +08:00
DEV: Stop injecting a service result object in the caller object
Currently, when calling a service with its block form, a `#result` method is automatically created on the caller object. Even if it never clashed so far, this could happen. This patch removes that method, and instead use a more classical way of doing things: the result object is now provided as an argument to the main block. This means if we need to access the result object in an outcome block, it will be done like this from now on: ```ruby MyService.call(params) do |result| on_success do # do something with the result object do_something(result) end end ``` In the same vein, this patch introduces the ability to match keys from the result object in the outcome blocks, like we already do with step definitions in a service. For example: ```ruby on_success do |model:, contract:| do_something(model, contract) end ``` Instead of ```ruby on_success do do_something(result.model, result.contract) end ```
This commit is contained in:

committed by
Loïc Guitaut

parent
07ff21d045
commit
f79dd5c8b5
@ -150,10 +150,13 @@ RSpec.describe Service::Runner do
|
||||
describe ".call" do
|
||||
subject(:runner) { described_class.call(service, &actions_block) }
|
||||
|
||||
let(:result) { object.result }
|
||||
let(:actions_block) { object.instance_eval(actions) }
|
||||
let(:service) { SuccessService }
|
||||
let(:actions) { "proc {}" }
|
||||
let(:service) { SuccessWithModelService }
|
||||
let(:actions) { <<-BLOCK }
|
||||
proc do |result|
|
||||
on_success { |fake_model:| [result, fake_model] }
|
||||
end
|
||||
BLOCK
|
||||
let(:object) do
|
||||
Class
|
||||
.new(ApplicationController) do
|
||||
@ -171,10 +174,12 @@ RSpec.describe Service::Runner do
|
||||
.new
|
||||
end
|
||||
|
||||
it "runs the provided service in the context of a controller" do
|
||||
runner
|
||||
expect(result).to be_a Service::Base::Context
|
||||
expect(result).to be_a_success
|
||||
it "allows access to the result object" do
|
||||
expect(runner.first).to be_a Service::Base::Context
|
||||
end
|
||||
|
||||
it "allows using keyword args in blocks" do
|
||||
expect(runner.last).to eq :model_found
|
||||
end
|
||||
|
||||
context "when using the on_success action" do
|
||||
@ -241,7 +246,7 @@ RSpec.describe Service::Runner do
|
||||
|
||||
context "when using the block argument" do
|
||||
let(:actions) { <<-BLOCK }
|
||||
proc do
|
||||
proc do |result|
|
||||
on_failed_policy(:test) { |policy| policy == result["result.policy.test"] }
|
||||
end
|
||||
BLOCK
|
||||
@ -279,7 +284,7 @@ RSpec.describe Service::Runner do
|
||||
|
||||
context "when using the block argument" do
|
||||
let(:actions) { <<-BLOCK }
|
||||
proc do
|
||||
proc do |result|
|
||||
on_failed_contract { |contract| contract == result["result.contract.default"] }
|
||||
end
|
||||
BLOCK
|
||||
@ -301,8 +306,9 @@ RSpec.describe Service::Runner do
|
||||
|
||||
context "when using the on_model_not_found action" do
|
||||
let(:actions) { <<-BLOCK }
|
||||
proc do
|
||||
on_model_not_found(:fake_model) { :no_model }
|
||||
proc do |result|
|
||||
on_success { [result] }
|
||||
on_model_not_found(:fake_model) { [:no_model, result] }
|
||||
end
|
||||
BLOCK
|
||||
|
||||
@ -311,7 +317,7 @@ RSpec.describe Service::Runner do
|
||||
let(:service) { FailureWithOptionalModelService }
|
||||
|
||||
it "does not run the provided block" do
|
||||
expect(runner).not_to eq :no_model
|
||||
expect(runner).not_to include :no_model
|
||||
end
|
||||
end
|
||||
|
||||
@ -320,13 +326,13 @@ RSpec.describe Service::Runner do
|
||||
|
||||
context "when not using the block argument" do
|
||||
it "runs the provided block" do
|
||||
expect(runner).to eq :no_model
|
||||
expect(runner).to include :no_model
|
||||
end
|
||||
end
|
||||
|
||||
context "when using the block argument" do
|
||||
let(:actions) { <<-BLOCK }
|
||||
proc do
|
||||
proc do |result|
|
||||
on_model_not_found(:fake_model) { |model| model == result["result.model.fake_model"] }
|
||||
end
|
||||
BLOCK
|
||||
@ -341,7 +347,7 @@ RSpec.describe Service::Runner do
|
||||
let(:service) { SuccessWithModelService }
|
||||
|
||||
it "does not run the provided block" do
|
||||
expect(runner).not_to eq :no_model
|
||||
expect(runner).not_to include :no_model
|
||||
end
|
||||
end
|
||||
end
|
||||
@ -351,7 +357,7 @@ RSpec.describe Service::Runner do
|
||||
let(:service) { FailureWithCollectionModelService }
|
||||
|
||||
it "runs the provided block" do
|
||||
expect(runner).to eq :no_model
|
||||
expect(runner).to include :no_model
|
||||
end
|
||||
end
|
||||
|
||||
@ -359,7 +365,7 @@ RSpec.describe Service::Runner do
|
||||
let(:service) { SuccessWithCollectionModelService }
|
||||
|
||||
it "does not run the provided block" do
|
||||
expect(runner).not_to eq :no_model
|
||||
expect(runner).not_to include :no_model
|
||||
end
|
||||
end
|
||||
end
|
||||
@ -371,23 +377,21 @@ RSpec.describe Service::Runner do
|
||||
before { Fabricate(:user) }
|
||||
|
||||
it "does not run the provided block" do
|
||||
expect(runner).not_to eq :no_model
|
||||
expect(runner).not_to include :no_model
|
||||
end
|
||||
|
||||
it "does not fetch records from the relation" do
|
||||
runner
|
||||
expect(result[:fake_model]).not_to be_loaded
|
||||
expect(runner.last[:fake_model]).not_to be_loaded
|
||||
end
|
||||
end
|
||||
|
||||
context "when the service fails" do
|
||||
it "runs the provided block" do
|
||||
expect(runner).to eq :no_model
|
||||
expect(runner).to include :no_model
|
||||
end
|
||||
|
||||
it "does not fetch records from the relation" do
|
||||
runner
|
||||
expect(result[:fake_model]).not_to be_loaded
|
||||
expect(runner.last[:fake_model]).not_to be_loaded
|
||||
end
|
||||
end
|
||||
end
|
||||
|
Reference in New Issue
Block a user