Bug #7450
closedWe mark not-required fields as required
Description
If the validator has some condition (:if => $smt, :unless => $smt) we shouldn't mark the field as required. In this case we simply can't tell without AJAX call because validity may depend on another field of the form. Btw there was a project translating AR validations to javascript (client side validations) but IIRC it's discontinued. Also it was quite big thing.
Updated by Marek Hulán over 10 years ago
- Related to Feature #6400: mark required fields in form added
Updated by Dominic Cleal over 10 years ago
Do you have an example field where this is wrong? There's an override option which can be passed into the form helper to disable it, we just need to know which field(s) require it :)
Updated by Marek Hulán over 10 years ago
I'm afraid we can just enforce required to be true. Looking to the code I see
required_mark = ' *' if is_required?(f, attr) || options[:required]
see https://github.com/theforeman/foreman/blob/develop/app/helpers/layout_helper.rb#L153 and https://github.com/theforeman/foreman/blob/develop/app/helpers/layout_helper.rb#L167 for more details.
I have no existing example since it's in form I'm currently working on for bonding.
something like
<%= text_f f, :attached_to, :help_inline => _("Identifier of the interface to which this interface belongs, e.g. eth1") %>
adding :required => false does not help obviously since is_required?(f, attr) is always true
Updated by Dominic Cleal over 10 years ago
Indeed, that should use options[:required] unless it's nil, and only then fall back to is_required.
Updated by Marek Hulán over 10 years ago
Yup, will send a PR shortly. I thought it would be also useful to make is_required?
aware of conditions and if some is present, return false (probably makes sense in most cases?). Thoughts?
Updated by The Foreman Bot over 10 years ago
- Status changed from New to Ready For Testing
- Target version set to 1.7.3
- Pull request https://github.com/theforeman/foreman/pull/1766 added
- Pull request deleted (
)
Updated by Marek Hulán over 10 years ago
- Status changed from Ready For Testing to Closed
- % Done changed from 0 to 100
Applied in changeset 3b87444aac04cf78e073bcb6e7f7778ef72f9c8e.
Updated by Dominic Cleal over 10 years ago
- Translation missing: en.field_release set to 21
Updated by Dominic Cleal over 10 years ago
- Related to Bug #7560: add :required => true on host fields that have conditional validation if host.managed? added
Updated by Marek Hulán about 7 years ago
- Related to Feature #21678: Mark required fields conditionally based on other fields added