Fork me on GitHub
#fulcro
<
2023-05-31
>
Jakub Holý (HolyJak)14:05:48

Do I understand correctly that form validation in RAD isn't recursive, ie only the top level form's attributes are checked (including edges to subforms, excluding attributes of these subforms)? What if I want all my forms to be valid before save is attempted? Ie. when creating an Order, I want to be sure not only that it has >0 order lines but also that each line has pos? :count

tony.kay16:05:04

There is only one state machine, and it is on the top-level form. It is responsible for all layers of validation, so that top-level form’s validator must cover all subforms as well.

Jakub Holý (HolyJak)10:06:54

> that top-level form’s validator must cover all subforms as well. I guess that is the problem, no? The default RAD form validator, if I read it correctly, only checks the attributes of the top-level form: https://github.com/fulcrologic/fulcro-rad/blob/462d749cd36553925d9c6f0673590b0c5b0d9f81/src/main/com/fulcrologic/rad/form.cljc#L460 Is that correct & intended? If I mirror what I believe RAD is doing, I will get this validator

(def valfn (attr/make-attribute-validator [r.order/type
                                             r.order/customer
                                             r.order/order-lines] true))
which claims my form is valid, even though the order-line is missing both its required attributes. If I add those attributes manually to the validator like this:
(def valfn (attr/make-attribute-validator [r.order/type
                                             r.order/customer
                                             r.order/order-lines

                                             r.order-line/product
                                             r.order-line/package-count] true))
then it correctly recongizes the form as invalid. So my question is: Given a form, what is the simplest way to construct a validator for the form including all subforms? Preferably without having to manually collect attributes from each one. And why isn’t this - ie validating also subform’s attributes - the default? Thank you!!!

tony.kay18:06:51

You are right that the built-in validator only checks the top-level form.

tony.kay18:06:23

Here’s the thing: I don’t know if you want to validate edges, validate sub-elements, etc. I guess the assumption should be that you want to validate it all. So, at present the code needs expansion for the “default” validator to do all of that. I just make a single global validator using all-attributes of the RAD model and explicitly set that on my forms. A subform could be to-one and be a picker, but I guess in that case there would be no fo/subforms entry. If you want to write a patch that tries to walk the form set using the component options and looks for the subforms recursively and collects all of the attributes I think that would be ok.

tony.kay19:06:17

So, I patched this in 1.5.1-SNAPSHOT

tony.kay19:06:27

see if you like that better

tony.kay19:06:10

Thanks for noticing…I got used to just using my global one and never re-addressed it

Jakub Holý (HolyJak)21:06:58

:star-struck: This looks awesome, thanks a lot. I am trying it now but somehow it does not seem to make a difference. I suppose I messed up my environment and will keep trying 🙂 It works as expected from the REPL, when I run ((attr/make-attribute-validator (form/form-and-subform-attributes my.order-forms/OrderLineForm) true) my-captured-form-props) - it returns :invalid where before it returned valid. But in the app it still tries to save the form. In the console log I see

Form  my.order-forms/OrderForm  valid?  false
Form  my.order-forms/OrderForm  dirty?  true
:order/order-lines is not marked complete
:order/order-lines is not marked complete
:order-line/package-count is not marked complete
:order-line/product is not marked complete
Form is not valid because required attribute is missing: :order-line/product
Form  my.order-forms/OrderLineForm  valid?  false
...
Trigger [:order/id …#fulcro/tempid["#uuid "5cca09cf-0afe-4a03-8af6-fd82cb67066c""]TempId] :event/save
Activating state  :state/saving
i.e. I would expect the save to have failed, replacing the last line with
Form is not valid because <some reason>
Activating state  :state/editing

Jakub Holý (HolyJak)21:06:26

I guess it does not fail b/c it skips checking the two :order-line/ attributes because the are not marked complete. That’s why com.fulcrologic.rad.form/valid? explicitly checks all-required-present? , but this check does not include subform’s attributes. Correct?

tony.kay23:06:42

You're saying that fields that are only marked required, but that have no validator function themselves are a problem? Yeah, I didn't look at those, I just tried it where attributes had a valid? function

Jakub Holý (HolyJak)07:06:38

One of them (package-count) has valid? , the other (product) does not, and both have required? In this case, valid? is not even triggered I think b/c I have added a new order line to the order but have not touched any of its fields, so they are both nil/missing. That’s why the are both “no marked complete” as logged above.

tony.kay16:06:18

Have you tried using the form debugger from the debug namespace?

tony.kay16:06:50

Save really should not trigger unless everything is checked... So sounds like a bug somewhere

tony.kay16:06:47

I tried it on the demo yesterday and it was working for me with the same structure you are describing on invoice

Jakub Holý (HolyJak)19:06:24

I am not sure what I do wrong. My OrderForm is marked as valid. When I look at its proposed-form-props from the :event/save there are some weird things:

::fs/config {
  fs/complete? #{:order/type :order/customer :order/order-lines}
  fs/fields #{:order/type} ; I guess ok since both customer and order-lines are subforms, not simple fields?
  fs/subforms {:order/customer {}, :order/order-lines {}} ; as expected
}
:order/order-lines {
  :order-line/id #fulcro/tempid[..] {
    fs/config {
      fs/complete? #{:order-line/package-count :order-line/product}
      fs/fields #{:order-line/package-count}
      fs/subforms {:order-line/product {}}
    }
    :order-line/id ... ; NO OTHER order-line property, neither count (expected, since unset?) nor the product ref
}}
So both package-count and product are marked as complete, and both have ao/required? true, yet neither has any value and does not fail the validation?! I have
defsc-form OrderLineForm 
  fo/attributes [r.order-line/product r.order-line/package-count]
  fo/field-styles {:order-line/product :pick-one}
  ...

defsc-form OrderForm
  fo/attributes [r.order/type r.order/customer r.order/order-lines]
  fo/field-styles {:order/customer :pick-one}
  fo/subforms {:order/order-lines {fo/ui OrderLineForm, ...}}
  ...

; with the key attributes
(defattr product :order-line/product :ref {ao/required? true, ...})
(defattr package-count :order-line/package-count :int {ao/required? true, ao/valid? (fn [value _ _] (some-> value pos?)), ...})
(defattr order-lines :order/order-lines :ref {ao/required? true, ao/cardinality :many, ...})
Any idea what could be the problem? 🙏🙏🙏 package-count has a valid? fn so it should have been invoked (b/c the field is marked as complete) and have failed, no? I am not sure how validation of product should have worked. I assume there is a reason that form/valid? has an explicit check for all-required-present? but this one only checks, I believe the top form’s attributes, not those of any subform.

Jakub Holý (HolyJak)20:06:33

Interestingly, if I copy these very same captured props I explore above and try to validate them, I do get the expected invalid:

(def form-props {::fs/config {..} ...})
(def valfn (attr/make-attribute-validator (form/form-and-subform-attributes my/OrderForm) true))
(valfn form-props)
; => :invalid
I have no idea why the almost same [removed some asm-id, cache options maps] data would produce different result in the app and in the clojure REPL 🤯

tony.kay04:06:06

If you change your RAD attributes and use hot code reload, then the validator might still have the OLD version of the attributes. Try stopping and starting the compiler, and reloading the page with the cache disabled to make sure you’re not getting stale things.

👍 2
tony.kay16:05:40

I.e. Your validator on the top-level form will be called on all fields even in subforms.

tony.kay16:05:13

validation is easy to compose, and usually easy to make from the RAD model itself.

Jakub Holý (HolyJak)21:05:12

Ok, so I need to dig into why it doesn't work then

Jakub Holý (HolyJak)10:06:54

> that top-level form’s validator must cover all subforms as well. I guess that is the problem, no? The default RAD form validator, if I read it correctly, only checks the attributes of the top-level form: https://github.com/fulcrologic/fulcro-rad/blob/462d749cd36553925d9c6f0673590b0c5b0d9f81/src/main/com/fulcrologic/rad/form.cljc#L460 Is that correct & intended? If I mirror what I believe RAD is doing, I will get this validator

(def valfn (attr/make-attribute-validator [r.order/type
                                             r.order/customer
                                             r.order/order-lines] true))
which claims my form is valid, even though the order-line is missing both its required attributes. If I add those attributes manually to the validator like this:
(def valfn (attr/make-attribute-validator [r.order/type
                                             r.order/customer
                                             r.order/order-lines

                                             r.order-line/product
                                             r.order-line/package-count] true))
then it correctly recongizes the form as invalid. So my question is: Given a form, what is the simplest way to construct a validator for the form including all subforms? Preferably without having to manually collect attributes from each one. And why isn’t this - ie validating also subform’s attributes - the default? Thank you!!!