From b8ff4391253b4f1cf980298572b5215475ce603e Mon Sep 17 00:00:00 2001 From: Arkadiy Zabazhanov Date: Sat, 24 Feb 2024 21:16:59 +0700 Subject: [PATCH 1/7] Introduce new form object --- .rubocop_todo.yml | 16 +- README.md | 121 ++++++--- lib/operations.rb | 1 + lib/operations/command.rb | 16 +- lib/operations/form.rb | 56 ++++- lib/operations/form/attribute.rb | 11 +- lib/operations/form/base.rb | 63 ++++- lib/operations/form/builder.rb | 65 +++-- ...recated_legacy_model_map_implementation.rb | 26 ++ lib/operations/inspect.rb | 12 +- operations.gemspec | 2 +- spec/operations/command_spec.rb | 25 +- spec/operations/form/base_spec.rb | 167 ++++++++++-- spec/operations/form/builder_spec.rb | 238 +++++++++++++----- ...ed_legacy_model_map_implementation_spec.rb | 35 +++ spec/operations/form_spec.rb | 165 ++++++++++++ 16 files changed, 838 insertions(+), 181 deletions(-) create mode 100644 lib/operations/form/deprecated_legacy_model_map_implementation.rb create mode 100644 spec/operations/form/deprecated_legacy_model_map_implementation_spec.rb create mode 100644 spec/operations/form_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index be08558..4dcb017 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2024-01-01 09:54:28 UTC using RuboCop version 1.59.0. +# on 2024-05-05 08:51:48 UTC using RuboCop version 1.59.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -14,7 +14,7 @@ Gemspec/DevelopmentDependencies: Exclude: - 'operations.gemspec' -# Offense count: 4 +# Offense count: 5 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: Max: 28 @@ -22,14 +22,20 @@ Metrics/AbcSize: # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 163 + Max: 145 # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ModuleLength: - Max: 111 + Max: 142 -# Offense count: 1 +# Offense count: 2 # Configuration parameters: CountKeywordArgs, MaxOptionalParameters. Metrics/ParameterLists: Max: 7 + +# Offense count: 1 +# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. +RSpec/VerifiedDoubles: + Exclude: + - 'spec/operations/form_spec.rb' diff --git a/README.md b/README.md index 9c42501..b434ef9 100644 --- a/README.md +++ b/README.md @@ -262,7 +262,7 @@ end The operation body can be any callable object (respond to the `call` method), even a lambda. But it is always better to define it as a class since there might be additional instance methods and [dependency injections](#dependency-injection). -In any event, the operation body should return a Dry::Monad::Result instance. In case of a Failure, it will be converted into an `Operation::Result#error` and in case of Success(), its content will be merged into the operation context. +In any event, the operation body should return a Dry::Monads::Result instance. In case of a Failure, it will be converted into an `Operation::Result#error` and in case of Success(), its content will be merged into the operation context. **Important:** since the Success result payload is merged inside of a context, it is supposed to be a hash. @@ -305,7 +305,7 @@ end Dependency injection can be used to provide IO clients with the operation. It could be DB repositories or API clients. The best way is to use Dry::Initializer for it since it provides the ability to define acceptable types. -If you still prefer to use ActiveRecord, it is worth creating a wrapper around it providing Dry::Monad-compatible interfaces. +If you still prefer to use ActiveRecord, it is worth creating a wrapper around it providing Dry::Monads-compatible interfaces. ```ruby class ActiveRecordRepository @@ -858,31 +858,38 @@ While we normally recommend using frontend-backend separation, it is still possi ```ruby class PostsController < ApplicationController def edit - @post_update = Post::Update.default.callable( - { post_id: params[:id] }, - current_user: current_user - ) + @post_update_form = Post::Update.default_form.build(params, current_user: current_user) - respond_with @post_update + respond_with @post_update_form end def update - # With operations we don't need strong parameters as the operation contract takes care of this. - @post_update = Post::Update.default.call( - { **params[:post_update_default_form], post_id: params[:id] }, - current_user: current_user - ) + @post_update_form = Post::Update.default_form.persist(params, current_user: current_user) + + respond_with @post_update_form, location: edit_post_url(@post_update_form.operation_result.context[:post]) + end +end +``` + +Where the form class is defined this way: + +```ruby +class Post::Update + def self.default + @default ||= Operations::Command.new(...) + end - respond_with @post_update, location: edit_post_url(@post_update.context[:post]) + def self.default_form + @default_form ||= Operations::Form.new(default) end end ``` -The key here is to use `Operations::Result#form` method for the form builder. +Then, the form can be used as any other form object: ```erb # views/posts/edit.html.erb -<%= form_for @post_update.form, url: post_url(@post_update.context[:post]), method: :patch do |f| %> +<%= form_for @post_update_form, url: post_url(@post_update_form.operation_result.context[:post]) do |f| %> <%= f.input :title %> <%= f.text_area :body %> <% end %> @@ -892,16 +899,16 @@ In cases when we need to populate the form data, it is possible to pass `form_hy ```ruby class Post::Update - def self.default - @default ||= Operations::Command.new( - ..., - form_hydrator: Post::Update::Hydrator.new + def self.default_form + @default_form ||= Operations::Form.new( + default, + hydrator: Post::Update::Hydrator.new ) end end class Post::Update::Hydrator - def call(form_class, params, post:, **) + def call(form_class, params, post:, **_context) value_attributes = form_class.attributes.keys - %i[post_id] data = value_attributes.index_with { |name| post.public_send(name) } @@ -912,23 +919,79 @@ end The general idea here is to figure out attributes we have in the contract (those attributes are also defined automatically in a generated form class) and then fetch those attributes from the model and merge them with the params provided within the request. -Also, in the case of, say, [simple_form](https://github.com/heartcombo/simple_form), we need to provide additional attributes information, like data type. It is possible to do this with an optional `form_model_map:` operation option: +Also, in the case of, say, [simple_form](https://github.com/heartcombo/simple_form), we need to provide additional attributes information, like data type. It is possible to do this with `model_map:` option: ```ruby class Post::Update - def self.default - @default ||= Operations::Command.new( - ..., - form_hydrator: Post::Update::Hydrator.new, - form_model_map: { - [%r{.+}] => "Post" - } + def self.default_form + @default_form ||= Operations::Form.new( + default, + hydrator: Post::Update::Hydrator.new, + model_map: Post::Update::ModelMap.new ) end end + +class Post::Update::ModelMap + def call(_path) + Post + end +end ``` -Here we define all the fields mapping to a model class with a regexp or just string values. +In forms, params imput is already transformed to extract the nested data with the form name. `form_for @post_update_form` will generate the form that send params nested under the `params[:post_update_form]` key. By default operation forms extract this form data and send it to the operation at the top level, so `{ id: 42, post_update_form: { title: "Post Title" } }` params will be sent to the operation as `{ id: 42, title: "Post Title" }`. Strong params are also accepted by the form, though they are being converted with `to_unsafe_hash`. + +It is possible to add more params transfomations to the form in cases when operation contract is different from the params structure: + +```ruby +class Post::Update + def self.default_form + @default_form ||= Operations::Form.new( + default, + params_transformations: [ + ParamsMap.new(id: :post_id), + NestedAttributes.new(:sections) + ] + ) + end + + contract do + required(:post_id).filled(:integer) + optional(:title).filled(:string) + optional(:sections).array(:hash) do + optional(:id).filled(:integer) + optional(:content).filled(:string) + optional(:_destroy).filled(:bool) + end + end +end + +# This will transform `{ id: 42, title: "Post Title" }` params to `{ post_id: 42, title: "Post Title" }` +class ParamsMap + extend Dry::Initializer + + param :params_map + + def call(_form_class, params, **_context) + params.transform_keys { |key| params_map[key] || key } + end +end + +# And this will transform nested attributes hash from the form to an array acceptable by the operation: +# from +# `{ id: 42, sections: { '0' => { id: 1, content: "First paragraph" }, 'new' => { content: 'New Paragraph' } } }` +# into +# `{ id: 42, sections: [{ id: 1, content: "First paragraph" }, { content: 'New Paragraph' }] }` +class NestedAttributes + extend Dry::Initializer + + param :name, Types::Coercible::Symbol + + def call(_form_class, params, **_context) + params[name] = params[:"#{name}_attrbutes"].values + end +end +``` ## Development diff --git a/lib/operations.rb b/lib/operations.rb index 32339e5..4253c60 100644 --- a/lib/operations.rb +++ b/lib/operations.rb @@ -20,6 +20,7 @@ require "operations/form/base" require "operations/form/attribute" require "operations/form/builder" +require "operations/form/deprecated_legacy_model_map_implementation" require "operations/command" require "operations/result" diff --git a/lib/operations/command.rb b/lib/operations/command.rb index d7c4c09..8f1c998 100644 --- a/lib/operations/command.rb +++ b/lib/operations/command.rb @@ -125,12 +125,11 @@ # which contains all the artifacts and the information about the errors # should they ever happen. class Operations::Command - UNDEFINED = Object.new.freeze - EMPTY_HASH = {}.freeze COMPONENTS = %i[contract policies idempotency preconditions operation on_success on_failure].freeze FORM_HYDRATOR = ->(_form_class, params, **_context) { params } extend Dry::Initializer + include Dry::Core::Constants include Dry::Monads[:result] include Dry::Monads::Do.for(:call_monad, :callable_monad, :validate_monad, :execute_operation) include Dry::Equalizer(*COMPONENTS) @@ -158,12 +157,7 @@ def sentry_context option :preconditions, Operations::Types::Array.of(Operations::Types.Interface(:call)), default: -> { [] } option :on_success, Operations::Types::Array.of(Operations::Types.Interface(:call)), default: -> { [] } option :on_failure, Operations::Types::Array.of(Operations::Types.Interface(:call)), default: -> { [] } - option :form_model_map, Operations::Types::Hash.map( - Operations::Types::Coercible::Array.of( - Operations::Types::String | Operations::Types::Symbol | Operations::Types.Instance(Regexp) - ), - Operations::Types::String - ), default: proc { {} } + option :form_model_map, Operations::Form::DeprecatedLegacyModelMapImplementation::TYPE, default: proc { {} } option :form_base, Operations::Types::Class, default: proc { ::Operations::Form::Base } option :form_class, Operations::Types::Class.optional, default: proc {}, reader: false option :form_hydrator, Operations::Types.Interface(:call), default: proc { FORM_HYDRATOR } @@ -196,11 +190,11 @@ def self.build(operation, contract = nil, **deps) end def initialize( - operation, policy: UNDEFINED, policies: [UNDEFINED], + operation, policy: Undefined, policies: [Undefined], precondition: nil, preconditions: [], after: [], **options ) policies_sum = Array.wrap(policy) + policies - result_policies = policies_sum - [UNDEFINED] unless policies_sum == [UNDEFINED, UNDEFINED] + result_policies = policies_sum - [Undefined] unless policies_sum == [Undefined, Undefined] options[:policies] = result_policies if result_policies preconditions.push(precondition) if precondition.present? @@ -391,7 +385,7 @@ def build_form_class .new(base_class: form_base) .build( key_map: contract.class.schema.key_map, - model_map: form_model_map, + model_map: Operations::Form::DeprecatedLegacyModelMapImplementation.new(form_model_map), namespace: operation.class, class_name: form_class_name ) diff --git a/lib/operations/form.rb b/lib/operations/form.rb index 20cb567..61fde88 100644 --- a/lib/operations/form.rb +++ b/lib/operations/form.rb @@ -2,6 +2,10 @@ # Configures and defines a form object factory. class Operations::Form + include Dry::Core::Constants + include Dry::Equalizer(:command, :form_class, :model_map, :params_transformations, :hydrator) + include Operations::Inspect.new(:form_class, :model_map, :params_transformations, :hydrator) + # We need to make deprecated inheritance from Operations::Form act exactly the # same way as from Operations::Form::Base. In order to do this, we are encapsulating all the # inheritable functionality in 2 modules and removing methods defined in Operations::Form @@ -19,6 +23,56 @@ def self.inherited(subclass) end subclass.extend Operations::Form::Base::ClassMethods - subclass.include Operations::Form::Base::InstanceMethods + subclass.prepend Operations::Form::Base::InstanceMethods + end + + include Dry::Initializer.define(lambda do + param :command, type: Operations::Types.Interface(:operation, :contract, :call) + option :base_class, type: Operations::Types::Class, default: proc { ::Operations::Form::Base } + option :model_map, type: Operations::Types.Interface(:call).optional, default: proc {} + option :model_name, type: Operations::Types::String.optional, default: proc {}, reader: false + option :params_transformations, type: Operations::Types::Coercible::Array.of(Operations::Types.Interface(:call)), + default: proc { [] } + option :hydrator, type: Operations::Types.Interface(:call).optional, default: proc {} + end) + + def build(params = EMPTY_HASH, **context) + instantiate_form(command.callable(transform_params(params, **context), **context)) + end + + def persist(params = EMPTY_HASH, **context) + instantiate_form(command.call(transform_params(params, **context), **context)) + end + + def form_class + @form_class ||= Operations::Form::Builder.new(base_class: base_class) + .build(key_map: key_map, model_map: model_map, model_name: model_name) + end + + private + + def transform_params(params, **context) + params = params.to_unsafe_hash if params.respond_to?(:to_unsafe_hash) + params = params.deep_symbolize_keys + params = params.merge(params[form_class.model_name.param_key.to_sym] || {}) + params_transformations.inject(params) do |value, transformation| + transformation.call(form_class, value, **context) + end + end + + def instantiate_form(operation_result) + form_class.new( + hydrator.call(form_class, operation_result.params, **operation_result.context), + messages: operation_result.errors.to_h, + operation_result: operation_result + ) + end + + def key_map + @key_map ||= command.contract.schema.key_map + end + + def model_name + @model_name ||= ("#{command.operation.class.name.underscore}_form" if command.operation.class.name) end end diff --git a/lib/operations/form/attribute.rb b/lib/operations/form/attribute.rb index 3afdda4..4fb05d8 100644 --- a/lib/operations/form/attribute.rb +++ b/lib/operations/form/attribute.rb @@ -5,14 +5,15 @@ # legacy UI. class Operations::Form::Attribute extend Dry::Initializer - include Dry::Equalizer(:name, :collection, :form, :model_name) + include Dry::Equalizer(:name, :collection, :model_name, :form) + include Operations::Inspect.new(:name, :collection, :model_name, :form) param :name, type: Operations::Types::Coercible::Symbol - option :collection, type: Operations::Types::Bool, optional: true, default: proc { false } - option :form, type: Operations::Types::Class, optional: true + option :collection, type: Operations::Types::Bool, default: proc { false } option :model_name, - type: Operations::Types::String | Operations::Types.Instance(Class).constrained(lt: ActiveRecord::Base), - optional: true + type: (Operations::Types::String | Operations::Types.Instance(Class).constrained(lt: ActiveRecord::Base)).optional, + default: proc {} + option :form, type: Operations::Types::Class.optional, default: proc {} def model_type @model_type ||= owning_model.type_for_attribute(string_name) if model_name diff --git a/lib/operations/form/base.rb b/lib/operations/form/base.rb index 3768630..5e2df5c 100644 --- a/lib/operations/form/base.rb +++ b/lib/operations/form/base.rb @@ -20,6 +20,9 @@ # # @see Operations::Form::Builder class Operations::Form::Base + BUILD_ASSOCIATION_PREFIX = %r{\Abuild_}.freeze + NESTED_ATTRIBUTES_WRITER_SUFFIX = %r{_attributes=\z}.freeze + # :nodoc: module ClassMethods def self.extended(base) @@ -40,8 +43,14 @@ def self.extended(base) ), default: proc { {} }, reader: :private + base.option :operation_result, default: proc {} base.class_attribute :attributes, instance_accessor: false, default: {} + base.class_attribute :primary_key, instance_accessor: false, default: :id + + base.define_method :initialize do |*args, **kwargs| + args.empty? && kwargs.present? ? super(kwargs, **{}) : super(*args, **kwargs) + end end def attribute(name, **options) @@ -63,6 +72,12 @@ def human_attribute_name(name, options = {}) def validators_on(name) attributes[name.to_sym]&.model_validators || [] end + + def model_name + @model_name ||= ActiveModel::Name.new(self) + end + + def reflect_on_association(...); end end # :nodoc: @@ -91,23 +106,43 @@ def assigned_attributes end end - def method_missing(name, *) - read_attribute(name) + # For now we gracefully return nil for unknown methods + def method_missing(name, *args, **kwargs) + build_attribute_name = build_attribute_name(name) + build_attribute = self.class.attributes[build_attribute_name] + plural_build_attribute = self.class.attributes[build_attribute_name.to_s.pluralize.to_sym] + + if has_attribute?(name) + read_attribute(name) + elsif build_attribute&.form + build_attribute.form.new(*args, **kwargs) + elsif plural_build_attribute&.form + plural_build_attribute.form.new(*args, **kwargs) + end end def respond_to_missing?(name, *) - self.class.attributes.key?(name) + has_attribute?(name) || + build_nested_form?(build_attribute_name(name)) || + self.class.attributes[nested_attribute_name(name)]&.form end def model_name - ActiveModel::Name.new(self.class) + self.class.model_name end - # This should return false if we want to use POST. - # Now it is going to generate PATCH form. def persisted? - true + !has_attribute?(self.class.primary_key) || read_attribute(self.class.primary_key).present? + end + + def new_record? + !persisted? + end + + def _destroy + Operations::Types::Params::Bool.call(read_attribute(:_destroy)) { false } end + alias_method :marked_for_destruction?, :_destroy # Probably can be always nil, it is used in automated URL derival. # We can make it work later but it will require additional concepts. @@ -143,6 +178,7 @@ def read_attribute(name) end end end + alias_method :read_attribute_for_validation, :read_attribute def to_hash { @@ -208,6 +244,19 @@ def wrap_object(path, data, form) data end end + + def build_attribute_name(name) + name.to_s.sub(BUILD_ASSOCIATION_PREFIX, "").to_sym if name.to_s.match?(BUILD_ASSOCIATION_PREFIX) + end + + def nested_attribute_name(name) + name.to_s.sub(NESTED_ATTRIBUTES_WRITER_SUFFIX, "").to_sym if name.to_s.match?(NESTED_ATTRIBUTES_WRITER_SUFFIX) + end + + def build_nested_form?(name) + !!(self.class.attributes[name]&.form || + self.class.attributes[name.to_s.pluralize.to_sym]&.form) + end end extend ClassMethods diff --git a/lib/operations/form/builder.rb b/lib/operations/form/builder.rb index 98ee5b5..a36ac2c 100644 --- a/lib/operations/form/builder.rb +++ b/lib/operations/form/builder.rb @@ -11,35 +11,46 @@ class Operations::Form::Builder option :base_class, Operations::Types::Instance(Class) - def build(key_map:, model_map:, namespace: nil, class_name: nil) + def build(key_map:, model_map:, namespace: nil, class_name: nil, model_name: nil) return namespace.const_get(class_name) if namespace && class_name && namespace.const_defined?(class_name) - traverse(key_map, model_map, namespace, class_name, []) + traverse(key_map, model_map, namespace, class_name, model_name, []) end private - def traverse(key_map, model_map, namespace, class_name, path) + def traverse(key_map, model_map, namespace, class_name, model_name, path) form = Class.new(base_class) - namespace.const_set(class_name, form) if namespace && class_name + namespace.const_set(class_name, form) if namespace&.name && class_name + define_model_name(form, model_name) if model_name && !form.name - key_map.each do |key| - key_path = path + [key.name] + key_map.each { |key| define_attribute(form, model_map, key, path) } + form + end - case key - when Dry::Schema::Key::Array - nested_form = traverse(key.member, model_map, form, key.name.to_s.underscore.classify, key_path) - form.attribute(key.name, form: nested_form, collection: true, **model_name(model_map, key_path)) - when Dry::Schema::Key::Hash - traverse_hash(form, model_map, key, path) - when Dry::Schema::Key - form.attribute(key.name, **model_name(model_map, key_path)) - else - raise "Unknown key_map key: #{key.class}" - end + def define_model_name(form, model_name) + form.define_singleton_method :model_name do + @model_name ||= ActiveModel::Name.new(self, nil, model_name) end + end - form + def define_attribute(form, model_map, key, path) + case key + when Dry::Schema::Key::Array + traverse_array(form, model_map, key, path) + when Dry::Schema::Key::Hash + traverse_hash(form, model_map, key, path) + when Dry::Schema::Key + form.attribute(key.name, model_name: model_map&.call(path + [key.name])) + else + raise "Unknown key_map key: #{key.class}" + end + end + + def traverse_array(form, model_map, key, path) + key_path = path + [key.name] + nested_form = traverse(key.member, model_map, form, key.name.to_s.underscore.classify, key.name.to_s, key_path) + form.attribute(key.name, form: nested_form, collection: true, model_name: model_map&.call(key_path)) end def traverse_hash(form, model_map, hash_key, path) @@ -55,8 +66,8 @@ def traverse_hash(form, model_map, hash_key, path) form.define_method :"#{hash_key.name}=", proc { |attributes| attributes } if nested_attributes_suffix key_path = path + [name] - nested_form = traverse(members, model_map, form, name.underscore.camelize, key_path) - form.attribute(name, form: nested_form, collection: collection, **model_name(model_map, key_path)) + nested_form = traverse(members, model_map, form, name.underscore.camelize, name.to_s.singularize, key_path) + form.attribute(name, form: nested_form, collection: collection, model_name: model_map&.call(key_path)) end def specify_form_attributes(hash_key, nested_attributes_suffix, nested_attributes_collection) @@ -68,18 +79,4 @@ def specify_form_attributes(hash_key, nested_attributes_suffix, nested_attribute [hash_key.name, hash_key.members, false] end end - - def model_name(model_map, path) - _, model_name = model_map.find do |pathspec, _model| - path.size == pathspec.size && path.zip(pathspec).all? do |slug, pattern| - pattern.is_a?(Regexp) ? pattern.match?(slug) : slug == pattern - end - end - - if model_name - { model_name: model_name } - else - {} - end - end end diff --git a/lib/operations/form/deprecated_legacy_model_map_implementation.rb b/lib/operations/form/deprecated_legacy_model_map_implementation.rb new file mode 100644 index 0000000..0c786c7 --- /dev/null +++ b/lib/operations/form/deprecated_legacy_model_map_implementation.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# Traverses the passed {Dry::Schema::KeyMap} and generates +# {Operations::Form::Base} classes on the fly. Handles nested structures. +# +# @see Operations::Form::Base +class Operations::Form::DeprecatedLegacyModelMapImplementation + extend Dry::Initializer + + TYPE = Operations::Types::Hash.map( + Operations::Types::Coercible::Array.of( + Operations::Types::String | Operations::Types::Symbol | Operations::Types.Instance(Regexp) + ), + Operations::Types::String + ) + + param :model_map_hash, TYPE, default: proc { {} } + + def call(path) + model_map_hash.find do |pathspec, _model| + path.size == pathspec.size && path.zip(pathspec).all? do |slug, pattern| + pattern.is_a?(Regexp) ? pattern.match?(slug) : slug == pattern + end + end&.second + end +end diff --git a/lib/operations/inspect.rb b/lib/operations/inspect.rb index 939d716..db4b588 100644 --- a/lib/operations/inspect.rb +++ b/lib/operations/inspect.rb @@ -5,24 +5,26 @@ class Operations::Inspect < Module extend Dry::Initializer param :attributes, Operations::Types::Coercible::Array.of(Operations::Types::Symbol), reader: false + param :value_methods, Operations::Types::Hash.map(Operations::Types::Symbol, Operations::Types::Symbol) def initialize(*attributes, **kwargs) - super(attributes.flatten(1), **kwargs) + super(attributes.flatten(1), kwargs) - define_pretty_print(@attributes) + define_pretty_print(@attributes, @value_methods) end private - def define_pretty_print(attributes) + def define_pretty_print(attributes, value_methods) define_method(:pretty_print) do |pp| - pp.object_group(self) do + object_group_method = self.class.name ? :object_group : :object_address_group + pp.public_send(object_group_method, self) do pp.seplist(attributes, -> { pp.text "," }) do |name| pp.breakable " " pp.group(1) do pp.text name.to_s pp.text "=" - pp.pp __send__(name) + pp.pp __send__(value_methods[name] || name) end end end diff --git a/operations.gemspec b/operations.gemspec index 5bb5d0e..1477ec5 100644 --- a/operations.gemspec +++ b/operations.gemspec @@ -29,7 +29,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency "appraisal" spec.add_development_dependency "database_cleaner-active_record" - spec.add_development_dependency "sqlite3" + spec.add_development_dependency "sqlite3", "~> 1.4" spec.add_runtime_dependency "activerecord", ">= 5.2.0" spec.add_runtime_dependency "activesupport", ">= 5.2.0" diff --git a/spec/operations/command_spec.rb b/spec/operations/command_spec.rb index 8df5dce..060d3fb 100644 --- a/spec/operations/command_spec.rb +++ b/spec/operations/command_spec.rb @@ -1008,8 +1008,29 @@ def call; end subject(:pretty_inspect) { command.pretty_inspect } specify do - expect(pretty_inspect) - .to match(%r{\A#]+}, "Proc:0x").gsub(%r{Class:0x[^>]+}, "Class:0x")).to eq(<<~INSPECT) + #, + contract=#<# schema=#"key?(:name) \ + AND key[name](str? AND filled?)"}> rules=[#]>, + policies=[#], + idempotency=[], + preconditions=[#], + on_success=[#], + on_failure=[#], + form_model_map={}, + form_base=#, + form_class=# + #}>, + form_hydrator=#, + configuration=# transaction=# after_commit=#>> + INSPECT end end end diff --git a/spec/operations/form/base_spec.rb b/spec/operations/form/base_spec.rb index 0907704..2b5aa8d 100644 --- a/spec/operations/form/base_spec.rb +++ b/spec/operations/form/base_spec.rb @@ -2,7 +2,7 @@ RSpec.describe Operations::Form::Base do shared_examples "a form base" do |form_base:| - subject(:form) { form_class.new(attributes, messages: messages) } + subject(:form) { form_class.new(attributes, messages: messages, **options) } let(:form_class) do local_author_class = author_class @@ -22,11 +22,13 @@ end let(:post_class) do Class.new(author_class) do + attribute :id attribute :text end end let(:attributes) { {} } let(:messages) { {} } + let(:options) { {} } before do stub_const("Dummy::Form", form_class) @@ -79,17 +81,66 @@ expect(pretty_inspect).to eq(<<~INSPECT) # - #, + #, :tags=> - #, + #, :author=> - #, + # + #}>>, :posts=> - #}> + # + #, + :id=> + #, + :text=> + #}>>}> INSPECT end end + describe "#initialize" do + specify { expect(form_class.new(name: "Name")).to have_attributes(name: "Name") } + + specify do + expect(form_class.new({ name: "Name" }, messages: { name: ["Name error"] })) + .to have_attributes(name: "Name", errors: have_attributes(to_hash: { name: ["Name error"] })) + end + end + describe "#has_attribute?" do specify do expect(form).to have_attribute(:name) @@ -149,9 +200,9 @@ tags: ["tag1"], author: have_attributes(class: author_class, attributes: { title: "Batman" }), posts: [ - have_attributes(class: post_class, attributes: { title: "Post1", text: nil }), + have_attributes(class: post_class, attributes: { id: nil, title: "Post1", text: nil }), "wtf", - have_attributes(class: post_class, attributes: { title: nil, text: nil }) + have_attributes(class: post_class, attributes: { id: nil, title: nil, text: nil }) ] ) end @@ -169,7 +220,7 @@ expect(form.attributes).to include( author: have_attributes(class: author_class, attributes: { title: "Batman" }), posts: [ - have_attributes(class: post_class, attributes: { title: "Post1", text: nil }) + have_attributes(class: post_class, attributes: { id: nil, title: "Post1", text: nil }) ] ) end @@ -199,11 +250,87 @@ end end + describe "#method_missing" do + let(:attributes) do + { + name: 42, + tags: ["tag1"] + } + end + + specify { expect(form.name).to eq(42) } + specify { expect(form.tags).to eq(["tag1"]) } + specify { expect(form.build_author).to be_a(author_class) & have_attributes(title: nil) } + + specify do + expect(form.build_author({ title: "foo" }, messages: {})) + .to be_a(author_class) & have_attributes(title: "foo") + end + + specify { expect(form.build_post).to be_a(post_class) & have_attributes(id: nil, title: nil) } + + specify do + expect(form.build_post({ title: "foo" }, messages: {})) + .to be_a(post_class) & have_attributes(id: nil, title: "foo") + end + + specify { expect(form.build_tag).to be_nil } + specify { expect(form.foobar).to be_nil } + end + + describe "#respond_to_missing?" do + specify { expect(form).to respond_to(:name) } + specify { expect(form).to respond_to(:tags) } + specify { expect(form).to respond_to(:build_author) } + specify { expect(form).to respond_to(:build_post) } + specify { expect(form).to respond_to(:author_attributes=) } + specify { expect(form).to respond_to(:posts_attributes=) } + specify { expect(form).not_to respond_to(:foobar) } + specify { expect(form).not_to respond_to(:build_tag) } + specify { expect(form).not_to respond_to(:tags_attributes=) } + end + describe "#model_name" do specify { expect(form.model_name).to be_a(ActiveModel::Name) } specify { expect(form.model_name.to_s).to eq "Dummy::Form" } end + describe "#persisted?" do + subject(:persisted?) { form.persisted? } + + it { is_expected.to be true } + + context "when form has primary_key" do + let(:form) { post_class.new } + + it { is_expected.to be false } + end + + context "when form has primary_key and it is present" do + let(:form) { post_class.new(id: 42) } + + it { is_expected.to be true } + end + end + + describe "#new_record?" do + subject(:new_record?) { form.new_record? } + + it { is_expected.to be false } + + context "when form has primary_key" do + let(:form) { post_class.new } + + it { is_expected.to be true } + end + + context "when form has primary_key and it is present" do + let(:form) { post_class.new(id: 42) } + + it { is_expected.to be false } + end + end + describe "#errors" do let(:attributes) do { @@ -360,17 +487,21 @@ describe "#pretty_inspect" do subject(:pretty_inspect) { form.pretty_inspect } + before do + allow(form.errors).to receive(:inspect).and_return("#") + allow(form.author.errors).to receive(:inspect).and_return("#") + end + specify do - expect(pretty_inspect).to match(%r{ - #nil, \ - \s\s:tags=>\[], \ - \s\s:author=>\n\s\s\s#nil\}, \ - \s\s\s\serrors=#>, \ - \s\s:posts=>\[\]\}, \ - \serrors=#> - }x) + expect(pretty_inspect).to eq(<<~INSPECT) + #nil, + :tags=>[], + :author=> + #nil}, errors=#>, + :posts=>[]}, + errors=#> + INSPECT end end end diff --git a/spec/operations/form/builder_spec.rb b/spec/operations/form/builder_spec.rb index 1ccad0b..ddba6e3 100644 --- a/spec/operations/form/builder_spec.rb +++ b/spec/operations/form/builder_spec.rb @@ -8,9 +8,8 @@ subject(:form_class) do form_builder.build( key_map: schema.key_map, - namespace: namespace, - class_name: "MyForm", - model_map: model_map + model_map: model_map, + **options ) end @@ -43,73 +42,186 @@ end end end - let(:namespace) { stub_const("DummyNamespace", Module.new) } - let(:model_map) { { ["name"] => "Dummy1", ["translations", %r{singular|plural}] => "Dummy2" } } + let(:model_map_hash) { { ["name"] => "Dummy1", ["translations", %r{singular|plural}] => "Dummy2" } } + let(:model_map) { Operations::Form::DeprecatedLegacyModelMapImplementation.new(model_map_hash) } - it "defines attributes tree correctly" do - expect(form_class).to be < base_class - expect(form_class.name).to eq("DummyNamespace::MyForm") - expect(form_class.attributes).to match( - name: have_attributes(collection: false, form: nil, model_name: "Dummy1"), - posts: have_attributes(collection: true, form: DummyNamespace::MyForm::Post, model_name: nil), - translations: have_attributes(collection: false, form: DummyNamespace::MyForm::Translations, model_name: nil) - ) - expect(DummyNamespace::MyForm::Post.attributes).to match( - tags: have_attributes(collection: false, form: nil, model_name: nil), - title: have_attributes(collection: false, form: nil, model_name: nil) - ) - expect(DummyNamespace::MyForm::Translations.attributes).to match( - version: have_attributes(collection: false, form: nil, model_name: nil), - singular: have_attributes( - collection: false, - form: DummyNamespace::MyForm::Translations::Singular, - model_name: "Dummy2" - ), - plural: have_attributes( - collection: true, - form: DummyNamespace::MyForm::Translations::Plural, - model_name: "Dummy2" - ), - en: have_attributes( - collection: false, - form: DummyNamespace::MyForm::Translations::En, - model_name: nil - ), - "zh-CN": have_attributes( - collection: false, - form: DummyNamespace::MyForm::Translations::ZhCn, - model_name: nil + context "with model_name" do + let(:options) { { model_name: "my_form" } } + + it "defines attributes tree correctly" do + expect(form_class).to have_attributes( + name: nil, + model_name: "my_form", + attributes: { + name: have_attributes(collection: false, form: nil, model_name: "Dummy1"), + posts: have_attributes( + collection: true, + form: have_attributes( + name: nil, + model_name: "posts", + attributes: { + tags: have_attributes(collection: false, form: nil, model_name: nil), + title: have_attributes(collection: false, form: nil, model_name: nil) + } + ), + model_name: nil + ), + translations: have_attributes( + collection: false, + form: have_attributes( + name: nil, + model_name: "translation", + attributes: { + version: have_attributes(collection: false, form: nil, model_name: nil), + singular: have_attributes( + collection: false, + form: have_attributes( + name: nil, + model_name: "singular", + attributes: { text: have_attributes(collection: false, form: nil, model_name: nil) } + ), + model_name: "Dummy2" + ), + plural: have_attributes( + collection: true, + form: have_attributes( + name: nil, + model_name: "plural", + attributes: { text: have_attributes(collection: false, form: nil, model_name: nil) } + ), + model_name: "Dummy2" + ), + en: have_attributes( + collection: false, + form: have_attributes( + name: nil, + model_name: "en", + attributes: { text: have_attributes(collection: false, form: nil, model_name: nil) } + ), + model_name: nil + ), + "zh-CN": have_attributes( + collection: false, + form: have_attributes( + name: nil, + model_name: "zh-CN", + attributes: { text: have_attributes(collection: false, form: nil, model_name: nil) } + ), + model_name: nil + ) + } + ), + model_name: nil + ) + } ) - ) - expect(DummyNamespace::MyForm::Translations.instance_methods(false)) - .to include(:singular_attributes=, :plural_attributes=) - expect(DummyNamespace::MyForm::Translations::Singular.attributes).to match( - text: have_attributes(collection: false, form: nil, model_name: nil) - ) - expect(DummyNamespace::MyForm::Translations::Plural.attributes).to match( - text: have_attributes(collection: false, form: nil, model_name: nil) - ) - expect(DummyNamespace::MyForm::Translations::En.attributes).to match( - text: have_attributes(collection: false, form: nil, model_name: nil) - ) - expect(DummyNamespace::MyForm::Translations::ZhCn.attributes).to match( - text: have_attributes(collection: false, form: nil, model_name: nil) - ) + expect(form_class.attributes[:translations].form.instance_methods(false)) + .to include(:singular_attributes=, :plural_attributes=) + end + + context "when called twice with the same params" do + let(:form1) { form_class } + let(:form2) do + form_builder.build( + key_map: schema.key_map, + model_map: model_map, + **options + ) + end + + it "does not return the same class" do + expect(form1).not_to equal(form2) + end + end end - context "when called twice with the same params" do - let(:form1) { form_class } - let(:form2) do - form_builder.build( - key_map: schema.key_map, - namespace: namespace, - class_name: "MyForm", - model_map: {} + context "with namespace and class_name" do + let(:options) { { namespace: namespace, class_name: "MyForm" } } + let(:namespace) { stub_const("DummyNamespace", Module.new) } + + it "defines attributes tree correctly" do + expect(form_class).to have_attributes( + name: "DummyNamespace::MyForm", + model_name: "DummyNamespace::MyForm", + attributes: { + name: have_attributes(collection: false, form: nil, model_name: "Dummy1"), + posts: have_attributes(collection: true, form: DummyNamespace::MyForm::Post, model_name: nil), + translations: have_attributes(collection: false, + form: DummyNamespace::MyForm::Translations, model_name: nil) + } + ) + expect(DummyNamespace::MyForm::Post).to have_attributes( + name: "DummyNamespace::MyForm::Post", + model_name: "DummyNamespace::MyForm::Post", + attributes: { + tags: have_attributes(collection: false, form: nil, model_name: nil), + title: have_attributes(collection: false, form: nil, model_name: nil) + } + ) + expect(DummyNamespace::MyForm::Translations).to have_attributes( + name: "DummyNamespace::MyForm::Translations", + model_name: "DummyNamespace::MyForm::Translations", + attributes: { + version: have_attributes(collection: false, form: nil, model_name: nil), + singular: have_attributes( + collection: false, + form: DummyNamespace::MyForm::Translations::Singular, + model_name: "Dummy2" + ), + plural: have_attributes( + collection: true, + form: DummyNamespace::MyForm::Translations::Plural, + model_name: "Dummy2" + ), + en: have_attributes( + collection: false, + form: DummyNamespace::MyForm::Translations::En, + model_name: nil + ), + "zh-CN": have_attributes( + collection: false, + form: DummyNamespace::MyForm::Translations::ZhCn, + model_name: nil + ) + } + ) + expect(DummyNamespace::MyForm::Translations.instance_methods(false)) + .to include(:singular_attributes=, :plural_attributes=) + expect(DummyNamespace::MyForm::Translations::Singular).to have_attributes( + name: "DummyNamespace::MyForm::Translations::Singular", + model_name: "DummyNamespace::MyForm::Translations::Singular", + attributes: { text: have_attributes(collection: false, form: nil, model_name: nil) } + ) + expect(DummyNamespace::MyForm::Translations::Plural).to have_attributes( + name: "DummyNamespace::MyForm::Translations::Plural", + model_name: "DummyNamespace::MyForm::Translations::Plural", + attributes: { text: have_attributes(collection: false, form: nil, model_name: nil) } + ) + expect(DummyNamespace::MyForm::Translations::En).to have_attributes( + name: "DummyNamespace::MyForm::Translations::En", + model_name: "DummyNamespace::MyForm::Translations::En", + attributes: { text: have_attributes(collection: false, form: nil, model_name: nil) } + ) + expect(DummyNamespace::MyForm::Translations::ZhCn).to have_attributes( + name: "DummyNamespace::MyForm::Translations::ZhCn", + model_name: "DummyNamespace::MyForm::Translations::ZhCn", + attributes: { text: have_attributes(collection: false, form: nil, model_name: nil) } ) end - it "does not redefine the constant" do - expect(form1).to equal(form2) + context "when called twice with the same params" do + let(:form1) { form_class } + let(:form2) do + form_builder.build( + key_map: schema.key_map, + model_map: model_map, + **options + ) + end + + it "does not redefine the constant" do + expect(form1).to equal(form2) + end end end end diff --git a/spec/operations/form/deprecated_legacy_model_map_implementation_spec.rb b/spec/operations/form/deprecated_legacy_model_map_implementation_spec.rb new file mode 100644 index 0000000..c9952dd --- /dev/null +++ b/spec/operations/form/deprecated_legacy_model_map_implementation_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +RSpec.describe Operations::Form::DeprecatedLegacyModelMapImplementation do + subject(:model_map) { described_class.new(model_map_hash) } + + let(:model_map_hash) { { ["name"] => "Dummy1", ["translations", %r{singular|plural}] => "Dummy2" } } + + describe "#call" do + subject(:call) { model_map.call(path) } + + context "with non-existing path" do + let(:path) { ["foobar"] } + + it { is_expected.to be_nil } + end + + context "with simple path" do + let(:path) { ["name"] } + + it { is_expected.to eq("Dummy1") } + end + + context "with unfinished path" do + let(:path) { ["translations"] } + + it { is_expected.to be_nil } + end + + context "with regexp-matched path" do + let(:path) { %w[translations singular] } + + it { is_expected.to eq("Dummy2") } + end + end +end diff --git a/spec/operations/form_spec.rb b/spec/operations/form_spec.rb new file mode 100644 index 0000000..8587f6e --- /dev/null +++ b/spec/operations/form_spec.rb @@ -0,0 +1,165 @@ +# frozen_string_literal: true + +RSpec.describe Operations::Form do + subject(:form) { described_class.new(command, **options) } + + let(:contract) do + Operations::Contract.build do + schema do + required(:entities).array(:hash) do + required(:id).filled(:integer) + end + optional(:name).filled(:string) + end + end + end + let(:operation) do + Class.new do + def call(_, **) + Dry::Monads::Success({}) + end + end + end + let(:command) do + Operations::Command.new( + operation.new, + contract: contract, + policy: nil + ) + end + let(:default_options) do + { + model_map: proc { |_path| "DummyModel" }, + hydrator: proc { |_form_class, params, **_context| { ignored: 42, name: "Batman" }.merge(params) } + } + end + let(:options) { default_options } + + before do + stub_const("DummyOperation", operation) + end + + describe "#build" do + subject(:build) { form.build(params, **context) } + + let(:params) { {} } + let(:context) { {} } + + specify do + expect(build).to be_a(Operations::Form::Base) & have_attributes( + entities: [], + name: "Batman", + attributes: { entities: [], name: "Batman" } + ) + expect(build.errors.to_hash).to be_empty + end + + context "when params are given" do + let(:params) do + double(to_unsafe_hash: { + entities: [{ "id" => 42 }], "dummy_operation_form" => { name: "Superman" } + }) + end + + specify do + expect(build).to be_a(Operations::Form::Base) & have_attributes( + entities: [be_a(Operations::Form::Base) & have_attributes( + id: 42, + attributes: { id: 42 }, + errors: be_empty + )], + name: "Superman", + attributes: { + entities: [be_a(Operations::Form::Base) & have_attributes( + id: 42, + attributes: { id: 42 }, + errors: be_empty + )], + name: "Superman" + }, + errors: be_empty + ) + end + end + end + + describe "#persist" do + subject(:persist) { form.persist(params, **context) } + + let(:params) { {} } + let(:context) { {} } + let(:options) do + default_options.merge( + model_name: "dummy_form", + params_transformations: lambda { |_form_class, params, **_context| + params.transform_keys { |key| key == :alias_name ? :name : key } + } + ) + end + + specify do + expect(persist).to be_a(Operations::Form::Base) & have_attributes( + entities: [], + name: "Batman", + attributes: { entities: [], name: "Batman" } + ) + expect(persist.errors.to_hash).to eq({ entities: ["is missing"] }) + end + + context "when params are given" do + let(:params) { { entities: [{ "id" => 42 }], "dummy_form" => { alias_name: "Superman" } } } + + specify do + expect(persist).to be_a(Operations::Form::Base) & have_attributes( + entities: [be_a(Operations::Form::Base) & have_attributes( + id: 42, + attributes: { id: 42 }, + errors: be_empty + )], + name: "Superman", + attributes: { + entities: [be_a(Operations::Form::Base) & have_attributes( + id: 42, + attributes: { id: 42 }, + errors: be_empty + )], + name: "Superman" + }, + errors: be_empty + ) + end + end + end + + describe "#pretty_print" do + subject(:pretty_inspect) { form.pretty_inspect } + + specify do + expect(pretty_inspect.gsub(%r{Proc:0x[^>]+}, "Proc:0x")).to eq(<<~INSPECT) + # + # + #}>>, + :name=> + #}>, + model_map=#, + params_transformations=[], + hydrator=#> + INSPECT + end + end +end From 868fbbf297fcf465b5063210eeb84f786ba865c9 Mon Sep 17 00:00:00 2001 From: Arkadiy Zabazhanov Date: Wed, 8 May 2024 12:56:17 +0700 Subject: [PATCH 2/7] Apply suggestions --- README.md | 2 +- lib/operations/form/base.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b434ef9..14afbc3 100644 --- a/README.md +++ b/README.md @@ -979,7 +979,7 @@ end # And this will transform nested attributes hash from the form to an array acceptable by the operation: # from -# `{ id: 42, sections: { '0' => { id: 1, content: "First paragraph" }, 'new' => { content: 'New Paragraph' } } }` +# `{ id: 42, sections_attributes: { '0' => { id: 1, content: "First paragraph" }, 'new' => { content: 'New Paragraph' } } }` # into # `{ id: 42, sections: [{ id: 1, content: "First paragraph" }, { content: 'New Paragraph' }] }` class NestedAttributes diff --git a/lib/operations/form/base.rb b/lib/operations/form/base.rb index 5e2df5c..c3cf6e9 100644 --- a/lib/operations/form/base.rb +++ b/lib/operations/form/base.rb @@ -20,8 +20,8 @@ # # @see Operations::Form::Builder class Operations::Form::Base - BUILD_ASSOCIATION_PREFIX = %r{\Abuild_}.freeze - NESTED_ATTRIBUTES_WRITER_SUFFIX = %r{_attributes=\z}.freeze + BUILD_ASSOCIATION_PREFIX = "build_" + NESTED_ATTRIBUTES_SUFFIX = "_attributes=" # :nodoc: module ClassMethods @@ -246,11 +246,11 @@ def wrap_object(path, data, form) end def build_attribute_name(name) - name.to_s.sub(BUILD_ASSOCIATION_PREFIX, "").to_sym if name.to_s.match?(BUILD_ASSOCIATION_PREFIX) + name.to_s.delete_prefix(BUILD_ASSOCIATION_PREFIX).to_sym if name.to_s.start_with?(BUILD_ASSOCIATION_PREFIX) end def nested_attribute_name(name) - name.to_s.sub(NESTED_ATTRIBUTES_WRITER_SUFFIX, "").to_sym if name.to_s.match?(NESTED_ATTRIBUTES_WRITER_SUFFIX) + name.to_s.delete_suffix(NESTED_ATTRIBUTES_SUFFIX).to_sym if name.to_s.end_with?(NESTED_ATTRIBUTES_SUFFIX) end def build_nested_form?(name) From 33ed275bb18703219acbf35491412126b8ef8915 Mon Sep 17 00:00:00 2001 From: Arkadiy Zabazhanov Date: Thu, 23 May 2024 20:45:20 +0700 Subject: [PATCH 3/7] Update README.md Co-authored-by: Adam Gradowski <31541037+adamgrad@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 14afbc3..28cab35 100644 --- a/README.md +++ b/README.md @@ -939,7 +939,7 @@ class Post::Update::ModelMap end ``` -In forms, params imput is already transformed to extract the nested data with the form name. `form_for @post_update_form` will generate the form that send params nested under the `params[:post_update_form]` key. By default operation forms extract this form data and send it to the operation at the top level, so `{ id: 42, post_update_form: { title: "Post Title" } }` params will be sent to the operation as `{ id: 42, title: "Post Title" }`. Strong params are also accepted by the form, though they are being converted with `to_unsafe_hash`. +In forms, params input is already transformed to extract the nested data with the form name. `form_for @post_update_form` will generate the form that send params nested under the `params[:post_update_form]` key. By default operation forms extract this form data and send it to the operation at the top level, so `{ id: 42, post_update_form: { title: "Post Title" } }` params will be sent to the operation as `{ id: 42, title: "Post Title" }`. Strong params are also accepted by the form, though they are being converted with `to_unsafe_hash`. It is possible to add more params transfomations to the form in cases when operation contract is different from the params structure: From d8a7727da6d4d980adbfb4539223d6e770672a15 Mon Sep 17 00:00:00 2001 From: Arkadiy Zabazhanov Date: Sat, 27 Jul 2024 15:43:25 +0700 Subject: [PATCH 4/7] Fix Rubocop offences --- lib/operations/contract/messages_resolver.rb | 2 +- operations.gemspec | 12 ++++++------ spec/spec_helper.rb | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/operations/contract/messages_resolver.rb b/lib/operations/contract/messages_resolver.rb index db22ab9..af34d9b 100644 --- a/lib/operations/contract/messages_resolver.rb +++ b/lib/operations/contract/messages_resolver.rb @@ -6,6 +6,6 @@ class Operations::Contract::MessagesResolver < Dry::Validation::Messages::Resolv def call(message:, meta: Dry::Schema::EMPTY_HASH, **rest) meta = meta.merge(code: message) if message.is_a?(Symbol) - super(message: message, meta: meta, **rest) + super end end diff --git a/operations.gemspec b/operations.gemspec index 1477ec5..a3c3336 100644 --- a/operations.gemspec +++ b/operations.gemspec @@ -31,12 +31,12 @@ Gem::Specification.new do |spec| spec.add_development_dependency "database_cleaner-active_record" spec.add_development_dependency "sqlite3", "~> 1.4" - spec.add_runtime_dependency "activerecord", ">= 5.2.0" - spec.add_runtime_dependency "activesupport", ">= 5.2.0" - spec.add_runtime_dependency "after_commit_everywhere" - spec.add_runtime_dependency "dry-monads" - spec.add_runtime_dependency "dry-struct" - spec.add_runtime_dependency "dry-validation" + spec.add_dependency "activerecord", ">= 5.2.0" + spec.add_dependency "activesupport", ">= 5.2.0" + spec.add_dependency "after_commit_everywhere" + spec.add_dependency "dry-monads" + spec.add_dependency "dry-struct" + spec.add_dependency "dry-validation" spec.metadata = { "rubygems_mfa_required" => "true" } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 625fa46..6dc67a5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -15,7 +15,7 @@ end end -class User < ActiveRecord::Base # rubocop:disable Rails/ApplicationRecord +class User < ActiveRecord::Base validates :name, presence: true def self.localized_attr_name_for(name, locale) From 1d90c5b7e3b639773d45d30821db36f7528145d7 Mon Sep 17 00:00:00 2001 From: Arkadiy Zabazhanov Date: Sat, 27 Jul 2024 16:00:27 +0700 Subject: [PATCH 5/7] Make hydrator optional --- lib/operations/form.rb | 10 +++++----- spec/operations/form_spec.rb | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/operations/form.rb b/lib/operations/form.rb index 61fde88..a0fe9a0 100644 --- a/lib/operations/form.rb +++ b/lib/operations/form.rb @@ -3,8 +3,8 @@ # Configures and defines a form object factory. class Operations::Form include Dry::Core::Constants - include Dry::Equalizer(:command, :form_class, :model_map, :params_transformations, :hydrator) - include Operations::Inspect.new(:form_class, :model_map, :params_transformations, :hydrator) + include Dry::Equalizer(:command, :model_map, :params_transformations, :hydrator, :form_class) + include Operations::Inspect.new(:model_name, :model_map, :params_transformations, :hydrator, :form_class) # We need to make deprecated inheritance from Operations::Form act exactly the # same way as from Operations::Form::Base. In order to do this, we are encapsulating all the @@ -28,12 +28,12 @@ def self.inherited(subclass) include Dry::Initializer.define(lambda do param :command, type: Operations::Types.Interface(:operation, :contract, :call) - option :base_class, type: Operations::Types::Class, default: proc { ::Operations::Form::Base } - option :model_map, type: Operations::Types.Interface(:call).optional, default: proc {} option :model_name, type: Operations::Types::String.optional, default: proc {}, reader: false + option :model_map, type: Operations::Types.Interface(:call).optional, default: proc {} option :params_transformations, type: Operations::Types::Coercible::Array.of(Operations::Types.Interface(:call)), default: proc { [] } option :hydrator, type: Operations::Types.Interface(:call).optional, default: proc {} + option :base_class, type: Operations::Types::Class, default: proc { ::Operations::Form::Base } end) def build(params = EMPTY_HASH, **context) @@ -62,7 +62,7 @@ def transform_params(params, **context) def instantiate_form(operation_result) form_class.new( - hydrator.call(form_class, operation_result.params, **operation_result.context), + hydrator&.call(form_class, operation_result.params, **operation_result.context) || {}, messages: operation_result.errors.to_h, operation_result: operation_result ) diff --git a/spec/operations/form_spec.rb b/spec/operations/form_spec.rb index 8587f6e..de1da2f 100644 --- a/spec/operations/form_spec.rb +++ b/spec/operations/form_spec.rb @@ -54,6 +54,19 @@ def call(_, **) expect(build.errors.to_hash).to be_empty end + context "when model_map and hydrator are missing" do + let(:default_options) { {} } + + specify do + expect(build).to be_a(Operations::Form::Base) & have_attributes( + entities: [], + name: nil, + attributes: { entities: [], name: nil } + ) + expect(build.errors.to_hash).to be_empty + end + end + context "when params are given" do let(:params) do double(to_unsafe_hash: { @@ -106,6 +119,19 @@ def call(_, **) expect(persist.errors.to_hash).to eq({ entities: ["is missing"] }) end + context "when model_map and hydrator are missing" do + let(:default_options) { {} } + + specify do + expect(persist).to be_a(Operations::Form::Base) & have_attributes( + entities: [], + name: nil, + attributes: { entities: [], name: nil } + ) + expect(persist.errors.to_hash).to eq({ entities: ["is missing"] }) + end + end + context "when params are given" do let(:params) { { entities: [{ "id" => 42 }], "dummy_form" => { alias_name: "Superman" } } } @@ -137,6 +163,10 @@ def call(_, **) specify do expect(pretty_inspect.gsub(%r{Proc:0x[^>]+}, "Proc:0x")).to eq(<<~INSPECT) #, + params_transformations=[], + hydrator=#, form_class=# #}>, - model_map=#, - params_transformations=[], - hydrator=#> + form=nil>}>> INSPECT end end From f03a3d2568655203f400c6e371bafbccb0759305 Mon Sep 17 00:00:00 2001 From: Arkadiy Zabazhanov Date: Sun, 28 Jul 2024 07:53:47 +0700 Subject: [PATCH 6/7] Improve documentation --- README.md | 5 +++-- lib/operations/form.rb | 18 ++++++++++++++++++ ...precated_legacy_model_map_implementation.rb | 5 +++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 28cab35..59d20de 100644 --- a/README.md +++ b/README.md @@ -926,8 +926,8 @@ class Post::Update def self.default_form @default_form ||= Operations::Form.new( default, - hydrator: Post::Update::Hydrator.new, - model_map: Post::Update::ModelMap.new + model_map: Post::Update::ModelMap.new, + hydrator: Post::Update::Hydrator.new ) end end @@ -948,6 +948,7 @@ class Post::Update def self.default_form @default_form ||= Operations::Form.new( default, + model_name: "post_update_form", # form name can be customized params_transformations: [ ParamsMap.new(id: :post_id), NestedAttributes.new(:sections) diff --git a/lib/operations/form.rb b/lib/operations/form.rb index a0fe9a0..b5ce226 100644 --- a/lib/operations/form.rb +++ b/lib/operations/form.rb @@ -1,6 +1,24 @@ # frozen_string_literal: true # Configures and defines a form object factory. +# Forms can be defined on top of commants and used in the user-facing controllers. +# Form objects are Rails-specific and support everything that is needed for Rails' +# form rendering helpers. They are designed to replace direct usage of ActiveRecord +# models in controllers and views and act as an integration bridge from Rails +# application to the Operations framework. +# +# @example +# +# command = Operations::Command.new(...) +# form = Operations::Form.new(command) +# +# @form_object = form.build(params) +# +# form_for @form_object, url: ... +# +# @form_object = form.persist(params) +# respond_with @form_object +# class Operations::Form include Dry::Core::Constants include Dry::Equalizer(:command, :model_map, :params_transformations, :hydrator, :form_class) diff --git a/lib/operations/form/deprecated_legacy_model_map_implementation.rb b/lib/operations/form/deprecated_legacy_model_map_implementation.rb index 0c786c7..bcceb0b 100644 --- a/lib/operations/form/deprecated_legacy_model_map_implementation.rb +++ b/lib/operations/form/deprecated_legacy_model_map_implementation.rb @@ -3,6 +3,11 @@ # Traverses the passed {Dry::Schema::KeyMap} and generates # {Operations::Form::Base} classes on the fly. Handles nested structures. # +# This class will be removed along with Operations::Command#form_model_map +# option removal along with the rest of form-related stuff. +# +# Don't use this class in you code, instead, copy it and modify to you liking. +# # @see Operations::Form::Base class Operations::Form::DeprecatedLegacyModelMapImplementation extend Dry::Initializer From c02799202caf5e138687b01bc450edc509958993 Mon Sep 17 00:00:00 2001 From: Arkadiy Zabazhanov Date: Sun, 28 Jul 2024 08:13:00 +0700 Subject: [PATCH 7/7] Update changelog --- CHANGELOG.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f27299..7c43a9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ # Changelog -## [0.6.3](https://github.com/BookingSync/operations/tree/main) +## [0.7.0](https://github.com/BookingSync/operations/tree/main) + +### Added + +- Implement new forms system detaching it from operations [\#47](https://github.com/BookingSync/operations/pull/47) ([pyromaniac](https://github.com/pyromaniac)) + +### Improvements + +- Better inspect for all the opjects [\#45](https://github.com/BookingSync/operations/pull/45) ([pyromaniac](https://github.com/pyromaniac)) +- Use #to_hash instead of #as_json [\#44](https://github.com/BookingSync/operations/pull/44) ([pyromaniac](https://github.com/pyromaniac)) ### Fixes