-
Notifications
You must be signed in to change notification settings - Fork 3
Add if/unless support #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| PATH | ||
| remote: . | ||
| specs: | ||
| opera (0.6.0) | ||
| opera (0.7.0) | ||
|
|
||
| GEM | ||
| remote: https://rubygems.org/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,6 +124,40 @@ end | |
| | `within :method do ... end` | Wraps nested steps with a custom method that must `yield`. If it doesn't yield, nested steps are skipped. | | ||
| | `always :method` | Executes a step unconditionally after all regular steps, even after a failure or an early finish. Must appear at the end of the operation — only other `always` steps may follow. Cannot be used inside blocks. Use `result.success?` / `result.failure?` inside the method to branch on outcome. | | ||
|
|
||
| ### Conditional execution (`:if` / `:unless`) | ||
|
|
||
| `step`, `operation`, and `operations` accept `:if` and `:unless` keyword | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to update |
||
| arguments for declarative conditional execution. The condition is evaluated | ||
| **before** the step's method is called -- if the condition is not met the | ||
| step is skipped entirely (no method invocation, no side effects, not recorded | ||
| in `result.executions`). | ||
|
|
||
| The condition value can be a **Symbol** (method name on the operation) or a | ||
| **Proc/Lambda** (evaluated via `instance_exec` in the operation instance | ||
| scope). | ||
|
|
||
| ```ruby | ||
| # Symbol form | ||
| step :notify_user, if: :notifications_enabled? | ||
| operation :create_internal_experience, if: :internal_experience_authorized? | ||
|
|
||
| # Lambda form | ||
| step :recalculate, unless: -> { params[:skip_recalculation] } | ||
| operation :reopen_and_reset, if: -> { profile_ids.present? } | ||
| ``` | ||
|
|
||
| When an `operation` or `operations` step is skipped, its | ||
| `context[:<method>_output]` slot is set to `nil` (matching the historical | ||
| `return Opera::Operation::Result.new` early-exit behavior). When a plain | ||
| `step` is skipped, no context output is set. | ||
|
|
||
| Passing both `:if` and `:unless` on the same step raises `ArgumentError` at | ||
| class load time. | ||
|
|
||
| `:if` / `:unless` are not supported on `validate`, `success`, `finish_if`, | ||
| `transaction`, `within`, or `always` -- conditional containers and validation | ||
| have ambiguous semantics. | ||
|
|
||
| ### Combining instructions | ||
|
|
||
| ```ruby | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Opera | ||
| module Operation | ||
| module Builder | ||
| # Parses keyword options passed to a Builder instruction (`step`, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. imo those comments are not needed. The code is clear |
||
| # `operation`, `transaction`, etc.) into a normalized hash that is merged | ||
| # into the instruction entry. | ||
| # | ||
| # Currently understands `:if` and `:unless`. New options can be added by | ||
| # extending ALLOWED_OPTIONS and the build logic. | ||
| class OptionsBuilder | ||
| ALLOWED_OPTIONS = %i[if unless].freeze | ||
|
|
||
| def self.build(opts) | ||
| return {} if opts.empty? | ||
|
|
||
| unknown = opts.keys - ALLOWED_OPTIONS | ||
| raise ArgumentError, "Unknown option(s): #{unknown.inspect}. Allowed: #{ALLOWED_OPTIONS}" if unknown.any? | ||
|
|
||
| { predicate: build_predicate(opts) }.compact | ||
| end | ||
|
|
||
| # Translates `:if` / `:unless` (Symbol or Proc) into a single Proc that | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the same here |
||
| # returns true when the step should run. Returns nil when neither is | ||
| # given. Raises if both are given. | ||
| def self.build_predicate(opts) | ||
| return nil unless opts[:if] || opts[:unless] | ||
| raise ArgumentError, 'Cannot use both :if and :unless on the same step' if opts[:if] && opts[:unless] | ||
|
|
||
| cond = opts[:if] || opts[:unless] | ||
| body = cond.is_a?(Symbol) ? proc { send(cond) } : cond | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rename
|
||
| opts.key?(:if) ? body : proc { !instance_exec(&body) } | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,11 @@ def execute_step(instruction) | |
|
|
||
| # rubocop:disable Metrics/MethodLength, Metrics/AbcSize, Metrics/CyclomaticComplexity | ||
| def evaluate_instruction(instruction) | ||
| if instruction[:predicate] && !condition_met?(instruction) | ||
| add_instruction_output(instruction, nil) if %i[operation operations].include?(instruction[:kind]) | ||
| return | ||
| end | ||
|
|
||
| case instruction[:kind] | ||
| when :step | ||
| Instructions::Executors::Step.new(operation).call(instruction) | ||
|
|
@@ -66,6 +71,14 @@ def evaluate_instruction(instruction) | |
| end | ||
| # rubocop:enable Metrics/MethodLength, Metrics/AbcSize, Metrics/CyclomaticComplexity | ||
|
|
||
| # Evaluates the `:predicate` Proc stored on a conditionable instruction. | ||
| # The predicate is built at class-load time from `:if` / `:unless` and | ||
| # already encodes the negation for `:unless`, so the executor only needs | ||
| # to call it in the operation instance scope. | ||
| def condition_met?(instruction) | ||
| operation.instance_exec(&instruction[:predicate]) | ||
| end | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also mentioned that on slack. Imo you could inline that above. You don't need 7 lines explaning what is going on here. Devs would know. |
||
|
|
||
| def result | ||
| operation.result | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Opera | ||
| VERSION = '0.6.0' | ||
| VERSION = '0.7.0' | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to update that too. It is for all instructions, not only
step/operation