Add support for mocking a form builder objects

[drakmail]

As a @tombeynon commented in a #26 it's not possible to generate Styleguides for components that need a form builder instance. In this PR I try to implement a possible solution for that issue. Now it's possible to pass in stubs config an ActiveRecord based class which will be used for mocking a form_builder need for component.

For example: if a component need a form parameter (which is must be an instance of Form Builder for, for example, Something model), now it's possible to mock it in stubs file like this:

:meta: 'information about the component button'
:stubs:
  -
    :text: "Hello"
    :form:
      !ruby/object:Something
        attributes:
          item: "test" # also you could pass a model attributes

[houndci-bot]

Line is too long. [100/80]

[devnacho]

@drakmail thank you very much for the contribution!

Will be checking the PR soon.

[kitop]

Hi @drakmail, thanks for your contribution!

I'm just curious if checking for ActiveRecord::Base when creating a form may hurt other issues. Maybe sometimes you want to pass an ActiveRecord object but not to use a form, only to display it's values.
Would love to hear you thoughts on that.

[drakmail]

@kitop Yes, I'm agree that it's a weak spot of current solution. There are an alternative way: create an internal proxy object for usage with forms (something like MountainView::FormBuilder) which will be just a mock for object. So it could receive any data and could be used like an ActiveRecord::Base based object. With this solution it will be possible to pass ActiveRecord models as is and use a mock objects for form builders.

But there is could be a problem with forms that depends on a model methods, which couldn't be mocked by MountainView::FormBuilder.

Hm, I get a good idea – maybe it's possible to use a convention for form builder names like SomethingFormBuilder... or maybe just pass a sibling with attributes hash that contains some meta information which need to be builded – AR object or a form builder object.

Thanks for a good question, tomorrow I'll read a Psych gem sources more thoroughly and try to upgrade solution if it's possible.

[kitop]

@drakmail thanks for your time!
Yes, it's not a simple problem, there will always be edge cases and caveats. Forms are a bit more complex than regular ruby objects.

Can I be curious and ask if you are using MountainView on a production app?

[drakmail]

@kitop Yes, our company making a new app and we are decided to use MountainView as a foundation for our frontend because it fits really well in our workflow.

PS. We are using rails 5 and MountainView doesn't have any compatibility problems with it :-)

[kitop]

@drakmail awesome!!! Glad to hear that :)

[tombeynon]

Hey guys - I've been thinking about this recently so thought I'd weigh in; feel free to ignore any or all of this!

Ultimately I don't think trying to mock a formbuilder is going to work for every edge case. As @kitop says, there's just too many edge cases and they can be really complex. I do think you're on the right lines with your ruby/object attribute in the stub; MountainView could use that attribute to build an instance, and pass the rest of the attributes defined in the stub as arguments to the new method. I think that would cover some simple use cases, but I do think we need to take it a step further...

There would still be many edge cases presented; one off the top of my head is how you would pass a persisted ActiveRecord object since we would need to use .find, or conversely, pass a form builder. In this instance, could we use something like a ComponentStub class? Similar to how we lookup the component class from my PRs, if a specific Stub class (e.g. CardStub) existed, we could expect it to respond to a properties method which returns everything needed for a stub for that component. This would then give us a LOT more flexibility; AR objects could be looked up, passed to a FormBuilder etc etc.

I've kind of typed this as I was thinking it, so it may not make any sense, but let me know what you guys think!

[drakmail]

@tombeynon Nice idea, but seems that I found a simpler solution (as I think 😄):

I want to add MountainView::Helpers module with FormFor class, which could be used to mock form builder for a specific model with a specific attributes. Example of something_component.yml:

:meta: 'There is a class with form builder object'
:stubs:
  -
    :id: 1
    :form:
      !ruby/object:MountainView::Helpers::FormBuilder # generates form builder stub with passed `Something` model
        model: !ruby/object:Something
          attributes:
            name: 'something name'

And now it's possible to pass any ruby objects (including AR models) without any conflicts with form builders. For example, I could pass an any AR model with preinitialized fields like this:

:meta: 'There is a class with form builder object'
:stubs:
  -
    :id: 1
    :some_model: !ruby/object:Something
      attributes:
        id: 14
        name: 'blabla'

[tombeynon]

@drakmail I guess my only problem with that is that you've had to create a stub class for a FormBuilder. That's great for this instance, but what if you're not working with forms, but some other complex object? My problem with only using the YAML file is that we can't use any ruby code to build the stub, and that will eventually restrict us. Using the route you've suggested I expect a LOT of work would end up going into creating stub classes for all the different use cases. What do you think?

[drakmail]

@tombeynon hm, but with !ruby/object:SOME_CLASS_NAME it's possible to initialize event custom ComponentStub without any modifications (if SOME_CLASS_NAME is visible to application). In ComponentStub you can define a def init_with(coder) method which will accept any passed parameters from yaml hash. So with that solution it's possible to:

1. Pass arbitrary PORO objects
2. Pass form builders preinitialized with any AR model (it also may be crafted by hand, but it's will be easier with helper class)
3. Pass any AR objects with attributes initialized with instance_variable_set
4. Implement custom ComponentStub which could implement more complex initialization logic with implemented #init_with(coder) method.

[tombeynon]

I think I follow; I totally agree that !ruby/object:SOME_CLASS_NAME works very well for PORO objects, I agree that's a nice solution.

:meta: 'There is a class with form builder object'
:stubs:
  -
    :id: 1
    :some_model: !ruby/object:Something
      attributes:
        id: 14
        name: 'blabla'

render_component 'whatever', {id: 1, some_model: Something.new({id: 14, name: 'blabla'})}

I just think instead of going down the route of providing MountainView::Helpers::FormBuilder and similar for specific use cases, that we instead go down a route where the user can define a more complex stub. Something like..

:meta: 'There is a class with custom stub object'
:stubs:
  -
    :id: 1
    :some_model:
      attributes:
        id: 14

class WhateverStub < MountainView::Stub
  include ActionView::Helpers::FormHelper

  def initialize(properties={}) # this would be defined in MountainView::Stub
    @properties = properties
  end

  def stubs
    {
      id: 1,
      form: form
    }
  end

  def form
    form_for(some_model)
  end

  def some_model
    Something.find(properties[:some_model][:attributes][:id])
  end    
end

render_component 'whatever', {id: 1, form: form}

Thinking out loud still, looking forward to your thoughts.

[drakmail]

@tombeynon oh, I got it. The idea is very close to what I implemented. But seems that WhateverStub is partially duplicates functionality of a WhateverComponent (Presenter class).

But I doesn't see any disadvantages for component stubbing with providing custom stub class, preinitialized AR model or form builder stub like that (part of README.md for new functionality):

Setting up the style guide

1. Add the following line to your routes.rb file.

mount MountainView::Engine => "/mountain_view"

2. Create stubs for your components. These stubs will be the examples in the style guide.

E.g: app/components/card/card.yml

-
  :title: "Aspen Snowmass"
  :description: "Aspen Snowmass is a winter resort complex located in Pitkin County in western Colorado in the United States. Owned and operated by the Aspen Skiing Company it comprises four skiing/snowboarding areas on four adjacent mountains in the vicinity of the towns of Aspen and Snowmass Village."
  :link: "http://google.com"
  :image_url: "http://i.imgur.com/QzuIJTo.jpg"
  :data:
  -
    :title: "Elevation"
    :number: '7879ft'
  -
    :title: "Depth"
    :number: '71"'
-
  :title: "Sunset on the Mountain"
  :description: "Three major ranges of the Alps – the Northern Calcareous Alps, Central Alps, and Southern Calcareous Alps – run west to east through Austria. The Central Alps, which consist largely of a granite base, are the largest and highest ranges in Austria."
  :link: "http://google.com"

If your component depends on a form builder object you can use the following statement:

:meta: 'There is a class with form builder object'
:stubs:
  -
    :form:
      !ruby/object:MountainView::Helpers::FormBuilder
        model: !ruby/object:Something
          attributes:
            name: 'something name'

Also you can stub any ruby class with the following method:

:meta: 'There is a class with an AR object'
:stubs:
  -
    :some_model: !ruby/object:Something
      attributes:
        name: 'blabla'

For any complex logic you can define a custom Compoenent Stub with init_with(coder) method which will receive a #map with attributes loaded from yaml file:

:meta: 'There is a class with a complex logic'
:stubs:
  -
    :some_model: !ruby/object:ComponentStub
      params:
        - 'blabla'
        - 'nothing'
      name: 'Some Name'

class ComponentStub < SomeModel # SomeModel could be AR class
  def init_with(coder)
    param1 = coder.map["params"].first
    somename = coder.map["name"]
    # or `@properties = coder.map`
    initialize(param1, somename) # call initializer of SomeModel
  end
end

[houndci-bot]

Line is too long. [90/80]

[kitop]

@tombeynon thanks for joining the discussion! Very helpful additions :)

@drakmail thanks for updating the PR.
As Tom mentioned, using !ruby/object definitively sounds like the right path, but there's still quite some work. Having a FormHelper is good, but still looks kinda weird to me.

Something else to look for is how the styleguide is displayed after this change:

Right now it displays "#<Something:0x007fd681a403d0>" as the value, which gives a hint it's a Something object, but may need more info.
Also, for forms, it displays "#<MountainView::Helpers::FormBuilder:0x007fd681a5fe38>", as we own that object, we can definitively do something better there, via the inspect method. Maybe something like displaying the form class with the attributes?

Having a MountainView::Stub can really help with that, by having a to_json method or similar we can override.

Thoughts?

[drakmail]

@kitop Thanks for feedback! I agree that FormBuilder helper looks a little weird, but seems that there are only way to mock form builder generated by form_for helper.

I think it's possible to override displaying of complex objects, it's very good idea. I think, it basically could be something like

{
  id: 1,
  some_model: Something.new(color: :red, kind: "default")
}

I'll try to implement it today

[drakmail]

I did a small research and found that it's possible to add custom domain models for Psych parser. So, now it's possible to change syntax to this:

:meta: 'There is a class with form builder object'
:stubs:
  -
    :id: 1
    :some_model: !mountain_view:Model # declaration with domain (`mountain_view`) inclusion
      class: Something # it's possible to use any custom DSL here, i think usage of `class` and `attributes` is a good variant
      attributes:
        name: 'blabla'
    :some_another_model: !Model # it's also possible to use shorter syntax
      class: Something
      attributes:
        name: 'blabla'
    :form:
      !Form # It's a form builder declaration
        for: !Model  # for model defined in DSL that was introduced early, best part that it's possible (potentially) any custom params to form for
          class: Something
          attributes:
            name: 'something name'

This approach allows to define custom wrappers for models (and POROs) for generating pretty to_json output.

PS. Maybe better name object declaration not a !Model but something like !Object.
PPS. Source of Psych with add_domain_type: https://github.com/tenderlove/psych/blob/master/lib/psych.rb#L476 , usage example in Psych tests: https://github.com/tenderlove/psych/blob/master/test/psych/test_yaml.rb#L610

[houndci-bot]

Unused block argument - k. If it's necessary, use _ or _k as an argument name to indicate that it won't be used.
Line is too long. [110/80]

[drakmail]

I'm updated PR to use new syntax for objects and forms. Example of _component.yml file:

:meta: 'information about the component button' # Optional
:stubs: # As many as you need
  -
    :title: "Small danger button"
    :size: "small"
    :color: "dng"
    :user: !Object
      class: User::Corporate
      attributes:
        email: "[email protected]"
    :form: !Form
      for: !Object
        class: User
        attributes:
          email: "[email protected]"

Generated Styleguide:

[houndci-bot]

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

[robcole]

Hey guys,

My team is brand-new to MountainView (ps - ❤️ - this is an amazing gem) - but this is a problem we're very interested in learning more about recommended solutions for. We're using it quite a bit with SimpleForm to create reusable form components, and don't see any clean paths forward for mocking the form builders.

Is this something that's actively being pursued still, or is it more or less abandoned at this point? With the last commits on master showing as over a year ago, just want to make sure we're looking in the right direction.

Rob

[kitop]

Hi @robcole thanks for you nice words!

This is not abandoned at all, but it is not a trivial problem to solve and we'd like to arrive to a nice solution to it. Any feedback, comments, tips, etc, are more than welcomed!

Having said that, yesterday we released a new version with a feature that may help with this: #49 (shoutout to @MikeRogers0 for the amazing work!). With this, you may be able to pass the form in a block to a component, and stub it (or at least a placeholder) in the styleguide view with the yield property.

Does that help? Happy to discuss and keep looking for a nice way to solve this issue.

[robcole]

@kitop Thanks for the response! I'll start digging into the block-based approach and see if I can find some clarity for how our team handles this, and (possibly) see if I can help contribute to a more generalizable solution. Just wanted to make sure this PR was still active as a "we intend solve this hard problem someday when we figure out a solution we like" sort of PR, since the discussion was quite old.

[kitop]

@robcole thanks for understanding!!
Please give it a try, and let us know how it goes. Also, feel free to share code fragments if you wish, having real world examples helps a lot.
I'm not using forms with MV currently, so I'm not very familiar with the problem. But I'm more than willing happy to help though, as it makes a lot of sense to have it work with forms.

[robcole]

@kitop I've read through the PR that brings in block support for components as well as the discussion for how stubs might work using it, but didn't see any clear resolution in the PR or in the current docs for how you might stub an object that's being passed to yield. Let me know if you have any additional info that might clarify what you were thinking / how you suggest working with that.

Thanks!

[MikeRogers0]

@robcole We didn't come to a resolution, what I've been doing in my current projects styleguide is:

:stubs:
  - 
    :id: "ExampleModal"
    :title: "A Title"
    :subtitle: "The Subtitle"
    :yield: "&block - You can pass a block to this component."

I'm all ears for a better solution though!

[kitop]

@robcole as @MikeRogers0 said, we didn't come to a resolution when mocking yields.
I think it's worth adding the block support as a first step though, and then we can come up with a better way to support it in the actual stubs.

You can either do as @MikeRogers0 suggested, or also use the meta field for better clarification on what the block expects.

Any feedback will be greatly appreciated!