From 37702aa25290cc7e241bd8271f78433aedd50aae Mon Sep 17 00:00:00 2001 From: Arkadiy Zabazhanov Date: Mon, 29 Jul 2024 11:20:25 +0700 Subject: [PATCH] Allow configuring form persistence state --- CHANGELOG.md | 3 +- README.md | 33 +++++++++- UPGRADING_FORMS.md | 101 ++++++++++++++++++++++++++++++ lib/operations/command.rb | 3 +- lib/operations/form.rb | 7 ++- lib/operations/form/base.rb | 3 +- lib/operations/form/builder.rb | 7 ++- spec/operations/form/base_spec.rb | 2 + spec/operations/form_spec.rb | 13 ++-- 9 files changed, 156 insertions(+), 16 deletions(-) create mode 100644 UPGRADING_FORMS.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c43a9e..5632138 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ ### Added -- Implement new forms system detaching it from operations [\#47](https://github.com/BookingSync/operations/pull/47) ([pyromaniac](https://github.com/pyromaniac)) +- Added `persisted:` option to the new forms definition. [\#48](https://github.com/BookingSync/operations/pull/48) ([pyromaniac](https://github.com/pyromaniac)) +- Implement new forms system detaching it from operations. Please check [UPGRADING_FORMS.md](UPGRADING_FORMS.md) for more details. [\#47](https://github.com/BookingSync/operations/pull/47) ([pyromaniac](https://github.com/pyromaniac)) ### Improvements diff --git a/README.md b/README.md index 59d20de..05d6690 100644 --- a/README.md +++ b/README.md @@ -853,6 +853,8 @@ In this case, `Order::MarkAsCompleted.system.call(...)` will be used in, say, co ### Form objects +Form objects were refactored to be separate from Command. Please check [UPGRADING_FORMS.md](UPGRADING_FORMS.md) for more details. + While we normally recommend using frontend-backend separation, it is still possible to use this framework with Rails view helpers: ```ruby @@ -885,7 +887,7 @@ class Post::Update end ``` -Then, the form can be used as any other form object: +Then, the form can be used as any other form object. Unfortunately, there is no way to figure out the correct route for the operation form object, so it have to be provided manually: ```erb # views/posts/edit.html.erb @@ -939,7 +941,18 @@ class Post::Update::ModelMap end ``` -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`. +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`. Though the form name can be customized if necessary: + +```ruby +class Post::Update + def self.default_form + @default_form ||= Operations::Form.new( + default, + model_name: "custom_post_update_form", # form name can be customized + ) + end +end +``` It is possible to add more params transfomations to the form in cases when operation contract is different from the params structure: @@ -948,7 +961,6 @@ 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) @@ -994,6 +1006,21 @@ class NestedAttributes end ``` +By default, the top-level form objects instantiated from the form will have `persisted?` flag set to `true`. This will result the form to use the `PATCH` werb like for any persisted AR object. If it is required to generate a form with the `POST` verb in case of operation, say, creating some objects, this default behavior can be customised: + +```ruby +class Post::Create + def self.default_form + @default_form ||= Operations::Form.new( + default, + persisted: false + ) + end +end +``` + +Note that operation itself is agnostic to the persistence layer, so there is no way for it to figure out this semanticsa automatically. + ## Development After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake spec` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. diff --git a/UPGRADING_FORMS.md b/UPGRADING_FORMS.md new file mode 100644 index 0000000..c5fd86e --- /dev/null +++ b/UPGRADING_FORMS.md @@ -0,0 +1,101 @@ +# New form objects + +In version 0.7.0, a new form objects were introduced. The old way of using form objects is deprecated and will be removed in 1.0.0. In order to upgrade your code to start using the new way please follow the guide: + +## Replace `Operations::Form` with `Operations::Form::Base` + +If you have any classes that were inherited from `Operations::Form`, please change them to inherit from `Operations::Form::Base` + +## Define forms as separate objects on top of the existing operations: + +```ruby +# Before +class Post::Update + def self.default + @default ||= Operations::Command.new( + ..., + form_hydrator: Post::Update::Hydrator.new, + form_model_map: { + [%r{.+}] => "Post" + } + ) + end +end + +# After +class Post::Update + def self.default + @default ||= Operations::Command.new(...) + end + + def self.default_form + @default_form ||= Operations::Form.new( + default, + hydrator: Post::Update::Hydrator.new, + model_map: Post::Update::ModelMap.new, + params_transformations: [ + ParamsMap.new(id: :post_id) + ] + ) + end +end +``` + +Where `Post::Update::ModelMap` can be a copy of [Operations::Form::DeprecatedLegacyModelMapImplementation](https://github.com/BookingSync/operations/blob/main/lib/operations/form/deprecated_legacy_model_map_implementation.rb) or your own implementation. + +And `ParamsMap` can be as simple as: + +```ruby +class ParamsMap + extend Dry::Initializer + + param :params_map + + def call(_form_class, params, **_context) + params.transform_keys { |key| params_map[key] || key } + end +end +``` + +## Change the way you use forms in you controllers and views: + +```ruby +# Before +class PostsController < ApplicationController + def edit + @post_update = Post::Update.default.callable( + { post_id: params[:id] }, + current_user: current_user + ) + + respond_with @post_update + 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 + ) + + respond_with @post_update, location: edit_post_url(@post_update.context[:post]) + end +end + +# After +class PostsController < ApplicationController + def edit + @post_update_form = Post::Update.default_form.build(params, current_user: current_user) + + respond_with @post_update_form + end + + def update + @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 +``` + +Notice that `callable` and `call` methond are replaced with `build` and `persist` respectively. diff --git a/lib/operations/command.rb b/lib/operations/command.rb index 8f1c998..29b1637 100644 --- a/lib/operations/command.rb +++ b/lib/operations/command.rb @@ -387,7 +387,8 @@ def build_form_class key_map: contract.class.schema.key_map, model_map: Operations::Form::DeprecatedLegacyModelMapImplementation.new(form_model_map), namespace: operation.class, - class_name: form_class_name + class_name: form_class_name, + persisted: true ) end diff --git a/lib/operations/form.rb b/lib/operations/form.rb index b5ce226..cfa591d 100644 --- a/lib/operations/form.rb +++ b/lib/operations/form.rb @@ -21,8 +21,8 @@ # class Operations::Form include Dry::Core::Constants - include Dry::Equalizer(:command, :model_map, :params_transformations, :hydrator, :form_class) - include Operations::Inspect.new(:model_name, :model_map, :params_transformations, :hydrator, :form_class) + include Dry::Equalizer(:command, :model_map, :persisted, :params_transformations, :hydrator, :form_class) + include Operations::Inspect.new(:model_name, :model_map, :persisted, :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 @@ -48,6 +48,7 @@ def self.inherited(subclass) param :command, type: Operations::Types.Interface(:operation, :contract, :call) option :model_name, type: Operations::Types::String.optional, default: proc {}, reader: false option :model_map, type: Operations::Types.Interface(:call).optional, default: proc {} + option :persisted, type: Operations::Types::Bool, default: proc { true } 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 {} @@ -64,7 +65,7 @@ def persist(params = EMPTY_HASH, **context) 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) + .build(key_map: key_map, model_map: model_map, model_name: model_name, persisted: persisted) end private diff --git a/lib/operations/form/base.rb b/lib/operations/form/base.rb index c3cf6e9..057a031 100644 --- a/lib/operations/form/base.rb +++ b/lib/operations/form/base.rb @@ -47,6 +47,7 @@ def self.extended(base) base.class_attribute :attributes, instance_accessor: false, default: {} base.class_attribute :primary_key, instance_accessor: false, default: :id + base.class_attribute :persisted, instance_accessor: false, default: nil base.define_method :initialize do |*args, **kwargs| args.empty? && kwargs.present? ? super(kwargs, **{}) : super(*args, **kwargs) @@ -132,7 +133,7 @@ def model_name end def persisted? - !has_attribute?(self.class.primary_key) || read_attribute(self.class.primary_key).present? + self.class.persisted.nil? ? read_attribute(self.class.primary_key).present? : self.class.persisted end def new_record? diff --git a/lib/operations/form/builder.rb b/lib/operations/form/builder.rb index a36ac2c..ada8383 100644 --- a/lib/operations/form/builder.rb +++ b/lib/operations/form/builder.rb @@ -11,18 +11,19 @@ class Operations::Form::Builder option :base_class, Operations::Types::Instance(Class) - def build(key_map:, model_map:, namespace: nil, class_name: nil, model_name: nil) + def build(key_map:, model_map:, namespace: nil, class_name: nil, model_name: nil, persisted: nil) return namespace.const_get(class_name) if namespace && class_name && namespace.const_defined?(class_name) - traverse(key_map, model_map, namespace, class_name, model_name, []) + traverse(key_map, model_map, namespace, class_name, model_name, [], persisted: persisted) end private - def traverse(key_map, model_map, namespace, class_name, model_name, path) + def traverse(key_map, model_map, namespace, class_name, model_name, path, persisted: nil) form = Class.new(base_class) namespace.const_set(class_name, form) if namespace&.name && class_name define_model_name(form, model_name) if model_name && !form.name + form.persisted = persisted key_map.each { |key| define_attribute(form, model_map, key, path) } form diff --git a/spec/operations/form/base_spec.rb b/spec/operations/form/base_spec.rb index 2b5aa8d..fd3a69f 100644 --- a/spec/operations/form/base_spec.rb +++ b/spec/operations/form/base_spec.rb @@ -9,6 +9,8 @@ local_post_class = post_class Class.new(form_base) do + self.persisted = true + attribute :name attribute :tags, collection: true attribute :author, form: local_author_class diff --git a/spec/operations/form_spec.rb b/spec/operations/form_spec.rb index de1da2f..3cd4a65 100644 --- a/spec/operations/form_spec.rb +++ b/spec/operations/form_spec.rb @@ -54,11 +54,12 @@ def call(_, **) expect(build.errors.to_hash).to be_empty end - context "when model_map and hydrator are missing" do - let(:default_options) { {} } + context "when model_map and hydrator are missing but persisted is false" do + let(:options) { { persisted: false } } specify do expect(build).to be_a(Operations::Form::Base) & have_attributes( + persisted?: false, entities: [], name: nil, attributes: { entities: [], name: nil } @@ -76,6 +77,7 @@ def call(_, **) specify do expect(build).to be_a(Operations::Form::Base) & have_attributes( + persisted?: true, entities: [be_a(Operations::Form::Base) & have_attributes( id: 42, attributes: { id: 42 }, @@ -119,11 +121,12 @@ 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) { {} } + context "when model_map and hydrator are missing but persisted is false" do + let(:options) { { persisted: false } } specify do expect(persist).to be_a(Operations::Form::Base) & have_attributes( + persisted?: false, entities: [], name: nil, attributes: { entities: [], name: nil } @@ -137,6 +140,7 @@ def call(_, **) specify do expect(persist).to be_a(Operations::Form::Base) & have_attributes( + persisted?: true, entities: [be_a(Operations::Form::Base) & have_attributes( id: 42, attributes: { id: 42 }, @@ -165,6 +169,7 @@ def call(_, **) #, + persisted=true, params_transformations=[], hydrator=#, form_class=#