reform-rails
reform-rails copied to clipboard
support nil as model for reform
In some cases reform is just used for coercion & validation without any persistence done. In these cases it was advised to use nil as reform model & to mark all properties as virtual.
this works fine, however fails as .persisted? is called on nil upon rendering in actionpack-3.2.22.2/lib/action_view/helpers/form_helper.rb:388
...
as = options[:as]
action, method = object.respond_to?(:persisted?) && object.persisted? ? [:edit, :put] : [:new, :post]
options[:html].reverse_merge!(
:class => as ? "#{action}_#{as}" : dom_class(object, action),
:id => as ? "#{action}_#{as}" : [options[:namespace], dom_id(object, action)].compact.join("_").presence,
:method => method
)
...
got two tests failing but they also fail before my change:
$ bundle exec ruby -Ilib:test test/model_reflections_test.rb
config.eager_load is set to nil. Please update your config/environments/*.rb files accordingly:
* development - set it to false
* test - set it to false (unless you use a tool that preloads your test environment)
* production - set it to true
DEPRECATION WARNING: Using a dynamic :controller segment in a route is deprecated and will be removed in Rails 5.1. (called from block in <top (required)> at reform_rails/test/rails-app/config/routes.rb:2)
DEPRECATION WARNING: Using a dynamic :action segment in a route is deprecated and will be removed in Rails 5.1. (called from block in <top (required)> at reform_rails/test/rails-app/config/routes.rb:2)
test/model_reflections_test.rb:6: warning: toplevel constant ActiveRecord referenced by Reform::Form::ActiveRecord
test/model_reflections_test.rb:7:in `<class:SongForm>': uninitialized constant Reform::Form::ActiveModel::ModelReflections (NameError)
from test/model_reflections_test.rb:5:in `<class:ModelReflectionTest>'
from test/model_reflections_test.rb:4:in `<main>'
and
$ bundle exec ruby -Ilib:test test/mongoid_test.rb
config.eager_load is set to nil. Please update your config/environments/*.rb files accordingly:
* development - set it to false
* test - set it to false (unless you use a tool that preloads your test environment)
* production - set it to true
DEPRECATION WARNING: Using a dynamic :controller segment in a route is deprecated and will be removed in Rails 5.1. (called from block in <top (required)> at /reform_rails/test/rails-app/config/routes.rb:2)
DEPRECATION WARNING: Using a dynamic :action segment in a route is deprecated and will be removed in Rails 5.1. (called from block in <top (required)> at /reform_rails/test/rails-app/config/routes.rb:2)
/.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `require': /.rvm/gems/ruby-2.2.3@reform_rails/gems/mongoid-1.0.6/lib/mongoid/associations/has_many.rb:79: syntax error, unexpected keyword_do_cond, expecting ':' (SyntaxError)
@documents = attributes ? attributes.collect do |attrs|
^
/.rvm/gems/ruby-2.2.3@reform_rails/gems/mongoid-1.0.6/lib/mongoid/associations/has_many.rb:84: syntax error, unexpected ':', expecting keyword_end
end : []
^
/.rvm/gems/ruby-2.2.3@reform_rails/gems/mongoid-1.0.6/lib/mongoid/associations/has_many.rb:99: syntax error, unexpected keyword_do_cond, expecting keyword_end
attributes.values.each do |attrs|
^
/.rvm/gems/ruby-2.2.3@reform_rails/gems/mongoid-1.0.6/lib/mongoid/associations/has_many.rb:139: syntax error, unexpected keyword_end, expecting end-of-input
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `block in require'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:259:in `load_dependency'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `require'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/mongoid-1.0.6/lib/mongoid/associations.rb:5:in `<top (required)>'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `require'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `block in require'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:259:in `load_dependency'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `require'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/mongoid-1.0.6/lib/mongoid.rb:40:in `<top (required)>'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `require'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `block in require'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:259:in `load_dependency'
from /.rvm/gems/ruby-2.2.3@reform_rails/gems/activesupport-5.0.0/lib/active_support/dependencies.rb:293:in `require'
from test/mongoid_test.rb:3:in `mongoid_present?'
from test/mongoid_test.rb:12:in `<main>'
:+1:
I would rather implement that as a module and then make people include it, e.g. Modelless
or something. That way, we can save all the if
s because I don't like if
s in particular. :stuck_out_tongue_winking_eye:
Make sense ;). However have you seen the related discussion on gitter :
Ievgen Kuzminov @iJackUA jul. 15 11:50
@pvmeerbe @apotonick I would agree that there is a confusion when you try to have
form/contract without a Model object. Maybe there should be an additional “feature”/mixin like
NotPersisted to prevent form to syncronisation with underlying model ? That is also helpful with
Operations that do not persist anything, but just calculate smth. or do other thing not directly
related to Model (but still require input params handling and validations).
I think it make sense to also add such a feature @ reform level directly, in which case the new reform-rails Modelless module could be called automatically.
Oh interesting, I did not see that discussion! :+1:
changed it to a separate module. Is this what you had in mind?
I didn't completely split it from the active_model code as I didn't dive into the meaning/pupose of all the code in there, so you have to include both ActiveModel & NotPersisted (see related test).
I guess it could be possible to split it more when using dry-validation, however the used Form builder could still request several active_model API calls...
Perhaps a feature 'NotPersisted' can be added to reform which will automatically
- include this new module
- mark all fields as virtual
Closing and reopening to trigger travis build
@fran-worley as a Repo Member you can start a rebuild via the Travis UI
@timoschilling that only works if a build existed in the first place. I setup travis after these PRs were open and this seemed to be the only way to trigger a build.
ah ok, never mind
What model/ class are you providing the form on initialisation? I'd like to find a more elegant way of doing this if possible, but would need to know a little more about how you would intend to use it.
Can you provide an example form ?
@fran-worley The goal was to use the validation and coercion logic from Reform, but not the persistence functionality. Perhaps in addition a representer could be inferred from the Reform contract
The cases I encountered:
- a form on a customer platform which required to execute some business logic/actions + some REST calls to other internal systems not available to the end customer. No real model is available in the Rails app
- an incoming REST api call requiring some business actions + the result is created by using a Roar representer
The last case is now solved using dry-validation instead of a Reform contract. The first case is currently solved using an OpenStruct as model.
Example of Reform of first case : http://pastebin.com/Fjz3SLwq. This form is feeded with OpenStruct.new as model. The form result is used to launch several REST api calls to an internal system. Each call produces a result object, which is forwarded to the user and not stored locally. I tried to implement this with dry-validation, but encountered problems with mapping possible dry errors on the form. I forgot the exact details...
Approach I'm using:
class BaseForm < Reform::Form
def initialize
super(nil)
end
def persisted?
false
end
end