reform icon indicating copy to clipboard operation
reform copied to clipboard

Dynamic form fields

Open darioghilardi opened this issue 9 years ago • 12 comments

Hello, I am trying to find a good approach to implement a form that is half static (fixed form fields), and half dynamic (form fields added by the application administrator).

I try to explain better the context with an example: Suppose an application that sells digital items to users. To purchase an item, the user should populate a form with his personal details: name, surname, email. The application administrator should be able to define through its administrative panel some custom form fields, different for every item that will be sold. For example, selling an ebook the administrator should be allowed to define a field called Number of books read per months, or Favourite book that is supposed to appear only on the purchase forms for this item. I created a model to store the list of these custom form fields that has a relationship with item, and a jsonb column into the registration model to store the value of this attributes.

Now with Reform I usually define form properties using the property :field_name construct, and validations using validates :field_name, presence: true. This is hard to do in this case because I don't know which attributes I will have in the form so I cannot statically define them in the form object, I only know the attributes at runtime, reading the database. Any hint for the best approach to implement this without hacking custom validations, like I'm doing now? It's really ugly :disappointed:

darioghilardi avatar Sep 28 '15 14:09 darioghilardi

@darioghilardi I have a similar setup to yours (dynamic properties) but I don't use jsonb. I just use standard SQL tables and associations. Then in my operation contracts I use a collection to gather the dynamic properties.

I could put together a gist showing my setup but it is a little complicated as it's not just items + dynamic properties. Let me know if you need more of a hint.

sauy7 avatar Oct 06 '15 20:10 sauy7

@darioghilardi To me it sounds as if you should have different form classes that map to different environments (e.g. Admin or normal user) or is your setup even more convoluted? My point is to have as less ifs as possible in your classes as this will kill any polymorphic techniques. Can you give us some more background, what different states your form(s?) can have?

apotonick avatar Oct 12 '15 00:10 apotonick

I'm facing a similar problem... I'm gonna simplify my use case...

I'm building a web interface to a 3rd party API, and I hit the API and get the available products (computers), and each computer has disk slots, some has 2, 4, 6... so I need to create a form field (select) for each one

a pseudo code would be:

product = Api.find_product_with_id(123)
product.disk_slots
# 2

# options would return `disk1`, `disk2`, `disk3`
product.options do |option|
  form.add_property options.name
end

I was thinking about always having a base reform class then class_eval the properties inside it, but I'm thinking this is too agressive heheh

fernandes avatar Apr 01 '16 19:04 fernandes

Theoretically, it's not a big problem to allow altering the definitions after instantiation, it's a simple hash. This must go into Disposable, though! :sunrise:

apotonick avatar Apr 01 '16 22:04 apotonick

so changing the disposable hash would be a better way then creating a class in runtime, right?

any example? :kissing_heart:

fernandes avatar Apr 01 '16 22:04 fernandes

No, no, you don't need to change or create classes at runtime! Let me play with it a bit. :kissing_heart:

apotonick avatar Apr 01 '16 22:04 apotonick

I am picturing something along the following (inspired by @dblock).

class SongRepresenter < Roar::Decorator # could also be form, maybe different DSL method
  property :id

  before_render do |definitions, options|
    represented.image_versions.each do |v|
      definitions.link "image:#{v.name}"
      # this will require the definitions instance to provide `#link`, which i like. 
    end
  end

  before_parse do

  end
end

References:

  • https://github.com/apotonick/roar/issues/187

apotonick avatar Apr 01 '16 23:04 apotonick

I think the before_render block should execute in context of the representer class, so neither definitions or options should not be passed in.

dblock avatar Apr 03 '16 13:04 dblock

If we did that, it would change the representer class per run, which is not good. I'd rather do that on the instance level!

apotonick avatar Apr 03 '16 22:04 apotonick

@apotonick You're right, so maybe in the context of the instance. It worries me that we're modifying parameters to a function (feels like a side effect).

dblock avatar Apr 04 '16 11:04 dblock

Ah, no, the definitions is the object that holds "the definitions", as in, the property objects (which is thrown away after the run).

apotonick avatar Apr 04 '16 12:04 apotonick

Hey there! I am currently in a similar position. I just found this issue, but have already posted on gitter.im.

Do you remember what the solutions were that you came up with?

:heartbeat:

Here my questions and explanations from gitter.im:

Hi there, magnificent beings! I have am using Reform::Form objects to validate input. I have a JSONB field metadata that I'd like to validate dynamically.

For example:

module Document::Contract
  class UpdateMetadata < Reform::Form
    include Disposable::Twin::Property::Hash

    property(:document_id, validates: { presence: true })
    property(:metadata, field: :hash) do
      # These properties should be selected dynamically depending on the type of the document referenced by `document_id`
      property(:test_metadata1, validates: { presence: true })
      property(:test_metadata2, validates: { presence: true })
    end
  end
end

Can I generate the Reform::Form dynamically upon validation so that I can add properties/validations depending on the given document_id ?

It would make my week, if that was something, that's feasible. :-)

And:

Ah, I forgot to mention that this cannot be easily solved by any polymorphism or switching between two static Reform::Form classes.

The properties should be defined by users for a dynamic set of document types (e.g. DocumentType has_many :property_definitions which define name, type and basic validations as string length, regex format etc.)

So I would have to iterate over these when I found my Document and the respective DocumentType. But I haven't found a way to hook that into the UpdateMetadata contract.

And:

@brindu Hey, thanks for taking the time. Unfortunately skip_if does not work.

Maybe this explains my case a little better.. I imagine something like this:

property(:metadata, field: :hash) do
  # These properties should be selected dynamically depending on the type of the document referenced by `document_id`
  Document.find(document_id).type.fields.each do |field|
    property(field.key, type: field.type)
    if field.is_mandatory
      validates field.key, presence: true
    end

    if field.max_length
      validates field.key, length: { max: field.max_length }
    end
  end
end

Unfortunately I do not have access to the document_id at the time the class is defined. So I would have to put the contract definition into something Proc-like.

I thought about something like:

document = Document.find(document_id)
Class.new(Reform::Form) do
  document.type.fields.each do |field|
    property(field.key, type: field.type)
    if field.is_mandatory
      validates field.key, presence: true
    end

    if field.max_length
      validates field.key, length: { max: field.max_length }
    end
  end
end

But that would define an unnamed class every time an operation uses the contract... I wonder whether there is a better way to adjust the schema of a contract upon runtime (optimally in a separate validation group or something).

Cheers! :beers:

leoc avatar Oct 24 '18 10:10 leoc